[PATCH 0/4] Add support for stm32mp25x RNG

[PATCH 0/4] Add support for stm32mp25x RNG

[Gatien Chevallier]

This patchset adds support for the Random Number
Generator(RNG) present on the stm32mp25x platforms.
On these platforms, the clock management and the RNG
parameters are different.

While there, update the RNG max clock frequency on
stm32mp15 platforms according to the latest specs.

Tested on the stm32mp257f-ev1 platform with a deep
power sequence with rngtest before/after the sequence with
satisfying results.

Same was done on stm32mp135f-dk to make sure no regression was added.

On stm32mp157c-dk2, I didn't perform a power sequence but the rngtest
results were satisfying.

Gatien Chevallier (4):
  dt-bindings: rng: add st,stm32mp25-rng support
  hwrng: stm32 - implement support for STM32MP25x platforms
  hwrng: stm32 - update STM32MP15 RNG max clock frequency
  arm64: dts: st: add RNG node on stm32mp251

 .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 ++++++++-
 arch/arm64/boot/dts/st/stm32mp251.dtsi        | 10 +++
 drivers/char/hw_random/stm32-rng.c            | 83 +++++++++++++++++--
 3 files changed, 126 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Gatien Chevallier]

Add RNG STM32MP25x platforms compatible. Update the clock
properties management to support all versions.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
index 340d01d481d1..c92ce92b6ac9 100644
--- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
+++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
@@ -18,12 +18,19 @@ properties:
     enum:
       - st,stm32-rng
       - st,stm32mp13-rng
+      - st,stm32mp25-rng
 
   reg:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: rng_clk
+      - const: rng_hclk
 
   resets:
     maxItems: 1
@@ -57,15 +64,43 @@ allOf:
       properties:
         st,rng-lock-conf: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp25-rng
+    then:
+      properties:
+        clocks:
+          description: >
+            RNG bus clock must be named "rng_hclk". The RNG kernel clock
+            must be named "rng_clk".
+          maxItems: 2
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+
 additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/clock/stm32mp1-clks.h>
     rng@54003000 {
       compatible = "st,stm32-rng";
       reg = <0x54003000 0x400>;
-      clocks = <&rcc RNG1_K>;
+      clocks = <&rcc 124>;
     };
 
+  - |
+    rng: rng@42020000 {
+      compatible = "st,stm32mp25-rng";
+      reg = <0x42020000 0x400>;
+      clocks = <&clk_rcbsec>, <&rcc 110>;
+      clock-names = "rng_clk", "rng_hclk";
+      resets = <&rcc 97>;
+      access-controllers = <&rifsc 92>;
+    };
 ...
-- 
2.25.1

[PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Gatien Chevallier]

Implement the support for STM32MP25x platforms. On this platform, a
security clock is shared between some hardware blocks. For the RNG,
it is the RNG kernel clock. Therefore, the gate is no more shared
between the RNG bus and kernel clocks as on STM32MP1x platforms and
the bus clock has to be managed on its own.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 drivers/char/hw_random/stm32-rng.c | 81 ++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 9d041a67c295..e7051005768d 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -49,6 +49,7 @@
 
 struct stm32_rng_data {
 	uint	max_clock_rate;
+	uint	nb_clock;
 	u32	cr;
 	u32	nscr;
 	u32	htcr;
@@ -73,6 +74,7 @@ struct stm32_rng_private {
 	struct device *dev;
 	void __iomem *base;
 	struct clk *clk;
+	struct clk *bus_clk;
 	struct reset_control *rst;
 	struct stm32_rng_config pm_conf;
 	const struct stm32_rng_data *data;
@@ -292,6 +294,14 @@ static int stm32_rng_init(struct hwrng *rng)
 	if (err)
 		return err;
 
+	if (priv->bus_clk) {
+		err = clk_prepare_enable(priv->bus_clk);
+		if (err) {
+			clk_disable_unprepare(priv->clk);
+			return err;
+		}
+	}
+
 	/* clear error indicators */
 	writel_relaxed(0, priv->base + RNG_SR);
 
@@ -329,6 +339,8 @@ static int stm32_rng_init(struct hwrng *rng)
 							10, 50000);
 		if (err) {
 			clk_disable_unprepare(priv->clk);
+			if (priv->bus_clk)
+				clk_disable_unprepare(priv->bus_clk);
 			dev_err(priv->dev, "%s: timeout %x!\n", __func__, reg);
 			return -EINVAL;
 		}
@@ -356,8 +368,11 @@ static int stm32_rng_init(struct hwrng *rng)
 						reg & RNG_SR_DRDY,
 						10, 100000);
 	if (err || (reg & ~RNG_SR_DRDY)) {
+		if (priv->bus_clk)
+			clk_disable_unprepare(priv->bus_clk);
 		clk_disable_unprepare(priv->clk);
 		dev_err(priv->dev, "%s: timeout:%x SR: %x!\n", __func__, err, reg);
+
 		return -EINVAL;
 	}
 
