From nobody Wed Aug 31 03:15:18 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4MHTnx50rMz4Zqx7; Wed, 31 Aug 2022 03:15:33 +0000 (UTC) (envelope-from ganbold@gmail.com) Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4MHTnx0l8Bz3Qbb; Wed, 31 Aug 2022 03:15:33 +0000 (UTC) (envelope-from ganbold@gmail.com) Received: by mail-io1-xd36.google.com with SMTP id p187so10909788iod.8; Tue, 30 Aug 2022 20:15:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=6ZssEmbKaSlfsGJhMmtP3+j3tMcVT2WZwbJ42npmsd8=; b=EYuKRNIxsEAdBQG4ki+TMXxm5FuVi4+B7QXDEzf5cMk2Ba3vcZXUIgoh3dCHTiGtOY nA0jL+T3+sPSKEXpifANsJw57mE0rwvfcCJbH14pgONMA0sApUlM6HTy1jwZB9bvOZ8j swqErt78wL2LtJ/sslrgTZHLarUG9sWfYw8LSLnOUYu+Ib53Mu1Xf1YpNBPXeeJi4Jex ppWBtxUvGkL7RmJr89bavi8yUCGgCVftTTW5nG+HgP7wWzmT8xAl6ywuGCd4ThZiNUL9 Sz0+H1CVHOEpfZTyivZadRnSWksdfWGUreI/wm9rUK8gqB6ZwR2NVQ3NY5sIyTy3XthF exjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=6ZssEmbKaSlfsGJhMmtP3+j3tMcVT2WZwbJ42npmsd8=; b=Kdb1EC1WEa8tCuAf5CBERLdX71BovRZnnL0NdkJAZ3vJRVQdRwC0+El+YpUV2wdPvc q8zU/ZaiSAdIkRjnKV9skXQLfTv9D0PncK0YlEU9q/qt5nI4jruWWZaL+OWJel27S7Jm VJAbDYaI0DMloAADsyHd/XL0gX4in67h7gAvAPeOEsm+45r9EuPkkTt5JXEWCZQMluE8 SuqhkY0PcsbcSjhhF3+XYxGhyHmaDZzhxxeQWArt+E0JImspVcGYSTsAnQQqioK01O9I wRAOLGI7vCJ+jR7ncWHk3uhT3qrFcl33Vuo/1qWHaHEvzzJT6Lz1G/kvJc9h1KwIZuT6 bPrA== X-Gm-Message-State: ACgBeo0+usAjtmPhf5J9SXRyHMDoe5TXAnzvmjOoBisblBkwDNmDWf3K 1RjpegrXs5OY+KRbwXD5DeNR0E7KOm5tvZ+dK1c/OyVvGTc= X-Google-Smtp-Source: AA6agR7FaJRktDhA8+/6+r9rd0lhSzMjgngD9qJD1GlCe2WdBX/cjMduQaLb3ML/IfcPPyIiMklqgQMw+91Ah/IlsOU= X-Received: by 2002:a05:6638:537:b0:349:b5d2:9182 with SMTP id j23-20020a056638053700b00349b5d29182mr13438014jar.5.1661915730587; Tue, 30 Aug 2022 20:15:30 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 References: <202208191322.27JDMv0b007432@gitrepo.freebsd.org> In-Reply-To: From: Ganbold Tsagaankhuu Date: Wed, 31 Aug 2022 11:15:18 +0800 Message-ID: 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. To: Peter Jeremy Cc: Ganbold Tsagaankhuu , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000c4b41005e780e836" X-Rspamd-Queue-Id: 4MHTnx0l8Bz3Qbb X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b=EYuKRNIx; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of ganbold@gmail.com designates 2607:f8b0:4864:20::d36 as permitted sender) smtp.mailfrom=ganbold@gmail.com X-Spamd-Result: default: False [-3.97 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-0.99)[-0.991]; NEURAL_HAM_SHORT(-0.98)[-0.980]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36:c]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20210112]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; ARC_NA(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::d36:from]; MLMMJ_DEST(0.00)[dev-commits-src-all@freebsd.org,dev-commits-src-main@freebsd.org]; FROM_EQ_ENVFROM(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; DKIM_TRACE(0.00)[gmail.com:+]; RCVD_TLS_LAST(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; FROM_HAS_DN(0.00)[]; FREEFALL_USER(0.00)[ganbold]; MID_RHS_MATCH_FROMTLD(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; RCPT_COUNT_FIVE(0.00)[5]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-ThisMailContainsUnwantedMimeParts: N --000000000000c4b41005e780e836 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Peter, On Sun, Aug 21, 2022 at 12:05 PM Peter Jeremy wrote: > On 2022-Aug-19 13:22:57 +0000, Ganbold Tsagaankhuu > wrote: > >The branch main has been updated by ganbold: > > > >URL: > https://cgit.FreeBSD.org/src/commit/?id=3D901df07a47684dca7b06f60d838a564= 56d751a23 > > > >commit 901df07a47684dca7b06f60d838a56456d751a23 > >Author: S=C3=B8ren Schmidt > >AuthorDate: 2022-08-19 13:22:01 +0000 > >Commit: Ganbold Tsagaankhuu > >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: on ofwbus0 > gpio0: mem 0xff210000-0xff2100ff irq 53 o= n > rk_pinctrl0 > gpio0: Unknown gpio version 48000000 > <> > > >@@ -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). > 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 =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); 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); } - 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; switch (sc->version) { case RK_GPIO_TYPE_V1: Ganbold > > -- > Peter Jeremy > --000000000000c4b41005e780e836 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Peter,

