Re: [PATCH for-4.4] mtd: fix cmdlinepart parser, early naming for auto-filled MTD

From: Heiko Schocher
Date: Tue Dec 15 2015 - 01:52:08 EST


Hello Brian,

Am 14.12.2015 um 20:24 schrieb Brian Norris:
On Fri, Dec 11, 2015 at 09:39:18PM -0800, Brian Norris wrote:
On Sat, Dec 12, 2015 at 05:45:21AM +0100, Heiko Schocher wrote:
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@xxxxxxx>
Signed-off-by: Brian Norris <computersforpeace@xxxxxxxxx>
Cc: Heiko Schocher <hs@xxxxxxx>
Cc: Frans Klaver <fransklaver@xxxxxxxxx>
---
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,

[...]

BTW, can you please include the relevant log snippets when replying in
the future?

Uh, Sorry for that, yes of course ...

Ugh, I see the problem. In nand_base.c, nand_get_flash_type():

if (!mtd->name)
mtd->name = type->name;

Specifically, I think we could hack around this with something like the
following additional patch. Untested:

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ece544efccc3..9f169566fba4 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3826,6 +3826,9 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
if (!type)
type = nand_flash_ids;

+ if (!mtd->name && mtd->dev.parent)
+ mtd->name = dev_name(mtd->dev.parent);
+
for (; type->name != NULL; type++) {
if (is_full_id_nand(type)) {
if (find_full_id_nand(mtd, chip, type, id_data, &busw))


Sorry, I had not yet the time to look into the issue deeper, Thanks for
digging into it. I tried therefore your patch (automated with tbot see
below [1]) on linux "v4.4-rc5" and

your patch works!

bye,
Heiko

[1] automated test with tbot [2]
http://xeidos.ddns.net/buildbot/builders/mcx/builds/7/steps/shell/logs/tbotlog

as there is a lot of things done automatically, search for
"Micron MT29F4G16ABADAWP"

All you see in the log is done automatically with tbot. The mcx board
is in munich, while I am doing the test from hungary. In my testsetup
tbot is integrated into buildbot, which is used for the webserver
and triggering builds.

What is done:
- get linux code (if it is not already checked out)
- apply patches
(I have also a testcase, which download all patches from patchwork,
which are delegated to me (for u-boot), check them with checkpatch
and apply it ... very helpful, if this is done in a nightly build,
you have only to look in the morning, if all is green, and you know
that all patches are checkpatch clean, apply celan, compile clean
and do not break the boards you have in your nightly build ... also
there is a testcase which starts a git bisect test [3] ...)

As I did not mainlined the board yet, there are a bunch of checkpatch
errors, so I disabled that the testcase fails, if there are errors.

- compile linux
- copy files to tftpdir
- boot them
- execute testcases
for example check in dmesg some strings with:
https://github.com/hsdenx/tbot/blob/master/src/tc/tc_lx_dmesg_grep.py

So detected our current bug ;-)

check register settings with devmem2:
https://github.com/hsdenx/tbot/blob/master/src/tc/tc_lx_check_reg_file.py

Thats very nice, as you can check for example complete pinmux register.
see as an example [4]

last but not least, tbot (and buildbot incl. webserver) is running on a
raspberry pi at my home! So no expensive hw costs.

[2] https://github.com/hsdenx/tbot
[3] git bisect automated:
https://github.com/hsdenx/tbot/blob/master/src/tc/tc_board_git_bisect.py
log as an example for U-Boot:
http://xeidos.ddns.net/buildbot/builders/tqm5200s/builds/3/steps/shell/logs/tbotlog
[4] register file
https://github.com/hsdenx/tbot/blob/master/src/files/ccu1_pinmux_scm.reg

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/