From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 2 Oct 2018 04:21:59 -0700 Subject: [U-Boot] [PATCH 2/7] drivers: spi: cf_spi: migrate to DM and DT In-Reply-To: <20180928112250.GB3830@jerusalem> References: <20180920210755.22381-1-angelo@sysam.it> <20180920210755.22381-3-angelo@sysam.it> <20180926185335.GA4492@jerusalem> <20180928112250.GB3830@jerusalem> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Angelo, On 28 September 2018 at 04:22, Angelo Dureghello wrote: > Hi Simon, > > On Thu, Sep 27, 2018 at 06:41:37AM -0700, Simon Glass wrote: >> Hi Angelo, >> >> On 26 September 2018 at 11:53, Angelo Dureghello wrote: >> > 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. >> >> OK, well I do think this should be in a separate patch. >> > this is done, ready into a v2 > >> > >> >> > - >> >> > 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. >> >> OK, can we remove these? At least they should not have a CONFIG_ >> prefix, so we can remove them from the whitelist. >> > > I verified that in particular 1 m68k board (ls1012aqds.h) wants different > defaults as cs-clock delays. This is something that atually can only be > done by those CONFIG_SYS_DSPI_CTARX. > > This settings may be moved into DT but all the related boards should have > been moved to use a dts. Not sure if i can do this now, since i cannot > test DT migration without owning the related physical board (hw). > > How does it work in general ? Should i move al boards to dts, leaving > tests to board maintaners in the future ? Can we keep those > CONFIG_SYS_DSPI_CTARX in this way and perform the all-boards conversion > to dts in a later step ? Yes you can migrate them forcibly since the alternative is presumably to delete their support from mainline. Regards, Simon