[PATCH 00/10] hwrng: stm32: support STM32MP13x platforms

[PATCH 00/10] hwrng: stm32: support STM32MP13x platforms

[Gatien Chevallier]

The STM32MP13x platforms have a RNG hardware block that supports
customization, a conditional reset sequences that allows to
recover from certain situations and a configuration locking
mechanism.

This series adds support for the mentionned features. Note that
the hardware RNG can and should be managed in the secure world
for this platform, hence the rng not being default enabled on
the STM32MP135F-DK board. 

Gatien Chevallier (10):
  dt-bindings: rng: introduce new compatible for STM32MP13x
  hwrng: stm32 - use devm_platform_get_and_ioremap_resource() API
  hwrng: stm32 - implement STM32MP13x support
  hwrng: stm32 - implement error concealment
  hwrng: stm32 - rework error handling in stm32_rng_read()
  hwrng: stm32 - restrain RNG noise source clock
  dt-bindings: rng: add st,rng-lock-conf
  hwrng: stm32 - support RNG configuration locking mechanism
  hwrng: stm32 - rework power management sequences
  ARM: dts: stm32: add RNG node for STM32MP13x platforms

 .../devicetree/bindings/rng/st,stm32-rng.yaml |  18 +-
 arch/arm/boot/dts/st/stm32mp131.dtsi          |   8 +
 drivers/char/hw_random/stm32-rng.c            | 509 +++++++++++++++---
 3 files changed, 452 insertions(+), 83 deletions(-)

-- 
2.25.1

[PATCH 01/10] dt-bindings: rng: introduce new compatible for STM32MP13x

[Gatien Chevallier]

Introduce st,stm32mp13-rng compatible.

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

diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
index 187b172d0cca..59abdc85a9fb 100644
--- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
+++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
@@ -15,7 +15,9 @@ maintainers:
 
 properties:
   compatible:
-    const: st,stm32-rng
+    enum:
+      - st,stm32-rng
+      - st,stm32mp13-rng
 
   reg:
     maxItems: 1
-- 
2.25.1

Re: [PATCH 01/10] dt-bindings: rng: introduce new compatible for STM32MP13x

[Rob Herring]

On Fri, 08 Sep 2023 18:51:11 +0200, Gatien Chevallier wrote:
> Introduce st,stm32mp13-rng compatible.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> ---
>  Documentation/devicetree/bindings/rng/st,stm32-rng.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 02/10] hwrng: stm32 - use devm_platform_get_and_ioremap_resource() API

[Gatien Chevallier]

Use devm_platform_get_and_ioremap_resource() to get and ioremap a
resource.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index efb6a9f9a11b..d64d25d0fee8 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -118,18 +118,13 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->dev.of_node;
 	struct stm32_rng_private *priv;
-	struct resource res;
-	int err;
+	struct resource *res;
 
 	priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	err = of_address_to_resource(np, 0, &res);
-	if (err)
-		return err;
-
-	priv->base = devm_ioremap_resource(dev, &res);
+	priv->base = devm_platform_get_and_ioremap_resource(ofdev, 0, &res);
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-- 
2.25.1

[PATCH 03/10] hwrng: stm32 - implement STM32MP13x support

[Gatien Chevallier]

The RNG present on STM32MP13x platforms introduces a customizable
configuration and the conditional reset.

STM32 RNG configuration should best fit the requirements of the
platform. Therefore, put a platform-specific RNG configuration
field in the platform data. Default RNG configuration for STM32MP13
is the NIST certified configuration [1].

While there, fix and the RNG init sequence to support all RNG
versions.

[1] https://csrc.nist.gov/projects/cryptographic-module-validation-program/entropy-validations/certificate/53

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index d64d25d0fee8..54bd5807bbac 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -17,22 +17,43 @@
 #include <linux/reset.h>
 #include <linux/slab.h>
 
-#define RNG_CR 0x00
-#define RNG_CR_RNGEN BIT(2)
-#define RNG_CR_CED BIT(5)
-
-#define RNG_SR 0x04
-#define RNG_SR_SEIS BIT(6)
-#define RNG_SR_CEIS BIT(5)
-#define RNG_SR_DRDY BIT(0)
-
-#define RNG_DR 0x08
+#define RNG_CR			0x00
+#define RNG_CR_RNGEN		BIT(2)
+#define RNG_CR_CED		BIT(5)
+#define RNG_CR_CONFIG1		GENMASK(11, 8)
+#define RNG_CR_NISTC		BIT(12)
+#define RNG_CR_CONFIG2		GENMASK(15, 13)
+#define RNG_CR_CONFIG3		GENMASK(25, 20)
+#define RNG_CR_CONDRST		BIT(30)
+#define RNG_CR_CONFLOCK		BIT(31)
+#define RNG_CR_ENTROPY_SRC_MASK	(RNG_CR_CONFIG1 | RNG_CR_NISTC | RNG_CR_CONFIG2 | RNG_CR_CONFIG3)
+#define RNG_CR_CONFIG_MASK	(RNG_CR_ENTROPY_SRC_MASK | RNG_CR_CED)
+
+#define RNG_SR		0x04
+#define RNG_SR_SEIS	BIT(6)
+#define RNG_SR_CEIS	BIT(5)
+#define RNG_SR_DRDY	BIT(0)
+
+#define RNG_DR			0x08
+
+#define RNG_NSCR		0x0C
+#define RNG_NSCR_MASK		GENMASK(17, 0)
+
+#define RNG_HTCR		0x10
+
+struct stm32_rng_data {
+	u32	cr;
+	u32	nscr;
+	u32	htcr;
+	bool	has_cond_reset;
+};
 
 struct stm32_rng_private {
 	struct hwrng rng;
 	void __iomem *base;
 	struct clk *clk;
 	struct reset_control *rst;
+	const struct stm32_rng_data *data;
 	bool ced;
 };
 
@@ -87,32 +108,145 @@ static int stm32_rng_init(struct hwrng *rng)
 	struct stm32_rng_private *priv =
 	    container_of(rng, struct stm32_rng_private, rng);
 	int err;
+	u32 reg;
 
 	err = clk_prepare_enable(priv->clk);
 	if (err)
 		return err;
 
-	if (priv->ced)
-		writel_relaxed(RNG_CR_RNGEN, priv->base + RNG_CR);
-	else
-		writel_relaxed(RNG_CR_RNGEN | RNG_CR_CED,
-			       priv->base + RNG_CR);
-
 	/* clear error indicators */
 	writel_relaxed(0, priv->base + RNG_SR);
 
