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

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

[Leto, Enrico]

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

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?
Or some other suggestion or recommendation how to do that ?

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

[Roger Quadros]

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? If it is then it shouldn't be
depending on the platform device name for anything.

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.

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

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

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.

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.

cheers,
-roger

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

[Boris Brezillon]

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

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

[Boris Brezillon]

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

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

[Roger Quadros]

Hi,

On 19/12/16 14:57, Boris Brezillon wrote:
> 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").

Thanks for this info and the patch Boris. I can give it a try.

Enrico, is the patch good enough for you?

cheers,
-roger