[PATCH v2 0/2] dt-bindings: mtd: spi-nand: Convert to DT schema

[PATCH v2 0/2] dt-bindings: mtd: spi-nand: Convert to DT schema

[Apurva Nandan]

Series to convert spi-nand.txt binding to YAML format and also fix up
snps-dw-apb-ssi for related dt_schema errors.

Changes in v2:
- Fixed dtschema erros in snps-dw-apb-ssi.
- Improved additionalProperties, reg, and compatible fields in
  spi-nand.yaml.

Apurva Nandan (2):
  dt-bindings: mtd: spi-nand: Convert to DT schema format
  dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of
    'spi-flash' in example

 .../devicetree/bindings/mtd/spi-nand.txt      |  5 --
 .../devicetree/bindings/mtd/spi-nand.yaml     | 62 +++++++++++++++++++
 .../bindings/spi/snps,dw-apb-ssi.yaml         |  2 +-
 3 files changed, 63 insertions(+), 6 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml

-- 
2.25.1

[PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Apurva Nandan]

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

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
+
+  '#address-cells': true
+  '#size-cells': true
+
+additionalProperties:
+  type: object
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        flash@6 {
+            #address-cells = <1>;
+            #size-cells = <1>;
+            compatible = "spi-nand";
+            reg = <0x6>;
+            spi-max-frequency = <42000000>;
+
+            partitions {
+                compatible = "fixed-partitions";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                partition@0 {
+                    label = "boot";
+                    reg = <0 0x200000>;
+                };
+
+                partition@200000 {
+                    label = "rootfs";
+                    reg = <0x200000 0xce0000>;
+                };
+            };
+        };
+    };
-- 
2.25.1

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Serge Semin]

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.
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?

> +
> +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.

-Sergey

> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        flash@6 {
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            compatible = "spi-nand";
> +            reg = <0x6>;
> +            spi-max-frequency = <42000000>;
> +
> +            partitions {
> +                compatible = "fixed-partitions";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                partition@0 {
> +                    label = "boot";
> +                    reg = <0 0x200000>;
> +                };
> +
> +                partition@200000 {
> +                    label = "rootfs";
> +                    reg = <0x200000 0xce0000>;
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.25.1
> 

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Pratyush Yadav]

On 20/09/21 06:06PM, 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
[...]
> 
> > +  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.
> 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}.

I proposed a patch series [0] to fix this for SPI NOR, which can be used 
here as well. A re-roll is long overdue though. Let me see when I can 
find time to get to it. In the meantime, this should be fine IMO.

[0] https://patchwork.kernel.org/project/spi-devel-general/list/?series=497129&state=%2A&archive=both


-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Rob Herring]

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.

> 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?

> 
> > +
> > +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

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Apurva Nandan]

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

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Rob Herring]

On Tue, Sep 21, 2021 at 8:43 AM Apurva Nandan <a-nandan@ti.com> wrote:
>
> 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?

Yeah, I suppose not.

Reviewed-by: Rob Herring <robh@kernel.org>

And actually, it's Miquel that should apply these 2 rather than Mark.

Rob

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Miquel Raynal]

Hello Rob, Mark,

robh@kernel.org wrote on Tue, 21 Sep 2021 14:16:02 -0500:

> On Tue, Sep 21, 2021 at 8:43 AM Apurva Nandan <a-nandan@ti.com> wrote:
> >
> > 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?  
> 
> Yeah, I suppose not.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> And actually, it's Miquel that should apply these 2 rather than Mark.

Yeah, but that's fine for this one, I don't expect conflicts anyway.

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v2 1/2] dt-bindings: mtd: spi-nand: Convert to DT schema format

[Rob Herring]

On Mon, 20 Sep 2021 19:57:12 +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
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml: spi-flash@1: $nodename:0: 'spi-flash@1' does not match '^flash(@.*)?$'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/spi-nand.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1530166

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

[PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Apurva Nandan]

Change the nodename in the example with spi-nand from 'spi-flash@1'
to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
so make the spi-nand examples in dt-bindings use it too for uniformity.

Signed-off-by: Apurva Nandan <a-nandan@ti.com>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index ca91201a9926..d7e08b03e204 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -171,7 +171,7 @@ examples:
       cs-gpios = <&gpio0 13 0>,
                  <&gpio0 14 0>;
       rx-sample-delay-ns = <3>;
