[U-Boot] [PATCH] spi : convert altera_spi to driver model

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Thomas Chou]

Convert altera_spi to driver model

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/spi/Kconfig      |   8 ++
 drivers/spi/altera_spi.c | 197 ++++++++++++++++++++++++++---------------------
 2 files changed, 119 insertions(+), 86 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8e04fce..2f8cf19 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -15,6 +15,14 @@ config DM_SPI
 
 if DM_SPI
 
+config ALTERA_SPI
+	bool "Altera SPI driver"
+	help
+	  Enable the Altera SPI driver. This driver can be used to
+	  access the SPI NOR flash on platforms embedding this Altera
+	  IP core. Please find details on the "Embedded Peripherals IP
+	  User Guide" of Altera.
+
 config CADENCE_QSPI
 	bool "Cadence QSPI driver"
 	help
diff --git a/drivers/spi/altera_spi.c b/drivers/spi/altera_spi.c
index a4d03d9..ff3512a 100644
--- a/drivers/spi/altera_spi.c
+++ b/drivers/spi/altera_spi.c
@@ -8,18 +8,19 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 #include <common.h>
-#include <asm/io.h>
+#include <dm.h>
+#include <errno.h>
 #include <malloc.h>
 #include <spi.h>
+#include <fdtdec.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 #ifndef CONFIG_ALTERA_SPI_IDLE_VAL
 #define CONFIG_ALTERA_SPI_IDLE_VAL 0xff
 #endif
 
