[PATCH 0/3] Introduce PRNG on SM8450

[PATCH 0/3] Introduce PRNG on SM8450

[Konrad Dybcio]

SM8450's PRNG seems to be the same good ol' IP, except without a core
clock.

For a lack of a better idea on how to test it, /proc/crypto reports that
the selftest has gone through..

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (3):
      dt-bindings: crypto: qcom,prng: Add SM8450
      crypto: qcom-rng: Make the core clock optional regardless of ACPI presence
      arm64: dts: qcom: sm8450: Add PRNG

 .../devicetree/bindings/crypto/qcom,prng.yaml      | 24 +++++++++++++++++-----
 arch/arm64/boot/dts/qcom/sm8450.dtsi               |  5 +++++
 drivers/crypto/qcom-rng.c                          | 10 +++------
 3 files changed, 27 insertions(+), 12 deletions(-)
---
base-commit: 21ef7b1e17d039053edaeaf41142423810572741
change-id: 20230811-topic-8450_prng-6af00873db4d

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>

[PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Konrad Dybcio]

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(-)

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

[PATCH 2/3] crypto: qcom-rng: Make the core clock optional regardless of ACPI presence

[Konrad Dybcio]

Some newer SoCs (like SM8450) do not require a clock vote for the PRNG
to function. Make it entirely optional and rely on the bindings checker
to ensure platforms that need it, consume one.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/crypto/qcom-rng.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index 72dd1a4ebac4..825a729f205e 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -173,13 +173,9 @@ static int qcom_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->base))
 		return PTR_ERR(rng->base);
 
-	/* ACPI systems have clk already on, so skip clk_get */
-	if (!has_acpi_companion(&pdev->dev)) {
-		rng->clk = devm_clk_get(&pdev->dev, "core");
-		if (IS_ERR(rng->clk))
-			return PTR_ERR(rng->clk);
-	}
-
+	rng->clk = devm_clk_get_optional(&pdev->dev, "core");
+	if (IS_ERR(rng->clk))
+		return PTR_ERR(rng->clk);
 
 	rng->skip_init = (unsigned long)device_get_match_data(&pdev->dev);
 

-- 
2.41.0

[PATCH 3/3] arm64: dts: qcom: sm8450: Add PRNG

[Konrad Dybcio]

Add the Qualcomm Pseudo-Random Number Generator.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 4bc1c46a5f44..48618e88a273 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -1738,6 +1738,11 @@ spi14: spi@a98000 {
 			};
 		};
 
+		rng: rng@10c3000 {
+			compatible = "qcom,sm8450-prng-ee", "qcom,prng-ee";
+			reg = <0 0x010c3000 0 0x1000>;
+		};
+
 		pcie0: pci@1c00000 {
 			compatible = "qcom,pcie-sm8450-pcie0";
 			reg = <0 0x01c00000 0 0x3000>,

-- 
2.41.0

Re: [PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/3] crypto: qcom-rng: Make the core clock optional regardless of ACPI presence

[Bjorn Andersson]

On Fri, Aug 11, 2023 at 10:50:57PM +0200, Konrad Dybcio wrote:
> Some newer SoCs (like SM8450) do not require a clock vote for the PRNG
> to function. Make it entirely optional and rely on the bindings checker
> to ensure platforms that need it, consume one.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

Re: (subset) [PATCH 0/3] Introduce PRNG on SM8450

[Bjorn Andersson]

On Fri, 11 Aug 2023 22:50:55 +0200, Konrad Dybcio wrote:
> SM8450's PRNG seems to be the same good ol' IP, except without a core
> clock.
> 
> For a lack of a better idea on how to test it, /proc/crypto reports that
> the selftest has gone through..
> 
> 
> [...]

Applied, thanks!

[3/3] arm64: dts: qcom: sm8450: Add PRNG
      commit: 76a6dd7bfcbb91217e4429d73c14ee67b441137d

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH 0/3] Introduce PRNG on SM8450

[Herbert Xu]

On Fri, Aug 11, 2023 at 10:50:55PM +0200, Konrad Dybcio wrote:
> SM8450's PRNG seems to be the same good ol' IP, except without a core
> clock.
> 
> For a lack of a better idea on how to test it, /proc/crypto reports that
> the selftest has gone through..
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Konrad Dybcio (3):
>       dt-bindings: crypto: qcom,prng: Add SM8450
>       crypto: qcom-rng: Make the core clock optional regardless of ACPI presence
>       arm64: dts: qcom: sm8450: Add PRNG
> 
>  .../devicetree/bindings/crypto/qcom,prng.yaml      | 24 +++++++++++++++++-----
>  arch/arm64/boot/dts/qcom/sm8450.dtsi               |  5 +++++
>  drivers/crypto/qcom-rng.c                          | 10 +++------
>  3 files changed, 27 insertions(+), 12 deletions(-)
> ---
> base-commit: 21ef7b1e17d039053edaeaf41142423810572741
> change-id: 20230811-topic-8450_prng-6af00873db4d

Patches 1-2 applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Om Prakash Singh]

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.

Re: [PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Krzysztof Kozlowski]

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

Re: [PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Om Prakash Singh]

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
> 

Re: [PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Krzysztof Kozlowski]

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

Re: [PATCH 1/3] dt-bindings: crypto: qcom,prng: Add SM8450

[Om Prakash Singh]

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
>