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@xxxxxxxxxxxxxx> --- 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