[PATCH 00/24] clk: Remove passing of negative errors through unsigned return

[Simon Glass]

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.

>
> 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.

Regards,
Simon