-#ifndef CONFIG_SYS_ALTERA_SPI_LIST
-#define CONFIG_SYS_ALTERA_SPI_LIST { CONFIG_SYS_SPI_BASE }
-#endif
-
 struct altera_spi_regs {
 	u32	rxdata;
 	u32	txdata;
@@ -29,102 +30,68 @@ struct altera_spi_regs {
 	u32	slave_sel;
 };
 
-#define ALTERA_SPI_STATUS_ROE_MSK	(1 << 3)
-#define ALTERA_SPI_STATUS_TOE_MSK	(1 << 4)
-#define ALTERA_SPI_STATUS_TMT_MSK	(1 << 5)
-#define ALTERA_SPI_STATUS_TRDY_MSK	(1 << 6)
-#define ALTERA_SPI_STATUS_RRDY_MSK	(1 << 7)
-#define ALTERA_SPI_STATUS_E_MSK		(1 << 8)
-
-#define ALTERA_SPI_CONTROL_IROE_MSK	(1 << 3)
-#define ALTERA_SPI_CONTROL_ITOE_MSK	(1 << 4)
-#define ALTERA_SPI_CONTROL_ITRDY_MSK	(1 << 6)
-#define ALTERA_SPI_CONTROL_IRRDY_MSK	(1 << 7)
-#define ALTERA_SPI_CONTROL_IE_MSK	(1 << 8)
-#define ALTERA_SPI_CONTROL_SSO_MSK	(1 << 10)
-
-static ulong altera_spi_base_list[] = CONFIG_SYS_ALTERA_SPI_LIST;
+struct altera_spi_platdata {
+	struct altera_spi_regs *regs;
+};
 
-struct altera_spi_slave {
-	struct spi_slave	slave;
-	struct altera_spi_regs	*regs;
+struct altera_spi_priv {
+	struct altera_spi_regs *regs;
 };
-#define to_altera_spi_slave(s) container_of(s, struct altera_spi_slave, slave)
 
-__weak int spi_cs_is_valid(unsigned int bus, unsigned int cs)
-{
-	return bus < ARRAY_SIZE(altera_spi_base_list) && cs < 32;
-}
+#define ALTERA_SPI_STATUS_RRDY_MSK	(1 << 7)
+#define ALTERA_SPI_CONTROL_SSO_MSK	(1 << 10)
 
-__weak void spi_cs_activate(struct spi_slave *slave)
+static void spi_cs_activate(struct udevice *dev, uint cs)
 {
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
-	writel(1 << slave->cs, &altspi->regs->slave_sel);
-	writel(ALTERA_SPI_CONTROL_SSO_MSK, &altspi->regs->control);
-}
+	struct udevice *bus = dev->parent;
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+	struct altera_spi_regs *const regs = priv->regs;
 
-__weak void spi_cs_deactivate(struct spi_slave *slave)
-{
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
-	writel(0, &altspi->regs->control);
-	writel(0, &altspi->regs->slave_sel);
+	writel(1 << cs, &regs->slave_sel);
+	writel(ALTERA_SPI_CONTROL_SSO_MSK, &regs->control);
 }
 
-void spi_init(void)
+static void spi_cs_deactivate(struct udevice *dev)
 {
-}
+	struct udevice *bus = dev->parent;
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+	struct altera_spi_regs *const regs = priv->regs;
 
-void spi_set_speed(struct spi_slave *slave, uint hz)
-{
-	/* altera spi core does not support programmable speed */
+	writel(0, &regs->control);
+	writel(0, &regs->slave_sel);
 }
 
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
-				  unsigned int max_hz, unsigned int mode)
+static int altera_spi_claim_bus(struct udevice *dev)
 {
-	struct altera_spi_slave *altspi;
-
-	if (!spi_cs_is_valid(bus, cs))
-		return NULL;
-
-	altspi = spi_alloc_slave(struct altera_spi_slave, bus, cs);
-	if (!altspi)
-		return NULL;
+	struct udevice *bus = dev->parent;
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+	struct altera_spi_regs *const regs = priv->regs;
 
-	altspi->regs = (struct altera_spi_regs *)altera_spi_base_list[bus];
-	debug("%s: bus:%i cs:%i base:%p\n", __func__, bus, cs, altspi->regs);
+	writel(0, &regs->control);
+	writel(0, &regs->slave_sel);
 
-	return &altspi->slave;
+	return 0;
 }
 
-void spi_free_slave(struct spi_slave *slave)
+static int altera_spi_release_bus(struct udevice *dev)
 {
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
-	free(altspi);
-}
+	struct udevice *bus = dev->parent;
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+	struct altera_spi_regs *const regs = priv->regs;
 
-int spi_claim_bus(struct spi_slave *slave)
-{
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
+	writel(0, &regs->slave_sel);
 
-	debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
-	writel(0, &altspi->regs->control);
-	writel(0, &altspi->regs->slave_sel);
 	return 0;
 }
 
-void spi_release_bus(struct spi_slave *slave)
+static int altera_spi_xfer(struct udevice *dev, unsigned int bitlen,
+			    const void *dout, void *din, unsigned long flags)
 {
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
+	struct udevice *bus = dev->parent;
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+	struct altera_spi_regs *const regs = priv->regs;
+	struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
 
-	debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
-	writel(0, &altspi->regs->slave_sel);
-}
-
-int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
-	     void *din, unsigned long flags)
-{
-	struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
 	/* assume spi core configured to do 8 bit transfers */
 	unsigned int bytes = bitlen / 8;
 	const unsigned char *txp = dout;
@@ -132,7 +99,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 	uint32_t reg, data, start;
 
 	debug("%s: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n", __func__,
-	      slave->bus, slave->cs, bitlen, bytes, flags);
+	      bus->seq, slave_plat->cs, bitlen, bytes, flags);
 
 	if (bitlen == 0)
 		goto done;
@@ -143,11 +110,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 	}
 
 	/* empty read buffer */
-	if (readl(&altspi->regs->status) & ALTERA_SPI_STATUS_RRDY_MSK)
-		readl(&altspi->regs->rxdata);
+	if (readl(&regs->status) & ALTERA_SPI_STATUS_RRDY_MSK)
+		readl(&regs->rxdata);
 
 	if (flags & SPI_XFER_BEGIN)
-		spi_cs_activate(slave);
+		spi_cs_activate(dev, slave_plat->cs);
 
 	while (bytes--) {
 		if (txp)
@@ -156,20 +123,20 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 			data = CONFIG_ALTERA_SPI_IDLE_VAL;
 
 		debug("%s: tx:%x ", __func__, data);
-		writel(data, &altspi->regs->txdata);
+		writel(data, &regs->txdata);
 
 		start = get_timer(0);
 		while (1) {
-			reg = readl(&altspi->regs->status);
+			reg = readl(&regs->status);
 			if (reg & ALTERA_SPI_STATUS_RRDY_MSK)
 				break;
 			if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
-				printf("%s: Transmission timed out!\n", __func__);
-				goto done;
+				debug("%s: Transmission timed out!\n", __func__);
+				return -1;
 			}
 		}
 
