Received-SPF: pass (google.com: domain of linux-kernel+bounces-28865-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 17 Jan 2024 11:51:52 +0100
From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: Jerome Brunet <jbrunet@baylibre.com>
Cc: Thierry Reding <thierry.reding@gmail.com>, 
	Neil Armstrong <neil.armstrong@linaro.org>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Kevin Hilman <khilman@baylibre.com>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-amlogic@lists.infradead.org, linux-pwm@vger.kernel.org, JunYi Zhao <junyi.zhao@amlogic.com>
Subject: Re: [PATCH v4 2/6] dt-bindings: pwm: amlogic: add new compatible for
 meson8 pwm type
Message-ID: <b3a5r4gti67zic6egpwnede7niakn54ewfpmxg4ojm6ye2qopx@rke57jcyuveg>
References: <20231222111658.832167-1-jbrunet@baylibre.com>
 <20231222111658.832167-3-jbrunet@baylibre.com>
 <4rdb2be2bfzak3s4uaizthcdcdwdrxnx4kr2sgn527hvsie3pb@gfqciim7yryz>
 <1jfrywxnu5.fsf@starbuckisacylon.baylibre.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="3bhlfpvugwaqjoz5"
Content-Disposition: inline
In-Reply-To: <1jfrywxnu5.fsf@starbuckisacylon.baylibre.com>


--3bhlfpvugwaqjoz5
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hello Jerome,

On Wed, Jan 17, 2024 at 11:16:31AM +0100, Jerome Brunet wrote:
> On Wed 17 Jan 2024 at 10:58, Uwe Kleine-K=F6nig <u.kleine-koenig@pengutro=
nix.de> wrote:
> > [[PGP Signed Part:Undecided]]
> > Hello,
> >
> > On Fri, Dec 22, 2023 at 12:16:50PM +0100, Jerome Brunet wrote:
> >> diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/=
Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> >> index a1d382aacb82..eece390114a3 100644
> >> --- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> >> @@ -21,23 +21,35 @@ properties:
> >>            - amlogic,meson-g12a-ee-pwm
> >>            - amlogic,meson-g12a-ao-pwm-ab
> >>            - amlogic,meson-g12a-ao-pwm-cd
> >> -          - amlogic,meson-s4-pwm

Either I still didn't grasp all the details of this change, or removing
amlogic,meson-s4-pwm in this commit is wrong.

> >> +        deprecated: true
> >>        - items:
> >>            - const: amlogic,meson-gx-pwm
> >>            - const: amlogic,meson-gxbb-pwm
> >> +        deprecated: true
> >>        - items:
> >>            - const: amlogic,meson-gx-ao-pwm
> >>            - const: amlogic,meson-gxbb-ao-pwm
> >> +        deprecated: true
> >>        - items:
> >>            - const: amlogic,meson8-pwm
> >>            - const: amlogic,meson8b-pwm
> >> +        deprecated: true
> >
> > I think deprecating the old binding and adding a new compatible should
> > be done in two commits.
>=20
> Hi Uwe,
>=20
> There was the same comment on v3 and Krzysztof said it should be done
> like this:
>=20
> https://lore.kernel.org/linux-pwm/e127dcef-3149-443a-9a8c-d24ef4054f09@li=
naro.org
>=20
> I tend to agree with Krzysztof on this but, as I previously said,
> I don't really mind one way or the other. Just have to pick one.

Ah, so the machines that used amlogic,meson-g12a-ee-pwm before are
supposed to use amlogic,meson-g12-pwm-v2 now. With that understood I
agree to you and Krzysztof.

I wonder if me not understanding that is a sign that the commit log
isn't optimal (or if it's only that I didn't properly read it :-).
Now that I understood the change better, the commit log is
understandable, but maybe still make it a bit more explicit that it
introduces a new way to formalize already supported hardware. Something
like:

	dt-bindings: pwm: amlogic: Add a new binding for meson8 pwm types

	The binding that is used up to now describe which input the PWM
	channel multiplexer should pick among its possible parents,
	which are hardcoded in the driver. This isn't a good binding in
	the sense that it should describe hardware but not usage.

	Add a new binding deprecating the old one that uses clocks in a
	better way and how clocks are usually used today: The list of
	clocks describe the inputs of the PWM block as they are realised
	in hardware.

	So deprecate the old bindings and introduce a compatible per SoC
	family to replace these.

I think I'd understand that better, but that might be because I wrote
it and it's subjective?

Best regards
Uwe

--=20
Pengutronix e.K.                           | Uwe Kleine-K=F6nig            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

--3bhlfpvugwaqjoz5
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmWnsUcACgkQj4D7WH0S
/k5jBgf+K4/2nlwNuhl98wP3lhKs9noKvnoecmlxEqzV+HowMVaRvasoTYxpWZst
F0kE3hWQF654JB/Whvdb9NVMJHUIfvuok6egBVBC7VKtGKR8sUUUD1vc0CnxV1OE
tosScnl0r/zPXyFWU6xYPzFHVrnndxBwjvneScCkA8W1hx8eLG3B8pFHFTjsxvOg
gQtlS46eDfnJ9llQDl1N2iZHdOdfAXqFegZB2RCHmIorS1z86x53Wci/tFvYLH6F
e6Jn0Nwrf6EPHzQK2FCQZOa9BwFJDZ6NiivNXMSzp4RgCSmNEySOlc4XVBoF4Sh7
hbnETJ5us1luELfbF4oSg2Y3H8c3Vw==
=iYzS
-----END PGP SIGNATURE-----

--3bhlfpvugwaqjoz5--