Hello,
On Tue, Jan 24, 2023 at 08:16:45AM +0100, Neil Armstrong wrote:
> Le 23/01/2023 à 22:22, Heiner Kallweit a écrit :
> > At first adjust some existing dtbs's so that they pass dtbs_check
> > after switching bindings to yaml.
>
> Thanks for this patchset, but please drop patches 1, 3 & 4, and take
> in account the existing compatible usage in your new bindings like
> I did in my conversion patchset.
>
> While we did remove some bad compatibles we introduced a few years ago,
> now the GXBB, GXL & GXM are now stable a aew LTS releases now and
> a few other projects uses them as-is (U-Boot, BSDs, ...) so changing
> the compatibles isn't an option anymore... and we can't know which
> one they use and how the implementation behaves we must document
> the existing usage without breaking any potential users (including linux).
I only looked into patch #1, and I support dropping it for stronger
reasons than not breaking things which maybe started to rely on the
existing contents.
In patch #1 you write:
| amlogic,meson-gx-pwm isn't a valid compatible string, so remove it.
| See drivers/pwm/pwm-meson.c.
|
| Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
| ---
| arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 8 ++++----
| 1 file changed, 4 insertions(+), 4 deletions(-)
|
| diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
| index a79a35e84..75d35dcfe 100644
| --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
| +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
| @@ -328,14 +328,14 @@ i2c_A: i2c@8500 {
| };
|
| pwm_ab: pwm@8550 {
| - compatible = "amlogic,meson-gx-pwm", "amlogic,meson-gxbb-pwm";
| + compatible = "amlogic,meson-gxbb-pwm";
There are two issues:
a) drivers/pwm/pwm-meson.c isn't the reference. The driver doesn't
justify which compatibles should be used. You should refer to the
binding document instead.
b) Having the SoC name as an additional compatible (i.e. the status quo
before your patch) is an advantage. While it doesn't hurt (apart from
making the dtb a tad bigger) it makes it possible to adapt the driver
if in the future someone discovers that the PWM component on GX is a
tad different from the GXBB one. In that case you can add a check in
the driver à la
if (of_device_is_compatible(np, amlogic,meson-gx-pwm))
do_the_special_gx_handling()
without having to adapt the device trees then (or use some ugly
code that somehow detects if it's running on GX).
So the driver not handling amlogic,meson-gx-pwm today is fine. I expect
the fix to be: Include that compatible in the binding.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
