Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 13 Jun 2021 11:53:11 +0200
From:   Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
        OpenBMC Maillist <openbmc@lists.ozlabs.org>,
        Tomer Maimon <tmaimon77@gmail.com>,
        Joel Stanley <joel@jms.id.au>,
        linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/8] dt-bindings: pinctrl: Add Nuvoton WPCM450
Message-ID: <YMXVh9FiW9SOWUCV@latitude>
References: <20210602120329.2444672-1-j.neuschaefer@gmx.net>
 <20210602120329.2444672-5-j.neuschaefer@gmx.net>
 <CACRpkdY6c750Dnh2H_6uEhOHJv-kLd_OpHqnuDKu4DQezZ2C_w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="eeAjEw1TF8Acq/Ni"
Content-Disposition: inline
In-Reply-To: <CACRpkdY6c750Dnh2H_6uEhOHJv-kLd_OpHqnuDKu4DQezZ2C_w@mail.gmail.com>
Precedence: bulk


--eeAjEw1TF8Acq/Ni
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 04, 2021 at 11:35:48AM +0200, Linus Walleij wrote:
> Hi Jonathan!
>=20
> thanks for your patch!

Hi again!

> On Wed, Jun 2, 2021 at 2:04 PM Jonathan Neusch=C3=A4fer
> <j.neuschaefer@gmx.net> wrote:
>=20
> > +  interrupts: true
>=20
> maxitems 4 right?

Yes.

> Make an enum:
>=20
> interrupts:
>   - description: what IRQ0 is for
>   - description: what IRQ1 is for
>   - description: what IRQ2 is for
>   - description: what IRQ3 is for
>=20
> And describe how these interrupts are used.

Good point.

- IRQ0 is for events (interrupts) from GPIOs  0 to  3.
- IRQ1 is for events (interrupts) from GPIOs  4 to 11.
- IRQ2 is for events (interrupts) from GPIOs 12 to 15.
- IRQ3 is for events (interrupts) from GPIOs 24 to 25.

> Because I am suspicious that they actually correspond to 4 different
> GPIO blocks, which should then be their own nodes.

Unfortunately, It's not that simple. The GPIO ports (as defined by the
groups of registers that do GPIO direction/input/output) are organised
like this:

- GPIO port 0 starts at GPIO   0 and is 16 GPIOs long.
- GPIO port 1 starts at GPIO  16 and is 16 GPIOs long.
- GPIO port 2 starts at GPIO  32 and is 16 GPIOs long.
- GPIO port 3 starts at GPIO  48 and is 16 GPIOs long.
- GPIO port 4 starts at GPIO  64 and is 16 GPIOs long.
- GPIO port 5 starts at GPIO  80 and is 16 GPIOs long.
- GPIO port 6 starts at GPIO  96 and is 18 GPIOs long.
- GPIO port 7 starts at GPIO 114 and is 14 GPIOs long.

(They didn't even make it so that each one has 16 GPIOs...)

As you can see, only a few GPIOs are connected to interrupt logic; most
of them are in port 0, and the remaining two are in port 1.

Forthermore, the GPIO ports don't all have the same set of registers, so
that the register layout of each can't be predicted by the offset of the
first register.

>=20
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    pinctrl: pinctrl@b8003000 {
> > +      compatible =3D "nuvoton,wpcm450-pinctrl";
> > +      reg =3D <0xb8003000 0x1000>;
> > +      gpio-controller;
> > +      #gpio-cells =3D <2>;
> > +      interrupts =3D <2 IRQ_TYPE_LEVEL_HIGH
> > +                    3 IRQ_TYPE_LEVEL_HIGH
> > +                    4 IRQ_TYPE_LEVEL_HIGH
> > +                    5 IRQ_TYPE_LEVEL_HIGH>;
>=20
> So these.
>=20
> > +      rmii2 {
> > +        groups =3D "rmii2";
> > +        function =3D "rmii2";
> > +      };
> > +
> > +      pinctrl_uid: uid {
> > +        pins =3D "gpio14";
> > +        input-debounce =3D <1>;
> > +      };
>=20
> I challenge you here and encourage you to put a node for each
> GPIO "port":
>=20
>   port0: gpio@0 {
>  ....
>   };
>   port1: gpio@1 {
>  ....
>   };

Hmm, well, if the unit addresses simply go from 0 to 7, rather than
encoding offsets, this could work. But it won't help much with the IRQ
problem.


Thanks,
Jonathan Neusch=C3=A4fer

--eeAjEw1TF8Acq/Ni
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAmDF1WYACgkQCDBEmo7z
X9vvgg/+NjP0ej1MKT9aUcOcF5Vp77+zynVdqbXduGemb16BNYTXsULObigZyQqe
5LJshwRpK5jRZNRIdWDscndY+rzg0vwjlcG0lF75a4Ol/bWs/uI/d7sIaZPO13s7
zfCWLw240N1NkNFDcK449Z1OrQ7nAkn6BEeJrZ1TTBHV6GyTGqD7WZGMRCA9Kcag
xFwjRNWEhxeuxaQklbm1PADy8qt4WBk7z4yt+SGZmJYZyHloAFJOsHVIiOlxSYQH
DWoFc9ydsK6jK+Rxdo8Xsgt7G2Ei2u/tBly1og5iR9gWh2abA3pO2xLIm9QDud9+
hIwPjzH/XAUbqVFy6fGmOM+qif1Uvs2rAcybx5ewRQRFhA7mQeOd3Wfv+DW05QXG
OKHBR0fGeqqE4gHBTscy5eftpmToIczYwFP+oZx8uIksReJ3IQv3SS5s9JjHFoe/
Yy06P8rm06jqvt+cZeSEg55yLe95IZvVMZxbbGNhRXF80afMExfVLBx6YJaSI8cz
7aHmTlDTG1HyLdCTLrDBDB8pP+fke/FN+uBiJucEg+jzz64qQheW30DyKjlSMP+W
QCx3k9eTBl5AzPqvpEfQwMcyZpMdMFpwg3Dm/kulKU/zIwI4nu1Ef1+5aTz8T+Ze
oRnm+9VQmzSV0W2HC24PxJrtbmpTbID1XRN/95gZfl0K17OFuZQ=
=negl
-----END PGP SIGNATURE-----

--eeAjEw1TF8Acq/Ni--