[U-Boot] [PATCH 1/3] spi: Add spi_write_then_read

Add support for SPI synchronous write followed by read, this is common interface call from spi-nor to spi drivers.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/spi/spi-uclass.c | 24 ++++++++++++++++++++++++ include/spi.h | 20 ++++++++++++++++++++ 2 files changed, 44 insertions(+)
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 88cb2a1262..76c4b53c16 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -108,6 +108,30 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, return dm_spi_xfer(slave->dev, bitlen, dout, din, flags); }
+int spi_write_then_read(struct spi_slave *slave, const u8 *opcode, + size_t n_opcode, const u8 *txbuf, u8 *rxbuf, + size_t n_buf) +{ + unsigned long flags = SPI_XFER_BEGIN; + int ret; + + if (n_buf == 0) + flags |= SPI_XFER_END; + + ret = spi_xfer(slave, n_opcode * 8, opcode, NULL, flags); + if (ret) { + debug("spi: failed to send command (%zu bytes): %d\n", + n_opcode, ret); + } else if (n_buf != 0) { + ret = spi_xfer(slave, n_buf * 8, txbuf, rxbuf, SPI_XFER_END); + if (ret) + debug("spi: failed to transfer %zu bytes of data: %d\n", + n_buf, ret); + } + + return ret; +} + #if !CONFIG_IS_ENABLED(OF_PLATDATA) static int spi_child_post_bind(struct udevice *dev) { diff --git a/include/spi.h b/include/spi.h index 378594163b..5eec0c4775 100644 --- a/include/spi.h +++ b/include/spi.h @@ -248,6 +248,26 @@ int spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen); int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, void *din, unsigned long flags);
+/** + * spi_write_then_read - SPI synchronous write followed by read + * + * This performs a half duplex transaction in which the first transaction + * is to send the opcode and if the length of buf is non-zero then it start + * the second transaction as tx or rx based on the need from respective slave. + * + * @slave: The SPI slave device with which opcode/data will be exchanged + * @opcode: opcode used for specific transfer + * @n_opcode: size of opcode, in bytes + * @txbuf: buffer into which data to be written + * @rxbuf: buffer into which data will be read + * @n_buf: size of buf (whether it's [tx|rx]buf), in bytes + * + * Returns: 0 on success, not 0 on failure + */ +int spi_write_then_read(struct spi_slave *slave, const u8 *opcode, + size_t n_opcode, const u8 *txbuf, u8 *rxbuf, + size_t n_buf); + /* Copy memory mapped data */ void spi_flash_copy_mmap(void *data, void *offset, size_t len);