-		data = readl(&altspi->regs->rxdata);
+		data = readl(&regs->rxdata);
 		if (rxp)
 			*rxp++ = data & 0xff;
 
@@ -178,7 +145,65 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 
 done:
 	if (flags & SPI_XFER_END)
-		spi_cs_deactivate(slave);
+		spi_cs_deactivate(dev);
 
 	return 0;
 }
+
+static int altera_spi_set_speed(struct udevice *bus, uint speed)
+{
+	return 0;
+}
+
+static int altera_spi_set_mode(struct udevice *bus, uint mode)
+{
+	return 0;
+}
+
+static int altera_spi_probe(struct udevice *bus)
+{
+	struct altera_spi_platdata *plat = dev_get_platdata(bus);
+	struct altera_spi_priv *priv = dev_get_priv(bus);
+
+	priv->regs = plat->regs;
+
+	return 0;
+}
+
+static int altera_spi_ofdata_to_platdata(struct udevice *bus)
+{
+	struct altera_spi_platdata *plat = dev_get_platdata(bus);
+
+	plat->regs = ioremap(dev_get_addr(bus),
+		sizeof(struct altera_spi_regs));
+
+	return 0;
+}
+
+static const struct dm_spi_ops altera_spi_ops = {
+	.claim_bus	= altera_spi_claim_bus,
+	.release_bus	= altera_spi_release_bus,
+	.xfer		= altera_spi_xfer,
+	.set_speed	= altera_spi_set_speed,
+	.set_mode	= altera_spi_set_mode,
+	/*
+	 * cs_info is not needed, since we require all chip selects to be
+	 * in the device tree explicitly
+	 */
+};
+
+static const struct udevice_id altera_spi_ids[] = {
+	{ .compatible = "altr,spi-1.0", },
+	{ }
+};
+
+U_BOOT_DRIVER(altera_spi) = {
+	.name	= "altera_spi",
+	.id	= UCLASS_SPI,
+	.of_match = altera_spi_ids,
+	.ops	= &altera_spi_ops,
+	.ofdata_to_platdata = altera_spi_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct altera_spi_platdata),
+	.priv_auto_alloc_size = sizeof(struct altera_spi_priv),
+	.probe	= altera_spi_probe,
+};
-- 
2.1.4

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Jagan Teki]

On 21 September 2015 at 18:28, Thomas Chou <thomas@wytron.com.tw> wrote:
> Convert altera_spi to driver model

Thanks for the patch, Thomas.

>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>

Reviewed-by: Jagan Teki <jteki@openedev.com>

> ---
>  drivers/spi/Kconfig      |   8 ++
>  drivers/spi/altera_spi.c | 197 ++++++++++++++++++++++++++---------------------
>  2 files changed, 119 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 8e04fce..2f8cf19 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -15,6 +15,14 @@ config DM_SPI
>
>  if DM_SPI
>
> +config ALTERA_SPI
> +       bool "Altera SPI driver"
> +       help
> +         Enable the Altera SPI driver. This driver can be used to
> +         access the SPI NOR flash on platforms embedding this Altera
> +         IP core. Please find details on the "Embedded Peripherals IP
> +         User Guide" of Altera.
> +
>  config CADENCE_QSPI
>         bool "Cadence QSPI driver"
>         help
> diff --git a/drivers/spi/altera_spi.c b/drivers/spi/altera_spi.c
> index a4d03d9..ff3512a 100644
> --- a/drivers/spi/altera_spi.c
> +++ b/drivers/spi/altera_spi.c
> @@ -8,18 +8,19 @@
>   * SPDX-License-Identifier:    GPL-2.0+
>   */
>  #include <common.h>
> -#include <asm/io.h>
> +#include <dm.h>
> +#include <errno.h>
>  #include <malloc.h>
>  #include <spi.h>
> +#include <fdtdec.h>
> +#include <asm/io.h>

Just check it fdtdec.h and errno.h not required to add - part of common.h

> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>  #ifndef CONFIG_ALTERA_SPI_IDLE_VAL
>  #define CONFIG_ALTERA_SPI_IDLE_VAL 0xff
>  #endif
>
> -#ifndef CONFIG_SYS_ALTERA_SPI_LIST
> -#define CONFIG_SYS_ALTERA_SPI_LIST { CONFIG_SYS_SPI_BASE }
> -#endif
> -
>  struct altera_spi_regs {
>         u32     rxdata;
>         u32     txdata;
> @@ -29,102 +30,68 @@ struct altera_spi_regs {
>         u32     slave_sel;
>  };
>
> -#define ALTERA_SPI_STATUS_ROE_MSK      (1 << 3)
> -#define ALTERA_SPI_STATUS_TOE_MSK      (1 << 4)
> -#define ALTERA_SPI_STATUS_TMT_MSK      (1 << 5)
> -#define ALTERA_SPI_STATUS_TRDY_MSK     (1 << 6)
> -#define ALTERA_SPI_STATUS_RRDY_MSK     (1 << 7)
> -#define ALTERA_SPI_STATUS_E_MSK                (1 << 8)
> -
> -#define ALTERA_SPI_CONTROL_IROE_MSK    (1 << 3)
> -#define ALTERA_SPI_CONTROL_ITOE_MSK    (1 << 4)
> -#define ALTERA_SPI_CONTROL_ITRDY_MSK   (1 << 6)
> -#define ALTERA_SPI_CONTROL_IRRDY_MSK   (1 << 7)
> -#define ALTERA_SPI_CONTROL_IE_MSK      (1 << 8)
> -#define ALTERA_SPI_CONTROL_SSO_MSK     (1 << 10)
> -
> -static ulong altera_spi_base_list[] = CONFIG_SYS_ALTERA_SPI_LIST;
> +struct altera_spi_platdata {
> +       struct altera_spi_regs *regs;
> +};
>
> -struct altera_spi_slave {
> -       struct spi_slave        slave;
> -       struct altera_spi_regs  *regs;
> +struct altera_spi_priv {
> +       struct altera_spi_regs *regs;
>  };
> -#define to_altera_spi_slave(s) container_of(s, struct altera_spi_slave, slave)
>
> -__weak int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> -{
> -       return bus < ARRAY_SIZE(altera_spi_base_list) && cs < 32;
> -}
> +#define ALTERA_SPI_STATUS_RRDY_MSK     (1 << 7)
> +#define ALTERA_SPI_CONTROL_SSO_MSK     (1 << 10)
>
> -__weak void spi_cs_activate(struct spi_slave *slave)
> +static void spi_cs_activate(struct udevice *dev, uint cs)
>  {
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> -       writel(1 << slave->cs, &altspi->regs->slave_sel);
> -       writel(ALTERA_SPI_CONTROL_SSO_MSK, &altspi->regs->control);
> -}
> +       struct udevice *bus = dev->parent;
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +       struct altera_spi_regs *const regs = priv->regs;
>
> -__weak void spi_cs_deactivate(struct spi_slave *slave)
> -{
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> -       writel(0, &altspi->regs->control);
> -       writel(0, &altspi->regs->slave_sel);
> +       writel(1 << cs, &regs->slave_sel);
> +       writel(ALTERA_SPI_CONTROL_SSO_MSK, &regs->control);
>  }
>
> -void spi_init(void)
> +static void spi_cs_deactivate(struct udevice *dev)
>  {
> -}
> +       struct udevice *bus = dev->parent;
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +       struct altera_spi_regs *const regs = priv->regs;
>
> -void spi_set_speed(struct spi_slave *slave, uint hz)
> -{
> -       /* altera spi core does not support programmable speed */
> +       writel(0, &regs->control);
> +       writel(0, &regs->slave_sel);
>  }
>
> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> -                                 unsigned int max_hz, unsigned int mode)
> +static int altera_spi_claim_bus(struct udevice *dev)
>  {
> -       struct altera_spi_slave *altspi;
> -
> -       if (!spi_cs_is_valid(bus, cs))
> -               return NULL;
> -
> -       altspi = spi_alloc_slave(struct altera_spi_slave, bus, cs);
> -       if (!altspi)
> -               return NULL;
> +       struct udevice *bus = dev->parent;
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +       struct altera_spi_regs *const regs = priv->regs;
>
> -       altspi->regs = (struct altera_spi_regs *)altera_spi_base_list[bus];
> -       debug("%s: bus:%i cs:%i base:%p\n", __func__, bus, cs, altspi->regs);
> +       writel(0, &regs->control);
> +       writel(0, &regs->slave_sel);
>
> -       return &altspi->slave;
> +       return 0;
>  }
>
> -void spi_free_slave(struct spi_slave *slave)
> +static int altera_spi_release_bus(struct udevice *dev)
>  {
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> -       free(altspi);
> -}
> +       struct udevice *bus = dev->parent;
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +       struct altera_spi_regs *const regs = priv->regs;
>
> -int spi_claim_bus(struct spi_slave *slave)
> -{
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> +       writel(0, &regs->slave_sel);
>
> -       debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> -       writel(0, &altspi->regs->control);
> -       writel(0, &altspi->regs->slave_sel);
>         return 0;
>  }
>
> -void spi_release_bus(struct spi_slave *slave)
> +static int altera_spi_xfer(struct udevice *dev, unsigned int bitlen,
> +                           const void *dout, void *din, unsigned long flags)
>  {
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> +       struct udevice *bus = dev->parent;
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +       struct altera_spi_regs *const regs = priv->regs;
> +       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
>
> -       debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> -       writel(0, &altspi->regs->slave_sel);
> -}
> -
> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> -            void *din, unsigned long flags)
> -{
> -       struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
>         /* assume spi core configured to do 8 bit transfers */
>         unsigned int bytes = bitlen / 8;
>         const unsigned char *txp = dout;
> @@ -132,7 +99,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>         uint32_t reg, data, start;
>
>         debug("%s: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n", __func__,
> -             slave->bus, slave->cs, bitlen, bytes, flags);
> +             bus->seq, slave_plat->cs, bitlen, bytes, flags);
>
>         if (bitlen == 0)
>                 goto done;
> @@ -143,11 +110,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>         }
>
>         /* empty read buffer */
> -       if (readl(&altspi->regs->status) & ALTERA_SPI_STATUS_RRDY_MSK)
> -               readl(&altspi->regs->rxdata);
> +       if (readl(&regs->status) & ALTERA_SPI_STATUS_RRDY_MSK)
> +               readl(&regs->rxdata);
>
>         if (flags & SPI_XFER_BEGIN)
> -               spi_cs_activate(slave);
> +               spi_cs_activate(dev, slave_plat->cs);
>
>         while (bytes--) {
>                 if (txp)
> @@ -156,20 +123,20 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>                         data = CONFIG_ALTERA_SPI_IDLE_VAL;
>
>                 debug("%s: tx:%x ", __func__, data);
> -               writel(data, &altspi->regs->txdata);
> +               writel(data, &regs->txdata);
>
>                 start = get_timer(0);
>                 while (1) {
> -                       reg = readl(&altspi->regs->status);
> +                       reg = readl(&regs->status);
>                         if (reg & ALTERA_SPI_STATUS_RRDY_MSK)
>                                 break;
>                         if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) {
> -                               printf("%s: Transmission timed out!\n", __func__);
> -                               goto done;
> +                               debug("%s: Transmission timed out!\n", __func__);
> +                               return -1;
>                         }
>                 }
>
> -               data = readl(&altspi->regs->rxdata);
> +               data = readl(&regs->rxdata);
>                 if (rxp)
>                         *rxp++ = data & 0xff;
>
> @@ -178,7 +145,65 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>
>  done:
>         if (flags & SPI_XFER_END)
> -               spi_cs_deactivate(slave);
> +               spi_cs_deactivate(dev);
>
>         return 0;
>  }
> +
> +static int altera_spi_set_speed(struct udevice *bus, uint speed)
> +{
> +       return 0;
> +}
> +
> +static int altera_spi_set_mode(struct udevice *bus, uint mode)
> +{
> +       return 0;
> +}
> +
> +static int altera_spi_probe(struct udevice *bus)
> +{
> +       struct altera_spi_platdata *plat = dev_get_platdata(bus);
> +       struct altera_spi_priv *priv = dev_get_priv(bus);
> +
> +       priv->regs = plat->regs;
> +
> +       return 0;
> +}
> +
> +static int altera_spi_ofdata_to_platdata(struct udevice *bus)
> +{
> +       struct altera_spi_platdata *plat = dev_get_platdata(bus);
> +
> +       plat->regs = ioremap(dev_get_addr(bus),
> +               sizeof(struct altera_spi_regs));
> +
> +       return 0;
> +}
> +
> +static const struct dm_spi_ops altera_spi_ops = {
> +       .claim_bus      = altera_spi_claim_bus,
> +       .release_bus    = altera_spi_release_bus,
> +       .xfer           = altera_spi_xfer,
> +       .set_speed      = altera_spi_set_speed,
> +       .set_mode       = altera_spi_set_mode,
> +       /*
> +        * cs_info is not needed, since we require all chip selects to be
> +        * in the device tree explicitly
> +        */
> +};
> +
> +static const struct udevice_id altera_spi_ids[] = {
> +       { .compatible = "altr,spi-1.0", },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(altera_spi) = {
> +       .name   = "altera_spi",
> +       .id     = UCLASS_SPI,
> +       .of_match = altera_spi_ids,
> +       .ops    = &altera_spi_ops,
> +       .ofdata_to_platdata = altera_spi_ofdata_to_platdata,
> +       .platdata_auto_alloc_size = sizeof(struct altera_spi_platdata),
> +       .priv_auto_alloc_size = sizeof(struct altera_spi_priv),
> +       .probe  = altera_spi_probe,
> +};
> --
> 2.1.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Marek Vasut]

