2022-12-31 22:03:29

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 0/3] crypto: Allwinner D1 crypto support

This series finishes adding crypto support for the Allwinner D1 SoC. The
driver patch from v1 was merged, but not the binding[1]. This turned out
to be a good thing, because I later found that the TRNG needed another
clock reference in the devicetree. That is fixed in v2. I include the DT
update here too, since the SoC DT has been on the list for a while[2].

[1]: https://lore.kernel.org/all/[email protected]/T/
[2]: https://lore.kernel.org/lkml/[email protected]/

Changes in v2:
- Add TRNG clock

Samuel Holland (3):
dt-bindings: crypto: sun8i-ce: Add compatible for D1
crypto: sun8i-ce - Add TRNG clock to the D1 variant
riscv: dts: allwinner: d1: Add crypto engine node

.../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
.../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 +++++++
.../crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 1 +
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 2 +-
4 files changed, 39 insertions(+), 9 deletions(-)

--
2.37.4


2022-12-31 22:03:34

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

D1 has a crypto engine similar to the one in other Allwinner SoCs.
Like H6, it has a separate MBUS clock gate.

It also requires the internal RC oscillator to be enabled for the TRNG
to return data, presumably because noise from the oscillator is used as
an entropy source. This is likely the case for earlier variants as well,
but it really only matters for H616 and newer SoCs, as H6 provides no
way to disable the internal oscillator.

Signed-off-by: Samuel Holland <[email protected]>
---
I noticed that the vendor driver has code to explicitly enable IOSC when
using the TRNG on A83T (search SS_TRNG_OSC_ADDR), but that is covered by
a different binding/driver in mainline.

Changes in v2:
- Add TRNG clock

.../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml
index 026a9f9e1aeb..4287678aa79f 100644
--- a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml
+++ b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml
@@ -14,6 +14,7 @@ properties:
enum:
- allwinner,sun8i-h3-crypto
- allwinner,sun8i-r40-crypto
+ - allwinner,sun20i-d1-crypto
- allwinner,sun50i-a64-crypto
- allwinner,sun50i-h5-crypto
- allwinner,sun50i-h6-crypto
@@ -29,6 +30,7 @@ properties:
- description: Bus clock
- description: Module clock
- description: MBus clock
+ - description: TRNG clock (RC oscillator)
minItems: 2

clock-names:
@@ -36,6 +38,7 @@ properties:
- const: bus
- const: mod
- const: ram
+ - const: trng
minItems: 2

resets:
@@ -44,19 +47,33 @@ properties:
if:
properties:
compatible:
- const: allwinner,sun50i-h6-crypto
+ enum:
+ - allwinner,sun20i-d1-crypto
then:
properties:
clocks:
- minItems: 3
+ minItems: 4
clock-names:
- minItems: 3
+ minItems: 4
else:
- properties:
- clocks:
- maxItems: 2
- clock-names:
- maxItems: 2
+ if:
+ properties:
+ compatible:
+ const: allwinner,sun50i-h6-crypto
+ then:
+ properties:
+ clocks:
+ minItems: 3
+ maxItems: 3
+ clock-names:
+ minItems: 3
+ maxItems: 3
+ else:
+ properties:
+ clocks:
+ maxItems: 2
+ clock-names:
+ maxItems: 2

required:
- compatible
--
2.37.4

2022-12-31 22:03:36

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 2/3] crypto: sun8i-ce - Add TRNG clock to the D1 variant

At least the D1 variant requires a separate clock for the TRNG.
Without this clock enabled, reading from /dev/hwrng reports:

sun8i-ce 3040000.crypto: DMA timeout for TRNG (tm=96) on flow 3

Experimentation shows that the necessary clock is the SoC's internal
RC oscillator. This makes sense, as noise from the oscillator can be
used as a source of entropy.

Signed-off-by: Samuel Holland <[email protected]>
---

Changes in v2:
- New patch for v2

drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 1 +
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index 9f6594699835..a6865ff4d400 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -118,6 +118,7 @@ static const struct ce_variant ce_d1_variant = {
{ "bus", 0, 200000000 },
{ "mod", 300000000, 0 },
{ "ram", 0, 400000000 },
+ { "trng", 0, 0 },
},
.esr = ESR_D1,
.prng = CE_ALG_PRNG,
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 8177aaba4434..27029fb77e29 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -105,7 +105,7 @@

#define MAX_SG 8

-#define CE_MAX_CLOCKS 3
+#define CE_MAX_CLOCKS 4

#define MAXFLOW 4

--
2.37.4

2023-01-01 16:14:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

On 31/12/2022 23:01, Samuel Holland wrote:
> D1 has a crypto engine similar to the one in other Allwinner SoCs.
> Like H6, it has a separate MBUS clock gate.
>
> It also requires the internal RC oscillator to be enabled for the TRNG
> to return data, presumably because noise from the oscillator is used as
> an entropy source. This is likely the case for earlier variants as well,
> but it really only matters for H616 and newer SoCs, as H6 provides no
> way to disable the internal oscillator.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---


Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-01-05 16:33:02

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] crypto: sun8i-ce - Add TRNG clock to the D1 variant

Dne sobota, 31. december 2022 ob 23:01:44 CET je Samuel Holland napisal(a):
> At least the D1 variant requires a separate clock for the TRNG.
> Without this clock enabled, reading from /dev/hwrng reports:
>
> sun8i-ce 3040000.crypto: DMA timeout for TRNG (tm=96) on flow 3
>
> Experimentation shows that the necessary clock is the SoC's internal
> RC oscillator. This makes sense, as noise from the oscillator can be
> used as a source of entropy.
>
> Signed-off-by: Samuel Holland <[email protected]>

