SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[George McCollister]

Every example of phylink SFP support I've seen is using an Ethernet
MAC with native SGMII.
Can phylink facilitate support of Fiber and Copper SFP modules
connected to an RGMII MAC if all of the following are true?

1) The MAC is connected via RGMII to a transceiver/PHY (such as
Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
you want to see a block diagram it's the first one here:
https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf

2) The 1G Ethernet driver has been converted to use phylink.

3) An I2C controller on the SoC is connected to the SFP cage.

4) TxFault, LOS and MOD-DEF0 are connected to GPIO on the SoC.

5) MDIO is connected to the intermediate PHY.

Any thoughts on what might be missing to support this (if anything)
would be appreciated.

Cheers,
George McCollister

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[Florian Fainelli]

+Russell, Andrew, Heiner,

On 9/13/2019 9:44 AM, George McCollister wrote:
> Every example of phylink SFP support I've seen is using an Ethernet
> MAC with native SGMII.
> Can phylink facilitate support of Fiber and Copper SFP modules
> connected to an RGMII MAC if all of the following are true?

I don't think that use case has been presented before, but phylink
sounds like the tool that should help solve it. From your description
below, it sounds like all the pieces are there to support it. Is the
Ethernet MAC driver upstream?

> 
> 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> you want to see a block diagram it's the first one here:
> https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> 
> 2) The 1G Ethernet driver has been converted to use phylink.
> 
> 3) An I2C controller on the SoC is connected to the SFP cage.
> 
> 4) TxFault, LOS and MOD-DEF0 are connected to GPIO on the SoC.
> 
> 5) MDIO is connected to the intermediate PHY.
> 
> Any thoughts on what might be missing to support this (if anything)
> would be appreciated-- 
Florian

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[Russell King - ARM Linux admin]

On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> +Russell, Andrew, Heiner,
> 
> On 9/13/2019 9:44 AM, George McCollister wrote:
> > Every example of phylink SFP support I've seen is using an Ethernet
> > MAC with native SGMII.
> > Can phylink facilitate support of Fiber and Copper SFP modules
> > connected to an RGMII MAC if all of the following are true?
> 
> I don't think that use case has been presented before, but phylink
> sounds like the tool that should help solve it. From your description
> below, it sounds like all the pieces are there to support it. Is the
> Ethernet MAC driver upstream?

It has been presented, and it's something I've been trying to support
for the last couple of years - in fact, I have patches in my tree that
support a very similar scenario on the Macchiatobin with the 88x3310
PHYs.

> > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > you want to see a block diagram it's the first one here:
> > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf

As mentioned above, this is no different from the Macchiatobin,
where we have:

                  .-------- RJ45
MAC ---- 88x3310 PHY
                  `-------- SFP+

except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.

Note that you're abusing the term "SGMII".  SGMII is a Cisco
modification of the IEEE 802.3 1000BASE-X protocol.  Fiber SFPs
exclusively use 1000BASE-X protocol.  However, some copper SFPs
(with a RJ45) do use SGMII.

> > 2) The 1G Ethernet driver has been converted to use phylink.

This is not necessary for this scenario.  The PHY driver needs to
be updated to know about SFP though.

See:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4

as an example of the 88x3310 supporting a SFP+ cage.  This patch is
also necessary:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990

and if anything is going to stand in the way of progress on this, it
is likely to be that patch.  I'll be attempting to post these after
the next merge window (i.o.w. probably posting them in three weeks
time.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[Florian Fainelli]

On 9/14/2019 1:48 AM, Russell King - ARM Linux admin wrote:
> On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
>> +Russell, Andrew, Heiner,
>>
>> On 9/13/2019 9:44 AM, George McCollister wrote:
>>> Every example of phylink SFP support I've seen is using an Ethernet
>>> MAC with native SGMII.
>>> Can phylink facilitate support of Fiber and Copper SFP modules
>>> connected to an RGMII MAC if all of the following are true?
>>
>> I don't think that use case has been presented before, but phylink
>> sounds like the tool that should help solve it. From your description
>> below, it sounds like all the pieces are there to support it. Is the
>> Ethernet MAC driver upstream?
> 
> It has been presented, and it's something I've been trying to support
> for the last couple of years - in fact, I have patches in my tree that
> support a very similar scenario on the Macchiatobin with the 88x3310
> PHYs.
> 
>>> 1) The MAC is connected via RGMII to a transceiver/PHY (such as
>>> Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
>>> you want to see a block diagram it's the first one here:
>>> https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> 
> As mentioned above, this is no different from the Macchiatobin,
> where we have:
> 
>                   .-------- RJ45
> MAC ---- 88x3310 PHY
>                   `-------- SFP+
> 
> except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
> 
> Note that you're abusing the term "SGMII".  SGMII is a Cisco
> modification of the IEEE 802.3 1000BASE-X protocol.  Fiber SFPs
> exclusively use 1000BASE-X protocol.  However, some copper SFPs
> (with a RJ45) do use SGMII.
> 
>>> 2) The 1G Ethernet driver has been converted to use phylink.
> 
> This is not necessary for this scenario.  The PHY driver needs to
> be updated to know about SFP though.
> 
> See:
> 
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4

