Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <ef34a0b2-3b13-863b-86b9-71832eace360@gmail.com>
Date:   Tue, 25 Jan 2022 21:21:04 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101
 Thunderbird/96.0
Subject: Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new
 dynamic-partitions node
To:     Ansuel Smith <ansuelsmth@gmail.com>
Cc:     Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Rob Herring <robh+dt@kernel.org>,
        linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220120202615.28076-1-ansuelsmth@gmail.com>
 <20220120202615.28076-2-ansuelsmth@gmail.com>
 <a823e730-853d-901b-1b9f-937e1ec76444@gmail.com>
 <61ef243a.1c69fb81.26cae.716b@mx.google.com>
From:   =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
In-Reply-To: <61ef243a.1c69fb81.26cae.716b@mx.google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 24.01.2022 23:12, Ansuel Smith wrote:
> On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
>> On 20.01.2022 21:26, Ansuel Smith wrote:
>>> Document new dynamic-partitions node used to provide an of node for
>>> partition registred at runtime by parsers. This is required for nvmem
>>> system to declare and detect nvmem-cells.
>>>
>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>>> ---
>>>    .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>>>    1 file changed, 59 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> new file mode 100644
>>> index 000000000000..7528e49f2d7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> @@ -0,0 +1,59 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Dynamic partitions
>>> +
>>> +description: |
>>> +  This binding can be used on platforms which have partitions registered at
>>> +  runtime by parsers or partition table present on the flash. Example are
>>> +  partitions declared from smem parser or cmdlinepart. This will create an
>>> +  of node for these dynamic partition where systems like Nvmem can get a
>>> +  reference to register nvmem-cells.
>>> +
>>> +  The partition table should be a node named "dynamic-partitions".
>>> +  Partitions are then defined as subnodes. Only the label is required
>>> +  as any other data will be taken from the parser.
>>> +
>>> +maintainers:
>>> +  - Ansuel Smith <ansuelsmth@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: dynamic-partitions
>>> +
>>> +patternProperties:
>>> +  "@[0-9a-f]+$":
>>> +    $ref: "partition.yaml#"
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +    partitions {
>>> +        compatible = "qcom,smem";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +    };
>>> +
>>> +    dynamic-partitions {
>>> +      compatible = "dynamic-partitions";
>>> +
>>> +      art: art {
>>> +        label = "0:art";
>>> +        read-only;
>>> +        compatible = "nvmem-cells";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +
>>> +        macaddr_art_0: macaddr@0 {
>>> +          reg = <0x0 0x6>;
>>> +        };
>>> +
>>> +        macaddr_art_6: macaddr@6 {
>>> +          reg = <0x6 0x6>;
>>> +        };
>>> +      };
>>> +    };
>>
>> First of all: I fully support such a feature. I need it for Broadom
>> platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
>> In my case bootloader partition is created dynamically (it doesn't have
>> const offset and size). It contains NVMEM data however that needs to be
>> described in DT.
>>
>> This binding however looks loose and confusing to me.
>>
> 
> I agree.
> 
>> First of all did you really mean to use "qcom,smem"? My first guess is
>> you meant "qcom,smem-part".
>>
> 
> Yes sorry, I was referring to the smem parser qcom,smem-part
> 
>> Secondly can't we have partitions defined just as subnodes of the
>> partitions { ... }; node?
>>
> 
> I would love to use it. My only concern is that due to the fact
> that we have to support legacy partition declaring, wonder if this could
> create some problem. I'm referring to declaring fixed partition without
> using any compatible/standard binding name.

Legacy partitioning won't kick in if you have "partitions" with
"compatible" string. We're safe here. Just checked to be sure.


> I remember we improved that with the introduction of the nvmem binding
> by making the fixed-partition compatible mandatory. But I would like to
> have extra check. Wonder if to be on the safe part we can consider
> appending to the "dynamic parser" a compatible like "dynamic-partitions"
> and use your way to declare them (aka keeping the dynamic-partition and
> removing the extra parallel partitions list)
> 
> Feel free to tell me it's just a stupid and unnecessary idea. I just
> have fear to introduce regression in the partition parsing logic.

I'm confused. I think all dynamic partitioners already have a
"compatible" set.

Can you post an example of DT binging you described above, please?