Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 31 Aug 2022 09:24:31 +0200
From:      Emmanuel Vadot <manu@bidouilliste.com>
To:        Ganbold Tsagaankhuu <ganbold@gmail.com>
Cc:        Peter Jeremy <peterj@freebsd.org>, Ganbold Tsagaankhuu <ganbold@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 901df07a4768 - main - Code refactoring for existing rk_gpio driver. It supports gpio type checking. Depending on gpio type some register addresses are different.
Message-ID:  <20220831092431.de8c7e3bd404feefde854533@bidouilliste.com>
In-Reply-To: <CAGtf9xOox5aPozG9qEw3d1APaE9LpbP=RZqLnd369MRk20BuDA@mail.gmail.com>
References:  <202208191322.27JDMv0b007432@gitrepo.freebsd.org> <YwGu8dSqAcVoMNt2@server.rulingia.com> <CAGtf9xOox5aPozG9qEw3d1APaE9LpbP=RZqLnd369MRk20BuDA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 31 Aug 2022 11:15:18 +0800
Ganbold Tsagaankhuu <ganbold@gmail.com> wrote:

> Peter,
>=20
> On Sun, Aug 21, 2022 at 12:05 PM Peter Jeremy <peterj@freebsd.org> wrote:
>=20
> > On 2022-Aug-19 13:22:57 +0000, Ganbold Tsagaankhuu <ganbold@FreeBSD.org>
> > wrote:
> > >The branch main has been updated by ganbold:
> > >
> > >URL:
> > https://cgit.FreeBSD.org/src/commit/?id=3D901df07a47684dca7b06f60d838a5=
6456d751a23
> > >
> > >commit 901df07a47684dca7b06f60d838a56456d751a23
> > >Author:     S=F8ren Schmidt <sos@FreeBSD.org>
> > >AuthorDate: 2022-08-19 13:22:01 +0000
> > >Commit:     Ganbold Tsagaankhuu <ganbold@FreeBSD.org>
> > >CommitDate: 2022-08-19 13:22:01 +0000
> > >
> > >    Code refactoring for existing rk_gpio driver.
> > >    It supports gpio type checking. Depending on gpio type some
> > >    register addresses are different.
> > >
> > >    Reviewed by:    manu
> > >    Differential Revision:  https://reviews.freebsd.org/D36262
> >
> > My Rock64 is now hanging on boot as follows:
> > rk_pinctrl0: <RockChip Pinctrl controller> on ofwbus0
> > gpio0: <RockChip GPIO Bank controller> mem 0xff210000-0xff2100ff irq 53=
 on
> > rk_pinctrl0
> > gpio0: Unknown gpio version 48000000
> > <<hang>>
> >
> > >@@ -170,6 +221,43 @@ rk_gpio_attach(device_t dev)
> > >               rk_gpio_detach(dev);
> > >               return (ENXIO);
> > >       }
> > >+      RK_GPIO_LOCK(sc);
> > >+      sc->version =3D rk_gpio_read_4(sc, RK_GPIO_VERSION);
> > >+      RK_GPIO_UNLOCK(sc);
> >
> > This call to rk_gpio_read_4() looks wrong:
> > a) rk_gpio_read_4() tests sc->version which this call is setting.
> > b) The second argument to rk_gpio_read_4() is a "enum gpio_regs", not
> >    an actual offset - RK_GPIO_VERSION (=3D0x78) is way outside the
> >    sc->regs array.
> > c) sc->regs is also uninitialised at this point.
> >
> > Maybe this should call RK_GPIO_READ() instead, but neither my RK3328
> > TRM (revision 1.2 from July 2017) nor my RK3399 TRM (revision 1.4 from
> > April 2017) document a GPIO register at offset 0x78 - both only go to
> > 0x60.  (If you have a later TRM for either chip, I would be interested
> > in a copy).
> >
>=20
> Does the following patch work for you?
>=20
> diff --git a/sys/arm64/rockchip/rk_gpio.c b/sys/arm64/rockchip/rk_gpio.c
> index c3b1044df2f7..d41a9077d0cc 100644
> --- a/sys/arm64/rockchip/rk_gpio.c
> +++ b/sys/arm64/rockchip/rk_gpio.c
> @@ -259,12 +259,13 @@ static int
>  rk_gpio_attach(device_t dev)
>  {
>         struct rk_gpio_softc *sc;
> -       phandle_t node;
> +       phandle_t parent_node, node;
>         int err, i;
>=20
>         sc =3D device_get_softc(dev);
>         sc->sc_dev =3D dev;
>         sc->pinctrl =3D device_get_parent(dev);
> +       parent_node =3D ofw_bus_get_node(sc->pinctrl);
>=20
>         node =3D ofw_bus_get_node(sc->sc_dev);
>         if (!OF_hasprop(node, "gpio-controller"))
> @@ -303,9 +304,11 @@ rk_gpio_attach(device_t dev)
>                 return (ENXIO);
>         }
>=20
> -       RK_GPIO_LOCK(sc);
> -       sc->version =3D rk_gpio_read_4(sc, RK_GPIO_VERSION);
> -       RK_GPIO_UNLOCK(sc);
> +       /* RK3568 has GPIO_VER_ID register */
> +       if (ofw_bus_node_is_compatible(parent_node,
> "rockchip,rk3568-pinctrl"))
> +               sc->version =3D RK_GPIO_TYPE_V2;
> +       else
> +               sc->version =3D RK_GPIO_TYPE_V1;
>=20
>         switch (sc->version) {
>         case RK_GPIO_TYPE_V1:
>=20
>=20
> Ganbold

 It does for me, thanks.

--=20
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20220831092431.de8c7e3bd404feefde854533>