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

[Krzysztof Kozlowski <krzk@kernel.org>]

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@oss.qualcomm.com>
>>> ---
>>>  .../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.

Best regards,
Krzysztof