linux-kernel - Re: [PATCH 11/11] soc: qcom: ice: Add explicit power-domain and clock voting calls for ICE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0f8c0e5e-4921-47e8-b602-e6c08bb1f6f8@oss.qualcomm.com>
Date: Tue, 3 Feb 2026 15:01:45 +0530
From: Harshal Dev <harshal.dev@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Abel Vesa <abel.vesa@....qualcomm.com>,
        cros-qcom-dts-watchers@...omium.org
Cc: Brian Masney <bmasney@...hat.com>,
        Neeraj Soni <neeraj.soni@....qualcomm.com>,
        Gaurav Kashyap <gaurav.kashyap@....qualcomm.com>,
        linux-arm-msm@...r.kernel.org, linux-crypto@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 11/11] soc: qcom: ice: Add explicit power-domain and clock
 voting calls for ICE

Hi Konrad,

On 1/30/2026 4:16 PM, Konrad Dybcio wrote:
> On 1/23/26 12:12 PM, Harshal Dev wrote:
>> Hi Krzysztof,
>>
>> On 1/23/2026 4:27 PM, Krzysztof Kozlowski wrote:
>>> On 23/01/2026 09:58, Krzysztof Kozlowski wrote:
>>>>>  
>>>>>  	return 0;
>>>>> @@ -584,6 +597,10 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>>>>>  	if (IS_ERR(engine->core_clk))
>>>>>  		return ERR_CAST(engine->core_clk);
>>>>>  
>>>>> +	engine->iface_clk = devm_clk_get_enabled(dev, "iface_clk");
>>>>> +	if (IS_ERR(engine->iface_clk))
>>>>> +		return ERR_CAST(engine->iface_clk);
>>>>
>>>> And here actual breakage of ALL in-tree and out-of-tree DTS.
>>>>
>>>> NAK.
>>>>
>>>> Please read internal guideline.
>>>
>>> Internal docs are pretty scattered and messy so I failed to find this
>>> there, which is surprising because this was frequent feedback. Therefore
>>> please update Kernel Upstreaming internal page with following:
>>>
>>> With few exceptions, it is not allowed to break the ABI, by making
>>> bindings or driver changes, where the existing or out of tree DTS would
>>> fail to boot. Updating in-tree DTS does not matter here, because DTS
>>> goes via different branch, thus driver branch would be always broken.
>>> This is explicitly documented in DT rules and explained also in
>>> maintainer-soc profile.
>>>
>>> You need to either provide strong justification for ABI break or make
>>> the changes backwards compatible.
> 
> If the ICE can not be powered on alone without this change (i.e. no UFS,
> just ICE), then please spell it out explicitly, Harshal. That makes for a
> valid reason to break the ABI.
> 
> Plus the fact that without an OPP table, the voltage requirements cannot
> be guaranteed to be met
> 

Ack, I have endorsed and stated this point on the DT-binding commit.
I'll wait for Krzysztof's view before updating the commit message to
strongly reflect this point.

>>
>> Ack and understood. Let me write this in a way that makes it backward
>> compatible by using devm_clk_get_optional_enabled(). Like I explained, for
>> Linux distros where CONFIG_SCSI_UFS_QCOM is override set to 'y'. This
>> clock vote isn't really needed during probe.
> 
> This is really a side-effect that we shouldn't be depending on, or
> even considering as a backup, since the UFS driver may change
> independently and stop behaving this way one day
> 
>> In qcom_ice_suspend/resume(). I'll only prepare/un-prepare this clock
>> if it was found during probe.
> 
> Clock APIs generally happily eat nullptrs

Ack, then I guess in either case we can continue to keep the calls to
prepare/un-prepare. Anyways, I am convinced we should specify and use the
'iface' clock.

Thanks,
Harshal

> 
> Konrad