All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Zink <j.zink@pengutronix.de>
To: linux-fpga@vger.kernel.org
Cc: devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Moritz Fischer <mdf@kernel.org>, Wu Hao <hao.wu@intel.com>,
	Xu Yilun <yilun.xu@intel.com>,
	kernel@pengutronix.de, Johannes Zink <j.zink@pengutronix.de>
Subject: [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility
Date: Thu, 25 Aug 2022 16:13:34 +0200	[thread overview]
Message-ID: <20220825141343.1375690-8-j.zink@pengutronix.de> (raw)
In-Reply-To: <20220825141343.1375690-1-j.zink@pengutronix.de>

The SPI message is written into the lowest-addressed bits of an
unsigned long variable, but be32_to_cpu is called on the least
significant bits of the variable. On big-endian 64-bit systems,
this would give a wrong result. Fix this by using the fixed-size
u32 instead of unsigned long. This clashes with the use of
test_bit, which is unnecessary for a single u32 variable, so
we adjust all usage sites appropriately and prefix the macros
with MACHXO2_ while at it.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 drivers/fpga/machxo2-spi.c | 110 ++++++++++++++++++-------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 5e12612c7289..d1a8f28e04e7 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/bitfield.h>
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -41,41 +42,40 @@
 #define MACHXO2_BUF_SIZE		(MACHXO2_PAGE_SIZE + 4)
 
 /* Status register bits, errors and error mask */
-#define BUSY	12
-#define DONE	8
-#define DVER	27
-#define ENAB	9
-#define ERRBITS	23
-#define ERRMASK	7
-#define FAIL	13
-
-#define ENOERR	0 /* no error */
-#define EID	1
-#define ECMD	2
-#define ECRC	3
-#define EPREAM	4 /* preamble error */
-#define EABRT	5 /* abort error */
-#define EOVERFL	6 /* overflow error */
-#define ESDMEOF	7 /* SDM EOF */
-
-static inline u8 get_err(unsigned long *status)
+#define MACHXO2_BUSY		BIT(12)
+#define MACHXO2_DONE		BIT(8)
+#define MACHXO2_DVER		BIT(27)
+#define MACHXO2_ENAB		BIT(9)
+#define MACHXO2_ERR		GENMASK(25, 23)
+#define MACHXO2_ERR_ENOERR	0 /* no error */
+#define MACHXO2_ERR_EID		1
+#define MACHXO2_ERR_ECMD	2
+#define MACHXO2_ERR_ECRC	3
+#define MACHXO2_ERR_EPREAM	4 /* preamble error */
+#define MACHXO2_ERR_EABRT	5 /* abort error */
+#define MACHXO2_ERR_EOVERFL	6 /* overflow error */
+#define MACHXO2_ERR_ESDMEOF	7 /* SDM EOF */
+#define MACHXO2_FAIL		BIT(13)
+
+static inline u8 get_err(u32 status)
 {
-	return (*status >> ERRBITS) & ERRMASK;
+	return FIELD_GET(MACHXO2_ERR, status);
 }
 
-static int get_status(struct spi_device *spi, unsigned long *status)
+static int get_status(struct spi_device *spi, u32 *status)
 {
 	struct spi_message msg;
 	struct spi_transfer rx, tx;
 	static const u8 cmd[] = LSC_READ_STATUS;
+	__be32 tmp;
 	int ret;
 
 	memset(&rx, 0, sizeof(rx));
 	memset(&tx, 0, sizeof(tx));
 	tx.tx_buf = cmd;
 	tx.len = sizeof(cmd);
-	rx.rx_buf = status;
-	rx.len = 4;
+	rx.rx_buf = &tmp;
+	rx.len = sizeof(tmp);
 	spi_message_init(&msg);
 	spi_message_add_tail(&tx, &msg);
 	spi_message_add_tail(&rx, &msg);
@@ -83,7 +83,7 @@ static int get_status(struct spi_device *spi, unsigned long *status)
 	if (ret)
 		return ret;
 
-	*status = be32_to_cpu(*status);
+	*status = be32_to_cpu(tmp);
 
 	return 0;
 }
@@ -91,30 +91,30 @@ static int get_status(struct spi_device *spi, unsigned long *status)
 static const char *get_err_string(u8 err)
 {
 	switch (err) {
-	case ENOERR:	return "No Error";
-	case EID:	return "ID ERR";
-	case ECMD:	return "CMD ERR";
-	case ECRC:	return "CRC ERR";
-	case EPREAM:	return "Preamble ERR";
-	case EABRT:	return "Abort ERR";
-	case EOVERFL:	return "Overflow ERR";
-	case ESDMEOF:	return "SDM EOF";
+	case MACHXO2_ERR_ENOERR:	return "No Error";
+	case MACHXO2_ERR_EID:		return "ID ERR";
+	case MACHXO2_ERR_ECMD:		return "CMD ERR";
+	case MACHXO2_ERR_ECRC:		return "CRC ERR";
+	case MACHXO2_ERR_EPREAM:	return "Preamble ERR";
+	case MACHXO2_ERR_EABRT:		return "Abort ERR";
+	case MACHXO2_ERR_EOVERFL:	return "Overflow ERR";
+	case MACHXO2_ERR_ESDMEOF:	return "SDM EOF";
 	}
 
-	return "Default switch case";
+	return "Unknown";
 }
 
-static void dump_status_reg(unsigned long *status)
+static void dump_status_reg(u32 status)
 {
-	pr_debug("machxo2 status: 0x%08lX - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
-		 *status, test_bit(DONE, status), test_bit(ENAB, status),
-		 test_bit(BUSY, status), test_bit(FAIL, status),
-		 test_bit(DVER, status), get_err_string(get_err(status)));
+	pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
+		  status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
+		  !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
+		  !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
 }
 
 static int wait_until_not_busy(struct spi_device *spi)
 {
-	unsigned long status;
+	u32 status;
 	int ret, loop = 0;
 
 	do {
@@ -123,7 +123,7 @@ static int wait_until_not_busy(struct spi_device *spi)
 			return ret;
 		if (++loop >= MACHXO2_MAX_BUSY_LOOP)
 			return -EBUSY;
-	} while (test_bit(BUSY, &status));
+	} while (status & MACHXO2_BUSY);
 
 	return 0;
 }
