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 37C57C433EF for ; Fri, 26 Nov 2021 18:54:21 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D0BBD830F6; Fri, 26 Nov 2021 19:54:18 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (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="WhmumW1W"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5B20F81DB4; Fri, 26 Nov 2021 19:54:16 +0100 (CET) Received: from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com [IPv6:2607:f8b0:4864:20::72d]) (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 EE9C5830F6 for ; Fri, 26 Nov 2021 19:54:10 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x72d.google.com with SMTP id p4so15688598qkm.7 for ; Fri, 26 Nov 2021 10:54:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=D8Or41Yusp238hGafu2ik+V+V/gMiryVIea34/+Auhw=; b=WhmumW1WdzpUZ+zZyRlQWFtoQaHwKjwHNLeZ5XuUeIjUcjLTQ7MI/DqhHXETFmpbTj ubwUYV4XaLlYL/9gUxRmD635L432XMKRK5L4k8GEEUpWqqRNqWDW8wOpizLmRilMKu99 W8Qot38Z7ljybkiD0NwAo3k+3gzBo/9ypcaIs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=D8Or41Yusp238hGafu2ik+V+V/gMiryVIea34/+Auhw=; b=xodJcVEDlk1b9EkV3AtBlivSYgbUm5eP3BUpzHEsC7j4yN4aPt9rFcCe//TZf4sp6j upU6CuhY3kiDo8R0RoYMlwh8CPgDardZmGKjH7vBWevx9SEkxLcVPaf4F1AQuCAYwydu kgA2blVGFMRAdv2IHtS37FVeOfnBJRP+pOBXjJ4yeLgQE+EaujY9/fJ/DN+Pn7j17Bea qhDsBJvc65WHfgQ4LQMESxICYY/8QZy/W9fxCmEJvoIXM7R2bUZRUEPAfa0ZqUOOjj8c EvrnUKN8BwZSUtrWPAzgu12Zn8q3IYMe5uUckc9KHyuPYAhSC7J7pNeL83GBo71PUjG5 VbKw== X-Gm-Message-State: AOAM5330p7o+1xb9MsGQznlpUhO6IIiBGeaKLfezViaBA5Un3wo+jLOR yZSSA57U0ZxSBOi9+IU3tPeGWA== X-Google-Smtp-Source: ABdhPJwYQ+xF+lKHSI43BhGqZ0h3V1Eny8JcE5qr8zqbhqifgWIShL64f6yvKc6Cr59YrR+/JiWaWw== X-Received: by 2002:a05:620a:2494:: with SMTP id i20mr15753519qkn.624.1637952849505; Fri, 26 Nov 2021 10:54:09 -0800 (PST) Received: from bill-the-cat (2603-6081-7b01-cbda-59cd-bfe9-f38b-99c0.res6.spectrum.com. [2603:6081:7b01:cbda:59cd:bfe9:f38b:99c0]) by smtp.gmail.com with ESMTPSA id 15sm3863168qtp.55.2021.11.26.10.54.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Nov 2021 10:54:08 -0800 (PST) Date: Fri, 26 Nov 2021 13:54:06 -0500 From: Tom Rini To: Sean Anderson Cc: "Peng Fan (OSS)" , Rob Herring , "lukma@denx.de" , "sjg@chromium.org" , "u-boot@lists.denx.de" Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults Message-ID: <20211126185406.GX24579@bill-the-cat> References: <20211029012801.15193-1-peng.fan@oss.nxp.com> <20211120125711.GO24579@bill-the-cat> <20211122132204.GV24579@bill-the-cat> <20211124141028.GS24579@bill-the-cat> <5a180e8f-ad09-d907-9735-caac38992afc@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="UA6dQAml4K7cbSXU" Content-Disposition: inline In-Reply-To: <5a180e8f-ad09-d907-9735-caac38992afc@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.37 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.2 at phobos.denx.de X-Virus-Status: Clean --UA6dQAml4K7cbSXU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 26, 2021 at 01:17:10PM -0500, Sean Anderson wrote: > On 11/24/21 9:10 AM, Tom Rini wrote: > > On Tue, Nov 23, 2021 at 09:16:14PM -0500, Sean Anderson wrote: > > > On 11/22/21 10:02 PM, Peng Fan (OSS) wrote: > > > > > Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults > > > > >=20 > > > > > On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote: > > > > > > + Rob > > > > > >=20 > > > > > > On 2021/11/20 20:57, Tom Rini wrote: > > > > > > > On Sat, Nov 20, 2021 at 12:10:54PM +0000, Peng Fan (OSS) wrot= e: > > > > > > > > > Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defa= ults > > > > > > > > >=20 > > > > > > > > > From: Peng Fan > > > > > > > > >=20 > > > > > > > > > Current code has a force clk_set_defaults in multiple sta= ges, > > > > > > > > > U-Boot reuse the same device tree and Linux Kernel device= tree, > > > > > > > > > but we not register all the clks as Linux Kernel, so > > > > > > > > > clk_set_defaults will fail and cause the clk provider reg= isteration fail. > > > > > > > > >=20 > > > > > > > > > So introduce a new property to ignore the default setting= s which > > > > > > > > > could be used by any node that wanna ignore default setti= ngs. > > > > > > > > >=20 > > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > --- > > > > > > > > >=20 > > > > > > > > > V2: > > > > > > > > > Add R-b tag > > > > > > > > > Tom, Simon > > > > > > > > > After a thought, I think still put it as a u-boot t= hing. > > > > > assigned-clock-x is > > > > > > > > > actually Linux specific, however I could not add th= e new property > > > > > to Linux, > > > > > > > > > because we are supporting SystemReady-IR, we need t= he > > > > > > > > > assigned-clock-x property > > > > > > > > > in linux working and ignore it in U-Boot. > > > > > > > >=20 > > > > > > > > Any more thoughts? > > > > > > >=20 > > > > > > > Just my continued request that you treat this as generic and = submit > > > > > > > the binding upstream so it can be in the device tree for the = platform. > > > > > > >=20 > > > > > >=20 > > > > > > As Sean said, this is to serve cast that linux and U-Boot use t= he same > > > > > > device tree, I mean U-Boot runtime export device tree to linux = for > > > > > > SR-IR (system-ready IR) booting. > > > > > >=20 > > > > > > Linux needs assigned-clocks to some reason, but U-Boot not need= that > > > > > > because the driver not added the support or not a must to have = that. > > > > > >=20 > > > > > > Because assigned-clocks failure in U-Boot will cause probe fail= now, > > > > > > the device driver will report failure. > > > > > >=20 > > > > > > You mean rename this to "ignore-clk-defaults" or keep > > > > > > "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" t= o linux > > > > > > device tree binding? > > > > > >=20 > > > > > > I could try to send to linux kernel with "firmware" as a prefix. > > > > >=20 > > > > > What I mean is that first I'm not seeing the description of the p= roperty as > > > > > being clear enough, either in commit message or the binding itsel= f. > > > > > That's why in my mind I keep seeing this as "we set the properties > > > > > linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't= know why > > > > > we need both. Is there not something we can do based on seeing > > > > > linux,assigned-clocks ? Showing something that makes use of the = property > > > > > you're wishing to add would be helpful. In fact, some specific d= ts snippets > > > > > would be helpful to understand what's going on here. > > > >=20 > > > > clk: clock-controller@30380000 { > > > > compatible =3D "fsl,imx8mp-ccm"; > > > > reg =3D <0x30380000 0x10000>; > > > > #clock-cells =3D <1>; > > > > clocks =3D <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext= 2>, > > > > <&clk_ext3>, <&clk_ext4>; > > > > clock-names =3D "osc_32k", "osc_24m", "clk_ext1", "clk_ex= t2", > > > > "clk_ext3", "clk_ext4"; > > > > assigned-clocks =3D <&clk IMX8MP_CLK_A53_SRC>, > > > > <&clk IMX8MP_CLK_A53_CORE>, > > > > <&clk IMX8MP_CLK_NOC>, > > > > <&clk IMX8MP_CLK_NOC_IO>, > > > > <&clk IMX8MP_CLK_GIC>, > > > > <&clk IMX8MP_CLK_AUDIO_AHB>, > > > > <&clk IMX8MP_CLK_AUDIO_AXI_SRC>, > > > > <&clk IMX8MP_CLK_IPG_AUDIO_ROOT>, > > > > <&clk IMX8MP_AUDIO_PLL1>, > > > > <&clk IMX8MP_AUDIO_PLL2>; > > > > assigned-clock-parents =3D <&clk IMX8MP_SYS_PLL1_800M>, > > > > <&clk IMX8MP_ARM_PLL_OUT>, > > > > <&clk IMX8MP_SYS_PLL2_1000M>, > > > > <&clk IMX8MP_SYS_PLL1_800M>, > > > > <&clk IMX8MP_SYS_PLL2_500M>, > > > > <&clk IMX8MP_SYS_PLL1_800M>, > > > > <&clk IMX8MP_SYS_PLL1_800M>; > > > > assigned-clock-rates =3D <0>, <0>, > > > > <1000000000>, > > > > <800000000>, > > > > <500000000>, > > > > <400000000>, > > > > <800000000>, > > > > <400000000>, > > > > <393216000>, > > > > <361267200>; > > > > u-boot,ignore-clk-defaults; > > > > }; > > > >=20 > > > > The above node is that I wanna have in U-Boot device tree. > > > > This node is same as the one used in linux device tree except the n= ew > > > > property introduced here. > > > >=20 > > > > In i.MX U-Boot, we not support the clks here in the clk driver, but= linux needs the assigned-clocks property, so I could not delete it > > > > because U-Boot runtime export the device tree to Linux. > > > >=20 > > > > So add a new property here for U-Boot specific usage, if the proper= ty > > > > is there, U-Boot ignore the assigned-clock settings for a node. > > > >=20 > > > > I hope this is clear and please suggest what to do next. > > >=20 > > > Hm. Well perhaps we can designate a specific return code to indicate > > > that we have a valid clock ID but it is just unimplemented. So in your > > > driver you would do > > >=20 > > > ulong my_get_rate(struct clk *clk) > > > { > > > switch (clk->id) { > > > case MY_CLK: > > > ... > > > default: > > > if (clk->id < MY_CLK_MAX) > > > return -ENOSYS; > > > else > > > return -EINVAL; > > > } > > > } > > >=20 > > > and then the clock rate assignment would see -ENOSYS and go on its me= rry > > > way. > >=20 > > Or even just have the driver that matches on "fsl,imx8mp-ccm" have a > > log_debug("Leaving clocks as-is, OS will handle them\n"); > > and returning success? > >=20 >=20 > assigned-clocks is handled by the uclass, so that seems a bit invasive fo= r me. I'm probably missing something still, sorry. We'd get kicked over to drivers/clk/imx/clk-imx8mp.c in this case, from the uclass figuring out who should own these clocks, right? I'm wondering why / if clk-imx8mp.c can't figure out what's appropriate to do (or not do, or otherwise just ignore an error). --=20 Tom --UA6dQAml4K7cbSXU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmGhLUcACgkQFHw5/5Y0 tyxP4Av+POdf4NcY9sH2gzrz1Am30bWgtyKY4l/8i1rGvz3l+JBuIniSmi5CXLNV xOD/fLNpRIhf8JtEUoPGIlhkF84945brM03cMpe5QNUIjHv2V1jwHRA+B0Vu+efg lnp/Rs1OirjPwOaMJxWuBKEEc62DSnXXqyKsbijYss2dRVu9EBzYSdxeVS38ThJC bMrhZujik5NvwOWsZheRhtpoIw0/6vyKdEZFZqdbkA7js5z38w0Flh5DeF3lmvN2 X2e14+VjlhIi9yXYY+vlT4KXTLyXNSzpkFI3/imjO7q8SL8vzrkXWvL4FzURB6Kw ExprG2n+RHWi30QBG95yUrpt+CgeIXHLjWLaX+xN3mH2PN4nd9N8XkYls25fgLSZ y+FoORSVIBzFUZ6pyjMTWmxZhYpsueGSuPk/sSYvelpiK6JO1p9lwzeVJ9cHv4ME mf7C2dquW5+vJxQatX1z/mOsVnRjPcfvtAXo3Bmasov/JfTyrQ6WtLphLtInsHCM Il4zxI3I =GGv4 -----END PGP SIGNATURE----- --UA6dQAml4K7cbSXU--