Messages in this thread | | | Subject | Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format | From | Apurva Nandan <> | Date | Tue, 21 Sep 2021 19:12:49 +0530 |
| |
Hi Rob,
On 21/09/21 2:46 am, Rob Herring wrote: > On Mon, Sep 20, 2021 at 06:06:51PM +0300, Serge Semin wrote: >> Hello Apurva >> >> On Mon, Sep 20, 2021 at 07:57:12PM +0530, Apurva Nandan wrote: >>> Convert spi-nand.txt binding to YAML format with an added example. >>> >>> Signed-off-by: Apurva Nandan <a-nandan@ti.com> >>> --- >>> .../devicetree/bindings/mtd/spi-nand.txt | 5 -- >>> .../devicetree/bindings/mtd/spi-nand.yaml | 62 +++++++++++++++++++ >>> 2 files changed, 62 insertions(+), 5 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt >>> create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml >> Thanks for the bindings conversion patch. There are several comments >> below. But before addressing them it would be better to also get a >> response from Rob. >> >>> diff --git a/Documentation/devicetree/bindings/mtd/spi-nand.txt b/Documentation/devicetree/bindings/mtd/spi-nand.txt >>> deleted file mode 100644 >>> index 8b51f3b6d55c..000000000000 >>> --- a/Documentation/devicetree/bindings/mtd/spi-nand.txt >>> +++ /dev/null >>> @@ -1,5 +0,0 @@ >>> -SPI NAND flash >>> - >>> -Required properties: >>> -- compatible: should be "spi-nand" >>> -- reg: should encode the chip-select line used to access the NAND chip >>> diff --git a/Documentation/devicetree/bindings/mtd/spi-nand.yaml b/Documentation/devicetree/bindings/mtd/spi-nand.yaml >>> new file mode 100644 >>> index 000000000000..601beba8d971 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mtd/spi-nand.yaml >>> @@ -0,0 +1,62 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/mtd/spi-nand.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: SPI NAND flash >>> + >>> +maintainers: >>> + - Apurva Nandan <a-nandan@ti.com> >>> + >>> +allOf: >>> + - $ref: "mtd.yaml#" >>> + >>> +properties: >>> + compatible: >>> + const: spi-nand >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + spi-max-frequency: true >>> + spi-rx-bus-width: true >>> + spi-tx-bus-width: true >>> + rx-sample-delay-ns: true >> Since it's an SPI-client device there are more than these properties >> could be set for it. See the SPI-controller bindings schema: >> Documentation/devicetree/bindings/spi/spi-controller.yaml >> So there is two possible ways to make it more generic: >> 1) Detach the spi-client part from the spi-controller.yaml bindings >> into a dedicated DT-schema file and refer to that new scheme from >> here. > Yes, as mentioned there's patches doing this. But the above is fine. > There's some value in defining here which properties are valid. Yeah right >> 2) Forget about these controller-specific properties and let the >> parental SPI-controller bindings parsing them. Of course there must be >> at least one of the next properties declared for it to work: >> {unevaluatedProperties, additionalProperties}. >> >> It's up to Rob to decided which approach is better though... >> >>> + >>> + '#address-cells': true >>> + '#size-cells': true >> Aren't they always equal to 1? > No SPI nand devices >4GB? Yeah, we have SPI NANDs >4GB, and "'#address-cells': true" allows those sizes. > >>> + >>> +additionalProperties: >>> + type: object >> I'd suggest to elaborate the way the partition sub-nodes looks >> like, for instance, the node names, supported compatible names, >> labels, etc. > That should probably all be in mtd.yaml. The question here is whether > partitions are always under a 'partitions' node. Maybe this is new > enough that only the new way has to be supported. Though if mtd.yaml > supported both forms, allowing both all the time is okay IMO. > > Rob
I had added the "partition" node properties in the v1 patch, but as per the reviews I removed it. I think we can prefer having them in mtd.yaml if needed, in a separate patch series. Do you prefer the mtd.yaml changes as a part of this series or as a separate patch?
Other than that, I don't find any need for a v3 patch re-roll, do you agree?
Thanks, Apurva Nandan
| |