Hi Adam,
Thanks for the tests,
And sorry for the regression on your board.
> From: Adam Ford <aford...@gmail.com>
> Sent: lundi 10 décembre 2018 23:48
>
> On Mon, Nov 19, 2018 at 11:55 AM Patrick Delaunay
> <patrick.delau...@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.kere...@st.com>
> > Signed-off-by: Patrick Delaunay <patrick.delau...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
