Date: Sun, 21 Aug 2022 09:39:52 +0200 From: Emmanuel Vadot <manu@bidouilliste.com> To: Peter Jeremy <peterj@freebsd.org> Cc: 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: <20220821093952.08d2a29ea9c2c47aae1e0ea5@bidouilliste.com> In-Reply-To: <YwGu8dSqAcVoMNt2@server.rulingia.com> References: <202208191322.27JDMv0b007432@gitrepo.freebsd.org> <YwGu8dSqAcVoMNt2@server.rulingia.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 21 Aug 2022 14:05:05 +1000 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=3D901df07a47684dca7b06f60d8= 38a56456d751a23 > > > >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. > > =20 > > Reviewed by: manu > > Differential Revision: https://reviews.freebsd.org/D36262 >=20 > 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 o= n rk_pinctrl0 > gpio0: Unknown gpio version 48000000 > <<hang>> >=20 > >@@ -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); >=20 > 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. Yes indeed, sorry to not have spotted that during review. > 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). We should match the version based on the compat string of the parent. In a perfect world the RK356x gpio controller shouldn't have the same compatible as the other ones as the register map isn't the same but ... --=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?20220821093952.08d2a29ea9c2c47aae1e0ea5>