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);
>
