From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1B109CCD199 for ; Mon, 20 Oct 2025 04:32:25 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 36C4A83741; Mon, 20 Oct 2025 06:31:56 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="q0Jwp1k/"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 3069B83893; Sun, 19 Oct 2025 18:21:19 +0200 (CEST) Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5A6BE82E34 for ; Sun, 19 Oct 2025 18:21:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-oi1-x22f.google.com with SMTP id 5614622812f47-43f715fb494so1074866b6e.1 for ; Sun, 19 Oct 2025 09:21:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; t=1760890875; x=1761495675; darn=lists.denx.de; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=6IN2Hs0lFKnvBGPVYYKjrKyfNuOWDyrrkzUtPxragA8=; b=q0Jwp1k/lY6AZZxyko13d9PBh2eFLnxN1KbmgJ5VOHtscS2dYidrI4Xs1mku/QD/+M U/Lvt5nAVbXOaz696e6SPu3BU0P0/sZlK1mtCe+/Zzfi+B/ZZU7tjoKzLlO+C7CGXbBt +R/8xOk5DuT3HNpSy5kk4Zzj2uWUCLtR9mwTU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760890875; x=1761495675; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=6IN2Hs0lFKnvBGPVYYKjrKyfNuOWDyrrkzUtPxragA8=; b=vZ9QTXBACshvRspDekctWDaBy7q2m++wt1p79ZHu0I16xiGc71UfH4gBAKmTVdw8zz in2uvUQHqTML1ri27PVPIVLK++k3lR9jr3uKFQ/Cnpq2a9/b/IaBmrMhyNaodIfkEJfw dE5CxkeRQPigaSdrqRanrHKb1SwxDo5jdDcE/ZcA6UVAaUis3McxfXzTTKG+3UPfq6qF 3uOahnmBt0BgYUsB9iN+AJzUW/ktLhCkaPgSXmep9XbOYauzCeObNRnzVKMWg/p65S2J OU8vSntPzQRjGPJbxx/5MWHJbOlk6UPaB0QJPBwX8OJU5PvcnQ5gQ/ctXyGoqKMdF0BB SYYQ== X-Forwarded-Encrypted: i=1; AJvYcCUM6PEbcL6Vd3B7wyZl2eHvICFd/68P+ZpKy4SGynCpITr3elQW9Psv4qnIAEd43HMMvSabmfw=@lists.denx.de X-Gm-Message-State: AOJu0Yyju9I5lwSDGRUvZrcaG9Wkw1KntWEAxmOu4BKpQZpVEl4E/tQG Gaax+kVFRgXZXHZ++slzVAEFXu0gSnORJywlq+NhG+4d4zOYBf/svfxqX+GshM31FDE= X-Gm-Gg: ASbGncsTMQs/+7QJsT0SCVReTcx6kU49dlMmPlpvkQ1tl7Ohr8+GY3BNP3LFdkzHfX/ bgSO+3SBpU1K3Jib39iQPE/CL02Hr3SjM24C/OgmTPUh5NJD4O6Qb9AjK/7X5id2SdaMGEMR/1y B4HrJ5D+ThsakyEtcupJl2Ij22m4u1c30D/NUtKE6FWL9gVGArzDFvyY8kFnw0Fmvlumqi58H4n BeO7Y3d4QWgDAyGGi7pvIXzX9bq1q2K2g+T+xr5A4bgXi3f792YgmB8tIy4iqtvxhBRaseZMmDX jhMZPCOjxldS2mMXnIKhM2i7hEeSs1hhfyNwUvYqDNzXeM0H0azhFUqk6/f66XpfQlmnCaqfehI fv5spFvypfN7mTIvnmY8xM+Oe+k05Zl4qJ0+iqwZxL1yjdfoLKPwS7KNfN62j3TbE8LsBcRB+3J 52PHoxn8/IjNUfoRT7MNI25WKP+Px2dKShWH6LM9k= X-Google-Smtp-Source: AGHT+IH6+h06Ez5dYGOMMqLhD3Qwz/f8vkfBfeU32QVDe0Ndd8W7J6+MUOAX3I1kjHoFFYSvI9rrIw== X-Received: by 2002:a05:6808:320a:b0:441:fa58:12c5 with SMTP id 5614622812f47-443a2e6ae92mr4396070b6e.4.1760890874854; Sun, 19 Oct 2025 09:21:14 -0700 (PDT) Received: from bill-the-cat (fixed-187-190-202-235.totalplay.net. [187.190.202.235]) by smtp.gmail.com with ESMTPSA id 006d021491bc7-651d3f892e7sm1332460eaf.18.2025.10.19.09.21.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 Oct 2025 09:21:13 -0700 (PDT) Date: Sun, 19 Oct 2025 10:21:04 -0600 From: Tom Rini To: Svyatoslav Ryhel Cc: Andrew Goodbody , Simon Glass , u-boot-amlogic@groups.io, u-boot@lists.denx.de, Paul Barker , uboot-snps-arc@synopsys.com, Dai Okamura , Aspeed BMC SW team , Joel Stanley , GSS_MTK_Uboot_upstream , adsp-linux@analog.com, uboot-stm32@st-md-mailman.stormreply.com, Lukasz Majewski , Sean Anderson , Neil Armstrong , Stefan Roese , Yao Zi , Leo Yu-Chi Liang , Nobuhiro Iwamatsu , Marek Vasut , Philipp Tomsich , Kever Yang , Quentin Schulz , Lukasz Czechowski , Jonas Karlman , Finley Xiao , Joseph Chen , Elaine Zhang , Heiko Stuebner , Eugeniy Paltsev , Peng Fan , Liviu Dudau , Michal Simek , Patrice Chotard , Miquel Raynal , Patrick Delaunay , Ye Li , Sam Protsenko , Marek Vasut , Alice Guo , Valentin Caron , Padmarao Begari , Minda Chen , Hal Feng , Sumit Garg , Tien Fong Chee , Alif Zakuan Yuslaimi , Naresh Kumar Ravulapalli , Muhammad Hazim Izzat Zamri , Tingting Meng , Kunihiko Hayashi , Ryan Chen , Chia-Wei Wang , Minkyu Kang , Heiko Schocher , Jonathan Currier , Ryder Lee , Weijie Gao , Chunfeng Yun , Sam Shih , Manivannan Sadhasivam , Thierry Reding , Nathan Barrett-Morrison , Greg Malysa , Ian Roberts , Vasileios Bimpikas , Utsav Agarwal , Arturs Artamonovs , Gabriel Fernandez , Marek Vasut , Dario Binacchi Subject: Re: [PATCH 00/24] clk: Remove passing of negative errors through unsigned return Message-ID: <20251019162104.GP6688@bill-the-cat> References: <20251015-clk_ops-v1-0-5f80f827407e@linaro.org> <20251018140414.GN6688@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="MVDYA0qyRDr5Hc90" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Mailman-Approved-At: Mon, 20 Oct 2025 06:31:54 +0200 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean --MVDYA0qyRDr5Hc90 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Oct 19, 2025 at 04:45:24PM +0300, Svyatoslav Ryhel wrote: > =D0=BD=D0=B4, 19 =D0=B6=D0=BE=D0=B2=D1=82. 2025=E2=80=AF=D1=80. =D0=BE 16= :05 Simon Glass =D0=BF=D0=B8=D1=88=D0=B5: > > > > Hi Tom, Andrew, > > > > On Sat, 18 Oct 2025 at 15:04, Tom Rini 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 > > > > wrote: > > > > > > > > > > This series removes the passing of negative errors through the .g= et_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 easie= r code > > > > > porting and other maintenance in the future. > > > > > Another series will address the calling of clk_get_rate and assoc= iated > > > > > 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 numeric= al > > > 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 (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_ge= t_rate > > > > > clk: thead: th1520-ap: Remove negative error returns from c= lk_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_ra= te > > > > > clk: rockchip: Remove negative error returns from clk_get_r= ate > > > > > clk: Remove negative error returns from clk_get_rate > > > > > clk: starfive: Remove negative error returns from clk_get_r= ate > > > > > clk: altera: Remove negative error returns from clk_get_rate > > > > > clk: uniphier: Remove negative error returns from clk_get_r= ate > > > > > clk: aspeed: Remove negative error returns from clk_get_rate > > > > > clk: nuvoton: Remove negative error returns from clk_get_ra= te > > > > > 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_r= ate > > > > > 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 > > > > > > > > > > > > > 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 ca= st > > > > 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 f= or > > > > 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 err= or > > > > return on the uclass interface. It is not uncommon to have the ucla= ss > > > > do some processing on values passed to/from driver. This allows peo= ple > > > > 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 =3D 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. > > >=20 > I agree with Simon. This patchset may cause a lot of problems for all > boards it changes in case clk ops fail, since it silences all error > returns. Any clock error will be untrackable unless you know where and > what to look specifically. I think it's worth going back to the original thread: https://lore.kernel.org/u-boot/f5c94319-8ef8-459d-88b2-836702779cfb@linaro.= org/ As part of the problem is that what we have today does not work. This is why I'm think it's OK to first return 0, always, as the invalid clock rate and then re-introduce error checking that can work. --=20 Tom --MVDYA0qyRDr5Hc90 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQTzzqh0PWDgGS+bTHor4qD1Cr/kCgUCaPUP7AAKCRAr4qD1Cr/k CnUJAQC4Y1/4HLaZ2gerYnoDoBy4GATgqJ2oHDuwnFTZ/wPg2wD/fSjy4/nx6hPG 6bms6gFDId9Ib/qy2x70pibfmRgFDAg= =eTzR -----END PGP SIGNATURE----- --MVDYA0qyRDr5Hc90--