Passing NAND mtdparts to OMAP2+ Kernel

Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

I posted this on the linux-omap list, and I was asked to post this on
the linux-mtd list:


I tried to remove the MTD partitions from the Linux device tree, and I
noticed that there was no partition information being pushed anymore
unless I changed the mtdparts name in U-Boot.

It appears as if the MTD drivers have changed a bit.  I found a few
e-mails floating around that attempt to fix this

Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
name for NAND drivers") attempts to address this, and someone over at
https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
well in a slightly different way.

Both of them refer to the same commit: 853f1c58c4b2 ("mtd: nand:
omap2: show parent device structure in sysfs") as the source for the
issue.

It now appears as if the name "omap2-nand.0" was replaced with
"30000000.nand"  Is this intentional? It seems like a lot of people
will have to update their bootloaders if they want to pass 'mtdparts'
from the bootloader to the kernel.

I am not a member of the MTD mailing list, so maybe this is more
properly sent over there, but since it's an OMAP related thing, I
thought I'd post it here.  My other ARM boards don't appear to be
impacted.  If the trend is to not pass the partition info from U-boot,
I can live with that too.  I was just more curious to know what the
long term plan is and what solution is recommended.

I have verified this in 4.11-RC4 and 4.9.18

adam

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

I posted this on the linux-omap list, and I was asked to post this on
the linux-mtd list:


I tried to remove the MTD partitions from the Linux device tree, and I
noticed that there was no partition information being pushed anymore
unless I changed the mtdparts name in U-Boot.

It appears as if the MTD drivers have changed a bit.  I found a few
e-mails floating around that attempt to fix this

Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
name for NAND drivers") attempts to address this, and someone over at
https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
well in a slightly different way.

Both of them refer to the same commit: 853f1c58c4b2 ("mtd: nand:
omap2: show parent device structure in sysfs") as the source for the
issue.

It now appears as if the name "omap2-nand.0" was replaced with
"30000000.nand"  Is this intentional? It seems like a lot of people
will have to update their bootloaders if they want to pass 'mtdparts'
from the bootloader to the kernel.

I am not a member of the MTD mailing list, so maybe this is more
properly sent over there, but since it's an OMAP related thing, I
thought I'd post it here.  My other ARM boards don't appear to be
impacted.  If the trend is to not pass the partition info from U-boot,
I can live with that too.  I was just more curious to know what the
long term plan is and what solution is recommended.

I have verified this in 4.11-RC4 and 4.9.18

adam

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

+Roger and Enrico

On Tue, 28 Mar 2017 10:43:01 -0500
Adam Ford <aford173@gmail.com> wrote:

> I posted this on the linux-omap list, and I was asked to post this on
> the linux-mtd list:
> 
> 
> I tried to remove the MTD partitions from the Linux device tree, and I
> noticed that there was no partition information being pushed anymore
> unless I changed the mtdparts name in U-Boot.
> 
> It appears as if the MTD drivers have changed a bit.  I found a few
> e-mails floating around that attempt to fix this
> 
> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> name for NAND drivers") attempts to address this, and someone over at
> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> well in a slightly different way.

Can you test the patch and let me know if solves the problem. If it
does, I'll send a clean version of the patch and queue it for 4.12.

Thanks,

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

+Roger and Enrico

On Tue, 28 Mar 2017 10:43:01 -0500
Adam Ford <aford173@gmail.com> wrote:

> I posted this on the linux-omap list, and I was asked to post this on
> the linux-mtd list:
> 
> 
> I tried to remove the MTD partitions from the Linux device tree, and I
> noticed that there was no partition information being pushed anymore
> unless I changed the mtdparts name in U-Boot.
> 
> It appears as if the MTD drivers have changed a bit.  I found a few
> e-mails floating around that attempt to fix this
> 
> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> name for NAND drivers") attempts to address this, and someone over at
> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> well in a slightly different way.

Can you test the patch and let me know if solves the problem. If it
does, I'll send a clean version of the patch and queue it for 4.12.

Thanks,

Boris

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> +Roger and Enrico
>
> On Tue, 28 Mar 2017 10:43:01 -0500
> Adam Ford <aford173@gmail.com> wrote:
>
>> I posted this on the linux-omap list, and I was asked to post this on
>> the linux-mtd list:
>>
>>
>> I tried to remove the MTD partitions from the Linux device tree, and I
>> noticed that there was no partition information being pushed anymore
>> unless I changed the mtdparts name in U-Boot.
>>
>> It appears as if the MTD drivers have changed a bit.  I found a few
>> e-mails floating around that attempt to fix this
>>
>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>> name for NAND drivers") attempts to address this, and someone over at
>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>> well in a slightly different way.
>
> Can you test the patch and let me know if solves the problem. If it
> does, I'll send a clean version of the patch and queue it for 4.12.
>

I tried to apply the patch directly, but it failed.  I then manually
copy-pasted it into the proper place, but it fails to compile.

drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
  mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
              ^~~~~~~~~~~~~
drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
from integer without a cast [-Wint-conversion]
  mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
            ^
cc1: some warnings being treated as errors

I use buildroot to build my toolchain and I am using gcc version 6.3.0
with glibc 2.24.  Is there supposed to be an include somewhere?  I am
not familiar with devm_kasprinf.


> Thanks,

Thank you

adam
>
> Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> +Roger and Enrico
>
> On Tue, 28 Mar 2017 10:43:01 -0500
> Adam Ford <aford173@gmail.com> wrote:
>
>> I posted this on the linux-omap list, and I was asked to post this on
>> the linux-mtd list:
>>
>>
>> I tried to remove the MTD partitions from the Linux device tree, and I
>> noticed that there was no partition information being pushed anymore
>> unless I changed the mtdparts name in U-Boot.
>>
>> It appears as if the MTD drivers have changed a bit.  I found a few
>> e-mails floating around that attempt to fix this
>>
>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>> name for NAND drivers") attempts to address this, and someone over at
>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>> well in a slightly different way.
>
> Can you test the patch and let me know if solves the problem. If it
> does, I'll send a clean version of the patch and queue it for 4.12.
>

I tried to apply the patch directly, but it failed.  I then manually
copy-pasted it into the proper place, but it fails to compile.

drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
  mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
              ^~~~~~~~~~~~~
drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
from integer without a cast [-Wint-conversion]
  mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
            ^
cc1: some warnings being treated as errors

I use buildroot to build my toolchain and I am using gcc version 6.3.0
with glibc 2.24.  Is there supposed to be an include somewhere?  I am
not familiar with devm_kasprinf.


> Thanks,

Thank you

adam
>
> Boris

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Roger Quadros]

Adam,

On 29/03/17 14:39, Adam Ford wrote:
> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
>> +Roger and Enrico
>>
>> On Tue, 28 Mar 2017 10:43:01 -0500
>> Adam Ford <aford173@gmail.com> wrote:
>>
>>> I posted this on the linux-omap list, and I was asked to post this on
>>> the linux-mtd list:
>>>
>>>
>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>> noticed that there was no partition information being pushed anymore
>>> unless I changed the mtdparts name in U-Boot.
>>>
>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>> e-mails floating around that attempt to fix this
>>>
>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>> name for NAND drivers") attempts to address this, and someone over at
>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>> well in a slightly different way.
>>
>> Can you test the patch and let me know if solves the problem. If it
>> does, I'll send a clean version of the patch and queue it for 4.12.
>>
> 
> I tried to apply the patch directly, but it failed.  I then manually
> copy-pasted it into the proper place, but it fails to compile.
> 
> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>               ^~~~~~~~~~~~~
> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>             ^
> cc1: some warnings being treated as errors
> 
> I use buildroot to build my toolchain and I am using gcc version 6.3.0
> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> not familiar with devm_kasprinf.
> 
> 

Does the below patch work for you?

cheers,
-roger

---
 drivers/mtd/nand/omap2.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2a52101..f693b8d 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->ecc.priv	= NULL;
 	nand_set_flash_node(nand_chip, dev->of_node);
 
+	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
+				   info->gpmc_cs);
+	if (!mtd->name) {
+		dev_err(&pdev->dev, "Failed to set MTD name\n");
+		return -ENOMEM;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(nand_chip->IO_ADDR_R))
-- 
2.7.4



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Roger Quadros]

