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

Eric Biggers
2023-03-10T20:00:55Z

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@kernel.org/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