-      spi-flash@1 {
+      flash@1 {
         compatible = "spi-nand";
         reg = <1>;
         rx-sample-delay-ns = <7>;
-- 
2.25.1

Re: [PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Serge Semin]

On Mon, Sep 20, 2021 at 07:57:13PM +0530, Apurva Nandan wrote:
> Change the nodename in the example with spi-nand from 'spi-flash@1'
> to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
> flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
> so make the spi-nand examples in dt-bindings use it too for uniformity.
> 
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Seems reasonable.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

-Sergey

> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index ca91201a9926..d7e08b03e204 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -171,7 +171,7 @@ examples:
>        cs-gpios = <&gpio0 13 0>,
>                   <&gpio0 14 0>;
>        rx-sample-delay-ns = <3>;
> -      spi-flash@1 {
> +      flash@1 {
>          compatible = "spi-nand";
>          reg = <1>;
>          rx-sample-delay-ns = <7>;
> -- 
> 2.25.1
> 

Re: [PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On Mon, Sep 20, 2021 at 07:57:13PM +0530, Apurva Nandan wrote:
> Change the nodename in the example with spi-nand from 'spi-flash@1'
> to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
> flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
> so make the spi-nand examples in dt-bindings use it too for uniformity.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Rob Herring]

On Mon, 20 Sep 2021 19:57:13 +0530, Apurva Nandan wrote:
> Change the nodename in the example with spi-nand from 'spi-flash@1'
> to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
> flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
> so make the spi-nand examples in dt-bindings use it too for uniformity.
> 
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0: /example-0/spi@fff00000/flash@1: failed to match any schema with compatible: ['spi-nand']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1530168

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Rob Herring]

On Mon, Sep 20, 2021 at 07:57:13PM +0530, Apurva Nandan wrote:
> Change the nodename in the example with spi-nand from 'spi-flash@1'
> to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
> flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
> so make the spi-nand examples in dt-bindings use it too for uniformity.
> 
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Fixes should come first. Maybe Mark can apply this before you respin.

Acked-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index ca91201a9926..d7e08b03e204 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -171,7 +171,7 @@ examples:
>        cs-gpios = <&gpio0 13 0>,
>                   <&gpio0 14 0>;
>        rx-sample-delay-ns = <3>;
> -      spi-flash@1 {
> +      flash@1 {
>          compatible = "spi-nand";
>          reg = <1>;
>          rx-sample-delay-ns = <7>;
> -- 
> 2.25.1
> 
> 

Re: [PATCH v2 2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example

[Apurva Nandan]

On 21/09/21 2:49 am, Rob Herring wrote:
> On Mon, Sep 20, 2021 at 07:57:13PM +0530, Apurva Nandan wrote:
>> Change the nodename in the example with spi-nand from 'spi-flash@1'
>> to 'flash@1' to make the schema uniform with both spi-nand and spi-nor
>> flashes. jedec,spi-nor.yaml uses 'flash@' nodename for spi-nor flashes,
>> so make the spi-nand examples in dt-bindings use it too for uniformity.
>>
>> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
>> ---
>>   Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> Fixes should come first. Maybe Mark can apply this before you respin.
>
> Acked-by: Rob Herring <robh@kernel.org>
Please tell if I need to re-roll this series for changing the order of 
the two patches.
>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> index ca91201a9926..d7e08b03e204 100644
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> @@ -171,7 +171,7 @@ examples:
>>         cs-gpios = <&gpio0 13 0>,
>>                    <&gpio0 14 0>;
>>         rx-sample-delay-ns = <3>;
>> -      spi-flash@1 {
>> +      flash@1 {
>>           compatible = "spi-nand";
>>           reg = <1>;
>>           rx-sample-delay-ns = <7>;
>> -- 
>> 2.25.1
>>
>>
Thanks,
Apurva Nandan

Re: (subset) [PATCH v2 0/2] dt-bindings: mtd: spi-nand: Convert to DT schema

[Mark Brown]

On Mon, 20 Sep 2021 19:57:11 +0530, Apurva Nandan wrote:
> Series to convert spi-nand.txt binding to YAML format and also fix up
> snps-dw-apb-ssi for related dt_schema errors.
> 
> Changes in v2:
> - Fixed dtschema erros in snps-dw-apb-ssi.
> - Improved additionalProperties, reg, and compatible fields in
>   spi-nand.yaml.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[2/2] dt-bindings: snps,dw-apb-ssi: Use 'flash' node name instead of 'spi-flash' in example
      commit: d7a48e27b38a94bf73c973c236461234610256d5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark