1. Patchew
  2. linux
  3. [PATCH net-next V2 0/5] Add support to SFP for PCI11x1x chips
  4. View patch

[PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag

Raju Lakkaraju posted 5 patches 2 months, 2 weeks ago
[PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Raju Lakkaraju 2 months, 2 weeks ago 
Support for SFP in the PCI11x1x devices is indicated by the "is_sfp_support_en"
flag in the STRAP register. This register is loaded at power up from the
PCI11x1x EEPROM contents (which specify the board configuration).

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
---
Change List:
============
V1 -> V2:
  - Change variable name from "chip_rev" to "fpga_rev" 
V0 -> V1:
  - No changes

 drivers/net/ethernet/microchip/lan743x_main.c | 34 +++++++++++++++----
 drivers/net/ethernet/microchip/lan743x_main.h |  3 ++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 4dc5adcda6a3..20a42a2c7b0e 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -28,9 +28,9 @@
 
 #define RFE_RD_FIFO_TH_3_DWORDS	0x3
 
-static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
+static int pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 {
-	u32 chip_rev;
+	u32 fpga_rev;
 	u32 cfg_load;
 	u32 hw_cfg;
 	u32 strap;
@@ -41,7 +41,7 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 	if (ret < 0) {
 		netif_err(adapter, drv, adapter->netdev,
 			  "Sys Lock acquire failed ret:%d\n", ret);
-		return;
+		return ret;
 	}
 
 	cfg_load = lan743x_csr_read(adapter, ETH_SYS_CONFIG_LOAD_STARTED_REG);
@@ -55,10 +55,15 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 			adapter->is_sgmii_en = true;
 		else
 			adapter->is_sgmii_en = false;
+
+		if ((strap & STRAP_SFP_USE_EN_) && (strap & STRAP_SFP_EN_))
+			adapter->is_sfp_support_en = true;
+		else
+			adapter->is_sfp_support_en = false;
 	} else {
-		chip_rev = lan743x_csr_read(adapter, FPGA_REV);
-		if (chip_rev) {
-			if (chip_rev & FPGA_SGMII_OP)
+		fpga_rev = lan743x_csr_read(adapter, FPGA_REV);
+		if (fpga_rev) {
+			if (fpga_rev & FPGA_SGMII_OP)
 				adapter->is_sgmii_en = true;
 			else
 				adapter->is_sgmii_en = false;
@@ -66,8 +71,21 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 			adapter->is_sgmii_en = false;
 		}
 	}
+
+	if (adapter->is_pci11x1x && !adapter->is_sgmii_en &&
+	    adapter->is_sfp_support_en) {
+		netif_err(adapter, drv, adapter->netdev,
+			  "Invalid eeprom cfg: sfp enabled with sgmii disabled");
+		return -EINVAL;
+	}
+
 	netif_dbg(adapter, drv, adapter->netdev,
 		  "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
+	netif_dbg(adapter, drv, adapter->netdev,
+		  "SFP support %sable\n", adapter->is_sfp_support_en ?
+		  "En" : "Dis");
+
+	return 0;
 }
 
 static bool is_pci11x1x_chip(struct lan743x_adapter *adapter)
@@ -3470,7 +3488,9 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 		adapter->max_tx_channels = PCI11X1X_MAX_TX_CHANNELS;
 		adapter->used_tx_channels = PCI11X1X_USED_TX_CHANNELS;
 		adapter->max_vector_count = PCI11X1X_MAX_VECTOR_COUNT;
-		pci11x1x_strap_get_status(adapter);
+		ret = pci11x1x_strap_get_status(adapter);
+		if (ret < 0)
+			return ret;
 		spin_lock_init(&adapter->eth_syslock_spinlock);
 		mutex_init(&adapter->sgmii_rw_lock);
 		pci11x1x_set_rfe_rd_fifo_threshold(adapter);
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 8ef897c114d3..f7e96496600b 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -36,6 +36,8 @@
 
 #define STRAP_READ			(0x0C)
 #define STRAP_READ_USE_SGMII_EN_	BIT(22)
+#define STRAP_SFP_USE_EN_		BIT(31)
+#define STRAP_SFP_EN_			BIT(15)
 #define STRAP_READ_SGMII_EN_		BIT(6)
 #define STRAP_READ_SGMII_REFCLK_	BIT(5)
 #define STRAP_READ_SGMII_2_5G_		BIT(4)
@@ -1079,6 +1081,7 @@ struct lan743x_adapter {
 	u8			max_tx_channels;
 	u8			used_tx_channels;
 	u8			max_vector_count;
+	bool			is_sfp_support_en;
 
 #define LAN743X_ADAPTER_FLAG_OTP		BIT(0)
 	u32			flags;
-- 
2.34.1
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Russell King (Oracle) 2 months, 2 weeks ago 
On Wed, Sep 11, 2024 at 09:40:50PM +0530, Raju Lakkaraju wrote:
>  {
> -	u32 chip_rev;
> +	u32 fpga_rev;
>  	u32 cfg_load;
>  	u32 hw_cfg;
>  	u32 strap;

...

>  	} else {
> -		chip_rev = lan743x_csr_read(adapter, FPGA_REV);
> -		if (chip_rev) {
> -			if (chip_rev & FPGA_SGMII_OP)
> +		fpga_rev = lan743x_csr_read(adapter, FPGA_REV);
> +		if (fpga_rev) {
> +			if (fpga_rev & FPGA_SGMII_OP)

This looks like an unrelated change.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Andrew Lunn 2 months, 2 weeks ago 
> +	if (adapter->is_pci11x1x && !adapter->is_sgmii_en &&
> +	    adapter->is_sfp_support_en) {
> +		netif_err(adapter, drv, adapter->netdev,
> +			  "Invalid eeprom cfg: sfp enabled with sgmii disabled");
> +		return -EINVAL;

is_sgmii_en actually means PCS? An SFP might need 1000BaseX or SGMII,
phylink will tell you the mode to put the PCS into.

	Andrew
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Raju Lakkaraju 2 months, 2 weeks ago 
Hi Andrew,

Thank you for review the patches.

The 09/11/2024 19:06, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> > +     if (adapter->is_pci11x1x && !adapter->is_sgmii_en &&
> > +         adapter->is_sfp_support_en) {
> > +             netif_err(adapter, drv, adapter->netdev,
> > +                       "Invalid eeprom cfg: sfp enabled with sgmii disabled");
> > +             return -EINVAL;
> 
> is_sgmii_en actually means PCS? An SFP might need 1000BaseX or SGMII,

No, not really.
The PCI11010/PCI1414 chip can support either an RGMII interface or
an SGMII/1000Base-X/2500Base-X interface.

To differentiate between these interfaces, we need to enable or disable
the SGMII/1000Base-X/2500Base-X interface in the EEPROM.
This configuration is reflected in the STRAP_READ register (0x0C),
specifically bit 6.

According to the datasheet,
the "Strap Register (STRAP)" bit 6 is described as "SGMII_EN_STRAP"
Therefore, the flag is named "is_sgmii_en".

> phylink will tell you the mode to put the PCS into.

Yes. You are correct.
Based on PCS information, it will swich between SGMII or 1000Base-X or
2500Base-X I/F.

> 
>         Andrew

-- 
Thanks,                                                                         
Raju
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Andrew Lunn 2 months, 2 weeks ago 
On Thu, Sep 12, 2024 at 11:59:04AM +0530, Raju Lakkaraju wrote:
> Hi Andrew,
> 
> Thank you for review the patches.
> 
> The 09/11/2024 19:06, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > > +     if (adapter->is_pci11x1x && !adapter->is_sgmii_en &&
> > > +         adapter->is_sfp_support_en) {
> > > +             netif_err(adapter, drv, adapter->netdev,
> > > +                       "Invalid eeprom cfg: sfp enabled with sgmii disabled");
> > > +             return -EINVAL;
> > 
> > is_sgmii_en actually means PCS? An SFP might need 1000BaseX or SGMII,
> 
> No, not really.
> The PCI11010/PCI1414 chip can support either an RGMII interface or
> an SGMII/1000Base-X/2500Base-X interface.

A generic name for SGMII/1000Base-X/2500Base-X would be PCS, or maybe
SERDES. To me, is_sgmii_en means SGMII is enabled, but in fact it
actually means SGMII/1000Base-X/2500Base-X is enabled. I just think
this is badly named. It would be more understandable if it was
is_pcs_en.

> According to the datasheet,
> the "Strap Register (STRAP)" bit 6 is described as "SGMII_EN_STRAP"
> Therefore, the flag is named "is_sgmii_en".

Just because the datasheet uses a bad name does not mean the driver
has to also use it.

	Andrew
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Christophe JAILLET 2 months, 2 weeks ago 
Le 11/09/2024 à 18:10, Raju Lakkaraju a écrit :
> Support for SFP in the PCI11x1x devices is indicated by the "is_sfp_support_en"
> flag in the STRAP register. This register is loaded at power up from the
> PCI11x1x EEPROM contents (which specify the board configuration).
> 
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> ---
> Change List:
> ============
> V1 -> V2:
>    - Change variable name from "chip_rev" to "fpga_rev"
> V0 -> V1:
>    - No changes
> 
>   drivers/net/ethernet/microchip/lan743x_main.c | 34 +++++++++++++++----
>   drivers/net/ethernet/microchip/lan743x_main.h |  3 ++
>   2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 4dc5adcda6a3..20a42a2c7b0e 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -28,9 +28,9 @@
>   
>   #define RFE_RD_FIFO_TH_3_DWORDS	0x3
>   
> -static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
> +static int pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>   {
> -	u32 chip_rev;
> +	u32 fpga_rev;
>   	u32 cfg_load;
>   	u32 hw_cfg;
>   	u32 strap;
> @@ -41,7 +41,7 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>   	if (ret < 0) {
>   		netif_err(adapter, drv, adapter->netdev,
>   			  "Sys Lock acquire failed ret:%d\n", ret);
> -		return;
> +		return ret;
>   	}
>   
>   	cfg_load = lan743x_csr_read(adapter, ETH_SYS_CONFIG_LOAD_STARTED_REG);
> @@ -55,10 +55,15 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>   			adapter->is_sgmii_en = true;
>   		else
>   			adapter->is_sgmii_en = false;
> +
> +		if ((strap & STRAP_SFP_USE_EN_) && (strap & STRAP_SFP_EN_))
> +			adapter->is_sfp_support_en = true;
> +		else
> +			adapter->is_sfp_support_en = false;
>   	} else {
> -		chip_rev = lan743x_csr_read(adapter, FPGA_REV);
> -		if (chip_rev) {
> -			if (chip_rev & FPGA_SGMII_OP)
> +		fpga_rev = lan743x_csr_read(adapter, FPGA_REV);
> +		if (fpga_rev) {
> +			if (fpga_rev & FPGA_SGMII_OP)
>   				adapter->is_sgmii_en = true;
>   			else
>   				adapter->is_sgmii_en = false;
> @@ -66,8 +71,21 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>   			adapter->is_sgmii_en = false;
>   		}
>   	}
> +
> +	if (adapter->is_pci11x1x && !adapter->is_sgmii_en &&
> +	    adapter->is_sfp_support_en) {
> +		netif_err(adapter, drv, adapter->netdev,
> +			  "Invalid eeprom cfg: sfp enabled with sgmii disabled");
> +		return -EINVAL;
> +	}
> +
>   	netif_dbg(adapter, drv, adapter->netdev,
>   		  "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> +	netif_dbg(adapter, drv, adapter->netdev,
> +		  "SFP support %sable\n", adapter->is_sfp_support_en ?
> +		  "En" : "Dis");

Hi,

Maybe using str_enable_disable() or str_enabled_disabled()?

CJ

> +
> +	return 0;
>   }
>   
>   static bool is_pci11x1x_chip(struct lan743x_adapter *adapter)
> @@ -3470,7 +3488,9 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
>   		adapter->max_tx_channels = PCI11X1X_MAX_TX_CHANNELS;
>   		adapter->used_tx_channels = PCI11X1X_USED_TX_CHANNELS;
>   		adapter->max_vector_count = PCI11X1X_MAX_VECTOR_COUNT;
> -		pci11x1x_strap_get_status(adapter);
> +		ret = pci11x1x_strap_get_status(adapter);
> +		if (ret < 0)
> +			return ret;
>   		spin_lock_init(&adapter->eth_syslock_spinlock);
>   		mutex_init(&adapter->sgmii_rw_lock);
>   		pci11x1x_set_rfe_rd_fifo_threshold(adapter);
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
> index 8ef897c114d3..f7e96496600b 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.h
> +++ b/drivers/net/ethernet/microchip/lan743x_main.h
> @@ -36,6 +36,8 @@
>   
>   #define STRAP_READ			(0x0C)
>   #define STRAP_READ_USE_SGMII_EN_	BIT(22)
> +#define STRAP_SFP_USE_EN_		BIT(31)
> +#define STRAP_SFP_EN_			BIT(15)
>   #define STRAP_READ_SGMII_EN_		BIT(6)
>   #define STRAP_READ_SGMII_REFCLK_	BIT(5)
>   #define STRAP_READ_SGMII_2_5G_		BIT(4)
> @@ -1079,6 +1081,7 @@ struct lan743x_adapter {
>   	u8			max_tx_channels;
>   	u8			used_tx_channels;
>   	u8			max_vector_count;
> +	bool			is_sfp_support_en;
>   
>   #define LAN743X_ADAPTER_FLAG_OTP		BIT(0)
>   	u32			flags;
Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag
Posted by Raju Lakkaraju 2 months, 2 weeks ago 
Hi Christophe,

Thank you for review the patches.

The 09/11/2024 18:44, Christophe JAILLET wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Le 11/09/2024 à 18:10, Raju Lakkaraju a écrit :
> > Support for SFP in the PCI11x1x devices is indicated by the "is_sfp_support_en"
> > flag in the STRAP register. This register is loaded at power up from the
> > PCI11x1x EEPROM contents (which specify the board configuration).
> > 
> > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > ---
> > Change List:
> > ============
> > V1 -> V2:
> >    - Change variable name from "chip_rev" to "fpga_rev"
> > V0 -> V1:
> >    - No changes
> > 
> >   drivers/net/ethernet/microchip/lan743x_main.c | 34 +++++++++++++++----
> >   drivers/net/ethernet/microchip/lan743x_main.h |  3 ++
> >   2 files changed, 30 insertions(+), 7 deletions(-)
> > 
> >       netif_dbg(adapter, drv, adapter->netdev,
> >                 "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> > +     netif_dbg(adapter, drv, adapter->netdev,
> > +               "SFP support %sable\n", adapter->is_sfp_support_en ?
> > +               "En" : "Dis");
> 
> Hi,
> 
> Maybe using str_enable_disable() or str_enabled_disabled()?
> 

Accepted. I will use str_enabled_disabled( ).

> CJ
> 
> > +
> > +     return 0;
> >   }
> > 
> 

-- 
Thanks,                                                                         
Raju

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.