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 2369AC433F5 for ; Thu, 19 May 2022 23:46:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E68BA10E0F4; Thu, 19 May 2022 23:46:04 +0000 (UTC) Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9B09610E033 for ; Thu, 19 May 2022 23:46:02 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 1A109B828AF; Thu, 19 May 2022 23:46:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 90C6DC385AA; Thu, 19 May 2022 23:45:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1653003959; bh=J/86NYRxXvo0kg+4FsDJ+dYPDoSwx3afdJ6QdTs8OyA=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=D9rd4xxAzi+lnV0CSGhJNepB2FAXckzHnWQwdFd5MSxsL+/jhg/jVTREPtoj0gGPG ctTGSF3Qk1uBFZoNhz7QvMwg5z+JWEdvsXVn2B+6/x3PZNVX0rt4otOkfWITG/IOYu qlYc+Y3KAjPU+sRXBXmj5MtTMJTUshr/hvqzi7fPcZxCRnpAQyE1mZb6aBiBrH6AfX 2L3+RMnZitJdoVtdpLLXMG2scFNHhq9OmgViHvJYun/lulKu747Xnf1H9aTJkyORy+ 9riGd3tUDuYRThZzmJVDGht+HDyYrVa6q9aGv+kZ6tgdLl6XFt67peXSMoK1CJ9o/X 8i9/Qk6HWLj+A== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20220314141643.22184-3-u.kleine-koenig@pengutronix.de> References: <20220314141643.22184-1-u.kleine-koenig@pengutronix.de> <20220314141643.22184-3-u.kleine-koenig@pengutronix.de> Subject: Re: [PATCH v8 02/16] clk: Provide new devm_clk helpers for prepared and enabled clocks From: Stephen Boyd To: Alessandro Zummo , Alexandre Belloni , Bartosz Golaszewski , Claudiu Beznea , Daniel Vetter , David Airlie , Fabio Estevam , Greg Kroah-Hartman , Guenter Roeck , Herbert Xu , Jean Delvare , Jerome Brunet , Jonathan Cameron , Kevin Hilman , Lars Povlsen , Lars-Peter Clausen , Lee Jones , Linus Walleij , Mark Brown , Martin Blumenstingl , Matt Mackall , Michael Hennerich , Michael Turquette , NXP Linux Team , Neil Armstrong , Nicolas Ferre , Nuno =?utf-8?q?S=C3=A1?= , Oleksij Rempel , Paul Cercueil , Russell King , Shawn Guo , Steen Hegelund , Thierry Reding , UNGLinuxDriver@microchip.com, Uwe =?utf-8?q?Kleine-K=C3=B6nig?= , Vinod Koul , Wim Van Sebroeck , linux-arm-kernel@lists.infradead.org, linux-hwmon@vger.kernel.org Date: Thu, 19 May 2022 16:45:57 -0700 User-Agent: alot/0.10 Message-Id: <20220519234559.90C6DC385AA@smtp.kernel.org> 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: Tomislav Denis , linux-iio@vger.kernel.org, Alexandre Torgue , dri-devel@lists.freedesktop.org, Bjorn Andersson , linux-i2c@vger.kernel.org, Nobuhiro Iwamatsu , linux-clk@vger.kernel.org, linux-watchdog@vger.kernel.org, linux-rtc@vger.kernel.org, Michal Simek , linux-stm32@st-md-mailman.stormreply.com, ".kernel.org"@freedesktop.org, Andy Gross , Alexandru Ardelean , Keguang Zhang , Patrice Chotard , kernel@pengutronix.de, linux-arm-msm@vger.kernel.org, Anand Ashok Dumbre , Vladimir Zapolskiy , linux-gpio@vger.kernel.org, =?utf-8?q?Andr=C3=A9?= Gustavo Nakagomi Lopez , Jonathan Cameron , linux-amlogic@lists.infradead.org, linux-pwm@vger, Amireddy Mallikarjuna reddy , linux-mips@vger.kernel.org, linux-spi@vger.kernel.org, Cai Huoqing , linux-crypto@vger.kernel.org, Maxime Coquelin , dmaengine@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" The Cc list is huge. Here it goes! Quoting Uwe Kleine-K=C3=B6nig (2022-03-14 07:16:29) > When a driver keeps a clock prepared (or enabled) during the whole > lifetime of the driver, these helpers allow to simplify the drivers. >=20 > Reviewed-by: Jonathan Cameron > Reviewed-by: Alexandru Ardelean > Signed-off-by: Uwe Kleine-K=C3=B6nig > --- I'm ready to merge it! I'm largely waiting for Russell to ack the clk.h change, but if that doesn't happen then I think we'll have to merge it anyway. Can you resend with collected acks, maybe just the ones you want me to merge through clk tree? Then I'll go ahead and stage it. Some nitpicks below. > drivers/clk/clk-devres.c | 31 ++++++++++++++ > include/linux/clk.h | 90 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 120 insertions(+), 1 deletion(-) >=20 > diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c > index fb7761888b30..4707fe718f0b 100644 > --- a/drivers/clk/clk-devres.c > +++ b/drivers/clk/clk-devres.c > @@ -67,12 +67,43 @@ struct clk *devm_clk_get(struct device *dev, const ch= ar *id) > } > EXPORT_SYMBOL(devm_clk_get); > =20 > +struct clk *devm_clk_get_prepared(struct device *dev, const char *id) > +{ > + return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepar= e); > + Nitpick: Remove newline > +} > +EXPORT_SYMBOL(devm_clk_get_prepared); > + > +struct clk *devm_clk_get_enabled(struct device *dev, const char *id) > +{ > + return __devm_clk_get(dev, id, clk_get, > + clk_prepare_enable, clk_disable_unprepare); > + Nitpick: Remove newline > +} > +EXPORT_SYMBOL(devm_clk_get_enabled); > + > struct clk *devm_clk_get_optional(struct device *dev, const char *id) > { > return __devm_clk_get(dev, id, clk_get_optional, NULL, NULL); > } > EXPORT_SYMBOL(devm_clk_get_optional); > =20 > +struct clk *devm_clk_get_optional_prepared(struct device *dev, const cha= r *id) > +{ > + return __devm_clk_get(dev, id, clk_get_optional, > + clk_prepare, clk_unprepare); > + Nitpick: Remove newline > +} > +EXPORT_SYMBOL(devm_clk_get_optional_prepared); > + > +struct clk *devm_clk_get_optional_enabled(struct device *dev, const char= *id) > +{ > + return __devm_clk_get(dev, id, clk_get_optional, > + clk_prepare_enable, clk_disable_unprepare); > + Nitpick: Remove newline > +} > +EXPORT_SYMBOL(devm_clk_get_optional_enabled); EXPORT_SYMBOL_GPL for all of these? Or make them macros and cut down on the number of symbols. > + > struct clk_bulk_devres { > struct clk_bulk_data *clks; > int num_clks; > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 266e8de3cb51..b011dbba7109 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -449,7 +449,7 @@ int __must_check devm_clk_bulk_get_all(struct device = *dev, > * the clock producer. (IOW, @id may be identical strings, but > * clk_get may return different clock producers depending on @dev.) > * > - * Drivers must assume that the clock source is not enabled. > + * Drivers must assume that the clock source is neither prepared nor ena= bled. > * > * devm_clk_get should not be called from within interrupt context. > * Can you split this off to another patch? It's updating the doc to clarify the assumed state of a clk returned from this API. > @@ -458,6 +458,47 @@ int __must_check devm_clk_bulk_get_all(struct device= *dev, > */ > struct clk *devm_clk_get(struct device *dev, const char *id); > =20 > +/** > + * devm_clk_get_prepared - devm_clk_get() + clk_prepare() > + * @dev: device for clock "consumer" > + * @id: clock consumer ID > + * > + * Returns a struct clk corresponding to the clock producer, or > + * valid IS_ERR() condition containing errno. The implementation > + * uses @dev and @id to determine the clock consumer, and thereby > + * the clock producer. (IOW, @id may be identical strings, but > + * clk_get may return different clock producers depending on @dev.) > + * > + * The returned clk (if valid) is prepared. Drivers must however assume = that the > + * clock is not enabled. > + * > + * devm_clk_get_prepared should not be called from within interrupt cont= ext. There's 'Context:' for this. Please use it. > + * > + * The clock will automatically be unprepared and freed when the > + * device is unbound from the bus. > + */ > +struct clk *devm_clk_get_prepared(struct device *dev, const char *id); > + > +/** > + * devm_clk_get_enabled - devm_clk_get() + clk_prepare_enable() > + * @dev: device for clock "consumer" > + * @id: clock consumer ID > + * > + * Returns a struct clk corresponding to the clock producer, or valid IS= _ERR() There's 'Return:' for this. Please use it. > + * condition containing errno. The implementation uses @dev and @id to > + * determine the clock consumer, and thereby the clock producer. (IOW, = @id may > + * be identical strings, but clk_get may return different clock producers > + * depending on @dev.) > + * > + * The returned clk (if valid) is prepared and enabled. > + * > + * devm_clk_get_prepared should not be called from within interrupt cont= ext. > + * > + * The clock will automatically be disabled, unprepared and freed when t= he > + * device is unbound from the bus. > + */ > +struct clk *devm_clk_get_enabled(struct device *dev, const char *id); > + > /** > * devm_clk_get_optional - lookup and obtain a managed reference to an o= ptional > * clock producer. > @@ -469,6 +510,29 @@ struct clk *devm_clk_get(struct device *dev, const c= har *id); > */ > struct clk *devm_clk_get_optional(struct device *dev, const char *id); > =20 > +/** > + * devm_clk_get_optional_prepared - devm_clk_get_optional() + clk_prepar= e() > + * @dev: device for clock "consumer" > + * @id: clock consumer ID > + * > + * Behaves the same as devm_clk_get_prepared() except where there is no = clock > + * producer. In this case, instead of returning -ENOENT, the function r= eturns > + * NULL. When it comes to kernel-doc I think the DRY principle should not apply. I don't want to have to jump to one doc block to jump to another doc block while holding the previous verbage in my head to understand what the difference is. Please be repetitive with documentation for APIs :) > + */ > +struct clk *devm_clk_get_optional_prepared(struct device *dev, const cha= r *id); > + > +/** > + * devm_clk_get_optional_enabled - devm_clk_get_optional() + > + * clk_prepare_enable() > + * @dev: device for clock "consumer" > + * @id: clock consumer ID > + * > + * Behaves the same as devm_clk_get_enabled() except where there is no c= lock > + * producer. In this case, instead of returning -ENOENT, the function r= eturns > + * NULL. > + */ > +struct clk *devm_clk_get_optional_enabled(struct device *dev, const char= *id); > + > /** > * devm_get_clk_from_child - lookup and obtain a managed reference to a > * clock producer from child node.