Messages in this thread Patch in this message | | | Subject | Re: [PATCH for-4.4] mtd: fix cmdlinepart parser, early naming for auto-filled MTD | From | Heiko Schocher <> | Date | Sat, 12 Dec 2015 05:45:21 +0100 |
| |
Hello Brian,
Am 12.12.2015 um 00:58 schrieb Brian Norris: > Commit 807f16d4db95 ("mtd: core: set some defaults when dev.parent is > set") attempted to provide some default settings for MTDs that > (a) assign the parent device and > (b) don't provide their own name or owner > > However, this isn't a perfect drop-in replacement for the boilerplate > found in some drivers, because the MTD name is used by partition > parsers like cmdlinepart, but the name isn't set until add_mtd_device(), > after the parsing is completed. This means cmdlinepart sees a NULL name > and therefore will not work properly. > > Fix this by moving the default name and owner assignment to be first in > the MTD registration process. > > Fixes: 807f16d4db95 ("mtd: core: set some defaults when dev.parent is set") > Reported-by: Heiko Schocher <hs@denx.de> > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > Cc: Heiko Schocher <hs@denx.de> > Cc: Frans Klaver <fransklaver@gmail.com> > --- > Heiko, can you provide testing feedback (e.g., 'Tested-by: ...')?
Sorry, does not work for me:
Based on: pollux:linux hs [20151212] $ git describe master v4.4-rc4-135-gb9d8545
and this patch, shows the same problem,
Adding: diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 93f664c..28dcf66 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1685,6 +1685,7 @@ static int omap_nand_probe(struct platform_device *pdev) info->ecc_opt = pdata->ecc_opt; mtd = &info->mtd; mtd->priv = &info->nand; + mtd->name = dev_name(&pdev->dev); mtd->dev.parent = &pdev->dev; nand_chip = &info->nand; nand_chip->ecc.priv = NULL; and it works again ...
bye, Heiko > > In testing this myself, it looks like cmdlinepart.c can actually work OK with a > single-MTD system, even when mtd->name isn't set. See this snippet: > > /* > * Search for the partition definition matching master->name. > * If master->name is not set, stop at first partition definition. > */ > for (part = partitions; part; part = part->next) { > if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id))) > break; > } > > But, I don't know *why* it does that... > > drivers/mtd/mtdcore.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index 95c13b2ffa79..ffa288474820 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -426,15 +426,6 @@ int add_mtd_device(struct mtd_info *mtd) > mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1; > mtd->writesize_mask = (1 << mtd->writesize_shift) - 1; > > - if (mtd->dev.parent) { > - if (!mtd->owner && mtd->dev.parent->driver) > - mtd->owner = mtd->dev.parent->driver->owner; > - if (!mtd->name) > - mtd->name = dev_name(mtd->dev.parent); > - } else { > - pr_debug("mtd device won't show a device symlink in sysfs\n"); > - } > - > /* Some chips always power up locked. Unlock them now */ > if ((mtd->flags & MTD_WRITEABLE) && (mtd->flags & MTD_POWERUP_LOCK)) { > error = mtd_unlock(mtd, 0, mtd->size); > @@ -549,6 +540,21 @@ static int mtd_add_device_partitions(struct mtd_info *mtd, > return 0; > } > > +/* > + * Set a few defaults based on the parent devices, if not provided by the > + * driver > + */ > +static void mtd_set_dev_defaults(struct mtd_info *mtd) > +{ > + if (mtd->dev.parent) { > + if (!mtd->owner && mtd->dev.parent->driver) > + mtd->owner = mtd->dev.parent->driver->owner; > + if (!mtd->name) > + mtd->name = dev_name(mtd->dev.parent); > + } else { > + pr_debug("mtd device won't show a device symlink in sysfs\n"); > + } > +} > > /** > * mtd_device_parse_register - parse partitions and register an MTD device. > @@ -587,6 +593,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types, > int ret; > struct mtd_partition *real_parts = NULL; > > + mtd_set_dev_defaults(mtd); > + > ret = parse_mtd_partitions(mtd, types, &real_parts, parser_data); > if (ret <= 0 && nr_parts && parts) { > real_parts = kmemdup(parts, sizeof(*parts) * nr_parts, >
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
| |