[U-Boot] [PATCH v3 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 3rd 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 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
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
+ In case of DT boot, SPI don't read default speed and mode + from CONFIG_*, but from platdata values computed from available + DT node
- TFTP Fixed UDP Port: CONFIG_TFTP_PORT diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..f1811d2 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/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

Hi Patrick,
This patch update the behavior introduced by commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
...
Reviewed-by: Petr Vorel petr.vorel@gmail.com
Nice idea.
Kind regards, Petr

On Mon, Jan 28, 2019 at 2:37 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
In case of DT boot, SPI don't read default speed and mode
from CONFIG_*, but from platdata values computed from available
DT node
This has to update in Kconfig help info.

Hi Jagan
From: Jagan Teki jagan@amarulasolutions.com Sent: samedi 9 février 2019 17:22 Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
On Mon, Jan 28, 2019 at 2:37 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
In case of DT boot, SPI don't read default speed and mode
from CONFIG_*, but from platdata values computed from available
DT node
This has to update in Kconfig help info.
Ok but witch Kconfig ? whitch config ?
drivers/mtd/spi/Kconfig config DM_SPI_FLASH
PS: In master branch, these define are not in yet managed in Kconfig, but they are still managed by defines: scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE And so documentation is done in README not in Kconfig
some migration in Kconfig is pending (moveconfig) ?
Regards Patrick

On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Jagan
From: Jagan Teki jagan@amarulasolutions.com Sent: samedi 9 février 2019 17:22 Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
On Mon, Jan 28, 2019 at 2:37 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
In case of DT boot, SPI don't read default speed and mode
from CONFIG_*, but from platdata values computed from available
DT node
This has to update in Kconfig help info.
Ok but witch Kconfig ? whitch config ?
drivers/mtd/spi/Kconfig config DM_SPI_FLASH
PS: In master branch, these define are not in yet managed in Kconfig, but they are still managed by defines: scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE And so documentation is done in README not in Kconfig
some migration in Kconfig is pending (moveconfig) ?
Yes, moving them and make changes on top would really nice to go. thanks!

Hi Jagan,
From: Jagan Teki jagan@amarulasolutions.com Sent: jeudi 14 février 2019 18:05
On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Jagan
From: Jagan Teki jagan@amarulasolutions.com Sent: samedi 9 février 2019 17:22 Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
On Mon, Jan 28, 2019 at 2:37 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
In case of DT boot, SPI don't read default speed and mode
from CONFIG_*, but from platdata values computed from
available
DT node
This has to update in Kconfig help info.
Ok but witch Kconfig ? whitch config ?
drivers/mtd/spi/Kconfig config DM_SPI_FLASH
PS: In master branch, these define are not in yet managed in Kconfig, but they
are still managed by defines:
scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE And so documentation is done in README not in Kconfig
some migration in Kconfig is pending (moveconfig) ?
Yes, moving them and make changes on top would really nice to go. thanks!
In fact it was a question... But I have my answer, no migration are pending on your side.
So I try yesterday and this morning to start migration in Kconfig but it is more difficult than expected initially (make defconfig freeze my PC for some board after my modificaitons).
So I don't expect to make the change on the top of the moving serie at a short term, but I will continue to work on that.
It is possible to inverse the proposed order... this patch first and after the Kconfig migration ?
Regards
Patrick

Hi Jagan,
From: Patrick DELAUNAY Sent: mardi 19 février 2019 13:28 Subject: RE: [PATCH v3] dm: spi: Read default speed and mode values from DT
Hi Jagan,
From: Jagan Teki jagan@amarulasolutions.com Sent: jeudi 14 février 2019 18:05
On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Jagan
From: Jagan Teki jagan@amarulasolutions.com Sent: samedi 9 février 2019 17:22 Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
On Mon, Jan 28, 2019 at 2:37 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.
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 v3: - complete rework of the patch-set to avoid regression
Changes in v2: - use variables to avoid duplicated code
README | 3 +++ cmd/sf.c | 3 +-- common/spl/spl_spi.c | 2 ++ drivers/spi/spi-uclass.c | 4 +++- include/spi.h | 9 +++++---- 5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/README b/README index 17d56b8..f7fe74f 100644 --- a/README +++ b/README @@ -2184,6 +2184,9 @@ The following options need to be configured: CONFIG_SF_DEFAULT_MODE (see include/spi.h) CONFIG_SF_DEFAULT_SPEED in Hz
In case of DT boot, SPI don't read default speed and mode
from CONFIG_*, but from platdata values computed
- from
available
DT node
This has to update in Kconfig help info.
Ok but witch Kconfig ? whitch config ?
drivers/mtd/spi/Kconfig config DM_SPI_FLASH
PS: In master branch, these define are not in yet managed in Kconfig, but they
are still managed by defines:
scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE And so documentation is done in README not in Kconfig
some migration in Kconfig is pending (moveconfig) ?
Yes, moving them and make changes on top would really nice to go. thanks!
In fact it was a question... But I have my answer, no migration are pending on your side.
So I try yesterday and this morning to start migration in Kconfig but it is more difficult than expected initially (make defconfig freeze my PC for some board after my modificaitons).
So I don't expect to make the change on the top of the moving serie at a short term, but I will continue to work on that.
It is possible to inverse the proposed order... this patch first and after the Kconfig migration ?
Finally I found and solve the issue, So I push the 2 series :
SPI migration in Kconfig = http://patchwork.ozlabs.org/project/uboot/list/?series=94485 V4 = http://patchwork.ozlabs.org/project/uboot/list/?series=94490
Regards
Patrick
participants (4)
-
Jagan Teki
-
Patrick DELAUNAY
-
Patrick Delaunay
-
Petr Vorel