On Mon, Nov 13, 2017 at 06:26 AM, Baruch Siach wrote:

On Sun, Nov 12, 2017 at 08:42:28PM +0100, Heinrich Schuchardt wrote:

...

On 11/12/2017 04:30 PM, Baruch Siach wrote:

...

Calling .set_speed with zero speed is definitely a bug. Instead of crashing, set hz to 1 so that we get the lowest possible frequency rate.

Did this actually occur? Why?

Yes. CONFIG_ARCH_MVEBU selects CONFIG_DM_SPI_FLASH. In cmd/sf.c:do_spi_flash_probe(), speed is set to 0 when CONFIG_DM_SPI_FLASH, passed to spi_flash_probe_bus_cs(), and from there to spi_get_bus_and_cs(). Unfortunately, since the ClearFog SPI flash node has no "spi-flash" compatible string, the spi_flash_std driver is not bound, so spi_child_post_bind() returns early without calling spi_slave_ofdata_to_platdata(). The dm_spi_slave_platdata speed field is left uninitialized, and we end up with speed=0 when calling spi_set_speed_mode().

This should be fixed with http://patchwork.ozlabs.org/patch/837360/ for ClearFog, by adding the "spi-flash" compatible. But "spi-flash" is non standard. Most kernel DT files use "jedec,spi-nor" instead. So you can expect this issue to show up again in the future.

This is exactly what I just got with the cadence-spi driver when starting my mach-socfpga board with the device tree working for linux.

I also 'fixed' it by changing the dts, but I'd rather use the same dts for linux and U-Boot.

Should this be fixed in the core code or is it a bug of each driver not detecting the '0'?

A workaround is to provide speed argument in the 'sf probe' command line.

This is not a valid workaround for me: the spi-nor is my boot device and I got this error even in SPL when loading U-Boot. Not using 'sf probe'.

Simon