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

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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 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

Konrad