* [PATCH v2 0/4] Add support for stm32mp25x RNG
@ 2024-10-11 15:41 Gatien Chevallier
2024-10-11 15:41 ` [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support Gatien Chevallier
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Gatien Chevallier @ 2024-10-11 15:41 UTC (permalink / raw)
To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve,
marex
Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel, 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.
Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
Changes in V2:
-Fixes in bindings
-Removed MP25 RNG example
-Renamed RNG clocks for mp25 to "core" and "bus"
---
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 | 30 +++++++-
arch/arm64/boot/dts/st/stm32mp251.dtsi | 10 +++
drivers/char/hw_random/stm32-rng.c | 87 +++++++++++++++++-----
3 files changed, 107 insertions(+), 20 deletions(-)
---
base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f
change-id: 20241011-rng-mp25-v2-b6460ef11e1f
Best regards,
--
Gatien Chevallier <gatien.chevallier@foss.st.com>
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support 2024-10-11 15:41 [PATCH v2 0/4] Add support for stm32mp25x RNG Gatien Chevallier @ 2024-10-11 15:41 ` Gatien Chevallier 2024-10-14 7:29 ` Krzysztof Kozlowski 2024-10-11 15:41 ` [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms Gatien Chevallier ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Gatien Chevallier @ 2024-10-11 15:41 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, 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> --- Changes in V2 -Fix missing min/maxItems -Removed MP25 RNG example -Renamed RNG clocks for mp25 to "core" and "bus" --- .../devicetree/bindings/rng/st,stm32-rng.yaml | 30 +++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml index 340d01d481d12ce8664a60db42182ddaf0d1385b..5d553f7f706f8d7c17aea07e4130a34af764a635 100644 --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml @@ -18,12 +18,20 @@ properties: enum: - st,stm32-rng - st,stm32mp13-rng + - st,stm32mp25-rng reg: maxItems: 1 clocks: - maxItems: 1 + minItems: 1 + maxItems: 2 + + clock-names: + minItems: 1 + items: + - const: core + - const: bus resets: maxItems: 1 @@ -57,6 +65,26 @@ allOf: properties: st,rng-lock-conf: false + - if: + properties: + compatible: + contains: + enum: + - st,stm32-rng + - st,stm32mp13-rng + then: + properties: + clocks: + maxItems: 1 + clock-names: false + else: + properties: + clocks: + minItems: 2 + maxItems: 2 + required: + - clock-names + additionalProperties: false examples: -- 2.25.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support 2024-10-11 15:41 ` [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support Gatien Chevallier @ 2024-10-14 7:29 ` Krzysztof Kozlowski 2024-10-14 9:31 ` Gatien CHEVALLIER 0 siblings, 1 reply; 18+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 7:29 UTC (permalink / raw) To: Gatien Chevallier Cc: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex, linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On Fri, Oct 11, 2024 at 05:41:41PM +0200, Gatien Chevallier wrote: > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + > + clock-names: > + minItems: 1 > + items: > + - const: core > + - const: bus > > resets: > maxItems: 1 > @@ -57,6 +65,26 @@ allOf: > properties: > st,rng-lock-conf: false > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - st,stm32-rng > + - st,stm32mp13-rng > + then: > + properties: > + clocks: > + maxItems: 1 > + clock-names: false > + else: > + properties: > + clocks: > + minItems: 2 > + maxItems: 2 Missing clock-names constraint. They *always* go in sync with clocks. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support 2024-10-14 7:29 ` Krzysztof Kozlowski @ 2024-10-14 9:31 ` Gatien CHEVALLIER 0 siblings, 0 replies; 18+ messages in thread From: Gatien CHEVALLIER @ 2024-10-14 9:31 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex, linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/14/24 09:29, Krzysztof Kozlowski wrote: > On Fri, Oct 11, 2024 at 05:41:41PM +0200, Gatien Chevallier wrote: >> clocks: >> - maxItems: 1 >> + minItems: 1 >> + maxItems: 2 >> + >> + clock-names: >> + minItems: 1 >> + items: >> + - const: core >> + - const: bus >> >> resets: >> maxItems: 1 >> @@ -57,6 +65,26 @@ allOf: >> properties: >> st,rng-lock-conf: false >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - st,stm32-rng >> + - st,stm32mp13-rng >> + then: >> + properties: >> + clocks: >> + maxItems: 1 >> + clock-names: false >> + else: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 > > Missing clock-names constraint. They *always* go in sync with clocks. > > Best regards, > Krzysztof > Done for V3, Best regards, Gatien ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-11 15:41 [PATCH v2 0/4] Add support for stm32mp25x RNG Gatien Chevallier 2024-10-11 15:41 ` [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support Gatien Chevallier @ 2024-10-11 15:41 ` Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut 2024-10-15 6:46 ` kernel test robot 2024-10-11 15:41 ` [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency Gatien Chevallier 2024-10-11 15:41 ` [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 Gatien Chevallier 3 siblings, 2 replies; 18+ messages in thread From: Gatien Chevallier @ 2024-10-11 15:41 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, 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> --- Changes in V2 -Renamed RNG clocks to "core" and "bus" -Use clk_bulk_* APIs instead of handling each clock. Just make sure that the RNG core clock is first --- drivers/char/hw_random/stm32-rng.c | 85 ++++++++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c index 9d041a67c295a54d283d235bbcf5a9ab7a8baa5c..62aa9f87415d2518b0c1cb5fb51b0b646422ed35 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; @@ -72,7 +73,7 @@ struct stm32_rng_private { struct hwrng rng; struct device *dev; void __iomem *base; - struct clk *clk; + struct clk_bulk_data *clk_bulk; struct reset_control *rst; struct stm32_rng_config pm_conf; const struct stm32_rng_data *data; @@ -266,7 +267,7 @@ static uint stm32_rng_clock_freq_restrain(struct hwrng *rng) unsigned long clock_rate = 0; uint clock_div = 0; - clock_rate = clk_get_rate(priv->clk); + clock_rate = clk_get_rate(priv->clk_bulk[0].clk); /* * Get the exponent to apply on the CLKDIV field in RNG_CR register @@ -276,7 +277,7 @@ static uint stm32_rng_clock_freq_restrain(struct hwrng *rng) 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); + pr_debug("RNG clk rate : %lu\n", clk_get_rate(priv->clk_bulk[0].clk) >> clock_div); return clock_div; } @@ -288,7 +289,7 @@ static int stm32_rng_init(struct hwrng *rng) int err; u32 reg; - err = clk_prepare_enable(priv->clk); + err = clk_bulk_prepare_enable(priv->data->nb_clock, priv->clk_bulk); if (err) return err; @@ -328,7 +329,7 @@ static int stm32_rng_init(struct hwrng *rng) (!(reg & RNG_CR_CONDRST)), 10, 50000); if (err) { - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); dev_err(priv->dev, "%s: timeout %x!\n", __func__, reg); return -EINVAL; } @@ -356,12 +357,13 @@ static int stm32_rng_init(struct hwrng *rng) reg & RNG_SR_DRDY, 10, 100000); if (err || (reg & ~RNG_SR_DRDY)) { - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); dev_err(priv->dev, "%s: timeout:%x SR: %x!\n", __func__, err, reg); + return -EINVAL; } - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); return 0; } @@ -379,7 +381,8 @@ 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); - clk_disable_unprepare(priv->clk); + + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); return 0; } @@ -389,7 +392,7 @@ static int __maybe_unused stm32_rng_suspend(struct device *dev) struct stm32_rng_private *priv = dev_get_drvdata(dev); int err; - err = clk_prepare_enable(priv->clk); + err = clk_bulk_prepare_enable(priv->data->nb_clock, priv->clk_bulk); if (err) return err; @@ -403,7 +406,7 @@ static int __maybe_unused stm32_rng_suspend(struct device *dev) writel_relaxed(priv->pm_conf.cr, priv->base + RNG_CR); - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); return 0; } @@ -414,7 +417,7 @@ static int __maybe_unused stm32_rng_runtime_resume(struct device *dev) int err; u32 reg; - err = clk_prepare_enable(priv->clk); + err = clk_bulk_prepare_enable(priv->data->nb_clock, priv->clk_bulk); if (err) return err; @@ -434,7 +437,7 @@ static int __maybe_unused stm32_rng_resume(struct device *dev) int err; u32 reg; - err = clk_prepare_enable(priv->clk); + err = clk_bulk_prepare_enable(priv->data->nb_clock, priv->clk_bulk); if (err) return err; @@ -462,7 +465,7 @@ static int __maybe_unused stm32_rng_resume(struct device *dev) reg & ~RNG_CR_CONDRST, 10, 100000); if (err) { - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); dev_err(priv->dev, "%s: timeout:%x CR: %x!\n", __func__, err, reg); return -EINVAL; } @@ -472,7 +475,7 @@ static int __maybe_unused stm32_rng_resume(struct device *dev) writel_relaxed(reg, priv->base + RNG_CR); } - clk_disable_unprepare(priv->clk); + clk_bulk_disable_unprepare(priv->data->nb_clock, priv->clk_bulk); return 0; } @@ -484,9 +487,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 +508,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 +543,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 +565,41 @@ static int stm32_rng_probe(struct platform_device *ofdev) priv->rng.read = stm32_rng_read; priv->rng.quality = 900; + if (!priv->data->nb_clock || priv->data->nb_clock > 2) + return -EINVAL; + + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * sizeof(*priv->clk_bulk), + GFP_KERNEL); + if (!priv->clk_bulk) + return -ENOMEM; + + if (priv->data->nb_clock == 2) { + struct clk *clk; + struct clk *bus_clk; + + clk = devm_clk_get(&ofdev->dev, "core"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + bus_clk = devm_clk_get(&ofdev->dev, "bus"); + if (IS_ERR(clk)) + return PTR_ERR(bus_clk); + + priv->clk_bulk[0].clk = clk; + priv->clk_bulk[0].id = "core"; + priv->clk_bulk[1].clk = bus_clk; + priv->clk_bulk[1].id = "bus"; + } else { + struct clk *clk; + + clk = devm_clk_get(&ofdev->dev, NULL); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + priv->clk_bulk[0].clk = clk; + priv->clk_bulk[0].id = "core"; + } + pm_runtime_set_autosuspend_delay(dev, 100); pm_runtime_use_autosuspend(dev); pm_runtime_enable(dev); -- 2.25.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-11 15:41 ` [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms Gatien Chevallier @ 2024-10-11 16:17 ` Marek Vasut 2024-10-14 8:38 ` Gatien CHEVALLIER 2024-10-15 6:46 ` kernel test robot 1 sibling, 1 reply; 18+ messages in thread From: Marek Vasut @ 2024-10-11 16:17 UTC (permalink / raw) To: Gatien Chevallier, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/11/24 5:41 PM, Gatien Chevallier wrote: [...] > @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct platform_device *ofdev) > priv->rng.read = stm32_rng_read; > priv->rng.quality = 900; > > + if (!priv->data->nb_clock || priv->data->nb_clock > 2) > + return -EINVAL; > + > + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * sizeof(*priv->clk_bulk), > + GFP_KERNEL); > + if (!priv->clk_bulk) > + return -ENOMEM; Try this: ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); ... // Swap the clock if they are not in the right order: if (priv->data->nb_clock == 2 && strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) { const char *id = priv->clk_bulk[1].id; struct clk *clk = priv->clk_bulk[1].clk; priv->clk_bulk[1].id = priv->clk_bulk[0].id; priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; priv->clk_bulk[0].id = id; priv->clk_bulk[0].clk = clk; } > + if (priv->data->nb_clock == 2) { > + struct clk *clk; > + struct clk *bus_clk; > + > + clk = devm_clk_get(&ofdev->dev, "core"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + bus_clk = devm_clk_get(&ofdev->dev, "bus"); > + if (IS_ERR(clk)) > + return PTR_ERR(bus_clk); > + > + priv->clk_bulk[0].clk = clk; > + priv->clk_bulk[0].id = "core"; > + priv->clk_bulk[1].clk = bus_clk; > + priv->clk_bulk[1].id = "bus"; > + } else { > + struct clk *clk; > + > + clk = devm_clk_get(&ofdev->dev, NULL); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + priv->clk_bulk[0].clk = clk; > + priv->clk_bulk[0].id = "core"; > + } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-11 16:17 ` Marek Vasut @ 2024-10-14 8:38 ` Gatien CHEVALLIER 2024-10-14 8:52 ` Marek Vasut 0 siblings, 1 reply; 18+ messages in thread From: Gatien CHEVALLIER @ 2024-10-14 8:38 UTC (permalink / raw) To: Marek Vasut, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/11/24 18:17, Marek Vasut wrote: > On 10/11/24 5:41 PM, Gatien Chevallier wrote: > > [...] > >> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct platform_device >> *ofdev) >> priv->rng.read = stm32_rng_read; >> priv->rng.quality = 900; >> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >> + return -EINVAL; >> + >> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >> sizeof(*priv->clk_bulk), >> + GFP_KERNEL); >> + if (!priv->clk_bulk) >> + return -ENOMEM; > > Try this: > > ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); > ... > // Swap the clock if they are not in the right order: > if (priv->data->nb_clock == 2 && > strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) > { > const char *id = priv->clk_bulk[1].id; > struct clk *clk = priv->clk_bulk[1].clk; > priv->clk_bulk[1].id = priv->clk_bulk[0].id; > priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; > priv->clk_bulk[0].id = id; > priv->clk_bulk[0].clk = clk; > } > Hi Marek, This won't work as the name returned by this API is clk->core->name. AFAICT, it doesn't correspond to the names present in the device tree under the "clock-names" property. Any other idea or are you fine with what's below? Thanks, Gatien >> + if (priv->data->nb_clock == 2) { >> + struct clk *clk; >> + struct clk *bus_clk; >> + >> + clk = devm_clk_get(&ofdev->dev, "core"); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + >> + bus_clk = devm_clk_get(&ofdev->dev, "bus"); >> + if (IS_ERR(clk)) >> + return PTR_ERR(bus_clk); >> + >> + priv->clk_bulk[0].clk = clk; >> + priv->clk_bulk[0].id = "core"; >> + priv->clk_bulk[1].clk = bus_clk; >> + priv->clk_bulk[1].id = "bus"; >> + } else { >> + struct clk *clk; >> + >> + clk = devm_clk_get(&ofdev->dev, NULL); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + >> + priv->clk_bulk[0].clk = clk; >> + priv->clk_bulk[0].id = "core"; >> + } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-14 8:38 ` Gatien CHEVALLIER @ 2024-10-14 8:52 ` Marek Vasut 2024-10-14 12:36 ` Gatien CHEVALLIER 0 siblings, 1 reply; 18+ messages in thread From: Marek Vasut @ 2024-10-14 8:52 UTC (permalink / raw) To: Gatien CHEVALLIER, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: > > > On 10/11/24 18:17, Marek Vasut wrote: >> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >> >> [...] >> >>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>> platform_device *ofdev) >>> priv->rng.read = stm32_rng_read; >>> priv->rng.quality = 900; >>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>> + return -EINVAL; >>> + >>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>> sizeof(*priv->clk_bulk), >>> + GFP_KERNEL); >>> + if (!priv->clk_bulk) >>> + return -ENOMEM; >> >> Try this: >> >> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >> ... >> // Swap the clock if they are not in the right order: >> if (priv->data->nb_clock == 2 && >> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >> { >> const char *id = priv->clk_bulk[1].id; >> struct clk *clk = priv->clk_bulk[1].clk; >> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >> priv->clk_bulk[0].id = id; >> priv->clk_bulk[0].clk = clk; >> } >> > > Hi Marek, > > This won't work as the name returned by this API is clk->core->name. > AFAICT, it doesn't correspond to the names present in the device tree > under the "clock-names" property. > Any other idea or are you fine with what's below? Hmmm, it is not great, but at least it reduces the changes throughout the driver, so that is an improvement. I guess one could do some of_clk_get() and clk_is_match() in probe to look up the clock in OF by name and then compare which clock is which before swapping them in clk_bulk[] array, but that might be too convoluted? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-14 8:52 ` Marek Vasut @ 2024-10-14 12:36 ` Gatien CHEVALLIER 2024-10-14 18:55 ` Marek Vasut 0 siblings, 1 reply; 18+ messages in thread From: Gatien CHEVALLIER @ 2024-10-14 12:36 UTC (permalink / raw) To: Marek Vasut, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/14/24 10:52, Marek Vasut wrote: > On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: >> >> >> On 10/11/24 18:17, Marek Vasut wrote: >>> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >>> >>> [...] >>> >>>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>>> platform_device *ofdev) >>>> priv->rng.read = stm32_rng_read; >>>> priv->rng.quality = 900; >>>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>>> + return -EINVAL; >>>> + >>>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>>> sizeof(*priv->clk_bulk), >>>> + GFP_KERNEL); >>>> + if (!priv->clk_bulk) >>>> + return -ENOMEM; >>> >>> Try this: >>> >>> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >>> ... >>> // Swap the clock if they are not in the right order: >>> if (priv->data->nb_clock == 2 && >>> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >>> { >>> const char *id = priv->clk_bulk[1].id; >>> struct clk *clk = priv->clk_bulk[1].clk; >>> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >>> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >>> priv->clk_bulk[0].id = id; >>> priv->clk_bulk[0].clk = clk; >>> } >>> >> >> Hi Marek, >> >> This won't work as the name returned by this API is clk->core->name. >> AFAICT, it doesn't correspond to the names present in the device tree >> under the "clock-names" property. >> Any other idea or are you fine with what's below? > Hmmm, it is not great, but at least it reduces the changes throughout > the driver, so that is an improvement. > > I guess one could do some of_clk_get() and clk_is_match() in probe to > look up the clock in OF by name and then compare which clock is which > before swapping them in clk_bulk[] array, but that might be too convoluted? Yes, probably too much. What's present in the patch is not close to perfection but has the advantage of being straightforward. If we agree on that, I'll send a V3 containing the modifications in the bindings file. Thanks for reviewing, Gatien ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-14 12:36 ` Gatien CHEVALLIER @ 2024-10-14 18:55 ` Marek Vasut 2024-10-15 15:10 ` Gatien CHEVALLIER 0 siblings, 1 reply; 18+ messages in thread From: Marek Vasut @ 2024-10-14 18:55 UTC (permalink / raw) To: Gatien CHEVALLIER, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/14/24 2:36 PM, Gatien CHEVALLIER wrote: > > > On 10/14/24 10:52, Marek Vasut wrote: >> On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: >>> >>> >>> On 10/11/24 18:17, Marek Vasut wrote: >>>> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >>>> >>>> [...] >>>> >>>>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>>>> platform_device *ofdev) >>>>> priv->rng.read = stm32_rng_read; >>>>> priv->rng.quality = 900; >>>>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>>>> + return -EINVAL; >>>>> + >>>>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>>>> sizeof(*priv->clk_bulk), >>>>> + GFP_KERNEL); >>>>> + if (!priv->clk_bulk) >>>>> + return -ENOMEM; >>>> >>>> Try this: >>>> >>>> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >>>> ... >>>> // Swap the clock if they are not in the right order: >>>> if (priv->data->nb_clock == 2 && >>>> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >>>> { >>>> const char *id = priv->clk_bulk[1].id; >>>> struct clk *clk = priv->clk_bulk[1].clk; >>>> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >>>> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >>>> priv->clk_bulk[0].id = id; >>>> priv->clk_bulk[0].clk = clk; >>>> } >>>> >>> >>> Hi Marek, >>> >>> This won't work as the name returned by this API is clk->core->name. >>> AFAICT, it doesn't correspond to the names present in the device tree >>> under the "clock-names" property. >>> Any other idea or are you fine with what's below? >> Hmmm, it is not great, but at least it reduces the changes throughout >> the driver, so that is an improvement. >> >> I guess one could do some of_clk_get() and clk_is_match() in probe to >> look up the clock in OF by name and then compare which clock is which >> before swapping them in clk_bulk[] array, but that might be too >> convoluted? > > Yes, probably too much. What's present in the patch is not close to > perfection but has the advantage of being straightforward. If we agree > on that, I'll send a V3 containing the modifications in the bindings > file. Errr, I'm sorry, maybe there is a way to do this better. Look at drivers/clk/clk-bulk.c : 15 static int __must_check of_clk_bulk_get(struct device_node *np, int num_clks, 16 struct clk_bulk_data *clks) 17 { 18 int ret; 19 int i; 20 21 for (i = 0; i < num_clks; i++) { 22 clks[i].id = NULL; 23 clks[i].clk = NULL; 24 } 25 26 for (i = 0; i < num_clks; i++) { 27 of_property_read_string_index(np, "clock-names", i, &clks[i].id); 28 clks[i].clk = of_clk_get(np, i); If I read this right, then clks[i].id should be the DT clock name. So the swap conditional above could use .id to identify whether the first position is core clock or not, like this: if (priv->data->nb_clock == 2 && strcmp(__clk_get_name(priv->clk_bulk[0].id), "core")) ^^ You might need to use devm_clk_bulk_get_all() to access the of_clk_bulk_get() . Or am I missing something still ? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-14 18:55 ` Marek Vasut @ 2024-10-15 15:10 ` Gatien CHEVALLIER 2024-10-15 15:39 ` Marek Vasut 0 siblings, 1 reply; 18+ messages in thread From: Gatien CHEVALLIER @ 2024-10-15 15:10 UTC (permalink / raw) To: Marek Vasut, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/14/24 20:55, Marek Vasut wrote: > On 10/14/24 2:36 PM, Gatien CHEVALLIER wrote: >> >> >> On 10/14/24 10:52, Marek Vasut wrote: >>> On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: >>>> >>>> >>>> On 10/11/24 18:17, Marek Vasut wrote: >>>>> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >>>>> >>>>> [...] >>>>> >>>>>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>>>>> platform_device *ofdev) >>>>>> priv->rng.read = stm32_rng_read; >>>>>> priv->rng.quality = 900; >>>>>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>>>>> sizeof(*priv->clk_bulk), >>>>>> + GFP_KERNEL); >>>>>> + if (!priv->clk_bulk) >>>>>> + return -ENOMEM; >>>>> >>>>> Try this: >>>>> >>>>> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >>>>> ... >>>>> // Swap the clock if they are not in the right order: >>>>> if (priv->data->nb_clock == 2 && >>>>> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >>>>> { >>>>> const char *id = priv->clk_bulk[1].id; >>>>> struct clk *clk = priv->clk_bulk[1].clk; >>>>> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >>>>> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >>>>> priv->clk_bulk[0].id = id; >>>>> priv->clk_bulk[0].clk = clk; >>>>> } >>>>> >>>> >>>> Hi Marek, >>>> >>>> This won't work as the name returned by this API is clk->core->name. >>>> AFAICT, it doesn't correspond to the names present in the device tree >>>> under the "clock-names" property. >>>> Any other idea or are you fine with what's below? >>> Hmmm, it is not great, but at least it reduces the changes throughout >>> the driver, so that is an improvement. >>> >>> I guess one could do some of_clk_get() and clk_is_match() in probe to >>> look up the clock in OF by name and then compare which clock is which >>> before swapping them in clk_bulk[] array, but that might be too >>> convoluted? >> >> Yes, probably too much. What's present in the patch is not close to >> perfection but has the advantage of being straightforward. If we agree >> on that, I'll send a V3 containing the modifications in the bindings >> file. > Errr, I'm sorry, maybe there is a way to do this better. Look at > drivers/clk/clk-bulk.c : > > 15 static int __must_check of_clk_bulk_get(struct device_node *np, int > num_clks, > 16 struct clk_bulk_data *clks) > 17 { > 18 int ret; > 19 int i; > 20 > 21 for (i = 0; i < num_clks; i++) { > 22 clks[i].id = NULL; > 23 clks[i].clk = NULL; > 24 } > 25 > 26 for (i = 0; i < num_clks; i++) { > 27 of_property_read_string_index(np, "clock-names", i, > &clks[i].id); > 28 clks[i].clk = of_clk_get(np, i); > > If I read this right, then clks[i].id should be the DT clock name. So > the swap conditional above could use .id to identify whether the first > position is core clock or not, like this: > > if (priv->data->nb_clock == 2 && > strcmp(__clk_get_name(priv->clk_bulk[0].id), "core")) > ^^ > > You might need to use devm_clk_bulk_get_all() to access the > of_clk_bulk_get() . > > Or am I missing something still ? Oooooh I see, devm_clk_bulk_get() and devm_clk_bulk_get_all() use a different path. I don't understand why, to be honest... The doc doesn't state this difference either. I'll give this a try while also correcting the issue that the robot highlighted. Best regards, Gatien ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-15 15:10 ` Gatien CHEVALLIER @ 2024-10-15 15:39 ` Marek Vasut 2024-10-16 7:54 ` Gatien CHEVALLIER 0 siblings, 1 reply; 18+ messages in thread From: Marek Vasut @ 2024-10-15 15:39 UTC (permalink / raw) To: Gatien CHEVALLIER, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/15/24 5:10 PM, Gatien CHEVALLIER wrote: > > > On 10/14/24 20:55, Marek Vasut wrote: >> On 10/14/24 2:36 PM, Gatien CHEVALLIER wrote: >>> >>> >>> On 10/14/24 10:52, Marek Vasut wrote: >>>> On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: >>>>> >>>>> >>>>> On 10/11/24 18:17, Marek Vasut wrote: >>>>>> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >>>>>> >>>>>> [...] >>>>>> >>>>>>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>>>>>> platform_device *ofdev) >>>>>>> priv->rng.read = stm32_rng_read; >>>>>>> priv->rng.quality = 900; >>>>>>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>>>>>> + return -EINVAL; >>>>>>> + >>>>>>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>>>>>> sizeof(*priv->clk_bulk), >>>>>>> + GFP_KERNEL); >>>>>>> + if (!priv->clk_bulk) >>>>>>> + return -ENOMEM; >>>>>> >>>>>> Try this: >>>>>> >>>>>> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >>>>>> ... >>>>>> // Swap the clock if they are not in the right order: >>>>>> if (priv->data->nb_clock == 2 && >>>>>> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >>>>>> { >>>>>> const char *id = priv->clk_bulk[1].id; >>>>>> struct clk *clk = priv->clk_bulk[1].clk; >>>>>> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >>>>>> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >>>>>> priv->clk_bulk[0].id = id; >>>>>> priv->clk_bulk[0].clk = clk; >>>>>> } >>>>>> >>>>> >>>>> Hi Marek, >>>>> >>>>> This won't work as the name returned by this API is clk->core->name. >>>>> AFAICT, it doesn't correspond to the names present in the device tree >>>>> under the "clock-names" property. >>>>> Any other idea or are you fine with what's below? >>>> Hmmm, it is not great, but at least it reduces the changes >>>> throughout the driver, so that is an improvement. >>>> >>>> I guess one could do some of_clk_get() and clk_is_match() in probe >>>> to look up the clock in OF by name and then compare which clock is >>>> which before swapping them in clk_bulk[] array, but that might be >>>> too convoluted? >>> >>> Yes, probably too much. What's present in the patch is not close to >>> perfection but has the advantage of being straightforward. If we agree >>> on that, I'll send a V3 containing the modifications in the bindings >>> file. >> Errr, I'm sorry, maybe there is a way to do this better. Look at >> drivers/clk/clk-bulk.c : >> >> 15 static int __must_check of_clk_bulk_get(struct device_node *np, >> int num_clks, >> 16 struct clk_bulk_data *clks) >> 17 { >> 18 int ret; >> 19 int i; >> 20 >> 21 for (i = 0; i < num_clks; i++) { >> 22 clks[i].id = NULL; >> 23 clks[i].clk = NULL; >> 24 } >> 25 >> 26 for (i = 0; i < num_clks; i++) { >> 27 of_property_read_string_index(np, "clock-names", >> i, &clks[i].id); >> 28 clks[i].clk = of_clk_get(np, i); >> >> If I read this right, then clks[i].id should be the DT clock name. So >> the swap conditional above could use .id to identify whether the first >> position is core clock or not, like this: >> >> if (priv->data->nb_clock == 2 && >> strcmp(__clk_get_name(priv->clk_bulk[0].id), "core")) >> ^^ >> >> You might need to use devm_clk_bulk_get_all() to access the >> of_clk_bulk_get() . >> >> Or am I missing something still ? > > Oooooh I see, devm_clk_bulk_get() and devm_clk_bulk_get_all() use > a different path. I don't understand why, to be honest... The doc > doesn't state this difference either. Indeed, but maybe git log could clarify that ? I learnt about this useful trick at last year Embedded Recipes: $ git log -L:clk_bulk_get_all:drivers/clk/clk-bulk.c > I'll give this a try while also correcting the issue that the robot > highlighted. Thank you ! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-15 15:39 ` Marek Vasut @ 2024-10-16 7:54 ` Gatien CHEVALLIER 0 siblings, 0 replies; 18+ messages in thread From: Gatien CHEVALLIER @ 2024-10-16 7:54 UTC (permalink / raw) To: Marek Vasut, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/15/24 17:39, Marek Vasut wrote: > On 10/15/24 5:10 PM, Gatien CHEVALLIER wrote: >> >> >> On 10/14/24 20:55, Marek Vasut wrote: >>> On 10/14/24 2:36 PM, Gatien CHEVALLIER wrote: >>>> >>>> >>>> On 10/14/24 10:52, Marek Vasut wrote: >>>>> On 10/14/24 10:38 AM, Gatien CHEVALLIER wrote: >>>>>> >>>>>> >>>>>> On 10/11/24 18:17, Marek Vasut wrote: >>>>>>> On 10/11/24 5:41 PM, Gatien Chevallier wrote: >>>>>>> >>>>>>> [...] >>>>>>> >>>>>>>> @@ -551,6 +565,41 @@ static int stm32_rng_probe(struct >>>>>>>> platform_device *ofdev) >>>>>>>> priv->rng.read = stm32_rng_read; >>>>>>>> priv->rng.quality = 900; >>>>>>>> + if (!priv->data->nb_clock || priv->data->nb_clock > 2) >>>>>>>> + return -EINVAL; >>>>>>>> + >>>>>>>> + priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * >>>>>>>> sizeof(*priv->clk_bulk), >>>>>>>> + GFP_KERNEL); >>>>>>>> + if (!priv->clk_bulk) >>>>>>>> + return -ENOMEM; >>>>>>> >>>>>>> Try this: >>>>>>> >>>>>>> ret = devm_clk_bulk_get(dev, priv->data->nb_clock, priv->clk_bulk); >>>>>>> ... >>>>>>> // Swap the clock if they are not in the right order: >>>>>>> if (priv->data->nb_clock == 2 && >>>>>>> strcmp(__clk_get_name(priv->clk_bulk[0].clk), "core")) >>>>>>> { >>>>>>> const char *id = priv->clk_bulk[1].id; >>>>>>> struct clk *clk = priv->clk_bulk[1].clk; >>>>>>> priv->clk_bulk[1].id = priv->clk_bulk[0].id; >>>>>>> priv->clk_bulk[1].clk = priv->clk_bulk[0].clk; >>>>>>> priv->clk_bulk[0].id = id; >>>>>>> priv->clk_bulk[0].clk = clk; >>>>>>> } >>>>>>> >>>>>> >>>>>> Hi Marek, >>>>>> >>>>>> This won't work as the name returned by this API is clk->core->name. >>>>>> AFAICT, it doesn't correspond to the names present in the device tree >>>>>> under the "clock-names" property. >>>>>> Any other idea or are you fine with what's below? >>>>> Hmmm, it is not great, but at least it reduces the changes >>>>> throughout the driver, so that is an improvement. >>>>> >>>>> I guess one could do some of_clk_get() and clk_is_match() in probe >>>>> to look up the clock in OF by name and then compare which clock is >>>>> which before swapping them in clk_bulk[] array, but that might be >>>>> too convoluted? >>>> >>>> Yes, probably too much. What's present in the patch is not close to >>>> perfection but has the advantage of being straightforward. If we agree >>>> on that, I'll send a V3 containing the modifications in the bindings >>>> file. >>> Errr, I'm sorry, maybe there is a way to do this better. Look at >>> drivers/clk/clk-bulk.c : >>> >>> 15 static int __must_check of_clk_bulk_get(struct device_node *np, >>> int num_clks, >>> 16 struct clk_bulk_data *clks) >>> 17 { >>> 18 int ret; >>> 19 int i; >>> 20 >>> 21 for (i = 0; i < num_clks; i++) { >>> 22 clks[i].id = NULL; >>> 23 clks[i].clk = NULL; >>> 24 } >>> 25 >>> 26 for (i = 0; i < num_clks; i++) { >>> 27 of_property_read_string_index(np, "clock-names", >>> i, &clks[i].id); >>> 28 clks[i].clk = of_clk_get(np, i); >>> >>> If I read this right, then clks[i].id should be the DT clock name. So >>> the swap conditional above could use .id to identify whether the >>> first position is core clock or not, like this: >>> >>> if (priv->data->nb_clock == 2 && >>> strcmp(__clk_get_name(priv->clk_bulk[0].id), "core")) >>> ^^ >>> >>> You might need to use devm_clk_bulk_get_all() to access the >>> of_clk_bulk_get() . >>> >>> Or am I missing something still ? >> >> Oooooh I see, devm_clk_bulk_get() and devm_clk_bulk_get_all() use >> a different path. I don't understand why, to be honest... The doc >> doesn't state this difference either. > > Indeed, but maybe git log could clarify that ? I learnt about this > useful trick at last year Embedded Recipes: > > $ git log -L:clk_bulk_get_all:drivers/clk/clk-bulk.c > Will definitely keep that somewhere, thanks. >> I'll give this a try while also correcting the issue that the robot >> highlighted. > Thank you ! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms 2024-10-11 15:41 ` [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut @ 2024-10-15 6:46 ` kernel test robot 1 sibling, 0 replies; 18+ messages in thread From: kernel test robot @ 2024-10-15 6:46 UTC (permalink / raw) To: Gatien Chevallier, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex Cc: oe-kbuild-all, linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, Gatien Chevallier Hi Gatien, kernel test robot noticed the following build warnings: [auto build test WARNING on 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f] url: https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/dt-bindings-rng-add-st-stm32mp25-rng-support/20241011-234913 base: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f patch link: https://lore.kernel.org/r/20241011-rng-mp25-v2-v2-2-76fd6170280c%40foss.st.com patch subject: [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms config: nios2-randconfig-r053-20241015 (https://download.01.org/0day-ci/archive/20241015/202410151421.5UhVRFdF-lkp@intel.com/config) compiler: nios2-linux-gcc (GCC) 14.1.0 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/202410151421.5UhVRFdF-lkp@intel.com/ cocci warnings: (new ones prefixed by >>) >> drivers/char/hw_random/stm32-rng.c:585:6-12: inconsistent IS_ERR and PTR_ERR on line 586. vim +585 drivers/char/hw_random/stm32-rng.c 530 531 static int stm32_rng_probe(struct platform_device *ofdev) 532 { 533 struct device *dev = &ofdev->dev; 534 struct device_node *np = ofdev->dev.of_node; 535 struct stm32_rng_private *priv; 536 struct resource *res; 537 538 priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); 539 if (!priv) 540 return -ENOMEM; 541 542 priv->base = devm_platform_get_and_ioremap_resource(ofdev, 0, &res); 543 if (IS_ERR(priv->base)) 544 return PTR_ERR(priv->base); 545 546 priv->rst = devm_reset_control_get(&ofdev->dev, NULL); 547 if (!IS_ERR(priv->rst)) { 548 reset_control_assert(priv->rst); 549 udelay(2); 550 reset_control_deassert(priv->rst); 551 } 552 553 priv->ced = of_property_read_bool(np, "clock-error-detect"); 554 priv->lock_conf = of_property_read_bool(np, "st,rng-lock-conf"); 555 priv->dev = dev; 556 557 priv->data = of_device_get_match_data(dev); 558 if (!priv->data) 559 return -ENODEV; 560 561 dev_set_drvdata(dev, priv); 562 563 priv->rng.name = dev_driver_string(dev); 564 priv->rng.init = stm32_rng_init; 565 priv->rng.read = stm32_rng_read; 566 priv->rng.quality = 900; 567 568 if (!priv->data->nb_clock || priv->data->nb_clock > 2) 569 return -EINVAL; 570 571 priv->clk_bulk = devm_kzalloc(dev, priv->data->nb_clock * sizeof(*priv->clk_bulk), 572 GFP_KERNEL); 573 if (!priv->clk_bulk) 574 return -ENOMEM; 575 576 if (priv->data->nb_clock == 2) { 577 struct clk *clk; 578 struct clk *bus_clk; 579 580 clk = devm_clk_get(&ofdev->dev, "core"); 581 if (IS_ERR(clk)) 582 return PTR_ERR(clk); 583 584 bus_clk = devm_clk_get(&ofdev->dev, "bus"); > 585 if (IS_ERR(clk)) > 586 return PTR_ERR(bus_clk); 587 588 priv->clk_bulk[0].clk = clk; 589 priv->clk_bulk[0].id = "core"; 590 priv->clk_bulk[1].clk = bus_clk; 591 priv->clk_bulk[1].id = "bus"; 592 } else { 593 struct clk *clk; 594 595 clk = devm_clk_get(&ofdev->dev, NULL); 596 if (IS_ERR(clk)) 597 return PTR_ERR(clk); 598 599 priv->clk_bulk[0].clk = clk; 600 priv->clk_bulk[0].id = "core"; 601 } 602 603 pm_runtime_set_autosuspend_delay(dev, 100); 604 pm_runtime_use_autosuspend(dev); 605 pm_runtime_enable(dev); 606 607 return devm_hwrng_register(dev, &priv->rng); 608 } 609 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency 2024-10-11 15:41 [PATCH v2 0/4] Add support for stm32mp25x RNG Gatien Chevallier 2024-10-11 15:41 ` [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support Gatien Chevallier 2024-10-11 15:41 ` [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms Gatien Chevallier @ 2024-10-11 15:41 ` Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut 2024-10-11 15:41 ` [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 Gatien Chevallier 3 siblings, 1 reply; 18+ messages in thread From: Gatien Chevallier @ 2024-10-11 15:41 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, 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 62aa9f87415d2518b0c1cb5fb51b0b646422ed35..ef8324b914fc2de2e7a6bd9eb69c081c26cd1ecd 100644 --- a/drivers/char/hw_random/stm32-rng.c +++ b/drivers/char/hw_random/stm32-rng.c @@ -507,7 +507,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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency 2024-10-11 15:41 ` [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency Gatien Chevallier @ 2024-10-11 16:17 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2024-10-11 16:17 UTC (permalink / raw) To: Gatien Chevallier, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/11/24 5:41 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> Reviewed-by: Marek Vasut <marex@denx.de> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 2024-10-11 15:41 [PATCH v2 0/4] Add support for stm32mp25x RNG Gatien Chevallier ` (2 preceding siblings ...) 2024-10-11 15:41 ` [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency Gatien Chevallier @ 2024-10-11 15:41 ` Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut 3 siblings, 1 reply; 18+ messages in thread From: Gatien Chevallier @ 2024-10-11 15:41 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve, marex Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, 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> --- Changes in V2 -Renamed RNG clocks to "core" and "bus" --- 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 1167cf63d7e87aaa15c5c1ed70a9f6511fd818d4..273da5f62294422b587b13404b499b5ffe6c148e 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 = "core", "bus"; + resets = <&rcc RNG_R>; + access-controllers = <&rifsc 92>; + status = "disabled"; + }; + spi8: spi@46020000 { #address-cells = <1>; #size-cells = <0>; -- 2.25.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 2024-10-11 15:41 ` [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 Gatien Chevallier @ 2024-10-11 16:17 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2024-10-11 16:17 UTC (permalink / raw) To: Gatien Chevallier, Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin, Alexandre Torgue, Lionel Debieve Cc: linux-crypto, devicetree, linux-stm32, linux-arm-kernel, linux-kernel On 10/11/24 5:41 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> Reviewed-by: Marek Vasut <marex@denx.de> ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-10-16 7:56 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-11 15:41 [PATCH v2 0/4] Add support for stm32mp25x RNG Gatien Chevallier 2024-10-11 15:41 ` [PATCH v2 1/4] dt-bindings: rng: add st,stm32mp25-rng support Gatien Chevallier 2024-10-14 7:29 ` Krzysztof Kozlowski 2024-10-14 9:31 ` Gatien CHEVALLIER 2024-10-11 15:41 ` [PATCH v2 2/4] hwrng: stm32 - implement support for STM32MP25x platforms Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut 2024-10-14 8:38 ` Gatien CHEVALLIER 2024-10-14 8:52 ` Marek Vasut 2024-10-14 12:36 ` Gatien CHEVALLIER 2024-10-14 18:55 ` Marek Vasut 2024-10-15 15:10 ` Gatien CHEVALLIER 2024-10-15 15:39 ` Marek Vasut 2024-10-16 7:54 ` Gatien CHEVALLIER 2024-10-15 6:46 ` kernel test robot 2024-10-11 15:41 ` [PATCH v2 3/4] hwrng: stm32 - update STM32MP15 RNG max clock frequency Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut 2024-10-11 15:41 ` [PATCH v2 4/4] arm64: dts: st: add RNG node on stm32mp251 Gatien Chevallier 2024-10-11 16:17 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).