| Message ID | 1626517079-9057-4-git-send-email-kewei.xu@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce an attribute to choose timing setting | expand |
Hi, On Sat, Jul 17, 2021 at 6:29 PM Kewei Xu <kewei.xu@mediatek.com> wrote: > > Due to changes in the hardware design of the handshaking signal > between i2c and dma, it is necessary to reset the handshaking > signal before each transfer to ensure that the multi-msgs can > be transferred correctly. This also affects MT8192. Has this been tested on that SoC as well? > Signed-off-by: Kewei Xu <kewei.xu@mediatek.com> > --- > drivers/i2c/busses/i2c-mt65xx.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c > index 222ff765e55d..c0108387f34b 100644 > --- a/drivers/i2c/busses/i2c-mt65xx.c > +++ b/drivers/i2c/busses/i2c-mt65xx.c > @@ -47,6 +47,9 @@ > #define I2C_RD_TRANAC_VALUE 0x0001 > #define I2C_SCL_MIS_COMP_VALUE 0x0000 > #define I2C_CHN_CLR_FLAG 0x0000 > +#define I2C_CLR_DEBUGCTR 0x0000 > +#define I2C_RELIABILITY 0x0010 > +#define I2C_DMAACK_ENABLE 0x0008 > > #define I2C_DMA_CON_TX 0x0000 > #define I2C_DMA_CON_RX 0x0001 > @@ -850,6 +853,17 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, > > reinit_completion(&i2c->msg_complete); > > + if (i2c->dev_comp->apdma_sync) { > + mtk_i2c_writew(i2c, I2C_CLR_DEBUGCTR, OFFSET_DEBUGCTRL); > + writel(I2C_DMA_HANDSHAKE_RST | I2C_DMA_WARM_RST, > + i2c->pdmabase + OFFSET_RST); > + writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_RST); I2C_DMA_WARM_RST is self-clearing. Is I2C_DMA_HANDSHAKE_RST not self-clearing? If both are self-clearing, don't you need to wait and check for them to cleared? If they aren't self-clearing, do you need to delay some time for them to complete? > + mtk_i2c_writew(i2c, I2C_HANDSHAKE_RST, OFFSET_SOFTRESET); > + mtk_i2c_writew(i2c, I2C_CHN_CLR_FLAG, OFFSET_SOFTRESET); Same here. No time delay needed? > + mtk_i2c_writew(i2c, I2C_RELIABILITY | I2C_DMAACK_ENABLE, > + OFFSET_DEBUGCTRL); A comment explaining what the section above does would be nice. AFAICU this is force resetting the DMA handling. Regards ChenYu > + } > + > control_reg = mtk_i2c_readw(i2c, OFFSET_CONTROL) & > ~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS); > if ((i2c->speed_hz > I2C_MAX_FAST_MODE_PLUS_FREQ) || (left_num >= 1)) > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Wed, 2021-08-11 at 16:41 +0800, Chen-Yu Tsai wrote: > Hi, > > On Sat, Jul 17, 2021 at 6:29 PM Kewei Xu <kewei.xu@mediatek.com> > wrote: > > > > Due to changes in the hardware design of the handshaking signal > > between i2c and dma, it is necessary to reset the handshaking > > signal before each transfer to ensure that the multi-msgs can > > be transferred correctly. > > This also affects MT8192. Has this been tested on that SoC as well? Yes, It has been tested on MT8192. > > > Signed-off-by: Kewei Xu <kewei.xu@mediatek.com> > > --- > > drivers/i2c/busses/i2c-mt65xx.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/i2c/busses/i2c-mt65xx.c > > b/drivers/i2c/busses/i2c-mt65xx.c > > index 222ff765e55d..c0108387f34b 100644 > > --- a/drivers/i2c/busses/i2c-mt65xx.c > > +++ b/drivers/i2c/busses/i2c-mt65xx.c > > @@ -47,6 +47,9 @@ > > #define I2C_RD_TRANAC_VALUE 0x0001 > > #define I2C_SCL_MIS_COMP_VALUE 0x0000 > > #define I2C_CHN_CLR_FLAG 0x0000 > > +#define I2C_CLR_DEBUGCTR 0x0000 > > +#define I2C_RELIABILITY 0x0010 > > +#define I2C_DMAACK_ENABLE 0x0008 > > > > #define I2C_DMA_CON_TX 0x0000 > > #define I2C_DMA_CON_RX 0x0001 > > @@ -850,6 +853,17 @@ static int mtk_i2c_do_transfer(struct mtk_i2c > > *i2c, struct i2c_msg *msgs, > > > > reinit_completion(&i2c->msg_complete); > > > > + if (i2c->dev_comp->apdma_sync) { > > + mtk_i2c_writew(i2c, I2C_CLR_DEBUGCTR, > > OFFSET_DEBUGCTRL); > > + writel(I2C_DMA_HANDSHAKE_RST | I2C_DMA_WARM_RST, > > + i2c->pdmabase + OFFSET_RST); > > + writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + > > OFFSET_RST); > > I2C_DMA_WARM_RST is self-clearing. Is I2C_DMA_HANDSHAKE_RST not > self-clearing? If both are self-clearing, don't you need to wait and > check for them to cleared? If they aren't self-clearing, do you need > to delay some time for them to complete? > Thank you for your suggestion. We will appropriately add the delay in the next version of patch according to the hardware behavior characteristics. Regards Kewei > > + mtk_i2c_writew(i2c, I2C_HANDSHAKE_RST, > > OFFSET_SOFTRESET); > > + mtk_i2c_writew(i2c, I2C_CHN_CLR_FLAG, > > OFFSET_SOFTRESET); > > Same here. No time delay needed? > > > + mtk_i2c_writew(i2c, I2C_RELIABILITY | > > I2C_DMAACK_ENABLE, > > + OFFSET_DEBUGCTRL); > > A comment explaining what the section above does would be nice. > AFAICU > this is force resetting the DMA handling. > > > Regards > ChenYu > > > + } > > + > > control_reg = mtk_i2c_readw(i2c, OFFSET_CONTROL) & > > ~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS); > > if ((i2c->speed_hz > I2C_MAX_FAST_MODE_PLUS_FREQ) || > > (left_num >= 1)) > > -- > > 2.18.0 > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c index 222ff765e55d..c0108387f34b 100644 --- a/drivers/i2c/busses/i2c-mt65xx.c +++ b/drivers/i2c/busses/i2c-mt65xx.c @@ -47,6 +47,9 @@ #define I2C_RD_TRANAC_VALUE 0x0001 #define I2C_SCL_MIS_COMP_VALUE 0x0000 #define I2C_CHN_CLR_FLAG 0x0000 +#define I2C_CLR_DEBUGCTR 0x0000 +#define I2C_RELIABILITY 0x0010 +#define I2C_DMAACK_ENABLE 0x0008 #define I2C_DMA_CON_TX 0x0000 #define I2C_DMA_CON_RX 0x0001 @@ -850,6 +853,17 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, reinit_completion(&i2c->msg_complete); + if (i2c->dev_comp->apdma_sync) { + mtk_i2c_writew(i2c, I2C_CLR_DEBUGCTR, OFFSET_DEBUGCTRL); + writel(I2C_DMA_HANDSHAKE_RST | I2C_DMA_WARM_RST, + i2c->pdmabase + OFFSET_RST); + writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_RST); + mtk_i2c_writew(i2c, I2C_HANDSHAKE_RST, OFFSET_SOFTRESET); + mtk_i2c_writew(i2c, I2C_CHN_CLR_FLAG, OFFSET_SOFTRESET); + mtk_i2c_writew(i2c, I2C_RELIABILITY | I2C_DMAACK_ENABLE, + OFFSET_DEBUGCTRL); + } + control_reg = mtk_i2c_readw(i2c, OFFSET_CONTROL) & ~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS); if ((i2c->speed_hz > I2C_MAX_FAST_MODE_PLUS_FREQ) || (left_num >= 1))
Due to changes in the hardware design of the handshaking signal between i2c and dma, it is necessary to reset the handshaking signal before each transfer to ensure that the multi-msgs can be transferred correctly. Signed-off-by: Kewei Xu <kewei.xu@mediatek.com> --- drivers/i2c/busses/i2c-mt65xx.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)