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

Tyrone Ting posted 11 patches 4 years, 4 months ago
There is a newer version of this series
.../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     |  17 +-
arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  16 ++
drivers/i2c/busses/Kconfig                    |   8 +-
drivers/i2c/busses/Makefile                   |   2 +-
drivers/i2c/busses/i2c-npcm7xx.c              | 251 +++++++++++-------
5 files changed, 193 insertions(+), 101 deletions(-)
[PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts
Posted by Tyrone Ting 4 years, 4 months ago
From: Tyrone Ting <kfting@nuvoton.com>

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
 - Rob Herring : https://lkml.org/lkml/2022/2/7/1166
                 https://lkml.org/lkml/2022/2/11/711
 - Krzysztof Kozlowski : https://lkml.org/lkml/2022/2/7/742
 - Jonathan Neuschäfer : https://lkml.org/lkml/2022/2/7/934
 - Jonathan Neuschäfer : https://lkml.org/lkml/2022/2/7/947
 - Jonathan Neuschäfer : https://lkml.org/lkml/2022/2/7/1057
 - Krzysztof Kozlowski : https://lkml.org/lkml/2022/2/7/1192
 - kernel test robot : https://lore.kernel.org/all/
                       202202072020.toQ349pg-lkp@intel.com/
 
Changes since version 1:
 - Add nuvoton,sys-mgr property in NPCM devicetree.
 - Describe the commit message in imperative mood.
 - Modify the description in i2c binding document to cover NPCM series.
 - Add new property in i2c binding document.
 - Create a new patch for client address calculation.
 - Create a new patch for updating gcr property name.
 - Create a new patch for removing unused clock node.
 - Explain EOB in the commit description.
 - Create a new patch for correcting NPCM register access width.
 - Remove some comment since the corresponding logic no longer exists.
 - Remove fixes tag while the patch adds an additional feature.
 - Use devicetree data field to support NPCM845.

Tali Perry (7):
  i2c: npcm: Fix client address calculation
  i2c: npcm: Update gcr property name
  i2c: npcm: Remove unused clock node
  i2c: npcm: Fix timeout calculation
  i2c: npcm: Add tx complete counter
  i2c: npcm: Handle spurious interrupts
  i2c: npcm: Remove own slave addresses 2:10

Tyrone Ting (4):
  arm: dts: add new property for NPCM i2c module
  dt-bindings: i2c: npcm: support NPCM845
  i2c: npcm: Correct register access width
  i2c: npcm: Support NPCM845

 .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     |  17 +-
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  16 ++
 drivers/i2c/busses/Kconfig                    |   8 +-
 drivers/i2c/busses/Makefile                   |   2 +-
 drivers/i2c/busses/i2c-npcm7xx.c              | 251 +++++++++++-------
 5 files changed, 193 insertions(+), 101 deletions(-)

-- 
2.17.1

Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts
Posted by Krzysztof Kozlowski 4 years, 4 months ago
On 20/02/2022 04:53, Tyrone Ting wrote:
> From: Tyrone Ting <kfting@nuvoton.com>
> 
> 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.


Best regards,
Krzysztof
Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts
Posted by Tyrone Ting 4 years, 4 months ago
Hi Krzysztof:

Thank you for your comments and please find my reply next to your comments.

Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> 於 2022年2月20日
週日 下午5:30寫道:
>
> On 20/02/2022 04:53, Tyrone Ting wrote:
> > From: Tyrone Ting <kfting@nuvoton.com>
> >
> > 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.

>
> Best regards,
> Krzysztof

Best regards,
Tyrone
Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts
Posted by Krzysztof Kozlowski 4 years, 4 months ago
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 <krzysztof.kozlowski@canonical.com> 於 2022年2月20日
> 週日 下午5:30寫道:
>>
>> On 20/02/2022 04:53, Tyrone Ting wrote:
>>> From: Tyrone Ting <kfting@nuvoton.com>
>>>
>>> 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
Re: [PATCH v2 00/11] i2c: npcm: Bug fixes timeout, spurious interrupts
Posted by Tyrone Ting 4 years, 4 months ago
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 <krzysztof.kozlowski@canonical.com> 於 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 <krzysztof.kozlowski@canonical.com> 於 2022年2月20日
> > 週日 下午5:30寫道:
> >>
> >> On 20/02/2022 04:53, Tyrone Ting wrote:
> >>> From: Tyrone Ting <kfting@nuvoton.com>
> >>>
> >>> 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