Message-ID: <b61a8521-c5d2-4b44-800a-c93833ff8f1a@alliedtelesis.co.nz>
Date: Sun, 24 Aug 2025 21:31:19 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Jonas Jelonek <jelonek.jonas@...il.com>, Andi Shyti
	<andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>
CC: "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>, Conor Dooley
	<conor+dt@...nel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Markus Stockhausen
	<markus.stockhausen@....de>, Sven Eckelmann <sven@...fation.org>, "Harshal
 Gohel" <hg@...onwunderlich.de>, Wolfram Sang
	<wsa+renesas@...g-engineering.com>
Subject: Re: [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver

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@...iedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@...iedtelesis.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

Powered by blists - more mailing lists