Re: [PATCH v2 3/3] ARM: dts: lpc32xx: Update spi clock properties

From: Robin Murphy
Date: Mon Mar 14 2022 - 13:56:11 EST


On 2022-03-14 12:32, Arnd Bergmann wrote:
On Mon, Mar 14, 2022 at 1:20 PM Robin Murphy <robin.murphy@xxxxxxx> wrote:
On 2022-03-14 11:50, Vladimir Zapolskiy wrote:
On 3/14/22 1:43 PM, Robin Murphy wrote:


Thank you, it explains, why "apb_pclk" is required for all PrimeCell
controllers on the SoC. Nevertheless, in commit 93898eb775e5 it was
misidentified with the sspclk clock, the latter one is the only clock
explicitly utilized by the driver in 2015 and till today. Fixes in dts
files should be preceded by a fix in the driver.

There's nothing to fix in the driver, though. In fact it can only be
working today because the Linux driver isn't very strict and simply
assumes that the first clock entry is SSPCLK *without* considering its
name (other consumers of the binding might be stricter; I don't know),
and because presumably the HCLK happens to be enabled already anyway.
Changing the driver behaviour would only stand to cause functional
regressions.

We can change the driver to look for an sspclk by name first, and
then fall back to the first clk if none is found. This would be backwards
compatible and allow new dts files to have an arbitrary order, though
we still need to be careful about changing the existing dts files after
that, to avoid breaking older kernels.

Yeah, I discounted that since schema is strict about the order of entries even when they're named, so we'd have nothing to gain except the potential to break compatibility in one direction and annoy users in the other. There's little point in having code to look up a clock by name when we know that for compatibility reasons it has to be the first clock, so it would be functionally identical to the fallback code that we also still have to have. All it could offer is additionally printing a message about it, and experience says that that achieves little except the aforementioned annoyance of users - I look as far as my Rockchip boards that have all been screaming an error at me every boot for nearly 4 years now, where the ethernet driver fails to get a clock that only exists on some SoCs *other* than the ones I have :(

Robin.