On Wed, May 30, 2018 at 09:34:39AM +1000, NeilBrown wrote:
> On Tue, May 29 2018, Sergio Paracuellos wrote:
>
> > Most gpio chips have two cells for interrupts and this should be also.
> > Set this property accordly fixing this up.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> > ---
> > drivers/staging/mt7621-dts/mt7621.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> > index d7e4981..bce6029 100644
> > --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> > +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> > @@ -70,7 +70,7 @@
> > interrupt-parent = <&gic>;
> > interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
> > interrupt-controller;
> > - #interrupt-cells = <1>;
> > + #interrupt-cells = <2>;
>
> Thanks for this ongoing effort.
>
> I thought I should test this change. It didn't quite go as I expected.
> My board has one GPIO wired to a push-button so it is normally
> configured with
>
> gpio-keys {
> compatible = "gpio-keys";
>
> reset {
> label = "reset";
> gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
> ...
>
> (though in upstream it still uses the old gpio-keys-polled).
> I removed the gpios line and replaced with
>
> interrupt-parent = <&gpio>;
> interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>
> which should produce a key-press event whenever the button is pressed.
> It didn't.
>
> The reason is
>
> .xlate = irq_domain_xlate_onecell,
>
> in irq_domain_ops in gpio-mt7621.c.
> "onecell" obviously correlated with #interrupt-cells = <1>.
> I changed it to
> .xlate = irq_domain_xlate_twocell,
>
> and now it works as expected. So we need to combine that change with
> the change to #interrupt-cells. I'm certain that we do really want 2
> cells here, as it is possible to change the trigger type.
>
> You might have noticed that I added
> interrupt-parent = <&gpio>;
>
> even though there is no 'gpio:' tag in the devicetree. I had to add
> one.
>
> - gpio@600 {
> + gpio: gpio@600 {
>
> so that I could refer to the gpio interrupts.
> This feels a bit untidy. The gpios are grouped into banks of 32:
> gpio0 gpio1 grio2
> but the interrupts are just a single bank of 96 interrupts.
> I don't know that this is a problem and I'm not advocating that we "fix"
> it. But it might be something that will be queried when we
> submit to linux-gpio - I really don't know.
>
> So if you could redo this patch to added the gpio: label and change
> the xlate function, that would be excellent.
> For all the rest:
> Reviewed-by: NeilBrown <neil@xxxxxxxxxx>
>
> Thanks a lot,
> NeilBrown
Thanks for your review and clear explanation, Neil. That really helps.
I have just send v2 version for this patch with the changes you are
pointing out here.
Hope this helps.
Best regards,
Sergio Paracuellos
>
> >
> > gpio0: bank@0 {
> > reg = <0>;
> > --
> > 2.7.4
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