Now, we have spi_write_then_read routine that would handle spi_xfer handling based on the tx_buf and rx_buf parameters.
So, replace individual flash read/write/cmd transfer call with spi_write_then_read.
Cc: Egnite GmbH info@egnite.de Cc: Daniel Gorsulowski daniel.gorsulowski@esd.eu Cc: Ilko Iliev iliev@ronetix.at Cc: Marek Vasut marex@denx.de Cc: Mateusz Kulikowski mateusz.kulikowski@gmail.com Cc: Alison Wang alison.wang@nxp.com Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/mtd/spi/sf_dataflash.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi/sf_dataflash.c b/drivers/mtd/spi/sf_dataflash.c index b6a2631747..55fb4bd31a 100644 --- a/drivers/mtd/spi/sf_dataflash.c +++ b/drivers/mtd/spi/sf_dataflash.c @@ -76,12 +76,14 @@ struct dataflash { static inline int dataflash_status(struct spi_slave *spi) { int ret; + u8 opcode = OP_READ_STATUS; u8 status; + /* * NOTE: at45db321c over 25 MHz wants to write * a dummy byte after the opcode... */ - ret = spi_flash_cmd(spi, OP_READ_STATUS, &status, 1); + ret = spi_write_then_read(spi, &opcode, 1, NULL, &status, 1); return ret ? -EIO : status; }
@@ -173,7 +175,7 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len) command[0], command[1], command[2], command[3], pageaddr);
- status = spi_flash_cmd_write(spi, command, 4, NULL, 0); + status = spi_write_then_read(spi, command, 4, NULL, NULL, 0); if (status < 0) { debug("%s: erase send command error!\n", dev->name); return -EIO; @@ -248,7 +250,7 @@ static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len, command[3] = (uint8_t)(addr >> 0);
/* plus 4 "don't care" bytes, command len: 4 + 4 "don't care" bytes */ - status = spi_flash_cmd_read(spi, command, 8, buf, len); + status = spi_write_then_read(spi, command, 8, NULL, buf, len);
spi_release_bus(spi);
@@ -327,7 +329,8 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len, debug("TRANSFER: (%x) %x %x %x\n", command[0], command[1], command[2], command[3]);
- status = spi_flash_cmd_write(spi, command, 4, NULL, 0); + status = spi_write_then_read(spi, command, 4, + NULL, NULL, 0); if (status < 0) { debug("%s: write(<pagesize) command error!\n", dev->name); @@ -352,8 +355,8 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len, debug("PROGRAM: (%x) %x %x %x\n", command[0], command[1], command[2], command[3]);
- status = spi_flash_cmd_write(spi, command, - 4, writebuf, writelen); + status = spi_write_then_read(spi, command, 4, + writebuf, NULL, writelen); if (status < 0) { debug("%s: write send command error!\n", dev->name); return -EIO; @@ -376,8 +379,8 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len, debug("COMPARE: (%x) %x %x %x\n", command[0], command[1], command[2], command[3]);
- status = spi_flash_cmd_write(spi, command, - 4, writebuf, writelen); + status = spi_write_then_read(spi, command, 4, + writebuf, NULL, writelen); if (status < 0) { debug("%s: write(compare) send command error!\n", dev->name); @@ -508,6 +511,7 @@ static struct data_flash_info *jedec_probe(struct spi_slave *spi) uint8_t id[5]; uint32_t jedec; struct data_flash_info *info; + u8 opcode = CMD_READ_ID; int status;
/* @@ -519,7 +523,7 @@ static struct data_flash_info *jedec_probe(struct spi_slave *spi) * That's not an error; only rev C and newer chips handle it, and * only Atmel sells these chips. */ - tmp = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id)); + tmp = spi_write_then_read(spi, &opcode, 1, NULL, id, sizeof(id)); if (tmp < 0) { printf("dataflash: error %d reading JEDEC ID\n", tmp); return ERR_PTR(tmp);

spi_write_then_read, will manage to do the respective spi_xfer based on the tx_buf, rx_buf so drop the legacy spi_flash_read/write/cm code.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/mtd/spi/Makefile | 2 +- drivers/mtd/spi/sf.c | 53 ----------------------------------- drivers/mtd/spi/sf_internal.h | 18 ------------ 3 files changed, 1 insertion(+), 72 deletions(-) delete mode 100644 drivers/mtd/spi/sf.c
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile index f99f6cb16e..20db1015d9 100644 --- a/drivers/mtd/spi/Makefile +++ b/drivers/mtd/spi/Makefile @@ -18,6 +18,6 @@ spi-nor-y += spi-nor-core.o endif
obj-$(CONFIG_SPI_FLASH) += spi-nor.o -obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o sf.o +obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c deleted file mode 100644 index ee3cf8b759..0000000000 --- a/drivers/mtd/spi/sf.c +++ /dev/null @@ -1,53 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * SPI flash interface - * - * Copyright (C) 2008 Atmel Corporation - * Copyright (C) 2010 Reinhard Meyer, EMK Elektronik - */ - -#include <common.h> -#include <spi.h> - -static int spi_flash_read_write(struct spi_slave *spi, - const u8 *cmd, size_t cmd_len, - const u8 *data_out, u8 *data_in, - size_t data_len) -{ - unsigned long flags = SPI_XFER_BEGIN; - int ret; - - if (data_len == 0) - flags |= SPI_XFER_END; - - ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, flags); - if (ret) { - debug("SF: Failed to send command (%zu bytes): %d\n", - cmd_len, ret); - } else if (data_len != 0) { - ret = spi_xfer(spi, data_len * 8, data_out, data_in, - SPI_XFER_END); - if (ret) - debug("SF: Failed to transfer %zu bytes of data: %d\n", - data_len, ret); - } - - return ret; -} - -int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd, - size_t cmd_len, void *data, size_t data_len) -{ - return spi_flash_read_write(spi, cmd, cmd_len, NULL, data, data_len); -} - -int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len) -{ - return spi_flash_cmd_read(spi, &cmd, 1, response, len); -} - -int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, - const void *data, size_t data_len) -{ - return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len); -} diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index a6bf734830..ece6fbc258 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -72,24 +72,6 @@ extern const struct flash_info spi_nor_ids[]; #define JEDEC_MFR(info) ((info)->id[0]) #define JEDEC_ID(info) (((info)->id[1]) << 8 | ((info)->id[2]))
-/* Send a single-byte command to the device and read the response */ -int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len); - -/* - * Send a multi-byte command to the device and read the response. Used - * for flash array reads, etc. - */ -int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd, - size_t cmd_len, void *data, size_t data_len); - -/* - * Send a multi-byte command to the device followed by (optional) - * data. Used for programming the flash array, etc. - */ -int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, - const void *data, size_t data_len); - - /* Get software write-protect value (BP bits) */ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash);

