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@gmail.com>
> ---
>  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@brown.name>

Thanks a lot,
NeilBrown

>  
>  			gpio0: bank@0 {
>  				reg = <0>;
> -- 
> 2.7.4