Re: [PATCH net-next 3/5] amd-xgbe: add support for new XPCS routines

["Rangoju, Raju" <raju.rangoju@amd.com>]

On 4/11/2025 3:48 PM, Larysa Zaremba wrote:
> On Tue, Apr 08, 2025 at 11:49:59PM +0530, Raju Rangoju wrote:
>> Add the necessary support to enable Crater ethernet device. Since the
>> BAR1 address cannot be used to access the XPCS registers on Crater, use
>> the smn functions.
>>
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
>> ---
>>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 79 ++++++++++++++++++++++++
>>   drivers/net/ethernet/amd/xgbe/xgbe.h     |  6 ++
>>   2 files changed, 85 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> index ae82dc3ac460..d75cf8df272f 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/bitrev.h>
>>   #include <linux/crc32.h>
>>   #include <linux/crc32poly.h>
>> +#include <linux/pci.h>
>>   
>>   #include "xgbe.h"
>>   #include "xgbe-common.h"
>> @@ -1066,6 +1067,78 @@ static void get_pcs_index_and_offset(struct xgbe_prv_data *pdata,
>>   	*offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>>   }
>>   
>> +static int xgbe_read_mmd_regs_v3(struct xgbe_prv_data *pdata, int prtad,
>> +				 int mmd_reg)
>> +{
>> +	unsigned int mmd_address, index, offset;
>> +	struct pci_dev *rdev;
>> +	unsigned long flags;
>> +	int mmd_data;
>> +
>> +	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>> +	if (!rdev)
>> +		return 0;
> 
> Why do you operate on the root device's config space? Is this SoC-specific,
> like in ixgbe_x550em_a_has_mii()? If so, would be nice to have a comment or at
> least something in the commit message.

Yes. We have additional patches in development that follow this path, 
and I'll ensure that future patches include relevant comments to clarify 
this.

> 
>> +
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>> +
>> +	get_pcs_index_and_offset(pdata, mmd_address, &index, &offset);
>> +
>> +	spin_lock_irqsave(&pdata->xpcs_lock, flags);
>> +	pci_write_config_dword(rdev, 0x60, (pdata->xphy_base + pdata->xpcs_window_sel_reg));
>> +	pci_write_config_dword(rdev, 0x64, index);
>> +	pci_write_config_dword(rdev, 0x60, pdata->xphy_base + offset);
>> +	pci_read_config_dword(rdev, 0x64, &mmd_data);
>> +	mmd_data = (offset % 4) ? FIELD_GET(XGBE_GEN_HI_MASK, mmd_data) :
>> +				  FIELD_GET(XGBE_GEN_LO_MASK, mmd_data);
>> +
>> +	pci_dev_put(rdev);
>> +	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>> +
>> +	return mmd_data;
>> +}
>> +
>> +static void xgbe_write_mmd_regs_v3(struct xgbe_prv_data *pdata, int prtad,
>> +				   int mmd_reg, int mmd_data)
>> +{
>> +	unsigned int pci_mmd_data, hi_mask, lo_mask;
>> +	unsigned int mmd_address, index, offset;
>> +	struct pci_dev *rdev;
>> +	unsigned long flags;
>> +
>> +	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>> +	if (!rdev)
>> +		return;
>> +
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>> +
>> +	get_pcs_index_and_offset(pdata, mmd_address, &index, &offset);
>> +
>> +	spin_lock_irqsave(&pdata->xpcs_lock, flags);
>> +	pci_write_config_dword(rdev, 0x60, (pdata->xphy_base + pdata->xpcs_window_sel_reg));
>> +	pci_write_config_dword(rdev, 0x64, index);
>> +	pci_write_config_dword(rdev, 0x60, pdata->xphy_base + offset);
>> +	pci_read_config_dword(rdev, 0x64, &pci_mmd_data);
>> +
>> +	if (offset % 4) {
>> +		hi_mask = FIELD_PREP(XGBE_GEN_HI_MASK, mmd_data);
>> +		lo_mask = FIELD_GET(XGBE_GEN_LO_MASK, pci_mmd_data);
>> +	} else {
>> +		hi_mask = FIELD_PREP(XGBE_GEN_HI_MASK,
>> +				     FIELD_GET(XGBE_GEN_HI_MASK, pci_mmd_data));
>> +		lo_mask = FIELD_GET(XGBE_GEN_LO_MASK, mmd_data);
>> +	}
>> +
>> +	pci_mmd_data = hi_mask | lo_mask;
>> +
>> +	pci_write_config_dword(rdev, 0x60, (pdata->xphy_base + pdata->xpcs_window_sel_reg));
>> +	pci_write_config_dword(rdev, 0x64, index);
>> +	pci_write_config_dword(rdev, 0x60, (pdata->xphy_base + offset));
>> +	pci_write_config_dword(rdev, 0x64, pci_mmd_data);
>> +	pci_dev_put(rdev);
>> +
>> +	spin_unlock_irqrestore(&pdata->xpcs_lock, flags);
>> +}
>> +
>>   static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>>   				 int mmd_reg)
>>   {
>> @@ -1160,6 +1233,9 @@ static int xgbe_read_mmd_regs(struct xgbe_prv_data *pdata, int prtad,
>>   	case XGBE_XPCS_ACCESS_V2:
>>   	default:
>>   		return xgbe_read_mmd_regs_v2(pdata, prtad, mmd_reg);
>> +
>> +	case XGBE_XPCS_ACCESS_V3:
>> +		return xgbe_read_mmd_regs_v3(pdata, prtad, mmd_reg);
>>   	}
>>   }
>>   
>> @@ -1173,6 +1249,9 @@ static void xgbe_write_mmd_regs(struct xgbe_prv_data *pdata, int prtad,
>>   	case XGBE_XPCS_ACCESS_V2:
>>   	default:
>>   		return xgbe_write_mmd_regs_v2(pdata, prtad, mmd_reg, mmd_data);
>> +
>> +	case XGBE_XPCS_ACCESS_V3:
>> +		return xgbe_write_mmd_regs_v3(pdata, prtad, mmd_reg, mmd_data);
>>   	}
>>   }
>>   
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> index 2e9b3be44ff8..6c49bf19e537 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> @@ -242,6 +242,10 @@
>>   #define XGBE_RV_PCI_DEVICE_ID	0x15d0
>>   #define XGBE_YC_PCI_DEVICE_ID	0x14b5
>>   
>> + /* Generic low and high masks */
>> +#define XGBE_GEN_HI_MASK	GENMASK(31, 16)
>> +#define XGBE_GEN_LO_MASK	GENMASK(15, 0)
>> +
>>   struct xgbe_prv_data;
>>   
>>   struct xgbe_packet_data {
>> @@ -460,6 +464,7 @@ enum xgbe_speed {
>>   enum xgbe_xpcs_access {
>>   	XGBE_XPCS_ACCESS_V1 = 0,
>>   	XGBE_XPCS_ACCESS_V2,
>> +	XGBE_XPCS_ACCESS_V3,
>>   };
>>   
>>   enum xgbe_an_mode {
>> @@ -951,6 +956,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.34.1
>>
>>