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

[Krzysztof Kozlowski]

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.

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

Best regards,
Krzysztof