[PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus

Sean Anderson posted 9 patches 3 months, 3 weeks ago
[PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus
Posted by Sean Anderson 3 months, 3 weeks ago
Add an example of the spi-buses property showcasing how to have devices
on both the upper and lower buses.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v2:
- New

 .../bindings/spi/spi-zynqmp-qspi.yaml         | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
index 02cf1314367b..c6a57fbb9dcf 100644
--- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
@@ -69,7 +69,7 @@ examples:
       #address-cells = <2>;
       #size-cells = <2>;
 
-      qspi: spi@ff0f0000 {
+      qspi: spi-controller@ff0f0000 {
         compatible = "xlnx,zynqmp-qspi-1.0";
         clocks = <&zynqmp_clk 53>, <&zynqmp_clk 82>;
         clock-names = "ref_clk", "pclk";
@@ -77,5 +77,25 @@ examples:
         interrupt-parent = <&gic>;
         reg = <0x0 0xff0f0000 0x0 0x1000>,
               <0x0 0xc0000000 0x0 0x8000000>;
+        num-cs = <3>;
+        cs-gpios = <0>, <0>, <&gpio 5>;
+
+        flash@0 {
+          reg = <0>;
+          spi-buses = <0>;
+          compatible = "jedec,spi-nor";
+        };
+
+        flash@1 {
+          reg = <1>;
+          spi-buses = <1>;
+          compatible = "jedec,spi-nor";
+        };
+
+        flash@2 {
+          reg = <2>;
+          spi-buses = <0>;
+          compatible = "jedec,spi-nor";
+        };
       };
     };
-- 
2.35.1.1320.gc452695387.dirty
Re: [PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus
Posted by David Lechner 3 months, 3 weeks ago
On 6/16/25 5:00 PM, Sean Anderson wrote:
> Add an example of the spi-buses property showcasing how to have devices
> on both the upper and lower buses.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - New
> 
>  .../bindings/spi/spi-zynqmp-qspi.yaml         | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> index 02cf1314367b..c6a57fbb9dcf 100644
> --- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml


In addition to changing the example, we could also extend the
spi-buses property for this controller since we know this controller
has 2 buses.

  properties:
    ...

    spi-buses:
      description: 0 is the "lower" bus, 1 is the "upper" bus
      maxItems: 2
      items:
        enum: [0, 1]

Not sure what to do about the default though since as discussed elsewhere,
this controller needs the default bus number to be the CS number for
backwards compatibility rather than `default: [0]` as is specified in the
previous patch.

I suppose we could leave default out of the generic binding and leave it
up to each individual controller to decide how to handle that.

> @@ -69,7 +69,7 @@ examples:
>        #address-cells = <2>;
>        #size-cells = <2>;
>  
> -      qspi: spi@ff0f0000 {
> +      qspi: spi-controller@ff0f0000 {

It seems more common to have spi@ rather than spi-controller@.
Is there a push to change this in general?

>          compatible = "xlnx,zynqmp-qspi-1.0";
>          clocks = <&zynqmp_clk 53>, <&zynqmp_clk 82>;
>          clock-names = "ref_clk", "pclk";
Re: [PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus
Posted by Sean Anderson 3 months, 3 weeks ago
On 6/18/25 14:27, David Lechner wrote:
> On 6/16/25 5:00 PM, Sean Anderson wrote:
>> Add an example of the spi-buses property showcasing how to have devices
>> on both the upper and lower buses.
>> 
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> 
>> Changes in v2:
>> - New
>> 
>>  .../bindings/spi/spi-zynqmp-qspi.yaml         | 22 ++++++++++++++++++-
>>  1 file changed, 21 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
>> index 02cf1314367b..c6a57fbb9dcf 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> 
> 
> In addition to changing the example, we could also extend the
> spi-buses property for this controller since we know this controller
> has 2 buses.
> 
>   properties:
>     ...
> 

OK, but this property is for the slaves not the master. I'm not sure what the right incantation is.

>     spi-buses:
>       description: 0 is the "lower" bus, 1 is the "upper" bus
>       maxItems: 2
>       items:
>         enum: [0, 1]
> 
> Not sure what to do about the default though since as discussed elsewhere,
> this controller needs the default bus number to be the CS number for
> backwards compatibility rather than `default: [0]` as is specified in the
> previous patch.
> 
> I suppose we could leave default out of the generic binding and leave it
> up to each individual controller to decide how to handle that.
> 
>> @@ -69,7 +69,7 @@ examples:
>>        #address-cells = <2>;
>>        #size-cells = <2>;
>>  
>> -      qspi: spi@ff0f0000 {
>> +      qspi: spi-controller@ff0f0000 {
> 
> It seems more common to have spi@ rather than spi-controller@.
> Is there a push to change this in general?

iirc I got a warning when running dt_binding_check. I can re-test this...

--Sean
Re: [PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus
Posted by David Lechner 3 months, 3 weeks ago
On 6/19/25 11:20 AM, Sean Anderson wrote:
> On 6/18/25 14:27, David Lechner wrote:
>> On 6/16/25 5:00 PM, Sean Anderson wrote:
>>> Add an example of the spi-buses property showcasing how to have devices
>>> on both the upper and lower buses.
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>> ---
>>>
>>> Changes in v2:
>>> - New
>>>
>>>  .../bindings/spi/spi-zynqmp-qspi.yaml         | 22 ++++++++++++++++++-
>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
>>> index 02cf1314367b..c6a57fbb9dcf 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
>>
>>
>> In addition to changing the example, we could also extend the
>> spi-buses property for this controller since we know this controller
>> has 2 buses.
>>
>>   properties:
>>     ...
>>
> 
> OK, but this property is for the slaves not the master. I'm not sure what the right incantation is.


I think using patternProperties, like in Documentation/devicetree/
bindings/spi/spi-controller.yaml

patternProperties:
  "^.*@[01]$":
    spi-buses:
      ...

> 
>>     spi-buses:
>>       description: 0 is the "lower" bus, 1 is the "upper" bus
>>       maxItems: 2
>>       items:
>>         enum: [0, 1]
>>
Re: [PATCH v2 2/9] dt-bindings: spi: zynqmp-qspi: Add example dual upper/lower bus
Posted by Rob Herring (Arm) 3 months, 3 weeks ago
On Mon, 16 Jun 2025 18:00:47 -0400, Sean Anderson wrote:
> Add an example of the spi-buses property showcasing how to have devices
> on both the upper and lower buses.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - New
> 
>  .../bindings/spi/spi-zynqmp-qspi.yaml         | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:40.15-25: Warning (reg_format): /example-0/soc/spi-controller@ff0f0000/flash@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:46.15-25: Warning (reg_format): /example-0/soc/spi-controller@ff0f0000/flash@1:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:52.15-25: Warning (reg_format): /example-0/soc/spi-controller@ff0f0000/flash@2:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:39.21-43.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@0: Relying on default #address-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:39.21-43.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@0: Relying on default #size-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:45.21-49.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@1: Relying on default #address-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:45.21-49.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@1: Relying on default #size-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:51.21-55.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@2: Relying on default #address-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dts:51.21-55.15: Warning (avoid_default_addr_size): /example-0/soc/spi-controller@ff0f0000/flash@2: Relying on default #size-cells value
Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: spi-controller@ff0f0000 (xlnx,zynqmp-qspi-1.0): $nodename:0: 'spi-controller@ff0f0000' does not match '^spi(@.*|-([0-9]|[1-9][0-9]+))?$'
	from schema $id: http://devicetree.org/schemas/spi/spi-zynqmp-qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.example.dtb: spi-controller@ff0f0000 (xlnx,zynqmp-qspi-1.0): Unevaluated properties are not allowed ('cs-gpios', 'flash@0', 'flash@1', 'flash@2', 'num-cs' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/spi-zynqmp-qspi.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250616220054.3968946-3-sean.anderson@linux.dev

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.