[U-Boot] [PATCH 2/7] drivers: spi: cf_spi: migrate to DM and DT

Simon Glass
2018-09-26T05:42:08Z

Hi Angelo,

On 20 September 2018 at 15:07, Angelo Dureghello <angelo@sysam.it> wrote:
> This patch converts cf_spi.c to DM and to read driver
> platform data from flat devicetree.
>
> ---
> Changes from v1:
> - split into 2 patches
>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> drivers/spi/Kconfig | 18 +-
> drivers/spi/cf_spi.c | 495 ++++++++++++++++--------
> include/dm/platform_data/spi_coldfire.h | 25 ++
> 3 files changed, 369 insertions(+), 169 deletions(-)
> create mode 100644 include/dm/platform_data/spi_coldfire.h
>

Good to see this.

> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index dcd719ff0a..974c5bbed8 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -80,6 +80,12 @@ config CADENCE_QSPI
> used to access the SPI NOR flash on platforms embedding this
> Cadence IP core.
>
> +config CF_SPI
> + bool "ColdFire SPI driver"
> + help
> + Enable the ColdFire SPI driver. This driver can be used on
> + some m68k SoCs.
> +
> config DESIGNWARE_SPI
> bool "Designware SPI driver"
> help
> @@ -244,18 +250,18 @@ config ZYNQMP_GQSPI
>
> endif # if DM_SPI
>
> -config SOFT_SPI
> - bool "Soft SPI driver"
> - help
> - Enable Soft SPI driver. This driver is to use GPIO simulate
> - the SPI protocol.

How come this is changing? That should be a separate patch.

> -
> config CF_SPI
> bool "ColdFire SPI driver"
> help
> Enable the ColdFire SPI driver. This driver can be used on
> some m68k SoCs.
>
> +config SOFT_SPI
> + bool "Soft SPI driver"
> + help
> + Enable Soft SPI driver. This driver is to use GPIO simulate
> + the SPI protocol.
> +
> config FSL_ESPI
> bool "Freescale eSPI driver"
> help
> diff --git a/drivers/spi/cf_spi.c b/drivers/spi/cf_spi.c
> index 522631cbbf..11a11f79c4 100644
> --- a/drivers/spi/cf_spi.c
> +++ b/drivers/spi/cf_spi.c
> @@ -6,16 +6,27 @@
> *
> * 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>
> + *
> */
>
> #include <common.h>
> +#include <dm.h>
> +#include <dm/platform_data/spi_coldfire.h>
> #include <spi.h>
> #include <malloc.h>
> #include <asm/immap.h>
> +#include <asm/io.h>
>
> -struct cf_spi_slave {
> +struct coldfire_spi_priv {
> +#ifndef CONFIG_DM_SPI
> struct spi_slave slave;
> +#endif
> + struct dspi *regs;
> uint baudrate;
> + int mode;
> int charbit;
> };
>
> @@ -38,14 +49,14 @@ 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);
> -}
> +/* Default values */
> +#define MCF_DSPI_DEFAULT_SCK_FREQ 10000000
> +#define MCF_DSPI_MAX_CHIPSELECTS 4
> +#define MCF_DSPI_MODE 0
>
> -static void cfspi_init(void)
> +static void __spi_init(struct coldfire_spi_priv *cfspi)
> {
> - volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
> + struct dspi *dspi = cfspi->regs;
>
> cfspi_port_conf(); /* port configuration */
>
> @@ -56,125 +67,32 @@ static void cfspi_init(void)
>
> /* Default setting in platform configuration */
> #ifdef CONFIG_SYS_DSPI_CTAR0
> - dspi->ctar[0] = CONFIG_SYS_DSPI_CTAR0;
> + writel(CONFIG_SYS_DSPI_CTAR0, &dspi->ctar[0]);

What is going on here? I think these CONFIG options are addresses? If
so, they should be read from the DT, not a CONFIG.

> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR1
> - dspi->ctar[1] = CONFIG_SYS_DSPI_CTAR1;
> + writel(CONFIG_SYS_DSPI_CTAR1, &dspi->ctar[1]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR2
> - dspi->ctar[2] = CONFIG_SYS_DSPI_CTAR2;
> + writel(CONFIG_SYS_DSPI_CTAR2, &dspi->ctar[2]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR3
> - dspi->ctar[3] = CONFIG_SYS_DSPI_CTAR3;
> + writel(CONFIG_SYS_DSPI_CTAR3, &dspi->ctar[3]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR4
> - dspi->ctar[4] = CONFIG_SYS_DSPI_CTAR4;
> + writel(CONFIG_SYS_DSPI_CTAR4, &dspi->ctar[4]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR5
> - dspi->ctar[5] = CONFIG_SYS_DSPI_CTAR5;
> + writel(CONFIG_SYS_DSPI_CTAR5, &dspi->ctar[5]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR6
> - dspi->ctar[6] = CONFIG_SYS_DSPI_CTAR6;
> + writel(CONFIG_SYS_DSPI_CTAR6, &dspi->ctar[6]);
> #endif
> #ifdef CONFIG_SYS_DSPI_CTAR7
> - dspi->ctar[7] = CONFIG_SYS_DSPI_CTAR7;
> + writel(CONFIG_SYS_DSPI_CTAR7, &dspi->ctar[7]);
> #endif
> }
>

[..]

Regards,
Simon