diff mbox series

[v10,5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

Message ID 20220131133049.77780-6-robert.marko@sartura.hr
State Accepted
Commit 54ae8c4b8c29e576d6dbfb49832522f782bb7190
Headers show
Series Add Delta TN48M CPLD support | expand

Commit Message

Robert Marko Jan. 31, 2022, 1:30 p.m. UTC
Add binding documents for the Delta TN48M CPLD drivers.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v7:
* Update bindings to reflect driver updates

Changes in v3:
* Include bindings for reset driver

Changes in v2:
* Implement MFD as a simple I2C MFD
* Add GPIO bindings as separate
---
 .../bindings/gpio/delta,tn48m-gpio.yaml       | 39 ++++++++
 .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
 .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
 3 files changed, 164 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
 create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml

Comments

Rob Herring (Arm) March 2, 2022, 9:39 p.m. UTC | #1
On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> On Mon, 31 Jan 2022, Robert Marko wrote:
> 
> > Add binding documents for the Delta TN48M CPLD drivers.
> > 
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> 
> This is missing a DT review.

How about this one[1]?

Rob

[1] https://lore.kernel.org/all/20210719225906.GA2769608@robh.at.kernel.org/
Robert Marko March 3, 2022, 12:41 p.m. UTC | #2
On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > On Mon, 31 Jan 2022, Robert Marko wrote:
> >
> > > Add binding documents for the Delta TN48M CPLD drivers.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> >
> > This is missing a DT review.
>
> How about this one[1]?
>
> Rob
>
> [1] https://lore.kernel.org/all/20210719225906.GA2769608@robh.at.kernel.org/

Hi Rob,
Thanks for reaching out.

As you can see the bindings have evolved since v6,
GPIO driver now only uses 2 distinct compatibles.

I am open to discussion in order to reach some kind of a consensus on
this matter
as the whole series is currently unfortunately stalled.

Regards,
Robert
Rob Herring (Arm) March 4, 2022, 9:47 p.m. UTC | #3
On Thu, Mar 03, 2022 at 01:41:13PM +0100, Robert Marko wrote:
> On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > > On Mon, 31 Jan 2022, Robert Marko wrote:
> > >
> > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > >
> > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > >
> > > This is missing a DT review.
> >
> > How about this one[1]?
> >
> > Rob
> >
> > [1] https://lore.kernel.org/all/20210719225906.GA2769608@robh.at.kernel.org/
> 
> Hi Rob,
> Thanks for reaching out.
> 
> As you can see the bindings have evolved since v6,
> GPIO driver now only uses 2 distinct compatibles.

Fundamentally, it hasn't really changed.

There's 2 main issues. First, I don't see the need for any child nodes. 
This would be sufficient:

cpld@41 {
    compatible = "delta,tn48m-cpld";
    reg = <0x41>;
    #reset-cells = <1>;
    #gpio-cells = <2>;
    gpio-controller;
};

You only need child nodes if the sub-blocks have their own resources or 
are widely reused in different configurations.

The 2nd issue is whether GPIOs are even GPIOs at all. I don't recall 
that Linus ever agreed.

Both issues kind of boil down to is there even more that 1 variation of 
this h/w where you have differing connections? AFAICT, Delta tn48m is a 
pretty specific device and I would guess something implemented in a CPLD 
is likely to change on every board design. At least that's my experience 
with 'board level logic'.

Rob
Robert Marko March 17, 2022, 10:19 a.m. UTC | #4
On Fri, Mar 4, 2022 at 10:47 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Mar 03, 2022 at 01:41:13PM +0100, Robert Marko wrote:
> > On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > > > On Mon, 31 Jan 2022, Robert Marko wrote:
> > > >
> > > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > > >
> > > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > >
> > > > This is missing a DT review.
> > >
> > > How about this one[1]?
> > >
> > > Rob
> > >
> > > [1] https://lore.kernel.org/all/20210719225906.GA2769608@robh.at.kernel.org/
> >
> > Hi Rob,
> > Thanks for reaching out.
> >
> > As you can see the bindings have evolved since v6,
> > GPIO driver now only uses 2 distinct compatibles.
>
> Fundamentally, it hasn't really changed.
>
> There's 2 main issues. First, I don't see the need for any child nodes.
> This would be sufficient:
>
> cpld@41 {
>     compatible = "delta,tn48m-cpld";
>     reg = <0x41>;
>     #reset-cells = <1>;
>     #gpio-cells = <2>;
>     gpio-controller;
> };
>
> You only need child nodes if the sub-blocks have their own resources or
> are widely reused in different configurations.
>
> The 2nd issue is whether GPIOs are even GPIOs at all. I don't recall
> that Linus ever agreed.
>
> Both issues kind of boil down to is there even more that 1 variation of
> this h/w where you have differing connections? AFAICT, Delta tn48m is a
> pretty specific device and I would guess something implemented in a CPLD
> is likely to change on every board design. At least that's my experience
> with 'board level logic'.

Hi Rob, sorry for the late reply.

Having one node was the route I went in v1, but that was rejected as
it would mean
having an MFD driver that just registers the sub-drivers.
That is what the simple-mfd-i2c driver was designed to get rid of and
inherit the regmap
from the parent.
For this to work, subnodes are required as we need to match on compatibles.

Using subnodes for GPIO-s also gets rid of hardcoding the register
layout in the driver per board,
as Delta chose to use a weird register layout in which the GPIO
registers have completely random offsets.
The layout is even weirder in the TN4810M which uses the same CPLD but
expanded and is easily
supportable in the same driver in the current form.
My goal and the requirement from the community was to make the GPIO
driver as simple as possible
and extendable so that boards like TN4810M can be easily added.

Also, the Kontron SL28CPLD does pretty much the same in regards to DT
as well as other things.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml?h=v5.16.15

It uses the same logic with different compatibles for GPIO to be able
to inform the kernel of the certain
bank capabilities.
I mean, using one compatible would be possible by using a boolean
property for example that tells you
that its output capable as well.

Regards,
Robert

>
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
new file mode 100644
index 000000000000..e3e668a12091
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
@@ -0,0 +1,39 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD GPIO controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  This module is part of the Delta TN48M multi-function device. For more
+  details see ../mfd/delta,tn48m-cpld.yaml.
+
+  Delta TN48M has an onboard Lattice CPLD that is used as an GPIO expander.
+  It provides 12 pins in total, they are input-only or ouput-only type.
+
+properties:
+  compatible:
+    enum:
+      - delta,tn48m-gpo
+      - delta,tn48m-gpi
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - gpio-controller
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
new file mode 100644
index 000000000000..f6967c1f6235
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
@@ -0,0 +1,90 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  Lattice CPLD onboard the TN48M switches is used for system
+  management.
+
+  It provides information about the hardware model, revision,
+  PSU status etc.
+
+  It is also being used as a GPIO expander and reset controller
+  for the switch MAC-s and other peripherals.
+
+properties:
+  compatible:
+    const: delta,tn48m-cpld
+
+  reg:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^gpio(@[0-9a-f]+)?$":
+    $ref: ../gpio/delta,tn48m-gpio.yaml
+
+  "^reset-controller?$":
+    $ref: ../reset/delta,tn48m-reset.yaml
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cpld@41 {
+            compatible = "delta,tn48m-cpld";
+            reg = <0x41>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            gpio@31 {
+                compatible = "delta,tn48m-gpo";
+                reg = <0x31>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            gpio@3a {
+                compatible = "delta,tn48m-gpi";
+                reg = <0x3a>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            gpio@40 {
+                compatible = "delta,tn48m-gpi";
+                reg = <0x40>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            reset-controller {
+              compatible = "delta,tn48m-reset";
+              #reset-cells = <1>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
new file mode 100644
index 000000000000..0e5ee8decc0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
@@ -0,0 +1,35 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD reset controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  This module is part of the Delta TN48M multi-function device. For more
+  details see ../mfd/delta,tn48m-cpld.yaml.
+
+  Reset controller modules provides resets for the following:
+  * 88F7040 SoC
+  * 88F6820 SoC
+  * 98DX3265 switch MAC-s
+  * 88E1680 PHY-s
+  * 88E1512 PHY
+  * PoE PSE controller
+
+properties:
+  compatible:
+    const: delta,tn48m-reset
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - "#reset-cells"
+
+additionalProperties: false