Hi Jagan,
spi_write_then_read, will manage to do the respective spi_xfer based on the tx_buf, rx_buf so drop the legacy spi_flash_read/write/cm code.
I guess that the 'sf' command will stay with us a bit longer to avoid compatibility issues on some legacy boards?
And I also assume that we now advice to switch to 'mtd' command instead of (now deprecated?) 'sf' ?
Signed-off-by: Jagan Teki jagan@amarulasolutions.com
drivers/mtd/spi/Makefile | 2 +- drivers/mtd/spi/sf.c | 53 ----------------------------------- drivers/mtd/spi/sf_internal.h | 18 ------------ 3 files changed, 1 insertion(+), 72 deletions(-) delete mode 100644 drivers/mtd/spi/sf.c
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile index f99f6cb16e..20db1015d9 100644 --- a/drivers/mtd/spi/Makefile +++ b/drivers/mtd/spi/Makefile @@ -18,6 +18,6 @@ spi-nor-y += spi-nor-core.o endif
obj-$(CONFIG_SPI_FLASH) += spi-nor.o -obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o sf.o +obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c deleted file mode 100644 index ee3cf8b759..0000000000 --- a/drivers/mtd/spi/sf.c +++ /dev/null @@ -1,53 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/*
- SPI flash interface
- Copyright (C) 2008 Atmel Corporation
- Copyright (C) 2010 Reinhard Meyer, EMK Elektronik
- */
-#include <common.h> -#include <spi.h>
-static int spi_flash_read_write(struct spi_slave *spi,
const u8 *cmd, size_t cmd_len,
const u8 *data_out, u8 *data_in,
size_t data_len)
-{
- unsigned long flags = SPI_XFER_BEGIN;
- int ret;
- if (data_len == 0)
flags |= SPI_XFER_END;
- ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, flags);
- if (ret) {
debug("SF: Failed to send command (%zu bytes): %d\n",
cmd_len, ret);
- } else if (data_len != 0) {
ret = spi_xfer(spi, data_len * 8, data_out, data_in,
SPI_XFER_END);
if (ret)
debug("SF: Failed to transfer %zu bytes of
data: %d\n",
data_len, ret);
- }
- return ret;
-}
-int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
size_t cmd_len, void *data, size_t data_len)
-{
- return spi_flash_read_write(spi, cmd, cmd_len, NULL, data,
data_len); -}
-int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len) -{
- return spi_flash_cmd_read(spi, &cmd, 1, response, len);
-}
-int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
const void *data, size_t data_len)
-{
- return spi_flash_read_write(spi, cmd, cmd_len, data, NULL,
data_len); -} diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index a6bf734830..ece6fbc258 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -72,24 +72,6 @@ extern const struct flash_info spi_nor_ids[]; #define JEDEC_MFR(info) ((info)->id[0]) #define JEDEC_ID(info) (((info)->id[1]) << 8 | ((info)->id[2])) -/* Send a single-byte command to the device and read the response */ -int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len); - -/*
- Send a multi-byte command to the device and read the response.
Used
- for flash array reads, etc.
- */
-int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
size_t cmd_len, void *data, size_t data_len);
-/*
- Send a multi-byte command to the device followed by (optional)
- data. Used for programming the flash array, etc.
- */
-int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
const void *data, size_t data_len);
/* Get software write-protect value (BP bits) */ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash);
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On Fri, Aug 2, 2019 at 4:24 PM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
spi_write_then_read, will manage to do the respective spi_xfer based on the tx_buf, rx_buf so drop the legacy spi_flash_read/write/cm code.
I guess that the 'sf' command will stay with us a bit longer to avoid compatibility issues on some legacy boards?
And I also assume that we now advice to switch to 'mtd' command instead of (now deprecated?) 'sf' ?
This sf.c code is an intermediate function calls b/w flash vs SPI and it does it related to any command stuff in upper layer. This series marked those intermediate functions into single API. the only user using these calls are dataflash driver, that indeed converted to use this new API.

On Mon, Jul 22, 2019 at 7:01 AM Jagan Teki jagan@amarulasolutions.com wrote:
Add support for SPI synchronous write followed by read, this is common interface call from spi-nor to spi drivers.
For the while series: Tested-by: Adam Ford aford173@gmail.com #da850-evm
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Jagan Teki jagan@amarulasolutions.com
drivers/spi/spi-uclass.c | 24 ++++++++++++++++++++++++ include/spi.h | 20 ++++++++++++++++++++ 2 files changed, 44 insertions(+)
[snip]
-- 2.18.0.321.gffc6fa0e3
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Tue, Jul 23, 2019 at 7:58 PM Adam Ford aford173@gmail.com wrote:
On Mon, Jul 22, 2019 at 7:01 AM Jagan Teki jagan@amarulasolutions.com wrote:
Add support for SPI synchronous write followed by read, this is common interface call from spi-nor to spi drivers.
For the while series: Tested-by: Adam Ford aford173@gmail.com #da850-evm
Applied to u-boot-spi/master
participants (3)
-
Adam Ford
-
Jagan Teki
-
Lukasz Majewski