Skip to contentSkip to conversation

dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip

PATCH Superseded Brad Larson 6 participants 14 messages Nov 16, 2022 – Feb 6, 2023

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH v7 6/17 dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

---

v7:
- Use system-controller for the device with four chip-selects connected over spi.
- Delete child by moving reset-controller into the parent.
- Updated and used dtschema-2022.11 and yamllint-1.28.0

v6:
- Expand description, rename nodes and change compatible usage

v5:
- Change to AMD Pensando instead of Pensando

v4:
- Change Maintained to Supported

 .../bindings/mfd/amd,pensando-elbasr.yaml     | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +74 -0
new file mode 100644
index 000000000000..ac44d7d0a91a
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/amd,pensando-elbasr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando Elba SoC Resource Controller bindings
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.
+
+maintainers:
+  - Brad Larson <[email protected]>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - amd,pensando-elbasr
+
+  "#reset-cells":
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+        status = "okay";
+
+        rstc: system-controller@0 {
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };
+
+        system-controller@1 {
+            compatible = "amd,pensando-elbasr";
+            reg = <1>;
+            spi-max-frequency = <12000000>;
+        };
+
+        system-controller@2 {
+            compatible = "amd,pensando-elbasr";
+            reg = <2>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        };
+
+        system-controller@3 {
+            compatible = "amd,pensando-elbasr";
+            reg = <3>;
+            spi-max-frequency = <12000000>;
+        };
+    };
Signature · 1 line
2.17.1
#1 lore raw
Quoted 31 lines · Brad Larson

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

---

v7:
- Use system-controller for the device with four chip-selects connected over spi.
- Delete child by moving reset-controller into the parent.
- Updated and used dtschema-2022.11 and yamllint-1.28.0

v6:
- Expand description, rename nodes and change compatible usage

v5:
- Change to AMD Pensando instead of Pensando

v4:
- Change Maintained to Supported

 .../bindings/mfd/amd,pensando-elbasr.yaml     | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0
new file mode 100644
index 000000000000..ac44d7d0a91a

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 7

Quoted 7 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +7 -0
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/amd,pensando-elbasr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando Elba SoC Resource Controller bindings

Drop "bindings"

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 18

Quoted 11 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +11 -0
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.
+
+maintainers:
+  - Brad Larson <[email protected]>
+
+properties:
+  compatible:
+    items:

You have only one item, so no need for list. Drop "items".

Quoted 18 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +18 -0
+      - enum:
+          - amd,pensando-elbasr
+
+  "#reset-cells":
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 46

Quoted 10 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +10 -0
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+        status = "okay";

Drop status.

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 48

Quoted 2 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +2 -0
+
+        rstc: system-controller@0 {

Drop label.

Quoted 8 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +8 -0
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };
+
+        system-controller@1 {
+            compatible = "amd,pensando-elbasr";

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 66

Quoted 10 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +10 -0
+            reg = <1>;
+            spi-max-frequency = <12000000>;
+        };
+
+        system-controller@2 {
+            compatible = "amd,pensando-elbasr";
+            reg = <2>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;

Keep only one or two examples. All others are practically the same.

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 74

Quoted 8 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +8 -0
+        };
+
+        system-controller@3 {
+            compatible = "amd,pensando-elbasr";
+            reg = <3>;
+            spi-max-frequency = <12000000>;
+        };
+    };

Best regards,
Krzysztof

#3 lore raw

PATCH v8 6/17 dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

---

v8:
- Apply review request changes and picked the two unique examples for the 4 chip-selects as one has the reset control support and the other an interrupt. Missed the --in-reply-to in git send-email for v7, included in this update.

v7:
- Use system-controller for the device with four chip-selects connected over spi.
- Delete child by moving reset-controller into the parent.
- Updated and used dtschema-2022.11 and yamllint-1.28.0

v6:
- Expand description, rename nodes and change compatible usage

v5:
- Change to AMD Pensando instead of Pensando

v4:
- Change Maintained to Supported

 .../bindings/mfd/amd,pensando-elbasr.yaml     | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +60 -0
