| Message ID | 1531918658-32278-1-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested, 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. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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(+)