Adam,

On 29/03/17 14:39, Adam Ford wrote:
> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
>> +Roger and Enrico
>>
>> On Tue, 28 Mar 2017 10:43:01 -0500
>> Adam Ford <aford173@gmail.com> wrote:
>>
>>> I posted this on the linux-omap list, and I was asked to post this on
>>> the linux-mtd list:
>>>
>>>
>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>> noticed that there was no partition information being pushed anymore
>>> unless I changed the mtdparts name in U-Boot.
>>>
>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>> e-mails floating around that attempt to fix this
>>>
>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>> name for NAND drivers") attempts to address this, and someone over at
>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>> well in a slightly different way.
>>
>> Can you test the patch and let me know if solves the problem. If it
>> does, I'll send a clean version of the patch and queue it for 4.12.
>>
> 
> I tried to apply the patch directly, but it failed.  I then manually
> copy-pasted it into the proper place, but it fails to compile.
> 
> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>               ^~~~~~~~~~~~~
> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>             ^
> cc1: some warnings being treated as errors
> 
> I use buildroot to build my toolchain and I am using gcc version 6.3.0
> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> not familiar with devm_kasprinf.
> 
> 

Does the below patch work for you?

cheers,
-roger

---
 drivers/mtd/nand/omap2.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2a52101..f693b8d 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->ecc.priv	= NULL;
 	nand_set_flash_node(nand_chip, dev->of_node);
 
+	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
+				   info->gpmc_cs);
+	if (!mtd->name) {
+		dev_err(&pdev->dev, "Failed to set MTD name\n");
+		return -ENOMEM;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(nand_chip->IO_ADDR_R))
-- 
2.7.4

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

On Wed, 29 Mar 2017 15:41:07 +0300
Roger Quadros <rogerq@ti.com> wrote:

> Adam,
> 
> On 29/03/17 14:39, Adam Ford wrote:
> > On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> >> +Roger and Enrico
> >>
> >> On Tue, 28 Mar 2017 10:43:01 -0500
> >> Adam Ford <aford173@gmail.com> wrote:
> >>  
> >>> I posted this on the linux-omap list, and I was asked to post this on
> >>> the linux-mtd list:
> >>>
> >>>
> >>> I tried to remove the MTD partitions from the Linux device tree, and I
> >>> noticed that there was no partition information being pushed anymore
> >>> unless I changed the mtdparts name in U-Boot.
> >>>
> >>> It appears as if the MTD drivers have changed a bit.  I found a few
> >>> e-mails floating around that attempt to fix this
> >>>
> >>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> >>> name for NAND drivers") attempts to address this, and someone over at
> >>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> >>> well in a slightly different way.  
> >>
> >> Can you test the patch and let me know if solves the problem. If it
> >> does, I'll send a clean version of the patch and queue it for 4.12.
> >>  
> > 
> > I tried to apply the patch directly, but it failed.  I then manually
> > copy-pasted it into the proper place, but it fails to compile.
> > 
> > drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> > drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> > function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
> >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> >               ^~~~~~~~~~~~~
> > drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> > from integer without a cast [-Wint-conversion]
> >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> >             ^
> > cc1: some warnings being treated as errors
> > 
> > I use buildroot to build my toolchain and I am using gcc version 6.3.0
> > with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> > not familiar with devm_kasprinf.