new file mode 100644
index 000000000000..622c93402a86
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/amd,pensando-elbasr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.
+
+maintainers:
+  - Brad Larson <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - amd,pensando-elbasr
+
+  "#reset-cells":
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+
+        system-controller@0 {
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };
+
+        system-controller@2 {
+            compatible = "amd,pensando-elbasr";
+            reg = <2>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
Signature · 1 line
2.17.1
maintainer
#4 lore raw
Quoted 11 lines · Brad Larson

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

---

v8:
- Apply review request changes and picked the two unique examples for the 4 chip-selects as one has the reset control support and the other an interrupt. Missed the --in-reply-to in git send-email for v7, included in this update.

No, you haven't. By default in git, you don't have to do anything. See --thread and --no-chain-reply-to options. If you are messing with --in-reply-to, you are doing it wrong.

Please resend the whole series properly threaded.

Quoted 27 lines

v7:
- Use system-controller for the device with four chip-selects connected over spi.
- Delete child by moving reset-controller into the parent.
- Updated and used dtschema-2022.11 and yamllint-1.28.0

v6:
- Expand description, rename nodes and change compatible usage

v5:
- Change to AMD Pensando instead of Pensando

v4:
- Change Maintained to Supported

 .../bindings/mfd/amd,pensando-elbasr.yaml     | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +1 -0
new file mode 100644
index 000000000000..622c93402a86
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 10 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +10 -0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/amd,pensando-elbasr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

Quoted 53 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +49 -0
+
+maintainers:
+  - Brad Larson <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - amd,pensando-elbasr
+
+  "#reset-cells":
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+
+        system-controller@0 {
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };
+
+        system-controller@2 {
+            compatible = "amd,pensando-elbasr";
+            reg = <2>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
Signature · 1 line
2.17.1
#5 lore raw

[AMD Official Use Only - General]

From: Rob Herring <[email protected]>
Sent: Wednesday, November 16, 2022 2:30 PM

Quoted 11 lines
Quoted 5 lines

v8:
- Apply review request changes and picked the two unique examples for the 4 chip-selects as one has the reset control support and the other an interrupt. Missed the --in-reply-to in git send-email for v7, included in this update.

No, you haven't. By default in git, you don't have to do anything. See --thread and --no-chain-reply-to options. If you are messing with --in-reply-to, you are doing it wrong.

Please resend the whole series properly threaded.

Will resend the series

Quoted 6 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 4 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -1
new file mode 100644
index 000000000000..622c93402a86
--- /dev/null

+++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 1 line
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0
@@ -0,0 +1,60 @@

...

Quoted 11 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 6 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +6 -0
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

I agree and the example for v7 had all 4 chip-selects shown. This is not a pick and choose device on what functions to use for a usable system. Krzysztof requested only showing two chip-selects in the example.
...

Quoted 16 lines
Quoted 16 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +16 -0
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+
+        system-controller@0 {
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };
+
Quoted 8 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 60

Quoted 8 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +8 -0
+        system-controller@2 {
+            compatible = "amd,pensando-elbasr";
+            reg = <2>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };

Regards,
Brad

#6 lore raw
Quoted 35 lines · Bradley

[AMD Official Use Only - General]

From: Rob Herring <[email protected]>
Sent: Wednesday, November 16, 2022 2:30 PM

Quoted 11 lines
Quoted 5 lines

v8:
- Apply review request changes and picked the two unique examples for the 4 chip-selects as one has the reset control support and the other an interrupt. Missed the --in-reply-to in git send-email for v7, included in this update.

No, you haven't. By default in git, you don't have to do anything. See --thread and --no-chain-reply-to options. If you are messing with --in-reply-to, you are doing it wrong.

Please resend the whole series properly threaded.

Will resend the series

Quoted 6 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 4 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -1
new file mode 100644
index 000000000000..622c93402a86
--- /dev/null

+++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 1 line
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0
@@ -0,0 +1,60 @@

...

Quoted 9 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 6 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +6 -0
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Quoted 6 lines
Quoted 2 lines

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

I agree and the example for v7 had all 4 chip-selects shown. This is not a pick and choose device on what functions to use for a usable system. Krzysztof requested only showing two chip-selects in the example.

The problem is that you describe here SPI controller (and its chip selects) but binding is for the SPI device. The example is not the problem...

Quoted 9 lines

...

Quoted 8 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 44

Quoted 8 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +8 -0
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;

Drop this property as well, unless it is necessary to explain "amd,pensando-elbasr" device.

Quoted 7 lines
Quoted 7 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 51

Quoted 7 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +7 -0
+
+        system-controller@0 {
+            compatible = "amd,pensando-elbasr";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            #reset-cells = <1>;
+        };

Best regards,
Krzysztof

maintainer
#7 lore raw
In reply to Larson, Bradley
Quoted 36 lines · Bradley

[AMD Official Use Only - General]

From: Rob Herring <[email protected]>
Sent: Wednesday, November 16, 2022 2:30 PM

Quoted 11 lines
Quoted 5 lines

v8:
- Apply review request changes and picked the two unique examples for the 4 chip-selects as one has the reset control support and the other an interrupt. Missed the --in-reply-to in git send-email for v7, included in this update.

No, you haven't. By default in git, you don't have to do anything. See --thread and --no-chain-reply-to options. If you are messing with --in-reply-to, you are doing it wrong.

Please resend the whole series properly threaded.

Will resend the series

Quoted 6 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 4 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -1
new file mode 100644
index 000000000000..622c93402a86
--- /dev/null

+++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Quoted 1 line
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0
@@ -0,0 +1,60 @@

...

Quoted 9 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 6 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +6 -0
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Quoted 4 lines
Quoted 2 lines

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

I agree and the example for v7 had all 4 chip-selects shown.

That is not what I said. Look at 'reg' above again. You have 1 device, but you have 4 nodes which looks like separate 4 devices. The exception would be if what's on each chip select is independent from each other.

Describe what your h/w has/is/does so we can provide better guidance.

Rob

#8 lore raw
In reply to Larson, Bradley

[AMD Official Use Only - General]

Hi Rob and Krzysztof,

Quoted 15 lines
Quoted 15 lines
Quoted 2 lines
Quoted 2 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0

...

Quoted 11 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 6 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +6 -0
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

Quoted 8 lines
Quoted 1 line

I agree and the example for v7 had all 4 chip-selects shown.

That is not what I said. Look at 'reg' above again. You have 1 device, but you have 4 nodes which looks like separate 4 devices. The exception would be if what's on each chip select is independent from each other.

Describe what your h/w has/is/does so we can provide better guidance.

This is the hardware design for every implementation with the Elba SoC

Elba <-- spi --> One FPGA or CPLD (4 chip-selects)

where there are four functions in the FPGA accessed by userspace applications except for emmc reset control which is a bit in a CS0 register. The IP at CS0 is a bunch of miscellaneous mgmt registers. CS1 is a designware I2C master/slave. CS2 is a Lattice dual I2C master. CS3 is internal storage for the CPLD or FPGA depending on the hardware implementation.

'reg = <0 1 2 3>' in the dt fragment below is indicating the chip-select which is what other bindings appear to be doing. Maybe one answer is delete this and add our compatible to spidev.c in the patchset we provide to customers. Adding our compatible to spidev.c was nack'ed. Recommendation?

Not including 'reg' results in this compile warning:

DTC arch/arm64/boot/dts/amd/elba-asic.dtb
arch/arm64/boot/dts/amd/elba-asic-common.dtsi:73.28-78.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@0: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:80.22-84.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@1: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:86.22-92.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@2: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:94.22-98.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@3: missing or empty reg property
CALL scripts/atomic/check-atomics.sh

&emmc {
        bus-width = <8>;
        cap-mmc-hw-reset;
        reset-names = "hw";
        resets = <&rstc 0>;
        status = "okay";
};
&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        num-cs = <4>;
        cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
                   <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";

        rstc: system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0>;
                spi-max-frequency = <12000000>;
                #reset-cells = <1>;
        };

        system-controller@1 {
                compatible = "amd,pensando-elbasr";
                reg = <1>;
                spi-max-frequency = <12000000>;
        };

        system-controller@2 {
                compatible = "amd,pensando-elbasr";
                reg = <2>;
                spi-max-frequency = <12000000>;
                interrupt-parent = <&porta>;
                interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
        };

        system-controller@3 {
                compatible = "amd,pensando-elbasr";
                reg = <3>;
                spi-max-frequency = <12000000>;
        };
};

