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

[Abel Vesa]

On 26-08-28 21:07:24, Konrad Dybcio wrote:
> On 8/28/26 2:57 PM, Abel Vesa wrote:
> > 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()
> 
> [...]
> 
> >> 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.
> 
> I can think of literally none..
> 
> > 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.
> 
> We should because the hardware is in a state that's otherwise unknown
> to Linux, I've been trying to push that forward, but many things
> currently can't be enabled without side effects.

So interestingly, on Eliza at least, ICE isn't left powered by the
bootloader. Are you saying that, in general, bootloader leaves things
enabled on Qualcomm platforms? I'm assuming that would be ABL that does
that, right?

> 
> With the right drivers in place, the hw can be safely powered off or
> at least parked, instead of letting it spin in whatever state the
> bootloader has left it in.

I understand that point, however, this kinda makes the ICE a one-off.
We do not do that for mdss DSI/DP controllers/PHYs on boards without
display, right?

Not to mention that, on an end user device, the ICE driver would have
to be provided as module for the sole purpose of powering it down. It
doesn't make sense.