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

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

[Mikhail Kshevetskiy]

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;
-- 
2.27.0

[PATCH 2/2] mtd: make spinand driver usable without CONFIG_DM

[Mikhail Kshevetskiy]

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 Pan<peterpandong@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 */
-- 
2.27.0

[PATCH 2/2] mtd: make spinand driver usable without CONFIG_DM

[Jagan Teki]

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.

[PATCH 2/2] mtd: make spinand driver usable without CONFIG_DM

[Mikhail Kshevetskiy]

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.



 

[PATCH 2/2] mtd: make spinand driver usable without CONFIG_DM

[Jagan Teki]

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.