[PATCH 1/2] spi: make spi-mem driver usable without CONFIG_DM_SPI, drop NODM variant

From: Ivan Morozko Ivan.Morozko@oktetlabs.ru
There is no reason to have a separate and highly restricted version of spi-mem driver for NODM case, it's quite easily fix DM based driver version and use it for all cases.
Signed-off-by: Ivan Morozko Ivan.Morozko@oktetlabs.ru Reviewed-by: Mikhail Kshevetskiy mikhail.kshevetskiy@oktetlabs.ru --- drivers/spi/Makefile | 3 +- drivers/spi/spi-mem-nodm.c | 107 ------------------------------------- drivers/spi/spi-mem.c | 37 ++++++++----- include/spi.h | 1 + 4 files changed, 26 insertions(+), 122 deletions(-) delete mode 100644 drivers/spi/spi-mem-nodm.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 54881a7412..6dff4eed83 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -8,12 +8,11 @@ ifdef CONFIG_DM_SPI obj-y += spi-uclass.o obj-$(CONFIG_SANDBOX) += spi-emul-uclass.o obj-$(CONFIG_SOFT_SPI) += soft_spi.o -obj-$(CONFIG_SPI_MEM) += spi-mem.o obj-$(CONFIG_TI_QSPI) += ti_qspi.o else obj-y += spi.o -obj-$(CONFIG_SPI_MEM) += spi-mem-nodm.o endif +obj-$(CONFIG_SPI_MEM) += spi-mem.o
obj-$(CONFIG_ALTERA_SPI) += altera_spi.o obj-$(CONFIG_ATH79_SPI) += ath79_spi.o diff --git a/drivers/spi/spi-mem-nodm.c b/drivers/spi/spi-mem-nodm.c deleted file mode 100644 index 765f05fe54..0000000000 --- a/drivers/spi/spi-mem-nodm.c +++ /dev/null @@ -1,107 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ - */ - -#include <log.h> -#include <malloc.h> -#include <spi.h> -#include <spi-mem.h> - -int spi_mem_exec_op(struct spi_slave *slave, - const struct spi_mem_op *op) -{ - unsigned int pos = 0; - const u8 *tx_buf = NULL; - u8 *rx_buf = NULL; - u8 *op_buf; - int op_len; - u32 flag; - int ret; - int i; - - if (op->data.nbytes) { - if (op->data.dir == SPI_MEM_DATA_IN) - rx_buf = op->data.buf.in; - else - tx_buf = op->data.buf.out; - } - - op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; - op_buf = calloc(1, op_len); - - ret = spi_claim_bus(slave); - if (ret < 0) - return ret; - - op_buf[pos++] = op->cmd.opcode; - - if (op->addr.nbytes) { - for (i = 0; i < op->addr.nbytes; i++) - op_buf[pos + i] = op->addr.val >> - (8 * (op->addr.nbytes - i - 1)); - - pos += op->addr.nbytes; - } - - if (op->dummy.nbytes) - memset(op_buf + pos, 0xff, op->dummy.nbytes); - - /* 1st transfer: opcode + address + dummy cycles */ - flag = SPI_XFER_BEGIN; - /* Make sure to set END bit if no tx or rx data messages follow */ - if (!tx_buf && !rx_buf) - flag |= SPI_XFER_END; - - ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag); - if (ret) - return ret; - - /* 2nd transfer: rx or tx data path */ - if (tx_buf || rx_buf) { - ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, - rx_buf, SPI_XFER_END); - if (ret) - return ret; - } - - spi_release_bus(slave); - - for (i = 0; i < pos; i++) - debug("%02x ", op_buf[i]); - debug("| [%dB %s] ", - tx_buf || rx_buf ? op->data.nbytes : 0, - tx_buf || rx_buf ? (tx_buf ? "out" : "in") : "-"); - for (i = 0; i < op->data.nbytes; i++) - debug("%02x ", tx_buf ? tx_buf[i] : rx_buf[i]); - debug("[ret %d]\n", ret); - - free(op_buf); - - if (ret < 0) - return ret; - - return 0; -} - -int spi_mem_adjust_op_size(struct spi_slave *slave, - struct spi_mem_op *op) -{ - unsigned int len; - - len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; - if (slave->max_write_size && len > slave->max_write_size) - return -EINVAL; - - if (op->data.dir == SPI_MEM_DATA_IN && slave->max_read_size) - op->data.nbytes = min(op->data.nbytes, - slave->max_read_size); - else if (slave->max_write_size) - op->data.nbytes = min(op->data.nbytes, - slave->max_write_size - len); - - if (!op->data.nbytes) - return -EINVAL; - - return 0; -} diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index d344701aeb..e4544edaad 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -105,6 +105,19 @@ void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr, EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data); #endif /* __UBOOT__ */
+static inline +struct spi_controller_mem_ops *spi_get_mem_ops(struct spi_slave *slave) +{ +#ifdef CONFIG_DM_SPI + struct udevice *bus = slave->dev->parent; + struct dm_spi_ops *ops = spi_get_ops(bus); + + return ops->mem_ops; +#else + return slave->mem_ops; +#endif +} + static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx) { u32 mode = slave->mode; @@ -181,11 +193,10 @@ EXPORT_SYMBOL_GPL(spi_mem_default_supports_op); bool spi_mem_supports_op(struct spi_slave *slave, const struct spi_mem_op *op) { - struct udevice *bus = slave->dev->parent; - struct dm_spi_ops *ops = spi_get_ops(bus); + struct spi_controller_mem_ops *mem_ops = spi_get_mem_ops(slave);
- if (ops->mem_ops && ops->mem_ops->supports_op) - return ops->mem_ops->supports_op(slave, op); + if (mem_ops && mem_ops->supports_op) + return mem_ops->supports_op(slave, op);
return spi_mem_default_supports_op(slave, op); } @@ -205,8 +216,7 @@ EXPORT_SYMBOL_GPL(spi_mem_supports_op); */ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) { - struct udevice *bus = slave->dev->parent; - struct dm_spi_ops *ops = spi_get_ops(bus); + struct spi_controller_mem_ops *mem_ops = spi_get_mem_ops(slave); unsigned int pos = 0; const u8 *tx_buf = NULL; u8 *rx_buf = NULL; @@ -222,7 +232,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) if (ret < 0) return ret;
- if (ops->mem_ops && ops->mem_ops->exec_op) { + if (mem_ops && mem_ops->exec_op) { #ifndef __UBOOT__ /* * Flush the message queue before executing our SPI memory @@ -243,7 +253,8 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) mutex_lock(&ctlr->bus_lock_mutex); mutex_lock(&ctlr->io_mutex); #endif - ret = ops->mem_ops->exec_op(slave, op); + + ret = mem_ops->exec_op(slave, op);
#ifndef __UBOOT__ mutex_unlock(&ctlr->io_mutex); @@ -425,13 +436,12 @@ EXPORT_SYMBOL_GPL(spi_mem_exec_op); */ int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op) { - struct udevice *bus = slave->dev->parent; - struct dm_spi_ops *ops = spi_get_ops(bus); + struct spi_controller_mem_ops *mem_ops = spi_get_mem_ops(slave);
- if (ops->mem_ops && ops->mem_ops->adjust_op_size) - return ops->mem_ops->adjust_op_size(slave, op); + if (mem_ops && mem_ops->adjust_op_size) + return mem_ops->adjust_op_size(slave, op);
- if (!ops->mem_ops || !ops->mem_ops->exec_op) { + if (!mem_ops || !mem_ops->exec_op) { unsigned int len;
len = sizeof(op->cmd.opcode) + op->addr.nbytes + @@ -450,6 +460,7 @@ int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op)
if (!op->data.nbytes) return -EINVAL; + }
return 0; diff --git a/include/spi.h b/include/spi.h index 5cc6d6e008..825737b120 100644 --- a/include/spi.h +++ b/include/spi.h @@ -138,6 +138,7 @@ struct spi_slave { #else unsigned int bus; unsigned int cs; + struct spi_controller_mem_ops *mem_ops; #endif uint mode; unsigned int wordlen;

From: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Here is an example of spinand driver initialization without CONFIG_DM enabled:
void board_nand_init(void) { static struct spinand_device spinand; static struct mtd_info mtd; static struct spi_slave *slave; int ret;
memset(&spinand, 0, sizeof(spinand)); memset(&mtd, 0, sizeof(mtd));
slave = spi_setup_slave(BUS, CS, MAX_HZ, MODE); if (!slave) { printf("SPI-NAND: Failed to set up SPI slave\n"); return; }
slave->mode |= (SPI_TX_BYTE | SPI_RX_SLOW); slave->max_read_size = SPI_MAX_READ_SIZE; slave->max_write_size = SPI_MAX_WRITE_SIZE;
ret = spinand_probe_no_dm(&spinand, slave, &mtd); if (!ret) nand_register(0, &mtd); }
Using of dual and quad wire transfer modes requires: * dual/quad speed capable hardware (both controller and flash) * physical presence of 4 data wires (quad mode only) * spi controller driver MUST supports slave->mem_ops.exec_op() operations (spi_xfer() interface will suits for single speed data transfer mode only)
Signed-off-by: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru --- drivers/mtd/nand/spi/Kconfig | 2 +- drivers/mtd/nand/spi/core.c | 46 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig index 0777dfdf0a..fd88e2cec9 100644 --- a/drivers/mtd/nand/spi/Kconfig +++ b/drivers/mtd/nand/spi/Kconfig @@ -1,6 +1,6 @@ menuconfig MTD_SPI_NAND bool "SPI NAND device Support" - depends on DM_MTD && DM_SPI + depends on (DM_MTD && DM_SPI) || (MTD && SPI) select MTD_NAND_CORE select SPI_MEM help diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index e0ebd9c04b..a24e2e6677 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -170,14 +170,22 @@ int spinand_select_target(struct spinand_device *spinand, unsigned int target) static int spinand_init_cfg_cache(struct spinand_device *spinand) { struct nand_device *nand = spinand_to_nand(spinand); +#ifdef CONFIG_DM struct udevice *dev = spinand->slave->dev; +#endif unsigned int target; int ret;
+#ifdef CONFIG_DM spinand->cfg_cache = devm_kzalloc(dev, sizeof(*spinand->cfg_cache) * nand->memorg.ntargets, GFP_KERNEL); +#else + spinand->cfg_cache = kzalloc(sizeof(*spinand->cfg_cache) * + nand->memorg.ntargets, + GFP_KERNEL); +#endif if (!spinand->cfg_cache) return -ENOMEM;
@@ -1135,6 +1144,7 @@ static void spinand_cleanup(struct spinand_device *spinand) kfree(spinand->scratchbuf); }
+#ifdef CONFIG_DM static int spinand_probe(struct udevice *dev) { struct spinand_device *spinand = dev_get_priv(dev); @@ -1187,6 +1197,40 @@ err_spinand_cleanup:
return ret; } +#endif /* CONFIG_DM */ + +#ifdef __UBOOT__ +int spinand_probe_no_dm(struct spinand_device *spinand, + struct spi_slave *slave, + struct mtd_info *mtd) +{ + struct nand_device *nand = spinand_to_nand(spinand); + int ret; + + nand->mtd = mtd; + mtd->priv = nand; + mtd->name = malloc(20); + if (!mtd->name) + return -ENOMEM; + sprintf(mtd->name, "spi-nand%d", spi_nand_idx++); + spinand->slave = slave; + + ret = spinand_init(spinand); + if (ret) + return ret; + + ret = add_mtd_device(mtd); + if (ret) + goto err_spinand_cleanup; + + return 0; + +err_spinand_cleanup: + spinand_cleanup(spinand); + + return ret; +} +#endif /* __UBOOT__ */
#ifndef __UBOOT__ static int spinand_remove(struct udevice *slave) @@ -1238,6 +1282,7 @@ MODULE_AUTHOR("Peter Panpeterpandong@micron.com"); MODULE_LICENSE("GPL v2"); #endif /* __UBOOT__ */
+#ifdef CONFIG_DM static const struct udevice_id spinand_ids[] = { { .compatible = "spi-nand" }, { /* sentinel */ }, @@ -1250,3 +1295,4 @@ U_BOOT_DRIVER(spinand) = { .priv_auto_alloc_size = sizeof(struct spinand_device), .probe = spinand_probe, }; +#endif /* CONFIG_DM */

On Mon, Jun 22, 2020 at 9:25 PM Mikhail Kshevetskiy mikhail.kshevetskiy@oktetlabs.ru wrote:
From: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Here is an example of spinand driver initialization without CONFIG_DM enabled:
void board_nand_init(void) { static struct spinand_device spinand; static struct mtd_info mtd; static struct spi_slave *slave; int ret;
memset(&spinand, 0, sizeof(spinand)); memset(&mtd, 0, sizeof(mtd)); slave = spi_setup_slave(BUS, CS, MAX_HZ, MODE); if (!slave) { printf("SPI-NAND: Failed to set up SPI slave\n"); return; } slave->mode |= (SPI_TX_BYTE | SPI_RX_SLOW); slave->max_read_size = SPI_MAX_READ_SIZE; slave->max_write_size = SPI_MAX_WRITE_SIZE; ret = spinand_probe_no_dm(&spinand, slave, &mtd); if (!ret) nand_register(0, &mtd);
}
Using of dual and quad wire transfer modes requires:
- dual/quad speed capable hardware (both controller and flash)
- physical presence of 4 data wires (quad mode only)
- spi controller driver MUST supports slave->mem_ops.exec_op() operations (spi_xfer() interface will suits for single speed data transfer mode only)
Signed-off-by: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Now it's time for dm only code, better not come up with nondm anymore as possible, please.
Jagan.

On Sat, 11 Jul 2020 14:16:23 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Mon, Jun 22, 2020 at 9:25 PM Mikhail Kshevetskiy mikhail.kshevetskiy@oktetlabs.ru wrote:
From: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Here is an example of spinand driver initialization without CONFIG_DM enabled:
void board_nand_init(void) { static struct spinand_device spinand; static struct mtd_info mtd; static struct spi_slave *slave; int ret;
memset(&spinand, 0, sizeof(spinand)); memset(&mtd, 0, sizeof(mtd)); slave = spi_setup_slave(BUS, CS, MAX_HZ, MODE); if (!slave) { printf("SPI-NAND: Failed to set up SPI slave\n"); return; } slave->mode |= (SPI_TX_BYTE | SPI_RX_SLOW); slave->max_read_size = SPI_MAX_READ_SIZE; slave->max_write_size = SPI_MAX_WRITE_SIZE; ret = spinand_probe_no_dm(&spinand, slave, &mtd); if (!ret) nand_register(0, &mtd);
}
Using of dual and quad wire transfer modes requires:
- dual/quad speed capable hardware (both controller and flash)
- physical presence of 4 data wires (quad mode only)
- spi controller driver MUST supports slave->mem_ops.exec_op() operations (spi_xfer() interface will suits for single speed data transfer mode only)
Signed-off-by: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Now it's time for dm only code, better not come up with nondm anymore as possible, please.
Jagan.
I know about it. These patches were in the queue for upstreaming from the new year :-(
We are using 2010 and 2016 year based u-boot in our projects and due to some reasons it's not easy to switch it to DM build. This two patches helps us with backporting of spinand driver from upsteam u-boot. I think we are not an only users of old u-boot in the world, so it maybe helpful for other people as well.
Just ignore these two patches if you think that NODM era is completely over.
Mikhail.

On Sat, Jul 11, 2020 at 3:06 PM Mikhail Kshevetskiy mkshevetskiy@oktetlabs.ru wrote:
On Sat, 11 Jul 2020 14:16:23 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Mon, Jun 22, 2020 at 9:25 PM Mikhail Kshevetskiy mikhail.kshevetskiy@oktetlabs.ru wrote:
From: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Here is an example of spinand driver initialization without CONFIG_DM enabled:
void board_nand_init(void) { static struct spinand_device spinand; static struct mtd_info mtd; static struct spi_slave *slave; int ret;
memset(&spinand, 0, sizeof(spinand)); memset(&mtd, 0, sizeof(mtd)); slave = spi_setup_slave(BUS, CS, MAX_HZ, MODE); if (!slave) { printf("SPI-NAND: Failed to set up SPI slave\n"); return; } slave->mode |= (SPI_TX_BYTE | SPI_RX_SLOW); slave->max_read_size = SPI_MAX_READ_SIZE; slave->max_write_size = SPI_MAX_WRITE_SIZE; ret = spinand_probe_no_dm(&spinand, slave, &mtd); if (!ret) nand_register(0, &mtd);
}
Using of dual and quad wire transfer modes requires:
- dual/quad speed capable hardware (both controller and flash)
- physical presence of 4 data wires (quad mode only)
- spi controller driver MUST supports slave->mem_ops.exec_op() operations (spi_xfer() interface will suits for single speed data transfer mode only)
Signed-off-by: Mikhail Kshevetskiy Mikhail.Kshevetskiy@oktetlabs.ru
Now it's time for dm only code, better not come up with nondm anymore as possible, please.
Jagan.
I know about it. These patches were in the queue for upstreaming from the new year :-(
We are using 2010 and 2016 year based u-boot in our projects and due to some reasons it's not easy to switch it to DM build. This two patches helps us with backporting of spinand driver from upsteam u-boot. I think we are not an only users of old u-boot in the world, so it maybe helpful for other people as well.
Yes, I agree with this point in terms of MTD code is a concern. However, supporting nondm at this point look traversing in the reverse direction as per as U-Boot development is a concern. I can guarantee that the community is always helping to improve code if you wish to use/migrate the mainline at any point.
Just ignore these two patches if you think that NODM era is completely over.
Thanks.
Jagan.
participants (3)
-
Jagan Teki
-
Mikhail Kshevetskiy
-
Mikhail Kshevetskiy