[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Angelo Dureghello]

This patch adds DM support to cf_spi.c.

How to use/test it:

1) enable the following options,

CONFIG_DM_SPI
CONFIG_DM_SPI_FLASH

2) add similar code into your board.c file

U_BOOT_DEVICE(coldfire_spi) = {
	.name = "spi_coldfire",
	.platdata = &mcf_spi_plat,
};

---
Changes from v1:
- split into 2 patches

Changes from v2:
- back in a single patch, no need to add fdt support or special
  config options
- doc/driver-model/spi-howto.txt has been re-checked and followed,
  the driver now builds as is.

Signed-off-by: Angelo Dureghello <angelo@sysam.it>
---
 drivers/spi/cf_spi.c                    | 421 +++++++++++++++---------
 include/dm/platform_data/spi_coldfire.h |  22 ++
 2 files changed, 293 insertions(+), 150 deletions(-)
 create mode 100644 include/dm/platform_data/spi_coldfire.h

diff --git a/drivers/spi/cf_spi.c b/drivers/spi/cf_spi.c
index 522631cbbf..7627f70a1f 100644
--- a/drivers/spi/cf_spi.c
+++ b/drivers/spi/cf_spi.c
@@ -6,15 +6,24 @@
  *
  * Copyright (C) 2004-2009 Freescale Semiconductor, Inc.
  * TsiChung Liew (Tsi-Chung.Liew at freescale.com)
+ *
+ * Support for device model:
+ * Copyright (C) 2018 Angelo Dureghello <angelo@sysam.it>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>
+#include <dm.h>
+#include <dm/platform_data/spi_coldfire.h>
 #include <spi.h>
 #include <malloc.h>
 #include <asm/immap.h>
 
