On 1/20/2020 12:41 PM, Selwin Sebastian wrote:
> V1000/R1000 processors are using the same PCI ids for the network
> device as SNOWYOWL processor 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.
>
> Signed-off-by: Selwin Sebastian <Selwin.Sebastian@amd.com>
> ---
> drivers/net/axgbe/axgbe_common.h | 2 ++
> drivers/net/axgbe/axgbe_ethdev.c | 40 +++++++++++++++++++++++++++++---
> 2 files changed, 39 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..f5dbba292 100644
> --- a/drivers/net/axgbe/axgbe_ethdev.c
> +++ b/drivers/net/axgbe/axgbe_ethdev.c
> @@ -29,6 +29,7 @@ static int axgbe_dev_info_get(struct rte_eth_dev *dev,
>
> /* The set of PCI devices this driver supports */
> #define AMD_PCI_VENDOR_ID 0x1022
> +#define AMD_PCI_RV_ROOT_COMPLEX_ID 0x15d0
> #define AMD_PCI_AXGBE_DEVICE_V2A 0x1458
> #define AMD_PCI_AXGBE_DEVICE_V2B 0x1459
>
> @@ -574,6 +575,29 @@ static void axgbe_default_config(struct axgbe_port *pdata)
> pdata->power_down = 0;
> }
>
> +static inline int
> +pci_device_cmp(const struct rte_device *dev, const void *_pci_id)
> +{
> + const struct rte_pci_device *pdev = RTE_DEV_TO_PCI_CONST(dev);
> + const struct rte_pci_id *pcid = _pci_id;
> +
> + if (pdev->id.vendor_id == AMD_PCI_VENDOR_ID &&
> + pdev->id.device_id == pcid->device_id)
> + return 0;
> + return 1;
> +}
What is the point of 'inline' keyword, since the function will be used with its
pointer, can we drop it?
> +
> +static bool pci_search_device(int device_id)
> +{
Better to follow coding convention as done in above function.
> + struct rte_bus *pci_bus;
> + struct rte_pci_id dev_id;
> +
> + dev_id.device_id = device_id;
> + pci_bus = rte_bus_find_by_name("pci");
> + return (pci_bus != NULL) &&
> + (pci_bus->find_device(NULL, pci_device_cmp, &dev_id) != NULL);
> +}
> +
> /*
> * It returns 0 on success.
> */
> @@ -605,6 +629,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;
>
> + /*
> + * Use root complex device ID to differentiate RV AXGBE vs SNOWY AXGBE
> + */
> + if (pci_search_device(AMD_PCI_RV_ROOT_COMPLEX_ID)) {
> + 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 +655,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);
>