@@ -379,6 +394,9 @@ static int __maybe_unused stm32_rng_runtime_suspend(struct device *dev)
 	reg = readl_relaxed(priv->base + RNG_CR);
 	reg &= ~RNG_CR_RNGEN;
 	writel_relaxed(reg, priv->base + RNG_CR);
+
+	if (priv->bus_clk)
+		clk_disable_unprepare(priv->bus_clk);
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
@@ -393,6 +411,14 @@ static int __maybe_unused stm32_rng_suspend(struct device *dev)
 	if (err)
 		return err;
 
+	if (priv->bus_clk) {
+		err = clk_prepare_enable(priv->bus_clk);
+		if (err) {
+			clk_disable_unprepare(priv->clk);
+			return err;
+		}
+	}
+
 	if (priv->data->has_cond_reset) {
 		priv->pm_conf.nscr = readl_relaxed(priv->base + RNG_NSCR);
 		priv->pm_conf.htcr = readl_relaxed(priv->base + RNG_HTCR);
@@ -403,6 +429,8 @@ static int __maybe_unused stm32_rng_suspend(struct device *dev)
 
 	writel_relaxed(priv->pm_conf.cr, priv->base + RNG_CR);
 
+	if (priv->bus_clk)
+		clk_disable_unprepare(priv->bus_clk);
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
@@ -418,6 +446,14 @@ static int __maybe_unused stm32_rng_runtime_resume(struct device *dev)
 	if (err)
 		return err;
 
+	if (priv->bus_clk) {
+		err = clk_prepare_enable(priv->bus_clk);
+		if (err) {
+			clk_disable_unprepare(priv->clk);
+			return err;
+		}
+	}
+
 	/* Clean error indications */
 	writel_relaxed(0, priv->base + RNG_SR);
 
@@ -438,6 +474,14 @@ static int __maybe_unused stm32_rng_resume(struct device *dev)
 	if (err)
 		return err;
 
+	if (priv->bus_clk) {
+		err = clk_prepare_enable(priv->bus_clk);
+		if (err) {
+			clk_disable_unprepare(priv->clk);
+			return err;
+		}
+	}
+
 	/* Clean error indications */
 	writel_relaxed(0, priv->base + RNG_SR);
 
@@ -462,6 +506,8 @@ static int __maybe_unused stm32_rng_resume(struct device *dev)
 							reg & ~RNG_CR_CONDRST, 10, 100000);
 
 		if (err) {
+			if (priv->bus_clk)
+				clk_disable_unprepare(priv->bus_clk);
 			clk_disable_unprepare(priv->clk);
 			dev_err(priv->dev, "%s: timeout:%x CR: %x!\n", __func__, err, reg);
 			return -EINVAL;
@@ -473,6 +519,8 @@ static int __maybe_unused stm32_rng_resume(struct device *dev)
 	}
 
 	clk_disable_unprepare(priv->clk);
+	if (priv->bus_clk)
+		clk_disable_unprepare(priv->bus_clk);
 
 	return 0;
 }
@@ -484,9 +532,19 @@ static const struct dev_pm_ops __maybe_unused stm32_rng_pm_ops = {
 				stm32_rng_resume)
 };
 