-struct cf_spi_slave {
+struct coldfire_spi_priv {
+#ifndef CONFIG_DM_SPI
 	struct spi_slave slave;
+#endif
 	uint baudrate;
 	int charbit;
 };
@@ -38,12 +47,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define SPI_MODE_MOD	0x00200000
 #define SPI_DBLRATE	0x00100000
 
-static inline struct cf_spi_slave *to_cf_spi_slave(struct spi_slave *slave)
-{
-	return container_of(slave, struct cf_spi_slave, slave);
-}
-
-static void cfspi_init(void)
+static void __spi_init(void)
 {
 	volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
 
@@ -81,7 +85,123 @@ static void cfspi_init(void)
 #endif
 }
 
-static void cfspi_tx(u32 ctrl, u16 data)
+int __spi_set_speed(struct coldfire_spi_priv *cfspi, uint bus, uint mode)
+{
+	/*
+	 * bit definition for mode:
+	 * bit 31 - 28: Transfer size 3 to 16 bits
+	 *     27 - 26: PCS to SCK delay prescaler
+	 *     25 - 24: After SCK delay prescaler
+	 *     23 - 22: Delay after transfer prescaler
+	 *     21     : Allow overwrite for bit 31-22 and bit 20-8
+	 *     20     : Double baud rate
+	 *     19 - 16: PCS to SCK delay scaler
+	 *     15 - 12: After SCK delay scaler
+	 *     11 -  8: Delay after transfer scaler
+	 *      7 -  0: SPI_CPHA, SPI_CPOL, SPI_LSB_FIRST
+	 */
+	volatile dspi_t *dspi = (dspi_t *)MMAP_DSPI;
+	int prescaler[] = { 2, 3, 5, 7 };
+	int scaler[] = {
+		2, 4, 6, 8,
+		16, 32, 64, 128,
+		256, 512, 1024, 2048,
+		4096, 8192, 16384, 32768
+	};
+	int i, j, pbrcnt, brcnt, diff, tmp, dbr = 0;
+	int best_i, best_j, bestmatch = 0x7FFFFFFF, baud_speed;
+	u32 bus_setup = 0;
+
+	tmp = (prescaler[3] * scaler[15]);
+	/* Maximum and minimum baudrate it can handle */
+	if ((cfspi->baudrate > (gd->bus_clk >> 1)) ||
+	    (cfspi->baudrate < (gd->bus_clk / tmp))) {
+		printf("Exceed baudrate limitation: Max %d - Min %d\n",
+		       (int)(gd->bus_clk >> 1), (int)(gd->bus_clk / tmp));
+		return -1;
+	}
+
+	/* Activate Double Baud when it exceed 1/4 the bus clk */
+	if ((CONFIG_SYS_DSPI_CTAR0 & DSPI_CTAR_DBR) ||
+	    (cfspi->baudrate > (gd->bus_clk / (prescaler[0] * scaler[0])))) {
+		bus_setup |= DSPI_CTAR_DBR;
+		dbr = 1;
+	}
+
+	/* Overwrite default value set in platform configuration file */
+	if (mode & SPI_MODE_MOD) {
+		/*
+		 * Check to see if it is enabled by default in platform
+		 * config, or manual setting passed by mode parameter
+		 */
+		if (mode & SPI_DBLRATE) {
+			bus_setup |= DSPI_CTAR_DBR;
+			dbr = 1;
+		}
+	}
+
+	pbrcnt = sizeof(prescaler) / sizeof(int);
+	brcnt = sizeof(scaler) / sizeof(int);
+
+	/* baudrate calculation - to closer value, may not be exact match */
+	for (best_i = 0, best_j = 0, i = 0; i < pbrcnt; i++) {
+		baud_speed = gd->bus_clk / prescaler[i];
+		for (j = 0; j < brcnt; j++) {
+			tmp = (baud_speed / scaler[j]) * (1 + dbr);
+
+			if (tmp > cfspi->baudrate)
+				diff = tmp - cfspi->baudrate;
+			else
+				diff = cfspi->baudrate - tmp;
+
+			if (diff < bestmatch) {
+				bestmatch = diff;
+				best_i = i;
+				best_j = j;
+			}
+		}
+	}
+	bus_setup |= (DSPI_CTAR_PBR(best_i) | DSPI_CTAR_BR(best_j));
+	dspi->ctar[bus] |= bus_setup;
+
+	return 0;
+}
+
+static int __spi_set_mode(struct coldfire_spi_priv *cfspi, uint bus, uint mode)
+{
+	volatile dspi_t *dspi = (dspi_t *)MMAP_DSPI;
+	u32 bus_setup = 0;
+
+	if (mode & SPI_CPOL)
+		bus_setup |= DSPI_CTAR_CPOL;
+	if (mode & SPI_CPHA)
+		bus_setup |= DSPI_CTAR_CPHA;
+	if (mode & SPI_LSB_FIRST)
+		bus_setup |= DSPI_CTAR_LSBFE;
+
+	/* Overwrite default value set in platform configuration file */
+	if (mode & SPI_MODE_MOD) {
+		if ((mode & 0xF0000000) == 0)
+			bus_setup |=
+			    dspi->ctar[bus] & 0x78000000;
+		else
+			bus_setup |= ((mode & 0xF0000000) >> 1);
+
+		bus_setup |= (mode & 0x0FC00000) >> 4;	/* PSCSCK, PASC, PDT */
+		bus_setup |= (mode & 0x000FFF00) >> 4;	/* CSSCK, ASC, DT */
+	} else {
+		bus_setup |= (dspi->ctar[bus] & 0x78FCFFF0);
+	}
+
+	cfspi->charbit =
+		((dspi->ctar[bus] & 0x78000000) == 0x78000000) ? 16 : 8;
+
+	dspi->ctar[bus] |= bus_setup;
+
+	return 0;
+}
+
+static void __cfspi_tx(u32 ctrl, u16 data)
 {
 	volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
 
@@ -90,7 +210,7 @@ static void cfspi_tx(u32 ctrl, u16 data)
 	dspi->tfr = (ctrl | data);
 }
 
-static u16 cfspi_rx(void)
+static u16 __cfspi_rx(void)
 {
 	volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
 
@@ -99,16 +219,15 @@ static u16 cfspi_rx(void)
 	return (dspi->rfr & 0xFFFF);
 }
 
-static int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout,
-		      void *din, ulong flags)
+static int __spi_xfer(struct coldfire_spi_priv *cfspi, uint cs, uint bitlen,
+		      const void *dout, void *din, ulong flags)
 {
-	struct cf_spi_slave *cfslave = to_cf_spi_slave(slave);
 	u16 *spi_rd16 = NULL, *spi_wr16 = NULL;
 	u8 *spi_rd = NULL, *spi_wr = NULL;
 	static u32 ctrl = 0;
 	uint len = bitlen >> 3;
 
-	if (cfslave->charbit == 16) {
+	if (cfspi->charbit == 16) {
 		bitlen >>= 1;
 		spi_wr16 = (u16 *) dout;
 		spi_rd16 = (u16 *) din;
@@ -120,25 +239,25 @@ static int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout,
 	if ((flags & SPI_XFER_BEGIN) == SPI_XFER_BEGIN)
 		ctrl |= DSPI_TFR_CONT;
 
-	ctrl = (ctrl & 0xFF000000) | ((1 << slave->cs) << 16);
+	ctrl = (ctrl & 0xFF000000) | ((1 << cs) << 16);
 
 	if (len > 1) {
 		int tmp_len = len - 1;
 		while (tmp_len--) {
 			if (dout != NULL) {
-				if (cfslave->charbit == 16)
-					cfspi_tx(ctrl, *spi_wr16++);
+				if (cfspi->charbit == 16)
+					__cfspi_tx(ctrl, *spi_wr16++);
 				else
-					cfspi_tx(ctrl, *spi_wr++);
-				cfspi_rx();
+					__cfspi_tx(ctrl, *spi_wr++);
+				__cfspi_rx();
 			}
 
 			if (din != NULL) {
-				cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
-				if (cfslave->charbit == 16)
-					*spi_rd16++ = cfspi_rx();
+				__cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
+				if (cfspi->charbit == 16)
+					*spi_rd16++ = __cfspi_rx();
 				else
-					*spi_rd++ = cfspi_rx();
+					*spi_rd++ = __cfspi_rx();
 			}
 		}
 
@@ -150,186 +269,188 @@ static int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout,
 
 	if (len) {
 		if (dout != NULL) {
-			if (cfslave->charbit == 16)
-				cfspi_tx(ctrl, *spi_wr16);
+			if (cfspi->charbit == 16)
+				__cfspi_tx(ctrl, *spi_wr16);
 			else
-				cfspi_tx(ctrl, *spi_wr);
-			cfspi_rx();
+				__cfspi_tx(ctrl, *spi_wr);
+			__cfspi_rx();
 		}
 
 		if (din != NULL) {
-			cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
-			if (cfslave->charbit == 16)
-				*spi_rd16 = cfspi_rx();
+			__cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
+			if (cfspi->charbit == 16)
+				*spi_rd16 = __cfspi_rx();
 			else
-				*spi_rd = cfspi_rx();
+				*spi_rd = __cfspi_rx();
 		}
 	} else {
 		/* dummy read */
-		cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
-		cfspi_rx();
+		__cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
+		__cfspi_rx();
 	}
 
 	return 0;
 }
 
-static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
-					   uint mode)
+#ifndef CONFIG_DM_SPI
+
+static inline struct coldfire_spi_priv *to_coldfire_spi_slave
+		(struct spi_slave *slave)
 {
-	/*
-	 * bit definition for mode:
-	 * bit 31 - 28: Transfer size 3 to 16 bits
-	 *     27 - 26: PCS to SCK delay prescaler
-	 *     25 - 24: After SCK delay prescaler
-	 *     23 - 22: Delay after transfer prescaler
-	 *     21     : Allow overwrite for bit 31-22 and bit 20-8
-	 *     20     : Double baud rate
-	 *     19 - 16: PCS to SCK delay scaler
-	 *     15 - 12: After SCK delay scaler
-	 *     11 -  8: Delay after transfer scaler
-	 *      7 -  0: SPI_CPHA, SPI_CPOL, SPI_LSB_FIRST
-	 */
-	volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
-	int prescaler[] = { 2, 3, 5, 7 };
-	int scaler[] = {
-		2, 4, 6, 8,
-		16, 32, 64, 128,
-		256, 512, 1024, 2048,
-		4096, 8192, 16384, 32768
-	};
-	int i, j, pbrcnt, brcnt, diff, tmp, dbr = 0;
-	int best_i, best_j, bestmatch = 0x7FFFFFFF, baud_speed;
-	u32 bus_setup = 0;
+	return container_of(slave, struct coldfire_spi_priv, slave);
+}
 
-	tmp = (prescaler[3] * scaler[15]);
-	/* Maximum and minimum baudrate it can handle */
-	if ((cfslave->baudrate > (gd->bus_clk >> 1)) ||
-	    (cfslave->baudrate < (gd->bus_clk / tmp))) {
-		printf("Exceed baudrate limitation: Max %d - Min %d\n",
-		       (int)(gd->bus_clk >> 1), (int)(gd->bus_clk / tmp));
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+				  unsigned int max_hz, unsigned int mode)
+{
+	struct coldfire_spi_priv *cfspi;
+
+	if (!spi_cs_is_valid(bus, cs))
 		return NULL;
-	}
 
-	/* Activate Double Baud when it exceed 1/4 the bus clk */
-	if ((CONFIG_SYS_DSPI_CTAR0 & DSPI_CTAR_DBR) ||
-	    (cfslave->baudrate > (gd->bus_clk / (prescaler[0] * scaler[0])))) {
-		bus_setup |= DSPI_CTAR_DBR;
-		dbr = 1;
-	}
+	cfspi = spi_alloc_slave(struct coldfire_spi_priv, bus, cs);
+	if (!cfspi)
+		return NULL;
 
-	if (mode & SPI_CPOL)
-		bus_setup |= DSPI_CTAR_CPOL;
-	if (mode & SPI_CPHA)
-		bus_setup |= DSPI_CTAR_CPHA;
-	if (mode & SPI_LSB_FIRST)
-		bus_setup |= DSPI_CTAR_LSBFE;
+	cfspi->baudrate = max_hz;
 
-	/* Overwrite default value set in platform configuration file */
-	if (mode & SPI_MODE_MOD) {
+	if (__spi_set_speed(cfspi, bus, mode))
+		return NULL;
 
-		if ((mode & 0xF0000000) == 0)
-			bus_setup |=
-			    dspi->ctar[cfslave->slave.bus] & 0x78000000;
-		else
-			bus_setup |= ((mode & 0xF0000000) >> 1);
+	if (__spi_set_mode(cfspi, bus, mode))
+		return NULL;
 
-		/*
-		 * Check to see if it is enabled by default in platform
-		 * config, or manual setting passed by mode parameter
-		 */
-		if (mode & SPI_DBLRATE) {
-			bus_setup |= DSPI_CTAR_DBR;
-			dbr = 1;
-		}
-		bus_setup |= (mode & 0x0FC00000) >> 4;	/* PSCSCK, PASC, PDT */
-		bus_setup |= (mode & 0x000FFF00) >> 4;	/* CSSCK, ASC, DT */
-	} else
-		bus_setup |= (dspi->ctar[cfslave->slave.bus] & 0x78FCFFF0);
+	return &cfspi->slave;
+}
 
-	cfslave->charbit =
-	    ((dspi->ctar[cfslave->slave.bus] & 0x78000000) ==
-	     0x78000000) ? 16 : 8;
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+	if (((cs >= 0) && (cs < 8)) && ((bus >= 0) && (bus < 8)))
+		return 1;
+	else
+		return 0;
+}
 
