diff mbox series

[v8,5/9] dt-bindings: qcom-qce: document clocks and clock-names as optional

Message ID 20230202135036.2635376-6-vladimir.zapolskiy@linaro.org
State New
Headers show
Series crypto: qcom-qce: Add YAML bindings & support for newer SoCs | expand

Commit Message

Vladimir Zapolskiy Feb. 2, 2023, 1:50 p.m. UTC 
From: Neil Armstrong <neil.armstrong@linaro.org>

On certain Snapdragon processors, the crypto engine clocks are enabled by
default by security firmware.

Drop clocks and clock-names from the required properties list.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
 1 file changed, 2 deletions(-)

Comments

Krzysztof Kozlowski Feb. 3, 2023, 8:17 a.m. UTC | #1 
On 02/02/2023 23:27, Vladimir Zapolskiy wrote:
> Hi Krzysztof,
> 
> On 2/2/23 15:53, Krzysztof Kozlowski wrote:
>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>
>>> On certain Snapdragon processors, the crypto engine clocks are enabled by
>>> default by security firmware.
>>
>> Then probably we should not require them only on these variants.
> 
> the rationale is clear, but here comes a minor problem, older platforms
> require clocks, when newer ones do not. When a generic SoC-specific compatible
> is introduced, let say "qcom,ipq4019-qce", it itself requires the clocks,
> but then newer platforms can not be based on this particular compatible,
> otherwise they will require clocks and this comes as invalid.
> 
> How to resolve it properly, shall there be another generic SoC-specific
> compatible without clocks and NOT based on that "qcom,ipq4019-qce" compatible?
> 
> By the way, QCE on SM8150 also shall not need the clocks.

Assuming you have:
1. ipq4019 requiring clocks
2. msm8996 compatible with ipq4019, requiring clocks
3. ipq6018 compatible with ipq4019, not requiring clocks

allOf:
  - if:
      properties:
        compatible:
          enum:
             - ipq4019-qce
    then:
      required:
        - clocks

  - if:
      properties:
        compatible:
          contains:
            enum:
               - msm8996-qce
    then:
      required:
        - clocks

That's not pretty.

Another solution is to make non-clock-requiring variants as their own
family:

1. msm8996-qce, ipq4019-qce
2. sm8550-qce, sm8150-qce

and then in the driver you need two entries - ipq4019 and sm8150.

I like the latter, because for clock-requiring variants your driver
should actually get them and require. For non-clock-requiring variants,
you just skip the clocks (do not fail). Therefore you need different
driver data for these two families.

Best regards,
Krzysztof
Dmitry Baryshkov Feb. 6, 2023, 11:45 p.m. UTC | #2 
On 02/02/2023 18:16, Vladimir Zapolskiy wrote:
> On 2/2/23 16:21, Neil Armstrong wrote:
>> On 02/02/2023 15:04, Vladimir Zapolskiy wrote:
>>> Hi Krzysztof,
>>>
>>> On 2/2/23 15:53, Krzysztof Kozlowski wrote:
>>>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>
>>>>> On certain Snapdragon processors, the crypto engine clocks are 
>>>>> enabled by
>>>>> default by security firmware.
>>>>
>>>> Then probably we should not require them only on these variants.
>>>
>>> I don't have the exact list of the affected SoCs, I believe Neil can 
>>> provide
>>> such a list, if you find it crucial.
>>
>> It's the case for SM8350, SM8450 & SM8550.
>>
> 
> On SM8250 there is no QCE clocks also, so I'll add it to the list, and I 
> hope
> that now the list is complete.
> 
> It could be that the relevant platforms are the ones with 
> 'qcom,no-clock-support'
> property of QCE in the downstream.
> 

Then, sc7180, sc8180x, sdx55, sm6150, sm7150, sm8150 also have this 
property in QCE device. And, I think, it should also be applicable to 
sc7280 and sc8280xp.
Vladimir Zapolskiy Feb. 10, 2023, 11:12 a.m. UTC | #3 
On 2/7/23 01:45, Dmitry Baryshkov wrote:
> On 02/02/2023 18:16, Vladimir Zapolskiy wrote:
>> On 2/2/23 16:21, Neil Armstrong wrote:
>>> On 02/02/2023 15:04, Vladimir Zapolskiy wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 2/2/23 15:53, Krzysztof Kozlowski wrote:
>>>>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>>
>>>>>> On certain Snapdragon processors, the crypto engine clocks are
>>>>>> enabled by
>>>>>> default by security firmware.
>>>>>
>>>>> Then probably we should not require them only on these variants.
>>>>
>>>> I don't have the exact list of the affected SoCs, I believe Neil can
>>>> provide
>>>> such a list, if you find it crucial.
>>>
>>> It's the case for SM8350, SM8450 & SM8550.
>>>
>>
>> On SM8250 there is no QCE clocks also, so I'll add it to the list, and I
>> hope
>> that now the list is complete.
>>
>> It could be that the relevant platforms are the ones with
>> 'qcom,no-clock-support'
>> property of QCE in the downstream.
>>
> 
> Then, sc7180, sc8180x, sdx55, sm6150, sm7150, sm8150 also have this
> property in QCE device. And, I think, it should also be applicable to
> sc7280 and sc8280xp.

So maybe do you have a better candidate among the SoCs for a QCE IP family
name than SM8150 based? Likely it could be the first released SoC among
mentioned above.

--
Best wishes,
Vladimir
Krzysztof Kozlowski Feb. 10, 2023, 12:03 p.m. UTC | #4 
On 10/02/2023 12:12, Vladimir Zapolskiy wrote:
> On 2/7/23 01:45, Dmitry Baryshkov wrote:
>> On 02/02/2023 18:16, Vladimir Zapolskiy wrote:
>>> On 2/2/23 16:21, Neil Armstrong wrote:
>>>> On 02/02/2023 15:04, Vladimir Zapolskiy wrote:
>>>>> Hi Krzysztof,
>>>>>
>>>>> On 2/2/23 15:53, Krzysztof Kozlowski wrote:
>>>>>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>>>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>>>
>>>>>>> On certain Snapdragon processors, the crypto engine clocks are
>>>>>>> enabled by
>>>>>>> default by security firmware.
>>>>>>
>>>>>> Then probably we should not require them only on these variants.
>>>>>
>>>>> I don't have the exact list of the affected SoCs, I believe Neil can
>>>>> provide
>>>>> such a list, if you find it crucial.
>>>>
>>>> It's the case for SM8350, SM8450 & SM8550.
>>>>
>>>
>>> On SM8250 there is no QCE clocks also, so I'll add it to the list, and I
>>> hope
>>> that now the list is complete.
>>>
>>> It could be that the relevant platforms are the ones with
>>> 'qcom,no-clock-support'
>>> property of QCE in the downstream.
>>>
>>
>> Then, sc7180, sc8180x, sdx55, sm6150, sm7150, sm8150 also have this
>> property in QCE device. And, I think, it should also be applicable to
>> sc7280 and sc8280xp.
> 
> So maybe do you have a better candidate among the SoCs for a QCE IP family
> name than SM8150 based? Likely it could be the first released SoC among
> mentioned above.

If you have access to the docs, you will see clear mapping of version to
the SoCs. Just choose the oldest SoC from the list (or something looking
as the oldest - there is no need to be very accurate).


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index 4e00e7925fed..a159089e8a6a 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -59,8 +59,6 @@  properties:
 required:
   - compatible
   - reg
-  - clocks
-  - clock-names
   - dmas
   - dma-names