It's a typo: s/devm_kasprinf/devm_kasprintf/.

> > 
> >   
> 
> Does the below patch work for you?

Should work indeed.

I had a closer look and it seems that the bug was actually introduced
by c9711ec5250b mtd: nand: omap: Clean up device tree
support.
The parent device name has changed when switching to the new DT
representation: omap2-nand.0 (where 0 is the controller instance) became
30000000.nand (where 30000000 is the base reg address in the physical
address space). Which means my proposal was incorrect (0 is not the CS
line, it's the NAND controller instance number), so we'd better
statically set it to "omap2-nand.0".

> 
> cheers,
> -roger
> 
> ---
>  drivers/mtd/nand/omap2.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 2a52101..f693b8d 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	nand_chip->ecc.priv	= NULL;
>  	nand_set_flash_node(nand_chip, dev->of_node);
>  

nand_set_flash_node() is now taking the "label" DT property into account
and assigning mtd->name to this value if present. I'd recommend doing

	if (!mtd->name)
		mtd->name = "omap2-nand.0";

> +	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
> +				   info->gpmc_cs);
> +	if (!mtd->name) {
> +		dev_err(&pdev->dev, "Failed to set MTD name\n");
> +		return -ENOMEM;
> +	}
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(nand_chip->IO_ADDR_R))


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

On Wed, 29 Mar 2017 15:41:07 +0300
Roger Quadros <rogerq@ti.com> wrote:

> Adam,
> 
> On 29/03/17 14:39, Adam Ford wrote:
> > On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> >> +Roger and Enrico
> >>
> >> On Tue, 28 Mar 2017 10:43:01 -0500
> >> Adam Ford <aford173@gmail.com> wrote:
> >>  
> >>> I posted this on the linux-omap list, and I was asked to post this on
> >>> the linux-mtd list:
> >>>
> >>>
> >>> I tried to remove the MTD partitions from the Linux device tree, and I
> >>> noticed that there was no partition information being pushed anymore
> >>> unless I changed the mtdparts name in U-Boot.
> >>>
> >>> It appears as if the MTD drivers have changed a bit.  I found a few
> >>> e-mails floating around that attempt to fix this
> >>>
> >>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> >>> name for NAND drivers") attempts to address this, and someone over at
> >>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> >>> well in a slightly different way.  
> >>
> >> Can you test the patch and let me know if solves the problem. If it
> >> does, I'll send a clean version of the patch and queue it for 4.12.
> >>  
> > 
> > I tried to apply the patch directly, but it failed.  I then manually
> > copy-pasted it into the proper place, but it fails to compile.
> > 
> > drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> > drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> > function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
> >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> >               ^~~~~~~~~~~~~
> > drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> > from integer without a cast [-Wint-conversion]
> >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> >             ^
> > cc1: some warnings being treated as errors
> > 
> > I use buildroot to build my toolchain and I am using gcc version 6.3.0
> > with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> > not familiar with devm_kasprinf.

It's a typo: s/devm_kasprinf/devm_kasprintf/.

> > 
> >   
> 
> Does the below patch work for you?

Should work indeed.

I had a closer look and it seems that the bug was actually introduced
by c9711ec5250b mtd: nand: omap: Clean up device tree
support.
The parent device name has changed when switching to the new DT
representation: omap2-nand.0 (where 0 is the controller instance) became
30000000.nand (where 30000000 is the base reg address in the physical
address space). Which means my proposal was incorrect (0 is not the CS
line, it's the NAND controller instance number), so we'd better
statically set it to "omap2-nand.0".

> 
> cheers,
> -roger
> 
> ---
>  drivers/mtd/nand/omap2.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 2a52101..f693b8d 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	nand_chip->ecc.priv	= NULL;
>  	nand_set_flash_node(nand_chip, dev->of_node);
>  

nand_set_flash_node() is now taking the "label" DT property into account
and assigning mtd->name to this value if present. I'd recommend doing

	if (!mtd->name)
		mtd->name = "omap2-nand.0";

> +	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
> +				   info->gpmc_cs);
> +	if (!mtd->name) {
> +		dev_err(&pdev->dev, "Failed to set MTD name\n");
> +		return -ENOMEM;
> +	}
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(nand_chip->IO_ADDR_R))

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