-	pbrcnt = sizeof(prescaler) / sizeof(int);
-	brcnt = sizeof(scaler) / sizeof(int);
+void spi_init(void)
+{
+	__spi_init();
+}
 
-	/* baudrate calculation - to closer value, may not be exact match */
-	for (best_i = 0, best_j = 0, i = 0; i < pbrcnt; i++) {
-		baud_speed = gd->bus_clk / prescaler[i];
-		for (j = 0; j < brcnt; j++) {
-			tmp = (baud_speed / scaler[j]) * (1 + dbr);
+void spi_free_slave(struct spi_slave *slave)
+{
+	struct coldfire_spi_priv *cfspi = to_coldfire_spi_slave(slave);
 
-			if (tmp > cfslave->baudrate)
-				diff = tmp - cfslave->baudrate;
-			else
-				diff = cfslave->baudrate - tmp;
+	free(cfspi);
+}
 
-			if (diff < bestmatch) {
-				bestmatch = diff;
-				best_i = i;
-				best_j = j;
-			}
-		}
-	}
-	bus_setup |= (DSPI_CTAR_PBR(best_i) | DSPI_CTAR_BR(best_j));
-	dspi->ctar[cfslave->slave.bus] = bus_setup;
+int spi_claim_bus(struct spi_slave *slave)
+{
+	return cfspi_claim_bus(slave->bus, slave->cs);
+}
 
