Message-ID: <d2abc63a-884d-4d48-b652-56989e55d0cd@jaguarmicro.com>
Date: Wed, 6 Nov 2024 15:27:42 +0800
From: Liu Peibao <loven.liu@...uarmicro.com>
To: Andi Shyti <andi.shyti@...nel.org>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
 Mika Westerberg <mika.westerberg@...ux.intel.com>,
 Jan Dabros <jsd@...ihalf.com>, "xiaowu . ding"
 <xiaowu.ding@...uarmicro.com>, Angus Chen <angus.chen@...uarmicro.com>,
 linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND TO CC MAILLIST] i2c: designware: fix master holding
 SCL low when I2C_DYNAMIC_TAR_UPDATE not set

On 2024/11/6 4:30, Andi Shyti wrote:
> External Mail: This email originated from OUTSIDE of the organization!
> Do not click links, open attachments or provide ANY information unless you recognize the sender and know the content is safe.
> 
> 
> Hi Liu,
> 
> On Fri, Nov 01, 2024 at 06:18:36PM +0800, Liu Peibao wrote:
>> On 2024/11/1 16:44, Andy Shevchenko wrote:
>>> External Mail: This email originated from OUTSIDE of the organization!
>>> Do not click links, open attachments or provide ANY information unless you recognize the sender and know the content is safe.
>>>
>>> On Fri, Nov 01, 2024 at 04:12:43PM +0800, Liu Peibao wrote:
>>>> When Tx FIFO empty and last command with no STOP bit set, the master
>>>> holds SCL low. If I2C_DYNAMIC_TAR_UPDATE is not set, BIT(13) MST_ON_HOLD
>>>> of IC_RAW_INTR_STAT is not Enabled, causing the __i2c_dw_disable()
>>>> timeout. This is quiet similar as commit 2409205acd3c ("i2c: designware:
>>>> fix __i2c_dw_disable() in case master is holding SCL low") mentioned.
>>>> Check BIT(7) MST_HOLD_TX_FIFO_EMPTY in IC_STATUS also which is available
>>>> when IC_STAT_FOR_CLK_STRETCH is set.
>>>
>>> Who are those people? Why Angus Chen is not a committer of the change?
>>> Please, consult with the Submitting Patches documentation to clarify on these
>>> tags.
>>>
>>
>> We have discussed and analyzed this issue together. I developed this patch.
>> This patch was also reviewed by Angus Chen and Xiaowu Ding.
> 
> The tag list follows a specific order: tags are sorted
> sequentially, with the last Signed-off-by (SoB) being the person
> sending the patch, which is your email.
> 
> The other SoBs are fine, but if someone contributed to
> development, consider using "Co-developed-by" instead.
> 
> If someone tested the patch, use "Tested-by"; if they reviewed
> it, use "Reviewed-by"; and if they simply agreed with the
> change, use "Acked-by."
> 
> Please ensure that "Reviewed-by," "Tested-by," or "Acked-by"
> tags are visible in the mailing list. I do not typically accept
> offline R-b, T-b, or A-b.
> 
> This is why Andy asked about those contributors. Three SoBs can
> seem unusual, but it's acceptable if justified. Reviewers may
> ask for clarification, and it's fine to specify contributors'
> roles. You can also provide extra details after the "---"
> delimiter.
>

I think this tag list should be much better than the original. ^-^

Fixes: 2409205acd3c ("i2c: designware: fix __i2c_dw_disable() in case master is holding SCL low")
Co-developed-by: xiaowu.ding <xiaowu.ding@...uarmicro.com>
Signed-off-by: xiaowu.ding <xiaowu.ding@...uarmicro.com>
Co-developed-by: Angus Chen <angus.chen@...uarmicro.com>
Signed-off-by: Angus Chen <angus.chen@...uarmicro.com>
Signed-off-by: Liu Peibao <loven.liu@...uarmicro.com>

>> And in this case, should I replace the "SoBs" with "Reviewed-by"?
>>
>>> Also, sounds to me that Fixes tag is needed.
>>>
>>
>> How about this tag:
>> Fixes: 2409205acd3c ("i2c: designware: fix __i2c_dw_disable() in case master is holding SCL low")
> 
> Sounds reasonable.
> 
> For accepting this patch I need an ack from either Andy, Jarkko
> or Mika.
> 
> As long as the fixes are limited to the commit message there is
> no need to resend the patch.
> 
> Thanks,
> Andi

Got it!

BR,
Peibao

Powered by blists - more mailing lists