diff mbox series

[v2,03/10] dt-bindings: qcom-qce: Fix compatibles combinations for SM8150 and IPQ4019 SoCs

Message ID 20230322114519.3412469-4-bhupesh.sharma@linaro.org
State New
Headers
Series arm64: qcom: Enable Crypto Engine for a few Qualcomm SoCs |

Commit Message

Bhupesh Sharma March 22, 2023, 11:45 a.m. UTC 
  Currently the compatible list available in 'qce' dt-bindings does not
support SM8150 and IPQ4019 SoCs directly, leading to following
'dtbs_check' error:

 arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano-griffin.dtb:
  crypto@1dfa000: compatible: 'oneOf' conditional failed, one must be fixed:
	['qcom,sm8150-qce', 'qcom,qce'] is too long
	['qcom,sm8150-qce', 'qcom,qce'] is too short

Fix the same.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Vladimir Zapolskiy March 27, 2023, 11:49 a.m. UTC | #1 
Hi Bhupesh,

On 3/22/23 13:45, Bhupesh Sharma wrote:
> Currently the compatible list available in 'qce' dt-bindings does not
> support SM8150 and IPQ4019 SoCs directly, leading to following
> 'dtbs_check' error:
> 
>   arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano-griffin.dtb:
>    crypto@1dfa000: compatible: 'oneOf' conditional failed, one must be fixed:
> 	['qcom,sm8150-qce', 'qcom,qce'] is too long
> 	['qcom,sm8150-qce', 'qcom,qce'] is too short
> 
> Fix the same.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>   Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> index e375bd981300..90ddf98a6df9 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> @@ -24,6 +24,12 @@ properties:
>           deprecated: true
>           description: Kept only for ABI backward compatibility
>   
> +      - items:
> +          - enum:
> +              - qcom,ipq4019-qce
> +              - qcom,sm8150-qce
> +          - const: qcom,qce
> +

thank you for the fix, the change is correct, please apply the tag:

Fixes: 00f3bc2db351 ("dt-bindings: qcom-qce: Add new SoC compatible strings for Qualcomm QCE IP")

But let me ask you to split the "items" into two:

       - items:
           - const: qcom,ipq4019-qce
           - const: qcom,qce

       - items:
           - const: qcom,sm8150-qce
           - const: qcom,qce

and put each of these two new items below a list of compatible values,
where a family compatible describes the list.

Nevertheless, please also add my tag:

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

--
Best wishes,
Vladimir
Krzysztof Kozlowski March 27, 2023, 12:19 p.m. UTC | #2 
On 27/03/2023 13:49, Vladimir Zapolskiy wrote:
> Hi Bhupesh,
> 
> On 3/22/23 13:45, Bhupesh Sharma wrote:
>> Currently the compatible list available in 'qce' dt-bindings does not
>> support SM8150 and IPQ4019 SoCs directly, leading to following
>> 'dtbs_check' error:
>>
>>   arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano-griffin.dtb:
>>    crypto@1dfa000: compatible: 'oneOf' conditional failed, one must be fixed:
>> 	['qcom,sm8150-qce', 'qcom,qce'] is too long
>> 	['qcom,sm8150-qce', 'qcom,qce'] is too short
>>
>> Fix the same.
>>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> index e375bd981300..90ddf98a6df9 100644
>> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> @@ -24,6 +24,12 @@ properties:
>>           deprecated: true
>>           description: Kept only for ABI backward compatibility
>>   
>> +      - items:
>> +          - enum:
>> +              - qcom,ipq4019-qce
>> +              - qcom,sm8150-qce
>> +          - const: qcom,qce
>> +
> 
> thank you for the fix, the change is correct, please apply the tag:
> 
> Fixes: 00f3bc2db351 ("dt-bindings: qcom-qce: Add new SoC compatible strings for Qualcomm QCE IP")
> 
> But let me ask you to split the "items" into two:
> 
>        - items:
>            - const: qcom,ipq4019-qce
>            - const: qcom,qce
> 
>        - items:
>            - const: qcom,sm8150-qce
>            - const: qcom,qce
> 

Why splitting these? The enum is the preferred way usually, so why here
do it differently?

Best regards,
Krzysztof
Bhupesh Sharma March 28, 2023, 5:12 a.m. UTC | #3 
On Mon, 27 Mar 2023 at 17:49, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 27/03/2023 13:49, Vladimir Zapolskiy wrote:
> > Hi Bhupesh,
> >
> > On 3/22/23 13:45, Bhupesh Sharma wrote:
> >> Currently the compatible list available in 'qce' dt-bindings does not
> >> support SM8150 and IPQ4019 SoCs directly, leading to following
> >> 'dtbs_check' error:
> >>
> >>   arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano-griffin.dtb:
> >>    crypto@1dfa000: compatible: 'oneOf' conditional failed, one must be fixed:
> >>      ['qcom,sm8150-qce', 'qcom,qce'] is too long
> >>      ['qcom,sm8150-qce', 'qcom,qce'] is too short
> >>
> >> Fix the same.
> >>
> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> >> ---
> >>   Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> >> index e375bd981300..90ddf98a6df9 100644
> >> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> >> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> >> @@ -24,6 +24,12 @@ properties:
> >>           deprecated: true
> >>           description: Kept only for ABI backward compatibility
> >>
> >> +      - items:
> >> +          - enum:
> >> +              - qcom,ipq4019-qce
> >> +              - qcom,sm8150-qce
> >> +          - const: qcom,qce
> >> +
> >
> > thank you for the fix, the change is correct, please apply the tag:
> >
> > Fixes: 00f3bc2db351 ("dt-bindings: qcom-qce: Add new SoC compatible strings for Qualcomm QCE IP")
> >
> > But let me ask you to split the "items" into two:
> >
> >        - items:
> >            - const: qcom,ipq4019-qce
> >            - const: qcom,qce
> >
> >        - items:
> >            - const: qcom,sm8150-qce
> >            - const: qcom,qce
> >
>
> Why splitting these? The enum is the preferred way usually, so why here
> do it differently?

Exactly, so our compatibles as per my patch can be :
"qcom,ipq4019-qce", "qcom,qce" or "qcom,sm8150-qce", "qcom,qce" which
is what we want to achieve as these are the base compatible versions
for further socs, with compatible strings as:

"qcom,<new-soc-with-crypto-same-as-ipq4019-qce", "qcom,ipq4019-qce",
"qcom,qce" , or
"qcom,<new-soc-with-crypto-same-as-sm8150-qce", "qcom,sm8150-qce", "qcom,qce"

Thanks,
Bhupesh
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index e375bd981300..90ddf98a6df9 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -24,6 +24,12 @@  properties:
         deprecated: true
         description: Kept only for ABI backward compatibility
 
+      - items:
+          - enum:
+              - qcom,ipq4019-qce
+              - qcom,sm8150-qce
+          - const: qcom,qce
+
       - items:
           - enum:
               - qcom,ipq6018-qce