-	return &cfslave->slave;
+void spi_release_bus(struct spi_slave *slave)
+{
+	cfspi_release_bus(slave->bus, slave->cs);
 }
-#endif				/* CONFIG_CF_DSPI */
 
-#ifdef CONFIG_CMD_SPI
-int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
+	     void *din, unsigned long flags)
 {
-	if (((cs >= 0) && (cs < 8)) && ((bus >= 0) && (bus < 8)))
-		return 1;
-	else
-		return 0;
+	struct coldfire_spi_priv *cfspi = to_coldfire_spi_slave(slave);
+
+	return __spi_xfer(cfspi, slave->cs, bitlen, dout, din, flags);
 }
 
+#else
+
 void spi_init(void)
 {
-	cfspi_init();
+	/* arch-related port configuration */
+	__spi_init();
 }
 
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
-				  unsigned int max_hz, unsigned int mode)
+static int coldfire_spi_claim_bus(struct udevice *dev)
 {
-	struct cf_spi_slave *cfslave;
+	struct udevice *bus = dev_get_parent(dev);
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
 
-	if (!spi_cs_is_valid(bus, cs))
-		return NULL;
+	return cfspi_claim_bus(bus->seq, plat->cs);
+}
 
-	cfslave = spi_alloc_slave(struct cf_spi_slave, bus, cs);
-	if (!cfslave)
-		return NULL;
+static int coldfire_spi_release_bus(struct udevice *dev)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
 
-	cfslave->baudrate = max_hz;
+	cfspi_release_bus(bus->seq, plat->cs);
 
-	/* specific setup */
-	return cfspi_setup_slave(cfslave, mode);
+	return 0;
 }
 
-void spi_free_slave(struct spi_slave *slave)
+static int coldfire_spi_xfer(struct udevice *dev, unsigned int bitlen,
+			     const void *dout, void *din,
+			     unsigned long flags)
 {
-	struct cf_spi_slave *cfslave = to_cf_spi_slave(slave);
+	struct udevice *bus = dev_get_parent(dev);
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
+	struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
 
-	free(cfslave);
+	return __spi_xfer(cfspi, plat->cs, bitlen, dout, din, flags);
 }
 
-int spi_claim_bus(struct spi_slave *slave)
+static int coldfire_spi_set_speed(struct udevice *bus, uint max_hz)
 {
-	return cfspi_claim_bus(slave->bus, slave->cs);
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
+	struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
+
+	return __spi_set_speed(cfspi, bus->seq, plat->mode);
 }
 
-void spi_release_bus(struct spi_slave *slave)
+static int coldfire_spi_set_mode(struct udevice *bus, uint mode)
 {
-	cfspi_release_bus(slave->bus, slave->cs);
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
+	struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
+
+	return __spi_set_mode(cfspi, bus->seq, plat->mode);
 }
 
-int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
-	     void *din, unsigned long flags)
+static int coldfire_spi_probe(struct udevice *bus)
+{
+	struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
+	struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
+
+	cfspi->baudrate = plat->speed_hz;
+
+	return 0;
+}
+
+static int coldfire_spi_ofdata_to_platdata(struct udevice *dev)
 {
-	return cfspi_xfer(slave, bitlen, dout, din, flags);
+        return -ENODEV;
 }
-#endif				/* CONFIG_CMD_SPI */
+
+static const struct udevice_id coldfire_spi_ids[] = {
+        { .compatible = "fsl,mcf-dspi" },
+        { }
+};
+
+
+static const struct dm_spi_ops coldfire_spi_ops = {
+	.claim_bus	= coldfire_spi_claim_bus,
+	.release_bus	= coldfire_spi_release_bus,
+	.xfer		= coldfire_spi_xfer,
+	.set_speed	= coldfire_spi_set_speed,
+	.set_mode	= coldfire_spi_set_mode,
+};
+
+U_BOOT_DRIVER(coldfire_spi) = {
+	.name = "spi_coldfire",
+	.id = UCLASS_SPI,
+	.of_match = coldfire_spi_ids,
+	.probe = coldfire_spi_probe,
+	.ops = &coldfire_spi_ops,
+	.priv_auto_alloc_size = sizeof(struct coldfire_spi_priv),
+	.platdata_auto_alloc_size = sizeof(struct coldfire_spi_platdata),
+};
+#endif				/* CONFIG_DM_SPI */
+#endif				/* CONFIG_CF_DSPI */
diff --git a/include/dm/platform_data/spi_coldfire.h b/include/dm/platform_data/spi_coldfire.h
new file mode 100644
index 0000000000..44094b428d
--- /dev/null
+++ b/include/dm/platform_data/spi_coldfire.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2015  Angelo Dureghello <angelo@sysam.it>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __spi_coldfire_h
+#define __spi_coldfire_h
+
+/*
+ * struct coldfire_spi_platdata - information about a coldfire spi module
+ *
+ * @speed_hz: SPI speed
+ */
+struct coldfire_spi_platdata {
+	uint speed_hz;
+	uint mode;
+	uint cs;
+};
+
+#endif /* __spi_coldfire_h */
+
-- 
2.17.0

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Jagan Teki]

