Re: [RFC PATCH v2 7/7] arm64: dts: qcom: Add the Inline Crypto Engine nodes

[Eric Biggers]

On Fri, Mar 10, 2023 at 09:13:29AM +0100, Krzysztof Kozlowski wrote:
> > FWIW, I tested this patchset on SDA845, and ICE continues to work fine.
> 
> Really?

Yes.

> I clearly see of_find_device_by_node -> "return NULL" and all old code gone,
> so ABI is broken. Are you sure you applied patch 1-6 and ICE was working?

Yes.

But I applied the whole series.  It sounds like you are talking about the case
where *only* patches 1-6 are applied?  I did not test that, sorry.

> > (Though if I understand the patchset correctly, the ICE clock is no longer
> > turned off when the UFS host controller is suspended.  That isn't ideal as it
> > wastes power.  I would like that to be fixed.)
> > 
> > Anyway, when you say "break the ICE", do you really mean "make an incompatible
> > change to the device-tree bindings"?
> 
> It breaks existing users of DTS and kernel.
> 
> > 
> > I'd think there would be no problem with that as long as everything is updated
> > at once, which this patchset does.
> 
> Which is obviously not possible. DTS always goes separate branch,
> always. It cannot be combined with code into the same branch! So how do
> you even imagine this?

It is not obvious.  DTS and drivers are both in the kernel source tree.
It's natural to think they can be updated together as per
Documentation/process/stable-api-nonsense.rst.

I'm *not* saying they can be -- you're the expert here, and it sounds like DTS
are considered a stable UAPI.  I'm merely saying that it's not obvious...
especially when my personal experience is that reviewers explicitly preferred a
patch breaking DTS compatibility to a patch not breaking DTS compatibility
(https://lore.kernel.org/linux-scsi/20200710072013.177481-1-ebiggers@xxxxxxxxxx/T/#u)

Anyway, based on the follow-up replies, it sounds like Abel has a plan to keep
compatibility with old DTS, so hopefully that works and isn't too much trouble.

- Eric