[PATCH v2] mtd: nand: pxa3xx: Fix registered MTD name

[Brian Norris]

On Fri, Oct 18, 2013 at 12:32 PM, Ezequiel Garcia
<ezequiel.garcia at free-electrons.com> wrote:
> On Fri, Oct 18, 2013 at 12:15:39PM -0700, Brian Norris wrote:
>> On Fri, Oct 18, 2013 at 11:14 AM, Ezequiel Garcia
>> <ezequiel.garcia at free-electrons.com> wrote:
[...]
>> >  drivers/mtd/nand/pxa3xx_nand.c | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
>> > index aa75adf..4d53e8e 100644
>> > --- a/drivers/mtd/nand/pxa3xx_nand.c
>> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> > @@ -46,6 +46,8 @@
>> >   */
>> >  #define INIT_BUFFER_SIZE       256
>> >
>> > +#define DRIVER_NAME            "pxa3xx_nand-0"
>> > +
>> >  /* registers and bit definitions */
>> >  #define NDCR           (0x00) /* Control register */
>> >  #define NDTR0CS0       (0x04) /* Timing Parameter 0 for CS0 */
>> > @@ -1339,7 +1341,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
>> >         for (cs = 0; cs < pdata->num_cs; cs++) {
>> >                 struct mtd_info *mtd = info->host[cs]->mtd;
>> >
>> > -               mtd->name = pdev->name;
>> > +               mtd->name = DRIVER_NAME;
>> >                 info->cs = cs;
>> >                 ret = pxa3xx_nand_scan(mtd);
>> >                 if (ret) {
>> > @@ -1425,7 +1427,7 @@ static int pxa3xx_nand_resume(struct platform_device *pdev)
>> >
>> >  static struct platform_driver pxa3xx_nand_driver = {
>> >         .driver = {
>> > -               .name   = "pxa3xx-nand",
>> > +               .name   = DRIVER_NAME,
>> >                 .of_match_table = pxa3xx_nand_dt_ids,
>> >         },
>> >         .probe          = pxa3xx_nand_probe,
>>
>> For mtdparts' sake, do we need both the driver name and the MTD name
>> to be the same? Or could we just stick the  "pxa3xx_nand-0" directly
>> in mtd->name only, where it's needed for compatibility, and allow the
>> driver name to be cleaner? Or is that a silly idea?
>>
>
> Hm... What's the impact of the driver's name?

I meant to ask you the exact same question, because I'm lazy :)

I'm not sure exactly. I thought it affected the device name (which is
important to users; it's exposed in sysfs in potentially important
ways), but that actually comes from the platform-device allocation or
from device tree. The most visible way I know of is that it prefixes
the messages seen via the dev_{printk,err,info,warn,..}() print
functions. This could be annoying, but not horrible.

The driver name does show up in /sys/bus/platform/drivers, but I'm not
sure if that's ever expected to be stable.

> I know it's just a small bloat, but still I don't like to see two stupid
> strings "pxa3xx-nand" and "pxa3xx_nand-0" bloating our kernels, so
> that's why I chose a common macro :-)

That's fine.

> What do you think?

I'm fine with just making both use DRIVER_NAME. So I'd take this patch as-is.

One possible consequence: it makes things a little more fragile if
people want to change one or both in the future, not noticing that
while the driver name may (?) be changed harmlessly, it can cause
mtdparts=... breakage.

Brian