[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.yi...@intel.com>
Sent: Thursday, January 9, 2020 3:48 PM
To: Sebastian, Selwin <selwin.sebast...@amd.com>; dev@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.yi...@intel.com>
> Sent: Tuesday, January 7, 2020 7:28 PM
> To: Sebastian, Selwin <selwin.sebast...@amd.com>; dev@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.yi...@intel.com>
>> Sent: Wednesday, December 11, 2019 5:12 PM
>> To: Sebastian, Selwin <selwin.sebast...@amd.com>; dev@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.sebast...@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);
>>>
