Hi Angelo,
On 26 September 2018 at 11:53, Angelo Dureghello angelo@sysam.it 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 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"helpEnable the ColdFire SPI driver. This driver can be used onsome 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"helpEnable Soft SPI driver. This driver is to use GPIO simulatethe 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.
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"helpEnable Soft SPI driver. This driver is to use GPIO simulatethe 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@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.
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.
Regards, Simon