[PATCH net-next 1/2] net: sfp: update comment for FS SFP-10G-T quirk

[PATCH net-next 1/2] net: sfp: update comment for FS SFP-10G-T quirk

[Marek Behún]

Update the comment for the Fibrestore SFP-10G-T module: since commit
e9301af385e7 ("net: sfp: fix PHY discovery for FS SFP-10G-T module")
we also do a 4 second wait before probing the PHY.

Fixes: e9301af385e7 ("net: sfp: fix PHY discovery for FS SFP-10G-T module")
Signed-off-by: Marek Behún <kabel@kernel.org>
---
Since this only fixes a comment and the next patch is based on this one,
I am sending to net-next instead of net.
---
 drivers/net/phy/sfp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 6e7639fc64dd..1af15f2da8a6 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -468,8 +468,9 @@ static const struct sfp_quirk sfp_quirks[] = {
 	SFP_QUIRK("ALCATELLUCENT", "3FE46541AA", sfp_quirk_2500basex,
 		  sfp_fixup_nokia),
 
-	// Fiberstore SFP-10G-T doesn't identify as copper, and uses the
-	// Rollball protocol to talk to the PHY.
+	// Fiberstore SFP-10G-T doesn't identify as copper, uses the Rollball
+	// protocol to talk to the PHY and needs 4 sec wait before probing the
+	// PHY.
 	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
 
 	// Fiberstore GPON-ONU-34-20BI can operate at 2500base-X, but report 1.2GBd
-- 
2.43.2

[PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Marek Behún]

Enhance the quirk for Fibrestore 2.5G copper SFP module. The original
commit e27aca3760c0 ("net: sfp: add quirk for FS's 2.5G copper SFP")
introducing the quirk says that the PHY is inaccessible, but that is
not true.

The module uses Rollball protocol to talk to the PHY, and needs a 4
second wait before probing it, same as FS 10G module.

The PHY inside the module is Realtek RTL8221B-VB-CG PHY, for which
the realtek driver we only recently gained support to set it up via
clause 45 accesses.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
This patch depends on realtek driver changes merged in
  c31bd5b6ff6f ("Merge branch 'rtl8226b-serdes-switching'")
which are currently only in net-next.

Frank, Russell, do you still have access to OEM SFP-2.5G-T module?
It could make sense to try this quirk also for those modeuls, instead
of the current sfp_quirk_oem_2_5g.
---
 drivers/net/phy/sfp.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 1af15f2da8a6..7d063cd3c6af 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -385,18 +385,23 @@ static void sfp_fixup_rollball(struct sfp *sfp)
 	sfp->phy_t_retry = msecs_to_jiffies(1000);
 }
 
-static void sfp_fixup_fs_10gt(struct sfp *sfp)
+static void sfp_fixup_fs_2_5gt(struct sfp *sfp)
 {
-	sfp_fixup_10gbaset_30m(sfp);
 	sfp_fixup_rollball(sfp);
 
-	/* The RollBall fixup is not enough for FS modules, the AQR chip inside
+	/* The RollBall fixup is not enough for FS modules, the PHY chip inside
 	 * them does not return 0xffff for PHY ID registers in all MMDs for the
 	 * while initializing. They need a 4 second wait before accessing PHY.
 	 */
 	sfp->module_t_wait = msecs_to_jiffies(4000);
 }
 
