Re: [PATCH v3 01/12] dt-bindings: crypto: qcom,ice: Allow power-domain and iface clk

[Harshal Dev]

On 3/18/2026 12:52 PM, Krzysztof Kozlowski wrote:
> On Tue, Mar 17, 2026 at 05:12:36PM +0200, Dmitry Baryshkov wrote:
>> On Tue, Mar 17, 2026 at 02:50:40PM +0530, Harshal Dev wrote:
>>> Update the inline-crypto engine DT binding in a backward compatible manner
>>> to allow specifying up to two clocks along with their names and associated
>>> power-domain.
>>
>> This should come after the "why" part.
>>
>>>
>>> When the 'clk_ignore_unused' flag is not passed on the kernel command line
>>> occasional unclocked ICE hardware register access are observed when the
>>> kernel disables the unused 'iface' clock before ICE can probe. On the other
>>> hand, when the 'pd_ignore_unused' flag is not passed on the command line,
>>> clock 'stuck' issues are observed if the power-domain required by ICE
>>> hardware is unused and thus disabled before ICE probe could happen.
>>
>> You can simply say that ICE requires these clocks and these power
>> domains to function. Accessing the hardware can fail if they are
>> disabled by the kernel for whater reasons.
> 
> Yeah, mentioning clk_ignore_unused/pd is redundant here.

Ack.

> 
>>
>>>
>>> To avoid these scenarios, the 'iface' clock and the associated power-domain
>>> should be specified in the ICE device tree node and enabled by ICE.
> 
> And this repeats the first paragraph.

Ack.

> 
>>>
>>> Fixes: f6ff91a47ac57 ("dt-bindings: crypto: Add Qualcomm Inline Crypto Engine")
>>> Signed-off-by: Harshal Dev <harshal.dev@xxxxxxxxxxxxxxxx>
>>> ---
>>>  .../bindings/crypto/qcom,inline-crypto-engine.yaml       | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
>>> index 876bf90ed96e..99c541e7fa8c 100644
>>> --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
>>> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
>>> @@ -30,6 +30,16 @@ properties:
>>>      maxItems: 1
>>>  
>>>    clocks:
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  clock-names:
>>> +    minItems: 1
>>> +    items:
>>> +      - const: core
>>> +      - const: iface
>>> +
>>> +  power-domains:
>>>      maxItems: 1
> 
> 
> 1. What the DTS is doing here?

Okay. I will add a description of the expectation imposed by this binding on
the DTS in the commit message of this patch.

> 2. How did you address "with explanation why this is a fix thus why this
> should go to current cycle." - where is this part?

My mistake, I will explicitly write in the commit message that this change
is fixing the issues caused by missing power-domain and clocks in the DTS
by preserving backward-compatibility for old devices and constraining these
resources for new ones, i.e, Eliza and Milos.

> 3. Where is Eliza and Milos?

I will merge the unnecessary commit I introduced as Patch 2 into this commit
and explain that we are making the clock and power-domain 'required' for
Eliza and Milos since they constitute unreleased ABI and can carry this
new constraint.

Regards,
Harshal

> 
> Best regards,
> Krzysztof
>