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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 18/03/2026 11:30, Harshal Dev wrote:
> 
> 
> 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@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 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.

You target this to fixes. Your subject and PATCH prefix should state
that. DTS is irrelevant in the git history in this context.

I stated previous the reason why this must go to the fixes.


Best regards,
Krzysztof