On 2/20/23 10:11, Sean Anderson wrote: > On 2/20/23 00:59, Samuel Holland wrote: >> clk_get_rate() can return an error value. Recompute the rate if the >> cached value is an error value. >> >> Fixes: 4aa78300a025 ("dm: clk: Define clk_get_parent_rate() for clk >> operations") >> Signed-off-by: Samuel Holland <sam...@sholland.org> >> --- >> >> drivers/clk/clk-uclass.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c >> index 5bce976b060..9d052e5a814 100644 >> --- a/drivers/clk/clk-uclass.c >> +++ b/drivers/clk/clk-uclass.c >> @@ -520,7 +520,8 @@ ulong clk_get_parent_rate(struct clk *clk) >> return -ENOSYS; >> /* Read the 'rate' if not already set or if proper flag set*/ >> - if (!pclk->rate || pclk->flags & CLK_GET_RATE_NOCACHE) >> + if (!pclk->rate || IS_ERR_VALUE(pclk->rate) || >> + pclk->flags & CLK_GET_RATE_NOCACHE) >> pclk->rate = clk_get_rate(pclk); >> return pclk->rate; > > The correct fix here is > > /* Read the 'rate' if not already set or if proper flag set*/ > if (!pclk->rate || pclk->flags & CLK_GET_RATE_NOCACHE) { > rate = clk_get_rate(pclk); > if (!IS_ERR_VALUE(rate)) > pclk->rate = rate; > } > > return rate; > > No point caching errors.
Good point. I will do this for v2. Regards, Samuel