Re: [PATCH 6.12 0139/1276] arm64: dts: qcom: sc7180: Add power-domain and iface clk for ice node

Hi Harshal,

> Hi Harshit,
> 
> Apologies for the late response, I am currently on vacation.
> 

You really don't need to apologize, and thanks for checking!

> On 24-07-2026 05:50 pm, Harshit Mogalapalli wrote:
....

>> I reviewed these four 6.12.y commits and checked the current 6.12 ICE driver and binding: Patches 139-142:
>> 
>> - e49e61994f4d ("arm64: dts: qcom: sc7180: Add power-domain and iface clk for ice node")
>> - 4960fae719ad ("arm64: dts: qcom: kodiak: Add power-domain and iface clk for ice node")
>> - bac1108e0040 ("arm64: dts: qcom: sm8450: Add power-domain and iface clk for ice node")
>> - c4a4e6c6255d ("arm64: dts: qcom: sm8650: Add power-domain and iface clk for ice node")
>> 
>> I may be missing something, so I wanted to sanity-check my reading.
>> 
>> These DTS patches add a second clock named "iface" and also add
>> power-domains to the ICE node.
>> 
>> Upstream commit 0d5dc5818191 updates the driver to request both clocks:
>> 
>> if (!engine->core_clk)
>>         engine->core_clk = devm_clk_get_optional_enabled(dev, "core");
>> 
>> engine->iface_clk = devm_clk_get_optional_enabled(dev, "iface");
>> 
>> But 6.12.y still only requests one clock:
>> 
>> engine->core_clk = devm_clk_get_optional_enabled(dev, "ice_core_clk");
>> if (!engine->core_clk)
>>         engine->core_clk = devm_clk_get_optional_enabled(dev, "ice");
>> if (!engine->core_clk)
>>         engine->core_clk = devm_clk_get_enabled(dev, NULL);
>> 
>> if (!qcom_ice_check_supported(engine))
>>         return ERR_PTR(-EOPNOTSUPP);
>> 
>> My understanding is that these DTS patches add a second clock to the ICE node, but the 6.12.y ICE driver still enables only one clock from that node.
>> 
>> If so, the new "iface" clock on the ICE node would never be requested on
>> 6.12.y, and ICE could still touch registers during probe and resume with only the "core" clock enabled. Is that the right reading?
>>
> 
> Yes this analysis is correct. I can see Greg already picked this patch:
> https://lore.kernel.org/all/[email protected]/
> 

Thanks for checking and sharing that.

>> Separately, the 6.12 binding still seems to have clocks: maxItems: 1 and
>> does not describe clock-names or power-domains.
>>
> 
> It doesn't right now, yes.
> 
>> If that reading is right, should 6.12.y take adapted versions of these two commits before the four DTS patches?
>> 
>> - 0d5dc5818191 ("soc: qcom: ice: Allow explicit votes on 'iface' clock for ICE")
>> - e27264daac7d ("dt-bindings: crypto: qcom,ice: Fix missing power-domain and iface clk")
>>
> 
> The 1st can be taken on-top of the four DTS, since describing the clocks first but enabling them
> later via the commit is totally fine.
> 

Sure.
> The 2nd one should be taken to make the DTS patches comply with an updated binding. The binding is
> written in a backward compatible way so it will not break any out of tree DTS still relying on
> the old binding. However, the patch must be adapted to 6.12 to not make the power-domain and
> clock-names mandatory for Eliza and Milos since those targets arrived in 7.1 kernel. Only the first
> portion of the patch is applicable for 6.12.
> 

Can you please help with that whenever you can ?

thanks,
Harshit