Hi Chuanhong,
thanks for your patch!
On Wed, Dec 25, 2024 at 4:59 AM Chuanhong Guo <gch981213@xxxxxxxxx> 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@xxxxxxxxx>
(...)
> + 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