+	reg = readl_relaxed(priv->base + RNG_CR);
+
+	/*
+	 * Keep default RNG configuration if none was specified.
+	 * 0 is an invalid value as it disables all entropy sources.
+	 */
+	if (priv->data->has_cond_reset && priv->data->cr) {
+		reg &= ~RNG_CR_CONFIG_MASK;
+		reg |= RNG_CR_CONDRST | (priv->data->cr & RNG_CR_ENTROPY_SRC_MASK);
+		if (priv->ced)
+			reg &= ~RNG_CR_CED;
+		else
+			reg |= RNG_CR_CED;
+		writel_relaxed(reg, priv->base + RNG_CR);
+
+		/* Health tests and noise control registers */
+		writel_relaxed(priv->data->htcr, priv->base + RNG_HTCR);
+		writel_relaxed(priv->data->nscr & RNG_NSCR_MASK, priv->base + RNG_NSCR);
+
+		reg &= ~RNG_CR_CONDRST;
+		reg |= RNG_CR_RNGEN;
+		writel_relaxed(reg, priv->base + RNG_CR);
+
+		err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_CR, reg,
+							(!(reg & RNG_CR_CONDRST)),
+							10, 50000);
+		if (err) {
+			dev_err((struct device *)priv->rng.priv,
+				"%s: timeout %x!\n", __func__, reg);
+			return -EINVAL;
+		}
+	} else {
+		/* Handle all RNG versions by checking if conditional reset should be set */
+		if (priv->data->has_cond_reset)
+			reg |= RNG_CR_CONDRST;
+
+		if (priv->ced)
+			reg &= ~RNG_CR_CED;
+		else
+			reg |= RNG_CR_CED;
+
+		writel_relaxed(reg, priv->base + RNG_CR);
+
+		if (priv->data->has_cond_reset)
+			reg &= ~RNG_CR_CONDRST;
+
+		reg |= RNG_CR_RNGEN;
+
+		writel_relaxed(reg, priv->base + RNG_CR);
+	}
+
+	err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_SR, reg,
+						reg & RNG_SR_DRDY,
+						10, 100000);
+	if (err | (reg & ~RNG_SR_DRDY)) {
+		clk_disable_unprepare(priv->clk);
+		dev_err((struct device *)priv->rng.priv,
+			"%s: timeout:%x SR: %x!\n", __func__, err, reg);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-static void stm32_rng_cleanup(struct hwrng *rng)
+static int stm32_rng_remove(struct platform_device *ofdev)
 {
-	struct stm32_rng_private *priv =
-	    container_of(rng, struct stm32_rng_private, rng);
+	pm_runtime_disable(&ofdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int stm32_rng_runtime_suspend(struct device *dev)
+{
+	u32 reg;
+	struct stm32_rng_private *priv = dev_get_drvdata(dev);
 
-	writel_relaxed(0, priv->base + RNG_CR);
+	reg = readl_relaxed(priv->base + RNG_CR);
+	reg &= ~RNG_CR_RNGEN;
+	writel_relaxed(reg, priv->base + RNG_CR);
 	clk_disable_unprepare(priv->clk);
+
+	return 0;
 }
 
+static int stm32_rng_runtime_resume(struct device *dev)
+{
+	u32 reg;
+	struct stm32_rng_private *priv = dev_get_drvdata(dev);
+
+	clk_prepare_enable(priv->clk);
+	reg = readl_relaxed(priv->base + RNG_CR);
+	reg |= RNG_CR_RNGEN;
+	writel_relaxed(reg, priv->base + RNG_CR);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops stm32_rng_pm_ops = {
+	SET_RUNTIME_PM_OPS(stm32_rng_runtime_suspend,
+			   stm32_rng_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+};
+
+static const struct stm32_rng_data stm32mp13_rng_data = {
+	.has_cond_reset = true,
+	.cr = 0x00F00D00,
+	.nscr = 0x2B5BB,
+	.htcr = 0x969D,
+};
+
+static const struct stm32_rng_data stm32_rng_data = {
+	.has_cond_reset = false,
+};
+
+static const struct of_device_id stm32_rng_match[] = {
+	{
+		.compatible = "st,stm32mp13-rng",
+		.data = &stm32mp13_rng_data,
+	},
+	{
+		.compatible = "st,stm32-rng",
+		.data = &stm32_rng_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_rng_match);
+
 static int stm32_rng_probe(struct platform_device *ofdev)
 {
 	struct device *dev = &ofdev->dev;
@@ -141,13 +275,14 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 
 	priv->ced = of_property_read_bool(np, "clock-error-detect");
 
+	priv->data = of_device_get_match_data(dev);
+	if (!priv->data)
+		return -ENODEV;
+
 	dev_set_drvdata(dev, priv);
 
 	priv->rng.name = dev_driver_string(dev);
-#ifndef CONFIG_PM
 	priv->rng.init = stm32_rng_init;
-	priv->rng.cleanup = stm32_rng_cleanup;
-#endif
 	priv->rng.read = stm32_rng_read;
 	priv->rng.priv = (unsigned long) dev;
 	priv->rng.quality = 900;
@@ -159,47 +294,6 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 	return devm_hwrng_register(dev, &priv->rng);
 }
 
-static int stm32_rng_remove(struct platform_device *ofdev)
-{
-	pm_runtime_disable(&ofdev->dev);
-
-	return 0;
-}
-
-#ifdef CONFIG_PM
-static int stm32_rng_runtime_suspend(struct device *dev)
-{
-	struct stm32_rng_private *priv = dev_get_drvdata(dev);
-
-	stm32_rng_cleanup(&priv->rng);
-
-	return 0;
-}
-
-static int stm32_rng_runtime_resume(struct device *dev)
-{
-	struct stm32_rng_private *priv = dev_get_drvdata(dev);
-
-	return stm32_rng_init(&priv->rng);
-}
-#endif
-
-static const struct dev_pm_ops stm32_rng_pm_ops = {
-	SET_RUNTIME_PM_OPS(stm32_rng_runtime_suspend,
-			   stm32_rng_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
-};
-
-
-static const struct of_device_id stm32_rng_match[] = {
-	{
-		.compatible = "st,stm32-rng",
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(of, stm32_rng_match);
-
 static struct platform_driver stm32_rng_driver = {
 	.driver = {
 		.name = "stm32-rng",
-- 
2.25.1

[PATCH 04/10] hwrng: stm32 - implement error concealment

[Gatien Chevallier]

The RNG driver should be capable of recovering from an error. Implement
an error concealment API. This avoids irrecoverable RNG state.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 54bd5807bbac..adefe8edfd07 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -29,10 +29,12 @@
 #define RNG_CR_ENTROPY_SRC_MASK	(RNG_CR_CONFIG1 | RNG_CR_NISTC | RNG_CR_CONFIG2 | RNG_CR_CONFIG3)
 #define RNG_CR_CONFIG_MASK	(RNG_CR_ENTROPY_SRC_MASK | RNG_CR_CED)
 
-#define RNG_SR		0x04
-#define RNG_SR_SEIS	BIT(6)
-#define RNG_SR_CEIS	BIT(5)
-#define RNG_SR_DRDY	BIT(0)
+#define RNG_SR			0x04
+#define RNG_SR_DRDY		BIT(0)
+#define RNG_SR_CECS		BIT(1)
+#define RNG_SR_SECS		BIT(2)
+#define RNG_SR_CEIS		BIT(5)
+#define RNG_SR_SEIS		BIT(6)
 
 #define RNG_DR			0x08
 
@@ -57,6 +59,107 @@ struct stm32_rng_private {
 	bool ced;
 };
 
+/*
+ * Extracts from the STM32 RNG specification when RNG supports CONDRST.
+ *
+ * When a noise source (or seed) error occurs, the RNG stops generating
+ * random numbers and sets to “1” both SEIS and SECS bits to indicate
+ * that a seed error occurred. (...)
+ *
+ * 1. Software reset by writing CONDRST at 1 and at 0 (see bitfield
+ * description for details). This step is needed only if SECS is set.
+ * Indeed, when SEIS is set and SECS is cleared it means RNG performed
+ * the reset automatically (auto-reset).
+ * 2. If SECS was set in step 1 (no auto-reset) wait for CONDRST
+ * to be cleared in the RNG_CR register, then confirm that SEIS is
+ * cleared in the RNG_SR register. Otherwise just clear SEIS bit in
+ * the RNG_SR register.
+ * 3. If SECS was set in step 1 (no auto-reset) wait for SECS to be
+ * cleared by RNG. The random number generation is now back to normal.
+ */
+static int stm32_rng_conceal_seed_error_cond_reset(struct stm32_rng_private *priv)
+{
+	struct device *dev = (struct device *)priv->rng.priv;
+	u32 sr = readl_relaxed(priv->base + RNG_SR);
+	u32 cr = readl_relaxed(priv->base + RNG_CR);
+	int err;
+
+	if (sr & RNG_SR_SECS) {
+		/* Conceal by resetting the subsystem (step 1.) */
+		writel_relaxed(cr | RNG_CR_CONDRST, priv->base + RNG_CR);
+		writel_relaxed(cr & ~RNG_CR_CONDRST, priv->base + RNG_CR);
+	} else {
+		/* RNG auto-reset (step 2.) */
+		writel_relaxed(sr & ~RNG_SR_SEIS, priv->base + RNG_SR);
+		goto end;
+	}
+
+	err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_CR, cr, !(cr & RNG_CR_CONDRST), 10,
+						100000);
+	if (err) {
+		dev_err(dev, "%s: timeout %x\n", __func__, sr);
+		return err;
+	}
+
+	/* Check SEIS is cleared (step 2.) */
+	if (readl_relaxed(priv->base + RNG_SR) & RNG_SR_SEIS)
+		return -EINVAL;
+
+	err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_SR, sr, !(sr & RNG_SR_SECS), 10,
+						100000);
+	if (err) {
+		dev_err(dev, "%s: timeout %x\n", __func__, sr);
+		return err;
+	}
+
+end:
+	return 0;
+}
+
+/*
+ * Extracts from the STM32 RNG specification, when CONDRST is not supported
+ *
+ * When a noise source (or seed) error occurs, the RNG stops generating
+ * random numbers and sets to “1” both SEIS and SECS bits to indicate
+ * that a seed error occurred. (...)
+ *
+ * The following sequence shall be used to fully recover from a seed
+ * error after the RNG initialization:
+ * 1. Clear the SEIS bit by writing it to “0”.
+ * 2. Read out 12 words from the RNG_DR register, and discard each of
+ * them in order to clean the pipeline.
+ * 3. Confirm that SEIS is still cleared. Random number generation is
+ * back to normal.
+ */
+static int stm32_rng_conceal_seed_error_sw_reset(struct stm32_rng_private *priv)
+{
+	unsigned int i = 0;
+	u32 sr = readl_relaxed(priv->base + RNG_SR);
+
+	writel_relaxed(sr & ~RNG_SR_SEIS, priv->base + RNG_SR);
+
+	for (i = 12; i != 0; i--)
+		(void)readl_relaxed(priv->base + RNG_DR);
+
+	if (readl_relaxed(priv->base + RNG_SR) & RNG_SR_SEIS)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int stm32_rng_conceal_seed_error(struct hwrng *rng)
+{
+	struct stm32_rng_private *priv = container_of(rng, struct stm32_rng_private, rng);
+
+	dev_dbg((struct device *)priv->rng.priv, "Concealing seed error\n");
+
+	if (priv->data->has_cond_reset)
+		return stm32_rng_conceal_seed_error_cond_reset(priv);
+	else
+		return stm32_rng_conceal_seed_error_sw_reset(priv);
+};
+
+
 static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 {
 	struct stm32_rng_private *priv =
@@ -66,6 +169,9 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 
 	pm_runtime_get_sync((struct device *) priv->rng.priv);
 
+	if (readl_relaxed(priv->base + RNG_SR) & RNG_SR_SEIS)
+		stm32_rng_conceal_seed_error(rng);
+
 	while (max >= sizeof(u32)) {
 		sr = readl_relaxed(priv->base + RNG_SR);
 		/* Manage timeout which is based on timer and take */
-- 
2.25.1

[PATCH 05/10] hwrng: stm32 - rework error handling in stm32_rng_read()

[Gatien Chevallier]

Try to conceal seed errors when possible. If, despite the error
concealing tries, a seed error is still present, then return an error.

A clock error does not compromise the hardware block and data can
still be read from RNG_DR. Just warn that the RNG clock is too slow
and clear RNG_SR.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index adefe8edfd07..3d32e0f4baef 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -43,6 +43,8 @@
 
 #define RNG_HTCR		0x10
 
+#define RNG_NB_RECOVER_TRIES	3
+
 struct stm32_rng_data {
 	u32	cr;
 	u32	nscr;
@@ -162,10 +164,10 @@ static int stm32_rng_conceal_seed_error(struct hwrng *rng)
 
 static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 {
-	struct stm32_rng_private *priv =
-	    container_of(rng, struct stm32_rng_private, rng);
+	struct stm32_rng_private *priv = container_of(rng, struct stm32_rng_private, rng);
+	unsigned int i = 0;
+	int retval = 0, err = 0;
 	u32 sr;
-	int retval = 0;
 
 	pm_runtime_get_sync((struct device *) priv->rng.priv);
 
@@ -174,30 +176,57 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 
 	while (max >= sizeof(u32)) {
 		sr = readl_relaxed(priv->base + RNG_SR);
-		/* Manage timeout which is based on timer and take */
-		/* care of initial delay time when enabling rng	*/
+		/*
+		 * Manage timeout which is based on timer and take
+		 * care of initial delay time when enabling the RNG.
+		 */
 		if (!sr && wait) {
-			int err;
-
 			err = readl_relaxed_poll_timeout_atomic(priv->base
 								   + RNG_SR,
 								   sr, sr,
 								   10, 50000);
-			if (err)
+			if (err) {
 				dev_err((struct device *)priv->rng.priv,
 					"%s: timeout %x!\n", __func__, sr);
+				break;
+			}
+		} else if (!sr) {
+			/* The FIFO is being filled up */
+			break;
 		}
 
-		/* If error detected or data not ready... */
 		if (sr != RNG_SR_DRDY) {
-			if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
-					"bad RNG status - %x\n", sr))
+			if (sr & RNG_SR_SEIS) {
+				err = stm32_rng_conceal_seed_error(rng);
+				i++;
+				if (err && i > RNG_NB_RECOVER_TRIES) {
+					dev_err((struct device *)priv->rng.priv,
+						"Couldn't recover from seed error\n");
+					return -ENOTRECOVERABLE;
+				}
+
+				continue;
+			}
+
+			if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)
 				writel_relaxed(0, priv->base + RNG_SR);
-			break;
 		}
 
+		/* Late seed error case: DR being 0 is an error status */
 		*(u32 *)data = readl_relaxed(priv->base + RNG_DR);
+		if (!*(u32 *)data) {
+			err = stm32_rng_conceal_seed_error(rng);
+			i++;
+			if (err && i > RNG_NB_RECOVER_TRIES) {
+				dev_err((struct device *)priv->rng.priv,
+					"Couldn't recover from seed error");
+				return -ENOTRECOVERABLE;
+			}
+
+			continue;
+		}
 
+		i = 0;
 		retval += sizeof(u32);
 		data += sizeof(u32);
 		max -= sizeof(u32);
-- 
2.25.1

Re: [PATCH 05/10] hwrng: stm32 - rework error handling in stm32_rng_read()

[kernel test robot]

Hi Gatien,

kernel test robot noticed the following build warnings:

[auto build test WARNING on atorgue-stm32/stm32-next]
[also build test WARNING on robh/for-next herbert-crypto-2.6/master herbert-cryptodev-2.6/master linus/master v6.5 next-20230908]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/dt-bindings-rng-introduce-new-compatible-for-STM32MP13x/20230909-005611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
patch link:    https://lore.kernel.org/r/20230908165120.730867-6-gatien.chevallier%40foss.st.com
patch subject: [PATCH 05/10] hwrng: stm32 - rework error handling in stm32_rng_read()
config: hexagon-randconfig-002-20230909 (https://download.01.org/0day-ci/archive/20230909/202309090508.CyBIfKeu-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230909/202309090508.CyBIfKeu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309090508.CyBIfKeu-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/char/hw_random/stm32-rng.c:9:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/char/hw_random/stm32-rng.c:9:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/char/hw_random/stm32-rng.c:9:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/char/hw_random/stm32-rng.c:210:35: warning: left operand of comma operator has no effect [-Wunused-value]
     210 |                         if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)
         |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:57: note: expanded from macro 'if'
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                                                         ^~~~~~~~~~~
   include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
      57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                                                    ^~~~
>> drivers/char/hw_random/stm32-rng.c:210:35: warning: left operand of comma operator has no effect [-Wunused-value]
     210 |                         if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)
         |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:57: note: expanded from macro 'if'
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                                                         ^~~~~~~~~~~
   include/linux/compiler.h:57:86: note: expanded from macro '__trace_if_var'
      57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                                                                                      ^~~~
   include/linux/compiler.h:68:3: note: expanded from macro '__trace_if_value'
      68 |         (cond) ?                                        \
         |          ^~~~
   8 warnings generated.


vim +210 drivers/char/hw_random/stm32-rng.c

   162	
   163	
   164	static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
   165	{
   166		struct stm32_rng_private *priv = container_of(rng, struct stm32_rng_private, rng);
   167		unsigned int i = 0;
   168		int retval = 0, err = 0;
   169		u32 sr;
   170	
   171		pm_runtime_get_sync((struct device *) priv->rng.priv);
   172	
   173		if (readl_relaxed(priv->base + RNG_SR) & RNG_SR_SEIS)
   174			stm32_rng_conceal_seed_error(rng);
   175	
   176		while (max >= sizeof(u32)) {
   177			sr = readl_relaxed(priv->base + RNG_SR);
   178			/*
   179			 * Manage timeout which is based on timer and take
   180			 * care of initial delay time when enabling the RNG.
   181			 */
   182			if (!sr && wait) {
   183				err = readl_relaxed_poll_timeout_atomic(priv->base
   184									   + RNG_SR,
   185									   sr, sr,
   186									   10, 50000);
   187				if (err) {
   188					dev_err((struct device *)priv->rng.priv,
   189						"%s: timeout %x!\n", __func__, sr);
   190					break;
   191				}
   192			} else if (!sr) {
   193				/* The FIFO is being filled up */
   194				break;
   195			}
   196	
   197			if (sr != RNG_SR_DRDY) {
   198				if (sr & RNG_SR_SEIS) {
   199					err = stm32_rng_conceal_seed_error(rng);
   200					i++;
   201					if (err && i > RNG_NB_RECOVER_TRIES) {
   202						dev_err((struct device *)priv->rng.priv,
   203							"Couldn't recover from seed error\n");
   204						return -ENOTRECOVERABLE;
   205					}
   206	
   207					continue;
   208				}
   209	
 > 210				if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)
   211					writel_relaxed(0, priv->base + RNG_SR);
   212			}
   213	
   214			/* Late seed error case: DR being 0 is an error status */
   215			*(u32 *)data = readl_relaxed(priv->base + RNG_DR);
   216			if (!*(u32 *)data) {
   217				err = stm32_rng_conceal_seed_error(rng);
   218				i++;
   219				if (err && i > RNG_NB_RECOVER_TRIES) {
   220					dev_err((struct device *)priv->rng.priv,
   221						"Couldn't recover from seed error");
   222					return -ENOTRECOVERABLE;
   223				}
   224	
   225				continue;
   226			}
   227	
   228			i = 0;
   229			retval += sizeof(u32);
   230			data += sizeof(u32);
   231			max -= sizeof(u32);
   232		}
   233	
   234		pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
   235		pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);
   236	
   237		return retval || !wait ? retval : -EIO;
   238	}
   239	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 05/10] hwrng: stm32 - rework error handling in stm32_rng_read()

[Herbert Xu]

On Fri, Sep 08, 2023 at 06:51:15PM +0200, Gatien Chevallier wrote:
>
> +			if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)

Introducing an unconditional WARN_ON is not acceptable.  If you
really need it, make it WARN_ON_ONCE.  But why does this need
to be a WARN instead of dev_err?

Cheers,
-- 
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

Re: [PATCH 05/10] hwrng: stm32 - rework error handling in stm32_rng_read()

[Gatien CHEVALLIER]

Hello Herbert,

On 9/12/23 06:02, Herbert Xu wrote:
> On Fri, Sep 08, 2023 at 06:51:15PM +0200, Gatien Chevallier wrote:
>>
>> +			if (WARN_ON(sr & RNG_SR_CEIS), "RNG clock too slow - %x\n", sr)
> 
> Introducing an unconditional WARN_ON is not acceptable.  If you
> really need it, make it WARN_ON_ONCE.  But why does this need
> to be a WARN instead of dev_err?
> 
> Cheers,

It was my mistake and not my intention to change the WARN_ON_ONCE to a
WARN_ON. I've already sent a V2 correcting this.

It was already a WARN_ON_ONCE on the first implementation of the driver.
This is not an unrecoverable error as it doesn't affect the quality of
the entropy delivered by the RNG. The output is just too slow. It's here
to warn the developer of an incorrect clock source setting.

Best regards,
Gatien

[PATCH 06/10] hwrng: stm32 - restrain RNG noise source clock

[Gatien Chevallier]

For NIST certification the noise source sampling may need to be
restrained.

This change implements an algorithm that gets the rate of the RNG
clock and apply the correct value in CLKDIV field in RNG_CR register
to force the RNG clock rate to be "max_clock_rate" maximum.

As it is platform-specific, implement it as a compat data.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 3d32e0f4baef..3b77f1fe6aea 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -23,11 +23,13 @@
 #define RNG_CR_CONFIG1		GENMASK(11, 8)
 #define RNG_CR_NISTC		BIT(12)
 #define RNG_CR_CONFIG2		GENMASK(15, 13)
+#define RNG_CR_CLKDIV_SHIFT	16
+#define RNG_CR_CLKDIV		GENMASK(19, 16)
 #define RNG_CR_CONFIG3		GENMASK(25, 20)
 #define RNG_CR_CONDRST		BIT(30)
 #define RNG_CR_CONFLOCK		BIT(31)
 #define RNG_CR_ENTROPY_SRC_MASK	(RNG_CR_CONFIG1 | RNG_CR_NISTC | RNG_CR_CONFIG2 | RNG_CR_CONFIG3)
-#define RNG_CR_CONFIG_MASK	(RNG_CR_ENTROPY_SRC_MASK | RNG_CR_CED)
+#define RNG_CR_CONFIG_MASK	(RNG_CR_ENTROPY_SRC_MASK | RNG_CR_CED | RNG_CR_CLKDIV)
 
 #define RNG_SR			0x04
 #define RNG_SR_DRDY		BIT(0)
@@ -46,6 +48,7 @@
 #define RNG_NB_RECOVER_TRIES	3
 
 struct stm32_rng_data {
+	uint	max_clock_rate;
 	u32	cr;
 	u32	nscr;
 	u32	htcr;
@@ -238,6 +241,28 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 	return retval || !wait ? retval : -EIO;
 }
 
+static uint stm32_rng_clock_freq_restrain(struct hwrng *rng)
+{
+	struct stm32_rng_private *priv =
+	    container_of(rng, struct stm32_rng_private, rng);
+	unsigned long clock_rate = 0;
+	uint clock_div = 0;
+
+	clock_rate = clk_get_rate(priv->clk);
+
+	/*
+	 * Get the exponent to apply on the CLKDIV field in RNG_CR register
+	 * No need to handle the case when clock-div > 0xF as it is physically
+	 * impossible
+	 */
+	while ((clock_rate >> clock_div) > priv->data->max_clock_rate)
+		clock_div++;
+
+	pr_debug("RNG clk rate : %lu\n", clk_get_rate(priv->clk) >> clock_div);
+
+	return clock_div;
+}
+
 static int stm32_rng_init(struct hwrng *rng)
 {
 	struct stm32_rng_private *priv =
@@ -259,8 +284,11 @@ static int stm32_rng_init(struct hwrng *rng)
 	 * 0 is an invalid value as it disables all entropy sources.
 	 */
 	if (priv->data->has_cond_reset && priv->data->cr) {
+		uint clock_div = stm32_rng_clock_freq_restrain(rng);
+
 		reg &= ~RNG_CR_CONFIG_MASK;
-		reg |= RNG_CR_CONDRST | (priv->data->cr & RNG_CR_ENTROPY_SRC_MASK);
+		reg |= RNG_CR_CONDRST | (priv->data->cr & RNG_CR_ENTROPY_SRC_MASK) |
+		       (clock_div << RNG_CR_CLKDIV_SHIFT);
 		if (priv->ced)
 			reg &= ~RNG_CR_CED;
 		else
@@ -360,6 +388,7 @@ static const struct dev_pm_ops stm32_rng_pm_ops = {
 
 static const struct stm32_rng_data stm32mp13_rng_data = {
 	.has_cond_reset = true,
+	.max_clock_rate = 48000000,
 	.cr = 0x00F00D00,
 	.nscr = 0x2B5BB,
 	.htcr = 0x969D,
@@ -367,6 +396,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,
 };
 
 static const struct of_device_id stm32_rng_match[] = {
-- 
2.25.1

[PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Gatien Chevallier]

If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
and RNG_NSCR will be locked. It is supported starting from the RNG
version present in the STM32MP13

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

diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
index 59abdc85a9fb..0055f14a8e3f 100644
--- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
+++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
@@ -37,6 +37,20 @@ required:
   - reg
   - clocks
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp13-rng
+    then:
+      properties:
+        st,rng-lock-conf:
+          type: boolean
+          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
+                       RNG_NSCR will be locked.
+
 additionalProperties: false
 
 examples:
-- 
2.25.1

Re: [PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Rob Herring]

On Fri, Sep 08, 2023 at 06:51:17PM +0200, Gatien Chevallier wrote:
> If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
> and RNG_NSCR will be locked. It is supported starting from the RNG
> version present in the STM32MP13

This should be squashed into the prior binding patch.

> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> ---
>  .../devicetree/bindings/rng/st,stm32-rng.yaml      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> index 59abdc85a9fb..0055f14a8e3f 100644
> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
> @@ -37,6 +37,20 @@ required:
>    - reg
>    - clocks
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp13-rng
> +    then:
> +      properties:
> +        st,rng-lock-conf:
> +          type: boolean
> +          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
> +                       RNG_NSCR will be locked.

Define the property at the top-level and then restrict its presence in 
a if/then schema.

> +
>  additionalProperties: false

Did you test this property is allowed? No, because additionalProperties 
won't work with properties defined in if/then schemas.

>  
>  examples:
> -- 
> 2.25.1
> 

Re: [PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Gatien CHEVALLIER]

Hello Rob,

On 9/11/23 17:09, Rob Herring wrote:
> On Fri, Sep 08, 2023 at 06:51:17PM +0200, Gatien Chevallier wrote:
>> If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
>> and RNG_NSCR will be locked. It is supported starting from the RNG
>> version present in the STM32MP13
> 
> This should be squashed into the prior binding patch.
> 

Ok, I will squash both for V3.

>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> ---
>>   .../devicetree/bindings/rng/st,stm32-rng.yaml      | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> index 59abdc85a9fb..0055f14a8e3f 100644
>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> @@ -37,6 +37,20 @@ required:
>>     - reg
>>     - clocks
>>   
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp13-rng
>> +    then:
>> +      properties:
>> +        st,rng-lock-conf:
>> +          type: boolean
>> +          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
>> +                       RNG_NSCR will be locked.
> 
> Define the property at the top-level and then restrict its presence in
> a if/then schema.
> 

Ok, will change in V3. Thanks for your input

>> +
>>   additionalProperties: false
> 
> Did you test this property is allowed? No, because additionalProperties
> won't work with properties defined in if/then schemas.
> 
>>   
>>   examples:
>> -- 
>> 2.25.1
>>

Best regards,
Gatien

Re: [PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Gatien CHEVALLIER]

Hello Rob,

On 9/11/23 17:09, Rob Herring wrote:
> On Fri, Sep 08, 2023 at 06:51:17PM +0200, Gatien Chevallier wrote:
>> If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
>> and RNG_NSCR will be locked. It is supported starting from the RNG
>> version present in the STM32MP13
> 
> This should be squashed into the prior binding patch.
> 
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> ---
>>   .../devicetree/bindings/rng/st,stm32-rng.yaml      | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> index 59abdc85a9fb..0055f14a8e3f 100644
>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>> @@ -37,6 +37,20 @@ required:
>>     - reg
>>     - clocks
>>   
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp13-rng
>> +    then:
>> +      properties:
>> +        st,rng-lock-conf:
>> +          type: boolean
>> +          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
>> +                       RNG_NSCR will be locked.
> 
> Define the property at the top-level and then restrict its presence in
> a if/then schema.
> 

Can you please point me to an example of such case. I can't find a way
to define at the top-level the property then restrict it to specific
compatibles.

Else I'd change
additionalProperties :false to
unevaluatedProperties: false

so the definition of the property is seen.

Best regards,
Gatien

>> +
>>   additionalProperties: false
> 
> Did you test this property is allowed? No, because additionalProperties
> won't work with properties defined in if/then schemas.
> 
>>   
>>   examples:
>> -- 
>> 2.25.1
>>

Re: [PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Krzysztof Kozlowski]

On 15/09/2023 11:28, Gatien CHEVALLIER wrote:
> Hello Rob,
> 
> On 9/11/23 17:09, Rob Herring wrote:
>> On Fri, Sep 08, 2023 at 06:51:17PM +0200, Gatien Chevallier wrote:
>>> If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
>>> and RNG_NSCR will be locked. It is supported starting from the RNG
>>> version present in the STM32MP13
>>
>> This should be squashed into the prior binding patch.
>>
>>>
>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>> ---
>>>   .../devicetree/bindings/rng/st,stm32-rng.yaml      | 14 ++++++++++++++
>>>   1 file changed, 14 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>> index 59abdc85a9fb..0055f14a8e3f 100644
>>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>> @@ -37,6 +37,20 @@ required:
>>>     - reg
>>>     - clocks
>>>   
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - st,stm32mp13-rng
>>> +    then:
>>> +      properties:
>>> +        st,rng-lock-conf:
>>> +          type: boolean
>>> +          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
>>> +                       RNG_NSCR will be locked.
>>
>> Define the property at the top-level and then restrict its presence in
>> a if/then schema.
>>
> 
> Can you please point me to an example of such case. I can't find a way
> to define at the top-level the property then restrict it to specific
> compatibles.

You can check my slides from the talks about not reaching 10 iterations
of bindings patches.

Or open example-schema (this should be your starting point):
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212


Also:
https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174
> 
> Else I'd change
> additionalProperties :false to
> unevaluatedProperties: false
> 
> so the definition of the property is seen.

No, why? Definition is there when you move it to the top as asked.

Best regards,
Krzysztof

Re: [PATCH 07/10] dt-bindings: rng: add st,rng-lock-conf

[Gatien CHEVALLIER]

On 9/15/23 12:33, Krzysztof Kozlowski wrote:
> On 15/09/2023 11:28, Gatien CHEVALLIER wrote:
>> Hello Rob,
>>
>> On 9/11/23 17:09, Rob Herring wrote:
>>> On Fri, Sep 08, 2023 at 06:51:17PM +0200, Gatien Chevallier wrote:
>>>> If st,rng-lock-conf is set, the RNG configuration in RNG_CR, RNG_HTCR
>>>> and RNG_NSCR will be locked. It is supported starting from the RNG
>>>> version present in the STM32MP13
>>>
>>> This should be squashed into the prior binding patch.
>>>
>>>>
>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>>>> ---
>>>>    .../devicetree/bindings/rng/st,stm32-rng.yaml      | 14 ++++++++++++++
>>>>    1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>>> index 59abdc85a9fb..0055f14a8e3f 100644
>>>> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>>> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
>>>> @@ -37,6 +37,20 @@ required:
>>>>      - reg
>>>>      - clocks
>>>>    
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - st,stm32mp13-rng
>>>> +    then:
>>>> +      properties:
>>>> +        st,rng-lock-conf:
>>>> +          type: boolean
>>>> +          description: If set, the RNG configuration in RNG_CR, RNG_HTCR and
>>>> +                       RNG_NSCR will be locked.
>>>
>>> Define the property at the top-level and then restrict its presence in
>>> a if/then schema.
>>>
>>
>> Can you please point me to an example of such case. I can't find a way
>> to define at the top-level the property then restrict it to specific
>> compatibles.
> 
> You can check my slides from the talks about not reaching 10 iterations
> of bindings patches.
> 
> Or open example-schema (this should be your starting point):
> https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
> 
> 
> Also:
> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174

Thank you for the links, it really helped me out.

>>
>> Else I'd change
>> additionalProperties :false to
>> unevaluatedProperties: false
>>
>> so the definition of the property is seen.
> 
> No, why? Definition is there when you move it to the top as asked.
> 
> Best regards,
> Krzysztof
> 

[PATCH 08/10] hwrng: stm32 - support RNG configuration locking mechanism

[Gatien Chevallier]

If "st,rng-lock-conf" DT binding property is set for a stm32-rng node,
the RNG configuration will be locked until next hardware block reset
or platform reset.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 3b77f1fe6aea..0243b889ac54 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -62,6 +62,7 @@ struct stm32_rng_private {
 	struct reset_control *rst;
 	const struct stm32_rng_data *data;
 	bool ced;
+	bool lock_conf;
 };
 
 /*
@@ -301,6 +302,9 @@ static int stm32_rng_init(struct hwrng *rng)
 
 		reg &= ~RNG_CR_CONDRST;
 		reg |= RNG_CR_RNGEN;
+		if (priv->lock_conf)
+			reg |= RNG_CR_CONFLOCK;
+
 		writel_relaxed(reg, priv->base + RNG_CR);
 
 		err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_CR, reg,
@@ -439,6 +443,7 @@ static int stm32_rng_probe(struct platform_device *ofdev)
 	}
 
 	priv->ced = of_property_read_bool(np, "clock-error-detect");
+	priv->lock_conf = of_property_read_bool(np, "st,rng-lock-conf");
 
 	priv->data = of_device_get_match_data(dev);
 	if (!priv->data)
-- 
2.25.1

[PATCH 09/10] hwrng: stm32 - rework power management sequences

[Gatien Chevallier]

Implement stm32_rng_suspend()/stm32_rng_resume() low-power APIs
called when the hardware block context will be lost.

There is no need to save the RNG_CR register in
stm32_rng_runtime_suspend() as the context is not lost. Therefore,
only enable/disable the RNG in the runtime sequences.

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

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 0243b889ac54..3ac6712324c7 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -55,11 +55,25 @@ struct stm32_rng_data {
 	bool	has_cond_reset;
 };
 
+/**
+ * struct stm32_rng_config - RNG configuration data
+ *
+ * @cr:			RNG configuration. 0 means default hardware RNG configuration
+ * @nscr:		Noise sources control configuration.
+ * @htcr:		Health tests configuration.
+ */
+struct stm32_rng_config {
+	u32 cr;
+	u32 nscr;
+	u32 htcr;
+};
+
 struct stm32_rng_private {
 	struct hwrng rng;
 	void __iomem *base;
 	struct clk *clk;
 	struct reset_control *rst;
+	struct stm32_rng_config pm_conf;
 	const struct stm32_rng_data *data;
 	bool ced;
 	bool lock_conf;
@@ -355,11 +369,10 @@ static int stm32_rng_remove(struct platform_device *ofdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
 static int stm32_rng_runtime_suspend(struct device *dev)
 {
-	u32 reg;
 	struct stm32_rng_private *priv = dev_get_drvdata(dev);
+	u32 reg;
 
 	reg = readl_relaxed(priv->base + RNG_CR);
 	reg &= ~RNG_CR_RNGEN;
@@ -369,25 +382,98 @@ static int stm32_rng_runtime_suspend(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused stm32_rng_suspend(struct device *dev)
+{
+	struct stm32_rng_private *priv = dev_get_drvdata(dev);
+
+	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);
+	}
+
+	/* Do not save that RNG is enabled as it will be handled at resume */
+	priv->pm_conf.cr = readl_relaxed(priv->base + RNG_CR) & ~RNG_CR_RNGEN;
+
+	writel_relaxed(priv->pm_conf.cr, priv->base + RNG_CR);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
 static int stm32_rng_runtime_resume(struct device *dev)
 {
-	u32 reg;
 	struct stm32_rng_private *priv = dev_get_drvdata(dev);
+	int err;
+	u32 reg;
+
+	err = clk_prepare_enable(priv->clk);
+	if (err)
+		return err;
+
+	/* Clean error indications */
+	writel_relaxed(0, priv->base + RNG_SR);
 
-	clk_prepare_enable(priv->clk);
 	reg = readl_relaxed(priv->base + RNG_CR);
 	reg |= RNG_CR_RNGEN;
 	writel_relaxed(reg, priv->base + RNG_CR);
 
 	return 0;
 }
-#endif
+
+static int __maybe_unused stm32_rng_resume(struct device *dev)
+{
+	struct stm32_rng_private *priv = dev_get_drvdata(dev);
+	int err;
+	u32 reg;
+
+	err = clk_prepare_enable(priv->clk);
+	if (err)
+		return err;
+
+	/* Clean error indications */
+	writel_relaxed(0, priv->base + RNG_SR);
+
+	if (priv->data->has_cond_reset) {
+		/*
+		 * Correct configuration in bits [29:4] must be set in the same
+		 * access that set RNG_CR_CONDRST bit. Else config setting is
+		 * not taken into account. CONFIGLOCK bit must also be unset but
+		 * it is not handled at the moment.
+		 */
+		writel_relaxed(priv->pm_conf.cr | RNG_CR_CONDRST, priv->base + RNG_CR);
+
+		writel_relaxed(priv->pm_conf.nscr, priv->base + RNG_NSCR);
+		writel_relaxed(priv->pm_conf.htcr, priv->base + RNG_HTCR);
+
+		reg = readl_relaxed(priv->base + RNG_CR);
+		reg |= RNG_CR_RNGEN;
+		reg &= ~RNG_CR_CONDRST;
+		writel_relaxed(reg, priv->base + RNG_CR);
+
+		err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_CR, reg,
+							reg & ~RNG_CR_CONDRST, 10, 100000);
+
+		if (err) {
+			clk_disable_unprepare(priv->clk);
+			dev_err((struct device *)priv->rng.priv,
+				"%s: timeout:%x CR: %x!\n", __func__, err, reg);
+			return -EINVAL;
+		}
+	} else {
+		reg = priv->pm_conf.cr;
+		reg |= RNG_CR_RNGEN;
+		writel_relaxed(reg, priv->base + RNG_CR);
+	}
+
+	return 0;
+}
 
 static const struct dev_pm_ops stm32_rng_pm_ops = {
 	SET_RUNTIME_PM_OPS(stm32_rng_runtime_suspend,
 			   stm32_rng_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(stm32_rng_suspend,
+				stm32_rng_resume)
 };
 
 static const struct stm32_rng_data stm32mp13_rng_data = {
-- 
2.25.1

Re: [PATCH 09/10] hwrng: stm32 - rework power management sequences

[kernel test robot]

Hi Gatien,

kernel test robot noticed the following build warnings:

[auto build test WARNING on atorgue-stm32/stm32-next]
[also build test WARNING on robh/for-next herbert-crypto-2.6/master herbert-cryptodev-2.6/master linus/master v6.5 next-20230908]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/dt-bindings-rng-introduce-new-compatible-for-STM32MP13x/20230909-005611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
patch link:    https://lore.kernel.org/r/20230908165120.730867-10-gatien.chevallier%40foss.st.com
patch subject: [PATCH 09/10] hwrng: stm32 - rework power management sequences
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230909/202309090255.4Ttcm8zy-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230909/202309090255.4Ttcm8zy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309090255.4Ttcm8zy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/char/hw_random/stm32-rng.c:403:12: warning: 'stm32_rng_runtime_resume' defined but not used [-Wunused-function]
     403 | static int stm32_rng_runtime_resume(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/char/hw_random/stm32-rng.c:371:12: warning: 'stm32_rng_runtime_suspend' defined but not used [-Wunused-function]
     371 | static int stm32_rng_runtime_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/stm32_rng_runtime_resume +403 drivers/char/hw_random/stm32-rng.c

af0d4442dd6813 Lionel Debieve    2019-04-01  370  
c6a97c42e399ad Daniel Thompson   2015-10-12 @371  static int stm32_rng_runtime_suspend(struct device *dev)
c6a97c42e399ad Daniel Thompson   2015-10-12  372  {
d6ba06b8b9a947 Daniel Thompson   2015-10-14  373  	struct stm32_rng_private *priv = dev_get_drvdata(dev);
f81a08733f43ae Gatien Chevallier 2023-09-08  374  	u32 reg;
c6a97c42e399ad Daniel Thompson   2015-10-12  375  
d9494cfe524b05 Gatien Chevallier 2023-09-08  376  	reg = readl_relaxed(priv->base + RNG_CR);
d9494cfe524b05 Gatien Chevallier 2023-09-08  377  	reg &= ~RNG_CR_RNGEN;
d9494cfe524b05 Gatien Chevallier 2023-09-08  378  	writel_relaxed(reg, priv->base + RNG_CR);
d9494cfe524b05 Gatien Chevallier 2023-09-08  379  	clk_disable_unprepare(priv->clk);
c6a97c42e399ad Daniel Thompson   2015-10-12  380  
c6a97c42e399ad Daniel Thompson   2015-10-12  381  	return 0;
c6a97c42e399ad Daniel Thompson   2015-10-12  382  }
c6a97c42e399ad Daniel Thompson   2015-10-12  383  
f81a08733f43ae Gatien Chevallier 2023-09-08  384  static int __maybe_unused stm32_rng_suspend(struct device *dev)
f81a08733f43ae Gatien Chevallier 2023-09-08  385  {
f81a08733f43ae Gatien Chevallier 2023-09-08  386  	struct stm32_rng_private *priv = dev_get_drvdata(dev);
f81a08733f43ae Gatien Chevallier 2023-09-08  387  
f81a08733f43ae Gatien Chevallier 2023-09-08  388  	if (priv->data->has_cond_reset) {
f81a08733f43ae Gatien Chevallier 2023-09-08  389  		priv->pm_conf.nscr = readl_relaxed(priv->base + RNG_NSCR);
f81a08733f43ae Gatien Chevallier 2023-09-08  390  		priv->pm_conf.htcr = readl_relaxed(priv->base + RNG_HTCR);
f81a08733f43ae Gatien Chevallier 2023-09-08  391  	}
f81a08733f43ae Gatien Chevallier 2023-09-08  392  
f81a08733f43ae Gatien Chevallier 2023-09-08  393  	/* Do not save that RNG is enabled as it will be handled at resume */
f81a08733f43ae Gatien Chevallier 2023-09-08  394  	priv->pm_conf.cr = readl_relaxed(priv->base + RNG_CR) & ~RNG_CR_RNGEN;
f81a08733f43ae Gatien Chevallier 2023-09-08  395  
f81a08733f43ae Gatien Chevallier 2023-09-08  396  	writel_relaxed(priv->pm_conf.cr, priv->base + RNG_CR);
f81a08733f43ae Gatien Chevallier 2023-09-08  397  
f81a08733f43ae Gatien Chevallier 2023-09-08  398  	clk_disable_unprepare(priv->clk);
f81a08733f43ae Gatien Chevallier 2023-09-08  399  
f81a08733f43ae Gatien Chevallier 2023-09-08  400  	return 0;
f81a08733f43ae Gatien Chevallier 2023-09-08  401  }
f81a08733f43ae Gatien Chevallier 2023-09-08  402  
c6a97c42e399ad Daniel Thompson   2015-10-12 @403  static int stm32_rng_runtime_resume(struct device *dev)
c6a97c42e399ad Daniel Thompson   2015-10-12  404  {
f81a08733f43ae Gatien Chevallier 2023-09-08  405  	struct stm32_rng_private *priv = dev_get_drvdata(dev);
f81a08733f43ae Gatien Chevallier 2023-09-08  406  	int err;
d9494cfe524b05 Gatien Chevallier 2023-09-08  407  	u32 reg;
f81a08733f43ae Gatien Chevallier 2023-09-08  408  
f81a08733f43ae Gatien Chevallier 2023-09-08  409  	err = clk_prepare_enable(priv->clk);
f81a08733f43ae Gatien Chevallier 2023-09-08  410  	if (err)
f81a08733f43ae Gatien Chevallier 2023-09-08  411  		return err;
f81a08733f43ae Gatien Chevallier 2023-09-08  412  
f81a08733f43ae Gatien Chevallier 2023-09-08  413  	/* Clean error indications */
f81a08733f43ae Gatien Chevallier 2023-09-08  414  	writel_relaxed(0, priv->base + RNG_SR);
f81a08733f43ae Gatien Chevallier 2023-09-08  415  
f81a08733f43ae Gatien Chevallier 2023-09-08  416  	reg = readl_relaxed(priv->base + RNG_CR);
f81a08733f43ae Gatien Chevallier 2023-09-08  417  	reg |= RNG_CR_RNGEN;
f81a08733f43ae Gatien Chevallier 2023-09-08  418  	writel_relaxed(reg, priv->base + RNG_CR);
f81a08733f43ae Gatien Chevallier 2023-09-08  419  
f81a08733f43ae Gatien Chevallier 2023-09-08  420  	return 0;
f81a08733f43ae Gatien Chevallier 2023-09-08  421  }
f81a08733f43ae Gatien Chevallier 2023-09-08  422  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 10/10] ARM: dts: stm32: add RNG node for STM32MP13x platforms

[Gatien Chevallier]

The RNG on STM32MP13 offers upgrades like customization of its
configuration and the conditional reset.

The hardware RNG should be managed in the secure world for but it
is supported on Linux. Therefore, is it not default enabled.

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

diff --git a/arch/arm/boot/dts/st/stm32mp131.dtsi b/arch/arm/boot/dts/st/stm32mp131.dtsi
index ac90fcbf0c09..39db82b782eb 100644
--- a/arch/arm/boot/dts/st/stm32mp131.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp131.dtsi
@@ -1220,6 +1220,14 @@ mdma: dma-controller@58000000 {
 			dma-requests = <48>;
 		};
 
+		rng: rng@54004000 {
+			compatible = "st,stm32mp13-rng";
+			reg = <0x54004000 0x400>;
+			clocks = <&rcc RNG1_K>;
+			resets = <&rcc RNG1_R>;
+			status = "disabled";
+		};
+
 		fmc: memory-controller@58002000 {
 			compatible = "st,stm32mp1-fmc2-ebi";
 			reg = <0x58002000 0x1000>;
-- 
2.25.1