[U-Boot] [PATCH v4 0/1] Read default speed and mode values from DT

This patch-set finish the previous serie: http://patchwork.ozlabs.org/project/uboot/list/?series=76834
and also replace the serie: "Remove defines for SPI default speed and mode " http://patchwork.ozlabs.org/project/uboot/list/?series=80769&state=*
This new version is a complete rework after several remarks and regression for these 2 patchset, mainly when SPI node configuration is not in DT, when platdata is used.
I take some time to check the code and this proposal is a rework of commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
This patch avoid spi_get_bus_and_cs() callers to force value of parameter speed to 0 for some board (without DT node) and regression for other board (as i was the case in my previous Series V2).
I see this kind of problem in file env/sf.c with first commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT") and come-back to the previous behavior with CONFIG_ in the commit 25a17652c9c2 ("fix: env: Fix the SPI flash device setup for DM mode")
So I decide to don't remove the defines used for default value of speed and mode but they are no more used when platdata is available.
That avoid regression when spi_get_bus_and_cs is directly called.
NB: I don't sure that the behavior of 'compatibility' function spi_setup_slave() will be still is still the correct on (in some case speed and mode will parameter will be not used).
Tested on my board only for the boot from SPI NOR so when spl_spi_load_image() is called in SPL.
Changes in v4: - rebase on v2019.04-rc2 - after Jagan review depends on Migrate SPI defines to Kconfig http://patchwork.ozlabs.org/project/uboot/list/?series=94485 - remove no more supported configuration: CONFIG_ENV_SPI_MAX_HZ=0 or CONFIG_ENV_SPI_MODE=0
Changes in v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
Patrick Delaunay (1): dm: spi: Read default speed and mode values from DT
cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ configs/da850_am18xxevm_defconfig | 4 ---- configs/da850evm_defconfig | 4 ---- configs/mscc_jr2_defconfig | 4 ---- configs/mscc_luton_defconfig | 4 ---- configs/mscc_ocelot_defconfig | 4 ---- configs/mscc_serval_defconfig | 4 ---- configs/mscc_servalt_defconfig | 4 ---- drivers/mtd/spi/Kconfig | 6 ++++++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 12 files changed, 17 insertions(+), 35 deletions(-)

