| Message ID | 20240911161054.4494-6-Raju.Lakkaraju@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support to SFP for PCI11x1x chips | expand |
> @@ -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
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
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 >
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
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
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 */
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(-)