Regarding that patch, the SFP attach/detach callbacks do not seem very
specific to the PHY driver, only the sfp_insert callback which needs to
check the interface selected by the SFP.

Do you think it would make sense to move some of that logic into the
core PHY library and only have PHY drivers can be used to connect a SFP
cage specify a "sfp_select_interface" callback that is responsible for
rejecting the mode the SFP has been configured in, if unsupported?
Likewise for parsing the "sfp" property, if we parse that property in
the core and do not have a sfp_select_interface callback defined, then
it is not going to work.

So far we know that both marvel10g.c and marvell.c, two drivers that
would already justify factoring things in the core, I recall some people
using Broadcom PHYs having the same use cases, so 3 possible drivers
needing that functionality.

> 
> as an example of the 88x3310 supporting a SFP+ cage.  This patch is
> also necessary:
> 
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990
> 
> and if anything is going to stand in the way of progress on this, it
> is likely to be that patch.  I'll be attempting to post these after
> the next merge window (i.o.w. probably posting them in three weeks
> time.)


-- 
Florian

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[Russell King - ARM Linux admin]

On Sat, Sep 14, 2019 at 10:15:26AM -0700, Florian Fainelli wrote:
> 
> 
> On 9/14/2019 1:48 AM, Russell King - ARM Linux admin wrote:
> > On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> >> +Russell, Andrew, Heiner,
> >>
> >> On 9/13/2019 9:44 AM, George McCollister wrote:
> >>> Every example of phylink SFP support I've seen is using an Ethernet
> >>> MAC with native SGMII.
> >>> Can phylink facilitate support of Fiber and Copper SFP modules
> >>> connected to an RGMII MAC if all of the following are true?
> >>
> >> I don't think that use case has been presented before, but phylink
> >> sounds like the tool that should help solve it. From your description
> >> below, it sounds like all the pieces are there to support it. Is the
> >> Ethernet MAC driver upstream?
> > 
> > It has been presented, and it's something I've been trying to support
> > for the last couple of years - in fact, I have patches in my tree that
> > support a very similar scenario on the Macchiatobin with the 88x3310
> > PHYs.
> > 
> >>> 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> >>> Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> >>> you want to see a block diagram it's the first one here:
> >>> https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> > 
> > As mentioned above, this is no different from the Macchiatobin,
> > where we have:
> > 
> >                   .-------- RJ45
> > MAC ---- 88x3310 PHY
> >                   `-------- SFP+
> > 
> > except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> > and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
> > 
> > Note that you're abusing the term "SGMII".  SGMII is a Cisco
> > modification of the IEEE 802.3 1000BASE-X protocol.  Fiber SFPs
> > exclusively use 1000BASE-X protocol.  However, some copper SFPs
> > (with a RJ45) do use SGMII.
> > 
> >>> 2) The 1G Ethernet driver has been converted to use phylink.
> > 
> > This is not necessary for this scenario.  The PHY driver needs to
> > be updated to know about SFP though.
> > 
> > See:
> > 
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4
> 
> Regarding that patch, the SFP attach/detach callbacks do not seem very
> specific to the PHY driver, only the sfp_insert callback which needs to
> check the interface selected by the SFP.
> 
> Do you think it would make sense to move some of that logic into the
> core PHY library and only have PHY drivers can be used to connect a SFP
> cage specify a "sfp_select_interface" callback that is responsible for
> rejecting the mode the SFP has been configured in, if unsupported?

It's not that simple.  The Marvell 1G PHYs which have a fiber interface
re-use the fiber interface for SGMII when configured for such a mode.
It's not as simple as "did the driver specify a callback for this
feature" but "does the PHY support a fiber interface _and_ does the PHY
configuration allow the fiber interface to be used?"  So, I think the
PHY driver needs to have a say (in terms of code) whether there is
support for fiber.

However, it'd be silly to specify a sfp property in a situation where
the fiber interface on a PHY can't be used.

In any case, the callback into the PHY driver needs to be as per the
"sfp_insert" method - some PHYs will only be able to support a limited
number of SFPs.  It seems, for example, the 88x3310 can support more
than just 10G modules - it allegedly can support 2500base-X, 1000base-X
and SGMII modules too if we hit it hard enough.

> Likewise for parsing the "sfp" property, if we parse that property in
> the core and do not have a sfp_select_interface callback defined, then
> it is not going to work.

Today, I've moved parsing the "sfp" property into sfp-bus, so that's no
longer a concern.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[George McCollister]

On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > +Russell, Andrew, Heiner,
> >
> > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > Every example of phylink SFP support I've seen is using an Ethernet
> > > MAC with native SGMII.
> > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > connected to an RGMII MAC if all of the following are true?
> >
> > I don't think that use case has been presented before, but phylink
> > sounds like the tool that should help solve it. From your description
> > below, it sounds like all the pieces are there to support it. Is the
> > Ethernet MAC driver upstream?
>
> It has been presented, and it's something I've been trying to support
> for the last couple of years - in fact, I have patches in my tree that
> support a very similar scenario on the Macchiatobin with the 88x3310
> PHYs.
>
> > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > you want to see a block diagram it's the first one here:
> > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
>
> As mentioned above, this is no different from the Macchiatobin,
> where we have:
>
>                   .-------- RJ45
> MAC ---- 88x3310 PHY
>                   `-------- SFP+
>
> except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
>
> Note that you're abusing the term "SGMII".  SGMII is a Cisco
> modification of the IEEE 802.3 1000BASE-X protocol.  Fiber SFPs
> exclusively use 1000BASE-X protocol.  However, some copper SFPs
> (with a RJ45) do use SGMII.
>
> > > 2) The 1G Ethernet driver has been converted to use phylink.
>
> This is not necessary for this scenario.  The PHY driver needs to
> be updated to know about SFP though.