Regards,
Brad

#9 lore raw

Hi Brad,

Quoted 24 lines · Bradley
Quoted 15 lines
Quoted 15 lines
Quoted 2 lines
Quoted 2 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +0 -0

...

Quoted 11 lines

Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml line 11

Quoted 6 lines
Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml new file +6 -0
+
+title: AMD Pensando Elba SoC Resource Controller
+
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

Quoted 8 lines
Quoted 1 line

I agree and the example for v7 had all 4 chip-selects shown.

That is not what I said. Look at 'reg' above again. You have 1 device, but you have 4 nodes which looks like separate 4 devices. The exception would be if what's on each chip select is independent from each other.

Describe what your h/w has/is/does so we can provide better guidance.

Quoted 7 lines

This is the hardware design for every implementation with the Elba SoC

Elba <-- spi --> One FPGA or CPLD (4 chip-selects)

where there are four functions in the FPGA accessed by userspace applications except for emmc reset control which is a bit in a CS0 register. The IP at CS0 is a bunch of miscellaneous mgmt registers. CS1 is a designware I2C master/slave. CS2 is a Lattice dual I2C master. CS3 is internal storage for the CPLD or FPGA depending on the hardware implementation.

'reg = <0 1 2 3>' in the dt fragment below is indicating the chip-select which is what other bindings appear to be doing. Maybe one answer is delete this and add our compatible to spidev.c in the patchset we provide to customers. Adding our compatible to spidev.c was nack'ed. Recommendation?

