| Message ID | 1531918658-32278-1-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 18, 2018 at 01:57:38PM +0100, Phil Edworthy wrote: > Behaves the same as (devm_)clk_get except where there is no clock > producer. In this case, instead of returning -ENOENT, the function > returns NULL. This makes error checking simpler and allows > clk_prepare_enable, etc to be called on the returned reference > without additional checks. How does this work with non-DT systems, where looking a clock up which isn't yet registered with clkdev returns -ENOENT ? (clkdev doesn't know when all clocks are registered with it.)
Hi Russell, On Wed, Jul 18, 2018 at 3:02 PM Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Jul 18, 2018 at 01:57:38PM +0100, Phil Edworthy wrote: > > Behaves the same as (devm_)clk_get except where there is no clock > > producer. In this case, instead of returning -ENOENT, the function > > returns NULL. This makes error checking simpler and allows > > clk_prepare_enable, etc to be called on the returned reference > > without additional checks. > > How does this work with non-DT systems, where looking a clock up which > isn't yet registered with clkdev returns -ENOENT ? > > (clkdev doesn't know when all clocks are registered with it.) Good question. I guess all drivers trying to handle optional clocks this way are already broken on non-DT systems where clocks may be registered late... Gr{oetje,eeting}s, Geert
Hi Russell, On 18 July 2018 14:19, Geert Uytterhoeven wrote: > On Wed, Jul 18, 2018 at 3:02 PM Russell King - ARM Linux wrote: > > On Wed, Jul 18, 2018 at 01:57:38PM +0100, Phil Edworthy wrote: > > > Behaves the same as (devm_)clk_get except where there is no clock > > > producer. In this case, instead of returning -ENOENT, the function > > > returns NULL. This makes error checking simpler and allows > > > clk_prepare_enable, etc to be called on the returned reference > > > without additional checks. > > > > How does this work with non-DT systems, where looking a clock up which > > isn't yet registered with clkdev returns -ENOENT ? > > > > (clkdev doesn't know when all clocks are registered with it.) > > Good question. > > I guess all drivers trying to handle optional clocks this way are already broken > on non-DT systems where clocks may be registered late... So how do non-DT systems that look a clock up which isn't yet registered with clkdev, determine that an optional clock is there or not? Thanks Phil
Hi Phil, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on clk/clk-next] [also build test WARNING on v4.18-rc5 next-20180718] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Phil-Edworthy/clk-Add-functions-to-get-optional-clocks/20180719-024451 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) mm/mempool.c:228: warning: Function parameter or member 'pool' not described in 'mempool_init' Error: Cannot open file arch/x86/kernel/cpu/mtrr/main.c Error: Cannot open file arch/x86/kernel/cpu/mtrr/main.c WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export arch/x86/kernel/cpu/mtrr/main.c' failed with return code 2 >> include/linux/clk.h:300: warning: Function parameter or member 'dev' not described in 'clk_get_optional' >> include/linux/clk.h:300: warning: Function parameter or member 'id' not described in 'clk_get_optional' >> include/linux/clk.h:366: warning: Function parameter or member 'dev' not described in 'devm_clk_get_optional' >> include/linux/clk.h:366: warning: Function parameter or member 'id' not described in 'devm_clk_get_optional' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:955: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' kernel/sched/fair.c:3760: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg' include/linux/device.h:93: warning: bad line: this bus. include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/device.h:94: warning: bad line: this bus. include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume_early' not described in 'regulator_ops' drivers/regulator/core.c:4465: warning: Excess function parameter 'state' description in 'regulator_suspend_late' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/usb/dwc3/gadget.c:510: warning: Excess function parameter 'dwc' description in 'dwc3_gadget_start_config' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver' drivers/gpu/drm/i915/i915_vma.h:48: warning: cannot understand function prototype: 'struct i915_vma ' drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 'fb_dirty' not described in 'tinydrm_device' drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 'crtc_state' not described in 'mipi_dbi_enable_flush' drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 'plane_state' not described in 'mipi_dbi_enable_flush' vim +300 include/linux/clk.h > 300 301 /** 302 * clk_bulk_get - lookup and obtain a number of references to clock producer. 303 * @dev: device for clock "consumer" 304 * @num_clks: the number of clk_bulk_data 305 * @clks: the clk_bulk_data table of consumer 306 * 307 * This helper function allows drivers to get several clk consumers in one 308 * operation. If any of the clk cannot be acquired then any clks 309 * that were obtained will be freed before returning to the caller. 310 * 311 * Returns 0 if all clocks specified in clk_bulk_data table are obtained 312 * successfully, or valid IS_ERR() condition containing errno. 313 * The implementation uses @dev and @clk_bulk_data.id to determine the 314 * clock consumer, and thereby the clock producer. 315 * The clock returned is stored in each @clk_bulk_data.clk field. 316 * 317 * Drivers must assume that the clock source is not enabled. 318 * 319 * clk_bulk_get should not be called from within interrupt context. 320 */ 321 int __must_check clk_bulk_get(struct device *dev, int num_clks, 322 struct clk_bulk_data *clks); 323 324 /** 325 * devm_clk_bulk_get - managed get multiple clk consumers 326 * @dev: device for clock "consumer" 327 * @num_clks: the number of clk_bulk_data 328 * @clks: the clk_bulk_data table of consumer 329 * 330 * Return 0 on success, an errno on failure. 331 * 332 * This helper function allows drivers to get several clk 333 * consumers in one operation with management, the clks will 334 * automatically be freed when the device is unbound. 335 */ 336 int __must_check devm_clk_bulk_get(struct device *dev, int num_clks, 337 struct clk_bulk_data *clks); 338 339 /** 340 * devm_clk_get - lookup and obtain a managed reference to a clock producer. 341 * @dev: device for clock "consumer" 342 * @id: clock consumer ID 343 * 344 * Returns a struct clk corresponding to the clock producer, or 345 * valid IS_ERR() condition containing errno. The implementation 346 * uses @dev and @id to determine the clock consumer, and thereby 347 * the clock producer. (IOW, @id may be identical strings, but 348 * clk_get may return different clock producers depending on @dev.) 349 * 350 * Drivers must assume that the clock source is not enabled. 351 * 352 * devm_clk_get should not be called from within interrupt context. 353 * 354 * The clock will automatically be freed when the device is unbound 355 * from the bus. 356 */ 357 struct clk *devm_clk_get(struct device *dev, const char *id); 358 359 /** 360 * devm_clk_get_optional - lookup and obtain a managed reference to an optional 361 * clock producer. 362 * Behaves the same as devm_clk_get except where there is no clock producer. In 363 * this case, instead of returning -ENOENT, the function returns NULL. 364 */ 365 struct clk *devm_clk_get_optional(struct device *dev, const char *id); > 366 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Quoting Phil Edworthy (2018-07-18 06:56:26) > Hi Russell, > > On 18 July 2018 14:19, Geert Uytterhoeven wrote: > > On Wed, Jul 18, 2018 at 3:02 PM Russell King - ARM Linux wrote: > > > On Wed, Jul 18, 2018 at 01:57:38PM +0100, Phil Edworthy wrote: > > > > Behaves the same as (devm_)clk_get except where there is no clock > > > > producer. In this case, instead of returning -ENOENT, the function > > > > returns NULL. This makes error checking simpler and allows > > > > clk_prepare_enable, etc to be called on the returned reference > > > > without additional checks. > > > > > > How does this work with non-DT systems, where looking a clock up which > > > isn't yet registered with clkdev returns -ENOENT ? > > > > > > (clkdev doesn't know when all clocks are registered with it.) > > > > Good question. > > > > I guess all drivers trying to handle optional clocks this way are already broken > > on non-DT systems where clocks may be registered late... > > So how do non-DT systems that look a clock up which isn't yet > registered with clkdev, determine that an optional clock is there > or not? > Short answer is they don't. I'd still prefer we have this API though. Can you rework this patch to be a little more invasive into the clk_get() path, perhaps by reworking __of_clk_get_by_name() a little to take an 'optional' argument, so that it only returns NULL when the clk is looked up from DT? The fallback path in clkdev where we have a DT based system looking up a clk through clkdev lookups doesn't seem to be a real scenario that we should worry about here. I think sometimes people use clkdev lookups when they're migrating to DT systems and things aren't wired up properly in DT, but that isn't the norm.
Hi Stephen, On 25 July 2018 23:37, Stephen Boyd wrote: > Quoting Phil Edworthy (2018-07-18 06:56:26) > > On 18 July 2018 14:19, Geert Uytterhoeven wrote: > > > On Wed, Jul 18, 2018 at 3:02 PM Russell King - ARM Linux wrote: > > > > On Wed, Jul 18, 2018 at 01:57:38PM +0100, Phil Edworthy wrote: > > > > > Behaves the same as (devm_)clk_get except where there is no > > > > > clock producer. In this case, instead of returning -ENOENT, the > > > > > function returns NULL. This makes error checking simpler and > > > > > allows clk_prepare_enable, etc to be called on the returned > > > > > reference without additional checks. > > > > > > > > How does this work with non-DT systems, where looking a clock up > > > > which isn't yet registered with clkdev returns -ENOENT ? > > > > > > > > (clkdev doesn't know when all clocks are registered with it.) > > > > > > Good question. > > > > > > I guess all drivers trying to handle optional clocks this way are > > > already broken on non-DT systems where clocks may be registered late... > > > > So how do non-DT systems that look a clock up which isn't yet > > registered with clkdev, determine that an optional clock is there or > > not? > > > > Short answer is they don't. I'd still prefer we have this API though. > > Can you rework this patch to be a little more invasive into the > clk_get() path, perhaps by reworking __of_clk_get_by_name() a little to > take an 'optional' argument, so that it only returns NULL when the clk is > looked up from DT? The fallback path in clkdev where we have a DT based > system looking up a clk through clkdev lookups doesn't seem to be a real > scenario that we should worry about here. I think sometimes people use > clkdev lookups when they're migrating to DT systems and things aren't wired > up properly in DT, but that isn't the norm. Do you mean something like this: diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 7f2cd1e..42a7d4e 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -57,7 +57,8 @@ EXPORT_SYMBOL(of_clk_get); static struct clk *__of_clk_get_by_name(struct device_node *np, const char *dev_id, - const char *name) + const char *name, + bool optional) { struct clk *clk = ERR_PTR(-ENOENT); @@ -79,6 +80,8 @@ static struct clk *__of_clk_get_by_name(struct device_node *np, if (PTR_ERR(clk) != -EPROBE_DEFER) pr_err("ERROR: could not get clock %pOF:%s(%i)\n", np, name ? name : "", index); + if (optional && PTR_ERR(clk) == -ENOENT) + clk = NULL; return clk; } @@ -109,15 +112,38 @@ struct clk *of_clk_get_by_name(struct device_node *np, const char *name) if (!np) return ERR_PTR(-ENOENT); - return __of_clk_get_by_name(np, np->full_name, name); + return __of_clk_get_by_name(np, np->full_name, name, false); } EXPORT_SYMBOL(of_clk_get_by_name); +/** + * of_clk_get_by_name_optional() - Parse and lookup an optional clock referenced + * by a device node + * @np: pointer to clock consumer node + * @name: name of consumer's clock input, or NULL for the first clock reference + * + * This function parses the clocks and clock-names properties, + * and uses them to look up the struct clk from the registered list of clock + * providers. + * It behaves the same as of_clk_get_by_name(), except when no clock is found. + * In this case, instead of returning -ENOENT, it returns NULL. + */ +struct clk *of_clk_get_by_name_optional(struct device_node *np, + const char *name) +{ + if (!np) + return ERR_PTR(-ENOENT); + + return __of_clk_get_by_name(np, np->full_name, name, true); +} +EXPORT_SYMBOL(of_clk_get_by_name_optional); + #else /* defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) */ static struct clk *__of_clk_get_by_name(struct device_node *np, const char *dev_id, - const char *name) + const char *name, + bool optional) { return ERR_PTR(-ENOENT); } @@ -200,7 +226,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) struct clk *clk; if (dev) { - clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id); + clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id, false); if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) return clk; } diff --git a/include/linux/clk.h b/include/linux/clk.h index 907202b..830209a 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -771,6 +771,7 @@ static inline void clk_bulk_disable_unprepare(int num_clks, #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) struct clk *of_clk_get(struct device_node *np, int index); struct clk *of_clk_get_by_name(struct device_node *np, const char *name); +struct clk *of_clk_get_by_name_optional(struct device_node *np, const char *name); struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec); #else static inline struct clk *of_clk_get(struct device_node *np, int index) --- A lot of drivers use devm_clk_get() so I think a devm_clk_get_optional() version would be useful. That would probably need an additional clk_get_optional() function. Thanks Phil
Quoting Phil Edworthy (2018-07-27 08:38:12) > On 25 July 2018 23:37, Stephen Boyd wrote: > > Short answer is they don't. I'd still prefer we have this API though. > > > > Can you rework this patch to be a little more invasive into the > > clk_get() path, perhaps by reworking __of_clk_get_by_name() a little to > > take an 'optional' argument, so that it only returns NULL when the clk is > > looked up from DT? The fallback path in clkdev where we have a DT based > > system looking up a clk through clkdev lookups doesn't seem to be a real > > scenario that we should worry about here. I think sometimes people use > > clkdev lookups when they're migrating to DT systems and things aren't wired > > up properly in DT, but that isn't the norm. > Do you mean something like this: > [...] > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 907202b..830209a 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -771,6 +771,7 @@ static inline void clk_bulk_disable_unprepare(int num_clks, > #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) > struct clk *of_clk_get(struct device_node *np, int index); > struct clk *of_clk_get_by_name(struct device_node *np, const char *name); > +struct clk *of_clk_get_by_name_optional(struct device_node *np, const char *name); > struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec); > #else > static inline struct clk *of_clk_get(struct device_node *np, int index) > > --- > A lot of drivers use devm_clk_get() so I think a devm_clk_get_optional() > version would be useful. That would probably need an additional > clk_get_optional() function. > Yes, this would be the first patch, and then the second patch would add clk_get_optional() and devm_clk_get_optional(). We shouldn't encourage consumers to move from device based to DT node based clk_get() for the optional clk API.
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c index d854e26..63295d9 100644 --- a/drivers/clk/clk-devres.c +++ b/drivers/clk/clk-devres.c @@ -34,6 +34,17 @@ struct clk *devm_clk_get(struct device *dev, const char *id) } EXPORT_SYMBOL(devm_clk_get); +struct clk *devm_clk_get_optional(struct device *dev, const char *id) +{ + struct clk *c = devm_clk_get(dev, id); + + if (PTR_ERR(c) == -ENOENT) + return NULL; + + return c; +} +EXPORT_SYMBOL(devm_clk_get_optional); + struct clk_bulk_devres { struct clk_bulk_data *clks; int num_clks; diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 7513411..7f2cd1e 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -209,6 +209,17 @@ struct clk *clk_get(struct device *dev, const char *con_id) } EXPORT_SYMBOL(clk_get); +struct clk *clk_get_optional(struct device *dev, const char *id) +{ + struct clk *c = clk_get(dev, id); + + if (PTR_ERR(c) == -ENOENT) + return NULL; + + return c; +} +EXPORT_SYMBOL(clk_get_optional); + void clk_put(struct clk *clk) { __clk_put(clk); diff --git a/include/linux/clk.h b/include/linux/clk.h index 0dbd088..907202b 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -258,6 +258,14 @@ static inline void clk_bulk_unprepare(int num_clks, struct clk_bulk_data *clks) struct clk *clk_get(struct device *dev, const char *id); /** + * clk_get_optional - lookup and obtain a reference to optional clock producer. + * + * Behaves the same as clk_get except where there is no clock producer. In this + * case, instead of returning -ENOENT, the function returns NULL. + */ +struct clk *clk_get_optional(struct device *dev, const char *id); + +/** * clk_bulk_get - lookup and obtain a number of references to clock producer. * @dev: device for clock "consumer" * @num_clks: the number of clk_bulk_data @@ -316,6 +324,14 @@ int __must_check devm_clk_bulk_get(struct device *dev, int num_clks, struct clk *devm_clk_get(struct device *dev, const char *id); /** + * devm_clk_get_optional - lookup and obtain a managed reference to an optional + * clock producer. + * Behaves the same as devm_clk_get except where there is no clock producer. In + * this case, instead of returning -ENOENT, the function returns NULL. + */ +struct clk *devm_clk_get_optional(struct device *dev, const char *id); + +/** * devm_get_clk_from_child - lookup and obtain a managed reference to a * clock producer from child node. * @dev: device for clock "consumer" @@ -603,6 +619,11 @@ static inline struct clk *clk_get(struct device *dev, const char *id) return NULL; } +static inline struct clk *clk_get_optional(struct device *dev, const char *id) +{ + return NULL; +} + static inline int __must_check clk_bulk_get(struct device *dev, int num_clks, struct clk_bulk_data *clks) { @@ -614,6 +635,12 @@ static inline struct clk *devm_clk_get(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 int __must_check devm_clk_bulk_get(struct device *dev, int num_clks, struct clk_bulk_data *clks) {
Behaves the same as (devm_)clk_get except where there is no clock producer. In this case, instead of returning -ENOENT, the function returns NULL. This makes error checking simpler and allows clk_prepare_enable, etc to be called on the returned reference without additional checks. Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> --- drivers/clk/clk-devres.c | 11 +++++++++++ drivers/clk/clkdev.c | 11 +++++++++++ include/linux/clk.h | 27 +++++++++++++++++++++++++++ 3 files changed, 49 insertions(+)