On Wed, 29 Mar 2017 15:10:02 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Wed, 29 Mar 2017 15:41:07 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
> > Adam,
> > 
> > On 29/03/17 14:39, Adam Ford wrote:  
> > > On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> > > <boris.brezillon@free-electrons.com> wrote:    
> > >> +Roger and Enrico
> > >>
> > >> On Tue, 28 Mar 2017 10:43:01 -0500
> > >> Adam Ford <aford173@gmail.com> wrote:
> > >>    
> > >>> I posted this on the linux-omap list, and I was asked to post this on
> > >>> the linux-mtd list:
> > >>>
> > >>>
> > >>> I tried to remove the MTD partitions from the Linux device tree, and I
> > >>> noticed that there was no partition information being pushed anymore
> > >>> unless I changed the mtdparts name in U-Boot.
> > >>>
> > >>> It appears as if the MTD drivers have changed a bit.  I found a few
> > >>> e-mails floating around that attempt to fix this
> > >>>
> > >>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> > >>> name for NAND drivers") attempts to address this, and someone over at
> > >>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> > >>> well in a slightly different way.    
> > >>
> > >> Can you test the patch and let me know if solves the problem. If it
> > >> does, I'll send a clean version of the patch and queue it for 4.12.
> > >>    
> > > 
> > > I tried to apply the patch directly, but it failed.  I then manually
> > > copy-pasted it into the proper place, but it fails to compile.
> > > 
> > > drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> > > drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> > > function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
> > >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> > >               ^~~~~~~~~~~~~
> > > drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> > > from integer without a cast [-Wint-conversion]
> > >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> > >             ^
> > > cc1: some warnings being treated as errors
> > > 
> > > I use buildroot to build my toolchain and I am using gcc version 6.3.0
> > > with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> > > not familiar with devm_kasprinf.  
> 
> It's a typo: s/devm_kasprinf/devm_kasprintf/.
> 
> > > 
> > >     
> > 
> > Does the below patch work for you?  
> 
> Should work indeed.
> 
> I had a closer look and it seems that the bug was actually introduced
> by c9711ec5250b mtd: nand: omap: Clean up device tree
> support.
> The parent device name has changed when switching to the new DT
> representation: omap2-nand.0 (where 0 is the controller instance) became
> 30000000.nand (where 30000000 is the base reg address in the physical
> address space). Which means my proposal was incorrect (0 is not the CS
> line, it's the NAND controller instance number), so we'd better
> statically set it to "omap2-nand.0".

Changing my mind (again :)). According to [1], the instance id is
related to the CS line, so devm_kasprintf() is the right solution.

Sorry for the noise.

> 
> > 
> > cheers,
> > -roger
> > 
> > ---
> >  drivers/mtd/nand/omap2.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index 2a52101..f693b8d 100644
> > --- a/drivers/mtd/nand/omap2.c
> > +++ b/drivers/mtd/nand/omap2.c
> > @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
> >  	nand_chip->ecc.priv	= NULL;
> >  	nand_set_flash_node(nand_chip, dev->of_node);
> >    
> 
> nand_set_flash_node() is now taking the "label" DT property into account
> and assigning mtd->name to this value if present. I'd recommend doing
> 
> 	if (!mtd->name)
> 		mtd->name = "omap2-nand.0";

This comment still stands, except it should be:

	if (!mtd->name)
		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
					   "omap2-nand.%d", info->gpmc_cs);

	if (!mtd->name) {
		dev_err(&pdev->dev, "Failed to set MTD name\n");
		return -ENOMEM;
	}

Regards,

Boris

[1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Boris Brezillon]

On Wed, 29 Mar 2017 15:10:02 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Wed, 29 Mar 2017 15:41:07 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
> > Adam,
> > 
> > On 29/03/17 14:39, Adam Ford wrote:  
> > > On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
> > > <boris.brezillon@free-electrons.com> wrote:    
> > >> +Roger and Enrico
> > >>
> > >> On Tue, 28 Mar 2017 10:43:01 -0500
> > >> Adam Ford <aford173@gmail.com> wrote:
> > >>    
> > >>> I posted this on the linux-omap list, and I was asked to post this on
> > >>> the linux-mtd list:
> > >>>
> > >>>
> > >>> I tried to remove the MTD partitions from the Linux device tree, and I
> > >>> noticed that there was no partition information being pushed anymore
> > >>> unless I changed the mtdparts name in U-Boot.
> > >>>
> > >>> It appears as if the MTD drivers have changed a bit.  I found a few
> > >>> e-mails floating around that attempt to fix this
> > >>>
> > >>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
> > >>> name for NAND drivers") attempts to address this, and someone over at
> > >>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
> > >>> well in a slightly different way.    
> > >>
> > >> Can you test the patch and let me know if solves the problem. If it
> > >> does, I'll send a clean version of the patch and queue it for 4.12.
> > >>    
> > > 
> > > I tried to apply the patch directly, but it failed.  I then manually
> > > copy-pasted it into the proper place, but it fails to compile.
> > > 
> > > drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
> > > drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
> > > function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
> > >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> > >               ^~~~~~~~~~~~~
> > > drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
> > > from integer without a cast [-Wint-conversion]
> > >   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
> > >             ^
> > > cc1: some warnings being treated as errors
> > > 
> > > I use buildroot to build my toolchain and I am using gcc version 6.3.0
> > > with glibc 2.24.  Is there supposed to be an include somewhere?  I am
> > > not familiar with devm_kasprinf.  
> 
> It's a typo: s/devm_kasprinf/devm_kasprintf/.
> 
> > > 
> > >     
> > 
> > Does the below patch work for you?  
> 
> Should work indeed.
> 
> I had a closer look and it seems that the bug was actually introduced
> by c9711ec5250b mtd: nand: omap: Clean up device tree
> support.
> The parent device name has changed when switching to the new DT
> representation: omap2-nand.0 (where 0 is the controller instance) became
> 30000000.nand (where 30000000 is the base reg address in the physical
> address space). Which means my proposal was incorrect (0 is not the CS
> line, it's the NAND controller instance number), so we'd better
> statically set it to "omap2-nand.0".

Changing my mind (again :)). According to [1], the instance id is
related to the CS line, so devm_kasprintf() is the right solution.

