On 10/02/2017 02:55 AM, Linus Walleij wrote:
> On Thu, Sep 28, 2017 at 4:22 PM, Grygorii Strashko
> <grygorii.stras...@ti.com> wrote:
>
>> Sry, but I do not agree with this series.
>> - no prof that it can be re-used by other drivers than tegra
>> (at least I do not see reasons to re-use it for any TI drivers)
>
> This is not necessarily a blocker if it can be shown that others than
> TI/OMAP can reuse it.
sure. My point is - this is big change in gpiolib, which is > 1000 lines,
but current re-usability just 2 drivers (I'm comparing with your work when
gpio irq infra was introduced - you did it bottom-up, by refactoring
existing drivers and moving common code in gpiolib, so re usability is great).
>
> I've looked at things like the imagination pistachio:
>
> pinctrl@18101C00 {
> compatible = "img,pistachio-system-pinctrl";
> reg = <0x18101C00 0x400>;
>
> gpio0: gpio0 {
> interrupts = <GIC_SHARED 71 IRQ_TYPE_LEVEL_HIGH>;
>
> gpio-controller;
> #gpio-cells = <2>;
>
> interrupt-controller;
> #interrupt-cells = <2>;
> };
>
> ...
>
> gpio5: gpio5 {
> interrupts = <GIC_SHARED 76 IRQ_TYPE_LEVEL_HIGH>;
>
> gpio-controller;
> #gpio-cells = <2>;
>
> interrupt-controller;
> #interrupt-cells = <2>;
> };
>
> This looks like a clear candidate.
> CC: to Andrew Bresticker, can you look into this?
>
[...]
gpio: gpio@226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
#gpio-cells = <2>;
reg = <0x226000 0x1000>;
interrupts = <42 IRQ_TYPE_EDGE_BOTH
43 IRQ_TYPE_EDGE_BOTH 44 IRQ_TYPE_EDGE_BOTH
45 IRQ_TYPE_EDGE_BOTH 46 IRQ_TYPE_EDGE_BOTH
47 IRQ_TYPE_EDGE_BOTH 48 IRQ_TYPE_EDGE_BOTH
49 IRQ_TYPE_EDGE_BOTH 50 IRQ_TYPE_EDGE_BOTH>;
ti,ngpio = <144>;
ti,davinci-gpio-unbanked = <0>;
status = "disabled";
interrupt-controller;
#interrupt-cells = <2>;
};
FYI. Above is gpio-dvinci example which defines the same, but without coding
gpio banks in DT (note 2 IRQ lines per bank, bank 32 pins).
>
> CC to Shawn Guo to look into this.
>
> So in short I think there can be others that can make good use of this
> infrastructure.
>
>> - all GPIO IRQs mapped statically
>
> This really needs to be fixed.
>
>> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins
>> which is waste of memory
>> - DT binding changes not documented and no DT examples
>> - below is ugly ;)
>> + bank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;
>> + pin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;
>
> These should be fixable quite easily I think. Thierry?
What I'm trying to understand is how GPIO client bindings will look like?
Now it is: gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; (pistachio_marduk.dts)
But as per of_gpio_banked_xlate() it expected to be
gpios = <&gpio [Linear gpio num] GPIO_ACTIVE_LOW>;
Wouldn't this break DT compatibility and prevent re-using of this feature
for pistachio, for example? (or i'm missing smth).
--
regards,
-grygorii
