diff mbox series

[v5,2/6] dt-bindings: ufs: qcom: Add ICE phandle

Message ID 20230403200530.2103099-3-abel.vesa@linaro.org
State New
Headers show
Series Add dedicated Qcom ICE driver | expand

Commit Message

Abel Vesa April 3, 2023, 8:05 p.m. UTC
Starting with SM8550, the ICE will have its own devicetree node
so add the qcom,ice property to reference it.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---

The v4 is here:
https://lore.kernel.org/all/20230327134734.3256974-4-abel.vesa@linaro.org/

Changes since v4:
 * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce
   it while making sure none of the other platforms are allowed to use it

Changes since v3:
 * dropped the "and drop core clock" part from subject line

Changes since v2:
 * dropped all changes except the qcom,ice property


 .../devicetree/bindings/ufs/qcom,ufs.yaml     | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Krzysztof Kozlowski April 4, 2023, 5:41 a.m. UTC | #1
On 03/04/2023 22:05, Abel Vesa wrote:
> Starting with SM8550, the ICE will have its own devicetree node
> so add the qcom,ice property to reference it.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> 
> The v4 is here:
> https://lore.kernel.org/all/20230327134734.3256974-4-abel.vesa@linaro.org/
> 
> Changes since v4:
>  * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce
>    it while making sure none of the other platforms are allowed to use it

Why?

Also, this does not solve my previous question still.

Best regards,
Krzysztof
Abel Vesa April 4, 2023, 8:59 a.m. UTC | #2
On 23-04-04 07:41:55, Krzysztof Kozlowski wrote:
> On 03/04/2023 22:05, Abel Vesa wrote:
> > Starting with SM8550, the ICE will have its own devicetree node
> > so add the qcom,ice property to reference it.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > 
> > The v4 is here:
> > https://lore.kernel.org/all/20230327134734.3256974-4-abel.vesa@linaro.org/
> > 
> > Changes since v4:
> >  * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce
> >    it while making sure none of the other platforms are allowed to use it
> 
> Why?

SM8550 will be the first platform to use the new DT bindings w.r.t ICE.

> 
> Also, this does not solve my previous question still.

Well, the clocks are not added for the a few platforms (which include
SM8550). Same for 'ice' reg range.. So the only thing left is to
enforce the qcom,ice property availability only for SM8550. I believe
it solves the mutual exclusiveness of the "ice" reg range along with the
clocks versus the qcom,ice property, by enforcing at compatible level.

Is this not enough?

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 4, 2023, 10:12 a.m. UTC | #3
On 04/04/2023 10:59, Abel Vesa wrote:
> On 23-04-04 07:41:55, Krzysztof Kozlowski wrote:
>> On 03/04/2023 22:05, Abel Vesa wrote:
>>> Starting with SM8550, the ICE will have its own devicetree node
>>> so add the qcom,ice property to reference it.
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>>>
>>> The v4 is here:
>>> https://lore.kernel.org/all/20230327134734.3256974-4-abel.vesa@linaro.org/
>>>
>>> Changes since v4:
>>>  * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce
>>>    it while making sure none of the other platforms are allowed to use it
>>
>> Why?
> 
> SM8550 will be the first platform to use the new DT bindings w.r.t ICE.

This I understand, but why other platforms cannot use it?

> 
>>
>> Also, this does not solve my previous question still.
> 
> Well, the clocks are not added for the a few platforms (which include
> SM8550). Same for 'ice' reg range.. So the only thing left is to
> enforce the qcom,ice property availability only for SM8550. I believe
> it solves the mutual exclusiveness of the "ice" reg range along with the
> clocks versus the qcom,ice property, by enforcing at compatible level.

Ah, I think I understand. That would work except I don't understand why
enforcing qcom,qce only for specific, new SoCs. Assuming it is a correct
hardware representation, we want it for everyone, don't we?

Best regards,
Krzysztof
Abel Vesa April 4, 2023, 10:41 a.m. UTC | #4
On 23-04-04 12:12:06, Krzysztof Kozlowski wrote:
> On 04/04/2023 10:59, Abel Vesa wrote:
> > On 23-04-04 07:41:55, Krzysztof Kozlowski wrote:
> >> On 03/04/2023 22:05, Abel Vesa wrote:
> >>> Starting with SM8550, the ICE will have its own devicetree node
> >>> so add the qcom,ice property to reference it.
> >>>
> >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> >>> ---
> >>>
> >>> The v4 is here:
> >>> https://lore.kernel.org/all/20230327134734.3256974-4-abel.vesa@linaro.org/
> >>>
> >>> Changes since v4:
> >>>  * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce
> >>>    it while making sure none of the other platforms are allowed to use it
> >>
> >> Why?
> > 
> > SM8550 will be the first platform to use the new DT bindings w.r.t ICE.
> 
> This I understand, but why other platforms cannot use it?

The platforms that do not have ICE support yet will be added in the same
subschema along with SM8550 when the ICE DT node will be added in their
dtsi.

> 
> > 
> >>
> >> Also, this does not solve my previous question still.
> > 
> > Well, the clocks are not added for the a few platforms (which include
> > SM8550). Same for 'ice' reg range.. So the only thing left is to
> > enforce the qcom,ice property availability only for SM8550. I believe
> > it solves the mutual exclusiveness of the "ice" reg range along with the
> > clocks versus the qcom,ice property, by enforcing at compatible level.
> 
> Ah, I think I understand. That would work except I don't understand why
> enforcing qcom,qce only for specific, new SoCs. Assuming it is a correct
> hardware representation, we want it for everyone, don't we?

Yes, but they will be added to the subschema (qcom,ice one) when their
their ICE support (ICE DT) will be added. This way, we keep the bindings
check without failures (for now).

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 4, 2023, 11:03 a.m. UTC | #5
On 04/04/2023 12:41, Abel Vesa wrote:
>>>>
>>>> Also, this does not solve my previous question still.
>>>
>>> Well, the clocks are not added for the a few platforms (which include
>>> SM8550). Same for 'ice' reg range.. So the only thing left is to
>>> enforce the qcom,ice property availability only for SM8550. I believe
>>> it solves the mutual exclusiveness of the "ice" reg range along with the
>>> clocks versus the qcom,ice property, by enforcing at compatible level.
>>
>> Ah, I think I understand. That would work except I don't understand why
>> enforcing qcom,qce only for specific, new SoCs. Assuming it is a correct
>> hardware representation, we want it for everyone, don't we?
> 
> Yes, but they will be added to the subschema (qcom,ice one) when their
> their ICE support (ICE DT) will be added. This way, we keep the bindings
> check without failures (for now).

I understand that then you will rework this if:then case, so I think it
is just easier to make it correct from the first place. If there is
qcom,qce, then reg is maxItems:1. Otherwise - maxItems can be 2. You
achieve the same result, all DTS validate, without any need of further
changes later.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index c5a06c048389..874de31d2c41 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -70,6 +70,10 @@  properties:
   power-domains:
     maxItems: 1
 
+  qcom,ice:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the Inline Crypto Engine node
+
   reg:
     minItems: 1
     maxItems: 2
@@ -187,6 +191,21 @@  allOf:
 
     # TODO: define clock bindings for qcom,msm8994-ufshc
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8550-ufshc
+    then:
+      properties:
+        qcom,ice:
+          maxItems: 1
+    else:
+      properties:
+        qcom,ice: false
+
+
 unevaluatedProperties: false
 
 examples: