[dpdk-dev] [PATCH v1] net/axgbe: Add a HW quirk for register definitions

Sebastian, Selwin Selwin.Sebastian at amd.com
Fri Jan 10 14:24:54 CET 2020


[AMD Official Use Only - Internal Distribution Only]

Hi Ferruh,
	I checked for FW version, PHY/MAC , registers for differentiating and it cannot be used. 

Thanks and Regards
Selwin Sebastian

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit at intel.com> 
Sent: Thursday, January 9, 2020 3:48 PM
To: Sebastian, Selwin <Selwin.Sebastian at amd.com>; dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1] net/axgbe: Add a HW quirk for register definitions

[CAUTION: External Email]

On 1/9/2020 7:15 AM, Sebastian, Selwin wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Ferruh,
>       I submitted v2 of the patch as per your guidelines. I checked sub-device ids and they are also the same. I am not aware of a better way to address this issue and even Linux driver is handling it using the same quirk.

Unfortunately HW quirks are happens. As a last try, can there be any FW version, PHY/MAC type, any specific register value to differentiate the device?
to prevent accessing the pci device list...

> Yes,  root complex device will always be the first device.
>
> Thanks and Regards
> Selwin Sebastian
>
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit at intel.com>
> Sent: Tuesday, January 7, 2020 7:28 PM
> To: Sebastian, Selwin <Selwin.Sebastian at amd.com>; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] net/axgbe: Add a HW quirk for 
> register definitions
>
> [CAUTION: External Email]
>
> On 12/17/2019 6:44 AM, Sebastian, Selwin wrote:
>> [AMD Official Use Only - Internal Distribution Only]
>>
>> Hi Ferruh,
>>       Current driver was developed for EPYC 3000 processors. New processors V1000/R1000 is also using the same PCI id for axgbe but register definitions for determining the window settings for indirect PCS access is changed. In order to identify processor, we are adding a quirk.
>> 15d0 is the pci id for V1000/R1000/Raven root complex( https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpci-ids.ucw.cz%2Fread%2FPC%2F1022&data=02%7C01%7CSelwin.Sebastian%40amd.com%7C4ca73d7c9e8b4271935c08d794ed3d57%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637141618970643135&sdata=DMGjrIDvnKMjjdOp2E4fVDd8YU0RFZALqTHoMpazwc4%3D&reserved=0 ). Hence read pci-id of root complex  to determine which processor and set the registers accordingly.
>>
>
> Got it, it is better to add a define for 0x15d0 with an explanation, and for the root complex device use a more descriptive variable name that 'pdev'.
>
> But still it is not really good idea to access the pci device list, isn't there any other way to differentiate the devices, sub-device id etc? And how does linux driver manages this?
>
> And is it guaranteed that root complex device always will be the first device?
>
>
>> Thanks and Regards
>> Selwin Sebastian
>>
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit at intel.com>
>> Sent: Wednesday, December 11, 2019 5:12 PM
>> To: Sebastian, Selwin <Selwin.Sebastian at amd.com>; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v1] net/axgbe: Add a HW quirk for 
>> register definitions
>>
>> [CAUTION: External Email]
>>
>> On 12/10/2019 3:29 PM, Selwin Sebastian wrote:
>>> V1000/R1000 processors are using the same PCI ids for the network 
>>> device but has altered register definitions for determining the 
>>> window settings for the indirect PCS access.Add support to check for 
>>> this hardware and if found use the new register values
>>
>> How they are differentiated, subdevice ids?
>> If so should we add subdevice fields check into DPDK?
>>
>>>
>>> Signed-off-by: Selwin Sebastian <selwin.sebastian at amd.com>
>>> ---
>>>  drivers/net/axgbe/axgbe_common.h |  2 ++ 
>>> drivers/net/axgbe/axgbe_ethdev.c | 18 +++++++++++++++---
>>>  2 files changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/axgbe/axgbe_common.h
>>> b/drivers/net/axgbe/axgbe_common.h
>>> index 34f60f156..4a3fbac16 100644
>>> --- a/drivers/net/axgbe/axgbe_common.h
>>> +++ b/drivers/net/axgbe/axgbe_common.h
>>> @@ -841,6 +841,8 @@
>>>  #define PCS_V1_WINDOW_SELECT         0x03fc
>>>  #define PCS_V2_WINDOW_DEF            0x9060
>>>  #define PCS_V2_WINDOW_SELECT         0x9064
>>> +#define PCS_V2_RV_WINDOW_DEF         0x1060
>>> +#define PCS_V2_RV_WINDOW_SELECT              0x1064
>>>
>>>  /* PCS register entry bit positions and sizes */
>>>  #define PCS_V2_WINDOW_DEF_OFFSET_INDEX       6
>>> diff --git a/drivers/net/axgbe/axgbe_ethdev.c
>>> b/drivers/net/axgbe/axgbe_ethdev.c
>>> index d1f160e79..25e182b8d 100644
>>> --- a/drivers/net/axgbe/axgbe_ethdev.c
>>> +++ b/drivers/net/axgbe/axgbe_ethdev.c
>>> @@ -31,6 +31,7 @@ static int  axgbe_dev_info_get(struct rte_eth_dev *dev,
>>>  #define AMD_PCI_VENDOR_ID       0x1022
>>>  #define AMD_PCI_AXGBE_DEVICE_V2A 0x1458  #define 
>>> AMD_PCI_AXGBE_DEVICE_V2B 0x1459
>>> +extern struct rte_pci_bus rte_pci_bus;
>>
>> Not sure about accessing the bus device list from a PMD...
>>
>>>
>>>  int axgbe_logtype_init;
>>>  int axgbe_logtype_driver;
>>> @@ -585,6 +586,7 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>>>       struct rte_pci_device *pci_dev;
>>>       uint32_t reg, mac_lo, mac_hi;
>>>       int ret;
>>> +     struct rte_pci_device *pdev;
>>>
>>>       eth_dev->dev_ops = &axgbe_eth_dev_ops;
>>>       eth_dev->rx_pkt_burst = &axgbe_recv_pkts; @@ -605,6 +607,17 @@ 
>>> eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>>>       pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
>>>       pdata->pci_dev = pci_dev;
>>>
>>> +     pdev = TAILQ_FIRST(&rte_pci_bus.device_list);
>>
>> Can you please describe what this does? You are reading first pci device and do you assume it is an axgbe device? And do you also assume there is single axgbe device?
>>
>> Why you are not simply using 'pci_dev' above?
>>
>>> +
>>> +     if (pdev->id.vendor_id == AMD_PCI_VENDOR_ID &&
>>> +             pdev->id.device_id == 0x15d0) {
>>
>> As far as I can see, '0x15d0' is not in the supported pci_id list, so why you are checking it here? That devices shouldn't be probed at all ...
>>
>>> +                     pdata->xpcs_window_def_reg = PCS_V2_RV_WINDOW_DEF;
>>> +                     pdata->xpcs_window_sel_reg = PCS_V2_RV_WINDOW_SELECT;
>>> +     } else {
>>> +             pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>>> +             pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
>>> +     }
>>> +
>>>       pdata->xgmac_regs =
>>>               (void *)pci_dev->mem_resource[AXGBE_AXGMAC_BAR].addr;
>>>       pdata->xprop_regs = (void *)((uint8_t *)pdata->xgmac_regs @@
>>> -620,14 +633,13 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>>>               pdata->vdata = &axgbe_v2b;
>>>
>>>       /* Configure the PCS indirect addressing support */
>>> -     reg = XPCS32_IOREAD(pdata, PCS_V2_WINDOW_DEF);
>>> +     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);
>>>       pdata->xpcs_window_size = 1 << (pdata->xpcs_window_size + 7);
>>>       pdata->xpcs_window_mask = pdata->xpcs_window_size - 1;
>>> -     pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>>> -     pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
>>> +
>>>       PMD_INIT_LOG(DEBUG,
>>>                    "xpcs window :%x, size :%x, mask :%x ", pdata->xpcs_window,
>>>                    pdata->xpcs_window_size, 
>>> pdata->xpcs_window_mask);
>>>


More information about the dev mailing list