Hi Adam,
Thanks for the tests, And sorry for the regression on your board.
From: Adam Ford aford173@gmail.com Sent: lundi 10 décembre 2018 23:48
On Mon, Nov 19, 2018 at 11:55 AM Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Christophe Kerello christophe.kerello@st.com Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Changes in v2:
- use variables to avoid duplicated code
common/spl/spl_spi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index 8cd4830..b348b45 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -74,15 +74,21 @@ static int spl_spi_load_image(struct spl_image_info
*spl_image,
unsigned payload_offs = CONFIG_SYS_SPI_U_BOOT_OFFS; struct spi_flash *flash; struct image_header *header;
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;Instead of
+#ifdef CONFIG_DM_SPI_FLASH
What about using:
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
/* In DM mode defaults will be taken from DT */max_hz = 0, spi_mode = 0;+#endif
This one-line change 'should' give you what you want, the settings of 0 for boards using the device tree. If board that uses OF_PLATDATA cannot load the device tree, it'll default back to the configs set above. You could probably combine all if statements into one, and I'd be fine with that too.
This one-line change made my board boot with this series. I haven't applied the follow-up series yet but if a v3 was posted with this change, I'd mark it as 'tested-by.
adam
I am not familiar with platdata (my platform use only device tree). So I take some time to check and answer.
I clearly no anticipate the issue for OF_PLATDATA, mainly caused by code in drivers/spi/spi-uclass.c
In function spi_get_bus_and_cs The parameter speed expect 0 to load parameter from device tree / with parent platdata
if (!speed) { speed = plat->max_hz; mode = plat->mode; } => it should the default behavior when parent is correctly initialized for ALL the user (SPL/ENV/Commands).
But for some case, when driver is automatically created (no device tree node or platform data)
Speed is ALSO used as max_hz entry for platdata. It is a bad idea when speed = 0
Simon already make a patch to avoid the issue
=> "dm: spi: prevent setting a speed of 0 Hz" SHA1 = 12bfb2e05fc29bfbec7eb76ea8cc02e130268801
But in your case , it is not enough ..... I don't known why,
I try to understood how the platdata for parent (SPI_UCLASS) can be configurated... I need to go deeper.
A solution can to change the code introduce by Simon :
+ if (speed) { + plat->max_hz = speed; + } else { + printf("Warning: SPI speed fallback to %u kHz\n", + SPI_DEFAULT_SPEED_HZ / 1000); + plat->max_hz = SPI_DEFAULT_SPEED_HZ; + }
some update can work in your case
if (speed) { plat->max_hz = speed; } else { #ifdef CONFIG_SF_DEFAULT_SPEED plat->max_hz = CONFIG_SF_DEFAULT_SPEED; #else printf("Warning: SPI speed fallback to %u kHz\n", SPI_DEFAULT_SPEED_HZ / 1000); plat->max_hz = SPI_DEFAULT_SPEED_HZ; #endif } If (mode) plat->mode = mode; else #ifdef CONFIG_SF_DEFAULT_MODE plat->mode = CONFIG_SF_DEFAULT_MODE #else plat->mode = SPI_MODE_3 #endif
In thise case the CONFIG_DEFAULT value are defined, they are used....
So the 2 series are not enough mature today, we need some work. But I start my holiday today, I will come back only in 3 weeks....
I will check this point when I will come back.
NB: for short term solution, for plaform with DT only flash descrition (as my platform), we can use
CONFIG_SF_DEFAULT_SPEED=0 CONFIG_SF_DEFAULT_MODE=0
That should be solved the current issue in my board without need to merge the 2 series (not yet tested).
/* * Load U-Boot image from SPI flash into RAM */ flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
CONFIG_SF_DEFAULT_SPEED,CONFIG_SF_DEFAULT_MODE);
max_hz,spi_mode); if (!flash) { puts("SPI probe failed.\n"); return -ENODEV;-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Regards
Patrick