The fragment below does not have "reg = <0 1 2 3>", but individual subnodes, each with their own "reg" property.

Quoted 43 lines

Not including 'reg' results in this compile warning:

DTC arch/arm64/boot/dts/amd/elba-asic.dtb
arch/arm64/boot/dts/amd/elba-asic-common.dtsi:73.28-78.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@0: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:80.22-84.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@1: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:86.22-92.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@2: missing or empty reg property arch/arm64/boot/dts/amd/elba-asic-common.dtsi:94.22-98.4: Warning (spi_bus_reg): /soc/spi@2800/system-controller@3: missing or empty reg property
CALL scripts/atomic/check-atomics.sh

&emmc {
        bus-width = <8>;
        cap-mmc-hw-reset;
        reset-names = "hw";
        resets = <&rstc 0>;
        status = "okay";
};
&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        num-cs = <4>;
        cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
                   <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";

        rstc: system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0>;
                spi-max-frequency = <12000000>;
                #reset-cells = <1>;
        };

        system-controller@1 {
                compatible = "amd,pensando-elbasr";
                reg = <1>;
                spi-max-frequency = <12000000>;
        };

        system-controller@2 {
                compatible = "amd,pensando-elbasr";
                reg = <2>;
                spi-max-frequency = <12000000>;
                interrupt-parent = <&porta>;
Quoted 8 lines
                interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
        };

        system-controller@3 {
                compatible = "amd,pensando-elbasr";
                reg = <3>;
                spi-max-frequency = <12000000>;
        };

The above describes 4 individual but further identical (they have the same compatible value) devices, while above you say they are not identical:

The IP at CS0 is a bunch of miscellaneous mgmt registers. CS1 is a designware I2C master/slave.
CS2 is a Lattice dual I2C master.
CS3 is internal storage for the CPLD or FPGA depending on the
hardware implementation.

So either this should be modelled as a single subnode with 4 chip selects[*]:

        system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0 1 2 3 4>;
                spi-max-frequency = <12000000>;
                #reset-cells = <1>;
        };

or as 4 separate subnodes, each using 4 different compatible values. Giving the wildly different functionalities provided by each, you also need 4 binding documents.

[*] I'm not sure the Linux SPI core actually supports this yet.

Quoted 1 line

};

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#10 lore raw

