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

[Abel Vesa]

On 26-06-30 13:25:13, 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
> 
> 
> > 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?

Wait, don't we do that for display nodes anyway ?

Also there might be a case where you just don't want the ICE enabled.

So I suggest we disable the ICE by default and enable it only on boards
where there is a consumer.

We don't normally leave stuff enabled just because runtime PM will
disabled it for us.