On Monday, September 21, 2015 at 02:58:31 PM, Thomas Chou wrote:
> Convert altera_spi to driver model
> 
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>

Hi!

> ---
>  drivers/spi/Kconfig      |   8 ++
>  drivers/spi/altera_spi.c | 197
> ++++++++++++++++++++++++++--------------------- 2 files changed, 119
> insertions(+), 86 deletions(-)

[...]

> +
> +static int altera_spi_set_speed(struct udevice *bus, uint speed)
> +{
> +	return 0;
> +}
> +
> +static int altera_spi_set_mode(struct udevice *bus, uint mode)
> +{
> +	return 0;
> +}

I suspect you might want to tweak the core code to check if those
.set_speed and .set_mode are assigned in dm_spi_ops structure and
if not, don't call them.

> +static int altera_spi_probe(struct udevice *bus)
> +{
> +	struct altera_spi_platdata *plat = dev_get_platdata(bus);
> +	struct altera_spi_priv *priv = dev_get_priv(bus);
> +
> +	priv->regs = plat->regs;
> +
> +	return 0;
> +}
> +
> +static int altera_spi_ofdata_to_platdata(struct udevice *bus)
> +{
> +	struct altera_spi_platdata *plat = dev_get_platdata(bus);
> +
> +	plat->regs = ioremap(dev_get_addr(bus),
> +		sizeof(struct altera_spi_regs));

I guess the same thing about ioremap() and ranges applies here?

> +	return 0;
> +}

