From nobody Wed Aug 31 02:11:47 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 4MHSNd09LZz4ZgXy; Wed, 31 Aug 2022 02:12:01 +0000 (UTC) (envelope-from ganbold@gmail.com) Received: from mail-il1-x133.google.com (mail-il1-x133.google.com [IPv6:2607:f8b0:4864:20::133]) (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 4MHSNc125bz3Mg9; Wed, 31 Aug 2022 02:12:00 +0000 (UTC) (envelope-from ganbold@gmail.com) Received: by mail-il1-x133.google.com with SMTP id t11so4060135ilf.3; Tue, 30 Aug 2022 19:12:00 -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=2xbh9qcb1AwD3K2ydKw7qEdIRUwNEG0KDme2Xcg09B4=; b=D1phrah5ga5GTqmIQD3oivwh+hlA2YYpJsH6IACZJnTm0t1ihGxvjpr+AXrZlK4dGF QBBkwuVcS6OgR454nSz0dBCzyQ2lZRO47EFtIvWVpMB5ol3JErlbe0JE5+69ssCe9wbU CXDcsZ1WgqGDscRU0sm4+ws8oQ59AmMV6jnNqZ+TaAIlCkVpxHTghyx01TJny+t0YQxW MdeetP0vH5SDmhsML6cGryyH6fojyBXCwZKiJC9hSkV1VLaNfN4bWX3mowqwl+b1fCOK qTUenQAk/fN0w3bHvPv1mHUF4QFAVzsS5b99X+DoXSbUvGNuc2U5kR1811AdfP1+xQ7p MC2w== 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=2xbh9qcb1AwD3K2ydKw7qEdIRUwNEG0KDme2Xcg09B4=; b=Pf1ioFzpdYbpZd0+baukUMtChh/rXK59dJfSO0RUfP5Knh11EXu+372lhPpwRDljtM 8/wCLATd4+fh9pVZmtn9N/1Gp45BaTextfgqcc9Ps0ZDrbFW6R+ia8yhnbW1SY8+uxJt Q49kkQ4DfptILqBG7NiJzwAblKBrO3VDrGTAIE1790WFs2fQuApivLcvkdkce/kdFXns Sfi98lvdivatoEwzxCVAdi5q6nD8YanoVd+NIoVLXJat5pHxnV2KTiPrwxOXqf7FQIpx S2nUTJH0bQnDLHZaWm6O347YGcPC/J/5jiL8wUjBLBrwiXrKx+8Oxq+elW0dG264CVtp aQuA== X-Gm-Message-State: ACgBeo1W2HkZZ5WGPlbxKzaCZLNl1zwwckP058U3ZJay4e+IDD5uDJ+6 HgogBJVSpgXm/Qwgc0oEodTJ2Cn2wwZLlFsJUAlqgJb8VM8= X-Google-Smtp-Source: AA6agR4LR06Gyol8E48SiIgwlNQZd80ZH1gnlspFshUfwsfkseYALoMVWqYTtfuI+p7zEGZuc43p24nvAe8amPdYrCI= X-Received: by 2002:a05:6e02:1d95:b0:2eb:39fe:fbc3 with SMTP id h21-20020a056e021d9500b002eb39fefbc3mr4159819ila.197.1661911919083; Tue, 30 Aug 2022 19:11:59 -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> <20220821093952.08d2a29ea9c2c47aae1e0ea5@bidouilliste.com> In-Reply-To: <20220821093952.08d2a29ea9c2c47aae1e0ea5@bidouilliste.com> From: Ganbold Tsagaankhuu Date: Wed, 31 Aug 2022 10:11:47 +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: Emmanuel Vadot Cc: Peter Jeremy , Ganbold Tsagaankhuu , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="00000000000095c53005e78005ad" X-Rspamd-Queue-Id: 4MHSNc125bz3Mg9 X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b=D1phrah5; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of ganbold@gmail.com designates 2607:f8b0:4864:20::133 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.989]; 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::133: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:+]; TO_MATCH_ENVRCPT_SOME(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)[]; RCVD_TLS_LAST(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; RCPT_COUNT_FIVE(0.00)[6]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-ThisMailContainsUnwantedMimeParts: N --00000000000095c53005e78005ad Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Aug 21, 2022 at 3:40 PM Emmanuel Vadot wrote: > On Sun, 21 Aug 2022 14:05:05 +1000 > 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 > on 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. > > 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). > Sorry for the break. I double checked those TRMs for RK3328/RK3399 and there seems no GPIO_VER_ID register as RK3568 does. > > 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 ... > As manu@ said, compat string match is maybe better. I will look into it. Ganbold > > -- > Emmanuel Vadot > --00000000000095c53005e78005ad Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Aug 21, 2022 at 3:40 PM Emman= uel Vadot <manu@bidouilliste.co= m> wrote:
On Sun, 21 Aug 2022 14:05:05 +1000
Peter Jeremy <pe= terj@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: http= s://cgit.FreeBSD.org/src/commit/?id=3D901df07a47684dca7b06f60d838a56456d751= a23
> >
> >commit 901df07a47684dca7b06f60d838a56456d751a23
> >Author:=C2=A0 =C2=A0 =C2=A0S=C3=B8ren Schmidt <sos@FreeBSD.org&= gt;
> >AuthorDate: 2022-08-19 13:22:01 +0000
> >Commit:=C2=A0 =C2=A0 =C2=A0Ganbold Tsagaankhuu <ganbold@FreeBSD= .org>
> >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 ty= pe some
> >=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://review= s.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)
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rk_gpio_detach(dev= );
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (ENXIO); > >=C2=A0 =C2=A0 =C2=A0}=C2=A0
> >+=C2=A0 =C2=A0 RK_GPIO_LOCK(sc);
> >+=C2=A0 =C2=A0 sc->version =3D rk_gpio_read_4(sc, RK_GPIO_VERSI= ON);
> >+=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&q= uot;, not
>=C2=A0 =C2=A0 an actual offset - RK_GPIO_VERSION (=3D0x78) is way outsi= de the
>=C2=A0 =C2=A0 sc->regs array.
> c) sc->regs is also uninitialised at this point.

=C2=A0Yes 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<= br> > 0x60.=C2=A0 (If you have a later TRM for either chip, I would be inter= ested
> in a copy).

Sorry for the break.
I double checked those TRMs for RK3328/RK3399 and there seems no
GPIO_VER_ID register as RK3568 does.
=C2=A0

=C2=A0We should match the version based on the compat string of the parent.=
=C2=A0In 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 ...=

As manu@ said, compat string match is = maybe better.
I will look into it.=C2=A0

Ganbold
=C2=A0
=C2=A0

--
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
--00000000000095c53005e78005ad--