DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* Re: [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions
  2020-01-14 17:30 [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions Selwin Sebastian
@ 2020-01-14 12:30 ` Ferruh Yigit
  2020-01-14 12:35 ` David Marchand
  1 sibling, 0 replies; 4+ messages in thread
From: Ferruh Yigit @ 2020-01-14 12:30 UTC (permalink / raw)
  To: Selwin Sebastian, dev; +Cc: thomas, david.marchand

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.Sebastian@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);
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions
  2020-01-14 17:30 [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions Selwin Sebastian
  2020-01-14 12:30 ` Ferruh Yigit
@ 2020-01-14 12:35 ` David Marchand
  2020-01-14 13:18   ` Ferruh Yigit
  1 sibling, 1 reply; 4+ messages in thread
From: David Marchand @ 2020-01-14 12:35 UTC (permalink / raw)
  To: Selwin Sebastian; +Cc: dev, Yigit, Ferruh, Thomas Monjalon

On Tue, Jan 14, 2020 at 12:58 PM Selwin Sebastian
<Selwin.Sebastian@amd.com> 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.

You can already iterate on a bus devices.

struct rte_bus *pci_bus = rte_bus_find_by_name("pci");
return pci_bus != NULL && pci_bus->find_device(NULL, callback, args) != NULL;

See: https://git.dpdk.org/dpdk/tree/drivers/bus/pci/pci_params.c#n38


-- 
David Marchand


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions
  2020-01-14 12:35 ` David Marchand
@ 2020-01-14 13:18   ` Ferruh Yigit
  0 siblings, 0 replies; 4+ messages in thread
From: Ferruh Yigit @ 2020-01-14 13:18 UTC (permalink / raw)
  To: David Marchand, Selwin Sebastian; +Cc: dev, Thomas Monjalon

On 1/14/2020 12:35 PM, David Marchand wrote:
> On Tue, Jan 14, 2020 at 12:58 PM Selwin Sebastian
> <Selwin.Sebastian@amd.com> 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.
> 
> You can already iterate on a bus devices.
> 
> struct rte_bus *pci_bus = rte_bus_find_by_name("pci");
> return pci_bus != NULL && pci_bus->find_device(NULL, callback, args) != NULL;
> 
> See: https://git.dpdk.org/dpdk/tree/drivers/bus/pci/pci_params.c#n38
> 

Nice, this looks better approach.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions
@ 2020-01-14 17:30 Selwin Sebastian
  2020-01-14 12:30 ` Ferruh Yigit
  2020-01-14 12:35 ` David Marchand
  0 siblings, 2 replies; 4+ messages in thread
From: Selwin Sebastian @ 2020-01-14 17:30 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, thomas, david.marchand, Selwin Sebastian

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.Sebastian@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;
+}
+
 /* 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);
+
 #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);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 17:30 [dpdk-dev] [PATCH v3] drivers: add a HW quirk for register definitions Selwin Sebastian
2020-01-14 12:30 ` Ferruh Yigit
2020-01-14 12:35 ` David Marchand
2020-01-14 13:18   ` Ferruh Yigit

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git