| Message ID | 20230811-topic-8450_prng-v1-1-01becceeb1ee@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Introduce PRNG on SM8450 | expand |
On Fri, Aug 11, 2023 at 10:50:56PM +0200, Konrad Dybcio wrote: > SM8450's PRNG does not require a core clock reference. Add a new > compatible with a qcom,prng-ee fallback and handle that. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > --- > .../devicetree/bindings/crypto/qcom,prng.yaml | 24 +++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/crypto/qcom,prng.yaml b/Documentation/devicetree/bindings/crypto/qcom,prng.yaml > index bb42f4588b40..36b0ebd9a44b 100644 > --- a/Documentation/devicetree/bindings/crypto/qcom,prng.yaml > +++ b/Documentation/devicetree/bindings/crypto/qcom,prng.yaml > @@ -11,9 +11,13 @@ maintainers: > > properties: > compatible: > - enum: > - - qcom,prng # 8916 etc. > - - qcom,prng-ee # 8996 and later using EE > + oneOf: > + - enum: > + - qcom,prng # 8916 etc. > + - qcom,prng-ee # 8996 and later using EE > + - items: > + - const: qcom,sm8450-prng-ee > + - const: qcom,prng-ee > > reg: > maxItems: 1 > @@ -28,8 +32,18 @@ properties: > required: > - compatible > - reg > - - clocks > - - clock-names > + > +allOf: > + - if: > + not: > + properties: > + compatible: > + contains: > + const: qcom,sm8450-prng-ee > + then: > + required: > + - clocks > + - clock-names > > additionalProperties: false > > > -- > 2.41.0 >
Instead of having SoC name "qcom,sm8450-prng-ee" we could use "qcom,rng-ee" as new IP core is not longer pseudo random number generator. so "prng" can be changed to "rng". Clock configuration is not needed on sm8550 as well. So it is better to use generic compatible string.
On 18/08/2023 18:17, Om Prakash Singh wrote: > Instead of having SoC name "qcom,sm8450-prng-ee" we could use "qcom,rng-ee" as > new IP core is not longer pseudo random number generator. so "prng" can be > changed to "rng". Clock configuration is not needed on sm8550 as well. So it is > better to use generic compatible string. I am not sure if I understand your point. You mean drop "p" in "prng" or drop specific compatible? The first depends in the block - if it is still pseudo. The second - why? That's contradictory to what is in the guidelines and what we have been pushing for very long time. Going against guidelines would require proper justification (and not some usual justification "I don't need it", because we talked about this many many times). One should not bring downstream poor practices to upstream, but the other way. You should fix downstream code. Best regards, Krzysztof
I meant first one. using "qcom,rng-ee". I am looking for generic compatible string for all SoCs for which core clock can be optional, same as we have "qcom,prng-ee". If we are using SoC name in compatible string, for each SoC support we need to update qcom,prng.yaml file. Please suggest approach that we can followed! Thanks, Om On 8/19/2023 1:15 PM, Krzysztof Kozlowski wrote: > On 18/08/2023 18:17, Om Prakash Singh wrote: >> Instead of having SoC name "qcom,sm8450-prng-ee" we could use "qcom,rng-ee" as >> new IP core is not longer pseudo random number generator. so "prng" can be >> changed to "rng". Clock configuration is not needed on sm8550 as well. So it is >> better to use generic compatible string. > > I am not sure if I understand your point. You mean drop "p" in "prng" or > drop specific compatible? The first depends in the block - if it is > still pseudo. The second - why? That's contradictory to what is in the > guidelines and what we have been pushing for very long time. Going > against guidelines would require proper justification (and not some > usual justification "I don't need it", because we talked about this many > many times). One should not bring downstream poor practices to upstream, > but the other way. You should fix downstream code. > > Best regards, > Krzysztof >
On 21/08/2023 02:52, Om Prakash Singh wrote: > I meant first one. using "qcom,rng-ee". Then please provide some reasons. > > I am looking for generic compatible string for all SoCs for which core > clock can be optional, same as we have "qcom,prng-ee". There is a generic compatible already... but anyway, is the clock really optional? Or just configured by firmware? > > If we are using SoC name in compatible string, for each SoC support we > need to update qcom,prng.yaml file. So you were talking about second case from my email? Still not sure what you want to propose, but just in case - please always follow DT bindings guidelines: https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 Best regards, Krzysztof
On 8/21/2023 11:37 AM, Krzysztof Kozlowski wrote: > On 21/08/2023 02:52, Om Prakash Singh wrote: >> I meant first one. using "qcom,rng-ee". > > Then please provide some reasons. > New IP block available on SM8450 and newer platform is true random number generator with it's entropy source. Also it is NIST SP800 90B compliant. By introducing "qcom,rng-ee" I am also planning to add hwrng support in driver. >> >> I am looking for generic compatible string for all SoCs for which core >> clock can be optional, same as we have "qcom,prng-ee". > > There is a generic compatible already... but anyway, is the clock really > optional? Or just configured by firmware? > Clock is configured using security firmware. >> >> If we are using SoC name in compatible string, for each SoC support we >> need to update qcom,prng.yaml file. > > So you were talking about second case from my email? Still not sure what > you want to propose, but just in case - please always follow DT bindings > guidelines: > > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/crypto/qcom,prng.yaml b/Documentation/devicetree/bindings/crypto/qcom,prng.yaml index bb42f4588b40..36b0ebd9a44b 100644 --- a/Documentation/devicetree/bindings/crypto/qcom,prng.yaml +++ b/Documentation/devicetree/bindings/crypto/qcom,prng.yaml @@ -11,9 +11,13 @@ maintainers: properties: compatible: - enum: - - qcom,prng # 8916 etc. - - qcom,prng-ee # 8996 and later using EE + oneOf: + - enum: + - qcom,prng # 8916 etc. + - qcom,prng-ee # 8996 and later using EE + - items: + - const: qcom,sm8450-prng-ee + - const: qcom,prng-ee reg: maxItems: 1 @@ -28,8 +32,18 @@ properties: required: - compatible - reg - - clocks - - clock-names + +allOf: + - if: + not: + properties: + compatible: + contains: + const: qcom,sm8450-prng-ee + then: + required: + - clocks + - clock-names additionalProperties: false
SM8450's PRNG does not require a core clock reference. Add a new compatible with a qcom,prng-ee fallback and handle that. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- .../devicetree/bindings/crypto/qcom,prng.yaml | 24 +++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)