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

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

On Mon, 19 Dec 2016 13:46:20 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> 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. 
> 

Just for the record, the commit introducing the regression is
853f1c58c4b2 ("mtd: nand: omap2: show parent device structure in
sysfs").

> 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))