[PATCH 00/24] clk: Remove passing of negative errors through unsigned return
Tom Rini
trini at konsulko.com
Sat Oct 18 16:04:14 CEST 2025
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.
> > 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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251018/88a781ef/attachment.sig>
More information about the U-Boot
mailing list