linux-kernel - Re: [PATCH v4 3/8] i2c: mediatek: Reset the handshake signal between i2c and dma

Open Source and information security mailing list archives

Message-ID: <fce8934ec25d18307df11e2f308a255da5a25ce3.camel@mediatek.com>
Date:   Sat, 21 Aug 2021 15:40:45 +0800
From:   Kewei Xu <kewei.xu@...iatek.com>
To:     Chen-Yu Tsai <wenst@...omium.org>
CC:     <wsa@...-dreams.de>, Matthias Brugger <matthias.bgg@...il.com>,
        "Rob Herring" <robh+dt@...nel.org>, <linux-i2c@...r.kernel.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        srv_heupstream <srv_heupstream@...iatek.com>,
        <leilk.liu@...iatek.com>, <qii.wang@...iatek.com>,
        <yuhan.wei@...iatek.com>, <ot_daolong.zhu@...iatek.com>,
        <liguo.zhang@...iatek.com>
Subject: Re: [PATCH v4 3/8] i2c: mediatek: Reset the handshake signal
 between i2c and dma

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@...iatek.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@...iatek.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@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives