diff mbox

[V5,1/3] dt-bindings: mtd: document linux,part-probe property

Message ID 20170417194746.10697-1-zajec5@gmail.com
State Not Applicable, archived
Headers show

Commit Message

Rafał Miłecki April 17, 2017, 7:47 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

Support for this property has been introduced in 2010 with commit
9d5da3a9b849 ("mtd: extend physmap_of to let the device tree specify the
parition probe") but it was never documented. Fix this by adding a
proper description and example.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 Documentation/devicetree/bindings/mtd/common.txt | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Brian Norris April 19, 2017, 8:37 p.m. UTC | #1
Hi,

On Mon, Apr 17, 2017 at 09:47:44PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Support for this property has been introduced in 2010 with commit
> 9d5da3a9b849 ("mtd: extend physmap_of to let the device tree specify the
> parition probe") but it was never documented. Fix this by adding a
> proper description and example.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/mtd/common.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
> index fc068b923d7a..1ada70e718b2 100644
> --- a/Documentation/devicetree/bindings/mtd/common.txt
> +++ b/Documentation/devicetree/bindings/mtd/common.txt
> @@ -6,10 +6,17 @@ Optional properties:
>    controller based name) in order to ease flash device identification
>    and/or describe what they are used for.
>  
> +- linux,part-probe: if present, this property should contain a list of strings
> +  with partition probes to be used for the flash device. A role of partition
> +  probe (parser) is to read/construct partition table and register found
> +  partitions. Getting partition table may be platform or device specific so
> +  various devices may use various Linux drivers for this purpose.

Have you reviewed the old threads about this? I thought I already
discouraged extending this binding to be generic. Particularly, we do
*not* want to codify things like 'linux,part-probe = "bcm47xxpart"',
which doesn't follow any accepted practice about naming. And you've also
failed to document any actual strings to put here, which hides the fact
that you're opening a can of worms.

Piece of a previous thread:
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062613.html
[PATCH] mtd: document linux-specific partition parser DT binding

I attempted to resolve that here, but never made time to address the few
not-so-constructive comments I received:

https://mail-archive.com/linux-kernel@vger.kernel.org/msg1035539.html
[RFC PATCH 0/7] mtd: partitions: add of_match_table support

I'd still prefer that approach to supporting this shortcut for all MTDs.

Brian

> +
>  Example:
>  
>  	flash@0 {
>  		label = "System-firmware";
> +		linux,part-probe = "cmdlinepart", "ofpart";
>  
>  		/* flash type specific properties */
>  	};
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Boris Brezillon April 19, 2017, 8:55 p.m. UTC | #2
On Wed, 19 Apr 2017 13:37:05 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> Hi,
> 
> On Mon, Apr 17, 2017 at 09:47:44PM +0200, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > Support for this property has been introduced in 2010 with commit
> > 9d5da3a9b849 ("mtd: extend physmap_of to let the device tree specify the
> > parition probe") but it was never documented. Fix this by adding a
> > proper description and example.
> > 
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > Acked-by: Rob Herring <robh@kernel.org>
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/common.txt | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
> > index fc068b923d7a..1ada70e718b2 100644
> > --- a/Documentation/devicetree/bindings/mtd/common.txt
> > +++ b/Documentation/devicetree/bindings/mtd/common.txt
> > @@ -6,10 +6,17 @@ Optional properties:
> >    controller based name) in order to ease flash device identification
> >    and/or describe what they are used for.
> >  
> > +- linux,part-probe: if present, this property should contain a list of strings
> > +  with partition probes to be used for the flash device. A role of partition
> > +  probe (parser) is to read/construct partition table and register found
> > +  partitions. Getting partition table may be platform or device specific so
> > +  various devices may use various Linux drivers for this purpose.  
> 
> Have you reviewed the old threads about this? I thought I already
> discouraged extending this binding to be generic. Particularly, we do
> *not* want to codify things like 'linux,part-probe = "bcm47xxpart"',
> which doesn't follow any accepted practice about naming. And you've also
> failed to document any actual strings to put here, which hides the fact
> that you're opening a can of worms.
> 
> Piece of a previous thread:
> http://lists.infradead.org/pipermail/linux-mtd/2015-October/062613.html
> [PATCH] mtd: document linux-specific partition parser DT binding
> 
> I attempted to resolve that here, but never made time to address the few
> not-so-constructive comments I received:
> 
> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1035539.html
> [RFC PATCH 0/7] mtd: partitions: add of_match_table support

Oops, completely forgot you had proposed an alternative solution. Sorry
about that.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
index fc068b923d7a..1ada70e718b2 100644
--- a/Documentation/devicetree/bindings/mtd/common.txt
+++ b/Documentation/devicetree/bindings/mtd/common.txt
@@ -6,10 +6,17 @@  Optional properties:
   controller based name) in order to ease flash device identification
   and/or describe what they are used for.
 
+- linux,part-probe: if present, this property should contain a list of strings
+  with partition probes to be used for the flash device. A role of partition
+  probe (parser) is to read/construct partition table and register found
+  partitions. Getting partition table may be platform or device specific so
+  various devices may use various Linux drivers for this purpose.
+
 Example:
 
 	flash@0 {
 		label = "System-firmware";
+		linux,part-probe = "cmdlinepart", "ofpart";
 
 		/* flash type specific properties */
 	};