netdev - Re: [PATCH v2 net-next 1/4] amd-xgbe: reorganize the code of XPCS access

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20231119163744.GC186930@vergenet.net>
Date: Sun, 19 Nov 2023 16:37:44 +0000
From: Simon Horman <horms@...nel.org>
To: Raju Rangoju <Raju.Rangoju@....com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, Shyam-sundar.S-k@....com
Subject: Re: [PATCH v2 net-next 1/4] amd-xgbe: reorganize the code of XPCS
 access

On Thu, Nov 16, 2023 at 07:24:13PM +0530, Raju Rangoju wrote:
> The xgbe_{read/write}_mmd_regs_v* functions have common code which can
> be moved to helper functions. Also, the xgbe_pci_probe() needs
> reorganization.
> 
> Add new helper functions to calculate the mmd_address for v1/v2 of xpcs
> access. And, convert if/else statements in xgbe_pci_probe() to switch
> case. This helps code look cleaner.
> 
> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>

...

> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index f409d7bd1f1e..d6071f34b7db 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -274,12 +274,16 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	/* Set the PCS indirect addressing definition registers */
>  	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> -	if (rdev &&
> -	    (rdev->vendor == PCI_VENDOR_ID_AMD) && (rdev->device == 0x15d0)) {
> +
> +	if (!(rdev && rdev->vendor == PCI_VENDOR_ID_AMD))
> +		goto err_pci_enable;

Hi Raju,

Jumping to err_pci_enable will result in the function returning ret.
However, it appears that ret is set to 0 here. Perhaps it should
be set to an negative error code instead?

Flagged by Smatch.

> +
> +	switch (rdev->device) {
> +	case 0x15d0:
>  		pdata->xpcs_window_def_reg = PCS_V2_RV_WINDOW_DEF;
>  		pdata->xpcs_window_sel_reg = PCS_V2_RV_WINDOW_SELECT;
> -	} else if (rdev && (rdev->vendor == PCI_VENDOR_ID_AMD) &&
> -		   (rdev->device == 0x14b5)) {
> +		break;
> +	case 0x14b5:
>  		pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
>  		pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
>  
> @@ -288,9 +292,11 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  		/* Yellow Carp devices do not need rrc */
>  		pdata->vdata->enable_rrc = 0;
> -	} else {
> +		break;
> +	default:
>  		pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>  		pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
> +		break;
>  	}
>  	pci_dev_put(rdev);
>  
> -- 
> 2.34.1
>