Sorry for the noise.

> 
> > 
> > cheers,
> > -roger
> > 
> > ---
> >  drivers/mtd/nand/omap2.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index 2a52101..f693b8d 100644
> > --- a/drivers/mtd/nand/omap2.c
> > +++ b/drivers/mtd/nand/omap2.c
> > @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
> >  	nand_chip->ecc.priv	= NULL;
> >  	nand_set_flash_node(nand_chip, dev->of_node);
> >    
> 
> nand_set_flash_node() is now taking the "label" DT property into account
> and assigning mtd->name to this value if present. I'd recommend doing
> 
> 	if (!mtd->name)
> 		mtd->name = "omap2-nand.0";

This comment still stands, except it should be:

	if (!mtd->name)
		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
					   "omap2-nand.%d", info->gpmc_cs);

	if (!mtd->name) {
		dev_err(&pdev->dev, "Failed to set MTD name\n");
		return -ENOMEM;
	}

Regards,

Boris

[1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Roger Quadros]

On 29/03/17 16:18, Boris Brezillon wrote:
> On Wed, 29 Mar 2017 15:10:02 +0200
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
>> On Wed, 29 Mar 2017 15:41:07 +0300
>> Roger Quadros <rogerq@ti.com> wrote:
>>
>>> Adam,
>>>
>>> On 29/03/17 14:39, Adam Ford wrote:  
>>>> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
>>>> <boris.brezillon@free-electrons.com> wrote:    
>>>>> +Roger and Enrico
>>>>>
>>>>> On Tue, 28 Mar 2017 10:43:01 -0500
>>>>> Adam Ford <aford173@gmail.com> wrote:
>>>>>    
>>>>>> I posted this on the linux-omap list, and I was asked to post this on
>>>>>> the linux-mtd list:
>>>>>>
>>>>>>
>>>>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>>>>> noticed that there was no partition information being pushed anymore
>>>>>> unless I changed the mtdparts name in U-Boot.
>>>>>>
>>>>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>>>>> e-mails floating around that attempt to fix this
>>>>>>
>>>>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>>>>> name for NAND drivers") attempts to address this, and someone over at
>>>>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>>>>> well in a slightly different way.    
>>>>>
>>>>> Can you test the patch and let me know if solves the problem. If it
>>>>> does, I'll send a clean version of the patch and queue it for 4.12.
>>>>>    
>>>>
>>>> I tried to apply the patch directly, but it failed.  I then manually
>>>> copy-pasted it into the proper place, but it fails to compile.
>>>>
>>>> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
>>>> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
>>>> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>               ^~~~~~~~~~~~~
>>>> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
>>>> from integer without a cast [-Wint-conversion]
>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>             ^
>>>> cc1: some warnings being treated as errors
>>>>
>>>> I use buildroot to build my toolchain and I am using gcc version 6.3.0
>>>> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
>>>> not familiar with devm_kasprinf.  
>>
>> It's a typo: s/devm_kasprinf/devm_kasprintf/.
>>
>>>>
>>>>     
>>>
>>> Does the below patch work for you?  
>>
>> Should work indeed.
>>
>> I had a closer look and it seems that the bug was actually introduced
>> by c9711ec5250b mtd: nand: omap: Clean up device tree
>> support.
>> The parent device name has changed when switching to the new DT
>> representation: omap2-nand.0 (where 0 is the controller instance) became
>> 30000000.nand (where 30000000 is the base reg address in the physical
>> address space). Which means my proposal was incorrect (0 is not the CS
>> line, it's the NAND controller instance number), so we'd better
>> statically set it to "omap2-nand.0".
> 
> Changing my mind (again :)). According to [1], the instance id is
> related to the CS line, so devm_kasprintf() is the right solution.
> 
> Sorry for the noise.
> 
>>
>>>
>>> cheers,
>>> -roger
>>>
>>> ---
>>>  drivers/mtd/nand/omap2.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>> index 2a52101..f693b8d 100644
>>> --- a/drivers/mtd/nand/omap2.c
>>> +++ b/drivers/mtd/nand/omap2.c
>>> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>  	nand_chip->ecc.priv	= NULL;
>>>  	nand_set_flash_node(nand_chip, dev->of_node);
>>>    
>>
>> nand_set_flash_node() is now taking the "label" DT property into account
>> and assigning mtd->name to this value if present. I'd recommend doing
>>
>> 	if (!mtd->name)
>> 		mtd->name = "omap2-nand.0";
> 
> This comment still stands, except it should be:
> 
> 	if (!mtd->name)
> 		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> 					   "omap2-nand.%d", info->gpmc_cs);
> 
> 	if (!mtd->name) {
> 		dev_err(&pdev->dev, "Failed to set MTD name\n");
> 		return -ENOMEM;
> 	}
> 
> Regards,
> 
> Boris
> 
> [1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133
> 

OK. Here is the updated patch. I'll send an proper patch by tomorrow.

Adam, please test it on your board if possible. Thanks.

---
 drivers/mtd/nand/omap2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2a52101..1b3d7cc 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,6 +1856,15 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->ecc.priv	= NULL;
 	nand_set_flash_node(nand_chip, dev->of_node);
 
+	if (!mtd->name) {
+		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
+					   info->gpmc_cs);
+		if (!mtd->name) {
+			dev_err(&pdev->dev, "Failed to set MTD name\n");
+			return -ENOMEM;
+		}
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(nand_chip->IO_ADDR_R))
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Roger Quadros]

