| Message ID | 04afe149-24b2-daec-5ef0-2d662ed79040@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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))