[PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver

Jonas Jelonek posted 12 patches 1 month, 1 week ago
There is a newer version of this series
.../bindings/i2c/realtek,rtl9301-i2c.yaml     |  45 +-
drivers/i2c/busses/i2c-rtl9300.c              | 488 ++++++++++--------
2 files changed, 325 insertions(+), 208 deletions(-)
[PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver
Posted by Jonas Jelonek 1 month, 1 week ago
This patch series for the RTL9300 I2C driver:
    - fixes issues, one of them in some cases causing data corruption
    - reworks significant parts of the current implementation
    - add support for the (quite similar) RTL9310 series

Goal of this is to fix critical issues, improve overall code quality and
simplify maintainance and further extension of the driver. Moreover, it
should be brought on par feature-wise with OpenWrt's downstream driver
to be able to drop the downstream version.

The first three patches address bugs in the current implementation, on
of them being critical and causing data corruption under certain
circumstances. Although the hardware doesn't support SMBus Quick Write,
the driver claims to support it with a broken implementation. This
causes to execute a 16-byte Write instead of a Quick Write, e.g. causing
corruption on not-write-protected SFP EEPROMs and soft-bricking them.
These three patches are also sent to 'stable' because they fix critical
issues.

Subsequent patches introduce various smaller and bigger enhancements.
These include:
    - use regmap_field + its API instead of macros + GENMASK + shifts
    - refactor xfer handling
    - variable renaming to avoid confusion
    - move some register operations, calling them somewhere else and
      less frequently
    - use guarded mutex instead of explicit mutex_lock/_unlock to
      simplify control flow

Finally, the last two patches add support for RTL9310 (mango) series to
the driver and adjust the dt-bindings accordingly.

Simple operations have been tested successfully on:
    - Zyxel XGS1210-12 (RTL9302B)
    - TP-Link TL-ST1008F v2.0 (RTL9303)
    - Netgear MS510TXM (RTL9313)

with Byte-Read, Word-Read and I2C-Block-Read. Other operations need
testing from people with devices available.

Compile-tested with Linux, run-tested as backport in OpenWrt on the
aforementioned devices.

--
Changelog

v6: - patch 'i2c: rtl9300: check if xfer length is valid'
        - renamed to 'ensure data length is within supported range'
        - added I2C quirk for zero length as suggested by Wolfram Sang
    - reordered patches to have backport-worthy fixes first and
      enhancements/others after
        - patches 'fix channel number bound check', 'check if xfer
          length is valid' and 'remove SMBus Quick operation support'
          were moved before all others
	- added CC: stable to first three patches
    - fixed commit message of 'dt-bindings: i2c: realtek,rtl9301-i2c:
      extend for RTL9310 support'
    - added a patch to use guard(mutex) instead of explicit lock/unlock
      as suggested by Markus Elfring
    - added Reviewed-by: Rob Herring ... to dt-bindings patches
    - added Tested-by: Sven Eckelmann ... to all patches (except the
      new patch in this version)

v5: - added more patches to fix further issues/do further cleanup
        - remove SMBus Quick support (not supported by hardware)
        - move setting SCL frequency to config_io
        - only set read message format (RD_MODE) once on probing
        - add check to avoid len = 0 being allowed as length
    - adjusted cover letter

v4: - fixed an incorrect check for number of channels which was already
      present in original code

v3: - narrowed vendor property per variant to be required only
      for RTL9310
    - narrowed usable child-node i2c addresses per variant
    - no changes to driver patches

v2: - Patch 1:
        - adjusted commit message
        - retained Tested-By and Reviewed-By from Chris Packham
    - Patch 2:
        - simplified check as suggested by Markus Stockhausen
        - fixed commit message
    - Patch 3 (all requested by Krzysztof):
        - use vendor property instead of generic
        - add front compatibles to make binding complete
        - fix commit message
    - reordered patches, dt-bindings patch now comes before its 'user'
    - properly add device-tree list and relevant maintainers to To/Cc

---
Jonas Jelonek (12):
  i2c: rtl9300: fix channel number bound check
  i2c: rtl9300: ensure data length is within supported range
  i2c: rtl9300: remove broken SMBus Quick operation support
  i2c: rtl9300: use regmap fields and API for registers
  dt-bindings: i2c: realtek,rtl9301-i2c: fix wording and typos
  i2c: rtl9300: rename internal sda_pin to sda_num
  i2c: rtl9300: move setting SCL frequency to config_io
  i2c: rtl9300: do not set read mode on every transfer
  i2c: rtl9300: separate xfer configuration and execution
  i2c: rtl9300: use scoped guard instead of explicit lock/unlock
  dt-bindings: i2c: realtek,rtl9301-i2c: extend for RTL9310 support
  i2c: rtl9300: add support for RTL9310 I2C controller

 .../bindings/i2c/realtek,rtl9301-i2c.yaml     |  45 +-
 drivers/i2c/busses/i2c-rtl9300.c              | 488 ++++++++++--------
 2 files changed, 325 insertions(+), 208 deletions(-)


base-commit: 57774430864b721082b9bafd17fc839f31251c7b
-- 
2.48.1
Re: [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver
Posted by Chris Packham 1 month, 1 week ago
Hi Jonas,

On 24/08/2025 23:33, Jonas Jelonek wrote:
> This patch series for the RTL9300 I2C driver:
>      - fixes issues, one of them in some cases causing data corruption
>      - reworks significant parts of the current implementation
>      - add support for the (quite similar) RTL9310 series
>
> Goal of this is to fix critical issues, improve overall code quality and
> simplify maintainance and further extension of the driver. Moreover, it
> should be brought on par feature-wise with OpenWrt's downstream driver
> to be able to drop the downstream version.
>
> The first three patches address bugs in the current implementation, on
> of them being critical and causing data corruption under certain
> circumstances. Although the hardware doesn't support SMBus Quick Write,
> the driver claims to support it with a broken implementation. This
> causes to execute a 16-byte Write instead of a Quick Write, e.g. causing
> corruption on not-write-protected SFP EEPROMs and soft-bricking them.
> These three patches are also sent to 'stable' because they fix critical
> issues.
>
> Subsequent patches introduce various smaller and bigger enhancements.
> These include:
>      - use regmap_field + its API instead of macros + GENMASK + shifts
>      - refactor xfer handling
>      - variable renaming to avoid confusion
>      - move some register operations, calling them somewhere else and
>        less frequently
>      - use guarded mutex instead of explicit mutex_lock/_unlock to
>        simplify control flow
>
> Finally, the last two patches add support for RTL9310 (mango) series to
> the driver and adjust the dt-bindings accordingly.
>
> Simple operations have been tested successfully on:
>      - Zyxel XGS1210-12 (RTL9302B)
>      - TP-Link TL-ST1008F v2.0 (RTL9303)
>      - Netgear MS510TXM (RTL9313)
>
> with Byte-Read, Word-Read and I2C-Block-Read. Other operations need
> testing from people with devices available.
>
> Compile-tested with Linux, run-tested as backport in OpenWrt on the
> aforementioned devices.

For the series

Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz> # On 
RTL9302C based board

> --
> Changelog
>
> v6: - patch 'i2c: rtl9300: check if xfer length is valid'
>          - renamed to 'ensure data length is within supported range'
>          - added I2C quirk for zero length as suggested by Wolfram Sang
>      - reordered patches to have backport-worthy fixes first and
>        enhancements/others after
>          - patches 'fix channel number bound check', 'check if xfer
>            length is valid' and 'remove SMBus Quick operation support'
>            were moved before all others
> 	- added CC: stable to first three patches
>      - fixed commit message of 'dt-bindings: i2c: realtek,rtl9301-i2c:
>        extend for RTL9310 support'
>      - added a patch to use guard(mutex) instead of explicit lock/unlock
>        as suggested by Markus Elfring
>      - added Reviewed-by: Rob Herring ... to dt-bindings patches
>      - added Tested-by: Sven Eckelmann ... to all patches (except the
>        new patch in this version)
>
> v5: - added more patches to fix further issues/do further cleanup
>          - remove SMBus Quick support (not supported by hardware)
>          - move setting SCL frequency to config_io
>          - only set read message format (RD_MODE) once on probing
>          - add check to avoid len = 0 being allowed as length
>      - adjusted cover letter
>
> v4: - fixed an incorrect check for number of channels which was already
>        present in original code
>
> v3: - narrowed vendor property per variant to be required only
>        for RTL9310
>      - narrowed usable child-node i2c addresses per variant
>      - no changes to driver patches
>
> v2: - Patch 1:
>          - adjusted commit message
>          - retained Tested-By and Reviewed-By from Chris Packham
>      - Patch 2:
>          - simplified check as suggested by Markus Stockhausen
>          - fixed commit message
>      - Patch 3 (all requested by Krzysztof):
>          - use vendor property instead of generic
>          - add front compatibles to make binding complete
>          - fix commit message
>      - reordered patches, dt-bindings patch now comes before its 'user'
>      - properly add device-tree list and relevant maintainers to To/Cc
>
> ---
> Jonas Jelonek (12):
>    i2c: rtl9300: fix channel number bound check
>    i2c: rtl9300: ensure data length is within supported range
>    i2c: rtl9300: remove broken SMBus Quick operation support
>    i2c: rtl9300: use regmap fields and API for registers
>    dt-bindings: i2c: realtek,rtl9301-i2c: fix wording and typos
>    i2c: rtl9300: rename internal sda_pin to sda_num
>    i2c: rtl9300: move setting SCL frequency to config_io
>    i2c: rtl9300: do not set read mode on every transfer
>    i2c: rtl9300: separate xfer configuration and execution
>    i2c: rtl9300: use scoped guard instead of explicit lock/unlock
>    dt-bindings: i2c: realtek,rtl9301-i2c: extend for RTL9310 support
>    i2c: rtl9300: add support for RTL9310 I2C controller
>
>   .../bindings/i2c/realtek,rtl9301-i2c.yaml     |  45 +-
>   drivers/i2c/busses/i2c-rtl9300.c              | 488 ++++++++++--------
>   2 files changed, 325 insertions(+), 208 deletions(-)
>
>
> base-commit: 57774430864b721082b9bafd17fc839f31251c7b
AW: [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver
Posted by markus.stockhausen@gmx.de 1 month, 1 week ago
> Von: Jonas Jelonek <jelonek.jonas@gmail.com> 
> Gesendet: Sonntag, 24. August 2025 13:34
> Betreff: [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver
>
> This patch series for the RTL9300 I2C driver:
>     - fixes issues, one of them in some cases causing data corruption
>     - reworks significant parts of the current implementation
>     - add support for the (quite similar) RTL9310 series
>
> Goal of this is to fix critical issues, improve overall code quality and
> simplify maintainance and further extension of the driver. Moreover, it
> should be brought on par feature-wise with OpenWrt's downstream driver
> to be able to drop the downstream version.
>
> The first three patches address bugs in the current implementation, on
> of them being critical and causing data corruption under certain
> circumstances. Although the hardware doesn't support SMBus Quick Write,
> the driver claims to support it with a broken implementation. This
> causes to execute a 16-byte Write instead of a Quick Write, e.g. causing
> corruption on not-write-protected SFP EEPROMs and soft-bricking them.
> These three patches are also sent to 'stable' because they fix critical
> issues.
>
> Subsequent patches introduce various smaller and bigger enhancements.
> These include:
>     - use regmap_field + its API instead of macros + GENMASK + shifts
>     - refactor xfer handling
>     - variable renaming to avoid confusion
>     - move some register operations, calling them somewhere else and
>       less frequently
>     - use guarded mutex instead of explicit mutex_lock/_unlock to
>       simplify control flow
>
> Finally, the last two patches add support for RTL9310 (mango) series to
> the driver and adjust the dt-bindings accordingly.
>
> Simple operations have been tested successfully on:
>     - Zyxel XGS1210-12 (RTL9302B)
>     - TP-Link TL-ST1008F v2.0 (RTL9303)
>     - Netgear MS510TXM (RTL9313)
>
> with Byte-Read, Word-Read and I2C-Block-Read. Other operations need
> testing from people with devices available.
>
> Compile-tested with Linux, run-tested as backport in OpenWrt on the
> aforementioned devices.

Repeated the downstream tests with this series backported to 6.12. 
Successfully tested on Linksys LGS328C (RTL9301) and LGS352C (RTL9311)

Tested-by: Markus Stockhausen <markus.stockhausen@gmx.de