[PATCH] i2c: imx_lpi2c: Fix MSR status check issue in STOP

[Heiko Schocher]

Hello Ye li,

On 28.04.26 17:20, Ye Li wrote:
> Hi Heiko,
> 
> On 4/28/2026 5:55 PM, Heiko Schocher wrote:
>> Hello Ye Li,
>>
>> On 28.04.26 10:53, Ye Li wrote:
>>> In bus_i2c_stop, the MSR SDF is checked in a loop after stop command
>>> is sent. Meanwhile, some error status in MSR is also checked by
>>> imx_lpci2c_check_clear_error. But the imx_lpci2c_check_clear_error
>>> will clear the MSR.
>>>
>>> It causes problem in below situation:
>>> In current loop, SDF does not set, but error status is found by
>>> imx_lpci2c_check_clear_error (for example, NDF), then NDF will be cleared
>>> and result has NDF error. However, because SDF does not set in this loop,
>>> it goes not next loop. When SDF is set in next loop,
>>> imx_lpci2c_check_clear_error is re-executed, but as the MSR is cleared,
>>> the result is 0. Then the stop return 0. But it should return NDF error.
>>>
>>> Signed-off-by: Ye Li <ye.li at nxp.com>
>>> ---
>>>   drivers/i2c/imx_lpi2c.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c
>>> index a309fd6f07c..e2b4fd334ec 100644
>>> --- a/drivers/i2c/imx_lpi2c.c
>>> +++ b/drivers/i2c/imx_lpi2c.c
>>> @@ -239,7 +239,6 @@ static int bus_i2c_stop(struct udevice *bus)
>>>       start_time = get_timer(0);
>>>       while (1) {
>>>           status = readl(&regs->msr);
>>> -        result = imx_lpci2c_check_clear_error(regs);
>>
>> Wouldn;t it be better to evaluate bits in result too, and return
>> if error bits found?
> 
> I find below commit in history, please take a look. If evaluate bits and return, it moves back to 
> previous clock issue.
> 
> commit a32effd2838a62a5d1dcc01f613508f1d90b5667
> Author: Gao Pan <pandy.gao at nxp.com>
> Date:   Sun Jul 8 11:46:41 2018 +0800
> 
>      imx: lpi2c: fix clock issue when NACK detected
> 
>      For LPI2C IP, NACK is detected by the rising edge of the ninth clock.
>      In current uboot driver, once NACK is detected, it will reset and then
>      disable LPI2C master. As a result, we can never see the falling edge
>      of the ninth clock.
> 
>      Signed-off-by: Gao Pan <pandy.gao at nxp.com>
>      Signed-off-by: Peng Fan <peng.fan at nxp.com>
>      Reviewed-by: Heiko Schocher <hs at denx.de>

Ah, okay. May Peng or Gao (added to this thread) can
than review your patch.

Thanks for the hint!

bye,
Heiko
> 
> 
> Best regards,
> Ye Li
> 
>>
>>>           /* stop detect flag */
>>>           if (status & LPI2C_MSR_SDF_MASK) {
>>>               /* clear stop flag */
>>> @@ -250,10 +249,13 @@ static int bus_i2c_stop(struct udevice *bus)
>>>           if (get_timer(start_time) > LPI2C_NACK_TOUT_MS) {
>>>               debug("stop timeout\n");
>>> +            result = imx_lpci2c_check_clear_error(regs);
>>>               return -ETIMEDOUT;
>>>           }
>>>       }
>>> +    result = imx_lpci2c_check_clear_error(regs);
>>> +
>>>       return result;
>>>   }
>>>
>>
>> bye,
>> Heiko
> 

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office at nabladev.com
Geschäftsführer : Stefano Babic