From: Stephen Boyd <sboyd@kernel.org> To: <u.kleine-koenig@pengutronix.de> Cc: Russell King <linux@armlinux.org.uk>, alexandre.belloni@bootlin.com, Michael Turquette <mturquette@baylibre.com>, thierry.reding@gmail.com, lee.jones@linaro.org, linux-clk@vger.kernel.org, linux-rtc@vger.kernel.org, Ludovic.Desroches@microchip.com, o.rempel@pengutronix.de, andy.shevchenko@gmail.com, aardelean@deviqon.com, linux-pwm@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, broonie@kernel.org, Jonathan.Cameron@huawei.com, linux-arm-kernel@lists.infradead.org, a.zummo@towertech.it, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, wsa@kernel.org, kernel@pengutronix.de, akpm@linux-foundation.org, torvalds@linux-foundation.org, Claudiu.Beznea@microchip.com Subject: Re: About clk maintainership [Was: Re: [PULL] Add variants of devm_clk_get for prepared and enabled clocks enabled clocks] Date: Thu, 05 Aug 2021 17:26:16 -0700 [thread overview] Message-ID: <162820957661.19113.17221558053361108175@swboyd.mtv.corp.google.com> (raw) In-Reply-To: <20210803104012.wf2buscbukxufesl@pengutronix.de> Quoting Uwe Kleine-König (2021-08-03 03:40:12) > On Tue, Aug 03, 2021 at 01:11:54AM -0700, Stephen Boyd wrote: > > > > Maybe this series would be more compelling if those various drivers that > > are hand rolling the devm action were converted to the consolidated > > official devm function. The truth is it's already happening in various > > subsystems so consolidating that logic into one place would be a win > > code size wise and very hard to ignore. > > > > Doing > > > > $ git grep devm_add_action | grep clk > > > > seems to catch quite a few of them. > > Another upside is that grepping for these drivers with a potential for > further improvement become easier to grep for as > devm_clk_get_{prepared,enabled} is a much better hint :-) Sorry, but that's a pretty weak argument. I'd think grepping for the absence of pm_ops in drivers would be the same hint. > > The changes to these drivers probably won't go through a clk tree, so > adding these patches before adding devm_clk_get_enabled() would only > help for the warm and cozy feeling that it is right to do so, correct? It isn't to feel warm and cozy. It's to demonstrate the need for consolidating code. Converting the i2c and spi drivers to use this is actively damaging the cause though. Those driver frameworks are more likely to encourage proper power management around bus transfers, so converting them to use the devm API moves them away from power management, not closer to it. This proves why this topic is always contentious. It's too easy to blindly convert drivers to get the clk and leave it enabled forever and then they never use power management. The janitors win and nobody else. Is there some way to avoid that trap? Maybe through some combination of a device PM function that indicates the driver has no runtime PM callbacks (pm_runtime_no_callbacks() perhaps?) and devm_clk_get_enabled() checking for that and returning the clk only when that call has been made (i.e. pm_runtime_has_no_callbacks())? This approach would fail to catch the case where system wide suspend/resume could turn the clk off but the driver doesn't do it. I'm not sure how much we care about that case though. > > As my focus is limited to (mostly) drivers/pwm and I already have quite > some other patch quests on my list: Don't we all? :)
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org> To: <u.kleine-koenig@pengutronix.de> Cc: Russell King <linux@armlinux.org.uk>, alexandre.belloni@bootlin.com, Michael Turquette <mturquette@baylibre.com>, thierry.reding@gmail.com, lee.jones@linaro.org, linux-clk@vger.kernel.org, linux-rtc@vger.kernel.org, Ludovic.Desroches@microchip.com, o.rempel@pengutronix.de, andy.shevchenko@gmail.com, aardelean@deviqon.com, linux-pwm@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, broonie@kernel.org, Jonathan.Cameron@huawei.com, linux-arm-kernel@lists.infradead.org, a.zummo@towertech.it, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, wsa@kernel.org, kernel@pengutronix.de, akpm@linux-foundation.org, torvalds@linux-foundation.org, Claudiu.Beznea@microchip.com Subject: Re: About clk maintainership [Was: Re: [PULL] Add variants of devm_clk_get for prepared and enabled clocks enabled clocks] Date: Thu, 05 Aug 2021 17:26:16 -0700 [thread overview] Message-ID: <162820957661.19113.17221558053361108175@swboyd.mtv.corp.google.com> (raw) In-Reply-To: <20210803104012.wf2buscbukxufesl@pengutronix.de> Quoting Uwe Kleine-König (2021-08-03 03:40:12) > On Tue, Aug 03, 2021 at 01:11:54AM -0700, Stephen Boyd wrote: > > > > Maybe this series would be more compelling if those various drivers that > > are hand rolling the devm action were converted to the consolidated > > official devm function. The truth is it's already happening in various > > subsystems so consolidating that logic into one place would be a win > > code size wise and very hard to ignore. > > > > Doing > > > > $ git grep devm_add_action | grep clk > > > > seems to catch quite a few of them. > > Another upside is that grepping for these drivers with a potential for > further improvement become easier to grep for as > devm_clk_get_{prepared,enabled} is a much better hint :-) Sorry, but that's a pretty weak argument. I'd think grepping for the absence of pm_ops in drivers would be the same hint. > > The changes to these drivers probably won't go through a clk tree, so > adding these patches before adding devm_clk_get_enabled() would only > help for the warm and cozy feeling that it is right to do so, correct? It isn't to feel warm and cozy. It's to demonstrate the need for consolidating code. Converting the i2c and spi drivers to use this is actively damaging the cause though. Those driver frameworks are more likely to encourage proper power management around bus transfers, so converting them to use the devm API moves them away from power management, not closer to it. This proves why this topic is always contentious. It's too easy to blindly convert drivers to get the clk and leave it enabled forever and then they never use power management. The janitors win and nobody else. Is there some way to avoid that trap? Maybe through some combination of a device PM function that indicates the driver has no runtime PM callbacks (pm_runtime_no_callbacks() perhaps?) and devm_clk_get_enabled() checking for that and returning the clk only when that call has been made (i.e. pm_runtime_has_no_callbacks())? This approach would fail to catch the case where system wide suspend/resume could turn the clk off but the driver doesn't do it. I'm not sure how much we care about that case though. > > As my focus is limited to (mostly) drivers/pwm and I already have quite > some other patch quests on my list: Don't we all? :) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-08-06 0:26 UTC|newest] Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-10 17:41 [PATCH v7 0/6] clk: provide new devm helpers for prepared and enabled clocks Uwe Kleine-König 2021-05-10 17:41 ` Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 1/6] clk: generalize devm_clk_get() a bit Uwe Kleine-König 2021-05-10 17:41 ` Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 2/6] clk: Provide new devm_clk_helpers for prepared and enabled clocks Uwe Kleine-König 2021-05-10 17:41 ` Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 3/6] pwm: atmel: Simplify using devm_clk_get_prepared() Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 4/6] rtc: at91sam9: Simplify using devm_clk_get_enabled() Uwe Kleine-König 2021-05-10 17:41 ` Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 5/6] i2c: imx: " Uwe Kleine-König 2021-05-10 17:41 ` Uwe Kleine-König 2021-05-10 17:41 ` [PATCH v7 6/6] spi: davinci: " Uwe Kleine-König 2021-05-11 7:58 ` [PATCH v7 0/6] clk: provide new devm helpers for prepared and enabled clocks Alexandru Ardelean 2021-05-11 7:58 ` Alexandru Ardelean 2021-05-24 11:09 ` Uwe Kleine-König 2021-05-24 11:09 ` Uwe Kleine-König 2021-06-09 20:21 ` [PULL] Add variants of devm_clk_get for prepared and enabled clocks " Uwe Kleine-König 2021-06-25 17:14 ` Uwe Kleine-König 2021-06-25 17:14 ` Uwe Kleine-König 2021-07-05 8:01 ` Uwe Kleine-König 2021-07-05 8:01 ` Uwe Kleine-König 2021-07-22 6:06 ` Uwe Kleine-König 2021-07-22 6:06 ` Uwe Kleine-König 2021-07-22 7:40 ` Wolfram Sang 2021-07-22 7:40 ` Wolfram Sang 2021-07-22 8:18 ` Uwe Kleine-König 2021-07-22 8:18 ` Uwe Kleine-König 2021-07-22 12:07 ` Wolfram Sang 2021-07-22 12:07 ` Wolfram Sang [not found] ` <CAHp75VfC=s12Unw3+Cn0ag71mM5i90=Jbwj4nYwB5cPKiUTRSA@mail.gmail.com> 2021-07-23 9:13 ` Uwe Kleine-König 2021-07-23 9:13 ` Uwe Kleine-König 2021-07-26 9:18 ` Claudiu.Beznea 2021-07-26 9:18 ` Claudiu.Beznea 2021-07-26 9:52 ` Andy Shevchenko 2021-07-26 9:52 ` Andy Shevchenko 2021-07-26 12:32 ` Wolfram Sang 2021-07-26 12:32 ` Wolfram Sang 2021-07-26 13:28 ` Andy Shevchenko 2021-07-26 13:28 ` Andy Shevchenko 2021-07-26 17:40 ` Wolfram Sang 2021-07-26 17:40 ` Wolfram Sang 2021-07-28 20:25 ` About clk maintainership [Was: Re: [PULL] Add variants of devm_clk_get for prepared and enabled clocks enabled clocks] Uwe Kleine-König 2021-07-28 20:25 ` Uwe Kleine-König 2021-07-28 20:40 ` Russell King (Oracle) 2021-07-28 20:40 ` Russell King (Oracle) 2021-07-31 7:41 ` Stephen Boyd 2021-07-31 7:41 ` Stephen Boyd 2021-07-31 8:07 ` Andy Shevchenko 2021-07-31 8:07 ` Andy Shevchenko 2021-07-31 12:00 ` Uwe Kleine-König 2021-07-31 12:00 ` Uwe Kleine-König 2021-08-02 9:36 ` Jonathan Cameron 2021-08-02 9:36 ` Jonathan Cameron 2021-08-02 9:48 ` Russell King (Oracle) 2021-08-02 9:48 ` Russell King (Oracle) 2021-08-02 15:27 ` Uwe Kleine-König 2021-08-02 15:27 ` Uwe Kleine-König 2021-08-02 16:38 ` Russell King (Oracle) 2021-08-02 16:38 ` Russell King (Oracle) 2021-08-02 17:13 ` Andy Shevchenko 2021-08-02 17:13 ` Andy Shevchenko 2021-08-02 20:28 ` Russell King (Oracle) 2021-08-02 20:28 ` Russell King (Oracle) 2021-08-03 8:11 ` Stephen Boyd 2021-08-03 8:11 ` Stephen Boyd 2021-08-03 10:40 ` Uwe Kleine-König 2021-08-03 10:40 ` Uwe Kleine-König 2021-08-06 0:26 ` Stephen Boyd [this message] 2021-08-06 0:26 ` Stephen Boyd 2021-09-14 13:22 ` Uwe Kleine-König 2021-09-14 13:22 ` Uwe Kleine-König 2021-09-14 13:52 ` Mark Brown 2021-09-14 13:52 ` Mark Brown 2021-08-03 7:44 ` Stephen Boyd 2021-08-03 7:44 ` Stephen Boyd
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=162820957661.19113.17221558053361108175@swboyd.mtv.corp.google.com \ --to=sboyd@kernel.org \ --cc=Claudiu.Beznea@microchip.com \ --cc=Jonathan.Cameron@huawei.com \ --cc=Ludovic.Desroches@microchip.com \ --cc=a.zummo@towertech.it \ --cc=aardelean@deviqon.com \ --cc=akpm@linux-foundation.org \ --cc=alexandre.belloni@bootlin.com \ --cc=andy.shevchenko@gmail.com \ --cc=arnd@arndb.de \ --cc=broonie@kernel.org \ --cc=kernel@pengutronix.de \ --cc=lee.jones@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pwm@vger.kernel.org \ --cc=linux-rtc@vger.kernel.org \ --cc=linux-spi@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mturquette@baylibre.com \ --cc=o.rempel@pengutronix.de \ --cc=thierry.reding@gmail.com \ --cc=torvalds@linux-foundation.org \ --cc=u.kleine-koenig@pengutronix.de \ --cc=wsa@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.