[PATCH v2 0/2] Add a "label" property to the mtd device

[PATCH v2 0/2] Add a "label" property to the mtd device

[Cédric Le Goater]

Hello,

Herer are a couple of patches adding a "label" property to the mtd
device, which can be used to identify a specific chip on a system with
multiple chips. This is common on the OpenPower boards, there can be
four chips holding the firmware for the BMC and the firmware for the
host, plus chips with golden images for recovery.

Thanks,

C.

Cédric Le Goater (2):
  mtd: name the mtd device with an optional label property
  dt-bindings: mtd: add a common label property to all mtd devices

 Documentation/devicetree/bindings/mtd/common.txt | 15 +++++++++++++++
 include/linux/mtd/mtd.h                          |  2 ++
 2 files changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/common.txt

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] mtd: name the mtd device with an optional label property

[Cédric Le Goater]

This can be used to easily identify a specific chip on a system with
multiple chips.

Drivers wanting to support this new label property will benefit from
it without a change. They might want to check in the future that
mtd->name is NULL before assigning a default name to the mtd device.
Other drivers will keep the current behavior, which is to override
mtd->name with their own value.

Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:

 - moved the use of the "label" property from mtd_set_dev_defaults()
   to mtd_set_of_node() to let drivers keep control on how mtd->name
   is set and allocated.

 include/linux/mtd/mtd.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 13f8052b9ff9..f4fe15517295 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -24,6 +24,7 @@
 #include <linux/uio.h>
 #include <linux/notifier.h>
 #include <linux/device.h>
+#include <linux/of.h>
 
 #include <mtd/mtd-abi.h>
 
@@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
 				   struct device_node *np)
 {
 	mtd->dev.of_node = np;
+	of_property_read_string(np, "label", &mtd->name);
 }
 
 static inline struct device_node *mtd_get_of_node(struct mtd_info *mtd)
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 2/2] dt-bindings: mtd: add a common label property to all mtd devices

[Cédric Le Goater]

This can be used to easily identify a specific chip on a system with
multiple chips.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/common.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/common.txt

diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
new file mode 100644
index 000000000000..fc068b923d7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/common.txt
@@ -0,0 +1,15 @@
+* Common properties of all MTD devices
+
+Optional properties:
+- label: user-defined MTD device name. Can be used to assign user
+  friendly names to MTD devices (instead of the flash model or flash
+  controller based name) in order to ease flash device identification
+  and/or describe what they are used for.
+
+Example:
+
+	flash@0 {
+		label = "System-firmware";
+
+		/* flash type specific properties */
+	};
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property

[Boris Brezillon]

On Wed, 25 Jan 2017 19:17:34 +0100
Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:

> This can be used to easily identify a specific chip on a system with
> multiple chips.
> 
> Drivers wanting to support this new label property will benefit from
> it without a change. They might want to check in the future that
> mtd->name is NULL before assigning a default name to the mtd device.
> Other drivers will keep the current behavior, which is to override
> mtd->name with their own value.
> 
> Suggested-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>

Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Brian, can you have a look at this patch, and if you're okay with it,
queue it for 4.11. I'll soon post the atmel NAND controller driver
rework which will depend on this patch, and that'd be great to have
this dependency already applied.

Thanks,

Boris

> ---
> 
>  Changes since v1:
> 
>  - moved the use of the "label" property from mtd_set_dev_defaults()
>    to mtd_set_of_node() to let drivers keep control on how mtd->name
>    is set and allocated.
> 
>  include/linux/mtd/mtd.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 13f8052b9ff9..f4fe15517295 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -24,6 +24,7 @@
>  #include <linux/uio.h>
>  #include <linux/notifier.h>
>  #include <linux/device.h>
> +#include <linux/of.h>
>  
>  #include <mtd/mtd-abi.h>
>  
> @@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
>  				   struct device_node *np)
>  {
>  	mtd->dev.of_node = np;
> +	of_property_read_string(np, "label", &mtd->name);
>  }
>  
>  static inline struct device_node *mtd_get_of_node(struct mtd_info *mtd)

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property

[Brian Norris]

On Wed, Jan 25, 2017 at 07:17:34PM +0100, Cédric Le Goater wrote:
> This can be used to easily identify a specific chip on a system with
> multiple chips.
> 
> Drivers wanting to support this new label property will benefit from
> it without a change. They might want to check in the future that
> mtd->name is NULL before assigning a default name to the mtd device.
> Other drivers will keep the current behavior, which is to override
> mtd->name with their own value.
> 
> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v1:
> 
>  - moved the use of the "label" property from mtd_set_dev_defaults()
>    to mtd_set_of_node() to let drivers keep control on how mtd->name
>    is set and allocated.
> 
>  include/linux/mtd/mtd.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 13f8052b9ff9..f4fe15517295 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -24,6 +24,7 @@
>  #include <linux/uio.h>
>  #include <linux/notifier.h>
>  #include <linux/device.h>
> +#include <linux/of.h>
>  
>  #include <mtd/mtd-abi.h>
>  
> @@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
>  				   struct device_node *np)
>  {
>  	mtd->dev.of_node = np;
> +	of_property_read_string(np, "label", &mtd->name);

Seems like this could be done only if '!mtd->name'?

As it stands, you're still clobbering some names, e.g. ones from
physmap_of.c. Notably, this driver already supports a "linux,mtd-name"
(which your new property should probably supersede), but it seems like
you're breaking compatibility.

Brian

>  }
>  
>  static inline struct device_node *mtd_get_of_node(struct mtd_info *mtd)
> -- 
> 2.7.4
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property

