| Message ID | 20230202135036.2635376-6-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | crypto: qcom-qce: Add YAML bindings & support for newer SoCs | expand |
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. Best regards, Krzysztof
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. -- Best wishes, Vladimir
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. Neil > > -- > Best wishes, > Vladimir
On 02/02/2023 15: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. So let's keep them required for explicit list of compatibles (older devices). Best regards, Krzysztof
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.
On 02/02/2023 17: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. > Yes this is what I figured out with the 5.10 device-trees I have checkouted. Neil
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. -- Best wishes, Vladimir
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
Hi Krzysztof, On 2/3/23 10:17, Krzysztof Kozlowski wrote: > 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. many thanks for the detailed explanation, the first of two solutions will be even more clumsy and convoluted, since there should be lists split into two baskets. Thus I will go with the second variant and add two family compatibles. -- Best wishes, Vladimir
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.
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
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 --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