* Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers
2016-01-04 19:11 [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers Brian Norris
@ 2016-01-04 20:31 ` Boris Brezillon
2016-01-05 18:25 ` Brian Norris
2016-01-05 6:11 ` Heiko Schocher
2016-01-05 15:28 ` [for-4.4] " Sørensen, Stefan
2 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2016-01-04 20:31 UTC (permalink / raw)
To: Brian Norris; +Cc: linux-mtd, linux-kernel, Heiko Schocher, Frans Klaver
Hi Brian,
On Mon, 4 Jan 2016 11:11:03 -0800
Brian Norris <computersforpeace@gmail.com> wrote:
> Commits such as commit 853f1c58c4b2 ("mtd: nand: omap2: show parent
> device structure in sysfs") attempt to rely on the core MTD code to set
> the MTD name based on the parent device. However, nand_base tries to set
> a different default name according to the flash name (e.g., extracted
> from the ONFI parameter page), which means NAND drivers will never make
> use of the MTD defaults. This is not the intention of commit
> 853f1c58c4b2.
>
> This results in problems when trying to use the cmdline partition
> parser, since the MTD name is different than expected. Let's fix this by
> providing a default NAND name, where possible.
>
> Note that this is not really a great default name in the long run, since
> this means that if there are multiple MTDs attached to the same
> controller device, they will have the same name. But that is an existing
> issue and requires future work on a better controller vs. flash chip
> abstraction to fix properly.
Yep, and IMHO, the chip numbering currently in use in most NAND
controller drivers is not the best solution either. We should somehow
find a way to describe the mtd id using something like:
mtd-id: <nand-controller-id>@<cs-line>
nand-controller-id: <nand-controller-ip-name>-<ip-address>
Anyway, let's discuss that after the nand-chip <-> nand-controller
separation rework.
>
> 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>
> ---
> This patch is needed in additon to commit 472b444eef93 ("mtd: fix cmdlinepart
> parser, early naming for auto-filled MTD") to fix Heiko's reported problem. At
> this point, I'm not sure if this should be targeted toward late 4.4 or for 4.5.
> It's a 4.4 regresssion, but a very small one. And I'm not sure if this will
> have wide enough impact that it should be given a longer time to be reviewed
> and tested. We can always send it to -stable later, if it's really needed.
>
> drivers/mtd/nand/nand_base.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> 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);
> +
Just nitpicking, but I think this would be better placed in
nand_set_defaults() or in nand_scan_ident() before nand_get_flash_type()
call.
Anyway,
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> for (; type->name != NULL; type++) {
> if (is_full_id_nand(type)) {
> if (find_full_id_nand(mtd, chip, type, id_data, &busw))
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers
2016-01-04 20:31 ` Boris Brezillon
@ 2016-01-05 18:25 ` Brian Norris
0 siblings, 0 replies; 5+ messages in thread
From: Brian Norris @ 2016-01-05 18:25 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd, linux-kernel, Heiko Schocher, Frans Klaver
Hi Boris,
On Mon, Jan 04, 2016 at 09:31:02PM +0100, Boris Brezillon wrote:
> On Mon, 4 Jan 2016 11:11:03 -0800
> Brian Norris <computersforpeace@gmail.com> wrote:
>
> > Commits such as commit 853f1c58c4b2 ("mtd: nand: omap2: show parent
> > device structure in sysfs") attempt to rely on the core MTD code to set
> > the MTD name based on the parent device. However, nand_base tries to set
> > a different default name according to the flash name (e.g., extracted
> > from the ONFI parameter page), which means NAND drivers will never make
> > use of the MTD defaults. This is not the intention of commit
> > 853f1c58c4b2.
> >
> > This results in problems when trying to use the cmdline partition
> > parser, since the MTD name is different than expected. Let's fix this by
> > providing a default NAND name, where possible.
> >
> > Note that this is not really a great default name in the long run, since
> > this means that if there are multiple MTDs attached to the same
> > controller device, they will have the same name. But that is an existing
> > issue and requires future work on a better controller vs. flash chip
> > abstraction to fix properly.
>
> Yep, and IMHO, the chip numbering currently in use in most NAND
> controller drivers is not the best solution either. We should somehow
> find a way to describe the mtd id using something like:
>
> mtd-id: <nand-controller-id>@<cs-line>
> nand-controller-id: <nand-controller-ip-name>-<ip-address>
Maybe dots ('.') instead of '@'? But otherwise, seems somewhat
reasonable. Maybe this could just be something like:
dev_name(mtd->dev.parent) + "." + chip_select
> Anyway, let's discuss that after the nand-chip <-> nand-controller
> separation rework.
Sure.
> > 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>
> > ---
> > This patch is needed in additon to commit 472b444eef93 ("mtd: fix cmdlinepart
> > parser, early naming for auto-filled MTD") to fix Heiko's reported problem. At
> > this point, I'm not sure if this should be targeted toward late 4.4 or for 4.5.
> > It's a 4.4 regresssion, but a very small one. And I'm not sure if this will
> > have wide enough impact that it should be given a longer time to be reviewed
> > and tested. We can always send it to -stable later, if it's really needed.
> >
> > drivers/mtd/nand/nand_base.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > 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);
> > +
>
> Just nitpicking, but I think this would be better placed in
> nand_set_defaults() or in nand_scan_ident() before nand_get_flash_type()
> call.
Hmm, nand_set_defaults() would probably be better. I'll resend this and
defer it to 4.5 (and 4.4.x).
> Anyway,
>
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Thanks,
Brian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers
2016-01-04 19:11 [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers Brian Norris
2016-01-04 20:31 ` Boris Brezillon
@ 2016-01-05 6:11 ` Heiko Schocher
2016-01-05 15:28 ` [for-4.4] " Sørensen, Stefan
2 siblings, 0 replies; 5+ messages in thread
From: Heiko Schocher @ 2016-01-05 6:11 UTC (permalink / raw)
To: Brian Norris; +Cc: linux-mtd, Boris Brezillon, linux-kernel, Frans Klaver
Hello Brian,
Am 04.01.2016 um 20:11 schrieb Brian Norris:
> Commits such as commit 853f1c58c4b2 ("mtd: nand: omap2: show parent
> device structure in sysfs") attempt to rely on the core MTD code to set
> the MTD name based on the parent device. However, nand_base tries to set
> a different default name according to the flash name (e.g., extracted
> from the ONFI parameter page), which means NAND drivers will never make
> use of the MTD defaults. This is not the intention of commit
> 853f1c58c4b2.
>
> This results in problems when trying to use the cmdline partition
> parser, since the MTD name is different than expected. Let's fix this by
> providing a default NAND name, where possible.
>
> Note that this is not really a great default name in the long run, since
> this means that if there are multiple MTDs attached to the same
> controller device, they will have the same name. But that is an existing
> issue and requires future work on a better controller vs. flash chip
> abstraction to fix properly.
>
> 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>
> ---
> This patch is needed in additon to commit 472b444eef93 ("mtd: fix cmdlinepart
> parser, early naming for auto-filled MTD") to fix Heiko's reported problem. At
> this point, I'm not sure if this should be targeted toward late 4.4 or for 4.5.
> It's a 4.4 regresssion, but a very small one. And I'm not sure if this will
> have wide enough impact that it should be given a longer time to be reviewed
> and tested. We can always send it to -stable later, if it's really needed.
Ok... but I wonder if nobody have the same problems as I ...
> drivers/mtd/nand/nand_base.c | 3 +++
> 1 file changed, 3 insertions(+)
Thanks!
works for me, so:
Tested-by: Heiko Schocher <hs@denx.de>
bye,
Heiko
>
> 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))
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [for-4.4] mtd: nand: assign reasonable default name for NAND drivers
2016-01-04 19:11 [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers Brian Norris
2016-01-04 20:31 ` Boris Brezillon
2016-01-05 6:11 ` Heiko Schocher
@ 2016-01-05 15:28 ` Sørensen, Stefan
2 siblings, 0 replies; 5+ messages in thread
From: Sørensen, Stefan @ 2016-01-05 15:28 UTC (permalink / raw)
To: computersforpeace@gmail.com, linux-mtd@lists.infradead.org
Cc: hs@denx.de, boris.brezillon@free-electrons.com,
linux-kernel@vger.kernel.org, fransklaver@gmail.com
Hi Brian,
On Mon, 2016-01-04 at 11:11 -0800, Brian Norris wrote:
> This patch is needed in additon to commit 472b444eef93 ("mtd: fix
> cmdlinepart parser, early naming for auto-filled MTD") to fix Heiko's
> reported problem. At this point, I'm not sure if this should be
> targeted toward late 4.4 or for 4.5. It's a 4.4 regresssion, but a
> very small one. And I'm not sure if this will have wide enough
> impact that it should be given a longer time to be reviewed and
> tested. We can always send it to -stable later, if it's really
> needed.
I have boards that will not boot 4.4 without these two patches, so I
hope to see it go in to 4.4.1 (I guess it is too late for 4.4).
Regards,
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread