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

[Qianyu Gong]

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".

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

Regards,
Qianyu