On Sun, Aug 21, 2022 at 12:05 PM Pe= ter Jeremy <peterj@freebsd.org= > wrote:
On 2= 022-Aug-19 13:22:57 +0000, Ganbold Tsagaankhuu <ganbold@FreeBSD.org> = wrote:
>The branch main has been updated by ganbold:
>
>URL: https://c= git.FreeBSD.org/src/commit/?id=3D901df07a47684dca7b06f60d838a56456d751a23
>
>commit 901df07a47684dca7b06f60d838a56456d751a23
>Author:=C2=A0 =C2=A0 =C2=A0S=C3=B8ren Schmidt <sos@FreeBSD.org> >AuthorDate: 2022-08-19 13:22:01 +0000
>Commit:=C2=A0 =C2=A0 =C2=A0Ganbold Tsagaankhuu <ganbold@FreeBSD.org&= gt;
>CommitDate: 2022-08-19 13:22:01 +0000
>
>=C2=A0 =C2=A0 Code refactoring for existing rk_gpio driver.
>=C2=A0 =C2=A0 It supports gpio type checking. Depending on gpio type so= me
>=C2=A0 =C2=A0 register addresses are different.
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 Reviewed by:=C2=A0 =C2=A0 manu
>=C2=A0 =C2=A0 Differential Revision:=C2=A0 https://reviews.fre= ebsd.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)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rk_gpio_detach(d= ev);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (ENXIO);<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>+=C2=A0 =C2=A0 =C2=A0 RK_GPIO_LOCK(sc);
>+=C2=A0 =C2=A0 =C2=A0 sc->version =3D rk_gpio_read_4(sc, RK_GPIO_VER= SION);
>+=C2=A0 =C2=A0 =C2=A0 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
=C2=A0 =C2=A0an actual offset - RK_GPIO_VERSION (=3D0x78) is way outside th= e
=C2=A0 =C2=A0sc->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.=C2=A0 (If you have a later TRM for either chip, I would be interested=
in a copy).

Does the following patch wo= rk for you?

diff --git a/sys/arm64/rockchip/rk_gpi= o.c b/sys/arm64/rockchip/rk_gpio.c
index c3b1044df2f7..d41a9077d0cc 1006= 44
--- a/sys/arm64/rockchip/rk_gpio.c
+++ b/sys/arm64/rockchip/rk_gpi= o.c
@@ -259,12 +259,13 @@ static int
=C2=A0rk_gpio_attach(device_t de= v)
=C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct rk_gpio_softc *sc;
-= =C2=A0 =C2=A0 =C2=A0 phandle_t node;
+ =C2=A0 =C2=A0 =C2=A0 phandle_t p= arent_node, node;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 int err, i;

=C2=A0 = =C2=A0 =C2=A0 =C2=A0 sc =3D device_get_softc(dev);
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 sc->sc_dev =3D dev;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 sc->pinctrl= =3D device_get_parent(dev);
+ =C2=A0 =C2=A0 =C2=A0 parent_node =3D ofw_= bus_get_node(sc->pinctrl);

=C2=A0 =C2=A0 =C2=A0 =C2=A0 node =3D o= fw_bus_get_node(sc->sc_dev);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!OF_hasp= rop(node, "gpio-controller"))
@@ -303,9 +304,11 @@ rk_gpio_att= ach(device_t dev)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 return (ENXIO);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

- =C2=A0 =C2=A0= =C2=A0 RK_GPIO_LOCK(sc);
- =C2=A0 =C2=A0 =C2=A0 sc->version =3D rk_g= pio_read_4(sc, RK_GPIO_VERSION);
- =C2=A0 =C2=A0 =C2=A0 RK_GPIO_UNLOCK(s= c);
+ =C2=A0 =C2=A0 =C2=A0 /* RK3568 has GPIO_VER_ID register */
+ = =C2=A0 =C2=A0 =C2=A0 if (ofw_bus_node_is_compatible(parent_node, "rock= chip,rk3568-pinctrl"))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 sc->version =3D RK_GPIO_TYPE_V2;
+ =C2=A0 =C2=A0 =C2=A0 else+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sc->version =3D RK_= GPIO_TYPE_V1;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 switch (sc->version) {<= br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 case RK_GPIO_TYPE_V1:


Ganbold

=C2=A0

--
Peter Jeremy
--000000000000c4b41005e780e836--