All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: linux-mtd@lists.infradead.org,
	David Woodhouse <dwmw2@infradead.org>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property
Date: Wed, 8 Feb 2017 14:38:59 -0800	[thread overview]
Message-ID: <20170208223859.GK94627@google.com> (raw)
In-Reply-To: <1485368255-12038-2-git-send-email-clg@kaod.org>

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
> 

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	devicetree@vger.kernel.org, Richard Weinberger <richard@nod.at>,
	Marek Vasut <marek.vasut@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	linux-mtd@lists.infradead.org,
	Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property
Date: Wed, 8 Feb 2017 14:38:59 -0800	[thread overview]
Message-ID: <20170208223859.GK94627@google.com> (raw)
In-Reply-To: <1485368255-12038-2-git-send-email-clg@kaod.org>

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/

  parent reply	other threads:[~2017-02-08 22:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-25 18:17 [PATCH v2 0/2] Add a "label" property to the mtd device Cédric Le Goater
2017-01-25 18:17 ` Cédric Le Goater
2017-01-25 18:17 ` [PATCH v2 1/2] mtd: name the mtd device with an optional label property Cédric Le Goater
2017-01-25 18:17   ` Cédric Le Goater
2017-01-25 20:31   ` Boris Brezillon
2017-01-25 20:31     ` Boris Brezillon
2017-02-08 22:38   ` Brian Norris [this message]
2017-02-08 22:38     ` Brian Norris
2017-02-09  7:11     ` Boris Brezillon
2017-02-09  7:11       ` Boris Brezillon
2017-02-09  9:20       ` Cédric Le Goater
2017-02-09  9:20         ` Cédric Le Goater
2017-01-25 18:17 ` [PATCH v2 2/2] dt-bindings: mtd: add a common label property to all mtd devices Cédric Le Goater
2017-01-25 18:17   ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170208223859.GK94627@google.com \
    --to=computersforpeace@gmail.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=clg@kaod.org \
    --cc=cyrille.pitchen@atmel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.