Messages in this thread |  | | From | ron minnich <> | Date | Fri, 27 Nov 2020 08:32:02 -0800 | Subject | Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons |
| |
I'm a bit worried about how tricky this starts to get. I'm inclined to go back to an earlier implementation which used a character that had not yet been used (iirc I used [] around the PCI ID in a very early version). What if we used, e.g, a single ! and searched for that? It need not be !; pick a character. Just something not already in use, as the ambiguity around which ':' delimits the device has become an issue, as you show.
Almost nothing in the original patch would change, save the character being searched for. By using a character we'd never used, we'd avoid breaking existing usage.
Comments?
On Sat, Nov 21, 2020 at 4:14 PM Sven Eckelmann <sven@narfation.org> wrote: > > On Wednesday, 29 April 2020 18:53:47 CET Ronald G. Minnich wrote: > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > > Looks like some drivers define MTD names with a colon in it, thus > > making mtdpart= parsing impossible. Let's fix the parser to gracefully > > handle that case: the last ':' in a partition definition sequence is > > considered instead of the first one. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > Signed-off-by: Ron Minnich <rminnich@google.com> > > Tested-by: Ron Minnich <rminnich@google.com> > > --- > > drivers/mtd/parsers/cmdlinepart.c | 23 ++++++++++++++++++++--- > > 1 file changed, 20 insertions(+), 3 deletions(-) > > This change broke OpenWrt booting on some IPQ40xx devices. Here for example > the parts of the cmdline which the u-boot on an OpenMesh A62 sets > automatically: > > root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait > > This is then parsed by newpart as: KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) > > And of course, this results in: > > mtd: partition has size 0 > [...] > /dev/root: Can't open blockdev > VFS: Cannot open root device "31:11" or unknown-block(31,11): error -6 > Please append a correct "root=" boot option; here are the available partitions: > 1f00 32768 mtdblock0 > (driver?) > Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,11) > CPU1: stopping > CPU: 1 PID: 0 Comm: > > This affects OpenWrt since the commit d6a9a92e3217 ("kernel: bump 5.4 to > 5.4.69") because this change was backported to Linux v5.4.69. Reverting > this change fixes the problem for me. > > And if I see it correctly, this is also affecting the kernel (4.14) for > the OpenWrt 18.06.x + 19.07.x branch because it can also be found in > there as part of the v4.14.200 release. > > Another workaround is to replace the first "(" with a NULL too. See the > attached patch for the one which I used to fix the OpenWrt bootup. > > Kind regards, > Sven
|  |