But all these are minor nits, thanks !

[...]

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Thomas Chou]

Hi Marek,

On 09/22/2015 08:10 AM, Marek Vasut wrote:
> I suspect you might want to tweak the core code to check if those
> .set_speed and .set_mode are assigned in dm_spi_ops structure and
> if not, don't call them.

I wonder if this worths, as altera_spi is the only one that cannot set 
speed and mode.

>> +	plat->regs = ioremap(dev_get_addr(bus),
>> +		sizeof(struct altera_spi_regs));
>
> I guess the same thing about ioremap() and ranges applies here?
>

Same thing about ioremap(), as almost every other arch does not include 
it in u-boot. You see that I just added it to nios2 lately.

Sometimes the address returned by dev_get_addr() may be used as memory, 
and does not need ioremap.

Best regards,
Thomas

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Marek Vasut]

On Tuesday, September 22, 2015 at 05:52:58 AM, Thomas Chou wrote:
> Hi Marek,

Hi,

> On 09/22/2015 08:10 AM, Marek Vasut wrote:
> > I suspect you might want to tweak the core code to check if those
> > .set_speed and .set_mode are assigned in dm_spi_ops structure and
> > if not, don't call them.
> 
> I wonder if this worths, as altera_spi is the only one that cannot set
> speed and mode.

