[PATCH 00/24] clk: Remove passing of negative errors through unsigned return
Simon Glass
sjg at chromium.org
Sun Oct 19 15:05:33 CEST 2025
Hi Tom, Andrew,
On Sat, 18 Oct 2025 at 15:04, Tom Rini <trini at konsulko.com> wrote:
>
> On Sat, Oct 18, 2025 at 09:34:42AM +0100, Simon Glass wrote:
> > Hi Andrew,
> >
> > On Wed, 15 Oct 2025 at 15:32, Andrew Goodbody
> > <andrew.goodbody at linaro.org> wrote:
> > >
> > > This series removes the passing of negative errors through the .get_rate
> > > function in the clk_ops struct. This function returns an unsigned long.
> > > The only value guaranteed to not be a valid clock rate is 0. This will
> > > also bring the drivers more in sync with Linux to allow for easier code
> > > porting and other maintenance in the future.
> > > Another series will address the calling of clk_get_rate and associated
> > > error handling.
> >
> > Some indication of the problem you ran into would be useful here.
>
> The problem statement is in the paragraph you're quoting. The numerical
> value of -ENOENT is a valid clock rate.
No, I mean a problem with a board, or something like that. We are
talking here about not being able to return a valid clock rate between
4294967040 and 4294967295, which is only even a theoretical problem on
32-bit machines. So I think it is reasonable to include a motivation.
>
> > > Signed-off-by: Andrew Goodbody <andrew.goodbody at linaro.org>
> > > ---
> > > Andrew Goodbody (24):
> > > clk: meson: Remove negative error returns from clk_get_rate
> > > clk: sifive: Remove negative error returns from clk_get_rate
> > > clk: armada-37xx: Remove negative error returns from clk_get_rate
> > > clk: thead: th1520-ap: Remove negative error returns from clk_get_rate
> > > clk: ccf: Remove negative error returns from clk_get_rate
> > > clk: at91: Remove negative error returns from clk_get_rate
> > > clk: renesas: Remove negative error returns from clk_get_rate
> > > clk: rockchip: Remove negative error returns from clk_get_rate
> > > clk: Remove negative error returns from clk_get_rate
> > > clk: starfive: Remove negative error returns from clk_get_rate
> > > clk: altera: Remove negative error returns from clk_get_rate
> > > clk: uniphier: Remove negative error returns from clk_get_rate
> > > clk: aspeed: Remove negative error returns from clk_get_rate
> > > clk: nuvoton: Remove negative error returns from clk_get_rate
> > > clk: exynos: Remove negative error returns from clk_get_rate
> > > clk: imx: Remove negative error returns from clk_get_rate
> > > clk: ti: Remove negative error returns from clk_get_rate
> > > clk: mediatek: Remove negative error returns from clk_get_rate
> > > clk: owl: Remove negative error returns from clk_get_rate
> > > clk: tegra: Remove negative error returns from clk_get_rate
> > > clk: adi: Remove negative error returns from clk_get_rate
> > > clk: sophgo: Remove negative error returns from clk_get_rate
> > > clk: stm32: Remove negative error returns from clk_get_rate
> > > clk: x86: Remove negative error returns from clk_get_rate
> > >
> > > drivers/clk/adi/clk-shared.c | 2 +-
> > > drivers/clk/altera/clk-agilex.c | 2 +-
> > > drivers/clk/altera/clk-agilex5.c | 2 +-
> > > drivers/clk/altera/clk-n5x.c | 2 +-
> > > drivers/clk/aspeed/clk_ast2500.c | 2 +-
> > > drivers/clk/aspeed/clk_ast2600.c | 2 +-
> > > drivers/clk/at91/compat.c | 6 ++--
> > > drivers/clk/clk-hsdk-cgu.c | 2 +-
> > > drivers/clk/clk-uclass.c | 4 +--
> > > drivers/clk/clk.c | 2 +-
> > > drivers/clk/clk_fixed_factor.c | 4 +--
> > > drivers/clk/clk_k210.c | 6 ++--
> > > drivers/clk/clk_sandbox.c | 4 +--
> > > drivers/clk/clk_scmi.c | 4 +--
> > > drivers/clk/clk_vexpress_osc.c | 2 +-
> > > drivers/clk/clk_zynq.c | 4 +--
> > > drivers/clk/clk_zynqmp.c | 40 ++++++++++-----------
> > > drivers/clk/exynos/clk-exynos7420.c | 2 +-
> > > drivers/clk/imx/clk-imx8qm.c | 6 ++--
> > > drivers/clk/imx/clk-imx8qxp.c | 6 ++--
> > > drivers/clk/imx/clk-imxrt1170.c | 2 +-
> > > drivers/clk/imx/clk-pllv3.c | 2 +-
> > > drivers/clk/intel/clk_intel.c | 2 +-
> > > drivers/clk/mediatek/clk-mtk.c | 2 +-
> > > drivers/clk/meson/a1.c | 10 +++---
> > > drivers/clk/meson/axg.c | 10 +++---
> > > drivers/clk/meson/g12a.c | 36 +++++++++----------
> > > drivers/clk/meson/gxbb.c | 20 +++++------
> > > drivers/clk/mvebu/armada-37xx-periph.c | 2 +-
> > > drivers/clk/mvebu/armada-37xx-tbg.c | 2 +-
> > > drivers/clk/nuvoton/clk_npcm.c | 10 +++---
> > > drivers/clk/owl/clk_owl.c | 2 +-
> > > drivers/clk/renesas/clk-rcar-gen2.c | 8 ++---
> > > drivers/clk/renesas/rzg2l-cpg.c | 8 ++---
> > > drivers/clk/rockchip/clk_px30.c | 24 ++++++-------
> > > drivers/clk/rockchip/clk_rk3036.c | 2 +-
> > > drivers/clk/rockchip/clk_rk3066.c | 8 ++---
> > > drivers/clk/rockchip/clk_rk3128.c | 6 ++--
> > > drivers/clk/rockchip/clk_rk3188.c | 6 ++--
> > > drivers/clk/rockchip/clk_rk322x.c | 4 +--
> > > drivers/clk/rockchip/clk_rk3288.c | 6 ++--
> > > drivers/clk/rockchip/clk_rk3308.c | 26 +++++++-------
> > > drivers/clk/rockchip/clk_rk3328.c | 6 ++--
> > > drivers/clk/rockchip/clk_rk3368.c | 8 ++---
> > > drivers/clk/rockchip/clk_rk3399.c | 12 +++----
> > > drivers/clk/rockchip/clk_rk3528.c | 20 +++++------
> > > drivers/clk/rockchip/clk_rk3568.c | 62 ++++++++++++++++----------------
> > > drivers/clk/rockchip/clk_rk3576.c | 36 +++++++++----------
> > > drivers/clk/rockchip/clk_rk3588.c | 32 ++++++++---------
> > > drivers/clk/rockchip/clk_rv1108.c | 4 +--
> > > drivers/clk/rockchip/clk_rv1126.c | 52 +++++++++++++--------------
> > > drivers/clk/sifive/sifive-prci.c | 8 ++---
> > > drivers/clk/sophgo/clk-cv1800b.c | 2 +-
> > > drivers/clk/starfive/clk-jh7110-pll.c | 2 +-
> > > drivers/clk/stm32/clk-stm32-core.c | 4 +--
> > > drivers/clk/stm32/clk-stm32f.c | 6 ++--
> > > drivers/clk/stm32/clk-stm32h7.c | 4 +--
> > > drivers/clk/tegra/tegra-car-clk.c | 2 +-
> > > drivers/clk/tegra/tegra186-clk.c | 2 +-
> > > drivers/clk/thead/clk-th1520-ap.c | 2 +-
> > > drivers/clk/ti/clk-am3-dpll-x2.c | 4 +--
> > > drivers/clk/ti/clk-divider.c | 4 +--
> > > drivers/clk/ti/clk-mux.c | 2 +-
> > > drivers/clk/ti/clk-sci.c | 2 +-
> > > drivers/clk/uniphier/clk-uniphier-core.c | 2 +-
> > > 65 files changed, 290 insertions(+), 290 deletions(-)
> > > ---
> > > base-commit: ecdc3872a767fb045be3296d4317ae978a14b022
> > > change-id: 20251010-clk_ops-3b7cc9ccd070
> > >
> > > Best regards,
> > > --
> > > Andrew Goodbody <andrew.goodbody at linaro.org>
> > >
> >
> > If you don't return an error, we cannot tell if the operation
> > succeeded, or not. U-Boot needs to be deterministic and we need to be
> > able to debug errors and detect them at runtime.
> >
> > We use ulong for the return value as a bit of a compromise, since it
> > is inefficient to use 64-bit on a 32-bit machine. Ideally it would be
> > long, but some clock rates are 3GHz and it would be confusing to cast
> > to ulong before using the value.
> >
> > An alternative we discussed was to return an integer error with the
> > clock rate returned in a parameter, but that seemed less efficient.
> >
> > With 64-bit machines, there really isn't a problem. Just checking for
> > a negative value is good enough, since the clock rate isn't going to
> > be 9 exahertz(?). Values between -CONFIG_ERR_PTR_OFFSET and 0 are
> > errors and are defined to be so.
> >
> > If you want clk_get_rate() to work like Linux (suppress / ignore
> > errors?), that's fine, but please create a clk_get_rate_err() (or
> > similar) which actually returns the correct error, and keep the error
> > return on the uclass interface. It is not uncommon to have the uclass
> > do some processing on values passed to/from driver. This allows people
> > who care to obtain the error.
>
> This is moving things in the right direction of having the error
> reporting and handling done where it can be done correctly. If there's
> further parts of the Linux kernel-like API we need, we can take those
> next.
Here is part of the patch:
--- a/drivers/clk/meson/a1.c
+++ b/drivers/clk/meson/a1.c
@@ -359,7 +359,7 @@ static ulong meson_div_get_rate(struct clk *clk,
unsigned long id)
info = meson_clk_get_info(clk, id, MESON_CLK_DIV);
if (IS_ERR(info))
- return PTR_ERR(info);
+ return 0;
I don't see anything in that fragment other than just ignoring errors.
Please, put this handling in the uclass function.
Regards,
Simon
More information about the U-Boot
mailing list