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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e542d2fc-0587-45a3-bc58-ee0a078a626a@wanadoo.fr>
Date: Wed, 11 Sep 2024 18:44:58 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, bryan.whitehead@...rochip.com,
 UNGLinuxDriver@...rochip.com, linux@...linux.org.uk,
 maxime.chevallier@...tlin.com, rdunlap@...radead.org, andrew@...n.ch,
 Steen.Hegelund@...rochip.com, daniel.machon@...rochip.com,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next V2 1/5] net: lan743x: Add SFP support check flag

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@...rochip.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;