Excellent, this is exactly the information I was looking for. I had
started converting the Ethernet driver to phylink but there was still
a lot of work to do and I fear there was a significant potential for
regressions. I'll abandon that and see if I can get it to work by
making similar changes to the 1G Marvell PHY driver.

I'm assuming I must set the sfp property of the PHY in DT instead of the MAC.

>
> See:
>
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4
>
> as an example of the 88x3310 supporting a SFP+ cage.  This patch is
> also necessary:
>
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990

Perfect.

>
> and if anything is going to stand in the way of progress on this, it
> is likely to be that patch.  I'll be attempting to post these after
> the next merge window (i.o.w. probably posting them in three weeks
> time.)

Please CC me on these patches as I'll follow your lead for what I do
on the 1G Marvell PHY driver. If you don't remember, that's fine, I
understand.

>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

Thanks,
George

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[George McCollister]

Russell,

On Mon, Sep 16, 2019 at 10:40 AM George McCollister
<george.mccollister@gmail.com> wrote:
>
> On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > > +Russell, Andrew, Heiner,
> > >
> > > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > > Every example of phylink SFP support I've seen is using an Ethernet
> > > > MAC with native SGMII.
> > > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > > connected to an RGMII MAC if all of the following are true?
> > >
> > > I don't think that use case has been presented before, but phylink
> > > sounds like the tool that should help solve it. From your description
> > > below, it sounds like all the pieces are there to support it. Is the
> > > Ethernet MAC driver upstream?
> >
> > It has been presented, and it's something I've been trying to support
> > for the last couple of years - in fact, I have patches in my tree that
> > support a very similar scenario on the Macchiatobin with the 88x3310
> > PHYs.
> >
> > > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > > you want to see a block diagram it's the first one here:
> > > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> >
> > As mentioned above, this is no different from the Macchiatobin,
> > where we have:
> >
> >                   .-------- RJ45
> > MAC ---- 88x3310 PHY
> >                   `-------- SFP+
> >
> > except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> > and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.

Did you test with an SFP+ module that has a PHY?

In my setup, nothing connects/attaches to the PHY (88E1111) in the
RJ45 SFP module I'm testing with (is this intended?). Apparently since
sfp_upstream_ops in the PHY driver used for the first PHY doesn't
provide a .connect_phy. This leaves .phy_link_change NULL. Eventually
phy_link_down tries to call .phy_link_change resulting in a NULL
pointer deference OOPs. If phy_link_change doesn't need to be called
for the second PHY I can just send a patch that doesn't call it if
it's NULL.

This is what I did:

I applied the following on top of net-next:
net: sfp: move fwnode parsing into sfp-bus layer
net: phy: provide phy driver start/stop hooks
net: phy: add core phylib sfp support

I then added SFP support to the marvell.c PHY driver like you did to
marvell10g.c. Just like with marvell10g.c I only provide .attach,
.detach and .module_insert sfp_upstream_ops.

I moved the sfp property from my Ethernet MAC to the PHY in DT (my DT
is a WIP and not upstream):
                ethphy1: ethernet-phy@1 {
                        reg = <1>;
+                       sfp = <&sfp>;
                };

Here is the console output, I've added some prints for debugging:

[    1.867336] sfp sfp: sfp_probe
[    1.870462] sfp sfp: sfp_probe - of_node
[    1.874520] libphy: SFP I2C Bus: probed
[    1.880290] sfp sfp: Host maximum power 2.0W
[    1.886681] sfp sfp: No tx_disable pin: SFP modules will always be emitting.
[    1.893771] sfp sfp: sfp_probe - returning 0
[    1.898392] libphy: Fixed MDIO Bus: probed
[    1.909454] fec 2188000.ethernet: Invalid MAC address: 00:00:00:00:00:00
[    1.916208] fec 2188000.ethernet: Using random MAC address: 62:2f:f1:55:56:dc
[    1.924075] libphy: fec_enet_mii_bus: probed
[    1.935382] libphy: phy_probe
[    1.938484] Marvell 88E1510 2188000.ethernet-1:00: marvell_probe
[    1.944561] Marvell 88E1510 2188000.ethernet-1:00: marvell_probe have fwnode
[    1.951688] sfp_register_upstream_node
[    1.955500] Marvell 88E1510 2188000.ethernet-1:00: sfp_bus = 00000000
[    1.968648] libphy: phy_probe
[    1.971745] Marvell 88E1510 2188000.ethernet-1:01: marvell_probe
[    1.977828] Marvell 88E1510 2188000.ethernet-1:01: marvell_probe have fwnode
[    1.984953] sfp_register_upstream_node
[    1.988895] sfp sfp: SM: enter empty:down:down event insert
[    1.994536] sfp sfp: SM: attached
[    1.997915] Marvell 88E1510 2188000.ethernet-1:01: marvell_sfp_attach
[    2.004415] Marvell 88E1510 2188000.ethernet-1:01: sfp_bus = de0ca240
[    2.012185] fec 2188000.ethernet eth0: registered PHC device 0
[    2.024656] fec 21b4000.ethernet: Invalid MAC address: 00:00:00:00:00:00
[    2.031461] fec 21b4000.ethernet: Using random MAC address: ce:e8:ce:0a:4e:3c
[    2.246517] libphy: fec_enet_mii_bus: probed
[    2.251440] fec 21b4000.ethernet eth1: registered PHC device 1
[    2.357150] sfp sfp: module OEM              SFP-GE-T         rev
   sn CSGETJ53492      dc 19050101
[    2.366568] Marvell 88E1510 2188000.ethernet-1:01: marvell_sfp_insert
[    2.373093] sfp sfp: Unknown/unsupported extended compliance code: 0x01

# ifconfig eth1 192.168.0.1
[   70.011960] libphy: phy_connect_direct
[   70.015830] Marvell 88E1510 2188000.ethernet-1:01: phy_attach_direct
[   70.025315] Marvell 88E1510 2188000.ethernet-1:01: marvell_config_init
[   70.034271] libphy: phy_resume
[   70.037955] Marvell 88E1510 2188000.ethernet-1:01: attached PHY
driver [Marvell 88E1510] (mii_bus:phy_addr=2188000.ethernet-1:01,
irq=POLL)
[   70.050591] Marvell 88E1510 2188000.ethernet-1:01: phy_start
[   70.060101] sfp sfp: SM: enter present:down:down event dev_up
[   70.131133] libphy: phy_probe
[   70.139706] Marvell 88E1111 i2c:sfp:16: marvell_probe
[   70.155806] Marvell 88E1111 i2c:sfp:16: sfp_add_phy - didnt call
connect_phy, ops = c0a4ca10
[   70.172962] sfp sfp: sfp_sm_probe_phy - calling phy_start
[   70.183163] Marvell 88E1111 i2c:sfp:16: phy_start
[   70.190973] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = UP
[   70.197994] Marvell 88E1510 2188000.ethernet-1:01:
phy_state_machine state = UP
[   70.213607] Marvell 88E1510 2188000.ethernet-1:01: phy_start_aneg -
state = UP
[   70.225367] Marvell 88E1510 2188000.ethernet-1:01: phy_link_down -
calling phy_link_change
[   70.233864] Marvell 88E1510 2188000.ethernet-1:01: phy_link_change
[   70.243735] Marvell 88E1510 2188000.ethernet-1:01: PHY state change
UP -> NOLINK
[   70.251805] Marvell 88E1111 i2c:sfp:16: phy_start_aneg - state = UP
[   70.266411] Marvell 88E1111 i2c:sfp:16: phy_link_down -
phy_link_change is NULL and would be called !!!!!!!!!!!!!

It would have attempted to call the NULL .phy_link_change here but I
added a check to prevent it.

When I plug in the RJ45 cable it comes up and I'm able to ping.
[   88.282725] Marvell 88E1510 2188000.ethernet-1:01: phy_link_up -
calling phy_link_change
[   88.294628] Marvell 88E1510 2188000.ethernet-1:01: phy_link_change
[   88.303103] fec 21b4000.ethernet eth1: Link is Up - 1Gbps/Full -
flow control off
[   88.312487] Marvell 88E1510 2188000.ethernet-1:01: PHY state change
NOLINK -> RUNNING
[   88.338803] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = NOLINK
[   89.364015] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = NOLINK
[   89.372943] Marvell 88E1510 2188000.ethernet-1:01:
phy_state_machine state = RUNNING
[   89.386467] Marvell 88E1111 i2c:sfp:16: phy_link_up -
phy_link_change is NULL and would be called !!!!!!!!!!!!!!!

It would have attempted to call the NULL .phy_link_change again here too.

[   89.394896] Marvell 88E1111 i2c:sfp:16: PHY state change NOLINK -> RUNNING

> >
> > Note that you're abusing the term "SGMII".  SGMII is a Cisco
> > modification of the IEEE 802.3 1000BASE-X protocol.  Fiber SFPs
> > exclusively use 1000BASE-X protocol.  However, some copper SFPs
> > (with a RJ45) do use SGMII.
> >
> > > > 2) The 1G Ethernet driver has been converted to use phylink.
> >
> > This is not necessary for this scenario.  The PHY driver needs to
> > be updated to know about SFP though.
>
> Excellent, this is exactly the information I was looking for. I had
> started converting the Ethernet driver to phylink but there was still
> a lot of work to do and I fear there was a significant potential for
> regressions. I'll abandon that and see if I can get it to work by
> making similar changes to the 1G Marvell PHY driver.
>
> I'm assuming I must set the sfp property of the PHY in DT instead of the MAC.
>
> >
> > See:
> >
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4
> >
> > as an example of the 88x3310 supporting a SFP+ cage.  This patch is
> > also necessary:
> >
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990
>
> Perfect.
>
> >
> > and if anything is going to stand in the way of progress on this, it
> > is likely to be that patch.  I'll be attempting to post these after
> > the next merge window (i.o.w. probably posting them in three weeks
> > time.)
>
> Please CC me on these patches as I'll follow your lead for what I do
> on the 1G Marvell PHY driver. If you don't remember, that's fine, I
> understand.
>
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
>
> Thanks,
> George

Regards,
George

Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?

[Russell King - ARM Linux admin]

On Wed, Sep 18, 2019 at 01:44:33PM -0500, George McCollister wrote:
> Russell,
> 
> On Mon, Sep 16, 2019 at 10:40 AM George McCollister
> <george.mccollister@gmail.com> wrote:
> >
> > On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > > > +Russell, Andrew, Heiner,
> > > >
> > > > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > > > Every example of phylink SFP support I've seen is using an Ethernet
> > > > > MAC with native SGMII.
> > > > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > > > connected to an RGMII MAC if all of the following are true?
> > > >
> > > > I don't think that use case has been presented before, but phylink
> > > > sounds like the tool that should help solve it. From your description
> > > > below, it sounds like all the pieces are there to support it. Is the
> > > > Ethernet MAC driver upstream?
> > >
> > > It has been presented, and it's something I've been trying to support
> > > for the last couple of years - in fact, I have patches in my tree that
> > > support a very similar scenario on the Macchiatobin with the 88x3310
> > > PHYs.
> > >
> > > > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > > > you want to see a block diagram it's the first one here:
> > > > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> > >
> > > As mentioned above, this is no different from the Macchiatobin,
> > > where we have:
> > >
> > >                   .-------- RJ45
> > > MAC ---- 88x3310 PHY
> > >                   `-------- SFP+
> > >
> > > except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> > > and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
> 
> Did you test with an SFP+ module that has a PHY?

