On 08/24/2016 02:21 PM, Hannes Schmelzer wrote:
On 08/24/2016 01:12 PM, Vignesh R wrote:
On Wednesday 24 August 2016 04:21 PM, Hannes Schmelzer wrote:
On 08/24/2016 12:35 PM, Vignesh R wrote:
Hi,
Hi Vignesh,
On Wednesday 24 August 2016 03:35 PM, Hannes Schmelzer wrote:
During probing flashes on the spi bus using the "sf probe" command, a maybe existing flash (from fdt) is unbound and removed to force the 'spi_flash_probe_bus_cs' really scanning the bus.
Today the bus is probed with speed 0, this triggers several fall-back mechanism (mostly in the low-level drivers) to catch the impossible zero speed. Result of this is, that the spi-flash runs at very low speed depending on the minimum given by low-level driver/hardware.
Values like 'spi-max-frequency' from devicetree are ignored totally today.
This commit changes as following:
- if there was already some flash binding in devicetree (having some spi-max-frequency within) speed is taken from it
- if no flash binding was present for speed the
'spi-max-frequency' from the responsible spi node is taken.
Signed-off-by: Hannes Schmelzer oe5hpm@oevsv.at
With commit 96907c0fe50a8 ("dm: spi: Read default speed and mode values from DT") sf probe picks spi-max-frequency from DT if not specified as argument.
But when sf probe is called second time, the command fails to pick up speed from DT. This is because flash node is unbound from the SPI controller children nodes. Below patch should fix this issue: https://patchwork.ozlabs.org/patch/659979/
sry ... i didn't took notice about this patch at the beginning of mine. Just reviewed and tested it.
The named patch makes things a bit better but not good. Speed for flash still has no relationship to 'spi-max-frequency' from the spi-bus nor with a maybe defined flash in dts.
Rather the #define CONFIG_ENV_SPI_MAX_HZ is used.
sf probe calls do_spi_flash_probe() -spi_flash_probe_bus_cs() -spi_get_bus_and_cs()
In spi_get_bus_and_cs(): if (!speed) { speed = plat->max_hz; mode = plat->mode; } This should set speed to spi-max-frequency as per flash DT node.
yes that should do. But have look a few lines up, around 296, there the newly created flash node has been modified with wrong values before. No glue why a new flash node is being created.
AFAIU, saveenv() uses CONFIG_ENV_SPI_MAX_HZ only when CONFIG_DM_SPI_FLASH is not defined. Could please explain how CONFIG_ENV_SPI_MAX_HZ takes precedence over spi-max-frequency during sf probe?
Thanks the discussion, I think we coming closer to the problem. Your'e right saveenv() behaves as you described, but not so env_relocate_spec().
There the flash is probed like this: env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
and thats the point where some existing dt node becomes modified with those values. Thats probably wrong doing so.
I think there should be same logic applied as in saveen().
Tested again, and confirming this behaviour.
Just for testing i fixed the probe there. Now i'm a bit confused about that the function spi_find_chip_select(bus, cs, &dev) in spi-uclass doesn't find child devices on the spi node. Instead there a new flash binding is created. spi_get_bus_and_cs: Binding new device 'spi_flash@0:0', busnum=0, cs=0, driver=spi_flash_std
Debugged into DM and found out that my devicetree didn't represent a valid flash-child. I've now modified it to:
=> fdt print spi0 spi@e000d000 { clock-names = "ref_clk", "pclk"; clocks = <0x00000001 0x0000000a 0x00000001 0x0000002b>; compatible = "xlnx,zynq-qspi-1.0"; status = "okay"; interrupt-parent = <0x00000003>; interrupts = <0x00000000 0x00000013 0x00000004>; reg = <0xe000d000 0x00001000>; #address-cells = <0x00000001>; #size-cells = <0x00000000>; u-boot,dm-pre-reloc; spi-max-frequency = <0x05f5e100>; spiflash@0 { u-boot,dm-pre-reloc; compatible = "spidev", "spi-flash"; spi-max-frequency = <0x05f5e100>; reg = <0x00000000>; }; }; =>
So that works and u-boot knows the flash from beginning and uses as correct flash-node from fdt instead creating a new one.
Finally i think, we can drop my patch and somebody (maybe me?) should fix the probing during environment relocation.
cheers, Hannes