On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> This patch adds DM support to cf_spi.c.
>
> How to use/test it:
>
> 1) enable the following options,
>
> CONFIG_DM_SPI
> CONFIG_DM_SPI_FLASH
>
> 2) add similar code into your board.c file
>
> U_BOOT_DEVICE(coldfire_spi) = {
>         .name = "spi_coldfire",
>         .platdata = &mcf_spi_plat,
> };
>
> ---
> Changes from v1:
> - split into 2 patches
>
> Changes from v2:
> - back in a single patch, no need to add fdt support or special
>   config options
> - doc/driver-model/spi-howto.txt has been re-checked and followed,
>   the driver now builds as is.

Nice, How about full dm conversion. I have seen few boards using this
driver and its quite manageble for full switching. what do you think?

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Angelo Dureghello]

Hi Jagan,

On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> > This patch adds DM support to cf_spi.c.
> >
> > How to use/test it:
> >
> > 1) enable the following options,
> >
> > CONFIG_DM_SPI
> > CONFIG_DM_SPI_FLASH
> >
> > 2) add similar code into your board.c file
> >
> > U_BOOT_DEVICE(coldfire_spi) = {
> >         .name = "spi_coldfire",
> >         .platdata = &mcf_spi_plat,
> > };
> >
> > ---
> > Changes from v1:
> > - split into 2 patches
> >
> > Changes from v2:
> > - back in a single patch, no need to add fdt support or special
> >   config options
> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
> >   the driver now builds as is.
> 
> Nice, How about full dm conversion. I have seen few boards using this
> driver and its quite manageble for full switching. what do you think?

Sorry, what do you mean exactly for full switching ?
My understanding is, to remove the non-dm part of the driver and add to
each board proper device struct and config options, correct ?
In this case, i can only test it on my stmark2 board, but i think could
be enough.

Regards,
Angelo

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Jagan Teki]

On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> Hi Jagan,
>
> On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
>> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> > This patch adds DM support to cf_spi.c.
>> >
>> > How to use/test it:
>> >
>> > 1) enable the following options,
>> >
>> > CONFIG_DM_SPI
>> > CONFIG_DM_SPI_FLASH
>> >
>> > 2) add similar code into your board.c file
>> >
>> > U_BOOT_DEVICE(coldfire_spi) = {
>> >         .name = "spi_coldfire",
>> >         .platdata = &mcf_spi_plat,
>> > };
>> >
>> > ---
>> > Changes from v1:
>> > - split into 2 patches
>> >
>> > Changes from v2:
>> > - back in a single patch, no need to add fdt support or special
>> >   config options
>> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
>> >   the driver now builds as is.
>>
>> Nice, How about full dm conversion. I have seen few boards using this
>> driver and its quite manageble for full switching. what do you think?
>
> Sorry, what do you mean exactly for full switching ?
> My understanding is, to remove the non-dm part of the driver and add to
> each board proper device struct and config options, correct ?

yes.

> In this case, i can only test it on my stmark2 board, but i think could
> be enough.

yes, for untested ones will ask other board maintainers.

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Angelo Dureghello]

Hi Jagan,

On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> > Hi Jagan,
> >
> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> >> > This patch adds DM support to cf_spi.c.
> >> >
> >> > How to use/test it:
> >> >
> >> > 1) enable the following options,
> >> >
> >> > CONFIG_DM_SPI
> >> > CONFIG_DM_SPI_FLASH
> >> >
> >> > 2) add similar code into your board.c file
> >> >
> >> > U_BOOT_DEVICE(coldfire_spi) = {
> >> >         .name = "spi_coldfire",
> >> >         .platdata = &mcf_spi_plat,
> >> > };
> >> >
> >> > ---
> >> > Changes from v1:
> >> > - split into 2 patches
> >> >
> >> > Changes from v2:
> >> > - back in a single patch, no need to add fdt support or special
> >> >   config options
> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
> >> >   the driver now builds as is.
> >>
> >> Nice, How about full dm conversion. I have seen few boards using this
> >> driver and its quite manageble for full switching. what do you think?
> >
> > Sorry, what do you mean exactly for full switching ?
> > My understanding is, to remove the non-dm part of the driver and add to
> > each board proper device struct and config options, correct ?
> 
> yes.
> 
> > In this case, i can only test it on my stmark2 board, but i think could
> > be enough.
> 
> yes, for untested ones will ask other board maintainers.

