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

[Harshal Dev]

Hi Kuldeep,

On 3/24/2026 4:16 PM, Kuldeep Singh wrote:
> 
> On 3/23/2026 2:47 PM, Harshal Dev wrote:
>> The DT bindings for inline-crypto engine do not specify the UFS_PHY_GDSC
>> power-domain and iface clock. Without enabling the iface clock and the
>> associated power-domain the ICE hardware cannot function correctly and
>> leads to unclocked hardware accesses being observed during probe.
>>
>> Fix the DT bindings for inline-crypto engine to require the UFS_PHY_GDSC
>> power-domain and iface clock for new devices (Eliza and Milos) introduced
>> in the current release (7.0) with yet-to-stabilize ABI, while preserving
>> backward compatibility for older devices.
>>
>> Fixes: 618195a7ac3df ("dt-bindings: crypto: qcom,inline-crypto-engine: Document the Eliza ICE")
>> Fixes: 85faec1e85555 ("dt-bindings: crypto: qcom,inline-crypto-engine: document the Milos ICE")
>> Signed-off-by: Harshal Dev <harshal.dev@xxxxxxxxxxxxxxxx>
>> ---
>>  .../bindings/crypto/qcom,inline-crypto-engine.yaml | 35 +++++++++++++++++++++-
>>  1 file changed, 34 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..ccb6b8dd8e11 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
>>  
>>    operating-points-v2: true
>> @@ -44,6 +54,25 @@ required:
>>  
>>  additionalProperties: false
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,eliza-inline-crypto-engine
>> +              - qcom,milos-inline-crypto-engine
>> +
>> +    then:
>> +      required:
>> +        - power-domains
>> +        - clock-names
>> +      properties:
>> +        clocks:
>> +          minItems: 2
>> +        clock-names:
>> +          minItems: 2
>> +
> 
> Hi Krzysztof,
> 
> As motive here is to enforce 2 clocks for upcoming targets and keep
> minItems as 1 for already merged ones for ensuring backward
> compatibility. Can we do like below?
> 
> allOf:
>   - if:
>       not:
>         properties:
>           compatible:
>             contains:
>               enum:
>                 - qcom,kaanapali-inline-crypto-engine
>                 - qcom,qcs8300-inline-crypto-engine
>                 - qcom,sa8775p-inline-crypto-engine
>                 - qcom,sc7180-inline-crypto-engine
>                 - qcom,sc7280-inline-crypto-engine
>                 - qcom,sm8450-inline-crypto-engine
>                 - qcom,sm8550-inline-crypto-engine
>                 - qcom,sm8650-inline-crypto-engine
>                 - qcom,sm8750-inline-crypto-engine
> 
>     then:
>       required:
>         - power-domains
>         - clock-names
>       properties:
>         clocks:
>           minItems: 2
>         clock-names:
>           minItems: 2
> 
> This will ensure for every new target addition, default clock count is
> enforced as 2 default.
> Please share your thoughts as well.
> 

I don't really have any particular objections to this proposal, but I can
see that other bindings where the need for an additional clock was realized
later on use a similar pattern as this patchset does:
https://elixir.bootlin.com/linux/v7.0-rc2/source/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml

I'll wait for Krzysztof to take a final call on this.

Regards,
Harshal