[Boris Brezillon]

On Wed, 8 Feb 2017 14:38:59 -0800
Brian Norris <computersforpeace@gmail.com> wrote:

> On Wed, Jan 25, 2017 at 07:17:34PM +0100, Cédric Le Goater wrote:
> > This can be used to easily identify a specific chip on a system with
> > multiple chips.
> > 
> > Drivers wanting to support this new label property will benefit from
> > it without a change. They might want to check in the future that
> > mtd->name is NULL before assigning a default name to the mtd device.
> > Other drivers will keep the current behavior, which is to override
> > mtd->name with their own value.
> > 
> > Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> > 
> >  Changes since v1:
> > 
> >  - moved the use of the "label" property from mtd_set_dev_defaults()
> >    to mtd_set_of_node() to let drivers keep control on how mtd->name
> >    is set and allocated.
> > 
> >  include/linux/mtd/mtd.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> > index 13f8052b9ff9..f4fe15517295 100644
> > --- a/include/linux/mtd/mtd.h
> > +++ b/include/linux/mtd/mtd.h
> > @@ -24,6 +24,7 @@
> >  #include <linux/uio.h>
> >  #include <linux/notifier.h>
> >  #include <linux/device.h>
> > +#include <linux/of.h>
> >  
> >  #include <mtd/mtd-abi.h>
> >  
> > @@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
> >  				   struct device_node *np)
> >  {
> >  	mtd->dev.of_node = np;
> > +	of_property_read_string(np, "label", &mtd->name);  
> 
> Seems like this could be done only if '!mtd->name'?
> 
> As it stands, you're still clobbering some names, e.g. ones from
> physmap_of.c.

Indeed. I thought all users of
mtd_set_of_node()/nand_set_flash_node()/spi_nor_set_flash_node() were
calling this function before initializing mtd->name, which is
apparently not true.

> Notably, this driver already supports a "linux,mtd-name"
> (which your new property should probably supersede), but it seems like
> you're breaking compatibility.

Well, we're not really breaking things until someone starts defining a
'label' property under the flash node ;-). Moreover, if we want to
deprecate 'linux,mtd-name' in favor of 'label', we should definitely
override the 'linux,mtd-name' value with the 'label' one if someone
defined both (which is bad ;-)).

Anyway, I agree, we'd better check if !mtd->name before assigning the
label name.
Cedric, can you send a v3?



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property

[Cédric Le Goater]

On 02/09/2017 08:11 AM, Boris Brezillon wrote:
> On Wed, 8 Feb 2017 14:38:59 -0800
> Brian Norris <computersforpeace@gmail.com> wrote:
> 
>> On Wed, Jan 25, 2017 at 07:17:34PM +0100, Cédric Le Goater wrote:
>>> This can be used to easily identify a specific chip on a system with
>>> multiple chips.
>>>
>>> Drivers wanting to support this new label property will benefit from
>>> it without a change. They might want to check in the future that
>>> mtd->name is NULL before assigning a default name to the mtd device.
>>> Other drivers will keep the current behavior, which is to override
>>> mtd->name with their own value.
>>>
>>> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>
>>>  Changes since v1:
>>>
>>>  - moved the use of the "label" property from mtd_set_dev_defaults()
>>>    to mtd_set_of_node() to let drivers keep control on how mtd->name
>>>    is set and allocated.
>>>
>>>  include/linux/mtd/mtd.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>>> index 13f8052b9ff9..f4fe15517295 100644
>>> --- a/include/linux/mtd/mtd.h
>>> +++ b/include/linux/mtd/mtd.h
>>> @@ -24,6 +24,7 @@
>>>  #include <linux/uio.h>
>>>  #include <linux/notifier.h>
>>>  #include <linux/device.h>
>>> +#include <linux/of.h>
>>>  
>>>  #include <mtd/mtd-abi.h>
>>>  
>>> @@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
>>>  				   struct device_node *np)
>>>  {
>>>  	mtd->dev.of_node = np;
>>> +	of_property_read_string(np, "label", &mtd->name);  
>>
>> Seems like this could be done only if '!mtd->name'?
>>
>> As it stands, you're still clobbering some names, e.g. ones from
>> physmap_of.c.
> 
> Indeed. I thought all users of
> mtd_set_of_node()/nand_set_flash_node()/spi_nor_set_flash_node() were
> calling this function before initializing mtd->name, which is
> apparently not true.
> 
>> Notably, this driver already supports a "linux,mtd-name"
>> (which your new property should probably supersede), but it seems like
>> you're breaking compatibility.
> 
> Well, we're not really breaking things until someone starts defining a
> 'label' property under the flash node ;-). Moreover, if we want to
> deprecate 'linux,mtd-name' in favor of 'label', we should definitely
> override the 'linux,mtd-name' value with the 'label' one if someone
> defined both (which is bad ;-)).
> 
> Anyway, I agree, we'd better check if !mtd->name before assigning the
> label name.
> Cedric, can you send a v3?


yes. will do in the morning.

Thanks,

C. 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/