[PATCH net-next 0/2] amd-xgbe: add support for AMD Crater

[PATCH net-next 0/2] amd-xgbe: add support for AMD Crater

[Raju Rangoju]

Add support for a new AMD Ethernet device called "Crater". It has a new
PCI ID, add this to the current list of supported devices in the
amd-xgbe devices.Also, the BAR1 addresses cannot be used to access the
PCS registers on Crater platform, use the indirect addressing via SMN
instead.

Raju Rangoju (2):
  amd-xgbe: add support for new pci device id 0x1641
  amd-xgbe: Add support for AMD Crater ethernet device

 drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 +++
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 33 +++++++++++++++++---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 34 ++++++++++++++++-----
 drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++++
 4 files changed, 65 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH net-next 1/2] amd-xgbe: add support for new pci device id 0x1641

[Raju Rangoju]

Add support for a new AMD Ethernet device called "Crater". It has a new
PCI ID, add this to the current list of supported devices in the
amd-xgbe devices.

Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index f409d7bd1f1e..a17359d43b45 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -510,6 +510,8 @@ static const struct pci_device_id xgbe_pci_table[] = {
 	  .driver_data = (kernel_ulong_t)&xgbe_v2a },
 	{ PCI_VDEVICE(AMD, 0x1459),
 	  .driver_data = (kernel_ulong_t)&xgbe_v2b },
+	{ PCI_VDEVICE(AMD, 0x1641),
+	  .driver_data = (kernel_ulong_t)&xgbe_v2b },
 	/* Last entry must be zero */
 	{ 0, }
 };
-- 
2.25.1

[PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device

[Raju Rangoju]

The AMD Crater device has new window settings for the XPCS access, add
support to adopt to the new window settings. There is a hardware bug
where in the BAR1 registers cannot be accessed directly. As a fallback
mechanism, access these PCS registers through indirect access via SMN.

Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 ++++
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 33 +++++++++++++++++----
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 32 +++++++++++++++-----
 drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++++
 4 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
index 3b70f6737633..e1f70f0528ef 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
@@ -900,6 +900,11 @@
 #define PCS_V2_RV_WINDOW_SELECT		0x1064
 #define PCS_V2_YC_WINDOW_DEF		0x18060
 #define PCS_V2_YC_WINDOW_SELECT		0x18064
+#define PCS_V2_RN_WINDOW_DEF		0xF8078
+#define PCS_V2_RN_WINDOW_SELECT		0xF807c
+
+#define PCS_RN_SMN_BASE_ADDR		0x11E00000
+#define PCS_RN_PORT_ADDR_SIZE		0x100000
 
 /* PCS register entry bit positions and sizes */
 #define PCS_V2_WINDOW_DEF_OFFSET_INDEX	6
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index f393228d41c7..da8ec218282f 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1176,8 +1176,17 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
 
 	spin_lock_irqsave(&pdata->xpcs_lock, flags);
-	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
-	mmd_data = XPCS16_IOREAD(pdata, offset);
+	if (pdata->vdata->is_crater) {
+		amd_smn_write(0,
+			      (pdata->xphy_base + pdata->xpcs_window_sel_reg),
+			      index);
+		amd_smn_read(0, pdata->xphy_base + offset, &mmd_data);
+		mmd_data = (offset % ALIGNMENT_VAL) ?
+			   ((mmd_data >> 16) & 0xffff) : (mmd_data & 0xffff);
+	} else {
+		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
+		mmd_data = XPCS16_IOREAD(pdata, offset);
+	}
 	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
 
 	return mmd_data;
@@ -1186,8 +1195,8 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 				   int mmd_reg, int mmd_data)
 {
+	unsigned int mmd_address, index, offset, crtr_mmd_data;
 	unsigned long flags;
-	unsigned int mmd_address, index, offset;
 
 	if (mmd_reg & XGBE_ADDR_C45)
 		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
@@ -1208,8 +1217,22 @@ static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
 	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
 
 	spin_lock_irqsave(&pdata->xpcs_lock, flags);
-	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
-	XPCS16_IOWRITE(pdata, offset, mmd_data);
+	if (pdata->vdata->is_crater) {
+		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
+		amd_smn_read(0, pdata->xphy_base + offset, &crtr_mmd_data);
+		if (offset % ALIGNMENT_VAL) {
+			crtr_mmd_data &= ~GENMASK(31, 16);
+			crtr_mmd_data |=  (mmd_data << 16);
+		} else {
+			crtr_mmd_data &= ~GENMASK(15, 0);
+			crtr_mmd_data |=  (mmd_data);
+		}
+		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
+		amd_smn_write(0, (pdata->xphy_base + offset), crtr_mmd_data);
+	} else {
+		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
+		XPCS16_IOWRITE(pdata, offset, mmd_data);
+	}
 	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
 }
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index a17359d43b45..90ad520d3c29 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -279,15 +279,21 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		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)) {
-		pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
-		pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
-
-		/* Yellow Carp devices do not need cdr workaround */
+		   ((rdev->device == 0x14b5) || (rdev->device == 0x1630))) {
+		/* Yellow Carp and Crater devices
+		 * do not need cdr workaround and RRC
+		 */
 		pdata->vdata->an_cdr_workaround = 0;
-
-		/* Yellow Carp devices do not need rrc */
 		pdata->vdata->enable_rrc = 0;
+
+		if (rdev->device == 0x1630) {
+			pdata->xpcs_window_def_reg = PCS_V2_RN_WINDOW_DEF;
+			pdata->xpcs_window_sel_reg = PCS_V2_RN_WINDOW_SELECT;
+			pdata->vdata->is_crater = true;
+		} else {
+			pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
+			pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
+		}
 	} else {
 		pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
 		pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
@@ -295,7 +301,17 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_dev_put(rdev);
 
 	/* Configure the PCS indirect addressing support */
-	reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
+	if (pdata->vdata->is_crater) {
+		reg = XP_IOREAD(pdata, XP_PROP_0);
+		pdata->xphy_base = PCS_RN_SMN_BASE_ADDR +
+				   (PCS_RN_PORT_ADDR_SIZE *
+				    XP_GET_BITS(reg, XP_PROP_0, PORT_ID));
+		if (netif_msg_probe(pdata))
+			dev_dbg(dev, "xphy_base = %#08x\n", pdata->xphy_base);
+		amd_smn_read(0, pdata->xphy_base + (pdata->xpcs_window_def_reg), &reg);
+	} else {
+		reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
+	}
 	pdata->xpcs_window = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, OFFSET);
 	pdata->xpcs_window <<= 6;
 	pdata->xpcs_window_size = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, SIZE);
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index ad136ed493ed..a161fac35643 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -133,6 +133,7 @@
 #include <linux/dcache.h>
 #include <linux/ethtool.h>
 #include <linux/list.h>
