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