[U-Boot] A problem about 'sf probe' using DM_SPI

[Simon Glass]

Hi,

On 20 April 2016 at 21:22, Qianyu Gong <qianyu.gong at nxp.com> wrote:
> Hi Simon,
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> Sent: Wednesday, April 20, 2016 10:41 PM
>> To: Qianyu Gong <qianyu.gong at nxp.com>
>> Cc: u-boot at lists.denx.de; Mingkai Hu <mingkai.hu at nxp.com>; Yao Yuan
>> <yao.yuan at nxp.com>; jteki at openedev.com
>> Subject: Re: A problem about 'sf probe' using DM_SPI
>>
>> Hi,
>>
>> On 12 April 2016 at 21:13, Qianyu Gong <qianyu.gong at nxp.com> wrote:
>> > Hi Simon,
>> >
>> >> -----Original Message-----
>> >> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> >> Sent: Saturday, April 09, 2016 3:13 AM
>> >> To: Qianyu Gong <qianyu.gong at nxp.com>
>> >> Cc: u-boot at lists.denx.de; Mingkai Hu <mingkai.hu at nxp.com>; Yao Yuan
>> >> <yao.yuan at nxp.com>; jteki at openedev.com
>> >> Subject: Re: A problem about 'sf probe' using DM_SPI
>> >>
>> >> Hi Qianyu,
>> >>
>> >> On 25 March 2016 at 03:34, Qianyu Gong <qianyu.gong at nxp.com> wrote:
>> >> > Hi Simon,
>> >> >
>> >> >
>> >> >
>> >> > I think I’m not very clear with this code in common/cmd_sf.c:
>> >> >
>> >> > “
>> >> >
>> >> > # ifdef CONFIG_DM_SPI_FLASH
>> >> >
>> >> >        /* Remove the old device, otherwise probe will just be a nop
>> >> > */
>> >> >
>> >> >        ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new);
>> >> >
>> >> >        if (!ret) {
>> >> >
>> >> >               device_remove(new);
>> >> >
>> >> >              device_unbind(new);
>> >> >
>> >> >      }
>> >> >
>> >> > ”
>> >> >
>> >> > I may understand the remove but why need to unbind the device?
>> >>
>> >> This is because otherwise 'sf probe' would be a nop if the device
>> >> already existed. The spi_flash_probe_bus_cs() call will simply return
>> >> the existing device. If something has changed on the bus, it would be
>> >> ignored.
>> >>
>> >> >
>> >> > The unbind would cause a series of free operations on the device
>> >> > list, correct?
>> >>
>> >> Yes
>> >>
>> >> >
>> >> > Then if I probe a flash twice, at the second time the driver model
>> >> > will create
>> >> >
>> >> > a new flash named ‘spi_flash at xx:xx’ using default settings because
>> >> > it doesn’t
>> >> >
>> >> > find such a device in the device list and never probes it from the
>> >> > board’s fdt again.
>> >>
>> >> Yes.
>> >>
>> >> >
>> >> > => dm tree
>> >> >
>> >> > Class       Probed   Name
>> >> >
>> >> > ----------------------------------------
>> >> >
>> >> > root        [ + ]    root_driver
>> >> >
>> >> > simple_bus  [ + ]    `-- soc
>> >> >
>> >> > spi         [ + ]           |-- dspi at 2100000
>> >> >
>> >> > spi_flash   [   ]        |    |-- n25q128a
>> >> >
>> >> > spi_flash   [ + ]        |   |-- spi_flash at 1:1
>> >> >
>> >> > spi_flash   [ + ]        |   `-- spi_flash at 1:2
>> >> >
>> >> > Fortunately the default SPI mode set by U-Boot is SPI_MODE_3 so it
>> >> > doesn’t cause
>> >> >
>> >> > issues on our boards. But if an SPI flash which only supports
>> >> > SPI_MODE_0 is used,
>> >> >
>> >> > I think it wouldn’t work properly from the second probe.
>> >> >
>> >> > Sorry if there is something wrong with my understandings.
>> >> > Just because I found my flash’s name was changed to spi-flash at xx:xx
>> >> > in dm tree after several probes, I began to read something about
>> >> > driver model.J
>> >>
>> >> Yes I think removing the unbind would be OK, except that we need a
>> >> way to show the messages at the end of spi_flash_scan().
>> >>
>> >> Probably these messages should move out of the uclass and into a
>> >> separate call. We could add a function like spi_flash_show(struct
>> >> udevice *dev) and call it from do_spi_flash_probe() and perhaps
>> >> saveenv().
>> >>
>> >> Regards,
>> >> Simon
>> >
>> > Thanks for your explanation.
>> > So you mean the flash messages? Seems that removing the unbind won't
>> > affect reading ID from a real flash.
>>
>> I don't really understand what you are asking here.
>>
>
> In your first reply,
>   >>
>   >> Yes I think removing the unbind would be OK, except that we need a
>   >> way to show the messages at the end of spi_flash_scan().
>   >>
> What messages were you referring to?
> I thought they were like
> "SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16 MiB".

Yes, these messages should be printed by the 'sf probe' command, not
the uclass or SPI flash code.

>
>> > But would the unbind be needed if the dts node is modified at
>> > runtime?(I'm not sure if it is necessary)
>>
>> You cannot modify the dts node at run-time in U-Boot. It is not currently
>> supported.
>>
> OK. I just found some fdt commands in U-Boot and thought it might be modified.

Driver model expects the FDT to be static once it has scanned it - see
initf_dm() and initr_dm() for where this happens.

The fdt command is normally used for modifying the DT that is passed
to the kernel rather than the one used for configuration by U-Boot.

Regards,
Simon