lkml.org 
[lkml]   [2015]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH for-4.4] mtd: fix cmdlinepart parser, early naming for auto-filled MTD
From
Date
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



\
 
 \ /
  Last update: 2015-12-12 06:01    [W:0.077 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site