From patchwork Sat Dec 12 04:45:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Schocher X-Patchwork-Id: 555947 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2001:1868:205::9]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 9E4D51402A1 for ; Sat, 12 Dec 2015 15:47:53 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1a7c48-0000MR-Kk; Sat, 12 Dec 2015 04:45:52 +0000 Received: from mail-out.m-online.net ([212.18.0.9]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1a7c44-0000Iz-Im for linux-mtd@lists.infradead.org; Sat, 12 Dec 2015 04:45:50 +0000 Received: from mail.nefkom.net (unknown [192.168.8.184]) by mail-out.m-online.net (Postfix) with ESMTP id 3pHbyC4Lwqz3hks6; Sat, 12 Dec 2015 05:45:23 +0100 (CET) X-Auth-Info: GbUsUYWQFm9lqc4nDB9PP2Vw/WUhlLqvPeyDvR4ylXg= Received: from localhost.localdomain (87.97.59.132.pool.invitel.hu [87.97.59.132]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp-auth.mnet-online.de (Postfix) with ESMTPSA id 3pHbyB4QdfzvdWS; Sat, 12 Dec 2015 05:45:22 +0100 (CET) Subject: Re: [PATCH for-4.4] mtd: fix cmdlinepart parser, early naming for auto-filled MTD To: Brian Norris References: <1449878281-94986-1-git-send-email-computersforpeace@gmail.com> From: Heiko Schocher Message-ID: <566BA661.9000407@denx.de> Date: Sat, 12 Dec 2015 05:45:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1449878281-94986-1-git-send-email-computersforpeace@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151211_204548_978772_4A7421F1 X-CRM114-Status: GOOD ( 27.03 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [212.18.0.9 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [212.18.0.9 listed in wl.mailspike.net] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.0 RCVD_IN_MSPIKE_WL Mailspike good senders X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: hs@denx.de Cc: Boris Brezillon , Frans Klaver , Heiko Schocher , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org 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 > Signed-off-by: Brian Norris > Cc: Heiko Schocher > Cc: Frans Klaver > --- > 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: 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, > 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;