diff mbox series

[v5,06/15] dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip

Message ID 20220613195658.5607-7-brad@pensando.io
State New
Headers show
Series Support AMD Pensando Elba SoC | expand

Commit Message

Brad Larson June 13, 2022, 7:56 p.m. UTC
From: Brad Larson <blarson@amd.com>

Add support for the AMD Pensando Elba SoC System Resource chip
using the SPI interface.  The Elba SR is a Multi-function Device
supporting device register access using CS0, smbus interface for
FRU and board peripherals using CS1, dual Lattice I2C masters for
transceiver management using CS2, and CS3 for flash access.

Signed-off-by: Brad Larson <blarson@amd.com>
---
 .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml

Comments

Rob Herring (Arm) June 14, 2022, 9:30 p.m. UTC | #1
On Mon, Jun 13, 2022 at 12:56:49PM -0700, Brad Larson wrote:
> From: Brad Larson <blarson@amd.com>
> 
> Add support for the AMD Pensando Elba SoC System Resource chip
> using the SPI interface.  The Elba SR is a Multi-function Device
> supporting device register access using CS0, smbus interface for
> FRU and board peripherals using CS1, dual Lattice I2C masters for
> transceiver management using CS2, and CS3 for flash access.
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
>  .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> new file mode 100644
> index 000000000000..13356800b1cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0 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 bindings attached to a SPI bus.
> +
> +maintainers:
> +  - Brad Larson <blarson@amd.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - amd,pensando-elbasr
> +      - const: simple-mfd
> +
> +  spi-max-frequency:
> +    description: Maximum SPI frequency of the device in Hz.
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +
> +patternProperties:
> +  '^reset-controller@[a-f0-9]+$':
> +    $ref: ../reset/amd,pensando-elbasr-reset.yaml

