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

[Harshal Dev]

Hi Krzysztof,

On 3/11/2026 11:58 PM, Krzysztof Kozlowski wrote:
> On 11/03/2026 19:25, Krzysztof Kozlowski wrote:
>> On 11/03/2026 10:37, Harshal Dev wrote:
>>>
>>>
>>> On 3/11/2026 1:55 AM, Krzysztof Kozlowski wrote:
>>>> On 10/03/2026 09:06, Harshal Dev wrote:
>>>>> Update the inline-crypto engine DT binding to allow specifying up to two
>>>>> clocks along with their names and associated power-domain. When the
>>>>> 'clk_ignore_unused' flag is not passed on the kernel command line
>>>>> occasional unclocked ICE hardware register access are observed during ICE
>>>>> driver probe based on the relative timing between the probe and the kernel
>>>>> disabling the unused clocks. 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. To avoid these scenarios, the 'iface' clock and
>>>>> the associated power-domain should be specified in the ICE device tree node
>>>>> and the 'iface' clock should be voted on by the ICE driver during probe.
>>>>>
>>>>> 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 c3408dcf5d20..d9a0a8adf645 100644
>>>>> --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
>>>>> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
>>>>> @@ -28,6 +28,16 @@ properties:
>>>>>      maxItems: 1
>>>>>  
>>>>>    clocks:
>>>>> +    minItems: 1
>>>>> +    maxItems: 2
>>>>> +
>>>>> +  clock-names:
>>>>> +    minItems: 1
>>>>> +    items:
>>>>> +      - const: ice_core_clk
>>>>
>>>> core
>>>
>>> Ack. I'll introduce a check for this specific name here as well:
>>> https://elixir.bootlin.com/linux/v7.0-rc3/source/drivers/soc/qcom/ice.c#L582
>>>
>>>>
>>>>> +      - const: iface_clk
>>>>
>>>> iface or bus
>>>
>>> Ack, will call it 'iface'.
>>>
>>>>
>>>> I don't understand why this is flexible and commit msg does not explain
>>>> that. Devices do not have one and two clocks at the same time. You miss
>>>> proper constraints.
>>>>
>>>
>>> I agree, it might confuse someone reading the commit message the first time.
>>> I'll re-write the commit message to make it explicit that even though these
>>> two properties are 'required', for the time being we are introducing 'iface'
>>> clk and 'power-domain' as an optional property to maintain bisectability,
>>> and that the properties would be made 'required' in a subsequent commit once
>>> the DTS changes which are part of this patch series have reached the top tree.
>>>
>>> Let me know if any concerns with this kind of commit message.
>>
>> So you are adding it for backwards compatibility? It's fine then,
>> although I had impression you are fixing something which is not working
>> correctly. New devices will need to constrain this.
> 

Yes, this is for backward compatibility.

> Except new devices, like Eliza and Milos. And then this should go to
> current fixes.

I'm not sure if I understand correctly, do you mean to say that except for Eliza
and Milos, new devices need to change their DT binding to 'required' with
corresponding DTS changes. And then, the patch updating the DT binding also needs
to be back-ported?

I'm assuming you're leaving out Eliza and Milos because they aren't supported
on the stable branches yet?

Apologies in advance if you meant something else and I have completely misunderstood
your comment.

Regards,
Harshal

> 
> Best regards,
> Krzysztof