netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
@ 2025-06-06  2:22 Chris Morgan
  2025-06-06 12:53 ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Morgan @ 2025-06-06  2:22 UTC (permalink / raw)
  To: netdev
  Cc: linux, andrew, hkallweit1, davem, edumazet, kuba, pabeni,
	Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add quirk for Potron SFP+ XGSPON ONU Stick (YV SFP+ONT-XGSPON).

This device uses pins 2 and 7 for UART communication, so disable
TX_FAULT and LOS. Additionally as it is an embedded system in an
SFP+ form factor provide it enough time to fully boot before we
attempt to use it.

https://www.potrontec.com/index/index/list/cat_id/2.html#11-83
https://pon.wiki/xgs-pon/ont/potron-technology/x-onu-sfpp/

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---

Changes since V1:
 - Call sfp_fixup_ignore_tx_fault() and sfp_fixup_ignore_los() instead
   of setting the state_hw_mask.

---
 drivers/net/phy/sfp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 347c1e0e94d9..a7fee449fa92 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -412,6 +412,19 @@ static void sfp_fixup_halny_gsfp(struct sfp *sfp)
 	sfp->state_hw_mask &= ~(SFP_F_TX_FAULT | SFP_F_LOS);
 }
 
+static void sfp_fixup_potron(struct sfp *sfp)
+{
+	/*
+	 * The TX_FAULT and LOS pins on this device are used for serial
+	 * communication, so ignore them. Additionally, provide extra
+	 * time for this device to fully start up.
+	 */
+
+	sfp_fixup_long_startup(sfp);
+	sfp_fixup_ignore_tx_fault(sfp);
+	sfp_fixup_ignore_los(sfp);
+}
+
 static void sfp_fixup_rollball_cc(struct sfp *sfp)
 {
 	sfp_fixup_rollball(sfp);
@@ -512,6 +525,8 @@ static const struct sfp_quirk sfp_quirks[] = {
 	SFP_QUIRK_F("Walsun", "HXSX-ATRC-1", sfp_fixup_fs_10gt),
 	SFP_QUIRK_F("Walsun", "HXSX-ATRI-1", sfp_fixup_fs_10gt),
 
+	SFP_QUIRK_F("YV", "SFP+ONU-XGSPON", sfp_fixup_potron),
+
 	// OEM SFP-GE-T is a 1000Base-T module with broken TX_FAULT indicator
 	SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06  2:22 [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick Chris Morgan
@ 2025-06-06 12:53 ` Andrew Lunn
  2025-06-06 14:44   ` Chris Morgan
  2025-06-06 16:47   ` Russell King (Oracle)
  0 siblings, 2 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-06-06 12:53 UTC (permalink / raw)
  To: Chris Morgan
  Cc: netdev, linux, hkallweit1, davem, edumazet, kuba, pabeni,
	Chris Morgan

On Thu, Jun 05, 2025 at 09:22:03PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add quirk for Potron SFP+ XGSPON ONU Stick (YV SFP+ONT-XGSPON).
> 
> This device uses pins 2 and 7 for UART communication, so disable
> TX_FAULT and LOS. Additionally as it is an embedded system in an
> SFP+ form factor provide it enough time to fully boot before we
> attempt to use it.
> 
> https://www.potrontec.com/index/index/list/cat_id/2.html#11-83
> https://pon.wiki/xgs-pon/ont/potron-technology/x-onu-sfpp/
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> 
> Changes since V1:
>  - Call sfp_fixup_ignore_tx_fault() and sfp_fixup_ignore_los() instead
>    of setting the state_hw_mask.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

You are supposed to wait 24 hours before posting a new version. We are
also in the merge window at the moment, so please post patches as RFC.

Russell asked the question, what does the SFP report about soft LOS in
its EEPROM? Does it correctly indicate it supports soft LOS? Does it
actually support soft LOS? Do we need to force on soft LOS? Maybe we
need a helper sfp_fixup_force_soft_los()?

    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 12:53 ` Andrew Lunn
@ 2025-06-06 14:44   ` Chris Morgan
  2025-06-06 15:33     ` Andrew Lunn
  2025-06-06 16:51     ` Russell King (Oracle)
  2025-06-06 16:47   ` Russell King (Oracle)
  1 sibling, 2 replies; 12+ messages in thread
From: Chris Morgan @ 2025-06-06 14:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chris Morgan, netdev, linux, hkallweit1, davem, edumazet, kuba,
	pabeni

On Fri, Jun 06, 2025 at 02:53:46PM +0200, Andrew Lunn wrote:
> On Thu, Jun 05, 2025 at 09:22:03PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add quirk for Potron SFP+ XGSPON ONU Stick (YV SFP+ONT-XGSPON).
> > 
> > This device uses pins 2 and 7 for UART communication, so disable
> > TX_FAULT and LOS. Additionally as it is an embedded system in an
> > SFP+ form factor provide it enough time to fully boot before we
> > attempt to use it.
> > 
> > https://www.potrontec.com/index/index/list/cat_id/2.html#11-83
> > https://pon.wiki/xgs-pon/ont/potron-technology/x-onu-sfpp/
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > 
> > Changes since V1:
> >  - Call sfp_fixup_ignore_tx_fault() and sfp_fixup_ignore_los() instead
> >    of setting the state_hw_mask.
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> You are supposed to wait 24 hours before posting a new version. We are
> also in the merge window at the moment, so please post patches as RFC.

Sorry, I'll make sure to slow down the commits moving forward.

> 
> Russell asked the question, what does the SFP report about soft LOS in
> its EEPROM? Does it correctly indicate it supports soft LOS? Does it
> actually support soft LOS? Do we need to force on soft LOS? Maybe we
> need a helper sfp_fixup_force_soft_los()?

So I'm a bit out of my element here and not sure how to check that. I
bought this module and had a hell of a time getting it to work on my
Banana Pi R4 because of the UART triggering repeated tx faults. After
applying these updates I was able to get it to work, so I figured it
would be a courtesy to upstream these for others not to suffer. I was
going to get this upstreamed, then request OpenWRT backport the fix,
then move this device to replace my current router (which might be why
I am guilty of rushing, sorry again). That said, I'm not sure how to
check if the module supports soft LOS or not. I did a dump with
ethtool -m but didn't see any references to LOS. Is there a bit on the
EEPROM I can check?

Thank you,
Chris

> 
>     Andrew
> 
> ---
> pw-bot: cr

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 14:44   ` Chris Morgan
@ 2025-06-06 15:33     ` Andrew Lunn
  2025-06-06 18:54       ` Chris Morgan
  2025-06-06 16:51     ` Russell King (Oracle)
  1 sibling, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2025-06-06 15:33 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Chris Morgan, netdev, linux, hkallweit1, davem, edumazet, kuba,
	pabeni

> So I'm a bit out of my element here and not sure how to check that.

No problems.

Please show us the output from ethtool -m, both the pretty printed and
the raw hex.

The relevant code is:

https://elixir.bootlin.com/linux/v6.15/source/drivers/net/phy/sfp.c#L872
https://elixir.bootlin.com/linux/v6.15/source/include/linux/sfp.h#L400

I don't think the pritty printed ethtool output shows it, but we
should be able to decode the raw hex, byte 83, bit 4. A lot depends on
how broken the SFP is. 

Or you can put a printk() in sfp_soft_start_poll().

You might also want to add #define DEBUG at the
very top of sfp.c, so you get debug prints with state changes. And
then pull the cable out/plug it in and see what gets reported.

	Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 12:53 ` Andrew Lunn
  2025-06-06 14:44   ` Chris Morgan
@ 2025-06-06 16:47   ` Russell King (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2025-06-06 16:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chris Morgan, netdev, hkallweit1, davem, edumazet, kuba, pabeni,
	Chris Morgan

On Fri, Jun 06, 2025 at 02:53:46PM +0200, Andrew Lunn wrote:
> On Thu, Jun 05, 2025 at 09:22:03PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add quirk for Potron SFP+ XGSPON ONU Stick (YV SFP+ONT-XGSPON).
> > 
> > This device uses pins 2 and 7 for UART communication, so disable
> > TX_FAULT and LOS. Additionally as it is an embedded system in an
> > SFP+ form factor provide it enough time to fully boot before we
> > attempt to use it.
> > 
> > https://www.potrontec.com/index/index/list/cat_id/2.html#11-83
> > https://pon.wiki/xgs-pon/ont/potron-technology/x-onu-sfpp/
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > 
> > Changes since V1:
> >  - Call sfp_fixup_ignore_tx_fault() and sfp_fixup_ignore_los() instead
> >    of setting the state_hw_mask.
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> You are supposed to wait 24 hours before posting a new version. We are
> also in the merge window at the moment, so please post patches as RFC.
> 
> Russell asked the question, what does the SFP report about soft LOS in
> its EEPROM? Does it correctly indicate it supports soft LOS? Does it
> actually support soft LOS? Do we need to force on soft LOS? Maybe we
> need a helper sfp_fixup_force_soft_los()?

Yep, too many people today seem to ignore questions and think they
mean "let's post a new patch". :(

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 14:44   ` Chris Morgan
  2025-06-06 15:33     ` Andrew Lunn
@ 2025-06-06 16:51     ` Russell King (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2025-06-06 16:51 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Fri, Jun 06, 2025 at 09:44:48AM -0500, Chris Morgan wrote:
> On Fri, Jun 06, 2025 at 02:53:46PM +0200, Andrew Lunn wrote:
> > On Thu, Jun 05, 2025 at 09:22:03PM -0500, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add quirk for Potron SFP+ XGSPON ONU Stick (YV SFP+ONT-XGSPON).
> > > 
> > > This device uses pins 2 and 7 for UART communication, so disable
> > > TX_FAULT and LOS. Additionally as it is an embedded system in an
> > > SFP+ form factor provide it enough time to fully boot before we
> > > attempt to use it.
> > > 
> > > https://www.potrontec.com/index/index/list/cat_id/2.html#11-83
> > > https://pon.wiki/xgs-pon/ont/potron-technology/x-onu-sfpp/
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > > 
> > > Changes since V1:
> > >  - Call sfp_fixup_ignore_tx_fault() and sfp_fixup_ignore_los() instead
> > >    of setting the state_hw_mask.
> > 
> > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> > 
> > You are supposed to wait 24 hours before posting a new version. We are
> > also in the merge window at the moment, so please post patches as RFC.
> 
> Sorry, I'll make sure to slow down the commits moving forward.
> 
> > 
> > Russell asked the question, what does the SFP report about soft LOS in
> > its EEPROM? Does it correctly indicate it supports soft LOS? Does it
> > actually support soft LOS? Do we need to force on soft LOS? Maybe we
> > need a helper sfp_fixup_force_soft_los()?
> 
> So I'm a bit out of my element here and not sure how to check that. I
> bought this module and had a hell of a time getting it to work on my
> Banana Pi R4 because of the UART triggering repeated tx faults. After
> applying these updates I was able to get it to work, so I figured it
> would be a courtesy to upstream these for others not to suffer. I was
> going to get this upstreamed, then request OpenWRT backport the fix,
> then move this device to replace my current router (which might be why
> I am guilty of rushing, sorry again). That said, I'm not sure how to
> check if the module supports soft LOS or not. I did a dump with
> ethtool -m but didn't see any references to LOS. Is there a bit on the
> EEPROM I can check?

Please send me (privately) the binary of:

ethtool -m ethX raw on > ethX.eeprom.bin

so I can see fully what it's reporting - I should then be able to make
further suggestions.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 15:33     ` Andrew Lunn
@ 2025-06-06 18:54       ` Chris Morgan
  2025-06-06 21:21         ` Russell King (Oracle)
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Morgan @ 2025-06-06 18:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chris Morgan, netdev, linux, hkallweit1, davem, edumazet, kuba,
	pabeni

On Fri, Jun 06, 2025 at 05:33:29PM +0200, Andrew Lunn wrote:
> > So I'm a bit out of my element here and not sure how to check that.
> 
> No problems.
> 
> Please show us the output from ethtool -m, both the pretty printed and
> the raw hex.
> 
> The relevant code is:
> 
> https://elixir.bootlin.com/linux/v6.15/source/drivers/net/phy/sfp.c#L872
> https://elixir.bootlin.com/linux/v6.15/source/include/linux/sfp.h#L400
> 
> I don't think the pritty printed ethtool output shows it, but we
> should be able to decode the raw hex, byte 83, bit 4. A lot depends on
> how broken the SFP is. 
> 
> Or you can put a printk() in sfp_soft_start_poll().
> 
> You might also want to add #define DEBUG at the
> very top of sfp.c, so you get debug prints with state changes. And
> then pull the cable out/plug it in and see what gets reported.

	Identifier					: 0x03 (SFP)
	Extended identifier				: 0x04 (GBIC/SFP defined by 2-wire interface ID)
	Connector					: 0x01 (SC)
	Transceiver codes				: 0x20 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
	Transceiver type				: 10G Ethernet: 10G Base-LR
	Encoding					: 0x03 (NRZ)
	BR, Nominal					: 10000MBd
	Rate identifier					: 0x00 (unspecified)
	Length (SMF,km)					: 20km
	Length (SMF)					: 20000m
	Length (50um)					: 0m
	Length (62.5um)					: 0m
	Length (Copper)					: 0m
	Length (OM3)					: 0m
	Laser wavelength				: 1270nm
	Vendor name					: YV
	Vendor OUI					: 00:00:00
	Vendor PN					: SFP+ONU-XGSPON
	Vendor rev					: A-01
	Option values					: 0x00 0x00
	BR margin, max					: 0%
	BR margin, min					: 0%
	Vendor SN					: 202504250094
	Date code					: 250425
	Optical diagnostics support			: Yes
	Laser bias current				: 0.000 mA
	Laser output power				: 0.0000 mW / -inf dBm
	Receiver signal average optical power		: 0.0000 mW / -inf dBm
	Module temperature				: 52.47 degrees C / 126.44 degrees F
	Module voltage					: 3.3912 V
	Alarm/warning flags implemented			: Yes
	Laser bias current high alarm			: Off
	Laser bias current low alarm			: On
	Laser bias current high warning			: Off
	Laser bias current low warning			: On
	Laser output power high alarm			: Off
	Laser output power low alarm			: On
	Laser output power high warning			: Off
	Laser output power low warning			: On
	Module temperature high alarm			: Off
	Module temperature low alarm			: Off
	Module temperature high warning			: Off
	Module temperature low warning			: Off
	Module voltage high alarm			: Off
	Module voltage low alarm			: Off
	Module voltage high warning			: Off
	Module voltage low warning			: Off
	Laser rx power high alarm			: Off
	Laser rx power low alarm			: On
	Laser rx power high warning			: Off
	Laser rx power low warning			: On
	Laser bias current high alarm threshold		: 60.000 mA
	Laser bias current low alarm threshold		: 0.000 mA
	Laser bias current high warning threshold	: 55.000 mA
	Laser bias current low warning threshold	: 0.000 mA
	Laser output power high alarm threshold		: 6.5535 mW / 8.16 dBm
	Laser output power low alarm threshold		: 1.5848 mW / 2.00 dBm
	Laser output power high warning threshold	: 6.5535 mW / 8.16 dBm
	Laser output power low warning threshold	: 1.9952 mW / 3.00 dBm
	Module temperature high alarm threshold		: 90.00 degrees C / 194.00 degrees F
	Module temperature low alarm threshold		: -50.00 degrees C / -58.00 degrees F
	Module temperature high warning threshold	: 85.00 degrees C / 185.00 degrees F
	Module temperature low warning threshold	: -45.00 degrees C / -49.00 degrees F
	Module voltage high alarm threshold		: 3.6000 V
	Module voltage low alarm threshold		: 3.0000 V
	Module voltage high warning threshold		: 3.4700 V
	Module voltage low warning threshold		: 3.1300 V
	Laser rx power high alarm threshold		: 0.1995 mW / -7.00 dBm
	Laser rx power low alarm threshold		: 0.0011 mW / -29.59 dBm
	Laser rx power high warning threshold		: 0.1584 mW / -8.00 dBm
	Laser rx power low warning threshold		: 0.0014 mW / -28.54 dBm

I'll send the bin dump in another message (privately). Since the OUI
is 00:00:00 and the serial number appears to be a datestamp, I'm not
seeing anything on here that's sensitive.

Thank you,
Chris

> 
> 	Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 18:54       ` Chris Morgan
@ 2025-06-06 21:21         ` Russell King (Oracle)
  2025-06-06 22:32           ` Chris Morgan
  0 siblings, 1 reply; 12+ messages in thread
From: Russell King (Oracle) @ 2025-06-06 21:21 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Fri, Jun 06, 2025 at 01:54:27PM -0500, Chris Morgan wrote:
> 	Option values					: 0x00 0x00

This suggests that LOS is not supported, nor any of the other hardware
signals. However, because early revisions of the SFP MSA didn't have
an option byte, and thus was zero, but did have the hardware signals,
we can't simply take this to mean the signals aren't implemented,
except for RX_LOS.

> I'll send the bin dump in another message (privately). Since the OUI
> is 00:00:00 and the serial number appears to be a datestamp, I'm not
> seeing anything on here that's sensitive.

I have augmented tools which can parse the binary dump, so I get a
bit more decode:

        Enhanced Options                          : soft TX_DISABLE
        Enhanced Options                          : soft TX_FAULT
        Enhanced Options                          : soft RX_LOS

So, this tells sfp.c that the status bits in the diagnostics address
offset 110 (SFP_STATUS) are supported.

Digging into your binary dump, SFP_STATUS has the value 0x02, which
indicates RX_LOS is set (signal lost), but TX_FAULT is clear (no
transmit fault.)

I'm guessing the SFP didn't have link at the time you took this
dump given that SFP_STATUS indicates RX_LOS was set?

Now, the problem with clearing bits in ->state_hw_mask is that
leads the SFP code to think "this hardware signal isn't implemented,
so I'll use the software specified signal instead where the module
indicates support via the enhanced options."

Setting bits in ->state_ignore_mask means that *both* the hardware
and software signals will be ignored, and if RX_LOS is ignored,
then the "Options" word needs to be updated to ensure that neither
inverted or normal LOS is reported there to avoid the state machines
waiting indefinitely for LOS to change. That is handled by
sfp_fixup_ignore_los().

If the soft bits in SFP_STATUS is reliable, then clearing the
appropriate flags in ->state_hw_mask for the hardware signals is
fine.

However, we have seen modules where this is not the case, and the
software bits seem to follow the wiggling of the hardware lines.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 21:21         ` Russell King (Oracle)
@ 2025-06-06 22:32           ` Chris Morgan
  2025-06-06 22:47             ` Russell King (Oracle)
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Morgan @ 2025-06-06 22:32 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Fri, Jun 06, 2025 at 10:21:37PM +0100, Russell King (Oracle) wrote:
> On Fri, Jun 06, 2025 at 01:54:27PM -0500, Chris Morgan wrote:
> > 	Option values					: 0x00 0x00
> 
> This suggests that LOS is not supported, nor any of the other hardware
> signals. However, because early revisions of the SFP MSA didn't have
> an option byte, and thus was zero, but did have the hardware signals,
> we can't simply take this to mean the signals aren't implemented,
> except for RX_LOS.
> 
> > I'll send the bin dump in another message (privately). Since the OUI
> > is 00:00:00 and the serial number appears to be a datestamp, I'm not
> > seeing anything on here that's sensitive.
> 
> I have augmented tools which can parse the binary dump, so I get a
> bit more decode:
> 
>         Enhanced Options                          : soft TX_DISABLE
>         Enhanced Options                          : soft TX_FAULT
>         Enhanced Options                          : soft RX_LOS
> 
> So, this tells sfp.c that the status bits in the diagnostics address
> offset 110 (SFP_STATUS) are supported.
> 
> Digging into your binary dump, SFP_STATUS has the value 0x02, which
> indicates RX_LOS is set (signal lost), but TX_FAULT is clear (no
> transmit fault.)
> 
> I'm guessing the SFP didn't have link at the time you took this
> dump given that SFP_STATUS indicates RX_LOS was set?
> 

That is correct.

> Now, the problem with clearing bits in ->state_hw_mask is that
> leads the SFP code to think "this hardware signal isn't implemented,
> so I'll use the software specified signal instead where the module
> indicates support via the enhanced options."
> 
> Setting bits in ->state_ignore_mask means that *both* the hardware
> and software signals will be ignored, and if RX_LOS is ignored,
> then the "Options" word needs to be updated to ensure that neither
> inverted or normal LOS is reported there to avoid the state machines
> waiting indefinitely for LOS to change. That is handled by
> sfp_fixup_ignore_los().
> 
> If the soft bits in SFP_STATUS is reliable, then clearing the
> appropriate flags in ->state_hw_mask for the hardware signals is
> fine.

I'll test this out more and resubmit once I confirm that simply setting
state_hw_mask (which means we don't do it in hardware) works just the
same on my device as state_ignore_mask. So if I understand correctly
that means we're doing the following:

sfp_fixup_long_startup(sfp);
sfp->state_hw_mask &= ~(SFP_F_TX_FAULT | SFP_F_LOS);

The long startup solves for the problem that the SFP+ device has to
boot up; and the state_hw_mask solves for the TX and LOS hardware
pins being used for UART but software TX fault and LOS still working.

> 
> However, we have seen modules where this is not the case, and the
> software bits seem to follow the wiggling of the hardware lines.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Thank you for all your help,
Chris

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 22:32           ` Chris Morgan
@ 2025-06-06 22:47             ` Russell King (Oracle)
  2025-06-11  3:43               ` Chris Morgan
  0 siblings, 1 reply; 12+ messages in thread
From: Russell King (Oracle) @ 2025-06-06 22:47 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Fri, Jun 06, 2025 at 05:32:43PM -0500, Chris Morgan wrote:
> On Fri, Jun 06, 2025 at 10:21:37PM +0100, Russell King (Oracle) wrote:
> > On Fri, Jun 06, 2025 at 01:54:27PM -0500, Chris Morgan wrote:
> > > 	Option values					: 0x00 0x00
> > 
> > This suggests that LOS is not supported, nor any of the other hardware
> > signals. However, because early revisions of the SFP MSA didn't have
> > an option byte, and thus was zero, but did have the hardware signals,
> > we can't simply take this to mean the signals aren't implemented,
> > except for RX_LOS.
> > 
> > > I'll send the bin dump in another message (privately). Since the OUI
> > > is 00:00:00 and the serial number appears to be a datestamp, I'm not
> > > seeing anything on here that's sensitive.
> > 
> > I have augmented tools which can parse the binary dump, so I get a
> > bit more decode:
> > 
> >         Enhanced Options                          : soft TX_DISABLE
> >         Enhanced Options                          : soft TX_FAULT
> >         Enhanced Options                          : soft RX_LOS
> > 
> > So, this tells sfp.c that the status bits in the diagnostics address
> > offset 110 (SFP_STATUS) are supported.
> > 
> > Digging into your binary dump, SFP_STATUS has the value 0x02, which
> > indicates RX_LOS is set (signal lost), but TX_FAULT is clear (no
> > transmit fault.)
> > 
> > I'm guessing the SFP didn't have link at the time you took this
> > dump given that SFP_STATUS indicates RX_LOS was set?
> > 
> 
> That is correct.

Are you able to confirm that SFP_STATUS RX_LOS clears when the
module has link?

> > Now, the problem with clearing bits in ->state_hw_mask is that
> > leads the SFP code to think "this hardware signal isn't implemented,
> > so I'll use the software specified signal instead where the module
> > indicates support via the enhanced options."
> > 
> > Setting bits in ->state_ignore_mask means that *both* the hardware
> > and software signals will be ignored, and if RX_LOS is ignored,
> > then the "Options" word needs to be updated to ensure that neither
> > inverted or normal LOS is reported there to avoid the state machines
> > waiting indefinitely for LOS to change. That is handled by
> > sfp_fixup_ignore_los().
> > 
> > If the soft bits in SFP_STATUS is reliable, then clearing the
> > appropriate flags in ->state_hw_mask for the hardware signals is
> > fine.
> 
> I'll test this out more and resubmit once I confirm that simply setting
> state_hw_mask (which means we don't do it in hardware) works just the
> same on my device as state_ignore_mask. So if I understand correctly
> that means we're doing the following:
> 
> sfp_fixup_long_startup(sfp);
> sfp->state_hw_mask &= ~(SFP_F_TX_FAULT | SFP_F_LOS);
> 
> The long startup solves for the problem that the SFP+ device has to
> boot up; and the state_hw_mask solves for the TX and LOS hardware
> pins being used for UART but software TX fault and LOS still working.

I'd prefer to have an additional couple of functions:

sfp_fixup_ignore_hw_tx_fault()
sfp_fixup_ignore_hw_los()

or possibly:

sfp_fixup_ignore_hw(struct sfp *sfp, unsigned int mask)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-06 22:47             ` Russell King (Oracle)
@ 2025-06-11  3:43               ` Chris Morgan
  2025-06-11  8:31                 ` Russell King (Oracle)
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Morgan @ 2025-06-11  3:43 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Fri, Jun 06, 2025 at 11:47:00PM +0100, Russell King (Oracle) wrote:
> On Fri, Jun 06, 2025 at 05:32:43PM -0500, Chris Morgan wrote:
> > On Fri, Jun 06, 2025 at 10:21:37PM +0100, Russell King (Oracle) wrote:
> > > On Fri, Jun 06, 2025 at 01:54:27PM -0500, Chris Morgan wrote:
> > > > 	Option values					: 0x00 0x00
> > > 
> > > This suggests that LOS is not supported, nor any of the other hardware
> > > signals. However, because early revisions of the SFP MSA didn't have
> > > an option byte, and thus was zero, but did have the hardware signals,
> > > we can't simply take this to mean the signals aren't implemented,
> > > except for RX_LOS.
> > > 
> > > > I'll send the bin dump in another message (privately). Since the OUI
> > > > is 00:00:00 and the serial number appears to be a datestamp, I'm not
> > > > seeing anything on here that's sensitive.
> > > 
> > > I have augmented tools which can parse the binary dump, so I get a
> > > bit more decode:
> > > 
> > >         Enhanced Options                          : soft TX_DISABLE
> > >         Enhanced Options                          : soft TX_FAULT
> > >         Enhanced Options                          : soft RX_LOS
> > > 
> > > So, this tells sfp.c that the status bits in the diagnostics address
> > > offset 110 (SFP_STATUS) are supported.
> > > 
> > > Digging into your binary dump, SFP_STATUS has the value 0x02, which
> > > indicates RX_LOS is set (signal lost), but TX_FAULT is clear (no
> > > transmit fault.)
> > > 
> > > I'm guessing the SFP didn't have link at the time you took this
> > > dump given that SFP_STATUS indicates RX_LOS was set?
> > > 
> > 
> > That is correct.
> 
> Are you able to confirm that SFP_STATUS RX_LOS clears when the
> module has link?

I believe this is the case. I've sent you a dump of my EEPROM when the
SFP+ is active (it's now powering my internet connection at home) in a
private message to confirm.

> 
> > > Now, the problem with clearing bits in ->state_hw_mask is that
> > > leads the SFP code to think "this hardware signal isn't implemented,
> > > so I'll use the software specified signal instead where the module
> > > indicates support via the enhanced options."
> > > 
> > > Setting bits in ->state_ignore_mask means that *both* the hardware
> > > and software signals will be ignored, and if RX_LOS is ignored,
> > > then the "Options" word needs to be updated to ensure that neither
> > > inverted or normal LOS is reported there to avoid the state machines
> > > waiting indefinitely for LOS to change. That is handled by
> > > sfp_fixup_ignore_los().
> > > 
> > > If the soft bits in SFP_STATUS is reliable, then clearing the
> > > appropriate flags in ->state_hw_mask for the hardware signals is
> > > fine.
> > 
> > I'll test this out more and resubmit once I confirm that simply setting
> > state_hw_mask (which means we don't do it in hardware) works just the
> > same on my device as state_ignore_mask. So if I understand correctly
> > that means we're doing the following:
> > 
> > sfp_fixup_long_startup(sfp);
> > sfp->state_hw_mask &= ~(SFP_F_TX_FAULT | SFP_F_LOS);
> > 
> > The long startup solves for the problem that the SFP+ device has to
> > boot up; and the state_hw_mask solves for the TX and LOS hardware
> > pins being used for UART but software TX fault and LOS still working.
> 
> I'd prefer to have an additional couple of functions:
> 
> sfp_fixup_ignore_hw_tx_fault()
> sfp_fixup_ignore_hw_los()
> 
> or possibly:
> 
> sfp_fixup_ignore_hw(struct sfp *sfp, unsigned int mask)
> 

Which of these would you prefer? Do you want a function for each
scenario or just a generic sfp_fixup_ignore_hw_fault_signal()? I can 
create functions for each and then apply them to my device (and
probably update the sfp_fixup_halny_gsfp() too since it's identical to
what I'm trying to do plus the delay bits).

> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Thank you,
Chris

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick
  2025-06-11  3:43               ` Chris Morgan
@ 2025-06-11  8:31                 ` Russell King (Oracle)
  0 siblings, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2025-06-11  8:31 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Andrew Lunn, Chris Morgan, netdev, hkallweit1, davem, edumazet,
	kuba, pabeni

On Tue, Jun 10, 2025 at 10:43:31PM -0500, Chris Morgan wrote:
> On Fri, Jun 06, 2025 at 11:47:00PM +0100, Russell King (Oracle) wrote:
> > On Fri, Jun 06, 2025 at 05:32:43PM -0500, Chris Morgan wrote:
> > > On Fri, Jun 06, 2025 at 10:21:37PM +0100, Russell King (Oracle) wrote:
> > > > On Fri, Jun 06, 2025 at 01:54:27PM -0500, Chris Morgan wrote:
> > > > > 	Option values					: 0x00 0x00
> > > > 
> > > > This suggests that LOS is not supported, nor any of the other hardware
> > > > signals. However, because early revisions of the SFP MSA didn't have
> > > > an option byte, and thus was zero, but did have the hardware signals,
> > > > we can't simply take this to mean the signals aren't implemented,
> > > > except for RX_LOS.
> > > > 
> > > > > I'll send the bin dump in another message (privately). Since the OUI
> > > > > is 00:00:00 and the serial number appears to be a datestamp, I'm not
> > > > > seeing anything on here that's sensitive.
> > > > 
> > > > I have augmented tools which can parse the binary dump, so I get a
> > > > bit more decode:
> > > > 
> > > >         Enhanced Options                          : soft TX_DISABLE
> > > >         Enhanced Options                          : soft TX_FAULT
> > > >         Enhanced Options                          : soft RX_LOS
> > > > 
> > > > So, this tells sfp.c that the status bits in the diagnostics address
> > > > offset 110 (SFP_STATUS) are supported.
> > > > 
> > > > Digging into your binary dump, SFP_STATUS has the value 0x02, which
> > > > indicates RX_LOS is set (signal lost), but TX_FAULT is clear (no
> > > > transmit fault.)
> > > > 
> > > > I'm guessing the SFP didn't have link at the time you took this
> > > > dump given that SFP_STATUS indicates RX_LOS was set?
> > > > 
> > > 
> > > That is correct.
> > 
> > Are you able to confirm that SFP_STATUS RX_LOS clears when the
> > module has link?
> 
> I believe this is the case. I've sent you a dump of my EEPROM when the
> SFP+ is active (it's now powering my internet connection at home) in a
> private message to confirm.

Yes, I can confirm this. The RX_LOS bit on SFP_STATUS appears to work
correctly, so all we need to do is ignore the hardware signal(s).

> > I'd prefer to have an additional couple of functions:
> > 
> > sfp_fixup_ignore_hw_tx_fault()
> > sfp_fixup_ignore_hw_los()
> > 
> > or possibly:
> > 
> > sfp_fixup_ignore_hw(struct sfp *sfp, unsigned int mask)
> > 
> 
> Which of these would you prefer? Do you want a function for each
> scenario or just a generic sfp_fixup_ignore_hw_fault_signal()? I can 
> create functions for each and then apply them to my device (and
> probably update the sfp_fixup_halny_gsfp() too since it's identical to
> what I'm trying to do plus the delay bits).

I think the latter as it's more flexible and less code.

Yes, please update sfp_fixup_halny_gsfp() as well.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-06-11  8:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-06  2:22 [PATCH V2] net: sfp: add quirk for Potron SFP+ XGSPON ONU Stick Chris Morgan
2025-06-06 12:53 ` Andrew Lunn
2025-06-06 14:44   ` Chris Morgan
2025-06-06 15:33     ` Andrew Lunn
2025-06-06 18:54       ` Chris Morgan
2025-06-06 21:21         ` Russell King (Oracle)
2025-06-06 22:32           ` Chris Morgan
2025-06-06 22:47             ` Russell King (Oracle)
2025-06-11  3:43               ` Chris Morgan
2025-06-11  8:31                 ` Russell King (Oracle)
2025-06-06 16:51     ` Russell King (Oracle)
2025-06-06 16:47   ` Russell King (Oracle)

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).