HI, Thank you for taking the time to review the patch and considering it. Thanks a bunch! ------------------------------------------ Best Regards Kimriver Liu > -----Original Message----- > From: Andi Shyti <andi.shyti@xxxxxxxxxx> > Sent: 2024年9月13日 16:53 > To: Liu Kimriver/刘金河 <kimriver.liu@xxxxxxxxxxxx> > Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>; jarkko.nikula@xxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; jsd@xxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux-> kernel@xxxxxxxxxxxxxxx; Andy Shevchenko <andy@xxxxxxxxxx> > Subject: Re: [PATCH v11] i2c: designware: fix controller is holding SCL low while ENABLE bit is disabled > Hi Kimriver, > please, don't send v12 anymore, I will take care of these little notes from Andy. > You did a great job at following up on all the reviews, thanks! > Andi > On Fri, Sep 13, 2024 at 08:31:18AM GMT, Liu Kimriver/刘金河 wrote: > > Hi Andy >> >> Subject: [PATCH v11] i2c: designware: fix controller is holding SCL low while ENABLE bit is disabled >> I will change the subject to： >> Subject: [PATCH v11] i2c: designware: fix controller is holding SCL >> low while the ENABLE bit is disabled >> > > >-----Original Message----- >> >From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > >Sent: 2024年9月13日 15:41 > > >To: Liu Kimriver/刘金河 <kimriver.liu@xxxxxxxxxxxx> > > >Cc: jarkko.nikula@xxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; > > >mika.westerberg@xxxxxxxxxxxxxxx; jsd@xxxxxxxxxxxx; > > >andi.shyti@xxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; > > >linux-kernel@xxxxxxxxxxxxxxx; Andy >Shevchenko <andy@xxxxxxxxxx> >> >Subject: Re: [PATCH v11] i2c: designware: fix controller is holding >> >SCL low while ENABLE bit is disabled >> >> >On Fri, Sep 13, 2024 at 6:35 AM Kimriver Liu <kimriver.liu@xxxxxxxxxxxx> wrote: > >>> > >>> It was observed that issuing the ABORT bit (IC_ENABLE[1]) will not > >>>work when IC_ENABLE is already disabled. > >>> > >>> Check if the ENABLE bit (IC_ENABLE[0]) is disabled when the > >>> controller is holding SCL low. If the ENABLE bit is disabled, the > >>> software needs to enable it before trying to issue the ABORT bit. > >>> otherwise, the controller ignores any write to ABORT bit. > >>> > >>>These kernel logs show up whenever an I2C transaction is attempted > >>> after this failure. > >>> i2c_designware e95e0000.i2c: timeout waiting for bus ready > >>> i2c_designware e95e0000.i2c: timeout in disabling adapter > >>> > >>> The patch fixes the issue where the controller cannot be disabled > >>> while SCL is held low if the ENABLE bit is already disabled. > >> > >>... > >> > >>> + /*Set ENABLE bit before setting ABORT*/ > > > > /* Set ENABLE bit before setting ABORT */ > > > >>Missing spaces > >> > >>... >> >> >> +/* >> >> + * This function waits controller idling before disabling I2C >> >> >waits for controller >> >> + * This function waits for controller idling before disabling I2C >> >> >> + * When the controller is not in the IDLE state, > >>> + * MST_ACTIVITY bit (IC_STATUS[5]) is set. > >>> + * Values: > >>> + * 0x1 (ACTIVE): Controller not idle >> >> + * 0x0 (IDLE): Controller is idle >> >> + * The function is called after returning the end of the current >> >> + transfer >> >> + * Returns: >> >> >> + * False when controller is in IDLE state. >> >> + * True when controller is in ACTIVE state. >> > >>Yeah, I know that this is a copy of what I suggested, but if we going > >>to amend, these should be with definite article >> >> > * False when the controller is in the IDLE state. >> > * True when the controller is in the ACTIVE state. >> >> >> >> >> + */ >> >> >... >> >> >> + return regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status, >> >> + !(status & DW_IC_STATUS_MASTER_ACTIVITY), >> >> + 1100, 20000) != 0; >> >> >You broke the indentation again. >> >> it has been indented and aligned from the web： >> https://lore.kernel.org/all/4ebc4e8882a52620cbca30f1bf25650cbc3723fb.1 >> 726197817.git.kimriver.liu@xxxxxxxxxxxx/ >> >> Thanks！ >> >> >-- ------------------------------------------ Best Regards Kimriver Liu