Reviewed-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2023-01-06 08:24:25

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] crypto: sun8i-ce - Add TRNG clock to the D1 variant

Le Sat, Dec 31, 2022 at 04:01:44PM -0600, Samuel Holland a ?crit :
> At least the D1 variant requires a separate clock for the TRNG.
> Without this clock enabled, reading from /dev/hwrng reports:
>
> sun8i-ce 3040000.crypto: DMA timeout for TRNG (tm=96) on flow 3
>
> Experimentation shows that the necessary clock is the SoC's internal
> RC oscillator. This makes sense, as noise from the oscillator can be
> used as a source of entropy.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>

Acked-by: Corentin Labbe <[email protected]>

Thanks

2023-01-06 08:25:05

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

Le Sat, Dec 31, 2022 at 04:01:43PM -0600, Samuel Holland a ?crit :
> D1 has a crypto engine similar to the one in other Allwinner SoCs.
> Like H6, it has a separate MBUS clock gate.
>
> It also requires the internal RC oscillator to be enabled for the TRNG
> to return data, presumably because noise from the oscillator is used as
> an entropy source. This is likely the case for earlier variants as well,
> but it really only matters for H616 and newer SoCs, as H6 provides no
> way to disable the internal oscillator.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> I noticed that the vendor driver has code to explicitly enable IOSC when
> using the TRNG on A83T (search SS_TRNG_OSC_ADDR), but that is covered by
> a different binding/driver in mainline.
>
> Changes in v2:
> - Add TRNG clock
>
> .../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
> 1 file changed, 25 insertions(+), 8 deletions(-)
>

Acked-by: Corentin Labbe <[email protected]>

Thanks

2023-01-13 03:54:59

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

On Sat, Dec 31, 2022 at 04:01:43PM -0600, Samuel Holland wrote:
> D1 has a crypto engine similar to the one in other Allwinner SoCs.
> Like H6, it has a separate MBUS clock gate.
>
> It also requires the internal RC oscillator to be enabled for the TRNG
> to return data, presumably because noise from the oscillator is used as
> an entropy source. This is likely the case for earlier variants as well,
> but it really only matters for H616 and newer SoCs, as H6 provides no
> way to disable the internal oscillator.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> I noticed that the vendor driver has code to explicitly enable IOSC when
> using the TRNG on A83T (search SS_TRNG_OSC_ADDR), but that is covered by
> a different binding/driver in mainline.
>
> Changes in v2:
> - Add TRNG clock
>
> .../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
> 1 file changed, 25 insertions(+), 8 deletions(-)

This doesn't have an ack from Rob Herring. Would you like me
to apply just the crypto patch by itself?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-01-13 08:39:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

On 13/01/2023 04:51, Herbert Xu wrote:
> On Sat, Dec 31, 2022 at 04:01:43PM -0600, Samuel Holland wrote:
>> D1 has a crypto engine similar to the one in other Allwinner SoCs.
>> Like H6, it has a separate MBUS clock gate.
>>
>> It also requires the internal RC oscillator to be enabled for the TRNG
>> to return data, presumably because noise from the oscillator is used as
>> an entropy source. This is likely the case for earlier variants as well,
>> but it really only matters for H616 and newer SoCs, as H6 provides no
>> way to disable the internal oscillator.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>> I noticed that the vendor driver has code to explicitly enable IOSC when
>> using the TRNG on A83T (search SS_TRNG_OSC_ADDR), but that is covered by
>> a different binding/driver in mainline.
>>
>> Changes in v2:
>> - Add TRNG clock
>>
>> .../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
>> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> This doesn't have an ack from Rob Herring. Would you like me
> to apply just the crypto patch by itself?

But it has my Reviewed-by, which is equivalent. Please take it via
crypto with the driver change.

Best regards,
Krzysztof

2023-01-13 08:42:01

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: crypto: sun8i-ce: Add compatible for D1

On Fri, Jan 13, 2023 at 09:33:32AM +0100, Krzysztof Kozlowski wrote:
>
> But it has my Reviewed-by, which is equivalent. Please take it via
> crypto with the driver change.

Thanks for the clarification. I'll take patches 1+2 then. Patch
3 doesn't apply to cryptodev as the file doesn't exist in my tree.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-01-13 08:42:25

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] crypto: Allwinner D1 crypto support

On Sat, Dec 31, 2022 at 04:01:42PM -0600, Samuel Holland wrote:
> This series finishes adding crypto support for the Allwinner D1 SoC. The
> driver patch from v1 was merged, but not the binding[1]. This turned out
> to be a good thing, because I later found that the TRNG needed another
> clock reference in the devicetree. That is fixed in v2. I include the DT
> update here too, since the SoC DT has been on the list for a while[2].
>
> [1]: https://lore.kernel.org/all/[email protected]/T/
> [2]: https://lore.kernel.org/lkml/[email protected]lland.org/
>
> Changes in v2:
> - Add TRNG clock
>
> Samuel Holland (3):
> dt-bindings: crypto: sun8i-ce: Add compatible for D1
> crypto: sun8i-ce - Add TRNG clock to the D1 variant
> riscv: dts: allwinner: d1: Add crypto engine node
>
> .../bindings/crypto/allwinner,sun8i-ce.yaml | 33 ++++++++++++++-----
> .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 +++++++
> .../crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 1 +
> drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 2 +-
> 4 files changed, 39 insertions(+), 9 deletions(-)
>
> --
> 2.37.4

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