On 29/03/17 16:18, Boris Brezillon wrote:
> On Wed, 29 Mar 2017 15:10:02 +0200
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
>> On Wed, 29 Mar 2017 15:41:07 +0300
>> Roger Quadros <rogerq@ti.com> wrote:
>>
>>> Adam,
>>>
>>> On 29/03/17 14:39, Adam Ford wrote:  
>>>> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
>>>> <boris.brezillon@free-electrons.com> wrote:    
>>>>> +Roger and Enrico
>>>>>
>>>>> On Tue, 28 Mar 2017 10:43:01 -0500
>>>>> Adam Ford <aford173@gmail.com> wrote:
>>>>>    
>>>>>> I posted this on the linux-omap list, and I was asked to post this on
>>>>>> the linux-mtd list:
>>>>>>
>>>>>>
>>>>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>>>>> noticed that there was no partition information being pushed anymore
>>>>>> unless I changed the mtdparts name in U-Boot.
>>>>>>
>>>>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>>>>> e-mails floating around that attempt to fix this
>>>>>>
>>>>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>>>>> name for NAND drivers") attempts to address this, and someone over at
>>>>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>>>>> well in a slightly different way.    
>>>>>
>>>>> Can you test the patch and let me know if solves the problem. If it
>>>>> does, I'll send a clean version of the patch and queue it for 4.12.
>>>>>    
>>>>
>>>> I tried to apply the patch directly, but it failed.  I then manually
>>>> copy-pasted it into the proper place, but it fails to compile.
>>>>
>>>> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
>>>> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
>>>> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>               ^~~~~~~~~~~~~
>>>> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
>>>> from integer without a cast [-Wint-conversion]
>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>             ^
>>>> cc1: some warnings being treated as errors
>>>>
>>>> I use buildroot to build my toolchain and I am using gcc version 6.3.0
>>>> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
>>>> not familiar with devm_kasprinf.  
>>
>> It's a typo: s/devm_kasprinf/devm_kasprintf/.
>>
>>>>
>>>>     
>>>
>>> Does the below patch work for you?  
>>
>> Should work indeed.
>>
>> I had a closer look and it seems that the bug was actually introduced
>> by c9711ec5250b mtd: nand: omap: Clean up device tree
>> support.
>> The parent device name has changed when switching to the new DT
>> representation: omap2-nand.0 (where 0 is the controller instance) became
>> 30000000.nand (where 30000000 is the base reg address in the physical
>> address space). Which means my proposal was incorrect (0 is not the CS
>> line, it's the NAND controller instance number), so we'd better
>> statically set it to "omap2-nand.0".
> 
> Changing my mind (again :)). According to [1], the instance id is
> related to the CS line, so devm_kasprintf() is the right solution.
> 
> Sorry for the noise.
> 
>>
>>>
>>> cheers,
>>> -roger
>>>
>>> ---
>>>  drivers/mtd/nand/omap2.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>> index 2a52101..f693b8d 100644
>>> --- a/drivers/mtd/nand/omap2.c
>>> +++ b/drivers/mtd/nand/omap2.c
>>> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>  	nand_chip->ecc.priv	= NULL;
>>>  	nand_set_flash_node(nand_chip, dev->of_node);
>>>    
>>
>> nand_set_flash_node() is now taking the "label" DT property into account
>> and assigning mtd->name to this value if present. I'd recommend doing
>>
>> 	if (!mtd->name)
>> 		mtd->name = "omap2-nand.0";
> 
> This comment still stands, except it should be:
> 
> 	if (!mtd->name)
> 		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> 					   "omap2-nand.%d", info->gpmc_cs);
> 
> 	if (!mtd->name) {
> 		dev_err(&pdev->dev, "Failed to set MTD name\n");
> 		return -ENOMEM;
> 	}
> 
> Regards,
> 
> Boris
> 
> [1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133
> 

OK. Here is the updated patch. I'll send an proper patch by tomorrow.

Adam, please test it on your board if possible. Thanks.

---
 drivers/mtd/nand/omap2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2a52101..1b3d7cc 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,6 +1856,15 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->ecc.priv	= NULL;
 	nand_set_flash_node(nand_chip, dev->of_node);
 
+	if (!mtd->name) {
+		mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
+					   info->gpmc_cs);
+		if (!mtd->name) {
+			dev_err(&pdev->dev, "Failed to set MTD name\n");
+			return -ENOMEM;
+		}
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(nand_chip->IO_ADDR_R))
-- 
2.7.4

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

