2022-02-21 09:10:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts

On 21/02/2022 09:16, Tyrone Ting wrote:
> Hi Krzysztof:
>
> Thank you for your comments and please find my reply next to your comments.
>
> Krzysztof Kozlowski <[email protected]> 於 2022年2月20日
> 週日 下午5:30寫道:
>>
>> On 20/02/2022 04:53, Tyrone Ting wrote:
>>> From: Tyrone Ting <[email protected]>
>>>
>>> This patchset includes the following fixes:
>>>
>>> - Add dt-bindings description for NPCM845.
>>> - Bug fix for timeout calculation.
>>> - Better handling of spurious interrupts.
>>> - Fix for event type in slave mode.
>>> - Removal of own slave addresses [2:10].
>>> - Support for next gen BMC (NPCM845).
>>>
>>> The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.
>>>
>>> Addressed comments from:
>>> - Jonathan Neuschäfer : https://lkml.org/lkml/2022/2/7/670
>>> - Krzysztof Kozlowski : https://lkml.org/lkml/2022/2/7/760
>>
>> How did you address the ABI change comment? I still see you break the
>> ABI with the introduction of a new, required property.
>>
>
> I add the new, required property "nuvoton,sys-mgr" in the file
> nuvoton-common-npcm7xx.dtsi.
> The file nuvoton-common-npcm7xx.dtsi is required by the existing
> upstream NPCM devicetree files.
> It is also updated and committed in this patch set [PATCH v2 01/11]
> arm: dts: add new property for NPCM i2c module.
> Please let me know if I misunderstand the meaning of "breaking the ABI".
> Thank you again.

Breaking the ABI means that old DTS stop working with new kernel. Your
change breaks old (and out-of-tree) DTS.

What is more, your change is not bisectable because DTS goes via
separate branch or tree than driver change.

You need to keep old code as fallback, if getting nuvoton,sys-mgr fails.

Best regards,
Krzysztof


2022-02-21 09:22:30

by Tyrone Ting

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts

Hi Krzysztof:

Got it and thank you for your comments.

I'll keep old code as fallback, if getting nuvoton,sys-mgr fails as
you point out.

Krzysztof Kozlowski <[email protected]> 於 2022年2月21日
週一 下午4:32寫道:
>
> On 21/02/2022 09:16, Tyrone Ting wrote:
> > Hi Krzysztof:
> >
> > Thank you for your comments and please find my reply next to your comments.
> >
> > Krzysztof Kozlowski <[email protected]> 於 2022年2月20日
> > 週日 下午5:30寫道:
> >>
> >> On 20/02/2022 04:53, Tyrone Ting wrote:
> >>> From: Tyrone Ting <[email protected]>
> >>>
> >>> This patchset includes the following fixes:
> >>>
> >>> - Add dt-bindings description for NPCM845.
> >>> - Bug fix for timeout calculation.
> >>> - Better handling of spurious interrupts.
> >>> - Fix for event type in slave mode.
> >>> - Removal of own slave addresses [2:10].
> >>> - Support for next gen BMC (NPCM845).
> >>>
> >>> The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.
> >>>
> >>> Addressed comments from:
> >>> - Jonathan Neuschäfer : https://lkml.org/lkml/2022/2/7/670
> >>> - Krzysztof Kozlowski : https://lkml.org/lkml/2022/2/7/760
> >>
> >> How did you address the ABI change comment? I still see you break the
> >> ABI with the introduction of a new, required property.
> >>
> >
> > I add the new, required property "nuvoton,sys-mgr" in the file
> > nuvoton-common-npcm7xx.dtsi.
> > The file nuvoton-common-npcm7xx.dtsi is required by the existing
> > upstream NPCM devicetree files.
> > It is also updated and committed in this patch set [PATCH v2 01/11]
> > arm: dts: add new property for NPCM i2c module.
> > Please let me know if I misunderstand the meaning of "breaking the ABI".
> > Thank you again.
>
> Breaking the ABI means that old DTS stop working with new kernel. Your
> change breaks old (and out-of-tree) DTS.
>
> What is more, your change is not bisectable because DTS goes via
> separate branch or tree than driver change.
>
> You need to keep old code as fallback, if getting nuvoton,sys-mgr fails.
>
> Best regards,
> Krzysztof

Best regards,
Tyrone

2022-03-01 20:17:05

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts


> I'll keep old code as fallback, if getting nuvoton,sys-mgr fails as
> you point out.

Yeah, fallback is much needed. And if you implement it, then you can
also split the series into two. One for the DTS changes and one for the
I2C changes. That would make upstreaming a lot easier.


Attachments:
(No filename) (294.00 B)
signature.asc (849.00 B)
Download all attachments

2022-03-02 23:56:16

by Tyrone Ting

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts

Hi Wolfram:

Thank you for your comment and it'll be addressed.

Wolfram Sang <[email protected]> 於 2022年3月2日 週三 上午3:45寫道:
>
>
> > I'll keep old code as fallback, if getting nuvoton,sys-mgr fails as
> > you point out.
>
> Yeah, fallback is much needed. And if you implement it, then you can
> also split the series into two. One for the DTS changes and one for the
> I2C changes. That would make upstreaming a lot easier.
>

Regards,
Tyrone