I verified right now, for a full switch i should enable CONFIG_DM for 
the architecture, and then select CONFIG_DM_SPI, moving as his child
the CONFIG_CF_SPI.

To enable CONFIG_DM for m68k means to add device tree, or there are 
of_xxx functions unresolved externals errors then.

For this small architecture, i am not that happy to add the devicetree
support. I should add a fixed 30 KB of binary size and several diffent
families/dtsi, with the benefit to configure mainly uart and spi.

If the devicetree become mandatory/forced, would be nice if you could accept
the migration as is (as from doc/driver-model/spi-howto.txt), and i could
then work on devicetree in a later step.

Let me know.

Best regards,
Angelo Dureghello

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Jagan Teki]

On Fri, Jun 29, 2018 at 1:18 AM, Angelo Dureghello <angelo@sysam.it> wrote:
> Hi Jagan,
>
> On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
>> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> > Hi Jagan,
>> >
>> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
>> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> >> > This patch adds DM support to cf_spi.c.
>> >> >
>> >> > How to use/test it:
>> >> >
>> >> > 1) enable the following options,
>> >> >
>> >> > CONFIG_DM_SPI
>> >> > CONFIG_DM_SPI_FLASH
>> >> >
>> >> > 2) add similar code into your board.c file
>> >> >
>> >> > U_BOOT_DEVICE(coldfire_spi) = {
>> >> >         .name = "spi_coldfire",
>> >> >         .platdata = &mcf_spi_plat,
>> >> > };
>> >> >
>> >> > ---
>> >> > Changes from v1:
>> >> > - split into 2 patches
>> >> >
>> >> > Changes from v2:
>> >> > - back in a single patch, no need to add fdt support or special
>> >> >   config options
>> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
>> >> >   the driver now builds as is.
>> >>
>> >> Nice, How about full dm conversion. I have seen few boards using this
>> >> driver and its quite manageble for full switching. what do you think?
>> >
>> > Sorry, what do you mean exactly for full switching ?
>> > My understanding is, to remove the non-dm part of the driver and add to
>> > each board proper device struct and config options, correct ?
>>
>> yes.
>>
>> > In this case, i can only test it on my stmark2 board, but i think could
>> > be enough.
>>
>> yes, for untested ones will ask other board maintainers.
>
> I verified right now, for a full switch i should enable CONFIG_DM for
> the architecture, and then select CONFIG_DM_SPI, moving as his child
> the CONFIG_CF_SPI.
>
> To enable CONFIG_DM for m68k means to add device tree, or there are
> of_xxx functions unresolved externals errors then.

Can you list me these funcs.

>
> For this small architecture, i am not that happy to add the devicetree
> support. I should add a fixed 30 KB of binary size and several diffent
> families/dtsi, with the benefit to configure mainly uart and spi.

I think we may rely some kind of platdata stuff for DT functions here,
if size is really matter with DT.

btw, you're patch seems checkpatch errors, check it the same?

total: 43 errors, 0 warnings, 0 checks, 607 lines checked

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Angelo Dureghello]

Hi Jagan,

On Tue, Aug 07, 2018 at 10:53:28AM +0530, Jagan Teki wrote:
> On Fri, Jun 29, 2018 at 1:18 AM, Angelo Dureghello <angelo@sysam.it> wrote:
> > Hi Jagan,
> >
> > On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
> >> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> >> > Hi Jagan,
> >> >
> >> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
> >> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> >> >> > This patch adds DM support to cf_spi.c.
> >> >> >
> >> >> > How to use/test it:
> >> >> >
> >> >> > 1) enable the following options,
> >> >> >
> >> >> > CONFIG_DM_SPI
> >> >> > CONFIG_DM_SPI_FLASH
> >> >> >
> >> >> > 2) add similar code into your board.c file
> >> >> >
> >> >> > U_BOOT_DEVICE(coldfire_spi) = {
> >> >> >         .name = "spi_coldfire",
> >> >> >         .platdata = &mcf_spi_plat,
> >> >> > };
> >> >> >
> >> >> > ---
> >> >> > Changes from v1:
> >> >> > - split into 2 patches
> >> >> >
> >> >> > Changes from v2:
> >> >> > - back in a single patch, no need to add fdt support or special
> >> >> >   config options
> >> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
> >> >> >   the driver now builds as is.
> >> >>
> >> >> Nice, How about full dm conversion. I have seen few boards using this
> >> >> driver and its quite manageble for full switching. what do you think?
> >> >
> >> > Sorry, what do you mean exactly for full switching ?
> >> > My understanding is, to remove the non-dm part of the driver and add to
> >> > each board proper device struct and config options, correct ?
> >>
> >> yes.
> >>
> >> > In this case, i can only test it on my stmark2 board, but i think could
> >> > be enough.
> >>
> >> yes, for untested ones will ask other board maintainers.
> >
> > I verified right now, for a full switch i should enable CONFIG_DM for
> > the architecture, and then select CONFIG_DM_SPI, moving as his child
> > the CONFIG_CF_SPI.
> >
> > To enable CONFIG_DM for m68k means to add device tree, or there are
> > of_xxx functions unresolved externals errors then.
> 
> Can you list me these funcs.
> 
Sure, i enabled CONFIG_DM in my stmark2_defconfig, getting