/schemas/reset/...

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        num-cs = <4>;
> +
> +        spi@0 {
> +          compatible = "amd,pensando-elbasr", "simple-mfd";
> +          reg = <0>;
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          spi-max-frequency = <12000000>;
> +
> +          rstc: reset-controller@0 {

Only one child does not make a MFD...

> +            compatible = "amd,pensando-elbasr-reset";
> +            reg = <0>;
> +            #reset-cells = <1>;
> +          };
> +        };
> +
> +        spi@1 {
> +          compatible = "amd,pensando-elbasr", "simple-mfd";
> +          reg = <1>;
> +          spi-max-frequency = <12000000>;

'simple-mfd' implies there are child nodes, but you have none. Is this 
complete?

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

This one has interrupt but the others don't?

> +        };
> +
> +        spi@3 {
> +          compatible = "amd,pensando-elbasr", "simple-mfd";
> +          reg = <3>;
> +          spi-max-frequency = <12000000>;
> +        };
> +    };
> +
> +...
> -- 
> 2.17.1
> 
>
Krzysztof Kozlowski June 20, 2022, 12:56 p.m. UTC | #2
On 13/06/2022 21:56, Brad Larson wrote:
> From: Brad Larson <blarson@amd.com>
> 
> Add support for the AMD Pensando Elba SoC System Resource chip
> using the SPI interface.  The Elba SR is a Multi-function Device
> supporting device register access using CS0, smbus interface for
> FRU and board peripherals using CS1, dual Lattice I2C masters for
> transceiver management using CS2, and CS3 for flash access.
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
>  .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> new file mode 100644
> index 000000000000..13356800b1cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0 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 bindings attached to a SPI bus.
> +
> +maintainers:
> +  - Brad Larson <blarson@amd.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - amd,pensando-elbasr
> +      - const: simple-mfd
> +
> +  spi-max-frequency:
> +    description: Maximum SPI frequency of the device in Hz.
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +
> +patternProperties:
> +  '^reset-controller@[a-f0-9]+$':
> +    $ref: ../reset/amd,pensando-elbasr-reset.yaml
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    spi0 {

Just "spi"

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        num-cs = <4>;
> +
> +        spi@0 {

"spi" is for SPI controllers. Use generic name matching the device.
Usually this is "system-controller", however Rob pointed out your
inaccurate bindings and example.

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


Best regards,
Krzysztof
Brad Larson July 3, 2022, 11:30 p.m. UTC | #3
Hi Rob,

On Tue, Jun 14, 2022 at 2:30 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Jun 13, 2022 at 12:56:49PM -0700, Brad Larson wrote:
> > From: Brad Larson <blarson@amd.com>
> >
> > Add support for the AMD Pensando Elba SoC System Resource chip
> > using the SPI interface.  The Elba SR is a Multi-function Device
> > supporting device register access using CS0, smbus interface for
> > FRU and board peripherals using CS1, dual Lattice I2C masters for
> > transceiver management using CS2, and CS3 for flash access.
> >
> > Signed-off-by: Brad Larson <blarson@amd.com>
> > ---
> >  .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> > new file mode 100644
> > index 000000000000..13356800b1cf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: (GPL-2.0 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
> ...
> > +patternProperties:
> > +  '^reset-controller@[a-f0-9]+$':
> > +    $ref: ../reset/amd,pensando-elbasr-reset.yaml
>
> /schemas/reset/...

Changed it to
       $ref: /schemas/reset/amd,pensando-elbasr-reset.yaml

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    spi0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        num-cs = <4>;
> > +
> > +        spi@0 {
> > +          compatible = "amd,pensando-elbasr", "simple-mfd";
> > +          reg = <0>;
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +          spi-max-frequency = <12000000>;
> > +
> > +          rstc: reset-controller@0 {
>
> Only one child does not make a MFD...

Looking over the approaches for other SoCs with an external
required controller (cpld, fpga) this appeared to be an
acceptable choice.  This device is accessed by several
different utilities/programs via /dev/pensr0.x, CS0 registers
for a variety of control/status, CS1 designware i2c master/slave,
CS2 lattice dual i2c masters, and CS3 for flash.

> > +            compatible = "amd,pensando-elbasr-reset";
> > +            reg = <0>;
> > +            #reset-cells = <1>;
> > +          };
> > +        };
> > +
> > +        spi@1 {
> > +          compatible = "amd,pensando-elbasr", "simple-mfd";
> > +          reg = <1>;
> > +          spi-max-frequency = <12000000>;
>
> 'simple-mfd' implies there are child nodes, but you have none. Is this
> complete?

This function is a designware i2c master/slave for board peripheral
access.  Removed simple-mfd.

> > +        };
> > +
> > +        spi@2 {
> > +          compatible = "amd,pensando-elbasr", "simple-mfd";
> > +          reg = <2>;
> > +          spi-max-frequency = <12000000>;
> > +          interrupt-parent = <&porta>;
> > +          interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>
> This one has interrupt but the others don't?

Yes, this function is a Lattice dual I2C master for transceiver
management.  The spi to i2c driver is not included in this patch set for
essential Elba SoC support.  Removed simple-mfd.

Regards,
Brad
Brad Larson July 3, 2022, 11:41 p.m. UTC | #4
Hi Krzysztof,

On Mon, Jun 20, 2022 at 5:56 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 13/06/2022 21:56, Brad Larson wrote:
> > From: Brad Larson <blarson@amd.com>
> >
> > Add support for the AMD Pensando Elba SoC System Resource chip
> > using the SPI interface.  The Elba SR is a Multi-function Device
> > supporting device register access using CS0, smbus interface for
> > FRU and board peripherals using CS1, dual Lattice I2C masters for
> > transceiver management using CS2, and CS3 for flash access.
> >
> > Signed-off-by: Brad Larson <blarson@amd.com>
> > ---
> >  .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
> ...
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    spi0 {
>
> Just "spi"

Changed to spi

> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        num-cs = <4>;
> > +
> > +        spi@0 {
>
> "spi" is for SPI controllers. Use generic name matching the device.
> Usually this is "system-controller", however Rob pointed out your
> inaccurate bindings and example.

Proposed the below change node in the reply to Rob.  The model I
followed for this was the Altera mfd/altera-a10sr.c

spi@0 {
        sr_regs@0 {
                rstc: reset-controller@0 {

        dw_i2c@1 {

        lattice_i2c@2 {

        flash@3 {

Regards,
Brad
Krzysztof Kozlowski July 5, 2022, 9:46 a.m. UTC | #5
On 04/07/2022 01:41, Brad Larson wrote:
> Hi Krzysztof,
> 
> On Mon, Jun 20, 2022 at 5:56 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 13/06/2022 21:56, Brad Larson wrote:
>>> From: Brad Larson <blarson@amd.com>
>>>
>>> Add support for the AMD Pensando Elba SoC System Resource chip
>>> using the SPI interface.  The Elba SR is a Multi-function Device
>>> supporting device register access using CS0, smbus interface for
>>> FRU and board peripherals using CS1, dual Lattice I2C masters for
>>> transceiver management using CS2, and CS3 for flash access.
>>>
>>> Signed-off-by: Brad Larson <blarson@amd.com>
>>> ---
>>>  .../bindings/mfd/amd,pensando-elbasr.yaml     | 93 +++++++++++++++++++
>>>  1 file changed, 93 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
>> ...
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +    spi0 {
>>
>> Just "spi"
> 
> Changed to spi
> 
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        num-cs = <4>;
>>> +
>>> +        spi@0 {
>>
>> "spi" is for SPI controllers. Use generic name matching the device.
>> Usually this is "system-controller", however Rob pointed out your
>> inaccurate bindings and example.
> 
> Proposed the below change node in the reply to Rob.  The model I
> followed for this was the Altera mfd/altera-a10sr.c

You pointed driver, so how is it related to bindings? Do not mix Linux
implementation with the bindings.

> 
> spi@0 {
>         sr_regs@0 {
>                 rstc: reset-controller@0 {

No underscores in node names. sr_regs is not generic name.

> 
>         dw_i2c@1 {

Again, not a generic name. If it is i2c controller, should be i2c. If it
is i2c device/client, should be something generic matching class of the
device.

> 
>         lattice_i2c@2 {
> 
>         flash@3 {

This looks ok, depending on compatible.

> 
> Regards,
> Brad


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
new file mode 100644
index 000000000000..13356800b1cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml
@@ -0,0 +1,93 @@ 
+# SPDX-License-Identifier: (GPL-2.0 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 bindings attached to a SPI bus.
+
+maintainers:
+  - Brad Larson <blarson@amd.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - amd,pensando-elbasr
+      - const: simple-mfd
+
+  spi-max-frequency:
+    description: Maximum SPI frequency of the device in Hz.
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+
+patternProperties:
+  '^reset-controller@[a-f0-9]+$':
+    $ref: ../reset/amd,pensando-elbasr-reset.yaml
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/reset/amd,pensando-elba-reset.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        num-cs = <4>;
+
+        spi@0 {
+          compatible = "amd,pensando-elbasr", "simple-mfd";
+          reg = <0>;
+          #address-cells = <1>;
+          #size-cells = <0>;
+          spi-max-frequency = <12000000>;
+
+          rstc: reset-controller@0 {
+            compatible = "amd,pensando-elbasr-reset";
+            reg = <0>;
+            #reset-cells = <1>;
+          };
+        };
+
+        spi@1 {
+          compatible = "amd,pensando-elbasr", "simple-mfd";
+          reg = <1>;
+          spi-max-frequency = <12000000>;
+        };
+
+        spi@2 {
+          compatible = "amd,pensando-elbasr", "simple-mfd";
+          reg = <2>;
+          spi-max-frequency = <12000000>;
+          interrupt-parent = <&porta>;
+          interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        };
+
+        spi@3 {
+          compatible = "amd,pensando-elbasr", "simple-mfd";
+          reg = <3>;
+          spi-max-frequency = <12000000>;
+        };
+    };
+
+...