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

[Simon Glass]

Hi Angelo,

On 20 September 2018 at 15:07, Angelo Dureghello <angelo at 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 at 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 at 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