On 1/14/2020 5:30 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.
>
> Added a new routine rte_pci_search_device to pci driver to search
> for a device.
>
> Signed-off-by: Selwin Sebastian <selwin.sebast...@amd.com>
> ---
> drivers/bus/pci/pci_common.c | 17 +++++++++++++++++
> drivers/bus/pci/rte_bus_pci.h | 12 ++++++++++++
> drivers/net/axgbe/axgbe_common.h | 2 ++
> drivers/net/axgbe/axgbe_ethdev.c | 18 +++++++++++++++---
> 4 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 3f5542076..e991a2580 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -393,6 +393,23 @@ rte_pci_add_device(struct rte_pci_device *pci_dev)
> TAILQ_INSERT_TAIL(&rte_pci_bus.device_list, pci_dev, next);
> }
>
> +/* Search for a specific device in PCI bus */
> +
> +int
> +rte_pci_search_device(int vendor_id, int device_id)
> +{
> + struct rte_pci_device *pdev;
> + pdev = TAILQ_FIRST(&rte_pci_bus.device_list);
> +
> + while (pdev != NULL) {
> + if (pdev->id.vendor_id == vendor_id &&
> + pdev->id.device_id == device_id)
> + return true;
> + pdev = TAILQ_NEXT(pdev, next);
> + }
> + return false;
> +}
Overall this is what I was asking, thanks.
A few details on the implementation,
What do you think returning the "struct rte_pci_device *", having it NULL
meaning device not found?
Also you can provide the "struct rte_pci_id" as input, instead of the just
vendor and device id?
Can be multiple ways to search the device in the bus, what about changing the
API name to 'rte_pci_search_device_by_id()' to clarify this search is by id?
What about having the API to use the existing 'pci_find_device' function, 'cmp'
function becomes simple and I think it makes the logic simpler but that is open
to discussion I guess.
> +
> /* Insert a device into a predefined position in PCI bus */
> void
> rte_pci_insert_device(struct rte_pci_device *exist_pci_dev,
> diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
> index 29bea6d70..42b6c7e03 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -354,6 +354,18 @@ void rte_pci_ioport_read(struct rte_pci_ioport *p,
> void rte_pci_ioport_write(struct rte_pci_ioport *p,
> const void *data, size_t len, off_t offset);
>
> +/*
> + * Search for a specific device in pci bus
> + *
> + * @param vendor_id
> + * vendor_id of the device to search
> + * @param device_id
> + * device_id of the device to search
> + * @return
> + * 1 on success, 0 on failure
> + */
> +int rte_pci_search_device(int vendor_id, int device_id);
There are a few process details to follow,
- Need to put the API to .map file, so it can be called by other libraries or
application.
- New APIs need to be experimental at least one release, this is done by:
-- put '__rte_experimental' to the API decleration
-- put API in .map file into the EXPERIMENTAL section
> +
> #ifdef __cplusplus
> }
> #endif
> 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..01975236b 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
>
> @@ -605,6 +606,18 @@ 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 (rte_pci_search_device(AMD_PCI_VENDOR_ID,
> + 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 +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);
>
