Re: [PATCH v8 2/3] arm64: dts: qcom: kodiak: enable inline crypto engine for SDHC

[Kuldeep Singh]

On 30-06-2026 16:55, Konrad Dybcio wrote:
> On 6/30/26 12:23 PM, Kuldeep Singh wrote:
>>> qcom_ice_probe()
>>>   -> qcom_ice_create()
>>>     -> devm_clk_get_optional_enabled()
>>>
>>> If we remove the _enabled suffix and put a ice_resume() in ice_get(),
>>> I believe this is no longer an issue
>>
>> I see your point.
>> devm_clk_get_optional_enabled turns clocks on, whereas using
>> devm_clk_get_optioanl will only get handle but don't enable clock.
>> Please note, clocks are needed at probe to read ice hw version 
> 
> Right, but we don't need them afterwards in the probe function. We
> can simply gate them. There's a parallel effort to enable runtime PM
> in the driver:
> 
> https://lore.kernel.org/linux-arm-msm/20260512033750.3393050-3-linlin.zhang@xxxxxxxxxxxxxxxx/
> 
> and with that all you'd need after getting the clock is a
> pm_runtime_resume_and_get() before the version check, followed by a
> pm_runtime_put() right after it

I understand issue can be fixed in driver too.
My argument/understanding was just why to even probe and check hw
version in this case.

> 
>> and then
>> later ufs/sdhc fetch ice instance using of_qcom_ice_get() and control
>> suspend/resume path.
>>
>> But my idea is, if storage itself isn't there(like sdhc on
>> qcs6490-rb3gen2) then why sdhc-ice should even probe and check hardware
>> version? as there's no significance in even probing ice.
> 
> On the developer/customer experience side, would you expect having to
> manually enable what's essentially a sub-feature of the storage media
> on every single board?

Umm ok, I feel it's pretty subjective as enable required featureset(and
it's dependencies) only if user is explicitly enabling it.
Controlling from driver/DT can anyway fix clock powered-on issue.

-- 
Regards
Kuldeep