[AMD Official Use Only - General]

Hi Geert,

From: Geert Uytterhoeven <[email protected]>
Sent: Friday, November 18, 2022 12:42 AM

Quoted 55 lines

...

Quoted 26 lines
&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        num-cs = <4>;
        cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
                   <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";

        rstc: system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0>;
                spi-max-frequency = <12000000>;
                #reset-cells = <1>;
        };

        system-controller@1 {
                compatible = "amd,pensando-elbasr";
                reg = <1>;
                spi-max-frequency = <12000000>;
        };

        system-controller@2 {
                compatible = "amd,pensando-elbasr";
                reg = <2>;
                spi-max-frequency = <12000000>;
                interrupt-parent = <&porta>;
Quoted 8 lines
                interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
        };

        system-controller@3 {
                compatible = "amd,pensando-elbasr";
                reg = <3>;
                spi-max-frequency = <12000000>;
        };

The above describes 4 individual but further identical (they have the same compatible value) devices, while above you say they are not identical:

The IP at CS0 is a bunch of miscellaneous mgmt registers. CS1 is a designware I2C master/slave.
CS2 is a Lattice dual I2C master.
CS3 is internal storage for the CPLD or FPGA depending on the
hardware implementation.

So either this should be modelled as a single subnode with 4 chip selects[*]:

        system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0 1 2 3 4>;
                spi-max-frequency = <12000000>;
                #reset-cells = <1>;
        };
Quoted 5 lines

or as 4 separate subnodes, each using 4 different compatible values. Giving the wildly different functionalities provided by each, you also need 4 binding documents.

[*] I'm not sure the Linux SPI core actually supports this yet.

Thanks, I was unfamiliar with reg <0 ...> with differences between the nodes such as reset-cells for one and interrupts on another. Below dts boots ok and will change binding and driver as needed.

&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        num-cs = <4>;
        cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
                   <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";

        rstc: system-controller@0 {
                compatible = "amd,pensando-elbasr";
                reg = <0 1 2 3>;
                spi-max-frequency = <12000000>;
                interrupt-parent = <&porta>;
                interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
                #reset-cells = <1>;
        };
};

Regards,
Brad

maintainer
#11 lore raw
In reply to Rob Herring

Hi Rob,

Quoted 5 lines · Rob Herring
Quoted 2 lines · Brad Larson

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

Please resend the whole series properly threaded.

Series sent as v9

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

This has been changed to one device and four chip selects. This binding error is occuring for snps,dw-apb-ssi.yaml using reg for the chip selects. Any guidance on how to fix?

$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: system-controller@0:reg: [[0], [1], [2], [3]] is too long
	From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml

Binding:

Documentation/devicetree/bindings/spi/amd,pensando-sr.yaml new file +68 -0
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/amd,pensando-sr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando SoC Resource Controller
+
+description: |
+  AMD Pensando SoC Resource Controller is a set of control/status
+  registers accessed on four chip-selects.  This device is present
+  in all Pensando SoC based designs.
+
+  CS0 is a set of miscellaneous control/status registers to
+  include reset control.  CS1/CS2 are for I2C peripherals.
+  CS3 is to access resource controller internal storage.
+
+maintainers:
+  - Brad Larson <[email protected]>
+
+properties:
+  compatible:
+    const: amd,pensando-sr
+
+  reg:
+    maxItems: 4
+    minimum: 0
+    maximum: 3
+    description:
+      Device chip select number
+
+  '#reset-cells':
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - '#reset-cells'
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+
+        system-controller@0 {
+            compatible = "amd,pensando-sr";
+            reg = <0 1 2 3>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+            #reset-cells = <1>;
+        };
+    };
+
+...

DT node, single device and using reg for four chip-selects

&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        num-cs = <4>;
        cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
                   <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";

        rstc: system-controller@0 {
                compatible = "amd,pensando-sr";
                reg = <0 1 2 3>;
                spi-max-frequency = <12000000>;
                interrupt-parent = <&porta>;
                interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
                #reset-cells = <1>;
        };
};

Regards,
Brad

#12 lore raw
Quoted 24 lines · Brad Larson

