linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonas Jelonek <jelonek.jonas@gmail.com>
To: Chris Packham <chris.packham@alliedtelesis.co.nz>,
	Andi Shyti <andi.shyti@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: linux-i2c@vger.kernel.org, Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Markus Stockhausen <markus.stockhausen@gmx.de>,
	Sven Eckelmann <sven@narfation.org>,
	Harshal Gohel <hg@simonwunderlich.de>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Jonas Jelonek <jelonek.jonas@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH v6 03/12] i2c: rtl9300: remove broken SMBus Quick operation support
Date: Sun, 24 Aug 2025 11:33:39 +0000	[thread overview]
Message-ID: <20250824113348.263475-4-jelonek.jonas@gmail.com> (raw)
In-Reply-To: <20250824113348.263475-1-jelonek.jonas@gmail.com>

Remove the SMBus Quick operation from this driver because it is not
natively supported by the hardware and is wrongly implemented in the
driver.

The I2C controllers in Realtek RTL9300 and RTL9310 are SMBus-compliant
but there doesn't seem to be native support for the SMBus Quick
operation. It is not explicitly mentioned in the documentation but
looking at the registers which configure an SMBus transaction, one can
see that the data length cannot be set to 0. This suggests that the
hardware doesn't allow any SMBus message without data bytes (except for
those it does on it's own, see SMBus Block Read).

The current implementation of SMBus Quick operation passes a length of
0 (which is actually invalid). Before the fix of a bug in a previous
commit, this led to a read operation of 16 bytes from any register (the
one of a former transaction or any other value.

This caused issues like soft-bricked SFP modules after a simple probe
with i2cdetect which uses Quick by default. Running this with SFP
modules whose EEPROM isn't write-protected, some of the initial bytes
are overwritten because a 16-byte write operation is executed instead of
a Quick Write. (This temporarily soft-bricked one of my DAC cables.)

Because SMBus Quick operation is obviously not supported on these
controllers (because a length of 0 cannot be set, even when no register
address is set), remove that instead of claiming there is support. There
also shouldn't be any kind of emulated 'Quick' which just does another
kind of operation in the background. Otherwise, specific issues occur
in case of a 'Quick' Write which actually writes unknown data to an
unknown register.

Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
Tested-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/i2c/busses/i2c-rtl9300.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index ebd4a85e1bde..9e6232075137 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -235,15 +235,6 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
 	}
 
 	switch (size) {
-	case I2C_SMBUS_QUICK:
-		ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 0);
-		if (ret)
-			goto out_unlock;
-		ret = rtl9300_i2c_reg_addr_set(i2c, 0, 0);
-		if (ret)
-			goto out_unlock;
-		break;
-
 	case I2C_SMBUS_BYTE:
 		if (read_write == I2C_SMBUS_WRITE) {
 			ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 0);
@@ -344,9 +335,9 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
 
 static u32 rtl9300_i2c_func(struct i2c_adapter *a)
 {
-	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
-	       I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
-	       I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_I2C_BLOCK;
+	return I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA |
+	       I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_DATA |
+	       I2C_FUNC_SMBUS_I2C_BLOCK;
 }
 
 static const struct i2c_algorithm rtl9300_i2c_algo = {
-- 
2.48.1


  parent reply	other threads:[~2025-08-24 11:34 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-24 11:33 [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 01/12] i2c: rtl9300: fix channel number bound check Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 02/12] i2c: rtl9300: ensure data length is within supported range Jonas Jelonek
2025-08-24 11:33 ` Jonas Jelonek [this message]
2025-08-24 11:33 ` [PATCH v6 04/12] i2c: rtl9300: use regmap fields and API for registers Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 05/12] dt-bindings: i2c: realtek,rtl9301-i2c: fix wording and typos Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 06/12] i2c: rtl9300: rename internal sda_pin to sda_num Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 07/12] i2c: rtl9300: move setting SCL frequency to config_io Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 08/12] i2c: rtl9300: do not set read mode on every transfer Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 09/12] i2c: rtl9300: separate xfer configuration and execution Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 10/12] i2c: rtl9300: use scoped guard instead of explicit lock/unlock Jonas Jelonek
2025-08-24 14:12   ` Markus Elfring
2025-08-24 17:44     ` Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 11/12] dt-bindings: i2c: realtek,rtl9301-i2c: extend for RTL9310 support Jonas Jelonek
2025-08-24 11:33 ` [PATCH v6 12/12] i2c: rtl9300: add support for RTL9310 I2C controller Jonas Jelonek
2025-08-24 21:31 ` [PATCH v6 00/12] i2c: fix, rework and extend RTL9300 I2C driver Chris Packham
2025-08-25 19:33 ` AW: " markus.stockhausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250824113348.263475-4-jelonek.jonas@gmail.com \
    --to=jelonek.jonas@gmail.com \
    --cc=andi.shyti@kernel.org \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hg@simonwunderlich.de \
    --cc=krzk+dt@kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus.stockhausen@gmx.de \
    --cc=robh@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=sven@narfation.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).