DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek
@ 2022-01-18  3:17 Wei Huang
  2022-01-18  3:17 ` [PATCH v1 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  3:17 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

This patchset is bug fix for raw/ifpga driver.
Changes:
* Fix SPI transaction.
* Fix pthread cannot join.
* Fix ifpga devices cleanup function.
* Fix port feature ID.

Tianfei Zhang (2):
  raw/ifpga/base: fix SPI transaction
  raw/ifpga: fix pthread cannot join

Wei Huang (2):
  raw/ifpga: fix ifpga devices cleanup function
  raw/ifpga/base: fix port feature ID

 drivers/raw/ifpga/base/ifpga_defines.h        |   6 +-
 drivers/raw/ifpga/base/opae_spi.c             |  12 ++
 drivers/raw/ifpga/base/opae_spi.h             |   4 +
 drivers/raw/ifpga/base/opae_spi_transaction.c | 215 +++++++++++++++-----------
 drivers/raw/ifpga/ifpga_rawdev.c              |  12 +-
 5 files changed, 150 insertions(+), 99 deletions(-)

-- 
1.8.3.1


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

* [PATCH v1 1/4] raw/ifpga/base: fix SPI transaction
  2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
@ 2022-01-18  3:17 ` Wei Huang
  2022-01-18  3:17 ` [PATCH v1 2/4] raw/ifpga: fix pthread cannot join Wei Huang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  3:17 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit

From: Tianfei Zhang <tianfei.zhang@intel.com>

When EOP is detected, 2 more bytes should be received
(may be a SPI_PACKET_ESC before last valid byte) then
rx should be finished.

Fixes: 96ebfcf8 ("raw/ifpga/base: add SPI and MAX10 device driver")
Cc: stable@dpdk.org

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/base/opae_spi.c             |  12 ++
 drivers/raw/ifpga/base/opae_spi.h             |   4 +
 drivers/raw/ifpga/base/opae_spi_transaction.c | 215 +++++++++++++++-----------
 3 files changed, 140 insertions(+), 91 deletions(-)

diff --git a/drivers/raw/ifpga/base/opae_spi.c b/drivers/raw/ifpga/base/opae_spi.c
index 9efeecb..ca3d41f 100644
--- a/drivers/raw/ifpga/base/opae_spi.c
+++ b/drivers/raw/ifpga/base/opae_spi.c
@@ -239,6 +239,18 @@ int spi_command(struct altera_spi_device *dev, unsigned int chip_select,
 	return 0;
 }
 
+int spi_write(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int wlen, void *wdata)
+{
+	return spi_command(dev, chip_select, wlen, wdata, 0, NULL);
+}
+
+int spi_read(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int rlen, void *rdata)
+{
+	return spi_command(dev, chip_select, 0, NULL, rlen, rdata);
+}
+
 struct altera_spi_device *altera_spi_alloc(void *base, int type)
 {
 	struct altera_spi_device *spi_dev =
diff --git a/drivers/raw/ifpga/base/opae_spi.h b/drivers/raw/ifpga/base/opae_spi.h
index af11656..bcff67d 100644
--- a/drivers/raw/ifpga/base/opae_spi.h
+++ b/drivers/raw/ifpga/base/opae_spi.h
@@ -117,6 +117,10 @@ struct spi_tran_header {
 	u32 addr;
 };
 
+int spi_read(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int rlen, void *rdata);
+int spi_write(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int wlen, void *wdata);
 int spi_command(struct altera_spi_device *dev, unsigned int chip_select,
 		unsigned int wlen, void *wdata, unsigned int rlen, void *rdata);
 void spi_cs_deactivate(struct altera_spi_device *dev);
diff --git a/drivers/raw/ifpga/base/opae_spi_transaction.c b/drivers/raw/ifpga/base/opae_spi_transaction.c
index 006cdb4..cd50d40 100644
--- a/drivers/raw/ifpga/base/opae_spi_transaction.c
+++ b/drivers/raw/ifpga/base/opae_spi_transaction.c
@@ -40,7 +40,7 @@ static void print_buffer(const char *string, void *buffer, int len)
 	printf("%s print buffer, len=%d\n", string, len);
 
 	for (i = 0; i < len; i++)
-		printf("%x ", *(p+i));
+		printf("%02x ", *(p+i));
 	printf("\n");
 }
 #else
@@ -72,43 +72,6 @@ static void reorder_phy_data(u8 bits_per_word,
 	}
 }
 
-enum {
-	SPI_FOUND_SOP,
-	SPI_FOUND_EOP,
-	SPI_NOT_FOUND,
-};
-
-static int resp_find_sop_eop(unsigned char *resp, unsigned int len,
-		int flags)
-{
-	int ret = SPI_NOT_FOUND;
-
-	unsigned char *b = resp;
-
-	/* find SOP */
-	if (flags != SPI_FOUND_SOP) {
-		while (b < resp + len && *b != SPI_PACKET_SOP)
-			b++;
-
-		if (*b != SPI_PACKET_SOP)
-			goto done;
-
-		ret = SPI_FOUND_SOP;
-	}
-
-	/* find EOP */
-	while (b < resp + len && *b != SPI_PACKET_EOP)
-		b++;
-
-	if (*b != SPI_PACKET_EOP)
-		goto done;
-
-	ret = SPI_FOUND_EOP;
-
-done:
-	return ret;
-}
-
 static void phy_tx_pad(unsigned char *phy_buf, unsigned int phy_buf_len,
 		unsigned int *aligned_len)
 {
@@ -137,6 +100,104 @@ static void phy_tx_pad(unsigned char *phy_buf, unsigned int phy_buf_len,
 		*p++ = SPI_BYTE_IDLE;
 }
 
+#define RX_ALL_IDLE_DATA (SPI_BYTE_IDLE << 24 | SPI_BYTE_IDLE << 16 |	\
+			 SPI_BYTE_IDLE << 8 | SPI_BYTE_IDLE)
+
+static bool all_idle_data(u8 *rxbuf)
+{
+	return *(u32 *)rxbuf == RX_ALL_IDLE_DATA;
+}
+
+static unsigned char *find_eop(u8 *rxbuf, u32 BPW)
+{
+	return memchr(rxbuf, SPI_PACKET_EOP, BPW);
+}
+
+static int do_spi_txrx(struct spi_transaction_dev *dev,
+		unsigned char *tx_buffer,
+		unsigned int tx_len, unsigned char *rx_buffer,
+		unsigned int rx_len,
+		unsigned int *actual_rx)
+{
+	unsigned int rx_cnt = 0;
+	int ret = 0;
+	unsigned int BPW = 4;
+	bool eop_found = false;
+	unsigned char *eop;
+	unsigned char *ptr;
+	unsigned char *rxbuf = rx_buffer;
+	int add_byte = 0;
+	unsigned long ticks;
+	unsigned long timeout;
+
+	/* send command */
+	ret = spi_write(dev->dev, dev->chipselect, tx_len, tx_buffer);
+	if (ret)
+		return -EBUSY;
+
+	timeout = rte_get_timer_cycles() +
+				msecs_to_timer_cycles(2000);
+
+	/* read out data */
+	while (rx_cnt < rx_len) {
+		ret = spi_read(dev->dev, dev->chipselect, BPW, rxbuf);
+		if (ret)
+			return -EBUSY;
+
+		/* skip all of invalid data */
+		if (!eop_found && all_idle_data(rxbuf)) {
+			ticks = rte_get_timer_cycles();
+			if (!time_after(ticks, timeout)) {
+				continue;
+			} else {
+				dev_err(dev, "read spi data timeout\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		rx_cnt += BPW;
+		if (!eop_found) {
+			/* EOP is found, we read 2 more bytes and exit. */
+			eop = find_eop(rxbuf, BPW);
+			if (eop) {
+				if ((BPW + rxbuf - eop) > 2) {
+					/*
+					 * check if the last 2 bytes are already
+					 * received in current word.
+					 */
+					break;
+				} else if ((BPW + rxbuf - eop) == 2) {
+					/*
+					 * skip if last byte is not SPI_BYTE_ESC
+					 * or SPI_PACKET_ESC. this is the valid
+					 * end of a response too.
+					 */
+					ptr = eop + 1;
+
+					if (*ptr != SPI_BYTE_ESC &&
+							*ptr != SPI_PACKET_ESC)
+						break;
+
+					add_byte = 1;
+				} else {
+					add_byte = 2;
+				}
+
+				rx_len = min(rx_len,
+						IFPGA_ALIGN(rx_cnt +
+							add_byte, BPW));
+				eop_found = true;
+			}
+		}
+		rxbuf += BPW;
+	}
+
+	*actual_rx = rx_cnt;
+	print_buffer("found valid data:", rx_buffer, rx_cnt);
+
+	return ret;
+}
+
 static int byte_to_core_convert(struct spi_transaction_dev *dev,
 		unsigned int send_len, unsigned char *send_data,
 		unsigned int resp_len, unsigned char *resp_data,
@@ -148,15 +209,9 @@ static int byte_to_core_convert(struct spi_transaction_dev *dev,
 	unsigned char *resp_packet = dev->buffer->bytes_resp;
 	unsigned char *p;
 	unsigned char current_byte;
-	unsigned char *tx_buffer;
 	unsigned int tx_len = 0;
-	unsigned char *rx_buffer;
-	unsigned int rx_len = 0;
-	int retry = 0;
-	int spi_flags;
-	unsigned long timeout = msecs_to_timer_cycles(1000);
-	unsigned long ticks;
 	unsigned int resp_max_len = 2 * resp_len;
+	unsigned int actual_rx;
 
 	print_buffer("before bytes:", send_data, send_len);
 
@@ -190,48 +245,15 @@ static int byte_to_core_convert(struct spi_transaction_dev *dev,
 
 	print_buffer("after order to spi:", send_packet, tx_len);
 
-	/* call spi */
-	tx_buffer = send_packet;
-	rx_buffer = resp_packet;
-	rx_len = resp_max_len;
-	spi_flags = SPI_NOT_FOUND;
-
-read_again:
-	ret = spi_command(dev->dev, dev->chipselect, tx_len, tx_buffer,
-			rx_len, rx_buffer);
+	ret = do_spi_txrx(dev, send_packet, tx_len, resp_packet,
+			resp_max_len, &actual_rx);
 	if (ret)
-		return -EBUSY;
-
-	print_buffer("read from spi:", rx_buffer, rx_len);
-
-	/* look for SOP firstly*/
-	ret = resp_find_sop_eop(rx_buffer, rx_len - 1, spi_flags);
-	if (ret != SPI_FOUND_EOP) {
-		tx_buffer = NULL;
-		tx_len = 0;
-		ticks = rte_get_timer_cycles();
-		if (time_after(ticks, timeout) &&
-				retry++ > SPI_MAX_RETRY) {
-			dev_err(NULL, "Have retry %d, found invalid packet data\n",
-				retry);
-			return -EBUSY;
-		}
-
-		if (ret == SPI_FOUND_SOP) {
-			rx_buffer += rx_len;
-			resp_max_len += rx_len;
-		}
-
-		spi_flags = ret;
-		goto read_again;
-	}
-
-	print_buffer("found valid data:", resp_packet, resp_max_len);
+		return ret;
 
 	/* analyze response packet */
 	i = 0;
 	p = resp_data;
-	while (i < resp_max_len) {
+	while (i < actual_rx) {
 		current_byte = resp_packet[i];
 		switch (current_byte) {
 		case SPI_BYTE_IDLE:
@@ -337,9 +359,13 @@ static int packet_to_byte_conver(struct spi_transaction_dev *dev,
 		current_byte = resp_packet[i];
 
 		switch (current_byte) {
-		case SPI_PACKET_ESC:
-		case SPI_PACKET_CHANNEL:
 		case SPI_PACKET_SOP:
+			dev_err(dev, "error on get SOP after SOP\n");
+			return -EINVAL;
+		case SPI_PACKET_CHANNEL:
+			i += 2;
+			break;
+		case SPI_PACKET_ESC:
 			i++;
 			current_byte = resp_packet[i];
 			*p++ = xor_20(current_byte);
@@ -348,23 +374,30 @@ static int packet_to_byte_conver(struct spi_transaction_dev *dev,
 		case SPI_PACKET_EOP:
 			i++;
 			current_byte = resp_packet[i];
-			if (current_byte == SPI_PACKET_ESC ||
-					current_byte == SPI_PACKET_CHANNEL ||
-					current_byte == SPI_PACKET_SOP) {
+			switch (current_byte) {
+			case SPI_PACKET_ESC:
 				i++;
 				current_byte = resp_packet[i];
 				*p++ = xor_20(current_byte);
-			} else
+				break;
+			case SPI_PACKET_CHANNEL:
+			case SPI_PACKET_SOP:
+			case SPI_PACKET_EOP:
+				dev_err(dev, "error get SOP/EOP after EOP\n");
+				return -EINVAL;
+			default:
 				*p++ = current_byte;
-			i = valid_resp_len;
-			break;
+				break;
+			}
+			goto done;
+
 		default:
 			*p++ = current_byte;
 			i++;
 		}
-
 	}
 
+done:
 	*valid = p - resp_buf;
 
 	print_buffer("after packet:", resp_buf, *valid);
-- 
1.8.3.1


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

* [PATCH v1 2/4] raw/ifpga: fix pthread cannot join
  2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  2022-01-18  3:17 ` [PATCH v1 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
@ 2022-01-18  3:17 ` Wei Huang
  2022-01-18  3:17 ` [PATCH v1 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  3:17 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit

From: Tianfei Zhang <tianfei.zhang@intel.com>

When we want to close a thread, we should set a flag to
thread handler function.

Fixes: 9c006c45 ("raw/ifpga: scan PCIe BDF device tree")
Cc: stable@dpdk.org

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 8d9db58..9663b67 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -497,7 +497,7 @@ static int set_surprise_link_check_aer(
 	int gsd_enable, ret;
 #define MS 1000
 
-	while (1) {
+	while (ifpga_monitor_start) {
 		gsd_enable = 0;
 		for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 			ifpga_rdev = &ifpga_rawdevices[i];
@@ -544,7 +544,9 @@ static int set_surprise_link_check_aer(
 {
 	int ret;
 
-	if (ifpga_monitor_start == 1) {
+	if (ifpga_monitor_start == 1 && ifpga_monitor_start_thread) {
+		ifpga_monitor_start = 0;
+
 		ret = pthread_cancel(ifpga_monitor_start_thread);
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't cancel the thread");
@@ -553,8 +555,6 @@ static int set_surprise_link_check_aer(
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't join the thread");
 
-		ifpga_monitor_start = 0;
-
 		return ret;
 	}
 
-- 
1.8.3.1


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

* [PATCH v1 3/4] raw/ifpga: fix ifpga devices cleanup function
  2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  2022-01-18  3:17 ` [PATCH v1 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
  2022-01-18  3:17 ` [PATCH v1 2/4] raw/ifpga: fix pthread cannot join Wei Huang
@ 2022-01-18  3:17 ` Wei Huang
  2022-01-18  3:17 ` [PATCH v1 4/4] raw/ifpga/base: fix port feature ID Wei Huang
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  4 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  3:17 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release()
in ifpga_rawdev_cleanup(), resources occupied by ifpga
raw devices such as threads can be released correctly.

Fixes: f724a802 ("raw/ifpga: add miscellaneous APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 9663b67..8d47fd5 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct rte_rawdev *dev, int port,
 void ifpga_rawdev_cleanup(void)
 {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;
 
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
-- 
1.8.3.1


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

* [PATCH v1 4/4] raw/ifpga/base: fix port feature ID
  2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
                   ` (2 preceding siblings ...)
  2022-01-18  3:17 ` [PATCH v1 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
@ 2022-01-18  3:17 ` Wei Huang
  2022-01-18  6:39   ` Xu, Rosen
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  4 siblings, 1 reply; 11+ messages in thread
From: Wei Huang @ 2022-01-18  3:17 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

Fix ID value of port features to match the definition from hardware.

Fixes: 473c88f9 ("drivers/raw: remove rawdev from directory names")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_defines.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ifpga/base/ifpga_defines.h b/drivers/raw/ifpga/base/ifpga_defines.h
index dca1518..8f62033 100644
--- a/drivers/raw/ifpga/base/ifpga_defines.h
+++ b/drivers/raw/ifpga/base/ifpga_defines.h
@@ -93,9 +93,9 @@ enum fpga_id_type {
 
 #define PORT_FEATURE_ID_HEADER FEATURE_ID_FIU_HEADER
 #define PORT_FEATURE_ID_ERROR 0x10
-#define PORT_FEATURE_ID_UMSG 0x12
-#define PORT_FEATURE_ID_UINT 0x13
-#define PORT_FEATURE_ID_STP 0x14
+#define PORT_FEATURE_ID_UMSG 0x11
+#define PORT_FEATURE_ID_UINT 0x12
+#define PORT_FEATURE_ID_STP 0x13
 #define PORT_FEATURE_ID_UAFU FEATURE_ID_AFU
 
 /*
-- 
1.8.3.1


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

* RE: [PATCH v1 4/4] raw/ifpga/base: fix port feature ID
  2022-01-18  3:17 ` [PATCH v1 4/4] raw/ifpga/base: fix port feature ID Wei Huang
@ 2022-01-18  6:39   ` Xu, Rosen
  0 siblings, 0 replies; 11+ messages in thread
From: Xu, Rosen @ 2022-01-18  6:39 UTC (permalink / raw)
  To: Huang, Wei, dev, Zhang,  Qi Z; +Cc: stable, Zhang, Tianfei, Yigit, Ferruh

Hi Wei,

Some issues of ci/iol-x86_64-unit-testing, pls check it. Thanks a lot.

Thanks,
Rosen

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Tuesday, January 18, 2022 11:18 AM
To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Huang, Wei <wei.huang@intel.com>
Subject: [PATCH v1 4/4] raw/ifpga/base: fix port feature ID

Fix ID value of port features to match the definition from hardware.

Fixes: 473c88f9 ("drivers/raw: remove rawdev from directory names")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_defines.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ifpga/base/ifpga_defines.h b/drivers/raw/ifpga/base/ifpga_defines.h
index dca1518..8f62033 100644
--- a/drivers/raw/ifpga/base/ifpga_defines.h
+++ b/drivers/raw/ifpga/base/ifpga_defines.h
@@ -93,9 +93,9 @@ enum fpga_id_type {
 
 #define PORT_FEATURE_ID_HEADER FEATURE_ID_FIU_HEADER  #define PORT_FEATURE_ID_ERROR 0x10 -#define PORT_FEATURE_ID_UMSG 0x12 -#define PORT_FEATURE_ID_UINT 0x13 -#define PORT_FEATURE_ID_STP 0x14
+#define PORT_FEATURE_ID_UMSG 0x11
+#define PORT_FEATURE_ID_UINT 0x12
+#define PORT_FEATURE_ID_STP 0x13
 #define PORT_FEATURE_ID_UAFU FEATURE_ID_AFU
 
 /*
--
1.8.3.1


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

* [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek
  2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
                   ` (3 preceding siblings ...)
  2022-01-18  3:17 ` [PATCH v1 4/4] raw/ifpga/base: fix port feature ID Wei Huang
@ 2022-01-18  7:37 ` Wei Huang
  2022-01-18  7:37   ` [PATCH v2 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
                     ` (3 more replies)
  4 siblings, 4 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  7:37 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

This patchset is bug fix for raw/ifpga driver.
Changes since v1:
* Refine commit log.

Tianfei Zhang (2):
  raw/ifpga/base: fix SPI transaction
  raw/ifpga: fix pthread cannot join

Wei Huang (2):
  raw/ifpga: fix ifpga devices cleanup function
  raw/ifpga/base: fix port feature ID

 drivers/raw/ifpga/base/ifpga_defines.h        |   6 +-
 drivers/raw/ifpga/base/opae_spi.c             |  12 ++
 drivers/raw/ifpga/base/opae_spi.h             |   4 +
 drivers/raw/ifpga/base/opae_spi_transaction.c | 215 +++++++++++++++-----------
 drivers/raw/ifpga/ifpga_rawdev.c              |  12 +-
 5 files changed, 150 insertions(+), 99 deletions(-)

-- 
1.8.3.1


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

* [PATCH v2 1/4] raw/ifpga/base: fix SPI transaction
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
@ 2022-01-18  7:37   ` Wei Huang
  2022-01-18  7:37   ` [PATCH v2 2/4] raw/ifpga: fix pthread cannot join Wei Huang
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  7:37 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit

From: Tianfei Zhang <tianfei.zhang@intel.com>

When EOP is detected, 2 more bytes should be received
(may be a SPI_PACKET_ESC before last valid byte) then
rx should be finished.

Fixes: 96ebfcf8 ("raw/ifpga/base: add SPI and MAX10 device driver")
Cc: stable@dpdk.org

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/base/opae_spi.c             |  12 ++
 drivers/raw/ifpga/base/opae_spi.h             |   4 +
 drivers/raw/ifpga/base/opae_spi_transaction.c | 215 +++++++++++++++-----------
 3 files changed, 140 insertions(+), 91 deletions(-)

diff --git a/drivers/raw/ifpga/base/opae_spi.c b/drivers/raw/ifpga/base/opae_spi.c
index 9efeecb..ca3d41f 100644
--- a/drivers/raw/ifpga/base/opae_spi.c
+++ b/drivers/raw/ifpga/base/opae_spi.c
@@ -239,6 +239,18 @@ int spi_command(struct altera_spi_device *dev, unsigned int chip_select,
 	return 0;
 }
 
+int spi_write(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int wlen, void *wdata)
+{
+	return spi_command(dev, chip_select, wlen, wdata, 0, NULL);
+}
+
+int spi_read(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int rlen, void *rdata)
+{
+	return spi_command(dev, chip_select, 0, NULL, rlen, rdata);
+}
+
 struct altera_spi_device *altera_spi_alloc(void *base, int type)
 {
 	struct altera_spi_device *spi_dev =
diff --git a/drivers/raw/ifpga/base/opae_spi.h b/drivers/raw/ifpga/base/opae_spi.h
index af11656..bcff67d 100644
--- a/drivers/raw/ifpga/base/opae_spi.h
+++ b/drivers/raw/ifpga/base/opae_spi.h
@@ -117,6 +117,10 @@ struct spi_tran_header {
 	u32 addr;
 };
 
+int spi_read(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int rlen, void *rdata);
+int spi_write(struct altera_spi_device *dev, unsigned int chip_select,
+		unsigned int wlen, void *wdata);
 int spi_command(struct altera_spi_device *dev, unsigned int chip_select,
 		unsigned int wlen, void *wdata, unsigned int rlen, void *rdata);
 void spi_cs_deactivate(struct altera_spi_device *dev);
diff --git a/drivers/raw/ifpga/base/opae_spi_transaction.c b/drivers/raw/ifpga/base/opae_spi_transaction.c
index 006cdb4..cd50d40 100644
--- a/drivers/raw/ifpga/base/opae_spi_transaction.c
+++ b/drivers/raw/ifpga/base/opae_spi_transaction.c
@@ -40,7 +40,7 @@ static void print_buffer(const char *string, void *buffer, int len)
 	printf("%s print buffer, len=%d\n", string, len);
 
 	for (i = 0; i < len; i++)
-		printf("%x ", *(p+i));
+		printf("%02x ", *(p+i));
 	printf("\n");
 }
 #else
@@ -72,43 +72,6 @@ static void reorder_phy_data(u8 bits_per_word,
 	}
 }
 
-enum {
-	SPI_FOUND_SOP,
-	SPI_FOUND_EOP,
-	SPI_NOT_FOUND,
-};
-
-static int resp_find_sop_eop(unsigned char *resp, unsigned int len,
-		int flags)
-{
-	int ret = SPI_NOT_FOUND;
-
-	unsigned char *b = resp;
-
-	/* find SOP */
-	if (flags != SPI_FOUND_SOP) {
-		while (b < resp + len && *b != SPI_PACKET_SOP)
-			b++;
-
-		if (*b != SPI_PACKET_SOP)
-			goto done;
-
-		ret = SPI_FOUND_SOP;
-	}
-
-	/* find EOP */
-	while (b < resp + len && *b != SPI_PACKET_EOP)
-		b++;
-
-	if (*b != SPI_PACKET_EOP)
-		goto done;
-
-	ret = SPI_FOUND_EOP;
-
-done:
-	return ret;
-}
-
 static void phy_tx_pad(unsigned char *phy_buf, unsigned int phy_buf_len,
 		unsigned int *aligned_len)
 {
@@ -137,6 +100,104 @@ static void phy_tx_pad(unsigned char *phy_buf, unsigned int phy_buf_len,
 		*p++ = SPI_BYTE_IDLE;
 }
 
+#define RX_ALL_IDLE_DATA (SPI_BYTE_IDLE << 24 | SPI_BYTE_IDLE << 16 |	\
+			 SPI_BYTE_IDLE << 8 | SPI_BYTE_IDLE)
+
+static bool all_idle_data(u8 *rxbuf)
+{
+	return *(u32 *)rxbuf == RX_ALL_IDLE_DATA;
+}
+
+static unsigned char *find_eop(u8 *rxbuf, u32 BPW)
+{
+	return memchr(rxbuf, SPI_PACKET_EOP, BPW);
+}
+
+static int do_spi_txrx(struct spi_transaction_dev *dev,
+		unsigned char *tx_buffer,
+		unsigned int tx_len, unsigned char *rx_buffer,
+		unsigned int rx_len,
+		unsigned int *actual_rx)
+{
+	unsigned int rx_cnt = 0;
+	int ret = 0;
+	unsigned int BPW = 4;
+	bool eop_found = false;
+	unsigned char *eop;
+	unsigned char *ptr;
+	unsigned char *rxbuf = rx_buffer;
+	int add_byte = 0;
+	unsigned long ticks;
+	unsigned long timeout;
+
+	/* send command */
+	ret = spi_write(dev->dev, dev->chipselect, tx_len, tx_buffer);
+	if (ret)
+		return -EBUSY;
+
+	timeout = rte_get_timer_cycles() +
+				msecs_to_timer_cycles(2000);
+
+	/* read out data */
+	while (rx_cnt < rx_len) {
+		ret = spi_read(dev->dev, dev->chipselect, BPW, rxbuf);
+		if (ret)
+			return -EBUSY;
+
+		/* skip all of invalid data */
+		if (!eop_found && all_idle_data(rxbuf)) {
+			ticks = rte_get_timer_cycles();
+			if (!time_after(ticks, timeout)) {
+				continue;
+			} else {
+				dev_err(dev, "read spi data timeout\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		rx_cnt += BPW;
+		if (!eop_found) {
+			/* EOP is found, we read 2 more bytes and exit. */
+			eop = find_eop(rxbuf, BPW);
+			if (eop) {
+				if ((BPW + rxbuf - eop) > 2) {
+					/*
+					 * check if the last 2 bytes are already
+					 * received in current word.
+					 */
+					break;
+				} else if ((BPW + rxbuf - eop) == 2) {
+					/*
+					 * skip if last byte is not SPI_BYTE_ESC
+					 * or SPI_PACKET_ESC. this is the valid
+					 * end of a response too.
+					 */
+					ptr = eop + 1;
+
+					if (*ptr != SPI_BYTE_ESC &&
+							*ptr != SPI_PACKET_ESC)
+						break;
+
+					add_byte = 1;
+				} else {
+					add_byte = 2;
+				}
+
+				rx_len = min(rx_len,
+						IFPGA_ALIGN(rx_cnt +
+							add_byte, BPW));
+				eop_found = true;
+			}
+		}
+		rxbuf += BPW;
+	}
+
+	*actual_rx = rx_cnt;
+	print_buffer("found valid data:", rx_buffer, rx_cnt);
+
+	return ret;
+}
+
 static int byte_to_core_convert(struct spi_transaction_dev *dev,
 		unsigned int send_len, unsigned char *send_data,
 		unsigned int resp_len, unsigned char *resp_data,
@@ -148,15 +209,9 @@ static int byte_to_core_convert(struct spi_transaction_dev *dev,
 	unsigned char *resp_packet = dev->buffer->bytes_resp;
 	unsigned char *p;
 	unsigned char current_byte;
-	unsigned char *tx_buffer;
 	unsigned int tx_len = 0;
-	unsigned char *rx_buffer;
-	unsigned int rx_len = 0;
-	int retry = 0;
-	int spi_flags;
-	unsigned long timeout = msecs_to_timer_cycles(1000);
-	unsigned long ticks;
 	unsigned int resp_max_len = 2 * resp_len;
+	unsigned int actual_rx;
 
 	print_buffer("before bytes:", send_data, send_len);
 
@@ -190,48 +245,15 @@ static int byte_to_core_convert(struct spi_transaction_dev *dev,
 
 	print_buffer("after order to spi:", send_packet, tx_len);
 
-	/* call spi */
-	tx_buffer = send_packet;
-	rx_buffer = resp_packet;
-	rx_len = resp_max_len;
-	spi_flags = SPI_NOT_FOUND;
-
-read_again:
-	ret = spi_command(dev->dev, dev->chipselect, tx_len, tx_buffer,
-			rx_len, rx_buffer);
+	ret = do_spi_txrx(dev, send_packet, tx_len, resp_packet,
+			resp_max_len, &actual_rx);
 	if (ret)
-		return -EBUSY;
-
-	print_buffer("read from spi:", rx_buffer, rx_len);
-
-	/* look for SOP firstly*/
-	ret = resp_find_sop_eop(rx_buffer, rx_len - 1, spi_flags);
-	if (ret != SPI_FOUND_EOP) {
-		tx_buffer = NULL;
-		tx_len = 0;
-		ticks = rte_get_timer_cycles();
-		if (time_after(ticks, timeout) &&
-				retry++ > SPI_MAX_RETRY) {
-			dev_err(NULL, "Have retry %d, found invalid packet data\n",
-				retry);
-			return -EBUSY;
-		}
-
-		if (ret == SPI_FOUND_SOP) {
-			rx_buffer += rx_len;
-			resp_max_len += rx_len;
-		}
-
-		spi_flags = ret;
-		goto read_again;
-	}
-
-	print_buffer("found valid data:", resp_packet, resp_max_len);
+		return ret;
 
 	/* analyze response packet */
 	i = 0;
 	p = resp_data;
-	while (i < resp_max_len) {
+	while (i < actual_rx) {
 		current_byte = resp_packet[i];
 		switch (current_byte) {
 		case SPI_BYTE_IDLE:
@@ -337,9 +359,13 @@ static int packet_to_byte_conver(struct spi_transaction_dev *dev,
 		current_byte = resp_packet[i];
 
 		switch (current_byte) {
-		case SPI_PACKET_ESC:
-		case SPI_PACKET_CHANNEL:
 		case SPI_PACKET_SOP:
+			dev_err(dev, "error on get SOP after SOP\n");
+			return -EINVAL;
+		case SPI_PACKET_CHANNEL:
+			i += 2;
+			break;
+		case SPI_PACKET_ESC:
 			i++;
 			current_byte = resp_packet[i];
 			*p++ = xor_20(current_byte);
@@ -348,23 +374,30 @@ static int packet_to_byte_conver(struct spi_transaction_dev *dev,
 		case SPI_PACKET_EOP:
 			i++;
 			current_byte = resp_packet[i];
-			if (current_byte == SPI_PACKET_ESC ||
-					current_byte == SPI_PACKET_CHANNEL ||
-					current_byte == SPI_PACKET_SOP) {
+			switch (current_byte) {
+			case SPI_PACKET_ESC:
 				i++;
 				current_byte = resp_packet[i];
 				*p++ = xor_20(current_byte);
-			} else
+				break;
+			case SPI_PACKET_CHANNEL:
+			case SPI_PACKET_SOP:
+			case SPI_PACKET_EOP:
+				dev_err(dev, "error get SOP/EOP after EOP\n");
+				return -EINVAL;
+			default:
 				*p++ = current_byte;
-			i = valid_resp_len;
-			break;
+				break;
+			}
+			goto done;
+
 		default:
 			*p++ = current_byte;
 			i++;
 		}
-
 	}
 
+done:
 	*valid = p - resp_buf;
 
 	print_buffer("after packet:", resp_buf, *valid);
-- 
1.8.3.1


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

* [PATCH v2 2/4] raw/ifpga: fix pthread cannot join
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  2022-01-18  7:37   ` [PATCH v2 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
@ 2022-01-18  7:37   ` Wei Huang
  2022-01-18  7:37   ` [PATCH v2 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
  2022-01-18  7:37   ` [PATCH v2 4/4] raw/ifpga/base: fix port feature ID Wei Huang
  3 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  7:37 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit

From: Tianfei Zhang <tianfei.zhang@intel.com>

When we want to close a thread, we should set a flag to
thread handler function.

Fixes: 9c006c45 ("raw/ifpga: scan PCIe BDF device tree")
Cc: stable@dpdk.org

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 8d9db58..9663b67 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -497,7 +497,7 @@ static int set_surprise_link_check_aer(
 	int gsd_enable, ret;
 #define MS 1000
 
-	while (1) {
+	while (ifpga_monitor_start) {
 		gsd_enable = 0;
 		for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 			ifpga_rdev = &ifpga_rawdevices[i];
@@ -544,7 +544,9 @@ static int set_surprise_link_check_aer(
 {
 	int ret;
 
-	if (ifpga_monitor_start == 1) {
+	if (ifpga_monitor_start == 1 && ifpga_monitor_start_thread) {
+		ifpga_monitor_start = 0;
+
 		ret = pthread_cancel(ifpga_monitor_start_thread);
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't cancel the thread");
@@ -553,8 +555,6 @@ static int set_surprise_link_check_aer(
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't join the thread");
 
-		ifpga_monitor_start = 0;
-
 		return ret;
 	}
 
-- 
1.8.3.1


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

* [PATCH v2 3/4] raw/ifpga: fix ifpga devices cleanup function
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
  2022-01-18  7:37   ` [PATCH v2 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
  2022-01-18  7:37   ` [PATCH v2 2/4] raw/ifpga: fix pthread cannot join Wei Huang
@ 2022-01-18  7:37   ` Wei Huang
  2022-01-18  7:37   ` [PATCH v2 4/4] raw/ifpga/base: fix port feature ID Wei Huang
  3 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  7:37 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release()
in ifpga_rawdev_cleanup(), resources occupied by ifpga
raw devices such as threads can be released correctly.

Fixes: f724a802 ("raw/ifpga: add miscellaneous APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 9663b67..8d47fd5 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct rte_rawdev *dev, int port,
 void ifpga_rawdev_cleanup(void)
 {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;
 
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
-- 
1.8.3.1


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

* [PATCH v2 4/4] raw/ifpga/base: fix port feature ID
  2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
                     ` (2 preceding siblings ...)
  2022-01-18  7:37   ` [PATCH v2 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
@ 2022-01-18  7:37   ` Wei Huang
  3 siblings, 0 replies; 11+ messages in thread
From: Wei Huang @ 2022-01-18  7:37 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, ferruh.yigit, Wei Huang

Fix ID value of port features to match the definition from hardware.

Fixes: 473c88f9 ("drivers/raw: remove rawdev from directory names")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_defines.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ifpga/base/ifpga_defines.h b/drivers/raw/ifpga/base/ifpga_defines.h
index dca1518..8f62033 100644
--- a/drivers/raw/ifpga/base/ifpga_defines.h
+++ b/drivers/raw/ifpga/base/ifpga_defines.h
@@ -93,9 +93,9 @@ enum fpga_id_type {
 
 #define PORT_FEATURE_ID_HEADER FEATURE_ID_FIU_HEADER
 #define PORT_FEATURE_ID_ERROR 0x10
-#define PORT_FEATURE_ID_UMSG 0x12
-#define PORT_FEATURE_ID_UINT 0x13
-#define PORT_FEATURE_ID_STP 0x14
+#define PORT_FEATURE_ID_UMSG 0x11
+#define PORT_FEATURE_ID_UINT 0x12
+#define PORT_FEATURE_ID_STP 0x13
 #define PORT_FEATURE_ID_UAFU FEATURE_ID_AFU
 
 /*
-- 
1.8.3.1


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

end of thread, other threads:[~2022-01-18  7:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18  3:17 [PATCH v1 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
2022-01-18  3:17 ` [PATCH v1 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
2022-01-18  3:17 ` [PATCH v1 2/4] raw/ifpga: fix pthread cannot join Wei Huang
2022-01-18  3:17 ` [PATCH v1 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
2022-01-18  3:17 ` [PATCH v1 4/4] raw/ifpga/base: fix port feature ID Wei Huang
2022-01-18  6:39   ` Xu, Rosen
2022-01-18  7:37 ` [PATCH v2 0/4] Fix bugs found in raw/ifpga development for arrow creek Wei Huang
2022-01-18  7:37   ` [PATCH v2 1/4] raw/ifpga/base: fix SPI transaction Wei Huang
2022-01-18  7:37   ` [PATCH v2 2/4] raw/ifpga: fix pthread cannot join Wei Huang
2022-01-18  7:37   ` [PATCH v2 3/4] raw/ifpga: fix ifpga devices cleanup function Wei Huang
2022-01-18  7:37   ` [PATCH v2 4/4] raw/ifpga/base: fix port feature ID Wei Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).