Re: [PATCH] clk: mvebu: ap806: fix legacy binding failing to probe

[Miquel Raynal]

Hi Bruno,

[...]

> Before v5.4, ap806_syscon_common_probe() had no compatible test and went
> straight from reading Sample at Reset to decoding freq_mode, so the
> legacy binding did work.  The commit introducing the test described
> itself as "No functional changes", ap806_syscon_legacy_of_match[] still
> advertises the legacy compatible, and the warnings above state only that
> the binding is deprecated.  The breakage therefore looks unintentional.
>
> Accept the legacy compatible in the AP806 branch so that the legacy
> binding works again as it did before v5.4, keeping the deprecation
> warnings intact.
>
> Fixes: be69e55df9af ("clk: mvebu: ap806: Prepare the introduction of AP807 clock support")
> Signed-off-by: Bruno Banelli <bbanelli@xxxxxxxxx>
> ---
>  drivers/clk/mvebu/ap806-system-controller.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> Found while booting mainline Linux with U-Boot's control FDT on a
> SolidRun MACCHIATObin (Armada 8040). U-Boot's arch/arm/dts/armada-ap80x.dtsi
> still uses the legacy binding, so any board booting Linux via U-Boot's EFI
> handover with the firmware-supplied FDT hits this.

While I agree with the report, I kind of dislike the fix which
re-instates a clearly unsupported compatible, legacy since 2019. Maybe
we can start by discussing the need: wouldn't it be more sensible to
update your DT? The switch has been operated in 2017, almost 10 years
ago, with a big fat deprecation notice (which you hit).

If we *really* want to fix this situation, I would prefer to avoid using
the long time deprecated compatible. Could we instead have the callers
(ap806_syscon_common_probe() and ap806_clock_probe()) give either an enum
or a *get_sar function pointer? The former would only provide the AP806
one, whereas the latter would still go through the same DT check.

Finally, if we go for a fix, it should be backported, so stable should
probably be Cc'ed.

Thanks,
Miquèl