I think you mean SFP module - SFP+ is for >1G.  No, that won't work for
two reasons.

1. The copper SFP module requires SGMII not 1000BASE-X.
2. netdev/phylib doesn't support stacking two PHYs on a single
   network device.

(1) can be worked around if the intermediary PHY can be configured for
it, but (2) is a lot harder to resolve, because phylib wants to put
the PHY into the netdev->phydev pointer, and you can't have two of
them.

I think fixing this properly is going to require quite a lot of
re-work, and as I don't have a setup that electrically supports SFP
modules in this setup (only SFP+ modules) it isn't something I'm
able to test.

> In my setup, nothing connects/attaches to the PHY (88E1111) in the
> RJ45 SFP module I'm testing with (is this intended?). Apparently since
> sfp_upstream_ops in the PHY driver used for the first PHY doesn't
> provide a .connect_phy.

Yep, because there's no way to stack phylib managed PHYs.

> This leaves .phy_link_change NULL. Eventually
> phy_link_down tries to call .phy_link_change resulting in a NULL
> pointer deference OOPs. If phy_link_change doesn't need to be called
> for the second PHY I can just send a patch that doesn't call it if
> it's NULL.

SGMII provides a way to discover the negotiated speed and duplex,
but not the pause settings back to the MAC - so in this setup, we
really do need to read the 88E1111 negotiation results (for the
pause settings) and communicate them back to the MAC for things
to work correctly.

It's something that we need to work out how to do...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up