[U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Mon, 8 Oct 2018 11:58:40 -0500
Adam Ford <aford173@gmail.com> wrote:

> On Mon, Oct 8, 2018 at 11:52 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Mon, Oct 8, 2018 at 11:28 AM Boris Brezillon
> > <boris.brezillon@bootlin.com> wrote:  
> > >
> > > Hi Adam,
> > >
> > > On Mon, 8 Oct 2018 11:13:40 -0500
> > > Adam Ford <aford173@gmail.com> wrote:
> > >  
> > > > On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:  
> > > > >
> > > > > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > > > >
> > > > > > Hi Adam,
> > > > > >  
> > > > > > > >  
> > > > > > > > >  
> > > > > > > > > >
> > > > > > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > > > > > partitions, so I know nand works.  My defconfig
> > > > > > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > > > > > I do store the partition information?  
> > > > > > > > >
> > > > > > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > > > > > _variable_ must be used in order to declare the partitions.  
> > > > > > > >
> > > > > > > > OK.  If I can get MTD working, I'll work to remove the other commands
> > > > > > > > like NAND and MTDPARTS  
> > > > > > >
> > > > > > > As of today, the process of migration is not entirely finished to DM
> > > > > > > and you might still need to issue *first* a "nand probe" to register
> > > > > > > the device operations.  
> > > > > >
> > > > > > Mmmh there is no nand probe actually, for raw nands like the one you
> > > > > > have it should work out of the box.  
> > > >
> > > > i haven't actually insterted debug code yet, but I started a quick
> > > > code review.  There is a function called 'mtd_probe_uclass_mtd_devs'
> > > > which states it will probe with DM compliant drivers.
> > > > I am thinking the nand and/or GPMC drivers are not yet DM compliant
> > > > yet.  The DM tree doesn't list any of the MTD parts.
> > > >
> > > > Is it save to assume it just wont' work until the drives are DM
> > > > compliant, or is this driver designed to play with the lower-level
> > > > drivers as-is?  
> > >
> > > The whole point of this rework was to allow both old (non-DM compliant)
> > > and new (DM compliant) drivers to be exposed the same way to the upper
> > > layers. If it doesn't work for regular NAND drivers it's a bug, and we
> > > should fix it.  
> >
> > That's what I assummed, so I kept trying to debug, but I wanted to
> > make sure I asked the question, because I've been burned by
> > assumptions before.
> >
> > I narrowed the hang  down to the following in driver/mtd/mtd_uboot,
> > mtd_probe_devices
> >
> > int mtd_probe_devices(void)
> > {
> > ...
> >      /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> >      if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))

Can you try with

	if (old_mtdparts && old_mtdids &&
	    !strcmp(mtdparts, old_mtdparts) &&
	    !strcmp(mtdids, old_mtdids))

> >           return 0;
> > }
> >
> > The above function neither returns nor exits. I added a brace and a
> > printf just before the return, and the printf never appears.
> > Only when I put #if 0 around the 'if' statement above, and 'mtd list'
> > returned and did a dump.
> >
> > I tried initializing
> > static char *old_mtdparts = '\0';
> > static char *old_mtdids = '\0';
> >
> > and
> >
> > static char *old_mtdparts = NULL;
> > static char *old_mtdids = NULL;
> >
> > Neither fixed the hang.
> >
> > adam
> >
> > The functioning dump (with the ifdef) is as follows:
> >
> > List of MTD devices:
> > * nand0
> >   - type: NAND flash
> >   - block size: 0x20000 bytes
> >   - min I/O: 0x800 bytes
> >   - OOB size: 64 bytes
> >   - OOB available: 10 bytes
> >   - ECC strength: 8 bits
> >   - ECC step size: 512 bytes
> >   - bitflip threshold: 6 bits
> >   - 0x000000000000-0x000020000000 : "nand0"
> >           - 0x000000000000-0x000000080000 : "MLO"
> >           - 0x000000080000-0x000000240000 : "u-boot"
> >           - 0x000000240000-0x000000260000 : "spl-os"
> >           - 0x000000260000-0x000000280000 : "u-boot-env"
> >           - 0x000000280000-0x000000880000 : "kernel"
> >           - 0x000000880000-0x000020000000 : "fs"
> >
> >
> >  
> > >  
> 
> Lastly, I noticed that I am not able to de-select CMD_MTDPARTS from my
> config because CMD_UBI (which I use) automatically selects
> CMD_MTDPARTS.  I only mention it because I know you're trying to
> deprecate CMD_MTDPARTS.
> 
> adam
> 
> > > Regards,
> > >
> > > Boris