Message ID | 1506480343-9597-2-git-send-email-yamada.masahiro@socionext.com |
---|---|
State | New |
Headers | show |
Series | gpio: uniphier: UniPhier GPIO driver | expand |
On Wed, Sep 27, 2017 at 4:45 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > This GPIO controller is used on UniPhier SoC family. > > The vendor specific property "socionext,interrupt-ranges" is for > specifying interrupt mapping to the parent interrupt controller > because the mapping is not contiguous. It works like "ranges", > but transforms "interrupts" instead of "reg". > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Acked-by: Rob Herring <robh@kernel.org> I don't think Rob has seen the new interrupt range thing? (It's not a big deal. Things like these are a bit fuzzy.) > + socionext,interrupt-ranges = <0 48 16>, <16 154 5>, <21 217 3>; If it is as you say, that other SoCs are doing the same, we should think about creating a generic property for this. Like hierarchy-interrupt-ranges or so. I kind of liked the old patch where it was just "interrupts" and then you looked it up from the irq subsystem. (tglx even ACKed the patch). But I want the DT people to say something here. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, 2017-09-27 22:42 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>: > On Wed, Sep 27, 2017 at 4:45 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > >> This GPIO controller is used on UniPhier SoC family. >> >> The vendor specific property "socionext,interrupt-ranges" is for >> specifying interrupt mapping to the parent interrupt controller >> because the mapping is not contiguous. It works like "ranges", >> but transforms "interrupts" instead of "reg". >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> Acked-by: Rob Herring <robh@kernel.org> > > I don't think Rob has seen the new interrupt range thing? > (It's not a big deal. Things like these are a bit fuzzy.) This judge is difficult. Rob gave Ack to v3. I added one more property to v4, so I removed Rob's Ack because it contained what Rob had not seen. http://patchwork.ozlabs.org/patch/810981/ But, he was annoyed by my carefulness. This time, I am keeping his Ack, but it would be appreciated if he cares to review this once again. >> + socionext,interrupt-ranges = <0 48 16>, <16 154 5>, <21 217 3>; > > If it is as you say, that other SoCs are doing the same, we should > think about creating a generic property for this. Like > hierarchy-interrupt-ranges or so. Of course, I thought of this. When we add a generic property, we need very careful review. If we want something generic, phandle of parent irqchip is necessary. interrupt-ranges = <parent-phandle parent-irq-base child-irq-base len>, <parent-phandle2 parent-irq-base2 child-irq-base2 len2>, ... The format is very similar to gpio-ranges. The phandle is necessary because it may have two or more interrupt parents. This is why we introduced "interrupts" property at first, but later we ended up with adding "interrupts-extended". Unfortunately, this does not fit with the current design of irqdomain because current irqdomain can only one parent. In my opinion, the hierarchy irq domain was badly designed. This is why it is so painful to write drivers. > I kind of liked the old patch where it was just "interrupts" and > then you looked it up from the irq subsystem. (tglx even ACKed > the patch). > > But I want the DT people to say something here. I think "interrupts" is for interrupt consumers. We need something different for inter-irqchip connection.
Hi Linus, 2017-09-27 22:42 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>: > On Wed, Sep 27, 2017 at 4:45 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > >> This GPIO controller is used on UniPhier SoC family. >> >> The vendor specific property "socionext,interrupt-ranges" is for >> specifying interrupt mapping to the parent interrupt controller >> because the mapping is not contiguous. It works like "ranges", >> but transforms "interrupts" instead of "reg". >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> Acked-by: Rob Herring <robh@kernel.org> > > I don't think Rob has seen the new interrupt range thing? > (It's not a big deal. Things like these are a bit fuzzy.) > >> + socionext,interrupt-ranges = <0 48 16>, <16 154 5>, <21 217 3>; > > If it is as you say, that other SoCs are doing the same, we should > think about creating a generic property for this. Like > hierarchy-interrupt-ranges or so. > > I kind of liked the old patch where it was just "interrupts" and > then you looked it up from the irq subsystem. (tglx even ACKed > the patch). > > But I want the DT people to say something here. > > Yours, > Linus Walleij I explained the technical background of this approach, but I have not had any comment. (DT people are silent, too) Is this driver applicable?
diff --git a/Documentation/devicetree/bindings/gpio/gpio-uniphier.txt b/Documentation/devicetree/bindings/gpio/gpio-uniphier.txt new file mode 100644 index 0000000..6d9251a --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-uniphier.txt @@ -0,0 +1,40 @@ +UniPhier GPIO controller + +Required properties: +- compatible: Should be "socionext,uniphier-gpio". +- reg: Specifies offset and length of the register set for the device. +- gpio-controller: Marks the device node as a GPIO controller. +- #gpio-cells: Should be 2. The first cell is the pin number and the second + cell is used to specify optional parameters. +- interrupt-parent: Specifies the parent interrupt controller. +- interrupt-controller: Marks the device node as an interrupt controller. +- #interrupt-cells: Should be 2. The first cell defines the interrupt number. + The second cell bits[3:0] is used to specify trigger type as follows: + 1 = low-to-high edge triggered + 2 = high-to-low edge triggered + 4 = active high level-sensitive + 8 = active low level-sensitive + Valid combinations are 1, 2, 3, 4, 8. +- ngpios: Specifies the number of GPIO lines. +- gpio-ranges: Mapping to pin controller pins (as described in gpio.txt) +- socionext,interrupt-ranges: Specifies an interrupt number mapping between + this GPIO controller and its interrupt parent, in the form of arbitrary + number of <child-interrupt-base parent-interrupt-base length> triplets. + +Optional properties: +- gpio-ranges-group-names: Used for named gpio ranges (as described in gpio.txt) + +Example: + gpio: gpio@55000000 { + compatible = "socionext,uniphier-gpio"; + reg = <0x55000000 0x200>; + interrupt-parent = <&aidet>; + interrupt-controller; + #interrupt-cells = <2>; + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 0 0>; + gpio-ranges-group-names = "gpio_range"; + ngpios = <248>; + socionext,interrupt-ranges = <0 48 16>, <16 154 5>, <21 217 3>; + };