From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from aposti.net (aposti.net [89.234.176.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB6E263AE for ; Wed, 5 Apr 2023 15:29:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crapouillou.net; s=mail; t=1680708591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mjb6j+bIXE7b8JKh7kXdb3mAp/fvDvyUInzyZqABl7c=; b=KkyZeRD8zRr9DUPi8E+yaAiJ6llkrkavOcg1hU9CiniByzzr5DJTX0SyfBAelvEuN8MeEv jL7uDqd+LBP5S8A6OuKPnyJCiz26onUF85KEMt1FysxEno2QjGawIYOL+G+P8JgGTbkscL 0Btl6oFLBPglYsRqL+QJzG3n9bKa4SU= Message-ID: <84dea45aa0a46f531d38369a31d08420dc43dfe3.camel@crapouillou.net> Subject: Re: [PATCH v2 56/65] clk: ingenic: cgu: Switch to determine_rate From: Paul Cercueil To: Maxime Ripard Cc: Aidan MacDonald , Stephen Boyd , Maxime Coquelin , Chen-Yu Tsai , Daniel Vetter , Nicolas Ferre , Thierry Reding , Jaroslav Kysela , Shawn Guo , Fabio Estevam , Ulf Hansson , Claudiu Beznea , Michael Turquette , Dinh Nguyen , Chunyan Zhang , Manivannan Sadhasivam , Andreas =?ISO-8859-1?Q?F=E4rber?= , Jonathan Hunter , Abel Vesa , Charles Keepax , Alessandro Zummo , Peter De Schrijver , Orson Zhai , Alexandre Torgue , Prashant Gaikwad , Liam Girdwood , Alexandre Belloni , Samuel Holland , Matthias Brugger , Richard Fitzgerald , Vinod Koul , NXP Linux Team , Sekhar Nori , Kishon Vijay Abraham I , Linus Walleij , Takashi Iwai , David Airlie , Luca Ceresoli , Jernej Skrabec , Pengutronix Kernel Team , Baolin Wang , David Lechner , Sascha Hauer , Mark Brown , Max Filippov , Geert Uytterhoeven , linux-stm32@st-md-mailman.stormreply.com, alsa-devel@alsa-project.org, linux-mediatek@lists.infradead.org, linux-phy@lists.infradead.org, linux-mips@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-actions@lists.infradead.org, linux-clk@vger.kernel.org, AngeloGioacchino Del Regno , patches@opensource.cirrus.com, linux-tegra@vger.kernel.org, linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Date: Wed, 05 Apr 2023 17:29:47 +0200 In-Reply-To: References: <20221107085417.xrsh6xy3ouwdkp4z@houat> <20221109110045.j24vwkaq3s4yzoy3@houat> <06a293adc75990ed3e297b076fc38d8a.sboyd@kernel.org> <20230324111959.frjf4neopbs67ugd@houat> <20230327192430.b2cp3yyrkzy4g4vw@penduick> <1e0e8e9fe44c27e844e7e918a985704e58da2c27.camel@crapouillou.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Le mercredi 05 avril 2023 =C3=A0 16:50 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > On Wed, Apr 05, 2023 at 02:57:26PM +0200, Paul Cercueil wrote: > > Le lundi 27 mars 2023 =C3=A0 21:24 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > > > On Fri, Mar 24, 2023 at 08:58:48PM +0000, Aidan MacDonald wrote: > > > > > > My suggestion: add a per-clock bitmap to keep track of > > > > > > which > > > > > > parents > > > > > > are allowed. Any operation that would select a parent clock > > > > > > not > > > > > > on the > > > > > > whitelist should fail. Automatic reparenting should only > > > > > > select > > > > > > from > > > > > > clocks on the whitelist. And we need new DT bindings for > > > > > > controlling > > > > > > the whitelist, for example: > > > > > >=20 > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-0 =3D <&clk1>, <&pll_c>; > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-1 =3D <&clk2>, <&pll_a>, <&pll= _b>; > > > > > >=20 > > > > > > This means that clk1 can only have pll_c as a parent, while > > > > > > clk2 can > > > > > > have pll_a or pll_b as parents. By default every clock will > > > > > > be > > > > > > able > > > > > > to use any parent, so a list is only needed if the machine > > > > > > needs a > > > > > > more restrictive policy. > > > > > >=20 > > > > > > assigned-clock-parents should disable automatic > > > > > > reparenting, > > > > > > but allow > > > > > > explicit clk_set_parent(). This will allow clock drivers to > > > > > > start doing > > > > > > reparenting without breaking old DTs. > > > > >=20 > > > > > I'm generally not a fan of putting all these policies in the > > > > > device > > > > > tree. Do you have an example where it wouldn't be possible to > > > > > do > > > > > exactly > > > > > this from the driver itself? > > > >=20 > > > > I'm confused. What's implicit in the example is clk1 and clk2 > > > > might > > > > have *other* possible choices of parent clock and the device > > > > tree > > > > is > > > > limiting what the OS is allowed to choose. > > > >=20 > > > > Why would you put such arbitrary limitations into the driver? > > >=20 > > > Why would we put such arbitrary limitations in the firmware? As > > > this > > > entire thread can attest, people are already using the device > > > tree to > > > work around the limitations of the Linux driver, or reduce the > > > features of Linux because they can rely on the device tree. > > > Either > > > way, it's linked to the state of the Linux driver, and any other > > > OS > > > or > > > Linux version could very well implement something more dynamic. > >=20 > > Probably because if we have to choose between setting policy in the > > kernel or in the firmware, it is arguably better to set it in the > > firmware. >=20 > I have a very different view on this I guess. Firmware is (most of > the > time) hard to update, and the policy depend on the state of support > of a > given OS so it's likely to evolve. The kernel is the best place to me > to > put that kind of policy. Why do you think differently? Because the clocks configuration can be board-specific. And you don't really want board-specific stuff in the driver. If we take the Ingenic JZ4770 SoC as example, on one board we parent everything we can to the PLL1 clock and set it to 432 MHz (the least common multiple). Then the PLL0 (which drives the DDR and CPU clocks) can be updated to overclock/underclock the CPU and RAM. So should that be hardcoded in the driver? Well, for a different board, for which overclock is not really needed, it's better to parent everything to PLL0 so that PLL1 can be shut down to save power. So what policy should be hardcoded in the driver? >=20 > > Especially when talking about clocks, as the firmware is already > > the > > one programming the boot clocks. >=20 > I'm not sure what your point is there. I don't think I ever saw a > firmware getting the clocks right for every possible scenario on a > given > platform. And if it was indeed the case, then we wouldn't even a > kernel > driver. My point is that there is already policy in how the firmware sets up the hardware; and most often than not, the kernel driver won't change that (e.g. you're probably not going to touch the DDR clock). It's better to have all policy in the firmware then, instead of having some in the firmware, and some in the kernel driver. Cheers, -Paul 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5F2F3C7619A for ; Wed, 5 Apr 2023 15:32:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CF79A10E9F3; Wed, 5 Apr 2023 15:32:22 +0000 (UTC) Received: from aposti.net (aposti.net [89.234.176.197]) by gabe.freedesktop.org (Postfix) with ESMTPS id CAC6B10E9F3 for ; Wed, 5 Apr 2023 15:32:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crapouillou.net; s=mail; t=1680708591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mjb6j+bIXE7b8JKh7kXdb3mAp/fvDvyUInzyZqABl7c=; b=KkyZeRD8zRr9DUPi8E+yaAiJ6llkrkavOcg1hU9CiniByzzr5DJTX0SyfBAelvEuN8MeEv jL7uDqd+LBP5S8A6OuKPnyJCiz26onUF85KEMt1FysxEno2QjGawIYOL+G+P8JgGTbkscL 0Btl6oFLBPglYsRqL+QJzG3n9bKa4SU= Message-ID: <84dea45aa0a46f531d38369a31d08420dc43dfe3.camel@crapouillou.net> Subject: Re: [PATCH v2 56/65] clk: ingenic: cgu: Switch to determine_rate From: Paul Cercueil To: Maxime Ripard Date: Wed, 05 Apr 2023 17:29:47 +0200 In-Reply-To: References: <20221107085417.xrsh6xy3ouwdkp4z@houat> <20221109110045.j24vwkaq3s4yzoy3@houat> <06a293adc75990ed3e297b076fc38d8a.sboyd@kernel.org> <20230324111959.frjf4neopbs67ugd@houat> <20230327192430.b2cp3yyrkzy4g4vw@penduick> <1e0e8e9fe44c27e844e7e918a985704e58da2c27.camel@crapouillou.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ulf Hansson , Prashant Gaikwad , Alexandre Belloni , Liam Girdwood , Michael Turquette , Sekhar Nori , Alexandre Torgue , dri-devel@lists.freedesktop.org, Jaroslav Kysela , Max Filippov , Thierry Reding , linux-phy@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, Abel Vesa , Kishon Vijay Abraham I , Geert Uytterhoeven , Samuel Holland , Chunyan Zhang , Takashi Iwai , linux-tegra@vger.kernel.org, Jernej Skrabec , Jonathan Hunter , Chen-Yu Tsai , NXP Linux Team , Orson Zhai , linux-mips@vger.kernel.org, Luca Ceresoli , linux-rtc@vger.kernel.org, linux-clk@vger.kernel.org, Charles Keepax , Aidan MacDonald , alsa-devel@alsa-project.org, Manivannan Sadhasivam , linux-kernel@vger.kernel.org, Sascha Hauer , linux-actions@lists.infradead.org, Richard Fitzgerald , Mark Brown , linux-mediatek@lists.infradead.org, Baolin Wang , Matthias Brugger , Pengutronix Kernel Team , linux-arm-kernel@lists.infradead.org, AngeloGioacchino Del Regno , Alessandro Zummo , linux-sunxi@lists.linux.dev, Stephen Boyd , patches@opensource.cirrus.com, Peter De Schrijver , Nicolas Ferre , Andreas =?ISO-8859-1?Q?F=E4rber?= , linux-renesas-soc@vger.kernel.org, Dinh Nguyen , Vinod Koul , Maxime Coquelin , David Lechner , Shawn Guo , Claudiu Beznea Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Le mercredi 05 avril 2023 =C3=A0 16:50 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > On Wed, Apr 05, 2023 at 02:57:26PM +0200, Paul Cercueil wrote: > > Le lundi 27 mars 2023 =C3=A0 21:24 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > > > On Fri, Mar 24, 2023 at 08:58:48PM +0000, Aidan MacDonald wrote: > > > > > > My suggestion: add a per-clock bitmap to keep track of > > > > > > which > > > > > > parents > > > > > > are allowed. Any operation that would select a parent clock > > > > > > not > > > > > > on the > > > > > > whitelist should fail. Automatic reparenting should only > > > > > > select > > > > > > from > > > > > > clocks on the whitelist. And we need new DT bindings for > > > > > > controlling > > > > > > the whitelist, for example: > > > > > >=20 > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-0 =3D <&clk1>, <&pll_c>; > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-1 =3D <&clk2>, <&pll_a>, <&pll= _b>; > > > > > >=20 > > > > > > This means that clk1 can only have pll_c as a parent, while > > > > > > clk2 can > > > > > > have pll_a or pll_b as parents. By default every clock will > > > > > > be > > > > > > able > > > > > > to use any parent, so a list is only needed if the machine > > > > > > needs a > > > > > > more restrictive policy. > > > > > >=20 > > > > > > assigned-clock-parents should disable automatic > > > > > > reparenting, > > > > > > but allow > > > > > > explicit clk_set_parent(). This will allow clock drivers to > > > > > > start doing > > > > > > reparenting without breaking old DTs. > > > > >=20 > > > > > I'm generally not a fan of putting all these policies in the > > > > > device > > > > > tree. Do you have an example where it wouldn't be possible to > > > > > do > > > > > exactly > > > > > this from the driver itself? > > > >=20 > > > > I'm confused. What's implicit in the example is clk1 and clk2 > > > > might > > > > have *other* possible choices of parent clock and the device > > > > tree > > > > is > > > > limiting what the OS is allowed to choose. > > > >=20 > > > > Why would you put such arbitrary limitations into the driver? > > >=20 > > > Why would we put such arbitrary limitations in the firmware? As > > > this > > > entire thread can attest, people are already using the device > > > tree to > > > work around the limitations of the Linux driver, or reduce the > > > features of Linux because they can rely on the device tree. > > > Either > > > way, it's linked to the state of the Linux driver, and any other > > > OS > > > or > > > Linux version could very well implement something more dynamic. > >=20 > > Probably because if we have to choose between setting policy in the > > kernel or in the firmware, it is arguably better to set it in the > > firmware. >=20 > I have a very different view on this I guess. Firmware is (most of > the > time) hard to update, and the policy depend on the state of support > of a > given OS so it's likely to evolve. The kernel is the best place to me > to > put that kind of policy. Why do you think differently? Because the clocks configuration can be board-specific. And you don't really want board-specific stuff in the driver. If we take the Ingenic JZ4770 SoC as example, on one board we parent everything we can to the PLL1 clock and set it to 432 MHz (the least common multiple). Then the PLL0 (which drives the DDR and CPU clocks) can be updated to overclock/underclock the CPU and RAM. So should that be hardcoded in the driver? Well, for a different board, for which overclock is not really needed, it's better to parent everything to PLL0 so that PLL1 can be shut down to save power. So what policy should be hardcoded in the driver? >=20 > > Especially when talking about clocks, as the firmware is already > > the > > one programming the boot clocks. >=20 > I'm not sure what your point is there. I don't think I ever saw a > firmware getting the clocks right for every possible scenario on a > given > platform. And if it was indeed the case, then we wouldn't even a > kernel > driver. My point is that there is already policy in how the firmware sets up the hardware; and most often than not, the kernel driver won't change that (e.g. you're probably not going to touch the DDR clock). It's better to have all policy in the firmware then, instead of having some in the firmware, and some in the kernel driver. Cheers, -Paul 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 00966C7619A for ; Wed, 5 Apr 2023 15:43:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ygiIlE/PJSCRYZvGjf4w200G8rBWFusTnN18W3pPMzk=; b=bi8m1ITf+own81 Mgp4Uxlz3YQJ622M6xBs0PjAQR/8GWGakRczp4fLKwJn5ZLu8r4K/CRSVanA08Q7MGpfjPbTbgJaJ ab9doSeVB7IKgCO+Xn7bSRCDjmbTbEiPt3TMwn8q2VljEaiDCKEEirHfrnNWFbsf+4YTpGClxY/yc XyG9Pl+iZfrb9LE3S5dYnI+s8nDFStifleCdfgHviAigsqwO2fUG8RCOLai8AG0BQe9cghzO8E8V3 ZUiAfHzr6UttkLXnxmec+jdi5mgfdldpxUQh0XbmEVba5CVop0f8rY2O5yMO0ii0VOVZUs1+kdO3Q ti3qnUmPE2LrZQA0qjsQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pk5Ic-004vej-2h; Wed, 05 Apr 2023 15:43:50 +0000 Received: from aposti.net ([89.234.176.197]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pk5H8-004tsL-2J; Wed, 05 Apr 2023 15:42:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crapouillou.net; s=mail; t=1680708591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mjb6j+bIXE7b8JKh7kXdb3mAp/fvDvyUInzyZqABl7c=; b=KkyZeRD8zRr9DUPi8E+yaAiJ6llkrkavOcg1hU9CiniByzzr5DJTX0SyfBAelvEuN8MeEv jL7uDqd+LBP5S8A6OuKPnyJCiz26onUF85KEMt1FysxEno2QjGawIYOL+G+P8JgGTbkscL 0Btl6oFLBPglYsRqL+QJzG3n9bKa4SU= Message-ID: <84dea45aa0a46f531d38369a31d08420dc43dfe3.camel@crapouillou.net> Subject: Re: [PATCH v2 56/65] clk: ingenic: cgu: Switch to determine_rate From: Paul Cercueil To: Maxime Ripard Cc: Aidan MacDonald , Stephen Boyd , Maxime Coquelin , Chen-Yu Tsai , Daniel Vetter , Nicolas Ferre , Thierry Reding , Jaroslav Kysela , Shawn Guo , Fabio Estevam , Ulf Hansson , Claudiu Beznea , Michael Turquette , Dinh Nguyen , Chunyan Zhang , Manivannan Sadhasivam , Andreas =?ISO-8859-1?Q?F=E4rber?= , Jonathan Hunter , Abel Vesa , Charles Keepax , Alessandro Zummo , Peter De Schrijver , Orson Zhai , Alexandre Torgue , Prashant Gaikwad , Liam Girdwood , Alexandre Belloni , Samuel Holland , Matthias Brugger , Richard Fitzgerald , Vinod Koul , NXP Linux Team , Sekhar Nori , Kishon Vijay Abraham I , Linus Walleij , Takashi Iwai , David Airlie , Luca Ceresoli , Jernej Skrabec , Pengutronix Kernel Team , Baolin Wang , David Lechner , Sascha Hauer , Mark Brown , Max Filippov , Geert Uytterhoeven , linux-stm32@st-md-mailman.stormreply.com, alsa-devel@alsa-project.org, linux-mediatek@lists.infradead.org, linux-phy@lists.infradead.org, linux-mips@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-actions@lists.infradead.org, linux-clk@vger.kernel.org, AngeloGioacchino Del Regno , patches@opensource.cirrus.com, linux-tegra@vger.kernel.org, linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Date: Wed, 05 Apr 2023 17:29:47 +0200 In-Reply-To: References: <20221107085417.xrsh6xy3ouwdkp4z@houat> <20221109110045.j24vwkaq3s4yzoy3@houat> <06a293adc75990ed3e297b076fc38d8a.sboyd@kernel.org> <20230324111959.frjf4neopbs67ugd@houat> <20230327192430.b2cp3yyrkzy4g4vw@penduick> <1e0e8e9fe44c27e844e7e918a985704e58da2c27.camel@crapouillou.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230405_084218_941686_94DB8071 X-CRM114-Status: GOOD ( 49.33 ) X-BeenThere: linux-phy@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux Phy Mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-phy" Errors-To: linux-phy-bounces+linux-phy=archiver.kernel.org@lists.infradead.org TGUgbWVyY3JlZGkgMDUgYXZyaWwgMjAyMyDDoCAxNjo1MCArMDIwMCwgTWF4aW1lIFJpcGFyZCBh IMOpY3JpdMKgOgo+IE9uIFdlZCwgQXByIDA1LCAyMDIzIGF0IDAyOjU3OjI2UE0gKzAyMDAsIFBh dWwgQ2VyY3VlaWwgd3JvdGU6Cj4gPiBMZSBsdW5kaSAyNyBtYXJzIDIwMjMgw6AgMjE6MjQgKzAy MDAsIE1heGltZSBSaXBhcmQgYSDDqWNyaXTCoDoKPiA+ID4gT24gRnJpLCBNYXIgMjQsIDIwMjMg YXQgMDg6NTg6NDhQTSArMDAwMCwgQWlkYW4gTWFjRG9uYWxkIHdyb3RlOgo+ID4gPiA+ID4gPiBN eSBzdWdnZXN0aW9uOiBhZGQgYSBwZXItY2xvY2sgYml0bWFwIHRvIGtlZXAgdHJhY2sgb2YKPiA+ ID4gPiA+ID4gd2hpY2gKPiA+ID4gPiA+ID4gcGFyZW50cwo+ID4gPiA+ID4gPiBhcmUgYWxsb3dl ZC4gQW55IG9wZXJhdGlvbiB0aGF0IHdvdWxkIHNlbGVjdCBhIHBhcmVudCBjbG9jawo+ID4gPiA+ ID4gPiBub3QKPiA+ID4gPiA+ID4gb24gdGhlCj4gPiA+ID4gPiA+IHdoaXRlbGlzdCBzaG91bGQg ZmFpbC4gQXV0b21hdGljIHJlcGFyZW50aW5nIHNob3VsZCBvbmx5Cj4gPiA+ID4gPiA+IHNlbGVj dAo+ID4gPiA+ID4gPiBmcm9tCj4gPiA+ID4gPiA+IGNsb2NrcyBvbiB0aGUgd2hpdGVsaXN0LiBB bmQgd2UgbmVlZCBuZXcgRFQgYmluZGluZ3MgZm9yCj4gPiA+ID4gPiA+IGNvbnRyb2xsaW5nCj4g PiA+ID4gPiA+IHRoZSB3aGl0ZWxpc3QsIGZvciBleGFtcGxlOgo+ID4gPiA+ID4gPiAKPiA+ID4g PiA+ID4gwqDCoMKgIGNsb2NrLXBhcmVudHMtMCA9IDwmY2xrMT4sIDwmcGxsX2M+Owo+ID4gPiA+ ID4gPiDCoMKgwqAgY2xvY2stcGFyZW50cy0xID0gPCZjbGsyPiwgPCZwbGxfYT4sIDwmcGxsX2I+ Owo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gVGhpcyBtZWFucyB0aGF0IGNsazEgY2FuIG9ubHkg aGF2ZSBwbGxfYyBhcyBhIHBhcmVudCwgd2hpbGUKPiA+ID4gPiA+ID4gY2xrMiBjYW4KPiA+ID4g PiA+ID4gaGF2ZSBwbGxfYSBvciBwbGxfYiBhcyBwYXJlbnRzLiBCeSBkZWZhdWx0IGV2ZXJ5IGNs b2NrIHdpbGwKPiA+ID4gPiA+ID4gYmUKPiA+ID4gPiA+ID4gYWJsZQo+ID4gPiA+ID4gPiB0byB1 c2UgYW55IHBhcmVudCwgc28gYSBsaXN0IGlzIG9ubHkgbmVlZGVkIGlmIHRoZSBtYWNoaW5lCj4g PiA+ID4gPiA+IG5lZWRzIGEKPiA+ID4gPiA+ID4gbW9yZSByZXN0cmljdGl2ZSBwb2xpY3kuCj4g PiA+ID4gPiA+IAo+ID4gPiA+ID4gPiBhc3NpZ25lZC1jbG9jay1wYXJlbnRzIHNob3VsZCBkaXNh YmxlIGF1dG9tYXRpYwo+ID4gPiA+ID4gPiByZXBhcmVudGluZywKPiA+ID4gPiA+ID4gYnV0IGFs bG93Cj4gPiA+ID4gPiA+IGV4cGxpY2l0IGNsa19zZXRfcGFyZW50KCkuIFRoaXMgd2lsbCBhbGxv dyBjbG9jayBkcml2ZXJzIHRvCj4gPiA+ID4gPiA+IHN0YXJ0IGRvaW5nCj4gPiA+ID4gPiA+IHJl cGFyZW50aW5nIHdpdGhvdXQgYnJlYWtpbmcgb2xkIERUcy4KPiA+ID4gPiA+IAo+ID4gPiA+ID4g SSdtIGdlbmVyYWxseSBub3QgYSBmYW4gb2YgcHV0dGluZyBhbGwgdGhlc2UgcG9saWNpZXMgaW4g dGhlCj4gPiA+ID4gPiBkZXZpY2UKPiA+ID4gPiA+IHRyZWUuIERvIHlvdSBoYXZlIGFuIGV4YW1w bGUgd2hlcmUgaXQgd291bGRuJ3QgYmUgcG9zc2libGUgdG8KPiA+ID4gPiA+IGRvCj4gPiA+ID4g PiBleGFjdGx5Cj4gPiA+ID4gPiB0aGlzIGZyb20gdGhlIGRyaXZlciBpdHNlbGY/Cj4gPiA+ID4g Cj4gPiA+ID4gSSdtIGNvbmZ1c2VkLiBXaGF0J3MgaW1wbGljaXQgaW4gdGhlIGV4YW1wbGUgaXMg Y2xrMSBhbmQgY2xrMgo+ID4gPiA+IG1pZ2h0Cj4gPiA+ID4gaGF2ZSAqb3RoZXIqIHBvc3NpYmxl IGNob2ljZXMgb2YgcGFyZW50IGNsb2NrIGFuZCB0aGUgZGV2aWNlCj4gPiA+ID4gdHJlZQo+ID4g PiA+IGlzCj4gPiA+ID4gbGltaXRpbmcgd2hhdCB0aGUgT1MgaXMgYWxsb3dlZCB0byBjaG9vc2Uu Cj4gPiA+ID4gCj4gPiA+ID4gV2h5IHdvdWxkIHlvdSBwdXQgc3VjaCBhcmJpdHJhcnkgbGltaXRh dGlvbnMgaW50byB0aGUgZHJpdmVyPwo+ID4gPiAKPiA+ID4gV2h5IHdvdWxkIHdlIHB1dCBzdWNo IGFyYml0cmFyeSBsaW1pdGF0aW9ucyBpbiB0aGUgZmlybXdhcmU/IEFzCj4gPiA+IHRoaXMKPiA+ ID4gZW50aXJlIHRocmVhZCBjYW4gYXR0ZXN0LCBwZW9wbGUgYXJlIGFscmVhZHkgdXNpbmcgdGhl IGRldmljZQo+ID4gPiB0cmVlIHRvCj4gPiA+IHdvcmsgYXJvdW5kIHRoZSBsaW1pdGF0aW9ucyBv ZiB0aGUgTGludXggZHJpdmVyLCBvciByZWR1Y2UgdGhlCj4gPiA+IGZlYXR1cmVzIG9mIExpbnV4 IGJlY2F1c2UgdGhleSBjYW4gcmVseSBvbiB0aGUgZGV2aWNlIHRyZWUuCj4gPiA+IEVpdGhlcgo+ ID4gPiB3YXksIGl0J3MgbGlua2VkIHRvIHRoZSBzdGF0ZSBvZiB0aGUgTGludXggZHJpdmVyLCBh bmQgYW55IG90aGVyCj4gPiA+IE9TCj4gPiA+IG9yCj4gPiA+IExpbnV4IHZlcnNpb24gY291bGQg dmVyeSB3ZWxsIGltcGxlbWVudCBzb21ldGhpbmcgbW9yZSBkeW5hbWljLgo+ID4gCj4gPiBQcm9i YWJseSBiZWNhdXNlIGlmIHdlIGhhdmUgdG8gY2hvb3NlIGJldHdlZW4gc2V0dGluZyBwb2xpY3kg aW4gdGhlCj4gPiBrZXJuZWwgb3IgaW4gdGhlIGZpcm13YXJlLCBpdCBpcyBhcmd1YWJseSBiZXR0 ZXIgdG8gc2V0IGl0IGluIHRoZQo+ID4gZmlybXdhcmUuCj4gCj4gSSBoYXZlIGEgdmVyeSBkaWZm ZXJlbnQgdmlldyBvbiB0aGlzIEkgZ3Vlc3MuIEZpcm13YXJlIGlzIChtb3N0IG9mCj4gdGhlCj4g dGltZSkgaGFyZCB0byB1cGRhdGUsIGFuZCB0aGUgcG9saWN5IGRlcGVuZCBvbiB0aGUgc3RhdGUg b2Ygc3VwcG9ydAo+IG9mIGEKPiBnaXZlbiBPUyBzbyBpdCdzIGxpa2VseSB0byBldm9sdmUuIFRo ZSBrZXJuZWwgaXMgdGhlIGJlc3QgcGxhY2UgdG8gbWUKPiB0bwo+IHB1dCB0aGF0IGtpbmQgb2Yg cG9saWN5LiBXaHkgZG8geW91IHRoaW5rIGRpZmZlcmVudGx5PwoKQmVjYXVzZSB0aGUgY2xvY2tz IGNvbmZpZ3VyYXRpb24gY2FuIGJlIGJvYXJkLXNwZWNpZmljLiBBbmQgeW91IGRvbid0CnJlYWxs eSB3YW50IGJvYXJkLXNwZWNpZmljIHN0dWZmIGluIHRoZSBkcml2ZXIuCgpJZiB3ZSB0YWtlIHRo ZSBJbmdlbmljIEpaNDc3MCBTb0MgYXMgZXhhbXBsZSwgb24gb25lIGJvYXJkIHdlIHBhcmVudApl dmVyeXRoaW5nIHdlIGNhbiB0byB0aGUgUExMMSBjbG9jayBhbmQgc2V0IGl0IHRvIDQzMiBNSHog KHRoZSBsZWFzdApjb21tb24gbXVsdGlwbGUpLiBUaGVuIHRoZSBQTEwwICh3aGljaCBkcml2ZXMg dGhlIEREUiBhbmQgQ1BVIGNsb2NrcykKY2FuIGJlIHVwZGF0ZWQgdG8gb3ZlcmNsb2NrL3VuZGVy Y2xvY2sgdGhlIENQVSBhbmQgUkFNLgoKU28gc2hvdWxkIHRoYXQgYmUgaGFyZGNvZGVkIGluIHRo ZSBkcml2ZXI/IFdlbGwsIGZvciBhIGRpZmZlcmVudCBib2FyZCwKZm9yIHdoaWNoIG92ZXJjbG9j ayBpcyBub3QgcmVhbGx5IG5lZWRlZCwgaXQncyBiZXR0ZXIgdG8gcGFyZW50CmV2ZXJ5dGhpbmcg dG8gUExMMCBzbyB0aGF0IFBMTDEgY2FuIGJlIHNodXQgZG93biB0byBzYXZlIHBvd2VyLiBTbyB3 aGF0CnBvbGljeSBzaG91bGQgYmUgaGFyZGNvZGVkIGluIHRoZSBkcml2ZXI/Cgo+IAo+ID4gRXNw ZWNpYWxseSB3aGVuIHRhbGtpbmcgYWJvdXQgY2xvY2tzLCBhcyB0aGUgZmlybXdhcmUgaXMgYWxy ZWFkeQo+ID4gdGhlCj4gPiBvbmUgcHJvZ3JhbW1pbmcgdGhlIGJvb3QgY2xvY2tzLgo+IAo+IEkn bSBub3Qgc3VyZSB3aGF0IHlvdXIgcG9pbnQgaXMgdGhlcmUuIEkgZG9uJ3QgdGhpbmsgSSBldmVy IHNhdyBhCj4gZmlybXdhcmUgZ2V0dGluZyB0aGUgY2xvY2tzIHJpZ2h0IGZvciBldmVyeSBwb3Nz aWJsZSBzY2VuYXJpbyBvbiBhCj4gZ2l2ZW4KPiBwbGF0Zm9ybS4gQW5kIGlmIGl0IHdhcyBpbmRl ZWQgdGhlIGNhc2UsIHRoZW4gd2Ugd291bGRuJ3QgZXZlbiBhCj4ga2VybmVsCj4gZHJpdmVyLgoK TXkgcG9pbnQgaXMgdGhhdCB0aGVyZSBpcyBhbHJlYWR5IHBvbGljeSBpbiBob3cgdGhlIGZpcm13 YXJlIHNldHMgdXAKdGhlIGhhcmR3YXJlOyBhbmQgbW9zdCBvZnRlbiB0aGFuIG5vdCwgdGhlIGtl cm5lbCBkcml2ZXIgd29uJ3QgY2hhbmdlCnRoYXQgKGUuZy4geW91J3JlIHByb2JhYmx5IG5vdCBn b2luZyB0byB0b3VjaCB0aGUgRERSIGNsb2NrKS4gSXQncwpiZXR0ZXIgdG8gaGF2ZSBhbGwgcG9s aWN5IGluIHRoZSBmaXJtd2FyZSB0aGVuLCBpbnN0ZWFkIG9mIGhhdmluZyBzb21lCmluIHRoZSBm aXJtd2FyZSwgYW5kIHNvbWUgaW4gdGhlIGtlcm5lbCBkcml2ZXIuCgpDaGVlcnMsCi1QYXVsCgot LSAKbGludXgtcGh5IG1haWxpbmcgbGlzdApsaW51eC1waHlAbGlzdHMuaW5mcmFkZWFkLm9yZwpo dHRwczovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1waHkK 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 alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BE84EC7618D for ; Thu, 6 Apr 2023 14:29:49 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id C34E7EA9; Thu, 6 Apr 2023 16:28:57 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz C34E7EA9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1680791387; bh=Mjb6j+bIXE7b8JKh7kXdb3mAp/fvDvyUInzyZqABl7c=; h=Subject:From:To:Date:In-Reply-To:References:CC:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=NcbhIgT/PU7KYvbnqf+BKy6/qaadfgjVXnYPDl5NdiTI7i1Vwnvs1lo7z2I3knb/6 wxfXb0/i2DG+Is5jGBF9PpcK6qu1RR3G0AKhCS30TnkKGzWPkbwVodED247h9JFJVI RNZrjKOKBcljMIEwO5lzk5PlwUdhU98MIUmKxNwk= Received: from mailman-core.alsa-project.org (mailman-core.alsa-project.org [10.254.200.10]) by alsa1.perex.cz (Postfix) with ESMTP id 03EC1F805A0; Thu, 6 Apr 2023 16:26:27 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id E0F79F80075; Wed, 5 Apr 2023 17:30:00 +0200 (CEST) Received: from aposti.net (aposti.net [89.234.176.197]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 53566F80075 for ; Wed, 5 Apr 2023 17:29:51 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 53566F80075 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key, unprotected) header.d=crapouillou.net header.i=@crapouillou.net header.a=rsa-sha256 header.s=mail header.b=KkyZeRD8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crapouillou.net; s=mail; t=1680708591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mjb6j+bIXE7b8JKh7kXdb3mAp/fvDvyUInzyZqABl7c=; b=KkyZeRD8zRr9DUPi8E+yaAiJ6llkrkavOcg1hU9CiniByzzr5DJTX0SyfBAelvEuN8MeEv jL7uDqd+LBP5S8A6OuKPnyJCiz26onUF85KEMt1FysxEno2QjGawIYOL+G+P8JgGTbkscL 0Btl6oFLBPglYsRqL+QJzG3n9bKa4SU= Message-ID: <84dea45aa0a46f531d38369a31d08420dc43dfe3.camel@crapouillou.net> Subject: Re: [PATCH v2 56/65] clk: ingenic: cgu: Switch to determine_rate From: Paul Cercueil To: Maxime Ripard Date: Wed, 05 Apr 2023 17:29:47 +0200 In-Reply-To: References: <20221107085417.xrsh6xy3ouwdkp4z@houat> <20221109110045.j24vwkaq3s4yzoy3@houat> <06a293adc75990ed3e297b076fc38d8a.sboyd@kernel.org> <20230324111959.frjf4neopbs67ugd@houat> <20230327192430.b2cp3yyrkzy4g4vw@penduick> <1e0e8e9fe44c27e844e7e918a985704e58da2c27.camel@crapouillou.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MailFrom: paul@crapouillou.net X-Mailman-Rule-Hits: max-recipients X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-alsa-devel.alsa-project.org-0; header-match-alsa-devel.alsa-project.org-1; nonmember-moderation; administrivia; implicit-dest; max-size; news-moderation; no-subject; digests; suspicious-header Message-ID-Hash: PNBDFVFZGSDQN3XYBESDYYHF32SM42UE X-Message-ID-Hash: PNBDFVFZGSDQN3XYBESDYYHF32SM42UE X-Mailman-Approved-At: Thu, 06 Apr 2023 14:26:03 +0000 CC: Aidan MacDonald , Stephen Boyd , Maxime Coquelin , Chen-Yu Tsai , Daniel Vetter , Nicolas Ferre , Thierry Reding , Shawn Guo , Fabio Estevam , Ulf Hansson , Claudiu Beznea , Michael Turquette , Dinh Nguyen , Chunyan Zhang , Manivannan Sadhasivam , Andreas =?ISO-8859-1?Q?F=E4rber?= , Jonathan Hunter , Abel Vesa , Charles Keepax , Alessandro Zummo , Peter De Schrijver , Orson Zhai , Alexandre Torgue , Prashant Gaikwad , Liam Girdwood , Alexandre Belloni , Samuel Holland , Matthias Brugger , Richard Fitzgerald , Vinod Koul , NXP Linux Team , Sekhar Nori , Kishon Vijay Abraham I , Linus Walleij , Takashi Iwai , David Airlie , Luca Ceresoli , Jernej Skrabec , Pengutronix Kernel Team , Baolin Wang , David Lechner , Sascha Hauer , Mark Brown , Max Filippov , Geert Uytterhoeven , linux-stm32@st-md-mailman.stormreply.com, alsa-devel@alsa-project.org, linux-mediatek@lists.infradead.org, linux-phy@lists.infradead.org, linux-mips@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-actions@lists.infradead.org, linux-clk@vger.kernel.org, AngeloGioacchino Del Regno , patches@opensource.cirrus.com, linux-tegra@vger.kernel.org, linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org X-Mailman-Version: 3.3.8 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Le mercredi 05 avril 2023 =C3=A0 16:50 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > On Wed, Apr 05, 2023 at 02:57:26PM +0200, Paul Cercueil wrote: > > Le lundi 27 mars 2023 =C3=A0 21:24 +0200, Maxime Ripard a =C3=A9crit=C2= =A0: > > > On Fri, Mar 24, 2023 at 08:58:48PM +0000, Aidan MacDonald wrote: > > > > > > My suggestion: add a per-clock bitmap to keep track of > > > > > > which > > > > > > parents > > > > > > are allowed. Any operation that would select a parent clock > > > > > > not > > > > > > on the > > > > > > whitelist should fail. Automatic reparenting should only > > > > > > select > > > > > > from > > > > > > clocks on the whitelist. And we need new DT bindings for > > > > > > controlling > > > > > > the whitelist, for example: > > > > > >=20 > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-0 =3D <&clk1>, <&pll_c>; > > > > > > =C2=A0=C2=A0=C2=A0 clock-parents-1 =3D <&clk2>, <&pll_a>, <&pll= _b>; > > > > > >=20 > > > > > > This means that clk1 can only have pll_c as a parent, while > > > > > > clk2 can > > > > > > have pll_a or pll_b as parents. By default every clock will > > > > > > be > > > > > > able > > > > > > to use any parent, so a list is only needed if the machine > > > > > > needs a > > > > > > more restrictive policy. > > > > > >=20 > > > > > > assigned-clock-parents should disable automatic > > > > > > reparenting, > > > > > > but allow > > > > > > explicit clk_set_parent(). This will allow clock drivers to > > > > > > start doing > > > > > > reparenting without breaking old DTs. > > > > >=20 > > > > > I'm generally not a fan of putting all these policies in the > > > > > device > > > > > tree. Do you have an example where it wouldn't be possible to > > > > > do > > > > > exactly > > > > > this from the driver itself? > > > >=20 > > > > I'm confused. What's implicit in the example is clk1 and clk2 > > > > might > > > > have *other* possible choices of parent clock and the device > > > > tree > > > > is > > > > limiting what the OS is allowed to choose. > > > >=20 > > > > Why would you put such arbitrary limitations into the driver? > > >=20 > > > Why would we put such arbitrary limitations in the firmware? As > > > this > > > entire thread can attest, people are already using the device > > > tree to > > > work around the limitations of the Linux driver, or reduce the > > > features of Linux because they can rely on the device tree. > > > Either > > > way, it's linked to the state of the Linux driver, and any other > > > OS > > > or > > > Linux version could very well implement something more dynamic. > >=20 > > Probably because if we have to choose between setting policy in the > > kernel or in the firmware, it is arguably better to set it in the > > firmware. >=20 > I have a very different view on this I guess. Firmware is (most of > the > time) hard to update, and the policy depend on the state of support > of a > given OS so it's likely to evolve. The kernel is the best place to me > to > put that kind of policy. Why do you think differently? Because the clocks configuration can be board-specific. And you don't really want board-specific stuff in the driver. If we take the Ingenic JZ4770 SoC as example, on one board we parent everything we can to the PLL1 clock and set it to 432 MHz (the least common multiple). Then the PLL0 (which drives the DDR and CPU clocks) can be updated to overclock/underclock the CPU and RAM. So should that be hardcoded in the driver? Well, for a different board, for which overclock is not really needed, it's better to parent everything to PLL0 so that PLL1 can be shut down to save power. So what policy should be hardcoded in the driver? >=20 > > Especially when talking about clocks, as the firmware is already > > the > > one programming the boot clocks. >=20 > I'm not sure what your point is there. I don't think I ever saw a > firmware getting the clocks right for every possible scenario on a > given > platform. And if it was indeed the case, then we wouldn't even a > kernel > driver. My point is that there is already policy in how the firmware sets up the hardware; and most often than not, the kernel driver won't change that (e.g. you're probably not going to touch the DDR clock). It's better to have all policy in the firmware then, instead of having some in the firmware, and some in the kernel driver. Cheers, -Paul