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

[Harshal Dev]

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.

Regards,
Harshal
 
>> +
>> +  power-domains:
>>      maxItems: 1
> 
> 
> Best regards,
> Krzysztof