Hi Rob,

Quoted 5 lines · Rob Herring
Quoted 2 lines · Brad Larson

Add support for the AMD Pensando Elba SoC System Resource chip using the SPI interface.

Please resend the whole series properly threaded.

Series sent as v9

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+description: |
+  AMD Pensando Elba SoC Resource Controller functions are
+  accessed with four chip-selects.  Reset control is on CS0.

One device with 4 chip-selects? Then I'd expect 'reg = <0 1 2 3>;'

Hard to say more because I don't have the whole thread nor remember what exactly we discussed before. That was 100s of bindings ago...

This has been changed to one device and four chip selects. This binding error is occuring for snps,dw-apb-ssi.yaml using reg for the chip selects. Any guidance on how to fix?

Quoted 7 lines
$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: system-controller@0:reg: [[0], [1], [2], [3]] is too long
	From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml

Maybe this would work in snps,dw-apb-ssi for children:

reg:
items:
minimum: 0
maximum: 3

Best regards,
Krzysztof

maintainer
#13 lore raw

Hi Krzysztof,

Quoted 1 line · Krzysztof Kozlowski

On 30/01/2023 20:12, Brad Larson wrote:

...

Quoted 12 lines
Quoted 11 lines

This has been changed to one device and four chip selects. This binding error is occuring for snps,dw-apb-ssi.yaml using reg for the chip selects. Any guidance on how to fix?

$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: system-controller@0:reg: [[0], [1], [2], [3]] is too long
	From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
Quoted 6 lines

Maybe this would work in snps,dw-apb-ssi for children:

reg:
items:
minimum: 0
maximum: 3

With the above change here in snps,dw-apb-ssi.yaml:

- if:
properties:
compatible:
contains:
const: amd,pensando-elba-spi
then:
properties:
reg:
items:
minimum: 0
maximum: 3
required:
- amd,pensando-elba-syscon
else:
properties:
amd,pensando-elba-syscon: false

this is the result:

$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: reg:0: [0, 10240, 0, 256] is too long
	From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: system-controller@0:reg: [[0], [1], [2], [3]] is too long
	From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml

The binding snps,dw-apb-ssi.yaml has patternProperties defined this way:

patternProperties:
"^.*@[0-9a-f]+$":
type: object
properties:
reg:
minimum: 0
maximum: 3

- Removing patternProperties makes the error go away indicating an issue with minimum/maximum regex check and the number of items in the reg property which shouldn't be related.

- Changing patternProperties to this makes the error go away. patternProperties:
"^.*@[0-9a-f]+$":
type: object
properties:
reg:
maxItems: 4

- Using spmi.yaml as a reference and changing patternProperties to the following:
patternProperties:
"^.*@[0-9a-f]+$":
type: object
properties:
reg:
items:
- maxItems: 4
items:
- minimum: 0
- maximum: 3
required:
- reg

results in:

$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
arch/arm64/Makefile:36: Detected assembler with broken .inst; disassembly will be unreliable
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml: patternProperties:^.*@[0-9a-f]+$:properties:reg:items: 'oneOf' conditional failed, one must be fixed:
	[{'maxItems': 4, 'items': [{'minimum': 0}, {'maximum': 3}]}] is not of type 'object'
	{'maxItems': 4, 'items': [{'minimum': 0}, {'maximum': 3}]} should not be valid under {'required': ['maxItems']}
		hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb

- With this version for patternProperties, to retain minimum/maximum, the original error occurs:
patternProperties:
"^.*@[0-9a-f]+$":
type: object
properties:
reg:
items:
- minItems: 1
items:
- minimum: 0
- maximum: 3
required:
- reg

$ make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
arch/arm64/Makefile:36: Detected assembler with broken .inst; disassembly will be unreliable
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json

DTC_CHK arch/arm64/boot/dts/amd/elba-asic.dtb
/home/brad/linux.v10/arch/arm64/boot/dts/amd/elba-asic.dtb: spi@2800: system-controller@0:reg: [[0], [1], [2], [3]] is too long
From schema: /home/brad/linux.v10/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml

Regards,
Brad