+static void sfp_fixup_fs_10gt(struct sfp *sfp)
+{
+	sfp_fixup_10gbaset_30m(sfp);
+	sfp_fixup_fs_2_5gt(sfp);
+}
+
 static void sfp_fixup_halny_gsfp(struct sfp *sfp)
 {
 	/* Ignore the TX_FAULT and LOS signals on this module.
@@ -473,6 +478,10 @@ static const struct sfp_quirk sfp_quirks[] = {
 	// PHY.
 	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
 
+	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the PHY and
+	// needs 4 sec wait before probing the PHY.
+	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
+
 	// Fiberstore GPON-ONU-34-20BI can operate at 2500base-X, but report 1.2GBd
 	// NRZ in their EEPROM
 	SFP_QUIRK("FS", "GPON-ONU-34-20BI", sfp_quirk_2500basex,
@@ -489,9 +498,6 @@ static const struct sfp_quirk sfp_quirks[] = {
 	SFP_QUIRK("HUAWEI", "MA5671A", sfp_quirk_2500basex,
 		  sfp_fixup_ignore_tx_fault),
 
-	// FS 2.5G Base-T
-	SFP_QUIRK_M("FS", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
-
 	// Lantech 8330-262D-E can operate at 2500base-X, but incorrectly report
 	// 2500MBd NRZ in their EEPROM
 	SFP_QUIRK_M("Lantech", "8330-262D-E", sfp_quirk_2500basex),
-- 
2.43.2

Re: [PATCH net-next 1/2] net: sfp: update comment for FS SFP-10G-T quirk

[Andrew Lunn]

On Mon, Apr 22, 2024 at 11:44:34AM +0200, Marek Behún wrote:
> Update the comment for the Fibrestore SFP-10G-T module: since commit
> e9301af385e7 ("net: sfp: fix PHY discovery for FS SFP-10G-T module")
> we also do a 4 second wait before probing the PHY.
> 
> Fixes: e9301af385e7 ("net: sfp: fix PHY discovery for FS SFP-10G-T module")
> Signed-off-by: Marek Behún <kabel@kernel.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Andrew Lunn]

> The PHY inside the module is Realtek RTL8221B-VB-CG PHY, for which
> the realtek driver we only recently gained support to set it up via
> clause 45 accesses.

This sentence does not parse very well.

The PHY inside the module is a Realtek RTL8221B-VB-CG. The realtek
driver recently gained support to set it up via clause 45 accesses.

???


    Andrew

---
pw-bot: cr

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Marek Behún]

On Mon, 22 Apr 2024 15:12:55 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > The PHY inside the module is Realtek RTL8221B-VB-CG PHY, for which
> > the realtek driver we only recently gained support to set it up via
> > clause 45 accesses.  
> 
> This sentence does not parse very well.
> 
> The PHY inside the module is a Realtek RTL8221B-VB-CG. The realtek
> driver recently gained support to set it up via clause 45 accesses.
> 
> ???
> 
> 
>     Andrew
> 
> ---
> pw-bot: cr

Sorry about this, will send v2 :)

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Eric Woudstra]

On 4/22/24 11:44, Marek Behún wrote:

> Frank, Russell, do you still have access to OEM SFP-2.5G-T module?
> It could make sense to try this quirk also for those modeuls, instead
> of the current sfp_quirk_oem_2_5

It was part of the previous patch-set until and including v2:
"rtl8221b/8251b add C45 instances and SerDes switching"

Note that it does:
	sfp->id.base.extended_cc = SFF8024_ECC_2_5GBASE_T;
As the OEM modules have not set this byte. We need it, so that we know
that the sfp_may_have_phy().

After v2 I have dropped it, as it would break functioning some sfp-modules.

As OEM vendors know the eeprom password of the Rollball sfp modules,
they use it in any way they want, not taking in to account that mainline
kernel uses it for unique identification.

Vendor 1 sells "OEM", "SFP-2.5G-T" with a rtl8221b on it.
Vendor 2 sells "OEM", "SFP-2.5G-T" with a yt8821 on it.

So on the OEM modules, we cannot rely solely on the two strings anymore.

Introducing the patch, would break the modules with the yt8821 on it. It
does not have any support in mainline.

Also any code I found for the yt8821 is C22 only. And even more, even
though we are facing with an almost similar MCU, RollBall protocol does
not work. I think it is almost the same mcu, as it responds to the same
eeprom password, and also the rollball password does something, but not
give the expected result.

So you for the OEM module it would have been:

+// For 2.5GBASE-T short-reach modules
+static void sfp_fixup_oem_2_5gbaset(struct sfp *sfp)
+{
+	sfp_fixup_rollball(sfp);
+	sfp->id.base.extended_cc = SFF8024_ECC_2_5GBASE_T;
+}
+

-	SFP_QUIRK_M("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
+	SFP_QUIRK_F("OEM", "SFP-2.5G-T", sfp_fixup_oem_2_5gbaset),

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Marek Behún]

On Tue, 23 Apr 2024 07:54:39 +0200
Eric Woudstra <ericwouds@gmail.com> wrote:

> On 4/22/24 11:44, Marek Behún wrote:
> 
> > Frank, Russell, do you still have access to OEM SFP-2.5G-T module?
> > It could make sense to try this quirk also for those modeuls, instead
> > of the current sfp_quirk_oem_2_5  
> 
> It was part of the previous patch-set until and including v2:
> "rtl8221b/8251b add C45 instances and SerDes switching"
> 
> Note that it does:
> 	sfp->id.base.extended_cc = SFF8024_ECC_2_5GBASE_T;
> As the OEM modules have not set this byte. We need it, so that we know
> that the sfp_may_have_phy().
> 
> After v2 I have dropped it, as it would break functioning some sfp-modules.
> 
> As OEM vendors know the eeprom password of the Rollball sfp modules,
> they use it in any way they want, not taking in to account that mainline
> kernel uses it for unique identification.
> 
> Vendor 1 sells "OEM", "SFP-2.5G-T" with a rtl8221b on it.
> Vendor 2 sells "OEM", "SFP-2.5G-T" with a yt8821 on it.
> 
> So on the OEM modules, we cannot rely solely on the two strings anymore.
> 
> Introducing the patch, would break the modules with the yt8821 on it. It
> does not have any support in mainline.
> 
> Also any code I found for the yt8821 is C22 only. And even more, even
> though we are facing with an almost similar MCU, RollBall protocol does
> not work. I think it is almost the same mcu, as it responds to the same
> eeprom password, and also the rollball password does something, but not
> give the expected result.
> 

What about I2C address 0x56?

I noticed that the Fibrestore FS SFP-2.5G-T with the realtek chip
also exposes clause 22 registers, but the current mdio-i2c driver wont
work, because the module implements the access in an incompatible way
(we would need to extend mdio-i2c).

Eric, can you check whether the motorcomm module exposes something on
address 0x56? With i2cdump, i.e. on omnia:
  i2cdump -y 5 0x56
(you will need to change 5 to the i2c controller of the sfp cage).
What does it print?

Marek

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Eric Woudstra]

On 4/23/24 10:40, Marek Behún wrote:

>>
>> Also any code I found for the yt8821 is C22 only. And even more, even
>> though we are facing with an almost similar MCU, RollBall protocol does
>> not work. I think it is almost the same mcu, as it responds to the same
>> eeprom password, and also the rollball password does something, but not
>> give the expected result.
>>
> 
> What about I2C address 0x56?
> 
> I noticed that the Fibrestore FS SFP-2.5G-T with the realtek chip
> also exposes clause 22 registers, but the current mdio-i2c driver wont
> work, because the module implements the access in an incompatible way
> (we would need to extend mdio-i2c).
> 
> Eric, can you check whether the motorcomm module exposes something on
> address 0x56? With i2cdump, i.e. on omnia:
>   i2cdump -y 5 0x56
> (you will need to change 5 to the i2c controller of the sfp cage).
> What does it print?
> 
> Marek

The device at 0x56 is not up at the time the eeprom is checked and read.
So when it is time to decide whether to use RollBall protocol or not,
the data at 0x56 cannot be used.

We have more info on i2cdumps on the BPI forum (rtl8221b topic and
yt8821 topic).

Re: [PATCH net-next 2/2] net: sfp: enhance quirk for Fibrestore 2.5G copper SFP module

[Marek Behún]

On Tue, 23 Apr 2024 10:51:09 +0200
Eric Woudstra <ericwouds@gmail.com> wrote:

> On 4/23/24 10:40, Marek Behún wrote:
> 
> >>
> >> Also any code I found for the yt8821 is C22 only. And even more, even
> >> though we are facing with an almost similar MCU, RollBall protocol does
> >> not work. I think it is almost the same mcu, as it responds to the same
> >> eeprom password, and also the rollball password does something, but not
> >> give the expected result.
> >>  
> > 
> > What about I2C address 0x56?
> > 
> > I noticed that the Fibrestore FS SFP-2.5G-T with the realtek chip
> > also exposes clause 22 registers, but the current mdio-i2c driver wont
> > work, because the module implements the access in an incompatible way
> > (we would need to extend mdio-i2c).
> > 
> > Eric, can you check whether the motorcomm module exposes something on
> > address 0x56? With i2cdump, i.e. on omnia:
> >   i2cdump -y 5 0x56
> > (you will need to change 5 to the i2c controller of the sfp cage).
> > What does it print?
> > 
> > Marek  
> 
> The device at 0x56 is not up at the time the eeprom is checked and read.
> So when it is time to decide whether to use RollBall protocol or not,
> the data at 0x56 cannot be used.

The fixup function for those modules could enable the module and wait
for the 0x56 i2c address...

> We have more info on i2cdumps on the BPI forum (rtl8221b topic and
> yt8821 topic).