RE: [PATCH v11] i2c: designware: fix controller is holding SCL low while ENABLE bit is disabled

[Liu Kimriver/刘金河]

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