+static const struct stm32_rng_data stm32mp25_rng_data = {
+	.has_cond_reset = true,
+	.max_clock_rate = 48000000,
+	.nb_clock = 2,
+	.cr = 0x00F00D00,
+	.nscr = 0x2B5BB,
+	.htcr = 0x969D,
+};
+
 static const struct stm32_rng_data stm32mp13_rng_data = {
 	.has_cond_reset = true,
 	.max_clock_rate = 48000000,
+	.nb_clock = 1,
 	.cr = 0x00F00D00,
 	.nscr = 0x2B5BB,
 	.htcr = 0x969D,
@@ -495,9 +553,14 @@ static const struct stm32_rng_data stm32mp13_rng_data = {
 static const struct stm32_rng_data stm32_rng_data = {
 	.has_cond_reset = false,
 	.max_clock_rate = 3000000,
+	.nb_clock = 1,
 };
 
 static const struct of_device_id stm32_rng_match[] = {
+	{
+		.compatible = "st,stm32mp25-rng",
+		.data = &stm32mp25_rng_data,
+	},
 	{
 		.compatible = "st,stm32mp13-rng",
 		.data = &stm32mp13_rng_data,
@@ -525,10 +588,6 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	priv->clk = devm_clk_get(&ofdev->dev, NULL);
-	if (IS_ERR(priv->clk))
-		return PTR_ERR(priv->clk);
-
 	priv->rst = devm_reset_control_get(&ofdev->dev, NULL);
 	if (!IS_ERR(priv->rst)) {
 		reset_control_assert(priv->rst);
@@ -551,6 +610,20 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 	priv->rng.read = stm32_rng_read;
 	priv->rng.quality = 900;
 
+	if (priv->data->nb_clock > 1) {
+		priv->clk = devm_clk_get(&ofdev->dev, "rng_clk");
+		if (IS_ERR(priv->clk))
+			return PTR_ERR(priv->clk);
+
+		priv->bus_clk = devm_clk_get(&ofdev->dev, "rng_hclk");
+		if (IS_ERR(priv->clk))
+			return PTR_ERR(priv->bus_clk);
+	} else {
+		priv->clk = devm_clk_get(&ofdev->dev, NULL);
+		if (IS_ERR(priv->clk))
+			return PTR_ERR(priv->clk);
+	}
+
 	pm_runtime_set_autosuspend_delay(dev, 100);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_enable(dev);
-- 
2.25.1

[PATCH 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency

[Gatien Chevallier]

RNG max clock frequency can be updated to 48MHz for stm32mp1x
platforms according to the latest specifications.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 drivers/char/hw_random/stm32-rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index e7051005768d..9f1c95218a5b 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -552,7 +552,7 @@ static const struct stm32_rng_data stm32mp13_rng_data = {
 
 static const struct stm32_rng_data stm32_rng_data = {
 	.has_cond_reset = false,
-	.max_clock_rate = 3000000,
+	.max_clock_rate = 48000000,
 	.nb_clock = 1,
 };
 
-- 
2.25.1

[PATCH 4/4] arm64: dts: st: add RNG node on stm32mp251

[Gatien Chevallier]

Update the device-tree stm32mp251.dtsi by adding the Random Number
Generator(RNG) node.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 1167cf63d7e8..40b96353a803 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -493,6 +493,16 @@ uart8: serial@40380000 {
 				status = "disabled";
 			};
 
+			rng: rng@42020000 {
+				compatible = "st,stm32mp25-rng";
+				reg = <0x42020000 0x400>;
+				clocks = <&clk_rcbsec>, <&rcc CK_BUS_RNG>;
+				clock-names = "rng_clk", "rng_hclk";
+				resets = <&rcc RNG_R>;
+				access-controllers = <&rifsc 92>;
+				status = "disabled";
+			};
+
 			spi8: spi@46020000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.25.1

Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Marek Vasut]

On 10/7/24 3:27 PM, Gatien Chevallier wrote:
> Add RNG STM32MP25x platforms compatible. Update the clock
> properties management to support all versions.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> ---
>   .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
>   1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> index 340d01d481d1..c92ce92b6ac9 100644
> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> @@ -18,12 +18,19 @@ properties:
>       enum:
>         - st,stm32-rng
>         - st,stm32mp13-rng
> +      - st,stm32mp25-rng
>   
>     reg:
>       maxItems: 1
>   
>     clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: rng_clk
> +      - const: rng_hclk
>   
>     resets:
>       maxItems: 1
> @@ -57,15 +64,43 @@ allOf:
>         properties:
>           st,rng-lock-conf: false
>   
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp25-rng
Maybe this match should be inverted, it is likely the next generation of 
stm32 will also use 2 input clock into the RNG block and it will be only 
the legacy MP1 that uses one clock.

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Marek Vasut]

On 10/7/24 3:27 PM, Gatien Chevallier wrote:
> Implement the support for STM32MP25x platforms. On this platform, a
> security clock is shared between some hardware blocks. For the RNG,
> it is the RNG kernel clock. Therefore, the gate is no more shared
> between the RNG bus and kernel clocks as on STM32MP1x platforms and
> the bus clock has to be managed on its own.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
A bit of a higher-level design question -- can you use 
drivers/clk/clk-bulk.c clk_bulk_*() to handle all these disparate count 
of clock easily ?

Re: [PATCH 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency

[Marek Vasut]

On 10/7/24 3:27 PM, Gatien Chevallier wrote:
> RNG max clock frequency can be updated to 48MHz for stm32mp1x
> platforms according to the latest specifications.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Does this also apply to MP13 ?

Re: [PATCH 4/4] arm64: dts: st: add RNG node on stm32mp251

[Marek Vasut]

On 10/7/24 3:27 PM, Gatien Chevallier wrote:
> Update the device-tree stm32mp251.dtsi by adding the Random Number
> Generator(RNG) node.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> ---
>   arch/arm64/boot/dts/st/stm32mp251.dtsi | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> index 1167cf63d7e8..40b96353a803 100644
> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> @@ -493,6 +493,16 @@ uart8: serial@40380000 {
>   				status = "disabled";
>   			};
>   
> +			rng: rng@42020000 {
> +				compatible = "st,stm32mp25-rng";
> +				reg = <0x42020000 0x400>;
> +				clocks = <&clk_rcbsec>, <&rcc CK_BUS_RNG>;
> +				clock-names = "rng_clk", "rng_hclk";
> +				resets = <&rcc RNG_R>;
> +				access-controllers = <&rifsc 92>;
It would be good if someone finally sorted the access-controllers 
property in all the MP2 nodes alphabetically ; that's separate 
patch/series though.

Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Gatien CHEVALLIER]

On 10/7/24 15:53, Marek Vasut wrote:
> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>> Add RNG STM32MP25x platforms compatible. Update the clock
>> properties management to support all versions.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> ---
>>   .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
>>   1 file changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml 
>> b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> index 340d01d481d1..c92ce92b6ac9 100644
>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> @@ -18,12 +18,19 @@ properties:
>>       enum:
>>         - st,stm32-rng
>>         - st,stm32mp13-rng
>> +      - st,stm32mp25-rng
>>     reg:
>>       maxItems: 1
>>     clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: rng_clk
>> +      - const: rng_hclk
>>     resets:
>>       maxItems: 1
>> @@ -57,15 +64,43 @@ allOf:
>>         properties:
>>           st,rng-lock-conf: false
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp25-rng
> Maybe this match should be inverted, it is likely the next generation of 
> stm32 will also use 2 input clock into the RNG block and it will be only 
> the legacy MP1 that uses one clock.

Hi, sure, makes more sense, I'll change it for V2.

Gatien

Re: [PATCH 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency

[Gatien CHEVALLIER]

On 10/7/24 15:55, Marek Vasut wrote:
> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>> RNG max clock frequency can be updated to 48MHz for stm32mp1x
>> platforms according to the latest specifications.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> Does this also apply to MP13 ?

Max frequency for MP13 is already 48MHz.

Gatien

Re: [PATCH 4/4] arm64: dts: st: add RNG node on stm32mp251

[Gatien CHEVALLIER]

On 10/7/24 15:55, Marek Vasut wrote:
> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>> Update the device-tree stm32mp251.dtsi by adding the Random Number
>> Generator(RNG) node.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> ---
>>   arch/arm64/boot/dts/st/stm32mp251.dtsi | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi 
>> b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> index 1167cf63d7e8..40b96353a803 100644
>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> @@ -493,6 +493,16 @@ uart8: serial@40380000 {
>>                   status = "disabled";
>>               };
>> +            rng: rng@42020000 {
>> +                compatible = "st,stm32mp25-rng";
>> +                reg = <0x42020000 0x400>;
>> +                clocks = <&clk_rcbsec>, <&rcc CK_BUS_RNG>;
>> +                clock-names = "rng_clk", "rng_hclk";
>> +                resets = <&rcc RNG_R>;
>> +                access-controllers = <&rifsc 92>;
> It would be good if someone finally sorted the access-controllers 
> property in all the MP2 nodes alphabetically ; that's separate 
> patch/series though.

I'll pin your comment to take a look into that in the near future.

Gatien

Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Krzysztof Kozlowski]

On 07/10/2024 15:27, Gatien Chevallier wrote:
> Add RNG STM32MP25x platforms compatible. Update the clock
> properties management to support all versions.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Regardless of the
reason, process needs improvement: please CC correct address.

> ---
>  .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> index 340d01d481d1..c92ce92b6ac9 100644
> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> @@ -18,12 +18,19 @@ properties:
>      enum:
>        - st,stm32-rng
>        - st,stm32mp13-rng
> +      - st,stm32mp25-rng
>  
>    reg:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:

Missing minItems

> +    items:
> +      - const: rng_clk
> +      - const: rng_hclk

Drop _clk and come with some reasonable names, e.g. "core" and "bus"?

>  
>    resets:
>      maxItems: 1
> @@ -57,15 +64,43 @@ allOf:
>        properties:
>          st,rng-lock-conf: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp25-rng
> +    then:
> +      properties:
> +        clocks:
> +          description: >
> +            RNG bus clock must be named "rng_hclk". The RNG kernel clock
> +            must be named "rng_clk".

Drop description, useless.

Missing minItems

> +          maxItems: 2
> +      required:
> +        - clock-names
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 1

Missing constrain for clock-names.

> +
>  additionalProperties: false
>  
>  examples:
>    - |
> -    #include <dt-bindings/clock/stm32mp1-clks.h>

Why?

>      rng@54003000 {
>        compatible = "st,stm32-rng";
>        reg = <0x54003000 0x400>;
> -      clocks = <&rcc RNG1_K>;
> +      clocks = <&rcc 124>;

Why?


>      };
>  
> +  - |
> +    rng: rng@42020000 {
> +      compatible = "st,stm32mp25-rng";
> +      reg = <0x42020000 0x400>;
> +      clocks = <&clk_rcbsec>, <&rcc 110>;
> +      clock-names = "rng_clk", "rng_hclk";
> +      resets = <&rcc 97>;
> +      access-controllers = <&rifsc 92>;


Difference in one property should not need new example, usually.

Best regards,
Krzysztof

Re: [PATCH 4/4] arm64: dts: st: add RNG node on stm32mp251

[Marek Vasut]

On 10/7/24 4:59 PM, Gatien CHEVALLIER wrote:
> 
> 
> On 10/7/24 15:55, Marek Vasut wrote:
>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>> Update the device-tree stm32mp251.dtsi by adding the Random Number
>>> Generator(RNG) node.
>>>
>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>> ---
>>>   arch/arm64/boot/dts/st/stm32mp251.dtsi | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/ 
>>> boot/dts/st/stm32mp251.dtsi
>>> index 1167cf63d7e8..40b96353a803 100644
>>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>> @@ -493,6 +493,16 @@ uart8: serial@40380000 {
>>>                   status = "disabled";
>>>               };
>>> +            rng: rng@42020000 {
>>> +                compatible = "st,stm32mp25-rng";
>>> +                reg = <0x42020000 0x400>;
>>> +                clocks = <&clk_rcbsec>, <&rcc CK_BUS_RNG>;
>>> +                clock-names = "rng_clk", "rng_hclk";
>>> +                resets = <&rcc RNG_R>;
>>> +                access-controllers = <&rifsc 92>;
>> It would be good if someone finally sorted the access-controllers 
>> property in all the MP2 nodes alphabetically ; that's separate patch/ 
>> series though.
> 
> I'll pin your comment to take a look into that in the near future.
Thank you !

Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Gatien CHEVALLIER]

On 10/7/24 17:00, Krzysztof Kozlowski wrote:
> On 07/10/2024 15:27, Gatien Chevallier wrote:
>> Add RNG STM32MP25x platforms compatible. Update the clock
>> properties management to support all versions.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> 
> You CC-ed an address, which suggests you do not work on mainline kernel
> or you do not use get_maintainers.pl/b4/patman. Regardless of the
> reason, process needs improvement: please CC correct address.
> 

Hi,

I'm using get_maintainers.pl so I'll check why I have an issue.

>> ---
>>   .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
>>   1 file changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> index 340d01d481d1..c92ce92b6ac9 100644
>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> @@ -18,12 +18,19 @@ properties:
>>       enum:
>>         - st,stm32-rng
>>         - st,stm32mp13-rng
>> +      - st,stm32mp25-rng
>>   
>>     reg:
>>       maxItems: 1
>>   
>>     clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  clock-names:
> 
> Missing minItems
> 

Ok, will add in V2

>> +    items:
>> +      - const: rng_clk
>> +      - const: rng_hclk
> 
> Drop _clk and come with some reasonable names, e.g. "core" and "bus"?
> 

Sure, makes sense. Will change in V2.

>>   
>>     resets:
>>       maxItems: 1
>> @@ -57,15 +64,43 @@ allOf:
>>         properties:
>>           st,rng-lock-conf: false
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp25-rng
>> +    then:
>> +      properties:
>> +        clocks:
>> +          description: >
>> +            RNG bus clock must be named "rng_hclk". The RNG kernel clock
>> +            must be named "rng_clk".
> 
> Drop description, useless.
> 
> Missing minItems
> 

Ok, will update in V2

>> +          maxItems: 2
>> +      required:
>> +        - clock-names
>> +    else:
>> +      properties:
>> +        clocks:
>> +          maxItems: 1
> 
> Missing constrain for clock-names.
> 
>> +
>>   additionalProperties: false
>>   
>>   examples:
>>     - |
>> -    #include <dt-bindings/clock/stm32mp1-clks.h>
> 
> Why?
> 
>>       rng@54003000 {
>>         compatible = "st,stm32-rng";
>>         reg = <0x54003000 0x400>;
>> -      clocks = <&rcc RNG1_K>;
>> +      clocks = <&rcc 124>;
> 
> Why?
> 
> 

I have an issue with the generated st,stm32-rng.example.dts example.
Because there are 2 binding files included, I have a collision with
clock bindings names between the "dt-bindings/clock/stm32mp1-clks.h"
and the "dt-bindings/clock/st,stm32mp25-rcc.h" files. For example:
CK_MCO1 or CK_SCMI_HSE. I replaced the bindings with numbers
to avoid this conflict.

If you think this binding update does not need the addition of an
example, I'll completely drop it and we won't have the issue.

Best regards,
Gatien

>>       };
>>   
>> +  - |
>> +    rng: rng@42020000 {
>> +      compatible = "st,stm32mp25-rng";
>> +      reg = <0x42020000 0x400>;
>> +      clocks = <&clk_rcbsec>, <&rcc 110>;
>> +      clock-names = "rng_clk", "rng_hclk";
>> +      resets = <&rcc 97>;
>> +      access-controllers = <&rifsc 92>;
> 
> 
> Difference in one property should not need new example, usually.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Krzysztof Kozlowski]

On 07/10/2024 18:22, Gatien CHEVALLIER wrote:
> 
> 
> On 10/7/24 17:00, Krzysztof Kozlowski wrote:
>> On 07/10/2024 15:27, Gatien Chevallier wrote:
>>> Add RNG STM32MP25x platforms compatible. Update the clock
>>> properties management to support all versions.
>>>
>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>
>> You CC-ed an address, which suggests you do not work on mainline kernel
>> or you do not use get_maintainers.pl/b4/patman. Regardless of the
>> reason, process needs improvement: please CC correct address.
>>
> 
> Hi,
> 
> I'm using get_maintainers.pl so I'll check why I have an issue.

If you use get_maintainers.pl and produced this cc-list, then clearly
you work on some old kernel or some weird fork (why weird? because which
fork would change these addresses..).

Don't.

Work on mainline or next.

Best regards,
Krzysztof

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Gatien CHEVALLIER]

On 10/7/24 15:54, Marek Vasut wrote:
> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>> Implement the support for STM32MP25x platforms. On this platform, a
>> security clock is shared between some hardware blocks. For the RNG,
>> it is the RNG kernel clock. Therefore, the gate is no more shared
>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>> the bus clock has to be managed on its own.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> A bit of a higher-level design question -- can you use 
> drivers/clk/clk-bulk.c clk_bulk_*() to handle all these disparate count 
> of clock easily ?

Hi, I'd like to make sure that we enable the core clock before the bus
clock so that the RNG hardware block can start its internal tests while
we ungate the bus clock. It's not a strong opinion but it feels better.

Cheers,
Gatien

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Marek Vasut]

On 10/11/24 11:55 AM, Gatien CHEVALLIER wrote:
> 
> 
> On 10/7/24 15:54, Marek Vasut wrote:
>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>> Implement the support for STM32MP25x platforms. On this platform, a
>>> security clock is shared between some hardware blocks. For the RNG,
>>> it is the RNG kernel clock. Therefore, the gate is no more shared
>>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>>> the bus clock has to be managed on its own.
>>>
>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> A bit of a higher-level design question -- can you use drivers/clk/ 
>> clk-bulk.c clk_bulk_*() to handle all these disparate count of clock 
>> easily ?
> 
> Hi, I'd like to make sure that we enable the core clock before the bus
> clock so that the RNG hardware block can start its internal tests while
> we ungate the bus clock. It's not a strong opinion but it feels better.
Maybe this could still work if the struct clk_bulk_data {} is ordered 
that way, so the bus clock are first, and the rest afterward ?

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Gatien CHEVALLIER]

On 10/11/24 13:24, Marek Vasut wrote:
> On 10/11/24 11:55 AM, Gatien CHEVALLIER wrote:
>>
>>
>> On 10/7/24 15:54, Marek Vasut wrote:
>>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>>> Implement the support for STM32MP25x platforms. On this platform, a
>>>> security clock is shared between some hardware blocks. For the RNG,
>>>> it is the RNG kernel clock. Therefore, the gate is no more shared
>>>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>>>> the bus clock has to be managed on its own.
>>>>
>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>> A bit of a higher-level design question -- can you use drivers/clk/ 
>>> clk-bulk.c clk_bulk_*() to handle all these disparate count of clock 
>>> easily ?
>>
>> Hi, I'd like to make sure that we enable the core clock before the bus
>> clock so that the RNG hardware block can start its internal tests while
>> we ungate the bus clock. It's not a strong opinion but it feels better.
> Maybe this could still work if the struct clk_bulk_data {} is ordered 
> that way, so the bus clock are first, and the rest afterward ?

I guess you meant, the core first.
Putting the bus clock first with the updated YAML doc generates a
warning when checking the bindings. I guess what you propose is OK
then. Core clock is defined first in the device tree.

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Marek Vasut]

On 10/11/24 2:07 PM, Gatien CHEVALLIER wrote:
> 
> 
> On 10/11/24 13:24, Marek Vasut wrote:
>> On 10/11/24 11:55 AM, Gatien CHEVALLIER wrote:
>>>
>>>
>>> On 10/7/24 15:54, Marek Vasut wrote:
>>>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>>>> Implement the support for STM32MP25x platforms. On this platform, a
>>>>> security clock is shared between some hardware blocks. For the RNG,
>>>>> it is the RNG kernel clock. Therefore, the gate is no more shared
>>>>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>>>>> the bus clock has to be managed on its own.
>>>>>
>>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>>> A bit of a higher-level design question -- can you use drivers/clk/ 
>>>> clk-bulk.c clk_bulk_*() to handle all these disparate count of clock 
>>>> easily ?
>>>
>>> Hi, I'd like to make sure that we enable the core clock before the bus
>>> clock so that the RNG hardware block can start its internal tests while
>>> we ungate the bus clock. It's not a strong opinion but it feels better.
>> Maybe this could still work if the struct clk_bulk_data {} is ordered 
>> that way, so the bus clock are first, and the rest afterward ?
> 
> I guess you meant, the core first.

Err, yes, core.

> Putting the bus clock first with the updated YAML doc generates a
> warning when checking the bindings. I guess what you propose is OK
> then. Core clock is defined first in the device tree.

Not in DT, leave DT as-is. Look at struct clk_bulk_data , I think when 
you use the clk_bulk_*() functions, you pass in a list of struct 
clk_bulk_data, which each describes one clock, so just make sure that 
list of struct clk_bulk_data in the driver is ordered the way you need 
it to be ordered and you should be fine.

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Gatien CHEVALLIER]

On 10/11/24 14:38, Marek Vasut wrote:
> On 10/11/24 2:07 PM, Gatien CHEVALLIER wrote:
>>
>>
>> On 10/11/24 13:24, Marek Vasut wrote:
>>> On 10/11/24 11:55 AM, Gatien CHEVALLIER wrote:
>>>>
>>>>
>>>> On 10/7/24 15:54, Marek Vasut wrote:
>>>>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>>>>> Implement the support for STM32MP25x platforms. On this platform, a
>>>>>> security clock is shared between some hardware blocks. For the RNG,
>>>>>> it is the RNG kernel clock. Therefore, the gate is no more shared
>>>>>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>>>>>> the bus clock has to be managed on its own.
>>>>>>
>>>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>>>> A bit of a higher-level design question -- can you use drivers/clk/ 
>>>>> clk-bulk.c clk_bulk_*() to handle all these disparate count of 
>>>>> clock easily ?
>>>>
>>>> Hi, I'd like to make sure that we enable the core clock before the bus
>>>> clock so that the RNG hardware block can start its internal tests while
>>>> we ungate the bus clock. It's not a strong opinion but it feels better.
>>> Maybe this could still work if the struct clk_bulk_data {} is ordered 
>>> that way, so the bus clock are first, and the rest afterward ?
>>
>> I guess you meant, the core first.
> 
> Err, yes, core.
> 
>> Putting the bus clock first with the updated YAML doc generates a
>> warning when checking the bindings. I guess what you propose is OK
>> then. Core clock is defined first in the device tree.
> 
> Not in DT, leave DT as-is. Look at struct clk_bulk_data , I think when 
> you use the clk_bulk_*() functions, you pass in a list of struct 
> clk_bulk_data, which each describes one clock, so just make sure that 
> list of struct clk_bulk_data in the driver is ordered the way you need 
> it to be ordered and you should be fine.

I've sent a V2 with something that is functional but not aesthetic.
You'll tell me if that's what you had in mind.

Best regards,
Gatien

Re: [PATCH 2/4] hwrng: stm32 - implement support for STM32MP25x platforms

[Marek Vasut]

On 10/11/24 5:51 PM, Gatien CHEVALLIER wrote:
> 
> 
> On 10/11/24 14:38, Marek Vasut wrote:
>> On 10/11/24 2:07 PM, Gatien CHEVALLIER wrote:
>>>
>>>
>>> On 10/11/24 13:24, Marek Vasut wrote:
>>>> On 10/11/24 11:55 AM, Gatien CHEVALLIER wrote:
>>>>>
>>>>>
>>>>> On 10/7/24 15:54, Marek Vasut wrote:
>>>>>> On 10/7/24 3:27 PM, Gatien Chevallier wrote:
>>>>>>> Implement the support for STM32MP25x platforms. On this platform, a
>>>>>>> security clock is shared between some hardware blocks. For the RNG,
>>>>>>> it is the RNG kernel clock. Therefore, the gate is no more shared
>>>>>>> between the RNG bus and kernel clocks as on STM32MP1x platforms and
>>>>>>> the bus clock has to be managed on its own.
>>>>>>>
>>>>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>>>>> A bit of a higher-level design question -- can you use drivers/ 
>>>>>> clk/ clk-bulk.c clk_bulk_*() to handle all these disparate count 
>>>>>> of clock easily ?
>>>>>
>>>>> Hi, I'd like to make sure that we enable the core clock before the bus
>>>>> clock so that the RNG hardware block can start its internal tests 
>>>>> while
>>>>> we ungate the bus clock. It's not a strong opinion but it feels 
>>>>> better.
>>>> Maybe this could still work if the struct clk_bulk_data {} is 
>>>> ordered that way, so the bus clock are first, and the rest afterward ?
>>>
>>> I guess you meant, the core first.
>>
>> Err, yes, core.
>>
>>> Putting the bus clock first with the updated YAML doc generates a
>>> warning when checking the bindings. I guess what you propose is OK
>>> then. Core clock is defined first in the device tree.
>>
>> Not in DT, leave DT as-is. Look at struct clk_bulk_data , I think when 
>> you use the clk_bulk_*() functions, you pass in a list of struct 
>> clk_bulk_data, which each describes one clock, so just make sure that 
>> list of struct clk_bulk_data in the driver is ordered the way you need 
>> it to be ordered and you should be fine.
> 
> I've sent a V2 with something that is functional but not aesthetic.
> You'll tell me if that's what you had in mind.
I sent you a slightly tweaked example which should work too and should 
be a bit nicer.