On Wed, Mar 29, 2017 at 9:06 AM, Roger Quadros <rogerq@ti.com> wrote:
> On 29/03/17 16:18, Boris Brezillon wrote:
>> On Wed, 29 Mar 2017 15:10:02 +0200
>> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>>
>>> On Wed, 29 Mar 2017 15:41:07 +0300
>>> Roger Quadros <rogerq@ti.com> wrote:
>>>
>>>> Adam,
>>>>
>>>> On 29/03/17 14:39, Adam Ford wrote:
>>>>> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
>>>>> <boris.brezillon@free-electrons.com> wrote:
>>>>>> +Roger and Enrico
>>>>>>
>>>>>> On Tue, 28 Mar 2017 10:43:01 -0500
>>>>>> Adam Ford <aford173@gmail.com> wrote:
>>>>>>
>>>>>>> I posted this on the linux-omap list, and I was asked to post this on
>>>>>>> the linux-mtd list:
>>>>>>>
>>>>>>>
>>>>>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>>>>>> noticed that there was no partition information being pushed anymore
>>>>>>> unless I changed the mtdparts name in U-Boot.
>>>>>>>
>>>>>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>>>>>> e-mails floating around that attempt to fix this
>>>>>>>
>>>>>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>>>>>> name for NAND drivers") attempts to address this, and someone over at
>>>>>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>>>>>> well in a slightly different way.
>>>>>>
>>>>>> Can you test the patch and let me know if solves the problem. If it
>>>>>> does, I'll send a clean version of the patch and queue it for 4.12.
>>>>>>
>>>>>
>>>>> I tried to apply the patch directly, but it failed.  I then manually
>>>>> copy-pasted it into the proper place, but it fails to compile.
>>>>>
>>>>> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
>>>>> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
>>>>> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>>               ^~~~~~~~~~~~~
>>>>> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
>>>>> from integer without a cast [-Wint-conversion]
>>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>>             ^
>>>>> cc1: some warnings being treated as errors
>>>>>
>>>>> I use buildroot to build my toolchain and I am using gcc version 6.3.0
>>>>> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
>>>>> not familiar with devm_kasprinf.
>>>
>>> It's a typo: s/devm_kasprinf/devm_kasprintf/.
>>>
>>>>>
>>>>>
>>>>
>>>> Does the below patch work for you?
>>>
>>> Should work indeed.
>>>
>>> I had a closer look and it seems that the bug was actually introduced
>>> by c9711ec5250b mtd: nand: omap: Clean up device tree
>>> support.
>>> The parent device name has changed when switching to the new DT
>>> representation: omap2-nand.0 (where 0 is the controller instance) became
>>> 30000000.nand (where 30000000 is the base reg address in the physical
>>> address space). Which means my proposal was incorrect (0 is not the CS
>>> line, it's the NAND controller instance number), so we'd better
>>> statically set it to "omap2-nand.0".
>>
>> Changing my mind (again :)). According to [1], the instance id is
>> related to the CS line, so devm_kasprintf() is the right solution.
>>
>> Sorry for the noise.
>>
>>>
>>>>
>>>> cheers,
>>>> -roger
>>>>
>>>> ---
>>>>  drivers/mtd/nand/omap2.c | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>>> index 2a52101..f693b8d 100644
>>>> --- a/drivers/mtd/nand/omap2.c
>>>> +++ b/drivers/mtd/nand/omap2.c
>>>> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>>     nand_chip->ecc.priv     = NULL;
>>>>     nand_set_flash_node(nand_chip, dev->of_node);
>>>>
>>>
>>> nand_set_flash_node() is now taking the "label" DT property into account
>>> and assigning mtd->name to this value if present. I'd recommend doing
>>>
>>>      if (!mtd->name)
>>>              mtd->name = "omap2-nand.0";
>>
>> This comment still stands, except it should be:
>>
>>       if (!mtd->name)
>>               mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
>>                                          "omap2-nand.%d", info->gpmc_cs);
>>
>>       if (!mtd->name) {
>>               dev_err(&pdev->dev, "Failed to set MTD name\n");
>>               return -ENOMEM;
>>       }
>>
>> Regards,
>>
>> Boris
>>
>> [1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133
>>
>
> OK. Here is the updated patch. I'll send an proper patch by tomorrow.
>
> Adam, please test it on your board if possible. Thanks.

I tested this against Linux master (4.11.0-rc4) and it works for me.
Thank you.  Go ahead and add me as 'tested-by' when you do you final
patch submission.


adam
>
> ---
>  drivers/mtd/nand/omap2.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 2a52101..1b3d7cc 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1856,6 +1856,15 @@ static int omap_nand_probe(struct platform_device *pdev)
>         nand_chip->ecc.priv     = NULL;
>         nand_set_flash_node(nand_chip, dev->of_node);
>
> +       if (!mtd->name) {
> +               mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
> +                                          info->gpmc_cs);
> +               if (!mtd->name) {
> +                       dev_err(&pdev->dev, "Failed to set MTD name\n");
> +                       return -ENOMEM;
> +               }
> +       }
> +
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
>         if (IS_ERR(nand_chip->IO_ADDR_R))
> --
> 2.7.4
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Passing NAND mtdparts to OMAP2+ Kernel

[Adam Ford]

On Wed, Mar 29, 2017 at 9:06 AM, Roger Quadros <rogerq@ti.com> wrote:
> On 29/03/17 16:18, Boris Brezillon wrote:
>> On Wed, 29 Mar 2017 15:10:02 +0200
>> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>>
>>> On Wed, 29 Mar 2017 15:41:07 +0300
>>> Roger Quadros <rogerq@ti.com> wrote:
>>>
>>>> Adam,
>>>>
>>>> On 29/03/17 14:39, Adam Ford wrote:
>>>>> On Tue, Mar 28, 2017 at 2:57 PM, Boris Brezillon
>>>>> <boris.brezillon@free-electrons.com> wrote:
>>>>>> +Roger and Enrico
>>>>>>
>>>>>> On Tue, 28 Mar 2017 10:43:01 -0500
>>>>>> Adam Ford <aford173@gmail.com> wrote:
>>>>>>
>>>>>>> I posted this on the linux-omap list, and I was asked to post this on
>>>>>>> the linux-mtd list:
>>>>>>>
>>>>>>>
>>>>>>> I tried to remove the MTD partitions from the Linux device tree, and I
>>>>>>> noticed that there was no partition information being pushed anymore
>>>>>>> unless I changed the mtdparts name in U-Boot.
>>>>>>>
>>>>>>> It appears as if the MTD drivers have changed a bit.  I found a few
>>>>>>> e-mails floating around that attempt to fix this
>>>>>>>
>>>>>>> Commit f7a8e38f07a17be907585 ("mtd: nand: assign reasonable default
>>>>>>> name for NAND drivers") attempts to address this, and someone over at
>>>>>>> https://patchwork.ozlabs.org/patch/707065/ attempted to address it as
>>>>>>> well in a slightly different way.
>>>>>>
>>>>>> Can you test the patch and let me know if solves the problem. If it
>>>>>> does, I'll send a clean version of the patch and queue it for 4.12.
>>>>>>
>>>>>
>>>>> I tried to apply the patch directly, but it failed.  I then manually
>>>>> copy-pasted it into the proper place, but it fails to compile.
>>>>>
>>>>> drivers/mtd/nand/omap2.c: In function ‘omap_nand_probe’:
>>>>> drivers/mtd/nand/omap2.c:1859:14: error: implicit declaration of
>>>>> function ‘devm_kasprinf’ [-Werror=implicit-function-declaration]
>>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>>               ^~~~~~~~~~~~~
>>>>> drivers/mtd/nand/omap2.c:1859:12: warning: assignment makes pointer
>>>>> from integer without a cast [-Wint-conversion]
>>>>>   mtd->name = devm_kasprinf(&pdev->dev, "omap2-nand.%d", info->gpmc_cs);
>>>>>             ^
>>>>> cc1: some warnings being treated as errors
>>>>>
>>>>> I use buildroot to build my toolchain and I am using gcc version 6.3.0
>>>>> with glibc 2.24.  Is there supposed to be an include somewhere?  I am
>>>>> not familiar with devm_kasprinf.
>>>
>>> It's a typo: s/devm_kasprinf/devm_kasprintf/.
>>>
>>>>>
>>>>>
>>>>
>>>> Does the below patch work for you?
>>>
>>> Should work indeed.
>>>
>>> I had a closer look and it seems that the bug was actually introduced
>>> by c9711ec5250b mtd: nand: omap: Clean up device tree
>>> support.
>>> The parent device name has changed when switching to the new DT
>>> representation: omap2-nand.0 (where 0 is the controller instance) became
>>> 30000000.nand (where 30000000 is the base reg address in the physical
>>> address space). Which means my proposal was incorrect (0 is not the CS
>>> line, it's the NAND controller instance number), so we'd better
>>> statically set it to "omap2-nand.0".
>>
>> Changing my mind (again :)). According to [1], the instance id is
>> related to the CS line, so devm_kasprintf() is the right solution.
>>
>> Sorry for the noise.
>>
>>>
>>>>
>>>> cheers,
>>>> -roger
>>>>
>>>> ---
>>>>  drivers/mtd/nand/omap2.c | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>>> index 2a52101..f693b8d 100644
>>>> --- a/drivers/mtd/nand/omap2.c
>>>> +++ b/drivers/mtd/nand/omap2.c
>>>> @@ -1856,6 +1856,13 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>>     nand_chip->ecc.priv     = NULL;
>>>>     nand_set_flash_node(nand_chip, dev->of_node);
>>>>
>>>
>>> nand_set_flash_node() is now taking the "label" DT property into account
>>> and assigning mtd->name to this value if present. I'd recommend doing
>>>
>>>      if (!mtd->name)
>>>              mtd->name = "omap2-nand.0";
>>
>> This comment still stands, except it should be:
>>
>>       if (!mtd->name)
>>               mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
>>                                          "omap2-nand.%d", info->gpmc_cs);
>>
>>       if (!mtd->name) {
>>               dev_err(&pdev->dev, "Failed to set MTD name\n");
>>               return -ENOMEM;
>>       }
>>
>> Regards,
>>
>> Boris
>>
>> [1]http://lxr.free-electrons.com/source/arch/arm/mach-omap2/gpmc-nand.c?v=4.6#L133
>>
>
> OK. Here is the updated patch. I'll send an proper patch by tomorrow.
>
> Adam, please test it on your board if possible. Thanks.

I tested this against Linux master (4.11.0-rc4) and it works for me.
Thank you.  Go ahead and add me as 'tested-by' when you do you final
patch submission.


adam
>
> ---
>  drivers/mtd/nand/omap2.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 2a52101..1b3d7cc 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1856,6 +1856,15 @@ static int omap_nand_probe(struct platform_device *pdev)
>         nand_chip->ecc.priv     = NULL;
>         nand_set_flash_node(nand_chip, dev->of_node);
>
> +       if (!mtd->name) {
> +               mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "omap2-nand.%d",
> +                                          info->gpmc_cs);
> +               if (!mtd->name) {
> +                       dev_err(&pdev->dev, "Failed to set MTD name\n");
> +                       return -ENOMEM;
> +               }
> +       }
> +
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
>         if (IS_ERR(nand_chip->IO_ADDR_R))
> --
> 2.7.4
>