This patch update the behavior introduced by commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
In case of DT boot, don't read default speed and mode for SPI from CONFIG_* but instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and SPI modes.
Remove also use in boards of the value speed=0 (no more supported) for ENV in SPI by using CONFIG_ENV_SPI_MAX_HZ=0.
DT values will be always used when available (full DM support of SPI slave with available DT node) even if speed and mode are requested; for example in splash screen support (in splash_sf_read_raw) or in SPL boot (in spl_spi_load_image). The caller of spi_get_bus_and_cs() no more need to force speed=0.
But the current behavior don't change if the SPI slave is not present (device with generic driver is created automatically) or if platdata is used (CONFIG_OF_PLATDATA).
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v4: - rebase on v2019.04-rc2 - after Jagan review depends on Migrate SPI defines to Kconfig http://patchwork.ozlabs.org/project/uboot/list/?series=94485 - remove no more supported configuration: CONFIG_ENV_SPI_MAX_HZ=0 or CONFIG_ENV_SPI_MODE=0
Changes in v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ configs/da850_am18xxevm_defconfig | 4 ---- configs/da850evm_defconfig | 4 ---- configs/mscc_jr2_defconfig | 4 ---- configs/mscc_luton_defconfig | 4 ---- configs/mscc_ocelot_defconfig | 4 ---- configs/mscc_serval_defconfig | 4 ---- configs/mscc_servalt_defconfig | 4 ---- drivers/mtd/spi/Kconfig | 6 ++++++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 12 files changed, 17 insertions(+), 35 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 738ef0e..6ccf98a 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,14 +81,13 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { unsigned int bus = CONFIG_SF_DEFAULT_BUS; unsigned int cs = CONFIG_SF_DEFAULT_CS; + /* In DM mode, defaults speed and mode will be taken from DT */ unsigned int speed = CONFIG_SF_DEFAULT_SPEED; unsigned int mode = CONFIG_SF_DEFAULT_MODE; char *endp; #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret; - /* In DM mode defaults will be taken from DT */ - speed = 0, mode = 0; #else struct spi_flash *new; #endif diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index 8cd4830..9b74473 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -77,6 +77,8 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
/* * Load U-Boot image from SPI flash into RAM + * In DM mode: defaults speed and mode will be + * taken from DT when available */
flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, diff --git a/configs/da850_am18xxevm_defconfig b/configs/da850_am18xxevm_defconfig index b1cf9bd..58745fe 100644 --- a/configs/da850_am18xxevm_defconfig +++ b/configs/da850_am18xxevm_defconfig @@ -38,10 +38,6 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="da850-evm" CONFIG_SPL_OF_PLATDATA=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_DM=y CONFIG_SPL_DM=y CONFIG_DA8XX_GPIO=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index e6031b4..5100ee7 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -40,10 +40,6 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="da850-evm" CONFIG_SPL_OF_PLATDATA=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_DM=y CONFIG_SPL_DM=y CONFIG_DM_GPIO=y diff --git a/configs/mscc_jr2_defconfig b/configs/mscc_jr2_defconfig index 95562b7..9276df2 100644 --- a/configs/mscc_jr2_defconfig +++ b/configs/mscc_jr2_defconfig @@ -38,10 +38,6 @@ CONFIG_OF_LIST="jr2_pcb110 jr2_pcb111 serval2_pcb112" CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y CONFIG_DM_GPIO=y diff --git a/configs/mscc_luton_defconfig b/configs/mscc_luton_defconfig index 162a514..0fdd9b8 100644 --- a/configs/mscc_luton_defconfig +++ b/configs/mscc_luton_defconfig @@ -44,10 +44,6 @@ CONFIG_OF_LIST="luton_pcb090 luton_pcb091" CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y CONFIG_DM_GPIO=y diff --git a/configs/mscc_ocelot_defconfig b/configs/mscc_ocelot_defconfig index b0dcfaf..edc476d 100644 --- a/configs/mscc_ocelot_defconfig +++ b/configs/mscc_ocelot_defconfig @@ -43,10 +43,6 @@ CONFIG_OF_LIST="ocelot_pcb120 ocelot_pcb123" CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y CONFIG_DM_GPIO=y diff --git a/configs/mscc_serval_defconfig b/configs/mscc_serval_defconfig index f2c9563..146188b 100644 --- a/configs/mscc_serval_defconfig +++ b/configs/mscc_serval_defconfig @@ -35,10 +35,6 @@ CONFIG_OF_LIST="serval_pcb106 serval_pcb105" CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y CONFIG_DM_GPIO=y diff --git a/configs/mscc_servalt_defconfig b/configs/mscc_servalt_defconfig index 027aaa4..a450f48 100644 --- a/configs/mscc_servalt_defconfig +++ b/configs/mscc_servalt_defconfig @@ -33,10 +33,6 @@ CONFIG_DEFAULT_DEVICE_TREE="servalt_pcb116" CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_IS_IN_SPI_FLASH=y -CONFIG_USE_ENV_SPI_MAX_HZ=y -CONFIG_ENV_SPI_MAX_HZ=0 -CONFIG_USE_ENV_SPI_MODE=y -CONFIG_ENV_SPI_MODE=0 CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y CONFIG_DM_GPIO=y diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 5671bca..d3b007a 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -62,6 +62,9 @@ config SF_DEFAULT_MODE The default mode may be provided by the platform to handle the common case when only a single serial flash is present on the system. + Not used for boot with device tree; the SPI driver reads + speed and mode from platdata values computed from + available node.
config SF_DEFAULT_SPEED int "SPI Flash default speed in Hz" @@ -71,6 +74,9 @@ config SF_DEFAULT_SPEED The default speed may be provided by the platform to handle the common case when only a single serial flash is present on the system. + Not used for boot with device tree; the SPI driver reads + speed and mode from platdata values computed from + available node.
if SPI_FLASH
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 2bc289a..88cb2a1 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -328,7 +328,9 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, }
plat = dev_get_parent_platdata(dev); - if (!speed) { + + /* get speed and mode from platdata when available */ + if (plat->max_hz) { speed = plat->max_hz; mode = plat->mode; } diff --git a/include/spi.h b/include/spi.h index 92427e5..3785941 100644 --- a/include/spi.h +++ b/include/spi.h @@ -496,14 +496,15 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp, * device and slave device. * * If no such slave exists, and drv_name is not NULL, then a new slave device - * is automatically bound on this chip select. + * is automatically bound on this chip select with requested speed and mode. * - * Ths new slave device is probed ready for use with the given speed and mode. + * Ths new slave device is probed ready for use with the speed and mode + * from platdata when available or the requested values. * * @busnum: SPI bus number * @cs: Chip select to look for - * @speed: SPI speed to use for this slave - * @mode: SPI mode to use for this slave + * @speed: SPI speed to use for this slave when not available in platdata + * @mode: SPI mode to use for this slave when not available in platdata * @drv_name: Name of driver to attach to this chip select * @dev_name: Name of the new device thus created * @busp: Returns bus device

On Wed, Feb 27, 2019 at 8:06 PM Patrick Delaunay patrick.delaunay@st.com wrote:
This patch update the behavior introduced by commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
In case of DT boot, don't read default speed and mode for SPI from CONFIG_* but instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and SPI modes.
Remove also use in boards of the value speed=0 (no more supported) for ENV in SPI by using CONFIG_ENV_SPI_MAX_HZ=0.
DT values will be always used when available (full DM support of SPI slave with available DT node) even if speed and mode are requested; for example in splash screen support (in splash_sf_read_raw) or in SPL boot (in spl_spi_load_image). The caller of spi_get_bus_and_cs() no more need to force speed=0.
But the current behavior don't change if the SPI slave is not present (device with generic driver is created automatically) or if platdata is used (CONFIG_OF_PLATDATA).
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Applied to u-boot-spi/master
participants (2)
-
Jagan Teki
-
Patrick Delaunay