drivers/spi/built-in.o: In function `dev_read_u32_default':
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:467: undefined reference to `ofnode_read_u32_default'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:467: undefined reference to `ofnode_read_u32_default'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:467: undefined reference to `ofnode_read_u32_default'
drivers/spi/built-in.o: In function `dev_read_bool':
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:478: undefined reference to `ofnode_read_bool'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:478: undefined reference to `ofnode_read_bool'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:478: undefined reference to `ofnode_read_bool'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:478: undefined reference to `ofnode_read_bool'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:478: undefined reference to `ofnode_read_bool'
drivers/spi/built-in.o: In function `dev_read_u32_default':
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:467: undefined reference to `ofnode_read_u32_default'
/home/angelo/sysam/u-boot-coldfire/include/dm/read.h:467: undefined reference to `ofnode_read_u32_default'
drivers/spi/built-in.o:(.u_boot_list_2_uclass_2_spi+0x8): undefined reference to `dm_scan_fdt_dev'
make: *** [Makefile:1350: u-boot] Error 1

> >
> > For this small architecture, i am not that happy to add the devicetree
> > support. I should add a fixed 30 KB of binary size and several diffent
> > families/dtsi, with the benefit to configure mainly uart and spi.
> 
> I think we may rely some kind of platdata stuff for DT functions here,
> if size is really matter with DT.
> 
> btw, you're patch seems checkpatch errors, check it the same?
>
Sorry, not sure why, i generally alays check by checkpatch.pl.
Can fix them all in a v3 if you don't have other points.

Waiting for your feedbacks.
 
> total: 43 errors, 0 warnings, 0 checks, 607 lines checked

Regards,
Angelo Dureghello

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Simon Glass]

Hi Angelo,

On 28 June 2018 at 21:48, Angelo Dureghello <angelo@sysam.it> wrote:
> Hi Jagan,
>
> On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
>> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> > Hi Jagan,
>> >
>> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
>> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> >> > This patch adds DM support to cf_spi.c.
>> >> >
>> >> > How to use/test it:
>> >> >
>> >> > 1) enable the following options,
>> >> >
>> >> > CONFIG_DM_SPI
>> >> > CONFIG_DM_SPI_FLASH
>> >> >
>> >> > 2) add similar code into your board.c file
>> >> >
>> >> > U_BOOT_DEVICE(coldfire_spi) = {
>> >> >         .name = "spi_coldfire",
>> >> >         .platdata = &mcf_spi_plat,
>> >> > };
>> >> >
>> >> > ---
>> >> > Changes from v1:
>> >> > - split into 2 patches
>> >> >
>> >> > Changes from v2:
>> >> > - back in a single patch, no need to add fdt support or special
>> >> >   config options
>> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
>> >> >   the driver now builds as is.
>> >>
>> >> Nice, How about full dm conversion. I have seen few boards using this
>> >> driver and its quite manageble for full switching. what do you think?
>> >
>> > Sorry, what do you mean exactly for full switching ?
>> > My understanding is, to remove the non-dm part of the driver and add to
>> > each board proper device struct and config options, correct ?
>>
>> yes.
>>
>> > In this case, i can only test it on my stmark2 board, but i think could
>> > be enough.
>>
>> yes, for untested ones will ask other board maintainers.
>
> I verified right now, for a full switch i should enable CONFIG_DM for
> the architecture, and then select CONFIG_DM_SPI, moving as his child
> the CONFIG_CF_SPI.
>
> To enable CONFIG_DM for m68k means to add device tree, or there are
> of_xxx functions unresolved externals errors then.
>
> For this small architecture, i am not that happy to add the devicetree
> support. I should add a fixed 30 KB of binary size and several diffent
> families/dtsi, with the benefit to configure mainly uart and spi.

Are you worried about the size of SPL when using device tree?

How about converting m68k to DT in U-Boot proper as a first step?

>
> If the devicetree become mandatory/forced, would be nice if you could accept
> the migration as is (as from doc/driver-model/spi-howto.txt), and i could
> then work on devicetree in a later step.

Regards,
Simon

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Angelo Dureghello]

Hi Simon,

On Fri, Sep 14, 2018 at 12:17:56PM +0200, Simon Glass wrote:
> Hi Angelo,
> 
> On 28 June 2018 at 21:48, Angelo Dureghello <angelo@sysam.it> wrote:
> > Hi Jagan,
> >
> > On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
> >> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> >> > Hi Jagan,
> >> >
> >> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
> >> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> >> >> > This patch adds DM support to cf_spi.c.
> >> >> >
> >> >> > How to use/test it:
> >> >> >
> >> >> > 1) enable the following options,
> >> >> >
> >> >> > CONFIG_DM_SPI
> >> >> > CONFIG_DM_SPI_FLASH
> >> >> >
> >> >> > 2) add similar code into your board.c file
> >> >> >
> >> >> > U_BOOT_DEVICE(coldfire_spi) = {
> >> >> >         .name = "spi_coldfire",
> >> >> >         .platdata = &mcf_spi_plat,
> >> >> > };
> >> >> >
> >> >> > ---
> >> >> > Changes from v1:
> >> >> > - split into 2 patches
> >> >> >
> >> >> > Changes from v2:
> >> >> > - back in a single patch, no need to add fdt support or special
> >> >> >   config options
> >> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
> >> >> >   the driver now builds as is.
> >> >>
> >> >> Nice, How about full dm conversion. I have seen few boards using this
> >> >> driver and its quite manageble for full switching. what do you think?
> >> >
> >> > Sorry, what do you mean exactly for full switching ?
> >> > My understanding is, to remove the non-dm part of the driver and add to
> >> > each board proper device struct and config options, correct ?
> >>
> >> yes.
> >>
> >> > In this case, i can only test it on my stmark2 board, but i think could
> >> > be enough.
> >>
> >> yes, for untested ones will ask other board maintainers.
> >
> > I verified right now, for a full switch i should enable CONFIG_DM for
> > the architecture, and then select CONFIG_DM_SPI, moving as his child
> > the CONFIG_CF_SPI.
> >
> > To enable CONFIG_DM for m68k means to add device tree, or there are
> > of_xxx functions unresolved externals errors then.
> >
> > For this small architecture, i am not that happy to add the devicetree
> > support. I should add a fixed 30 KB of binary size and several diffent
> > families/dtsi, with the benefit to configure mainly uart and spi.
> 
> Are you worried about the size of SPL when using device tree?
>

