| Message ID | 20220220035321.3870-1-warp5tw@gmail.com |
|---|---|
| Headers | show |
| Series | i2c: npcm: Bug fixes timeout, spurious interrupts | expand |
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
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
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
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
> 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.
Hi Wolfram: Thank you for your comment and it'll be addressed. Wolfram Sang <wsa@kernel.org> 於 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
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(-)