Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dfa6507c68bae9c62520cf100172e1a79bc201c1.camel@icenowy.me>
Subject: Re: [PATCH 06/12] clk: sunxi=ng: add support for R329 CCUs
From:   Icenowy Zheng <uwu@icenowy.me>
To:     Samuel Holland <samuel@sholland.org>
Cc:     Rob Herring <robh+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Andre Przywara <andre.przywara@arm.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Date:   Tue, 12 Jul 2022 20:16:49 +0800
In-Reply-To: <4fc9873e87c11dce23099a24be34465f09f3a9a4.camel@aosc.io>
References: <20220422140902.1058101-1-icenowy@aosc.io>
         <BYAPR20MB2472930AAFFBDC0ACB9A7487BCF79@BYAPR20MB2472.namprd20.prod.outlook.com>
         <c858b944-d72f-4e59-6a1a-329b5b8949c4@sholland.org>
         <4fc9873e87c11dce23099a24be34465f09f3a9a4.camel@aosc.io>
Organization: Anthon Open-Source Community
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.4 
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

=E5=9C=A8 2022-07-12=E6=98=9F=E6=9C=9F=E4=BA=8C=E7=9A=84 19:57 +0800=EF=BC=
=8CIcenowy Zheng=E5=86=99=E9=81=93=EF=BC=9A
> =E5=9C=A8 2022-04-23=E6=98=9F=E6=9C=9F=E5=85=AD=E7=9A=84 21:12 -0500=EF=
=BC=8CSamuel Holland=E5=86=99=E9=81=93=EF=BC=9A
> > On 4/22/22 10:41 AM, icenowy@outlook.com=C2=A0wrote:
> > > From: Icenowy Zheng <icenowy@aosc.io>
> > >=20
> > > Allwinner R329 has two CCUs, one in CPUX and another in PRCM.
> > >=20
> > > Add support for them.
> > >=20
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >=20
> > There is a typo in your commit title. =3D should be -.
> >=20
> > Thanks for updating the driver to use .fw_name and be loadable as a
> > module. All
> > of those changes look good.
> >=20
> > There are still some missing clocks here compared to the BSP, and a
> > couple of
> > other minor issues. Please see my earlier review:
> >=20
> > https://lore.kernel.org/linux-sunxi/99a74950-fdc0-ecfe-e5f0-ba4a7d8751f=
0@sholland.org/
> >=20
> > So far it's been consistent that any settable bits in the CCU
> > registers actually
> > do something. So I would expect all of those bits to have an index
> > reserved in
> > the binding, even if we do not model them. I want to avoid having
> > to
>=20
> Sorry but I don't think it proper to reserve unclear bits, because
> we're just allocating the numbers as a random sequence (in fact it's
> the sequence that it gets implemented).
>=20
> Or consider a structural number scheme, in which a value can be
> uniquely predicted by its name?

Well by thinking a little further, I realized our current CCU DT
binding is just based on implementation details of sunxi-ng drivers
(for example, some intermediate clocks get a number too, just not
exposed to the DT binding header). This continues to prove that
reserving numbers is just needed at all, and the current way to make
CCU DT bindings might be totally wrong.

Maybe we should start to use reasonable slice numbers in the next CCU
driver?

>=20
> > go back and
> > add gates to the binding out-of-order later, like we are doing for
> > H6.
> >=20
> > Regards,
> > Samuel
>=20
>=20
>=20