Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300981 > unrolled thread

[PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers

Back to article view | Back to linux.kernel

Contents

  [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers Brian Norris <computersforpeace@gmail.com> - 2016-01-04 20:20 +0100
    Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for  NAND drivers Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 21:40 +0100
      Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for  NAND drivers Brian Norris <computersforpeace@gmail.com> - 2016-01-05 19:30 +0100
    Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for  NAND drivers Heiko Schocher <hs@denx.de> - 2016-01-05 07:20 +0100
    Re: [for-4.4] mtd: nand: assign reasonable default name for NAND  drivers Sørensen, Stefan   <Stefan.Sorensen@spectralink.com> - 2016-01-05 16:30 +0100

#1300981 — [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers

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.

 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);
+
 	for (; type->name != NULL; type++) {
 		if (is_full_id_nand(type)) {
 			if (find_full_id_nand(mtd, chip, type, id_data, &busw))
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1301054 — Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301851 — Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301286 — Re: [PATCH for-4.4] mtd: nand: assign reasonable default name for NAND drivers

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301598 — Re: [for-4.4] mtd: nand: assign reasonable default name for NAND drivers

SGkgQnJpYW4sDQoNCk9uIE1vbiwgMjAxNi0wMS0wNCBhdCAxMToxMSAtMDgwMCwgQnJpYW4gTm9y
cmlzIHdyb3RlOg0KPiBUaGlzIHBhdGNoIGlzIG5lZWRlZCBpbiBhZGRpdG9uIHRvIGNvbW1pdCA0
NzJiNDQ0ZWVmOTMgKCJtdGQ6IGZpeCANCj4gY21kbGluZXBhcnQgcGFyc2VyLCBlYXJseSBuYW1p
bmcgZm9yIGF1dG8tZmlsbGVkIE1URCIpIHRvIGZpeCBIZWlrbydzDQo+IHJlcG9ydGVkIHByb2Js
ZW0uIEF0IHRoaXMgcG9pbnQsIEknbSBub3Qgc3VyZSBpZiB0aGlzIHNob3VsZCBiZQ0KPiB0YXJn
ZXRlZCB0b3dhcmQgbGF0ZSA0LjQgb3IgZm9yIDQuNS4gSXQncyBhIDQuNCByZWdyZXNzc2lvbiwg
YnV0IGENCj4gdmVyeSBzbWFsbCBvbmUuIEFuZCBJJ20gbm90IHN1cmUgaWYgdGhpcyB3aWxsIGhh
dmUgd2lkZSBlbm91Z2gNCj4gaW1wYWN0wqB0aGF0IGl0IHNob3VsZCBiZSBnaXZlbiBhIGxvbmdl
ciB0aW1lIHRvIGJlIHJldmlld2VkIGFuZA0KPiB0ZXN0ZWQuIFdlIGNhbiBhbHdheXMgc2VuZCBp
dCB0byAtc3RhYmxlIGxhdGVyLCBpZiBpdCdzIHJlYWxseQ0KPiBuZWVkZWQuDQoNCkkgaGF2ZSBi
b2FyZHMgdGhhdCB3aWxsIG5vdCBib290IDQuNCB3aXRob3V0IHRoZXNlIHR3byBwYXRjaGVzLCBz
byBJDQpob3BlIHRvIHNlZSBpdCBnbyBpbiB0byA0LjQuMSAoSSBndWVzcyBpdCBpcyB0b28gbGF0
ZSBmb3IgNC40KS4NCg0KUmVnYXJkcyzCoA0KwqBTdGVmYW4=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web