Re: [PATCH 1/1] mtd/intel-spi: Support cmdline-based partition

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

Hi all,

On Wed, 1 Apr 2020 09:41:48 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Hi ron,
> 
> ron minnich <rminnich@xxxxxxxxx> wrote on Mon, 30 Mar 2020 08:53:22
> -0700:
> 
> > On Mon, Mar 30, 2020 at 12:27 AM Miquel Raynal
> > <miquel.raynal@xxxxxxxxxxx> wrote:
> >   
> > > Would it be hard to support an extra ':' after the MTD device name?
> > > This way would would allow anything inside the optional '(' ')' but
> > > would keep the trailing ':'.
> > >
> > > toTay:
> > >         mtdparts=name:part1,part2
> > >
> > > So:
> > >         mtdparts=(0000:00:1f.5):25165824(BIOS),-(squashfs)    
> > 
> > 
> > I thought about that ':' too. It does add a bit more to the code, and
> > a bit more in the way of error cases. I always worry, when code is
> > going into flash,
> > about errors where something looks close to right but is wrong. (says
> > the person who just typed it instead of is a few times :-)
> > 
> > What if we did this:
> > mtdparts=[0000:00:1f.5]25165824(BIOS),-(squashfs)
> > 
> > Is the "]" 'enough different' that we do not need the ':'?
> > 
> > I kind of like the [] better anyway as it makes the mtdid stand out a
> > bit more from the part names? But is it enough that we don't need the
> > ':'? Would you still prefer the () as opposed to the []?  
> 
> I like the [] as well, maybe more than () because at least it does not
> conflict with the partition names. But I really prefer keeping the : if
> the code is still readable.
> 
> It is much easier to explain to people : "if you have a : in the name,
> enclose it with []".

Sorry to chime in so late in the discussion, but I wonder if any of
that is necessary. Can't we just split the string per device (split
strings every time we see a ';'), and then find the last ':' in each of
those strings and consider everything before that last ':' to be the MTD
name. That should work even if the MTD name contains one or more ':'.

Don't get me wrong, I'm perfectly fine with intel enclosing the PCI
address in [] to make it clearer, but I see that other drivers use ':'
in their MTD device names (the atmel raw NAND controller driver to name
one), so I think it'd be good to make the mtd part parsing robust to
this use case.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/