| Message ID | 20241225035851.420952-2-gch981213@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: add support for GPIO controller on Siflower SoCs | expand |
Hi Chuanhong, thanks for your patch! On Wed, Dec 25, 2024 at 4:59 AM Chuanhong Guo <gch981213@gmail.com> wrote: > Add dt binding doc for the GPIO controller found on Siflower SF19A2890 > and various other Siflower MIPS and RISC-V SoCs. > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> (...) > + interrupts: > + description: > + Interrupt mapping, one interrupt per 16 GPIOs. So from the driver it is very clear that this is lumping together several GPIO blocks with 16 GPIOs in each into a bigger GPIO controller, despite the instances are identical. They even each have an individual IRQ. > + ngpios: > + description: > + The number of GPIOs available on the controller implementation. > + minimum: 1 I would say minimum: 1 maximum: 16 default: 16. One instance per block/bank. > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/interrupt-controller/mips-gic.h> > + gpio@19d00000 { > + compatible = "siflower,sf19a2890-gpio"; > + reg = <0x19d00000 0x100000>; Just use 4 instances. Since (looking at the driver) it seems there is an IRQ register that is "off the bulk" I would do something like: instance 0: reg = <0x19d00000 0x40>, <0x19d04000 4>; instance: 1: reg = <0x19d00040 0x40>, <0x19d04004 4>; (...etc...) You can add reg-names if you don't want the implicit order of registers. (Perhaps the bindings maintainers will push for this as well.) > + interrupts = <GIC_SHARED 246 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SHARED 247 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SHARED 248 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SHARED 249 IRQ_TYPE_LEVEL_HIGH>; Just one IRQ and handle just one block per instance. > + clocks = <&gpioclk 0>; > + resets = <&gpiorst 0>; > + > + gpio-controller; > + #gpio-cells = <2>; > + ngpios = <49>; Just omit this on instances 0,1,2 and set to 1 on instance 3. > + gpio-ranges = <&pinctrl 0 0 49>; Augment this accordingly to one instance per bank/range. Yours, Linus Walleij
Hello! On Sat, Dec 28, 2024 at 12:45 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > Hi Chuanhong, > > thanks for your patch! > > On Wed, Dec 25, 2024 at 4:59 AM Chuanhong Guo <gch981213@gmail.com> wrote: > > > Add dt binding doc for the GPIO controller found on Siflower SF19A2890 > > and various other Siflower MIPS and RISC-V SoCs. > > > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > (...) > > + interrupts: > > + description: > > + Interrupt mapping, one interrupt per 16 GPIOs. > > So from the driver it is very clear that this is lumping together several > GPIO blocks with 16 GPIOs in each into a bigger GPIO controller, despite > the instances are identical. They even each have an individual IRQ. > > > + ngpios: > > + description: > > + The number of GPIOs available on the controller implementation. > > + minimum: 1 > > I would say minimum: 1 maximum: 16 default: 16. > > One instance per block/bank. > > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/irq.h> > > + #include <dt-bindings/interrupt-controller/mips-gic.h> > > + gpio@19d00000 { > > + compatible = "siflower,sf19a2890-gpio"; > > + reg = <0x19d00000 0x100000>; > > Just use 4 instances. Since (looking at the driver) it seems there > is an IRQ register that is "off the bulk" I would do something like: > > instance 0: > > reg = <0x19d00000 0x40>, <0x19d04000 4>; > > instance: 1: > > reg = <0x19d00040 0x40>, <0x19d04004 4>; > > (...etc...) Actually, this weird GPIO controller uses one 0x40 block per pin (The 49 pin controller uses 0x40 * 49 or about 3K memory space just for IO registers!), and they share a single reset signal from the reset controller. I don't know how one reset could be shared across multiple platform devices so I don't think this kind of split is possible. -- Regards, Chuanhong Guo
On Sun, Dec 29, 2024 at 4:06 AM Chuanhong Guo <gch981213@gmail.com> wrote: > I don't know how one reset could be shared across multiple platform > devices so I don't think this kind of split is possible. In general I don't think shared reset lines in silicon is any problem whatsoever, they are reference counted, just like clocks or regulators, and there is a flag RESET_CONTROL_FLAGS_BIT_SHARED especially for this purpose. You might have to fix the reset driver, or fix up the way they are defined in the device tree, I don't know how, but certainly it is possible. If in doubt consult the reset controller maintainer. (Philipp.) Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml b/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml new file mode 100644 index 000000000000..7dab1e3f159c --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/siflower,sf19a2890-gpio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Siflower SF19A2890 GPIO controller + +maintainers: + - Chuanhong Guo <gch981213@gmail.com> + +properties: + compatible: + const: siflower,sf19a2890-gpio + + gpio-controller: true + "#gpio-cells": + const: 2 + + gpio-ranges: true + + reg: + maxItems: 1 + + interrupts: + description: + Interrupt mapping, one interrupt per 16 GPIOs. + minItems: 1 + maxItems: 10 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + ngpios: + description: + The number of GPIOs available on the controller implementation. + minimum: 1 + +required: + - compatible + - clocks + - gpio-controller + - gpio-ranges + - interrupt-controller + - interrupts + - ngpios + - reg + - resets + - "#gpio-cells" + - "#interrupt-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/mips-gic.h> + gpio@19d00000 { + compatible = "siflower,sf19a2890-gpio"; + reg = <0x19d00000 0x100000>; + interrupts = <GIC_SHARED 246 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 247 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 248 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 249 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&gpioclk 0>; + resets = <&gpiorst 0>; + + gpio-controller; + #gpio-cells = <2>; + ngpios = <49>; + gpio-ranges = <&pinctrl 0 0 49>; + + interrupt-controller; + #interrupt-cells = <2>; + };
Add dt binding doc for the GPIO controller found on Siflower SF19A2890 and various other Siflower MIPS and RISC-V SoCs. Signed-off-by: Chuanhong Guo <gch981213@gmail.com> --- .../gpio/siflower,sf19a2890-gpio.yaml | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml