Re: [PATCH RFC API ONLY] mtd: get parsers from lookup table

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

On Mon, 16 Jul 2018 22:58:19 +0200
Rafał Miłecki <rafal@milecki.pl> wrote:

> On 2018-07-16 18:23, Boris Brezillon wrote:
> > Hi Rafal,
> > 
> > On Mon, 16 Jul 2018 13:17:12 +0200
> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >   
> >> From: Rafał Miłecki <rafal@milecki.pl>
> >> 
> >> Existing implementation of specifying flash device parsers became a 
> >> bit
> >> hacky and needs a cleanup. Currently it requires:
> >> 1) Passing parsers in a custom platform data by arch code
> >> or
> >> 2) Hardcoding parsers table in a flash driver
> >> 
> >> The purpose of the new implementation is to:
> >> 1) Clean up flash drivers
> >> 2) Have a generic solution
> >> 3) Avoid code duplication
> >> 
> >> That new implementation assigns table of parsers to a MTD's parent
> >> device. That way flash driver doesn't have to take care of passing
> >> parsers. It's inspired by GPIO lookup table.
> >> 
> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >> ---
> >> It's obviously a RFC patch only. It doesn't really implement anything.
> >> It DOESN'T COMPILE.
> >> 
> >> I'd like to know if you like this idea and if it's worth for me to
> >> actually spend time implementing it.  
> > 
> > I love the idea. Actually, I'd like to extend that to fixed partition
> > definitions (those passed to mtd_device_register()) so that they can be
> > parsed by the fixed-partition parser (the one already taking care of
> > compatible = "fixed-partitions").  
> 
> That was something I planned to handle next, so it's great to hear we
> share the ideas for further improvements! That will extremely simplify
> mtd_device_register().

Cool!

> 
> In 99% cases there is only one place in the code registering a given
> platform device. As there is only one place registering "sharpsl-nand"
> device, we can assume the resulting name will be "sharpsl-nand.0".
> 
> If there was another code doing the same, you couldn't know if you
> should expect "sharpsl-nand.0" or "sharpsl-nand.1".

It's only true if you use platform_device->id = PLATFORM_DEVID_AUTO,
and I think we can assume linking a lookup table to a device with ->id
= PLATFORM_DEVID_AUTO is broken. In other cases, the id defined at
platform_device definition time (either PLATFORM_DEVID_NONE or a
statically assigned id).

> 
> I believe we should be fine guessing in these 99% cases and only depend
> on order (semi-risking a race) in that 1% (or less).