@@ -169,14 +169,14 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
 
 static bool machxo2_status_done(unsigned long status)
 {
-	return !test_bit(BUSY, &status) && test_bit(DONE, &status) &&
-		get_err(&status) == ENOERR;
+	return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) == MACHXO2_DONE) &&
+		get_err(status) == MACHXO2_ERR_ENOERR);
 }
 
 static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
 {
 	struct spi_device *spi = mgr->priv;
-	unsigned long status;
+	u32 status;
 
 	get_status(spi, &status);
 	if (machxo2_status_done(status))
@@ -195,7 +195,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
 	static const u8 enable[] = ISC_ENABLE;
 	static const u8 erase[] = ISC_ERASE;
 	static const u8 initaddr[] = LSC_INITADDRESS;
-	unsigned long status;
+	u32 status;
 	int ret;
 
 	if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
@@ -205,7 +205,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
 	}
 
 	get_status(spi, &status);
-	dump_status_reg(&status);
+	dump_status_reg(status);
 	memset(tx, 0, sizeof(tx));
 	spi_message_init(&msg);
 	tx[0].tx_buf = &enable;
@@ -226,11 +226,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
 		goto fail;
 
 	get_status(spi, &status);
-	if (test_bit(FAIL, &status)) {
+	if (status & MACHXO2_FAIL) {
 		ret = -EINVAL;
 		goto fail;
 	}
-	dump_status_reg(&status);
+	dump_status_reg(status);
 
 	spi_message_init(&msg);
 	tx[2].tx_buf = &initaddr;
@@ -241,7 +241,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
 		goto fail;
 
 	get_status(spi, &status);
-	dump_status_reg(&status);
+	dump_status_reg(status);
 
 	return 0;
 fail:
@@ -258,7 +258,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
 	struct spi_transfer tx;
 	static const u8 progincr[] = LSC_PROGINCRNV;
 	u8 payload[MACHXO2_BUF_SIZE];
-	unsigned long status;
+	u32 status;
 	int i, ret;
 
 	if (count % MACHXO2_PAGE_SIZE != 0) {
@@ -266,7 +266,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
 		return -EINVAL;
 	}
 	get_status(spi, &status);
-	dump_status_reg(&status);
+	dump_status_reg(status);
 	memcpy(payload, &progincr, sizeof(progincr));
 	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
 		memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
@@ -284,7 +284,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
 		}
 	}
 	get_status(spi, &status);
-	dump_status_reg(&status);
+	dump_status_reg(status);
 
 	return 0;
 }
@@ -297,7 +297,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
 	struct spi_transfer tx[2];
 	static const u8 progdone[] = ISC_PROGRAMDONE;
 	static const u8 refresh[] = LSC_REFRESH;
-	unsigned long status;
+	u32 status;
 	int ret, refreshloop = 0;
 
 	memset(tx, 0, sizeof(tx));
@@ -313,8 +313,8 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
 		goto fail;
 
 	get_status(spi, &status);
