From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752427AbbJKUEH (ORCPT ); Sun, 11 Oct 2015 16:04:07 -0400 Received: from mail-qk0-f174.google.com ([209.85.220.174]:36389 "EHLO mail-qk0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751278AbbJKUED (ORCPT ); Sun, 11 Oct 2015 16:04:03 -0400 Date: Sun, 11 Oct 2015 13:04:02 -0700 From: Brian Norris To: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala Cc: David Woodhouse , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, Boris Brezillon , Michal Suchanek Subject: Re: [PATCH v3 3/5] mtd: ofpart: update devicetree binding specification Message-ID: <20151011200402.GE3696@localhost> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi DT maintainers, It's a bit hypocritical of me, since I've been a slow reviewer as well, but... can we get some review on this one? Usually, I'm comfortable taking driver DT bindings without your review, but this one is a bit more generic and is more far-reaching than the average driver. I'm not a big fan of this change, and I don't quite understand why the bus driver (the SPI bus, which is a level up from the SPI device / MTD node) can specify its grandchildren (see spi-samsung.txt). But given the constraints, I think Michal's solution is OK. And I do agree that MTD's ofpart should be bit more specific. Anyway, a quick look and an Ack/Nak would be appreciated. Thanks, Brian On Tue, Aug 18, 2015 at 03:34:08PM -0000, Michal Suchanek wrote: > To avoid conflict with other drivers using subnodes of the mtd device > create only one ofpart-specific node rather than any number of > arbitrary partition subnodes. > > Signed-off-by: Michal Suchanek > --- > v3: > > - rename DT node ofpart -> partitions > --- > .../devicetree/bindings/mtd/partition.txt | 68 +++++++++++++--------- > 1 file changed, 40 insertions(+), 28 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt > index 8e5557d..8c2aff7 100644 > --- a/Documentation/devicetree/bindings/mtd/partition.txt > +++ b/Documentation/devicetree/bindings/mtd/partition.txt > @@ -4,10 +4,16 @@ Partitions can be represented by sub-nodes of an mtd device. This can be used > on platforms which have strong conventions about which portions of a flash are > used for what purposes, but which don't use an on-flash partition table such > as RedBoot. > + > +The partition table should be partitions subnode of the mtd node. Partitions are > +defined in subnodes of the partitions node. > + > +For backwards compatibility partitions as direct subnodes of the mtd device are > +supported. This use is discouraged. > NOTE: if the sub-node has a compatible string, then it is not a partition. > > -#address-cells & #size-cells must both be present in the mtd device. There are > -two valid values for both: > +#address-cells & #size-cells must both be present in the partitions subnode of the > +mtd device. There are two valid values for both: > <1>: for partitions that require a single 32-bit cell to represent their > size/address (aka the value is below 4 GiB) > <2>: for partitions that require two 32-bit cells to represent their > @@ -28,44 +34,50 @@ Examples: > > > flash@0 { > - #address-cells = <1>; > - #size-cells = <1>; > + partitions { > + #address-cells = <1>; > + #size-cells = <1>; > > - partition@0 { > - label = "u-boot"; > - reg = <0x0000000 0x100000>; > - read-only; > - }; > + partition@0 { > + label = "u-boot"; > + reg = <0x0000000 0x100000>; > + read-only; > + }; > > - uimage@100000 { > - reg = <0x0100000 0x200000>; > + uimage@100000 { > + reg = <0x0100000 0x200000>; > + }; > }; > }; > > flash@1 { > - #address-cells = <1>; > - #size-cells = <2>; > + partitions { > + #address-cells = <1>; > + #size-cells = <2>; > > - /* a 4 GiB partition */ > - partition@0 { > - label = "filesystem"; > - reg = <0x00000000 0x1 0x00000000>; > + /* a 4 GiB partition */ > + partition@0 { > + label = "filesystem"; > + reg = <0x00000000 0x1 0x00000000>; > + }; > }; > }; > > flash@2 { > - #address-cells = <2>; > - #size-cells = <2>; > + partitions { > + #address-cells = <2>; > + #size-cells = <2>; > > - /* an 8 GiB partition */ > - partition@0 { > - label = "filesystem #1"; > - reg = <0x0 0x00000000 0x2 0x00000000>; > - }; > + /* an 8 GiB partition */ > + partition@0 { > + label = "filesystem #1"; > + reg = <0x0 0x00000000 0x2 0x00000000>; > + }; > > - /* a 4 GiB partition */ > - partition@200000000 { > - label = "filesystem #2"; > - reg = <0x2 0x00000000 0x1 0x00000000>; > + /* a 4 GiB partition */ > + partition@200000000 { > + label = "filesystem #2"; > + reg = <0x2 0x00000000 0x1 0x00000000>; > + }; > }; > }; > -- > 2.1.4 >