Hi Tom, Andrew, On Sat, 18 Oct 2025 at 15:04, Tom Rini <[email protected]> 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 > > <[email protected]> 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 <[email protected]> > > > --- > > > 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 <[email protected]> > > > > > > > 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
