Re: [PATCH 01/11] dt-bindings: crypto: qcom,ice: Require power-domain and iface clk

From: Harshal Dev

Date: Tue Mar 03 2026 - 01:30:06 EST


Hi Bjorn and Manivannan,

On 2/20/2026 9:29 PM, Bjorn Andersson wrote:
> On Fri, Feb 20, 2026 at 08:01:59PM +0530, Manivannan Sadhasivam wrote:
>> On Mon, Feb 09, 2026 at 11:13:06AM +0530, Harshal Dev wrote:
>>> On 2/6/2026 4:20 PM, Krzysztof Kozlowski wrote:
>>>> On 06/02/2026 11:07, Harshal Dev wrote:
>>>>> On 2/5/2026 4:47 PM, Krzysztof Kozlowski wrote:
>>>>>> On 03/02/2026 10:26, Harshal Dev wrote:
>>>>>>> On 1/26/2026 3:59 PM, Konrad Dybcio wrote:
>>>>>>>> On 1/23/26 12:04 PM, Harshal Dev wrote:
>>>>>>>>> On 1/23/2026 2:27 PM, Krzysztof Kozlowski wrote:
>>>>>>>>>> On 23/01/2026 08:11, Harshal Dev wrote:
> [..]
>>>>>> My NAK for driver change stays. This is wrong approach - you cannot
>>>>>> break working DTS.
>>>>>>
>>>>>
>>>>> I agree that this patch in it's current form will break both the in-kernel and
>>>>> out of tree DTS written in accordance with the old binding. If this isn't acceptable
>>>>
>>>> What? You just said few lines above:
>>>> "it will still continue to work if:"
>>>>
>>>
>>> I hope I am clear now, 'it' referred to the in-tree ICE driver and not to this particular
>>> DT schema commit. :)
>>>
>>>> So either this will continue to work or not. I don't understand this
>>>> thread and honestly do not have patience for it. I gave you already
>>>> reasoning what is wrong and why it is. Now it is just wasting my time.
>>>>
>>>
>>> Apologies again for the confusion. I totally agree, as replied previously too, that the
>>> updated DT binding breaks backward compatibility. Like I said, I will post another patch
>>> to preserve the correctness of existing in-tree and out-of-tree DTS.
>>>
>>
>> The ICE hardware cannot work without 'iface' clock and the power domain, which
>> are shared with the UFS PHY. One can argue that ICE is actually a part of the
>> peripherals like UFS/eMMC, but I don't have access to internal layout, so cannot
>> comment on that. I ran into this issue today when I tried to rmmod ice driver
>> together with ufs_qcom driver and got SError when reloading the module because
>> ice driver was trying to access unclocked/unpowered register.
>>
>> But you should mark the resources as 'required' in the binding and justify the
>> ABI break. No need to preserve backwards compatibility here as the binding was
>> wrong from day one.
>>
>
> Marking it "required" in the binding, implies that it's fine for the
> driver to fail in its absence. If I understand correctly that will
> prevent UFS and eMMC from probing, unless you have a DTB from "the
> future".
>
> Even if I merge the dt-binding change through the qcom-tree (together
> with the driver change) I will not guarantee that torvalds/master will
> remain bisectable - because dts changes and driver changes goes in
> different branches.
>
>
> As such, the pragmatic approach is to introduce the clock as optional
> and then once we're "certain" that the dts changes has propagated we
> can consider breaking the backwards compatibility.
>

Apologies for the late response, I am partially on vacation since last week.

First of all, thank you for acknowledging that in order to fix the bug with
the driver we need to break the DT backward compatibility. Now the only issue
as Bjorn mentioned is preserving bisectability since the changes reach the
top from different trees.

I agree, I can send a version 2 of this patch where I keep the iface clock
and power-domain as optional, and also accommodate the same in the driver
sources to ensure that UFS/EMMC probe does not fail even if the dts changes have
not yet reached the top tree. Once all the changes are merged, I will send
another patch to update the DT bindings for clocks and power-domain to 'required'
with accompanying changes on the driver side.

And so, kernels from that version on-wards will not probe UFS/EMMC with older DTS
which do not specify the iface and/or the power-domain. In my opinion, a probe
failure is a lot better than observing an un-clocked register access when the
DT bindings show the iface-clock and power-domain as optional.

I hope everyone is fine with this plan.

Regards,
Harshal

> Regards,
> Bjorn
>
>>> The only point I am trying to highlight for everyone's awareness is that as per this bug
>>> report https://lore.kernel.org/all/ZZYTYsaNUuWQg3tR@x1/ the kernel fails to boot with the
>>> existing DTS when the above two conditions aren't satisfied.
>>>
>>
>> And you sent the fix after almost 2 years. Atleast I'm happy that you got around
>> to fix it.
>>
>> - Mani
>>
>> --
>> மணிவண்ணன் சதாசிவம்