From mboxrd@z Thu Jan 1 00:00:00 1970 From: Angelo Dureghello Date: Wed, 26 Sep 2018 20:53:35 +0200 Subject: [U-Boot] [PATCH 2/7] drivers: spi: cf_spi: migrate to DM and DT In-Reply-To: References: <20180920210755.22381-1-angelo@sysam.it> <20180920210755.22381-3-angelo@sysam.it> Message-ID: <20180926185335.GA4492@jerusalem> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, thanks for the review. On Tue, Sep 25, 2018 at 10:42:08PM -0700, Simon Glass wrote: > Hi Angelo, > > On 20 September 2018 at 15:07, Angelo Dureghello 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 > > --- > > 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. > I just respected Kconfig alphabetical order, SOFT_SPI is just moved after. > > - > > 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 > > + * > > */ > > > > #include > > +#include > > +#include > > #include > > #include > > #include > > +#include > > > > -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. > These are just default settings for each channel (bus), actually coming from the include/configs/boardxxx.h. Their speed an mode bitfields are rewritten later, with values coming from devicetree. Some driver #define the default value inside the driver itself, in case i may change in this way. No one seems reading them from device tree. > > #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 Regards, Angelo