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.

Peter,

On Sun, Aug 21, 2022 at 12:05 PM Peter Jeremy <peterj@freebsd.org> wrote:

> 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=901df07a47684dca7b06f60d838a56456d751a23
> >
> >commit 901df07a47684dca7b06f60d838a56456d751a23
> >Author:     Søren 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 = 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 (=0x78) 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).
>

Does the following patch work for you?

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;

        sc = device_get_softc(dev);
        sc->sc_dev = dev;
        sc->pinctrl = device_get_parent(dev);
+       parent_node = ofw_bus_get_node(sc->pinctrl);

        node = 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);
        }

-       RK_GPIO_LOCK(sc);
-       sc->version = 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 = RK_GPIO_TYPE_V2;
+       else
+               sc->version = RK_GPIO_TYPE_V1;

        switch (sc->version) {
        case RK_GPIO_TYPE_V1:


Ganbold



>
> --
> Peter Jeremy
>