From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hannes Schmelzer <hannes@schmelzer.or.at>
Date: Wed, 24 Aug 2016 14:21:06 +0200
Subject: [U-Boot] [PATCH v1] cmd/sf: probe flash with speed of last
 known flash or speed from devicetree
In-Reply-To: <196cb9a1-b5d9-f1bb-1d5a-ac2dfd2fe3a0@ti.com>
References: <1472033154-31475-1-git-send-email-oe5hpm@oevsv.at>
 <86295f5e-70ad-e660-4ff4-2046e12a6780@ti.com>
 <57BD7C19.4030401@schmelzer.or.at>
 <196cb9a1-b5d9-f1bb-1d5a-ac2dfd2fe3a0@ti.com>
Message-ID: <57BD9132.7030408@schmelzer.or.at>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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().

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 at 0:0', busnum=0, cs=0, 
driver=spi_flash_std

my devicetree look like this:

=> fdt print spi0
spi at 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>;
         S25FL256S at 0 {
                 #address-cells = <0x00000001>;
                 #size-cells = <0x00000001>;
                 compatible = "spansion,S25FL256S";
                 reg = <0x00000000>;
                 spi-max-frequency = <0x05f5e100>;
         };
};
=>

cheers,
Hannes