+#include <asm/amd_nb.h>
 
 #define XGBE_DRV_NAME		"amd-xgbe"
 #define XGBE_DRV_DESC		"AMD 10 Gigabit Ethernet Driver"
@@ -305,6 +306,9 @@
 /* MDIO port types */
 #define XGMAC_MAX_C22_PORT		3
 
+ /* offset alignment */
+#define ALIGNMENT_VAL			4
+
 /* Link mode bit operations */
 #define XGBE_ZERO_SUP(_ls)		\
 	ethtool_link_ksettings_zero_link_mode((_ls), supported)
@@ -1046,6 +1050,7 @@ struct xgbe_version_data {
 	unsigned int rx_desc_prefetch;
 	unsigned int an_cdr_workaround;
 	unsigned int enable_rrc;
+	bool is_crater;
 };
 
 struct xgbe_prv_data {
@@ -1056,6 +1061,7 @@ struct xgbe_prv_data {
 	struct device *dev;
 	struct platform_device *phy_platdev;
 	struct device *phy_dev;
+	unsigned int xphy_base;
 
 	/* Version related data */
 	struct xgbe_version_data *vdata;
-- 
2.25.1

Re: [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device

[Simon Horman]

On Wed, Oct 18, 2023 at 08:14:50PM +0530, Raju Rangoju wrote:
> The AMD Crater device has new window settings for the XPCS access, add
> support to adopt to the new window settings. There is a hardware bug
> where in the BAR1 registers cannot be accessed directly. As a fallback
> mechanism, access these PCS registers through indirect access via SMN.
> 
> Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>

Hi Sudheesh,

some minor feedback from my side.

> ---
>  drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 ++++
>  drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 33 +++++++++++++++++----
>  drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 32 +++++++++++++++-----
>  drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++++
>  4 files changed, 63 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> index 3b70f6737633..e1f70f0528ef 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> @@ -900,6 +900,11 @@
>  #define PCS_V2_RV_WINDOW_SELECT		0x1064
>  #define PCS_V2_YC_WINDOW_DEF		0x18060
>  #define PCS_V2_YC_WINDOW_SELECT		0x18064
> +#define PCS_V2_RN_WINDOW_DEF		0xF8078
> +#define PCS_V2_RN_WINDOW_SELECT		0xF807c
> +
> +#define PCS_RN_SMN_BASE_ADDR		0x11E00000
> +#define PCS_RN_PORT_ADDR_SIZE		0x100000
>  
>  /* PCS register entry bit positions and sizes */
>  #define PCS_V2_WINDOW_DEF_OFFSET_INDEX	6
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index f393228d41c7..da8ec218282f 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -1176,8 +1176,17 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>  	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>  
>  	spin_lock_irqsave(&pdata->xpcs_lock, flags);
> -	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> -	mmd_data = XPCS16_IOREAD(pdata, offset);
> +	if (pdata->vdata->is_crater) {
> +		amd_smn_write(0,
> +			      (pdata->xphy_base + pdata->xpcs_window_sel_reg),
> +			      index);
> +		amd_smn_read(0, pdata->xphy_base + offset, &mmd_data);
> +		mmd_data = (offset % ALIGNMENT_VAL) ?
> +			   ((mmd_data >> 16) & 0xffff) : (mmd_data & 0xffff);

I wonder if it would be nice to use FIELD_GET() here...

> +	} else {
> +		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> +		mmd_data = XPCS16_IOREAD(pdata, offset);
> +	}
>  	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>  
>  	return mmd_data;
> @@ -1186,8 +1195,8 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>  static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>  				   int mmd_reg, int mmd_data)
>  {
> +	unsigned int mmd_address, index, offset, crtr_mmd_data;
>  	unsigned long flags;
> -	unsigned int mmd_address, index, offset;
>  
>  	if (mmd_reg & XGBE_ADDR_C45)
>  		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
> @@ -1208,8 +1217,22 @@ static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>  	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>  
>  	spin_lock_irqsave(&pdata->xpcs_lock, flags);
> -	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> -	XPCS16_IOWRITE(pdata, offset, mmd_data);
> +	if (pdata->vdata->is_crater) {
> +		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
> +		amd_smn_read(0, pdata->xphy_base + offset, &crtr_mmd_data);
> +		if (offset % ALIGNMENT_VAL) {
> +			crtr_mmd_data &= ~GENMASK(31, 16);
> +			crtr_mmd_data |=  (mmd_data << 16);
> +		} else {
> +			crtr_mmd_data &= ~GENMASK(15, 0);
> +			crtr_mmd_data |=  (mmd_data);
> +		}

... and FIELD_PREP() here.

> +		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);
> +		amd_smn_write(0, (pdata->xphy_base + offset), crtr_mmd_data);
> +	} else {
> +		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> +		XPCS16_IOWRITE(pdata, offset, mmd_data);
> +	}
>  	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>  }
>  
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index a17359d43b45..90ad520d3c29 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -279,15 +279,21 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		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)) {
> -		pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> -		pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> -
> -		/* Yellow Carp devices do not need cdr workaround */
> +		   ((rdev->device == 0x14b5) || (rdev->device == 0x1630))) {
> +		/* Yellow Carp and Crater devices
> +		 * do not need cdr workaround and RRC
> +		 */
>  		pdata->vdata->an_cdr_workaround = 0;
> -
> -		/* Yellow Carp devices do not need rrc */
>  		pdata->vdata->enable_rrc = 0;
> +
> +		if (rdev->device == 0x1630) {

Not strictly related to this patch, but I am wondering if we could create
#defines for magic numbers like this one.

> +			pdata->xpcs_window_def_reg = PCS_V2_RN_WINDOW_DEF;
> +			pdata->xpcs_window_sel_reg = PCS_V2_RN_WINDOW_SELECT;
> +			pdata->vdata->is_crater = true;

Is 'is_crater' necessary?
pdata has a pointer to the pci_dev, AFAICT.

> +		} else {
> +			pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> +			pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> +		}
>  	} else {
>  		pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>  		pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
> @@ -295,7 +301,17 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	pci_dev_put(rdev);
>  
>  	/* Configure the PCS indirect addressing support */
> -	reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> +	if (pdata->vdata->is_crater) {
> +		reg = XP_IOREAD(pdata, XP_PROP_0);
> +		pdata->xphy_base = PCS_RN_SMN_BASE_ADDR +
> +				   (PCS_RN_PORT_ADDR_SIZE *
> +				    XP_GET_BITS(reg, XP_PROP_0, PORT_ID));
> +		if (netif_msg_probe(pdata))
> +			dev_dbg(dev, "xphy_base = %#08x\n", pdata->xphy_base);
> +		amd_smn_read(0, pdata->xphy_base + (pdata->xpcs_window_def_reg), &reg);
> +	} else {
> +		reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> +	}
>  	pdata->xpcs_window = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, OFFSET);
>  	pdata->xpcs_window <<= 6;
>  	pdata->xpcs_window_size = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, SIZE);
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
> index ad136ed493ed..a161fac35643 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
> @@ -133,6 +133,7 @@
>  #include <linux/dcache.h>
>  #include <linux/ethtool.h>
>  #include <linux/list.h>
> +#include <asm/amd_nb.h>
>  
>  #define XGBE_DRV_NAME		"amd-xgbe"
>  #define XGBE_DRV_DESC		"AMD 10 Gigabit Ethernet Driver"
> @@ -305,6 +306,9 @@
>  /* MDIO port types */
>  #define XGMAC_MAX_C22_PORT		3
>  
> + /* offset alignment */
> +#define ALIGNMENT_VAL			4
> +
>  /* Link mode bit operations */
>  #define XGBE_ZERO_SUP(_ls)		\
>  	ethtool_link_ksettings_zero_link_mode((_ls), supported)
> @@ -1046,6 +1050,7 @@ struct xgbe_version_data {
>  	unsigned int rx_desc_prefetch;
>  	unsigned int an_cdr_workaround;
>  	unsigned int enable_rrc;
> +	bool is_crater;
>  };
>  
>  struct xgbe_prv_data {
> @@ -1056,6 +1061,7 @@ struct xgbe_prv_data {
>  	struct device *dev;
>  	struct platform_device *phy_platdev;
>  	struct device *phy_dev;
> +	unsigned int xphy_base;
>  
>  	/* Version related data */
>  	struct xgbe_version_data *vdata;
> -- 
> 2.25.1
> 
> 

Re: [PATCH net-next 0/2] amd-xgbe: add support for AMD Crater

[Tom Lendacky]

On 10/18/23 09:44, Raju Rangoju wrote:
> Add support for a new AMD Ethernet device called "Crater". It has a new
> PCI ID, add this to the current list of supported devices in the
> amd-xgbe devices.Also, the BAR1 addresses cannot be used to access the
> PCS registers on Crater platform, use the indirect addressing via SMN
> instead.

It looks like this should be a three patch series with new "Crater" device 
support, followed by a quirk/exception for the BAR1 address issue, and 
then finally your first patch being the last patch so that the device 
isn't enabled until the final patch.

Thanks,
Tom

> 
> Raju Rangoju (2):
>    amd-xgbe: add support for new pci device id 0x1641
>    amd-xgbe: Add support for AMD Crater ethernet device
> 
>   drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 +++
>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 33 +++++++++++++++++---
>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 34 ++++++++++++++++-----
>   drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++++
>   4 files changed, 65 insertions(+), 13 deletions(-)
> 

Re: [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device

[Tom Lendacky]

On 10/18/23 09:44, Raju Rangoju wrote:
> The AMD Crater device has new window settings for the XPCS access, add
> support to adopt to the new window settings. There is a hardware bug
> where in the BAR1 registers cannot be accessed directly. As a fallback
> mechanism, access these PCS registers through indirect access via SMN. >
> Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com>
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 ++++
>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 33 +++++++++++++++++----
>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 32 +++++++++++++++-----
>   drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++++
>   4 files changed, 63 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> index 3b70f6737633..e1f70f0528ef 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> @@ -900,6 +900,11 @@
>   #define PCS_V2_RV_WINDOW_SELECT		0x1064
>   #define PCS_V2_YC_WINDOW_DEF		0x18060
>   #define PCS_V2_YC_WINDOW_SELECT		0x18064
> +#define PCS_V2_RN_WINDOW_DEF		0xF8078
> +#define PCS_V2_RN_WINDOW_SELECT		0xF807c
> +
> +#define PCS_RN_SMN_BASE_ADDR		0x11E00000
> +#define PCS_RN_PORT_ADDR_SIZE		0x100000
>   
>   /* PCS register entry bit positions and sizes */
>   #define PCS_V2_WINDOW_DEF_OFFSET_INDEX	6
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index f393228d41c7..da8ec218282f 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -1176,8 +1176,17 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>   	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>   
>   	spin_lock_irqsave(&pdata->xpcs_lock, flags);
> -	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> -	mmd_data = XPCS16_IOREAD(pdata, offset);
> +	if (pdata->vdata->is_crater) {

This feels like it should be a new xgbe_read_mmd_regs_v3() and devices 
with the this bug should have a new XGBE_XPCS_ACCESS_V3 type for 
pdata->vdata->xpcs_access.

Feel free to write a pre-patch, too, that creates helper functions to 
calculate the mmd_address for v1/2/3 and the index/offset for v2/3 types.

> +		amd_smn_write(0,
> +			      (pdata->xphy_base + pdata->xpcs_window_sel_reg),

No need for the parentheses here and all the parameters can be on one line 
now that there is a 100 character line limit.

> +			      index);
> +		amd_smn_read(0, pdata->xphy_base + offset, &mmd_data);
> +		mmd_data = (offset % ALIGNMENT_VAL) ?
> +			   ((mmd_data >> 16) & 0xffff) : (mmd_data & 0xffff);
> +	} else {
> +		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> +		mmd_data = XPCS16_IOREAD(pdata, offset);
> +	}
>   	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>   
>   	return mmd_data;
> @@ -1186,8 +1195,8 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>   static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>   				   int mmd_reg, int mmd_data)
>   {
> +	unsigned int mmd_address, index, offset, crtr_mmd_data;
>   	unsigned long flags;
> -	unsigned int mmd_address, index, offset;
>   
>   	if (mmd_reg & XGBE_ADDR_C45)
>   		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
> @@ -1208,8 +1217,22 @@ static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>   	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>   
>   	spin_lock_irqsave(&pdata->xpcs_lock, flags);
> -	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> -	XPCS16_IOWRITE(pdata, offset, mmd_data);
> +	if (pdata->vdata->is_crater) {
> +		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);

No parentheses needed on "(pdata->xphy_base + pdata->xpcs_window_sel_reg)"

> +		amd_smn_read(0, pdata->xphy_base + offset, &crtr_mmd_data);
> +		if (offset % ALIGNMENT_VAL) {
> +			crtr_mmd_data &= ~GENMASK(31, 16);
> +			crtr_mmd_data |=  (mmd_data << 16);
> +		} else {
> +			crtr_mmd_data &= ~GENMASK(15, 0);
> +			crtr_mmd_data |=  (mmd_data);
> +		}
> +		amd_smn_write(0, (pdata->xphy_base + pdata->xpcs_window_sel_reg), index);

Ditto on the parentheses here...

> +		amd_smn_write(0, (pdata->xphy_base + offset), crtr_mmd_data);

and here.

> +	} else {
> +		XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
> +		XPCS16_IOWRITE(pdata, offset, mmd_data);
> +	}

Ditto here, having a v3 function will make this much cleaner.

>   	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>   }
>   
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index a17359d43b45..90ad520d3c29 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -279,15 +279,21 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		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)) {
> -		pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> -		pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> -
> -		/* Yellow Carp devices do not need cdr workaround */
> +		   ((rdev->device == 0x14b5) || (rdev->device == 0x1630))) {

At this point it might make sense to move to a switch statement for each 
device.

> +		/* Yellow Carp and Crater devices
> +		 * do not need cdr workaround and RRC

Please use the full width available for comments.

> +		 */
>   		pdata->vdata->an_cdr_workaround = 0;
> -
> -		/* Yellow Carp devices do not need rrc */
>   		pdata->vdata->enable_rrc = 0;
> +
> +		if (rdev->device == 0x1630) {
> +			pdata->xpcs_window_def_reg = PCS_V2_RN_WINDOW_DEF;
> +			pdata->xpcs_window_sel_reg = PCS_V2_RN_WINDOW_SELECT;
> +			pdata->vdata->is_crater = true;
> +		} else {
> +			pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
> +			pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
> +		}
>   	} else {
>   		pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>   		pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
> @@ -295,7 +301,17 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	pci_dev_put(rdev);
>   
>   	/* Configure the PCS indirect addressing support */
> -	reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> +	if (pdata->vdata->is_crater) {
> +		reg = XP_IOREAD(pdata, XP_PROP_0);
> +		pdata->xphy_base = PCS_RN_SMN_BASE_ADDR +
> +				   (PCS_RN_PORT_ADDR_SIZE *
> +				    XP_GET_BITS(reg, XP_PROP_0, PORT_ID));
> +		if (netif_msg_probe(pdata))
> +			dev_dbg(dev, "xphy_base = %#08x\n", pdata->xphy_base);
> +		amd_smn_read(0, pdata->xphy_base + (pdata->xpcs_window_def_reg), &reg);

No need for the parentheses around "(pdata->xpcs_window_def_reg)"

> +	} else {
> +		reg = XPCS32_IOREAD(pdata, pdata->xpcs_window_def_reg);
> +	}
>   	pdata->xpcs_window = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, OFFSET);
>   	pdata->xpcs_window <<= 6;
>   	pdata->xpcs_window_size = XPCS_GET_BITS(reg, PCS_V2_WINDOW_DEF, SIZE);
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
> index ad136ed493ed..a161fac35643 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
> @@ -133,6 +133,7 @@
>   #include <linux/dcache.h>
>   #include <linux/ethtool.h>
>   #include <linux/list.h>
> +#include <asm/amd_nb.h>

