Message ID | 20220314141643.22184-3-u.kleine-koenig@pengutronix.de (mailing list archive) |
---|---|
State | Handled Elsewhere, archived |
Headers | show |
Series | clk: provide new devm helpers for prepared and enabled clocks | expand |
On Mon, 14 Mar 2022 15:16:29 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > When a driver keeps a clock prepared (or enabled) during the whole > lifetime of the driver, these helpers allow to simplify the drivers. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> One trivial thing below. > --- > drivers/clk/clk-devres.c | 31 ++++++++++++++ > include/linux/clk.h | 90 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 120 insertions(+), 1 deletion(-) > > 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 char *id) > } > EXPORT_SYMBOL(devm_clk_get); > > +struct clk *devm_clk_get_prepared(struct device *dev, const char *id) > +{ > + return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepare); Nitpick but this spacing before } in functions is rather unusual and not in keeping with the existing code in this file. > + > +} > +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); > + > +} > +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); > > +struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *id) > +{ > + return __devm_clk_get(dev, id, clk_get_optional, > + clk_prepare, clk_unprepare); > + > +} > +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); > + > +} > +EXPORT_SYMBOL(devm_clk_get_optional_enabled); > + > 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 enabled. > * > * devm_clk_get should not be called from within interrupt context. > * > @@ -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); > > +/** > + * 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 context. > + * > + * 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() > + * 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 context. > + * > + * The clock will automatically be disabled, unprepared and freed when the > + * 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 optional > * clock producer. > @@ -469,6 +510,29 @@ struct clk *devm_clk_get(struct device *dev, const char *id); > */ > struct clk *devm_clk_get_optional(struct device *dev, const char *id); > > +/** > + * devm_clk_get_optional_prepared - devm_clk_get_optional() + clk_prepare() > + * @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 returns > + * NULL. > + */ > +struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *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 clock > + * producer. In this case, instead of returning -ENOENT, the function returns > + * 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. > @@ -813,12 +877,36 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id) > return NULL; > } > > +static inline struct clk *devm_clk_get_prepared(struct device *dev, > + const char *id) > +{ > + return NULL; > +} > + > +static inline struct clk *devm_clk_get_enabled(struct device *dev, > + const char *id) > +{ > + return NULL; > +} > + > static inline struct clk *devm_clk_get_optional(struct device *dev, > const char *id) > { > return NULL; > } > > +static inline struct clk *devm_clk_get_optional_prepared(struct device *dev, > + const char *id) > +{ > + return NULL; > +} > + > +static inline struct clk *devm_clk_get_optional_enabled(struct device *dev, > + const char *id) > +{ > + return NULL; > +} > + > static inline int __must_check devm_clk_bulk_get(struct device *dev, int num_clks, > struct clk_bulk_data *clks) > {
On Sat, Mar 19, 2022 at 06:29:36PM +0000, Jonathan Cameron wrote: > On Mon, 14 Mar 2022 15:16:29 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > When a driver keeps a clock prepared (or enabled) during the whole > > lifetime of the driver, these helpers allow to simplify the drivers. > > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > One trivial thing below. > > > --- > > drivers/clk/clk-devres.c | 31 ++++++++++++++ > > include/linux/clk.h | 90 +++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 120 insertions(+), 1 deletion(-) > > > > 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 char *id) > > } > > EXPORT_SYMBOL(devm_clk_get); > > > > +struct clk *devm_clk_get_prepared(struct device *dev, const char *id) > > +{ > > + return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepare); > > Nitpick but this spacing before } in functions is rather unusual and not > in keeping with the existing code in this file. > > > + > > +} ack, I fixed that in my tree, so this will be part of an v9. I won't send it just for this change, though. I fixed three further functions that had a similar empty line, too. Thanks for looking Uwe
The Cc list is huge. Here it goes! Quoting Uwe Kleine-König (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. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- 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(-) > > 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 char *id) > } > EXPORT_SYMBOL(devm_clk_get); > > +struct clk *devm_clk_get_prepared(struct device *dev, const char *id) > +{ > + return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepare); > + 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); > > +struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *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 enabled. > * > * 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); > > +/** > + * 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 context. 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 context. > + * > + * The clock will automatically be disabled, unprepared and freed when the > + * 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 optional > * clock producer. > @@ -469,6 +510,29 @@ struct clk *devm_clk_get(struct device *dev, const char *id); > */ > struct clk *devm_clk_get_optional(struct device *dev, const char *id); > > +/** > + * devm_clk_get_optional_prepared - devm_clk_get_optional() + clk_prepare() > + * @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 returns > + * 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 char *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 clock > + * producer. In this case, instead of returning -ENOENT, the function returns > + * 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.
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 char *id) } EXPORT_SYMBOL(devm_clk_get); +struct clk *devm_clk_get_prepared(struct device *dev, const char *id) +{ + return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepare); + +} +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); + +} +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); +struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *id) +{ + return __devm_clk_get(dev, id, clk_get_optional, + clk_prepare, clk_unprepare); + +} +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); + +} +EXPORT_SYMBOL(devm_clk_get_optional_enabled); + 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 enabled. * * devm_clk_get should not be called from within interrupt context. * @@ -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); +/** + * 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 context. + * + * 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() + * 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 context. + * + * The clock will automatically be disabled, unprepared and freed when the + * 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 optional * clock producer. @@ -469,6 +510,29 @@ struct clk *devm_clk_get(struct device *dev, const char *id); */ struct clk *devm_clk_get_optional(struct device *dev, const char *id); +/** + * devm_clk_get_optional_prepared - devm_clk_get_optional() + clk_prepare() + * @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 returns + * NULL. + */ +struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *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 clock + * producer. In this case, instead of returning -ENOENT, the function returns + * 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. @@ -813,12 +877,36 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id) return NULL; } +static inline struct clk *devm_clk_get_prepared(struct device *dev, + const char *id) +{ + return NULL; +} + +static inline struct clk *devm_clk_get_enabled(struct device *dev, + const char *id) +{ + return NULL; +} + static inline struct clk *devm_clk_get_optional(struct device *dev, const char *id) { return NULL; } +static inline struct clk *devm_clk_get_optional_prepared(struct device *dev, + const char *id) +{ + return NULL; +} + +static inline struct clk *devm_clk_get_optional_enabled(struct device *dev, + const char *id) +{ + return NULL; +} + static inline int __must_check devm_clk_bulk_get(struct device *dev, int num_clks, struct clk_bulk_data *clks) {