| Message ID | 20220131133049.77780-6-robert.marko@sartura.hr |
|---|---|
| State | New |
| Headers | show |
| Series | Add Delta TN48M CPLD support | expand |
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/
On Wed, 02 Mar 2022, Rob Herring 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/ Looks like the ball is in your court Robert.
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
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
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 --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
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