Received-SPF: pass (google.com: domain of linux-kernel+bounces-18552-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Luca Weiss <luca@z3ntu.xyz>
To: ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
 Bjorn Andersson <andersson@kernel.org>,
 Konrad Dybcio <konrad.dybcio@linaro.org>,
 Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>,
 Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] clk: qcom: hfpll: Add QCS404-specific compatible
Date: Sat, 06 Jan 2024 11:19:48 +0100
Message-ID: <6706261.DvuYhMxLoT@z3ntu.xyz>
In-Reply-To: <588dd6c3-7d2b-48db-b5ea-48a07077dc5d@linaro.org>
References:
 <20231231-hfpll-yaml-v1-0-359d44a4e194@z3ntu.xyz>
 <20231231-hfpll-yaml-v1-2-359d44a4e194@z3ntu.xyz>
 <588dd6c3-7d2b-48db-b5ea-48a07077dc5d@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"

On Dienstag, 2. J=E4nner 2024 11:41:26 CET Krzysztof Kozlowski wrote:
> On 31/12/2023 15:48, Luca Weiss wrote:
> > It doesn't appear that the configuration is for the HFPLL is generic, so
>=20
> That's ok...
>=20
> > add a qcs404-specific compatible and rename the existing struct to
>=20
> but why this is the solution? If the qcom,hfpll compatible was
> deprecated, but it is not. This commit is contradictory to the bindings.
>=20
> > qcs404.
> >=20
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> >=20
> >  drivers/clk/qcom/hfpll.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/drivers/clk/qcom/hfpll.c b/drivers/clk/qcom/hfpll.c
> > index dac27e31ef60..5b12982519be 100644
> > --- a/drivers/clk/qcom/hfpll.c
> > +++ b/drivers/clk/qcom/hfpll.c
> > @@ -14,7 +14,7 @@
> >=20
> >  #include "clk-regmap.h"
> >  #include "clk-hfpll.h"
> >=20
> > -static const struct hfpll_data hdata =3D {
> > +static const struct hfpll_data qcs404 =3D {
> >=20
> >  	.mode_reg =3D 0x00,
> >  	.l_reg =3D 0x04,
> >  	.m_reg =3D 0x08,
> >=20
> > @@ -84,10 +84,12 @@ static const struct hfpll_data msm8976_cci =3D {
> >=20
> >  };
> > =20
> >  static const struct of_device_id qcom_hfpll_match_table[] =3D {
> >=20
> > -	{ .compatible =3D "qcom,hfpll", .data =3D &hdata },
> >=20
> >  	{ .compatible =3D "qcom,msm8976-hfpll-a53", .data =3D &msm8976_a53 },
> >  	{ .compatible =3D "qcom,msm8976-hfpll-a72", .data =3D &msm8976_a72 },
> >  	{ .compatible =3D "qcom,msm8976-hfpll-cci", .data =3D &msm8976_cci },
> >=20
> > +	{ .compatible =3D "qcom,qcs404-hfpll", .data =3D &qcs404 },
> > +	/* deprecated, use SoC-specific compatible */
>=20
> Why? That's not a deprecated compatible. You now expect to create many
> unnecessary entries, which is not really needed. This is opposite of
> what we try to achieve with compatibility lists.

Just "qcom,hfpll" is not allowed by the bindings.

The problem is that it's actually unclear to me what "qcom,hfpll" was suppo=
sed=20
to be currently. It was added originally for MSM8974 and friends (see git l=
og)=20
but then is currently only used by QCS404 while in QCS404 downstream msm-4.=
4=20
(I think it was 4.4) I see different driver data than what's here.

So I wanted to just move what's used here to be qcs404-specific and then in=
 an=20
upcoming patch add a msm8974-specific compatible with different driver data.

Also wouldn't the "don't extend driver lists when not neccessary" mean usin=
g=20
something like "qcom,msm1234-hfpll", "qcom,qcs404-hfpll", "qcom,hfpll" then=
?=20
That was kind of my idea if some other SoC can reuse e.g. qcs404 data?

Regards
Luca

>=20
> Best regards,
> Krzysztof