Why is this needed in this header?

Also, this is buildable for arm64 only, so you'll need to protect the use 
of this include with a "#ifdef CONFIG_X86" when you put it in the proper 
file(s).

>   
>   #define XGBE_DRV_NAME		"amd-xgbe"
>   #define XGBE_DRV_DESC		"AMD 10 Gigabit Ethernet Driver"
> @@ -305,6 +306,9 @@
>   /* MDIO port types */
>   #define XGMAC_MAX_C22_PORT		3
>   
> + /* offset alignment */
> +#define ALIGNMENT_VAL			4

This is a bit generic, how about something that describes what this is a 
bit more.

> +
>   /* Link mode bit operations */
>   #define XGBE_ZERO_SUP(_ls)		\
>   	ethtool_link_ksettings_zero_link_mode((_ls), supported)
> @@ -1046,6 +1050,7 @@ struct xgbe_version_data {
>   	unsigned int rx_desc_prefetch;
>   	unsigned int an_cdr_workaround;
>   	unsigned int enable_rrc;
> +	bool is_crater;

Probably better to name this "use_smn" or such vs is_crater.

>   };
>   
>   struct xgbe_prv_data {
> @@ -1056,6 +1061,7 @@ struct xgbe_prv_data {
>   	struct device *dev;
>   	struct platform_device *phy_platdev;
>   	struct device *phy_dev;
> +	unsigned int xphy_base;

And then probably better to name this smn_base.

Thanks,
Tom

>   
>   	/* Version related data */
>   	struct xgbe_version_data *vdata;

Re: [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device

[kernel test robot]

Hi Raju,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Raju-Rangoju/amd-xgbe-add-support-for-new-pci-device-id-0x1641/20231018-224804
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231018144450.2061125-3-Raju.Rangoju%40amd.com
patch subject: [PATCH net-next 2/2] amd-xgbe: Add support for AMD Crater ethernet device
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20231022/202310221212.UV7uO9yX-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231022/202310221212.UV7uO9yX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310221212.UV7uO9yX-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/amd/xgbe/xgbe-main.c:125:
>> drivers/net/ethernet/amd/xgbe/xgbe.h:136:10: fatal error: asm/amd_nb.h: No such file or directory
     136 | #include <asm/amd_nb.h>
         |          ^~~~~~~~~~~~~~
   compilation terminated.


vim +136 drivers/net/ethernet/amd/xgbe/xgbe.h

   119	
   120	#include <linux/dma-mapping.h>
   121	#include <linux/netdevice.h>
   122	#include <linux/workqueue.h>
   123	#include <linux/phy.h>
   124	#include <linux/if_vlan.h>
   125	#include <linux/bitops.h>
   126	#include <linux/ptp_clock_kernel.h>
   127	#include <linux/timecounter.h>
   128	#include <linux/net_tstamp.h>
   129	#include <net/dcbnl.h>
   130	#include <linux/completion.h>
   131	#include <linux/cpumask.h>
   132	#include <linux/interrupt.h>
   133	#include <linux/dcache.h>
   134	#include <linux/ethtool.h>
   135	#include <linux/list.h>
 > 136	#include <asm/amd_nb.h>
   137	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki