Re: mtd, nand, omap2: how to pass the NAND device name to mtdparts ?

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Roger, Enrico,

On Mon, 19 Dec 2016 13:11:48 +0200
Roger Quadros <rogerq@ti.com> wrote:

> Hi,
> 
> On 19/12/16 11:30, Leto, Enrico wrote:
> > Hello All,
> > 
> > I'm trying to port some projects to kernel 4.4. It crashes because the driver name given by mtdparts (omap2-nand.0) is no more accepted. It's running fine if I replace it with the device name (e.g. S34ML04G1).  
> 
> What crashes? Is it a user space application?

IIUC, Enrico complains about the mtdparts vs in-kernel device name
mismatch which prevents partition creation. Indeed, the first
information passed through the mtdparts parameter is the MTD device
name, and this name is usually defined by the NAND controller driver.
If the controller driver does not set the name, then the core assign
one based on the NAND model name.
But, in this case, if the MTD device name, does not match the mtdparts
one, then the cmdline partition parser will never creates the
partitions.

> If it is then it shouldn't be
> depending on the platform device name for anything.

I agree, but that's how it currently works. I remember having a
discussion with Brian regarding a standardized naming scheme, but it's
not been implemented yet.

Usually, on a given board, NAND devices are identified by the
controller and the CS line they are connected to. I guess this is what
omap2-nand.0 means: omap2-nand controller, CS0 line.

> 
> The mtd_dev_info has the mtd device name but I don't think it contains
> the chip name. Rather it will contain MTD type and partition name.

Actually, for partitions, it will contain the partition name, not the
chip name. But here the problem is not on the partition, but on the
device itself (which is used by the cmdline partition parser to
identify whether partitions should be created for a specific device or
not). 

> 
> The chip names are cryptic and mean nothing to user space. Why do you
> want user space to depend on chip names?

I don't think that's what Enrico asked for, but I might be wrong.

> 
> > 
> > We use different NANDs from different manufacturers. I'm searching for a convenient solution to pass the device name to mtdparts.
> > 
> > Should I read the device name from u-boot and to pass it to mtdparts?  

No, please don't do that.

> 
> Kernel can also read the chip model so we don't need to depend on u-boot.
> 
> > Or some other suggestion or recommendation how to do that ?  
> 
> As the kernel mtd driver (nand/spi/etc) already knows this information, it is
> a matter of just providing it to userspace. Maybe adding another sysfs parameter
> like modelname or something will do.

The MTD device name is already exported (/sys/class/mtd/mtdX/name). We
could export the model/part name, but again I'm not sure this is what
Enrico wants here.

> 
> But as this was not yet done there might be a good reason why the model name was
> not provided to user space. Maybe Brian/Boris can comment.

No good reason AFAIK, except no-one ever needed it. Usually, userspace
libs/apps need to know about the device capabilities/specificities, and
this is already provided in a generic way.

Regarding Enrico's problem, I think the omap2-nand driver should
explicitly set the mtd->name field (see below), as is done in many other
NAND controller drivers. 

Regards,

Boris

--->8---
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 2a52101120d4..33770cb1b756 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,6 +1856,12 @@ 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_kasprinf(&pdev->dev, "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))