Hi Jonas,
On 10/4/26 11:13 PM, Jonas Karlman wrote:
The vendor TF-A blobs for RK3562 leave the TRNG and RK RNG clocks gated
when returning to U-Boot proper.
Try to enable clocks while reading or otherwise accessing TRNG/RKRNG
regs to ensure random data can successfully be read on affected SoCs.
This is not expected to have any impact on SoCs using a clock driver
with unimplemented enable/disable ops.
Signed-off-by: Jonas Karlman <[email protected]>
---
[...]
@@ -306,6 +308,10 @@ static int rockchip_rng_read(struct udevice *dev, void
*data, size_t len)
if (!pdata->soc_data || !pdata->soc_data->rk_rng_read)
return -EINVAL;
+ ret = clk_enable_bulk(&pdata->clks);
+ if (ret && ret != -ENOENT)
Because I was confused that clk_enable_bulk() could return -ENOENT,
maybe add a comment stating this is to catch clock drivers that
implement a .enable() callback and return -ENOENT for unsupported
clocks? RK3399 and PX30 seem to be the only one doing that on Rockchip.
Same comment for rockchip_rng_probe().
I'm also not entirely sure this is proper, shouldn't we rather implement
support for the clocks instead of just saying "it's fine, don't worry"?
I think only RK3399 would be impacted (as only RK3399 and PX30 clock
drivers implement an .enable, and PX30 DTS doesn't have clocks
specified), with missing support for HCLK_M_CRYPTO[01],
HCLK_S_CRYPTO[01], and SCLK_CRYPTO[01].
While looking at it, I also saw that there are resets in the node, maybe
something we'll need to control in the future too?
+ return ret;
+
for (i = 0; i < len / RK_HW_RNG_MAX; i++, buf += RK_HW_RNG_MAX) {
ret = pdata->soc_data->rk_rng_read(dev, buf, RK_HW_RNG_MAX);
if (ret)
@@ -317,6 +323,7 @@ static int rockchip_rng_read(struct udevice *dev, void
*data, size_t len)
len % RK_HW_RNG_MAX);
exit:
+ clk_disable_bulk(&pdata->clks);
return ret;
}
@@ -336,12 +343,22 @@ static int rockchip_rng_of_to_plat(struct udevice *dev)
static int rockchip_rng_probe(struct udevice *dev)
{
struct rk_rng_plat *pdata = dev_get_priv(dev);
- int ret = 0;
+ int ret;
+
+ ret = clk_get_bulk(dev, &pdata->clks);> + if (ret && ret != -ENOENT
&& ret != -ENOSYS)
I think what you want is clk_bulk_get_optional() from the Linux kernel,
so please implement that in U-Boot. We have it for reset, clk (not for
bulk yet) and gpio, so this is the right way to go.
Cheers,
Quentin