I'm expecting that EPCQ Wundercontroller to have the same problem (see [1]
section 42, Altera GQSPI). Altera tried to push this into mainline Linux
even [2], without much success, in particular because they duplicated half
of the spi-nor stack to do it. I suspect this driver is coming to U-Boot
sooner or later.

But maybe we should do this change to the SPI NOR stuff once this driver
hits the U-Boot ML instead of preemptively adding such checks. What do you
think ?

[1] https://www.altera.com/content/dam/altera-
www/global/en_US/pdfs/literature/ug/ug_embedded_ip.pdf
[2] https://lwn.net/Articles/636882/

> >> +	plat->regs = ioremap(dev_get_addr(bus),
> >> +		sizeof(struct altera_spi_regs));
> > 
> > I guess the same thing about ioremap() and ranges applies here?
> 
> Same thing about ioremap(), as almost every other arch does not include
> it in u-boot. You see that I just added it to nios2 lately.
> 
> Sometimes the address returned by dev_get_addr() may be used as memory,
> and does not need ioremap.

Gotcha.

Best regards,
Marek Vasut

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Thomas Chou]

Hi Marek,

On 09/22/2015 12:13 PM, Marek Vasut wrote:
> I'm expecting that EPCQ Wundercontroller to have the same problem (see [1]
> section 42, Altera GQSPI). Altera tried to push this into mainline Linux
> even [2], without much success, in particular because they duplicated half
> of the spi-nor stack to do it. I suspect this driver is coming to U-Boot
> sooner or later.
>
> But maybe we should do this change to the SPI NOR stuff once this driver
> hits the U-Boot ML instead of preemptively adding such checks. What do you
> think ?

Just sent the patch to skip the checks as separated.
[PATCH] spi: ignore set speed and mode if not available

Best regards,
Thomas

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Marek Vasut]

On Tuesday, September 22, 2015 at 09:45:22 AM, Thomas Chou wrote:
> Hi Marek,

Hi!

> On 09/22/2015 12:13 PM, Marek Vasut wrote:
> > I'm expecting that EPCQ Wundercontroller to have the same problem (see
> > [1] section 42, Altera GQSPI). Altera tried to push this into mainline
> > Linux even [2], without much success, in particular because they
> > duplicated half of the spi-nor stack to do it. I suspect this driver is
> > coming to U-Boot sooner or later.
> > 
> > But maybe we should do this change to the SPI NOR stuff once this driver
> > hits the U-Boot ML instead of preemptively adding such checks. What do
> > you think ?
> 
> Just sent the patch to skip the checks as separated.
> [PATCH] spi: ignore set speed and mode if not available

Thanks :)

Best regards,
Marek Vasut

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Thomas Chou]

Hi Jagan,

On 09/22/2015 02:01 AM, Jagan Teki wrote:
> On 21 September 2015 at 18:28, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Convert altera_spi to driver model
>
> Thanks for the patch, Thomas.
>
>>
>> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
>
> Reviewed-by: Jagan Teki <jteki@openedev.com>

May I pick up this patch? As a custodian of the nios arch, I would like 
to include it to the series of patches that convert all nios2 related 
drivers to driver model and device tree control, which will be after 
v2015.10. Thank you in advance.

Best regards,
Thomas Chou

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Jagan Teki]

On 28 September 2015 at 06:10, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Jagan,
>
> On 09/22/2015 02:01 AM, Jagan Teki wrote:
>>
>> On 21 September 2015 at 18:28, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>
>>> Convert altera_spi to driver model
>>
>>
>> Thanks for the patch, Thomas.
>>
>>>
>>> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
>>
>>
>> Reviewed-by: Jagan Teki <jteki@openedev.com>
>
>
> May I pick up this patch? As a custodian of the nios arch, I would like to
> include it to the series of patches that convert all nios2 related drivers
> to driver model and device tree control, which will be after v2015.10. Thank
> you in advance.

Please pick.

-- Jagan.

[U-Boot] [PATCH] spi : convert altera_spi to driver model

[Thomas Chou]

On 09/21/2015 08:58 PM, Thomas Chou wrote:
> Convert altera_spi to driver model
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>   drivers/spi/Kconfig      |   8 ++
>   drivers/spi/altera_spi.c | 197 ++++++++++++++++++++++++++---------------------
>   2 files changed, 119 insertions(+), 86 deletions(-)

Applied to u-boot-nios.