-	dump_status_reg(&status);
-	if (!test_bit(DONE, &status)) {
+	dump_status_reg(status);
+	if (!(status & MACHXO2_DONE)) {
 		machxo2_cleanup(mgr);
 		ret = -EINVAL;
 		goto fail;
@@ -333,7 +333,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
 
 		/* check refresh status */
 		get_status(spi, &status);
-		dump_status_reg(&status);
+		dump_status_reg(status);
 		if (machxo2_status_done(status))
 			break;
 		if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
@@ -344,7 +344,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
 	} while (1);
 
 	get_status(spi, &status);
-	dump_status_reg(&status);
+	dump_status_reg(status);
 
 	return 0;
 fail:
-- 
2.30.2


  parent reply	other threads:[~2022-08-25 14:16 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
2022-08-25 14:13 ` [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML Johannes Zink
2022-08-30 20:30   ` Rob Herring
2022-08-31  7:12     ` Johannes Zink
2022-08-25 14:13 ` [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties Johannes Zink
2022-08-29  7:39   ` Xu Yilun
     [not found]     ` <9d5512768acb4d57f339942007402a9ed9483e84.camel@pengutronix.de>
     [not found]       ` <YwzWt8KjyfdyqehI@yilunxu-OptiPlex-7050>
2022-08-31  7:38         ` Johannes Zink
2022-09-03 14:49           ` Xu Yilun
2022-08-30 20:36   ` Rob Herring
2022-08-31  7:07     ` Johannes Zink
2022-08-25 14:13 ` [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init Johannes Zink
2022-08-25 18:51   ` Rob Herring
2022-08-26  7:56     ` Johannes Zink
2022-08-29  7:45   ` Xu Yilun
     [not found]     ` <a42d72cd71c96ca675f5bb0cf59128c7f1cb04bb.camel@pengutronix.de>
     [not found]       ` <YwzZYM6GU0GiqBiq@yilunxu-OptiPlex-7050>
2022-08-31  7:51         ` Johannes Zink
2022-08-31  8:08           ` Johannes Zink
2022-08-25 14:13 ` [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible Johannes Zink
2022-08-30 20:40   ` Rob Herring
2022-08-31  7:10     ` Johannes Zink
2022-08-25 14:13 ` [PATCH 05/16] fpga: machxo2-spi: remove #ifdef DEBUG Johannes Zink
2022-08-25 14:13 ` [PATCH 06/16] fpga: machxo2-spi: factor out status check for readability Johannes Zink
2022-08-25 14:13 ` Johannes Zink [this message]
2022-08-29  8:19   ` [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility Xu Yilun
2022-08-29 10:41     ` Johannes Zink
2022-08-25 14:13 ` [PATCH 08/16] fpga: machxo2-spi: simplify with spi_sync_transfer() Johannes Zink
2022-08-25 14:13 ` [PATCH 09/16] fpga: machxo2-spi: simplify spi write commands Johannes Zink
2022-08-25 14:13 ` [PATCH 10/16] fpga: machxo2-spi: prepare extraction of common code Johannes Zink
2022-08-25 14:13 ` [PATCH 11/16] fpga: machxo2: move non-spi-related functionality to " Johannes Zink
2022-08-25 14:13 ` [PATCH 12/16] fpga: machxo2: improve status register dump Johannes Zink
2022-08-25 14:13 ` [PATCH 13/16] fpga: machxo2: add optional additional flash areas to be erased Johannes Zink
2022-08-25 14:13 ` [PATCH 14/16] fpga: machxo2: add program initialization signalling via gpio Johannes Zink
2022-08-25 14:13 ` [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA Johannes Zink
2022-08-29  9:26   ` Xu Yilun
2022-08-29 10:51     ` Johannes Zink
2022-08-29 14:57       ` Xu Yilun
2022-08-31  7:56         ` Johannes Zink
2022-08-25 14:13 ` [PATCH 16/16] fpga: machxo2: add configuration over i2c Johannes Zink
2022-08-29  9:47   ` Xu Yilun
2022-08-29 13:21     ` Johannes Zink
2022-08-29 14:45       ` Xu Yilun
2022-08-31 16:07         ` Johannes Zink
2022-08-25 15:25 ` [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Ivan Bornyakov
2022-08-26  6:32   ` Johannes Zink
2022-08-26  8:15     ` Ivan Bornyakov
2022-08-26  8:25   ` Sascha Hauer
2022-08-26  9:00     ` Ivan Bornyakov
2022-08-26  9:19       ` Ivan Bornyakov
2022-08-26 15:26         ` Xu Yilun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220825141343.1375690-8-j.zink@pengutronix.de \
    --to=j.zink@pengutronix.de \
    --cc=devicetree@vger.kernel.org \
    --cc=hao.wu@intel.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-fpga@vger.kernel.org \
    --cc=mdf@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=yilun.xu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.