Re: [PATCH 01/11] dt-bindings: crypto: qcom,ice: Require power-domain and iface clk
From: Bjorn Andersson <andersson@kernel.org>
Date: 2026-02-20 15:59:45
Also in:
linux-arm-msm, linux-devicetree, lkml
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:quoted
On 2/6/2026 4:20 PM, Krzysztof Kozlowski wrote:quoted
On 06/02/2026 11:07, Harshal Dev wrote:quoted
On 2/5/2026 4:47 PM, Krzysztof Kozlowski wrote:quoted
On 03/02/2026 10:26, Harshal Dev wrote:quoted
On 1/26/2026 3:59 PM, Konrad Dybcio wrote:quoted
On 1/23/26 12:04 PM, Harshal Dev wrote:quoted
On 1/23/2026 2:27 PM, Krzysztof Kozlowski wrote:quoted
On 23/01/2026 08:11, Harshal Dev wrote:
[..]
quoted
quoted
quoted
quoted
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 acceptableWhat? 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. :)quoted
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. Regards, Bjorn
quoted
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/ (local) 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 -- மணிவண்ணன் சதாசிவம்