From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Date: Mon, 8 Oct 2018 19:27:00 +0200 Subject: [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command In-Reply-To: References: <20181001134331.9756-1-miquel.raynal@bootlin.com> <20181001134331.9756-6-miquel.raynal@bootlin.com> <20181003144259.333c1b00@xps13> <20181003145733.3ffc748e@xps13> <20181003153544.0cb7cde2@xps13> <20181008182806.00918e6d@bbrezillon> Message-ID: <20181008192700.036586ab@bbrezillon> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, 8 Oct 2018 11:58:40 -0500 Adam Ford wrote: > On Mon, Oct 8, 2018 at 11:52 AM Adam Ford wrote: > > > > On Mon, Oct 8, 2018 at 11:28 AM Boris Brezillon > > wrote: > > > > > > Hi Adam, > > > > > > On Mon, 8 Oct 2018 11:13:40 -0500 > > > Adam Ford wrote: > > > > > > > On Wed, Oct 3, 2018 at 8:41 AM Adam Ford wrote: > > > > > > > > > > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal 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