From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-04.galae.net (smtpout-04.galae.net [185.171.202.116]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BBF7C383998 for ; Thu, 21 May 2026 14:50:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.171.202.116 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779375060; cv=none; b=uKNhC8fCCVpBJDJw07nl4nXQBNBkdjA1a09OLGQ8FAC/qrKxMMrCig8XJ+bGs2dmk9ryfbU6SmxNy4w2NfbND67WN01DrsGeo81kCocDxyZWTVQSUjaPOsMsE13TyGAX7hwGSLkANgOCrst++zIZY7XbqhTxIIQzxluloVWVs5c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779375060; c=relaxed/simple; bh=a52OfR4iIxB7NR4aDT4cxoCuDzhHBrR+NfXk0hq4l9g=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=DEegeJpHXYLhdxUQb1SsnOcHUca8yBvc9yPbevopH1Rby9nT+2nmkabGS+ZF3VJEIvZZsscIdPpuFcDh5I6t/wfmLI4oHApXN0eyB0JzqvdJuL8KzaToiEiHRCKY2NstO126JfdT2j/j+nLGSLR33o9wqrGcwZsM4J5H2MmOJwI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=nB7dqaCb; arc=none smtp.client-ip=185.171.202.116 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="nB7dqaCb" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-04.galae.net (Postfix) with ESMTPS id 3D425C2C64D; Thu, 21 May 2026 14:51:50 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 044E760495; Thu, 21 May 2026 14:50:56 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 4E841107E89AF; Thu, 21 May 2026 16:50:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1779375055; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=fTteTqIzCkDDRfgTUqBszUa4Ju7v0nJjWKenJsd6KUA=; b=nB7dqaCbGmYNQpadDDIPmQoye+tBE1lUNZvnmsMHr5H8yJL6gZlt6Z1KK1CsY6pxBD7CV7 0lg+Pcu0jDpSIlvZubgvSTe1r49CPtRBhdzx8oBEATZSE7hhmFjpp0VEIbWyr9UWVHTqsV lG9cBJf2qbTkdmqxNydPfcP+ePMw9Utsrqf7SnHTGCuL1dZHqtjCyOmzIufrWaW8CNMxP+ YQAZteWlPklCBBlLPXLKMb0e6qeHAfFMpSWxn94l0h17LbH+RxTn3Q/b4oglUYEL0tZWUt fpA6iLiaCGy1JFmy+U0dOPw3+OyGY95GLWgebbtNiJvNsPZPm7Qam6gkoJDq2A== Message-ID: Date: Thu, 21 May 2026 16:50:44 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v6] net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c To: Petr Wozniak , netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org, andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, bjorn@mork.no, linux-phy@lists.infradead.org, jan@3e8.eu References: <20260521025755.24649-1-petr.wozniak@gmail.com> Content-Language: en-US From: Maxime Chevallier In-Reply-To: <20260521025755.24649-1-petr.wozniak@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 Hi, On 5/21/26 04:57, Petr Wozniak wrote: > The "OEM"/"SFP-10G-T" quirk entry in sfp_fixup_rollball_cc() > unconditionally forces MDIO_I2C_ROLLBALL for all modules matching that > vendor/part-number combination. This works for modules that genuinely > implement a RollBall I2C-to-MDIO bridge, but silently breaks modules > that share the same EEPROM strings without having such a bridge. > > The Realtek RTL8261BE-CG is one such module: a pure copper 10G SFP+ > media converter with no I2C-to-MDIO bridge. Its EEPROM reports > vendor="OEM", part="SFP-10G-T-I", and -- critically -- Vendor OUI > 00:00:00, making OUI-based differentiation impossible. With > MDIO_I2C_ROLLBALL forced, the module silently ACKs the unlock password > write, the MDIO bus is created, but no PHY responds; the SFP state > machine cycles through the RollBall PHY-probe retry window before > reporting no PHY. > > Move the probe into i2c_mii_init_rollball() in mdio-i2c.c, where the > RollBall protocol constants are already defined. After sending the > unlock password, issue a CMD_READ and poll for CMD_DONE up to 200 ms > (10 x 20 ms, matching the existing rollball poll tolerance). A genuine > RollBall bridge asserts CMD_DONE within that window; modules without a > bridge never do, so i2c_mii_init_rollball() returns -ENODEV. > mdio_i2c_alloc() propagates -ENODEV to the caller to signal that no > bridge is present and PHY probing should be skipped. > sfp_sm_add_mdio_bus() catches -ENODEV and transitions > sfp->mdio_protocol to MDIO_I2C_NONE so the rest of the state machine > skips PHY probing for this module. > > Any I2C-level error (NACK, timeout) during the probe is also treated as > -ENODEV: if the module does not respond at I2C address 0x51 at all, > there is certainly no RollBall bridge there, and SFP initialization > should not abort. > > The probe writes are safe with respect to SFP EEPROM integrity: only > modules explicitly listed in the quirk table enter this path, and the > RollBall password unlock write to 0x51 was already issued by > i2c_mii_init_rollball() before the probe for all such modules. Any > module without a device at 0x51 NACKs the transfer and is treated as > -ENODEV. > > Add "OEM"/"SFP-10G-T-I" to the quirk table so RTL8261BE modules enter > the probe path; genuine RollBall modules continue to work as before. > > Signed-off-by: Petr Wozniak > Reviewed-by: Maxime Chevallier > --- > > Changes since v5 (Sashiko AI review): > - Treat I2C NACK/errors in i2c_mii_init_rollball() as -ENODEV so > modules without a 0x51 EEPROM do not abort SFP initialization > - Replace fixed 70 ms wait with 10 x 20 ms poll (total 200 ms), > matching the existing i2c_rollball_mii_poll() tolerance and > preventing false -ENODEV on slow RollBall bridges > > Changes since v4 (feedback from Maxime Chevallier): > - Fix commit message: replace "stalls" with accurate description of > the RollBall PHY-probe retry window > - Fix variable declaration order in i2c_mii_probe_rollball() to > follow reverse-xmas tree (descending line length) > - Remove spurious alignment space on "SFP-10G-T" quirk entry > - Document that -ENODEV from mdio_i2c_alloc() means no bridge present, > PHY probing should be skipped > > Changes since v3 (feedback from Jakub Kicinski): > - Drop spurious Tested-by: tag -- author and tester are the same person > - Use PATCH net-next subject prefix > - Move -ENODEV handling from sfp_i2c_mdiobus_create() into > sfp_sm_add_mdio_bus() so bus-creation code does not mutate > sfp->mdio_protocol; the state machine is the correct place for > protocol-state transitions > - Split combined variable declaration for clarity > > Changes since v2: > - Compile-tested and hardware-tested on BPI-R4 (MT7988A, 6.12.87) > - RTL8261BE (OEM/SFP-10G-T-I): probes MDIO_I2C_NONE, link Up 10Gbps > - Genuine RollBall (OEM/SFP-10G-T): bridge detected, link Up 10Gbps > > drivers/net/mdio/mdio-i2c.c | 65 ++++++++++++++++++++++++++++++------ > drivers/net/phy/sfp.c | 16 ++++++++-- > 2 files changed, 68 insertions(+), 13 deletions(-) > --- a/drivers/net/mdio/mdio-i2c.c > +++ b/drivers/net/mdio/mdio-i2c.c > @@ -352,6 +352,54 @@ > return 0; > } > > +static int i2c_mii_probe_rollball(struct i2c_adapter *i2c) > +{ > + u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 }; > + u8 cmd_buf[] = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ }; > + u8 cmd_addr = ROLLBALL_CMD_ADDR; > + struct i2c_msg msgs[2]; > + u8 result; > + int ret; > + int i; > + > + msgs[0].addr = ROLLBALL_PHY_I2C_ADDR; > + msgs[0].flags = 0; > + msgs[0].len = sizeof(data_buf); > + msgs[0].buf = data_buf; > + msgs[1].addr = ROLLBALL_PHY_I2C_ADDR; > + msgs[1].flags = 0; > + msgs[1].len = sizeof(cmd_buf); > + msgs[1].buf = cmd_buf; > + > + ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs)); > + if (ret < 0) > + return -ENODEV; > + if (ret) > + return ret; > + > + msgs[0].addr = ROLLBALL_PHY_I2C_ADDR; > + msgs[0].flags = 0; > + msgs[0].len = 1; > + msgs[0].buf = &cmd_addr; > + msgs[1].addr = ROLLBALL_PHY_I2C_ADDR; > + msgs[1].flags = I2C_M_RD; > + msgs[1].len = 1; > + msgs[1].buf = &result; > + > + for (i = 0; i < 10; i++) { > + msleep(20); > + ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs)); > + if (ret < 0) > + return -ENODEV; > + if (ret) > + return ret; I think this check is not needed, it seems to me that i2c_transfer_rollball() always returns either a negative number, or zero. All the calls in i2c_transfer_rollball() end-up returning the return value from __i2c_transfer_err(), which is always either negative or 0. There's a similar check below that can be simplified as well Maxime