diff mbox series

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

Message ID 20231116135416.3371367-2-Raju.Rangoju@amd.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series amd-xgbe: add support for AMD Crater | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1127 this patch: 1127
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 1154 this patch: 1154
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1154 this patch: 1154
netdev/checkpatch warning WARNING: line length of 91 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Raju Rangoju Nov. 16, 2023, 1:54 p.m. UTC 
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@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 43 ++++++++++++------------
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 16 ++++++---
 2 files changed, 32 insertions(+), 27 deletions(-)

Comments

Simon Horman Nov. 19, 2023, 4:37 p.m. UTC | #1 
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@amd.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
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index f393228d41c7..6cd003c24a64 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1150,6 +1150,21 @@  static int xgbe_set_gpio(struct xgbe_prv_data *pdata, unsigned int gpio)
 	return 0;
 }
 
+static unsigned int get_mmd_address(struct xgbe_prv_data *pdata, int mmd_reg)
+{
+	return (mmd_reg & XGBE_ADDR_C45) ?
+		mmd_reg & ~XGBE_ADDR_C45 :
+		(pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
+}
+
+static unsigned int get_index_offset(struct xgbe_prv_data *pdata, unsigned int mmd_address,
+				     unsigned int *index)
+{
+	mmd_address <<= 1;
+	*index = mmd_address & ~pdata->xpcs_window_mask;
+	return pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
+}
+
 static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 				 int mmd_reg)
 {
@@ -1157,10 +1172,7 @@  static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	unsigned int mmd_address, index, offset;
 	int mmd_data;
 
-	if (mmd_reg & XGBE_ADDR_C45)
-		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
-	else
-		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
+	mmd_address = get_mmd_address(pdata, mmd_reg);
 
 	/* The PCS registers are accessed using mmio. The underlying
 	 * management interface uses indirect addressing to access the MMD
@@ -1171,9 +1183,7 @@  static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	 * register offsets must therefore be adjusted by left shifting the
 	 * offset 1 bit and reading 16 bits of data.
 	 */
-	mmd_address <<= 1;
-	index = mmd_address & ~pdata->xpcs_window_mask;
-	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
+	offset = get_index_offset(pdata, mmd_address, &index);
 
 	spin_lock_irqsave(&pdata->xpcs_lock, flags);
 	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
@@ -1189,10 +1199,7 @@  static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	unsigned long flags;
 	unsigned int mmd_address, index, offset;
 
-	if (mmd_reg & XGBE_ADDR_C45)
-		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
-	else
-		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
+	mmd_address = get_mmd_address(pdata, mmd_reg);
 
 	/* The PCS registers are accessed using mmio. The underlying
 	 * management interface uses indirect addressing to access the MMD
@@ -1203,9 +1210,7 @@  static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	 * register offsets must therefore be adjusted by left shifting the
 	 * offset 1 bit and writing 16 bits of data.
 	 */
-	mmd_address <<= 1;
-	index = mmd_address & ~pdata->xpcs_window_mask;
-	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
+	offset = get_index_offset(pdata, mmd_address, &index);
 
 	spin_lock_irqsave(&pdata->xpcs_lock, flags);
 	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
@@ -1220,10 +1225,7 @@  static int xgbe_read_mmd_regs_v1(struct xgbe_prv_data *pdata, int prtad,
 	unsigned int mmd_address;
 	int mmd_data;
 
-	if (mmd_reg & XGBE_ADDR_C45)
-		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
-	else
-		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
+	mmd_address = get_mmd_address(pdata, mmd_reg);
 
 	/* The PCS registers are accessed using mmio. The underlying APB3
 	 * management interface uses indirect addressing to access the MMD
@@ -1248,10 +1250,7 @@  static void xgbe_write_mmd_regs_v1(struct xgbe_prv_data *pdata, int prtad,
 	unsigned int mmd_address;
 	unsigned long flags;
 
-	if (mmd_reg & XGBE_ADDR_C45)
-		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
-	else
-		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
+	mmd_address = get_mmd_address(pdata, mmd_reg);
 
 	/* The PCS registers are accessed using mmio. The underlying APB3
 	 * management interface uses indirect addressing to access the MMD
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;
+
+	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);