* [PATCH v2 0/2] Add functions to get optional clocks @ 2018-07-30 8:36 Phil Edworthy 2018-07-30 8:36 ` [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function Phil Edworthy 2018-07-30 8:36 ` [PATCH v2 2/2] clk: Add functions to get optional clocks Phil Edworthy 0 siblings, 2 replies; 7+ messages in thread From: Phil Edworthy @ 2018-07-30 8:36 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Russell King Cc: Geert Uytterhoeven, Simon Horman, Andy Shevchenko, linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy Quite a few drivers get an optional clock, e.g. a clock required to access peripheral's registers that is always enabled on some devices. Phil Edworthy (2): clk: Add of_clk_get_by_name_optional() function clk: Add functions to get optional clocks drivers/clk/clk-devres.c | 18 ++++++++++++++++-- drivers/clk/clkdev.c | 49 +++++++++++++++++++++++++++++++++++++++++++----- include/linux/clk.h | 30 +++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 7 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function 2018-07-30 8:36 [PATCH v2 0/2] Add functions to get optional clocks Phil Edworthy @ 2018-07-30 8:36 ` Phil Edworthy 2018-07-30 8:55 ` Geert Uytterhoeven 2018-07-30 8:36 ` [PATCH v2 2/2] clk: Add functions to get optional clocks Phil Edworthy 1 sibling, 1 reply; 7+ messages in thread From: Phil Edworthy @ 2018-07-30 8:36 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Russell King Cc: Geert Uytterhoeven, Simon Horman, Andy Shevchenko, linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy Quite a few drivers get an optional clock, e.g. a clock required to access peripheral's registers that is always enabled on some devices. This function behaves the same as of_clk_get_by_name() except that it will return NULL instead of -ENOENT. This makes error checking for users easier 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/clkdev.c | 34 ++++++++++++++++++++++++++++++---- include/linux/clk.h | 1 + 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 9ab3db8..c1b7262 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -54,7 +54,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); @@ -73,6 +74,8 @@ static struct clk *__of_clk_get_by_name(struct device_node *np, if (!IS_ERR(clk)) { break; } else if (name && index >= 0) { + if (optional && PTR_ERR(clk) == -ENOENT) + clk = NULL; if (PTR_ERR(clk) != -EPROBE_DEFER) pr_err("ERROR: could not get clock %pOF:%s(%i)\n", np, name ? name : "", index); @@ -106,15 +109,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); } @@ -197,7 +223,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) struct clk *clk; if (dev && dev->of_node) { - 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 4f750c4..8cb5455 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -777,6 +777,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) -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function 2018-07-30 8:36 ` [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function Phil Edworthy @ 2018-07-30 8:55 ` Geert Uytterhoeven 2018-07-30 9:25 ` Phil Edworthy 2018-07-30 10:57 ` Andy Shevchenko 0 siblings, 2 replies; 7+ messages in thread From: Geert Uytterhoeven @ 2018-07-30 8:55 UTC (permalink / raw) To: Phil Edworthy Cc: Michael Turquette, Stephen Boyd, Russell King, Simon Horman, Andy Shevchenko, linux-clk, Linux Kernel Mailing List, Linux ARM Hi Phil, On Mon, Jul 30, 2018 at 10:36 AM Phil Edworthy <phil.edworthy@renesas.com> wrote: > Quite a few drivers get an optional clock, e.g. a clock required > to access peripheral's registers that is always enabled on some > devices. > > This function behaves the same as of_clk_get_by_name() except that > it will return NULL instead of -ENOENT. This makes error checking > for users easier 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> Thanks for your patch! > --- a/drivers/clk/clkdev.c > +++ b/drivers/clk/clkdev.c > @@ -54,7 +54,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); > > @@ -73,6 +74,8 @@ static struct clk *__of_clk_get_by_name(struct device_node *np, > if (!IS_ERR(clk)) { > break; > } else if (name && index >= 0) { > + if (optional && PTR_ERR(clk) == -ENOENT) > + clk = NULL; This only handles the "name && index >= 0" case. If that condition is never true, the loop will end, eventually, and the last value of clk will be returned. Hence there should be a similar check at the end of the function. > if (PTR_ERR(clk) != -EPROBE_DEFER) > pr_err("ERROR: could not get clock %pOF:%s(%i)\n", > np, name ? name : "", index); Hence if not found, this will always print an error, even if the clock is optional? > @@ -106,15 +109,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); Shouldn't this return NULL? Or let __of_clk_get_by_name() handle that (cfr. above)? Hmm, of_clk_get_by_name() has a similar check, while the current __of_clk_get_by_name() already handle np == NULL, too. > + > + 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); > } > @@ -197,7 +223,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) > struct clk *clk; > > if (dev && dev->of_node) { > - 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 4f750c4..8cb5455 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -777,6 +777,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) No dummy version of of_clk_get_by_name_optional() for the !CLK || !COMMON_CLK case? It seems of_clk_get_by_name() and of_clk_get_by_name_optional() can just be simple wrappers around __of_clk_get_by_name(), differing only in the value of the "optional" parameter. Hence I think it makes sense to move them to the header file, and make them static inline. Then only __of_clk_get_by_name() needs a (static inline) dummy for the !OF || !COMMON_CLK case. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function 2018-07-30 8:55 ` Geert Uytterhoeven @ 2018-07-30 9:25 ` Phil Edworthy 2018-07-30 10:57 ` Andy Shevchenko 1 sibling, 0 replies; 7+ messages in thread From: Phil Edworthy @ 2018-07-30 9:25 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Russell King, Simon Horman, Andy Shevchenko, linux-clk, Linux Kernel Mailing List, Linux ARM SGkgR2VlcnQsDQoNCk9uIDMwIEp1bHkgMjAxOCAwOTo1NiwgR2VlcnQgVXl0dGVyaG9ldmVuIHdy b3RlOg0KPiBPbiBNb24sIEp1bCAzMCwgMjAxOCBhdCAxMDozNiBBTSBQaGlsIEVkd29ydGh5IHdy b3RlOg0KPiA+IFF1aXRlIGEgZmV3IGRyaXZlcnMgZ2V0IGFuIG9wdGlvbmFsIGNsb2NrLCBlLmcu IGEgY2xvY2sgcmVxdWlyZWQgdG8NCj4gPiBhY2Nlc3MgcGVyaXBoZXJhbCdzIHJlZ2lzdGVycyB0 aGF0IGlzIGFsd2F5cyBlbmFibGVkIG9uIHNvbWUgZGV2aWNlcy4NCj4gPg0KPiA+IFRoaXMgZnVu Y3Rpb24gYmVoYXZlcyB0aGUgc2FtZSBhcyBvZl9jbGtfZ2V0X2J5X25hbWUoKSBleGNlcHQgdGhh dCBpdA0KPiA+IHdpbGwgcmV0dXJuIE5VTEwgaW5zdGVhZCBvZiAtRU5PRU5ULiBUaGlzIG1ha2Vz IGVycm9yIGNoZWNraW5nIGZvcg0KPiA+IHVzZXJzIGVhc2llciBhbmQgYWxsb3dzIGNsa19wcmVw YXJlX2VuYWJsZSwgZXRjIHRvIGJlIGNhbGxlZCBvbiB0aGUNCj4gPiByZXR1cm5lZCByZWZlcmVu Y2Ugd2l0aG91dCBhZGRpdGlvbmFsIGNoZWNrcy4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFBo aWwgRWR3b3J0aHkgPHBoaWwuZWR3b3J0aHlAcmVuZXNhcy5jb20+DQo+IA0KPiBUaGFua3MgZm9y IHlvdXIgcGF0Y2ghDQo+IA0KPiA+IC0tLSBhL2RyaXZlcnMvY2xrL2Nsa2Rldi5jDQo+ID4gKysr IGIvZHJpdmVycy9jbGsvY2xrZGV2LmMNCj4gPiBAQCAtNTQsNyArNTQsOCBAQCBFWFBPUlRfU1lN Qk9MKG9mX2Nsa19nZXQpOw0KPiA+DQo+ID4gIHN0YXRpYyBzdHJ1Y3QgY2xrICpfX29mX2Nsa19n ZXRfYnlfbmFtZShzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wLA0KPiA+ICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBjaGFyICpkZXZfaWQsDQo+ID4gLSAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNvbnN0IGNoYXIgKm5hbWUpDQo+ID4gKyAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNvbnN0IGNoYXIgKm5hbWUsDQo+ ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGJvb2wgb3B0aW9uYWwp DQo+ID4gIHsNCj4gPiAgICAgICAgIHN0cnVjdCBjbGsgKmNsayA9IEVSUl9QVFIoLUVOT0VOVCk7 DQo+ID4NCj4gPiBAQCAtNzMsNiArNzQsOCBAQCBzdGF0aWMgc3RydWN0IGNsayAqX19vZl9jbGtf Z2V0X2J5X25hbWUoc3RydWN0DQo+IGRldmljZV9ub2RlICpucCwNCj4gPiAgICAgICAgICAgICAg ICAgaWYgKCFJU19FUlIoY2xrKSkgew0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIGJyZWFr Ow0KPiA+ICAgICAgICAgICAgICAgICB9IGVsc2UgaWYgKG5hbWUgJiYgaW5kZXggPj0gMCkgew0K PiA+ICsgICAgICAgICAgICAgICAgICAgICAgIGlmIChvcHRpb25hbCAmJiBQVFJfRVJSKGNsaykg PT0gLUVOT0VOVCkNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNsayA9IE5V TEw7DQo+IA0KPiBUaGlzIG9ubHkgaGFuZGxlcyB0aGUgIm5hbWUgJiYgaW5kZXggPj0gMCIgY2Fz ZS4NCj4gSWYgdGhhdCBjb25kaXRpb24gaXMgbmV2ZXIgdHJ1ZSwgdGhlIGxvb3Agd2lsbCBlbmQs IGV2ZW50dWFsbHksIGFuZCB0aGUgbGFzdCB2YWx1ZQ0KPiBvZiBjbGsgd2lsbCBiZSByZXR1cm5l ZC4gSGVuY2UgdGhlcmUgc2hvdWxkIGJlIGEgc2ltaWxhciBjaGVjayBhdCB0aGUgZW5kIG9mDQo+ IHRoZSBmdW5jdGlvbi4NCk9vcHMsIEkgd2FzIG9ubHkgY29uc2lkZXJpbmcgbmFtZWQgb3B0aW9u YWwgY2xvY2tzLCBJJ2xsIGZpeCB0aGlzLg0KDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAg aWYgKFBUUl9FUlIoY2xrKSAhPSAtRVBST0JFX0RFRkVSKQ0KPiA+ICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgcHJfZXJyKCJFUlJPUjogY291bGQgbm90IGdldCBjbG9jayAlcE9GOiVz KCVpKVxuIiwNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbnAs IG5hbWUgPyBuYW1lIDogIiIsIGluZGV4KTsNCj4gDQo+IEhlbmNlIGlmIG5vdCBmb3VuZCwgdGhp cyB3aWxsIGFsd2F5cyBwcmludCBhbiBlcnJvciwgZXZlbiBpZiB0aGUgY2xvY2sgaXMNCj4gb3B0 aW9uYWw/DQpSaWdodC4NCg0KPiA+IEBAIC0xMDYsMTUgKzEwOSwzOCBAQCBzdHJ1Y3QgY2xrICpv Zl9jbGtfZ2V0X2J5X25hbWUoc3RydWN0DQo+IGRldmljZV9ub2RlICpucCwgY29uc3QgY2hhciAq bmFtZSkNCj4gPiAgICAgICAgIGlmICghbnApDQo+ID4gICAgICAgICAgICAgICAgIHJldHVybiBF UlJfUFRSKC1FTk9FTlQpOw0KPiA+DQo+ID4gLSAgICAgICByZXR1cm4gX19vZl9jbGtfZ2V0X2J5 X25hbWUobnAsIG5wLT5mdWxsX25hbWUsIG5hbWUpOw0KPiA+ICsgICAgICAgcmV0dXJuIF9fb2Zf Y2xrX2dldF9ieV9uYW1lKG5wLCBucC0+ZnVsbF9uYW1lLCBuYW1lLCBmYWxzZSk7DQo+ID4gIH0N Cj4gPiAgRVhQT1JUX1NZTUJPTChvZl9jbGtfZ2V0X2J5X25hbWUpOw0KPiA+DQo+ID4gKy8qKg0K PiA+ICsgKiBvZl9jbGtfZ2V0X2J5X25hbWVfb3B0aW9uYWwoKSAtIFBhcnNlIGFuZCBsb29rdXAg YW4gb3B0aW9uYWwgY2xvY2sNCj4gPiArcmVmZXJlbmNlZA0KPiA+ICsgKiBieSBhIGRldmljZSBu b2RlDQo+ID4gKyAqIEBucDogcG9pbnRlciB0byBjbG9jayBjb25zdW1lciBub2RlDQo+ID4gKyAq IEBuYW1lOiBuYW1lIG9mIGNvbnN1bWVyJ3MgY2xvY2sgaW5wdXQsIG9yIE5VTEwgZm9yIHRoZSBm aXJzdCBjbG9jaw0KPiA+ICtyZWZlcmVuY2UNCj4gPiArICoNCj4gPiArICogVGhpcyBmdW5jdGlv biBwYXJzZXMgdGhlIGNsb2NrcyBhbmQgY2xvY2stbmFtZXMgcHJvcGVydGllcywNCj4gPiArICog YW5kIHVzZXMgdGhlbSB0byBsb29rIHVwIHRoZSBzdHJ1Y3QgY2xrIGZyb20gdGhlIHJlZ2lzdGVy ZWQgbGlzdA0KPiA+ICtvZiBjbG9jaw0KPiA+ICsgKiBwcm92aWRlcnMuDQo+ID4gKyAqIEl0IGJl aGF2ZXMgdGhlIHNhbWUgYXMgb2ZfY2xrX2dldF9ieV9uYW1lKCksIGV4Y2VwdCB3aGVuIG5vIGNs b2NrIGlzDQo+IGZvdW5kLg0KPiA+ICsgKiBJbiB0aGlzIGNhc2UsIGluc3RlYWQgb2YgcmV0dXJu aW5nIC1FTk9FTlQsIGl0IHJldHVybnMgTlVMTC4NCj4gPiArICovDQo+ID4gK3N0cnVjdCBjbGsg Km9mX2Nsa19nZXRfYnlfbmFtZV9vcHRpb25hbChzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wLA0KPiA+ ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBjaGFyICpuYW1l KSB7DQo+ID4gKyAgICAgICBpZiAoIW5wKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gRVJS X1BUUigtRU5PRU5UKTsNCj4gDQo+IFNob3VsZG4ndCB0aGlzIHJldHVybiBOVUxMPw0KSSB3YXNu 4oCZdCBzdXJlIG9uIHRoaXMsIEkgdGhvdWdodCB0aGUgbm9kZSBzaG91bGQgYmUgdmFsaWQsIGJ1 dCBJIGd1ZXNzIHRoZSBub2RlDQppcyBhbHNvIG9wdGlvbmFsLiBJJ2xsIGZpeC4NCg0KPiBPciBs ZXQgX19vZl9jbGtfZ2V0X2J5X25hbWUoKSBoYW5kbGUgdGhhdCAoY2ZyLiBhYm92ZSk/DQoNCj4g SG1tLCBvZl9jbGtfZ2V0X2J5X25hbWUoKSBoYXMgYSBzaW1pbGFyIGNoZWNrLCB3aGlsZSB0aGUg Y3VycmVudA0KPiBfX29mX2Nsa19nZXRfYnlfbmFtZSgpIGFscmVhZHkgaGFuZGxlIG5wID09IE5V TEwsIHRvby4NClllcyBJIHNlZSwgSSdsbCBjbGVhbiB0aGF0IHVwLg0KDQogDQo+ID4gKw0KPiA+ ICsgICAgICAgcmV0dXJuIF9fb2ZfY2xrX2dldF9ieV9uYW1lKG5wLCBucC0+ZnVsbF9uYW1lLCBu YW1lLCB0cnVlKTsgfQ0KPiA+ICtFWFBPUlRfU1lNQk9MKG9mX2Nsa19nZXRfYnlfbmFtZV9vcHRp b25hbCk7DQo+ID4gKw0KPiA+ICAjZWxzZSAvKiBkZWZpbmVkKENPTkZJR19PRikgJiYgZGVmaW5l ZChDT05GSUdfQ09NTU9OX0NMSykgKi8NCj4gPg0KPiA+ICBzdGF0aWMgc3RydWN0IGNsayAqX19v Zl9jbGtfZ2V0X2J5X25hbWUoc3RydWN0IGRldmljZV9ub2RlICpucCwNCj4gPiAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgY29uc3QgY2hhciAqZGV2X2lkLA0KPiA+IC0g ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBjaGFyICpuYW1lKQ0K PiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBjaGFyICpu YW1lLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBib29sIG9w dGlvbmFsKQ0KPiA+ICB7DQo+ID4gICAgICAgICByZXR1cm4gRVJSX1BUUigtRU5PRU5UKTsNCj4g PiAgfQ0KPiA+IEBAIC0xOTcsNyArMjIzLDcgQEAgc3RydWN0IGNsayAqY2xrX2dldChzdHJ1Y3Qg ZGV2aWNlICpkZXYsIGNvbnN0IGNoYXINCj4gKmNvbl9pZCkNCj4gPiAgICAgICAgIHN0cnVjdCBj bGsgKmNsazsNCj4gPg0KPiA+ICAgICAgICAgaWYgKGRldiAmJiBkZXYtPm9mX25vZGUpIHsNCj4g PiAtICAgICAgICAgICAgICAgY2xrID0gX19vZl9jbGtfZ2V0X2J5X25hbWUoZGV2LT5vZl9ub2Rl LCBkZXZfaWQsIGNvbl9pZCk7DQo+ID4gKyAgICAgICAgICAgICAgIGNsayA9IF9fb2ZfY2xrX2dl dF9ieV9uYW1lKGRldi0+b2Zfbm9kZSwgZGV2X2lkLA0KPiA+ICsgY29uX2lkLCBmYWxzZSk7DQo+ ID4gICAgICAgICAgICAgICAgIGlmICghSVNfRVJSKGNsaykgfHwgUFRSX0VSUihjbGspID09IC1F UFJPQkVfREVGRVIpDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIGNsazsNCj4g PiAgICAgICAgIH0NCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9jbGsuaCBiL2luY2x1 ZGUvbGludXgvY2xrLmggaW5kZXgNCj4gPiA0Zjc1MGM0Li44Y2I1NDU1IDEwMDY0NA0KPiA+IC0t LSBhL2luY2x1ZGUvbGludXgvY2xrLmgNCj4gPiArKysgYi9pbmNsdWRlL2xpbnV4L2Nsay5oDQo+ ID4gQEAgLTc3Nyw2ICs3NzcsNyBAQCBzdGF0aWMgaW5saW5lIHZvaWQgY2xrX2J1bGtfZGlzYWJs ZV91bnByZXBhcmUoaW50DQo+ID4gbnVtX2Nsa3MsICAjaWYgZGVmaW5lZChDT05GSUdfT0YpICYm IGRlZmluZWQoQ09ORklHX0NPTU1PTl9DTEspDQo+ID4gc3RydWN0IGNsayAqb2ZfY2xrX2dldChz dHJ1Y3QgZGV2aWNlX25vZGUgKm5wLCBpbnQgaW5kZXgpOyAgc3RydWN0IGNsaw0KPiA+ICpvZl9j bGtfZ2V0X2J5X25hbWUoc3RydWN0IGRldmljZV9ub2RlICpucCwgY29uc3QgY2hhciAqbmFtZSk7 DQo+ID4gK3N0cnVjdCBjbGsgKm9mX2Nsa19nZXRfYnlfbmFtZV9vcHRpb25hbChzdHJ1Y3QgZGV2 aWNlX25vZGUgKm5wLCBjb25zdA0KPiA+ICtjaGFyICpuYW1lKTsNCj4gPiAgc3RydWN0IGNsayAq b2ZfY2xrX2dldF9mcm9tX3Byb3ZpZGVyKHN0cnVjdCBvZl9waGFuZGxlX2FyZ3MNCj4gPiAqY2xr c3BlYyk7ICAjZWxzZSAgc3RhdGljIGlubGluZSBzdHJ1Y3QgY2xrICpvZl9jbGtfZ2V0KHN0cnVj dA0KPiA+IGRldmljZV9ub2RlICpucCwgaW50IGluZGV4KQ0KPiANCj4gTm8gZHVtbXkgdmVyc2lv biBvZiBvZl9jbGtfZ2V0X2J5X25hbWVfb3B0aW9uYWwoKSBmb3IgdGhlICFDTEsgfHwNCj4gIUNP TU1PTl9DTEsgY2FzZT8NCj4gDQo+IEl0IHNlZW1zIG9mX2Nsa19nZXRfYnlfbmFtZSgpIGFuZCBv Zl9jbGtfZ2V0X2J5X25hbWVfb3B0aW9uYWwoKSBjYW4NCj4ganVzdCBiZSBzaW1wbGUgd3JhcHBl cnMgYXJvdW5kIF9fb2ZfY2xrX2dldF9ieV9uYW1lKCksIGRpZmZlcmluZyBvbmx5IGluDQo+IHRo ZSB2YWx1ZSBvZiB0aGUgIm9wdGlvbmFsIiBwYXJhbWV0ZXIuIEhlbmNlIEkgdGhpbmsgaXQgbWFr ZXMgc2Vuc2UgdG8gbW92ZQ0KPiB0aGVtIHRvIHRoZSBoZWFkZXIgZmlsZSwgYW5kIG1ha2UgdGhl bSBzdGF0aWMgaW5saW5lLg0KPiBUaGVuIG9ubHkgX19vZl9jbGtfZ2V0X2J5X25hbWUoKSBuZWVk cyBhIChzdGF0aWMgaW5saW5lKSBkdW1teSBmb3IgdGhlDQo+ICFPRiB8fCAhQ09NTU9OX0NMSyBj YXNlLg0KTWFrZXMgc2Vuc2UuDQoNClRoYW5rcyENClBoaWwNCg0K ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function 2018-07-30 8:55 ` Geert Uytterhoeven 2018-07-30 9:25 ` Phil Edworthy @ 2018-07-30 10:57 ` Andy Shevchenko 2018-07-30 11:29 ` Geert Uytterhoeven 1 sibling, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2018-07-30 10:57 UTC (permalink / raw) To: Geert Uytterhoeven, Phil Edworthy Cc: Michael Turquette, Stephen Boyd, Russell King, Simon Horman, linux-clk, Linux Kernel Mailing List, Linux ARM On Mon, 2018-07-30 at 10:55 +0200, Geert Uytterhoeven wrote: > On Mon, Jul 30, 2018 at 10:36 AM Phil Edworthy > <phil.edworthy@renesas.com> wrote: > > +struct clk *of_clk_get_by_name_optional(struct device_node *np, > > + const char *name) > > +{ > > + if (!np) > > + return ERR_PTR(-ENOENT); > > Shouldn't this return NULL? > Or let __of_clk_get_by_name() handle that (cfr. above)? > > Hmm, of_clk_get_by_name() has a similar check, while the current > __of_clk_get_by_name() already handle np == NULL, too. This check is needed to prevent NULL pointer dereference below. And I agree that absence of device node should be considered as okay case for optional clock (!CONFIG_OF case, for example). > > + > > + return __of_clk_get_by_name(np, np->full_name, name, true); > > +} -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function 2018-07-30 10:57 ` Andy Shevchenko @ 2018-07-30 11:29 ` Geert Uytterhoeven 0 siblings, 0 replies; 7+ messages in thread From: Geert Uytterhoeven @ 2018-07-30 11:29 UTC (permalink / raw) To: Andy Shevchenko Cc: Phil Edworthy, Michael Turquette, Stephen Boyd, Russell King, Simon Horman, linux-clk, Linux Kernel Mailing List, Linux ARM Hi Andy, On Mon, Jul 30, 2018 at 12:57 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, 2018-07-30 at 10:55 +0200, Geert Uytterhoeven wrote: > > On Mon, Jul 30, 2018 at 10:36 AM Phil Edworthy > > <phil.edworthy@renesas.com> wrote: > > > > +struct clk *of_clk_get_by_name_optional(struct device_node *np, > > > + const char *name) > > > +{ > > > + if (!np) > > > + return ERR_PTR(-ENOENT); > > > > Shouldn't this return NULL? > > Or let __of_clk_get_by_name() handle that (cfr. above)? > > > > Hmm, of_clk_get_by_name() has a similar check, while the current > > __of_clk_get_by_name() already handle np == NULL, too. > > This check is needed to prevent NULL pointer dereference below. Thank, I had missed that unexpected detail. > > > + > > > + return __of_clk_get_by_name(np, np->full_name, name, true); > > > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] clk: Add functions to get optional clocks 2018-07-30 8:36 [PATCH v2 0/2] Add functions to get optional clocks Phil Edworthy 2018-07-30 8:36 ` [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function Phil Edworthy @ 2018-07-30 8:36 ` Phil Edworthy 1 sibling, 0 replies; 7+ messages in thread From: Phil Edworthy @ 2018-07-30 8:36 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Russell King Cc: Geert Uytterhoeven, Simon Horman, Andy Shevchenko, linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy 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 | 18 ++++++++++++++++-- drivers/clk/clkdev.c | 17 +++++++++++++++-- include/linux/clk.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c index d854e26..a2bb01a 100644 --- a/drivers/clk/clk-devres.c +++ b/drivers/clk/clk-devres.c @@ -14,7 +14,7 @@ static void devm_clk_release(struct device *dev, void *res) clk_put(*(struct clk **)res); } -struct clk *devm_clk_get(struct device *dev, const char *id) +static struct clk *__devm_clk_get(struct device *dev, const char *id, bool optional) { struct clk **ptr, *clk; @@ -22,7 +22,10 @@ struct clk *devm_clk_get(struct device *dev, const char *id) if (!ptr) return ERR_PTR(-ENOMEM); - clk = clk_get(dev, id); + if (!optional) + clk = clk_get(dev, id); + else + clk = clk_get_optional(dev, id); if (!IS_ERR(clk)) { *ptr = clk; devres_add(dev, ptr); @@ -32,8 +35,19 @@ struct clk *devm_clk_get(struct device *dev, const char *id) return clk; } + +struct clk *devm_clk_get(struct device *dev, const char *id) +{ + return __devm_clk_get(dev, id, false); +} EXPORT_SYMBOL(devm_clk_get); +struct clk *devm_clk_get_optional(struct device *dev, const char *id) +{ + return __devm_clk_get(dev, id, true); +} +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 c1b7262..484a444 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -217,21 +217,34 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id) } EXPORT_SYMBOL(clk_get_sys); -struct clk *clk_get(struct device *dev, const char *con_id) +static struct clk *internal_clk_get(struct device *dev, const char *con_id, + bool optional) { const char *dev_id = dev ? dev_name(dev) : NULL; struct clk *clk; if (dev && dev->of_node) { - clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id, false); + clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id, + optional); if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER) return clk; } return clk_get_sys(dev_id, con_id); } + +struct clk *clk_get(struct device *dev, const char *con_id) +{ + return internal_clk_get(dev, con_id, false); +} EXPORT_SYMBOL(clk_get); +struct clk *clk_get_optional(struct device *dev, const char *con_id) +{ + return internal_clk_get(dev, con_id, true); +} +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 8cb5455..22040ea 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -291,6 +291,16 @@ 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. + * @dev: device for clock "consumer" + * @id: clock consumer ID + * + * 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 @@ -349,6 +359,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" @@ -636,6 +654,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) { @@ -647,6 +670,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) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-07-30 11:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-30 8:36 [PATCH v2 0/2] Add functions to get optional clocks Phil Edworthy 2018-07-30 8:36 ` [PATCH v2 1/2] clk: Add of_clk_get_by_name_optional() function Phil Edworthy 2018-07-30 8:55 ` Geert Uytterhoeven 2018-07-30 9:25 ` Phil Edworthy 2018-07-30 10:57 ` Andy Shevchenko 2018-07-30 11:29 ` Geert Uytterhoeven 2018-07-30 8:36 ` [PATCH v2 2/2] clk: Add functions to get optional clocks Phil Edworthy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox