[PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface

Raju Lakkaraju posted 5 patches 2 months, 2 weeks ago
[PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Raju Lakkaraju 2 months, 2 weeks ago
Support for 2.5G SFP modules utilizing the 2500Base-X interface.
The implementation includes integration with the Phylink subsystem to
manage the link state and configuration dynamically.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
---
Change List:
============
V2 : Include this new patch in the V2 series. 

 drivers/net/ethernet/microchip/lan743x_main.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index ef76d0c1642f..7fe699e5a134 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1495,7 +1495,10 @@ static void lan743x_phy_interface_select(struct lan743x_adapter *adapter)
 	data = lan743x_csr_read(adapter, MAC_CR);
 	id_rev = adapter->csr.id_rev & ID_REV_ID_MASK_;
 
-	if (adapter->is_pci11x1x && adapter->is_sgmii_en)
+	if (adapter->is_pci11x1x && adapter->is_sgmii_en &&
+	    adapter->is_sfp_support_en)
+		adapter->phy_interface = PHY_INTERFACE_MODE_2500BASEX;
+	else if (adapter->is_pci11x1x && adapter->is_sgmii_en)
 		adapter->phy_interface = PHY_INTERFACE_MODE_SGMII;
 	else if (id_rev == ID_REV_ID_LAN7430_)
 		adapter->phy_interface = PHY_INTERFACE_MODE_GMII;
@@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
 	lan743x_phy_interface_select(adapter);
 
 	switch (adapter->phy_interface) {
+	case PHY_INTERFACE_MODE_2500BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 		__set_bit(PHY_INTERFACE_MODE_SGMII,
 			  adapter->phylink_config.supported_interfaces);
@@ -3412,12 +3416,13 @@ static int lan743x_phylink_connect(struct lan743x_adapter *adapter)
 	struct device_node *dn = adapter->pdev->dev.of_node;
 	struct net_device *dev = adapter->netdev;
 	struct phy_device *phydev;
-	int ret;
+	int ret = 0;
 
 	if (dn)
 		ret = phylink_of_phy_connect(adapter->phylink, dn, 0);
 
-	if (!dn || (ret && !lan743x_phy_handle_exists(dn))) {
+	if (!adapter->is_sfp_support_en &&
+	    (!dn || (ret && !lan743x_phy_handle_exists(dn)))) {
 		phydev = phy_find_first(adapter->mdiobus);
 		if (phydev) {
 			/* attach the mac to the phy */
-- 
2.34.1
Re: [PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Andrew Lunn 2 months, 2 weeks ago
> @@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
>  	lan743x_phy_interface_select(adapter);
>  
>  	switch (adapter->phy_interface) {
> +	case PHY_INTERFACE_MODE_2500BASEX:
>  	case PHY_INTERFACE_MODE_SGMII:
>  		__set_bit(PHY_INTERFACE_MODE_SGMII,
>  			  adapter->phylink_config.supported_interfaces);

I _think_ you also need to set the PHY_INTERFACE_MODE_2500BASEX bit in
phylink_config.supported_interfaces if you actually support it.

Have you tested an SFP module capable of 2500BASEX?

	Andrew
Re: [PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Raju Lakkaraju 2 months, 2 weeks ago
Hi Andrew,

The 09/11/2024 19:31, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> > @@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
> >       lan743x_phy_interface_select(adapter);
> >
> >       switch (adapter->phy_interface) {
> > +     case PHY_INTERFACE_MODE_2500BASEX:
> >       case PHY_INTERFACE_MODE_SGMII:
> >               __set_bit(PHY_INTERFACE_MODE_SGMII,
> >                         adapter->phylink_config.supported_interfaces);
> 
> I _think_ you also need to set the PHY_INTERFACE_MODE_2500BASEX bit in
> phylink_config.supported_interfaces if you actually support it.
> 
It's already add support. Here it's showing only diff changes

> Have you tested an SFP module capable of 2500BASEX?
> 
Yes. I test SFP module (FS's make 2.5G Cu SFP (SFP-2.5G-T))
it's working as expected.

>         Andrew

-- 
Thanks,                                                                         
Raju
Re: [PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Maxime Chevallier 2 months, 2 weeks ago
On Wed, 11 Sep 2024 19:31:01 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > @@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
> >  	lan743x_phy_interface_select(adapter);
> >  
> >  	switch (adapter->phy_interface) {
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> >  	case PHY_INTERFACE_MODE_SGMII:
> >  		__set_bit(PHY_INTERFACE_MODE_SGMII,
> >  			  adapter->phylink_config.supported_interfaces);  
> 
> I _think_ you also need to set the PHY_INTERFACE_MODE_2500BASEX bit in
> phylink_config.supported_interfaces if you actually support it.

It's actually being set a bit below. However that raises the
question of why.

On the variant that don't have this newly-introduced SFP support but do
have sgmii support (!is_sfp_support_en && is_sgmii_en), can this chip
actually support 2500BaseX ?

If so, is there a point in getting a different default interface
returned from lan743x_phy_interface_select() depending on wether or not
there's SFP support ?

Maxime
Re: [PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Raju Lakkaraju 2 months, 2 weeks ago
The 09/11/2024 22:01, Maxime Chevallier wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, 11 Sep 2024 19:31:01 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > > @@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
> > >     lan743x_phy_interface_select(adapter);
> > >
> > >     switch (adapter->phy_interface) {
> > > +   case PHY_INTERFACE_MODE_2500BASEX:
> > >     case PHY_INTERFACE_MODE_SGMII:
> > >             __set_bit(PHY_INTERFACE_MODE_SGMII,
> > >                       adapter->phylink_config.supported_interfaces);
> >
> > I _think_ you also need to set the PHY_INTERFACE_MODE_2500BASEX bit in
> > phylink_config.supported_interfaces if you actually support it.
> 
> It's actually being set a bit below. However that raises the
> question of why.
> 
> On the variant that don't have this newly-introduced SFP support but do
> have sgmii support (!is_sfp_support_en && is_sgmii_en), can this chip
> actually support 2500BaseX ?

Yes. 
PCI11010/PCI11414 chip's PCS support SGMII/2500Baxe-X I/F at 2.5Gpbs
We need to over clocking at a bit rate of 3.125 Gbps for 2.5Gbps event SGMII
I/F

From data sheet:
"The SGMII interface also supports over clocking at a bit rate of 3.125 Gbps for an effective 2.5 Gbps data rate. 10 and
100 Mbps modes are also scaled up by 2.5x but are most likely not useful."

> 
> If so, is there a point in getting a different default interface
> returned from lan743x_phy_interface_select() depending on wether or not
> there's SFP support ?

Yes.

This LAN743x driver support following chips
 1. LAN7430 - GMII I/F
 2. LAN7431 - MII I/F
 3. PCI11010/PCI11414 - RGMII or SGMII/1000Base-X/2500Base-X

> 
> Maxime
> 

-- 
Thanks,                                                                         
Raju
Re: [PATCH net-next V2 5/5] net: lan743x: Add Support for 2.5G SFP with 2500Base-X Interface
Posted by Maxime Chevallier 2 months, 2 weeks ago
Hi Raju,

On Thu, 12 Sep 2024 12:31:33 +0530
Raju Lakkaraju <Raju.Lakkaraju@microchip.com> wrote:

> The 09/11/2024 22:01, Maxime Chevallier wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Wed, 11 Sep 2024 19:31:01 +0200
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >   
> > > > @@ -3359,6 +3362,7 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
> > > >     lan743x_phy_interface_select(adapter);
> > > >
> > > >     switch (adapter->phy_interface) {
> > > > +   case PHY_INTERFACE_MODE_2500BASEX:
> > > >     case PHY_INTERFACE_MODE_SGMII:
> > > >             __set_bit(PHY_INTERFACE_MODE_SGMII,
> > > >                       adapter->phylink_config.supported_interfaces);  
> > >
> > > I _think_ you also need to set the PHY_INTERFACE_MODE_2500BASEX bit in
> > > phylink_config.supported_interfaces if you actually support it.  
> > 
> > It's actually being set a bit below. However that raises the
> > question of why.
> > 
> > On the variant that don't have this newly-introduced SFP support but do
> > have sgmii support (!is_sfp_support_en && is_sgmii_en), can this chip
> > actually support 2500BaseX ?  
> 
> Yes. 
> PCI11010/PCI11414 chip's PCS support SGMII/2500Baxe-X I/F at 2.5Gpbs
> We need to over clocking at a bit rate of 3.125 Gbps for 2.5Gbps event SGMII
> I/F
> 
> From data sheet:
> "The SGMII interface also supports over clocking at a bit rate of 3.125 Gbps for an effective 2.5 Gbps data rate. 10 and
> 100 Mbps modes are also scaled up by 2.5x but are most likely not useful."
> 
> > 
> > If so, is there a point in getting a different default interface
> > returned from lan743x_phy_interface_select() depending on wether or not
> > there's SFP support ?  
> 
> Yes.
> 
> This LAN743x driver support following chips
>  1. LAN7430 - GMII I/F
>  2. LAN7431 - MII I/F
>  3. PCI11010/PCI11414 - RGMII or SGMII/1000Base-X/2500Base-X

In your patch there's the following change :

@@ -1495,7 +1495,10 @@ static void lan743x_phy_interface_select(struct lan743x_adapter *adapter)
 	data = lan743x_csr_read(adapter, MAC_CR);
 	id_rev = adapter->csr.id_rev & ID_REV_ID_MASK_;
 
-	if (adapter->is_pci11x1x && adapter->is_sgmii_en)
+	if (adapter->is_pci11x1x && adapter->is_sgmii_en &&
+	    adapter->is_sfp_support_en)
+		adapter->phy_interface = PHY_INTERFACE_MODE_2500BASEX;
+	else if (adapter->is_pci11x1x && adapter->is_sgmii_en)
 		adapter->phy_interface = PHY_INTERFACE_MODE_SGMII;
 	else if (id_rev == ID_REV_ID_LAN7430_)
 		adapter->phy_interface = PHY_INTERFACE_MODE_GMII;

From what I get, if the chip is a pci11x1x and has sgmii_en, it doesn't
really matter wether or not the "is_sfp_support" it set, as you support
the same sets of interface modes.

The phy_interface will be re-configured the moment the SFP module is
plugged, so it shouldn't matter wether you set the default interface to
SGMII or 2500BaseX.

So, the change quoted above doesn't really bring anything, am I correct
?

Thanks,

Maxime