ColdFire/m68k arch is not supporting SPL so this is not a problem.
 
> How about converting m68k to DT in U-Boot proper as a first step?
> 

It was a matter of deadlines, due to the request to switch cf_spi
driver to dm, i preferred to start executing this switch.

Ok, if the switch to DT is by popular demand from our team,
i can start the conversion. 

This patch could still be applied in the meantime, as you prefer.

> >
> > If the devicetree become mandatory/forced, would be nice if you could accept
> > the migration as is (as from doc/driver-model/spi-howto.txt), and i could
> > then work on devicetree in a later step.
> 
> Regards,
> Simon

Regards,
Angelo

[U-Boot] [PATCH v3] drivers: spi: migrate cf_spi to DM

[Jagan Teki]

On Sat, Sep 15, 2018 at 2:43 PM, Angelo Dureghello <angelo@sysam.it> wrote:
> Hi Simon,
>
> On Fri, Sep 14, 2018 at 12:17:56PM +0200, Simon Glass wrote:
>> Hi Angelo,
>>
>> On 28 June 2018 at 21:48, Angelo Dureghello <angelo@sysam.it> wrote:
>> > Hi Jagan,
>> >
>> > On Wed, Jun 27, 2018 at 02:54:42PM +0530, Jagan Teki wrote:
>> >> On Wed, Jun 27, 2018 at 2:27 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> >> > Hi Jagan,
>> >> >
>> >> > On Wed, Jun 27, 2018 at 12:08:26PM +0530, Jagan Teki wrote:
>> >> >> On Tue, Jun 26, 2018 at 10:28 PM, Angelo Dureghello <angelo@sysam.it> wrote:
>> >> >> > This patch adds DM support to cf_spi.c.
>> >> >> >
>> >> >> > How to use/test it:
>> >> >> >
>> >> >> > 1) enable the following options,
>> >> >> >
>> >> >> > CONFIG_DM_SPI
>> >> >> > CONFIG_DM_SPI_FLASH
>> >> >> >
>> >> >> > 2) add similar code into your board.c file
>> >> >> >
>> >> >> > U_BOOT_DEVICE(coldfire_spi) = {
>> >> >> >         .name = "spi_coldfire",
>> >> >> >         .platdata = &mcf_spi_plat,
>> >> >> > };
>> >> >> >
>> >> >> > ---
>> >> >> > Changes from v1:
>> >> >> > - split into 2 patches
>> >> >> >
>> >> >> > Changes from v2:
>> >> >> > - back in a single patch, no need to add fdt support or special
>> >> >> >   config options
>> >> >> > - doc/driver-model/spi-howto.txt has been re-checked and followed,
>> >> >> >   the driver now builds as is.
>> >> >>
>> >> >> Nice, How about full dm conversion. I have seen few boards using this
>> >> >> driver and its quite manageble for full switching. what do you think?
>> >> >
>> >> > Sorry, what do you mean exactly for full switching ?
>> >> > My understanding is, to remove the non-dm part of the driver and add to
>> >> > each board proper device struct and config options, correct ?
>> >>
>> >> yes.
>> >>
>> >> > In this case, i can only test it on my stmark2 board, but i think could
>> >> > be enough.
>> >>
>> >> yes, for untested ones will ask other board maintainers.
>> >
>> > I verified right now, for a full switch i should enable CONFIG_DM for
>> > the architecture, and then select CONFIG_DM_SPI, moving as his child
>> > the CONFIG_CF_SPI.
>> >
>> > To enable CONFIG_DM for m68k means to add device tree, or there are
>> > of_xxx functions unresolved externals errors then.
>> >
>> > For this small architecture, i am not that happy to add the devicetree
>> > support. I should add a fixed 30 KB of binary size and several diffent
>> > families/dtsi, with the benefit to configure mainly uart and spi.
>>
>> Are you worried about the size of SPL when using device tree?
>>
>
> ColdFire/m68k arch is not supporting SPL so this is not a problem.
>
>> How about converting m68k to DT in U-Boot proper as a first step?
>>
>
> It was a matter of deadlines, due to the request to switch cf_spi
> driver to dm, i preferred to start executing this switch.
>
> Ok, if the switch to DT is by popular demand from our team,
> i can start the conversion.
>
> This patch could still be applied in the meantime, as you prefer.

Better to send the full conversion along with DT-changes