A read or a write of the whole array goes out as one transfer, which fails on a controller that limits the transfer size or only offers spi-mem operations. spi-mem answers both, as spi_mem_adjust_op_size() cuts an operation to what the controller takes and spi_mem_exec_op() claims the bus per operation, so the driver needs no explicit bus claim.
The test sets a transfer size smaller than the data length, forcing the loop to process multiple chunks. Signed-off-by: Heiko Schocher <[email protected]> --- checkpatch asks for a blank line after the test function, before the DM_TEST() that registers it. But in test/dm/ all over is without a blank line, so ignore this warning. Changes in v3: - New patch. Simon pointed out that a single spi_xfer() over the whole array fails where the SPI NAND driver would work drivers/mtd/devices/Kconfig | 1 + drivers/mtd/devices/mchp23k256.c | 88 ++++++++++++++++++++------------ test/dm/mchp23k256.c | 44 ++++++++++++++++ 3 files changed, 99 insertions(+), 34 deletions(-) diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig index 0ba17618f1b..3f0ee428712 100644 --- a/drivers/mtd/devices/Kconfig +++ b/drivers/mtd/devices/Kconfig @@ -3,6 +3,7 @@ config MTD_MCHP23K256 bool "Microchip 23K256 and 23LCV1024 SPI SRAM" depends on DM_SPI && DM_MTD + select SPI_MEM help Enable support for the Microchip 23K256 and 23LCV1024 serial SRAMs, a 32 KiB and a 128 KiB SRAM on an SPI bus, the latter with battery diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c index 624fb9e65e1..06c422dfe58 100644 --- a/drivers/mtd/devices/mchp23k256.c +++ b/drivers/mtd/devices/mchp23k256.c @@ -10,6 +10,7 @@ #include <dm.h> #include <dm/device_compat.h> #include <spi.h> +#include <spi-mem.h> #include <vsprintf.h> #include <linux/bitops.h> #include <linux/mtd/mtd.h> @@ -23,9 +24,6 @@ /* MODE register, bits 7:6 select byte, page or sequential operation */ #define MCHP23K256_MODE_SEQ BIT(6) -/* The widest address of the supported parts, in bytes */ -#define MCHP23K256_ADDR_MAX 3 - /** * struct mchp23_caps - what one supported device looks like * @@ -50,8 +48,10 @@ struct mchp23k256_priv { /** * mchp23k256_command() - run one read or write command over the whole length * - * Sequential operation was selected during probe, so the device serves a - * transfer of any length from a single command and the bus is claimed once. + * Sequential operation was selected during probe, so the device serves + * a transfer of any length from a single command. spi_mem_exec_op() + * claims the bus for each operation, so the driver never calls + * spi_claim_bus() itself. * * @dev: the SRAM device * @opcode: MCHP23K256_CMD_READ or MCHP23K256_CMD_WRITE @@ -67,26 +67,52 @@ static int mchp23k256_command(struct udevice *dev, u8 opcode, u32 offset, const struct mchp23_caps *caps = (const struct mchp23_caps *)dev_get_driver_data(dev); struct spi_slave *spi = dev_get_parent_priv(dev); - u8 cmd[1 + MCHP23K256_ADDR_MAX]; - uint i; + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1), + SPI_MEM_OP_ADDR(caps->addr_width, + offset, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_IN(0, NULL, 1)); + const u8 *out = data_out; + u8 *in = data_in; int ret; - cmd[0] = opcode; - for (i = 0; i < caps->addr_width; i++) - cmd[1 + i] = offset >> (8 * (caps->addr_width - i - 1)); - - ret = spi_claim_bus(spi); - if (ret) { - dev_dbg(dev, "failed to claim SPI bus: %d\n", ret); - return ret; + if (!in) + op.data.dir = SPI_MEM_DATA_OUT; + + while (len) { + op.data.nbytes = len; + if (in) + op.data.buf.in = in; + else + op.data.buf.out = out; + + /* + * A controller may move less than asked for in one operation, + * which is what the loop is for. The device takes the address + * of every operation, so the pieces need no order. + */ + ret = spi_mem_adjust_op_size(spi, &op); + if (ret) { + dev_dbg(dev, "cannot size the operation: %d\n", ret); + return ret; + } + + ret = spi_mem_exec_op(spi, &op); + if (ret) { + dev_dbg(dev, "operation at 0x%llx failed: %d\n", + op.addr.val, ret); + return ret; + } + + op.addr.val += op.data.nbytes; + len -= op.data.nbytes; + if (in) + in += op.data.nbytes; + else + out += op.data.nbytes; } - ret = spi_write_then_read(spi, cmd, 1 + caps->addr_width, data_out, - data_in, len); - - spi_release_bus(spi); - - return ret; + return 0; } static int mchp23k256_read(struct mtd_info *mtd, loff_t from, size_t len, @@ -126,21 +152,15 @@ static int mchp23k256_probe(struct udevice *dev) struct mtd_info *mtd = dev_get_uclass_priv(dev); struct mchp23k256_priv *priv = dev_get_priv(dev); struct spi_slave *spi = dev_get_parent_priv(dev); - u8 cmd[2]; + u8 mode = MCHP23K256_MODE_SEQ; + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(MCHP23K256_CMD_WRITE_STATUS, 1), + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(sizeof(mode), &mode, 1)); int ret; - ret = spi_claim_bus(spi); - if (ret) { - dev_dbg(dev, "failed to claim SPI bus: %d\n", ret); - return ret; - } - - cmd[0] = MCHP23K256_CMD_WRITE_STATUS; - cmd[1] = MCHP23K256_MODE_SEQ; - ret = spi_write_then_read(spi, cmd, sizeof(cmd), NULL, NULL, 0); - - spi_release_bus(spi); - + ret = spi_mem_exec_op(spi, &op); if (ret) { dev_dbg(dev, "failed to select sequential operation: %d\n", ret); diff --git a/test/dm/mchp23k256.c b/test/dm/mchp23k256.c index cab8dbee547..a359deaca2f 100644 --- a/test/dm/mchp23k256.c +++ b/test/dm/mchp23k256.c @@ -35,6 +35,9 @@ #define MCHP23_ONE_ADDR 0x100 #define MCHP23_ONE_DATA_LEN 4 +/* What a controller with a limited transfer size would move in one go */ +#define MCHP23_CHUNK 16 + /* * The second partition in the test device tree. Its offset is not aligned * to any erase block size, which does not matter for a device without @@ -178,6 +181,47 @@ static int dm_test_mchp23k256_small(struct unit_test_state *uts) } DM_TEST(dm_test_mchp23k256_small, UTF_SCAN_PDATA | UTF_SCAN_FDT); +/* + * A controller that transfers less data than requested must not cause + * data loss, which is why the driver loops until all data is transferred. + */ +static int dm_test_mchp23k256_chunks(struct unit_test_state *uts) +{ + u8 src[MCHP23_LEN], dst[MCHP23_LEN]; + struct spi_slave *slave; + struct udevice *dev; + struct mtd_info *mtd; + size_t retlen; + int i; + + ut_assertok(uclass_get_device_by_ofnode(UCLASS_MTD, + ofnode_path(MCHP23_PATH), + &dev)); + mtd = dev_get_uclass_priv(dev); + + /* Stand in for a controller that cannot do the whole transfer */ + slave = dev_get_parent_priv(dev); + slave->max_write_size = MCHP23_CHUNK; + slave->max_read_size = MCHP23_CHUNK; + + for (i = 0; i < MCHP23_LEN; i++) + src[i] = 0xc0 + i; + + ut_assertok(mtd_write(mtd, MCHP23_OFFSET, MCHP23_LEN, &retlen, src)); + ut_asserteq(MCHP23_LEN, retlen); + + memset(dst, 0, MCHP23_LEN); + ut_assertok(mtd_read(mtd, MCHP23_OFFSET, MCHP23_LEN, &retlen, dst)); + ut_asserteq(MCHP23_LEN, retlen); + ut_asserteq_mem(src, dst, MCHP23_LEN); + + slave->max_write_size = 0; + slave->max_read_size = 0; + + return 0; +} +DM_TEST(dm_test_mchp23k256_chunks, UTF_SCAN_PDATA | UTF_SCAN_FDT); + static int dm_test_mchp23k256_part(struct unit_test_state *uts) { struct mtd_info *mtd, *part; -- 2.55.0
