From: Phil Edworthy <phil.edworthy@renesas.com> To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: Stephen Boyd <sboyd@kernel.org>, Michael Turquette <mturquette@baylibre.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Russell King <linux@armlinux.org.uk>, "linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, Geert Uytterhoeven <geert@linux-m68k.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH v6 1/6] clk: Add of_clk_get_by_name_optional() function Date: Mon, 19 Nov 2018 12:53:46 +0000 [thread overview] Message-ID: <TY1PR01MB17695E8ED56BBC03AAA7FD8FF5D80@TY1PR01MB1769.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <20181119104603.qzuvpoha622l4xvy@pengutronix.de> Hi Uwe, On 19 November 2018 10:46 Uwe Kleine-König wrote: > On Mon, Nov 19, 2018 at 10:41:42AM +0000, Phil Edworthy wrote: > > On 16 November 2018 16:11 Uwe Kleine-König wrote: > > > On Fri, Nov 16, 2018 at 05:01:28PM +0100, Uwe Kleine-König wrote: > > > > Other than that I think the patch is fine > > > > > > Thinking again, I wonder why not just do: > > > > > > static inline struct clk *clk_get_optional(struct device *dev, const char > *id) { > > > struct clk *c = clk_get(dev, id); > > > > > > if (c == ERR_PTR(-ENOENT)) > > > return NULL; > > > else > > > return c; > > > } > > > > Unfortunately, underneath this __of_clk_get_by_name() returns -EINVAL > > when looking for a named clock, and the "clock-names" OF property > > can't be found or the name is not in that prop. This is because the > > index returned by of_property_match_string() will be an error code and > > is then currently always passed to __of_clk_get(). > > > > If, as you said, I split the patches into one that fixes the error > > code, and then adds clk_get_optional() like above, it will make more sense. > > Sounds like a good plan. Now that I have removed of_clk_get_by_name_optional(), I see that clk_get() deals with __of_clk_get_by_name() returning -EINVAL and -ENOENT the same way. In both cases, clk_get_sys() will return -ENOENT... i.e. I no longer need to modify __of_clk_get_by_name(). All I need is a simple wrapper just as you have outlined above. > > btw, do we need to add of_clk_get_by_name_optional()? I only added it > > as a counterpart to of_clk_get_by_name(), but it may not be needed. > > I don't need it. Given that it is easy to add when someone has a need, I'd say, > skip it for now. I'm wondering if we actually need clk_get_optional(). For me at least, I just want devm_clk_get_optional(). That would get rid of the arch patches. Thanks Phil
WARNING: multiple messages have this Message-ID (diff)
From: phil.edworthy@renesas.com (Phil Edworthy) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v6 1/6] clk: Add of_clk_get_by_name_optional() function Date: Mon, 19 Nov 2018 12:53:46 +0000 [thread overview] Message-ID: <TY1PR01MB17695E8ED56BBC03AAA7FD8FF5D80@TY1PR01MB1769.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <20181119104603.qzuvpoha622l4xvy@pengutronix.de> Hi Uwe, On 19 November 2018 10:46 Uwe Kleine-K?nig wrote: > On Mon, Nov 19, 2018 at 10:41:42AM +0000, Phil Edworthy wrote: > > On 16 November 2018 16:11 Uwe Kleine-K?nig wrote: > > > On Fri, Nov 16, 2018 at 05:01:28PM +0100, Uwe Kleine-K?nig wrote: > > > > Other than that I think the patch is fine > > > > > > Thinking again, I wonder why not just do: > > > > > > static inline struct clk *clk_get_optional(struct device *dev, const char > *id) { > > > struct clk *c = clk_get(dev, id); > > > > > > if (c == ERR_PTR(-ENOENT)) > > > return NULL; > > > else > > > return c; > > > } > > > > Unfortunately, underneath this __of_clk_get_by_name() returns -EINVAL > > when looking for a named clock, and the "clock-names" OF property > > can't be found or the name is not in that prop. This is because the > > index returned by of_property_match_string() will be an error code and > > is then currently always passed to __of_clk_get(). > > > > If, as you said, I split the patches into one that fixes the error > > code, and then adds clk_get_optional() like above, it will make more sense. > > Sounds like a good plan. Now that I have removed of_clk_get_by_name_optional(), I see that clk_get() deals with __of_clk_get_by_name() returning -EINVAL and -ENOENT the same way. In both cases, clk_get_sys() will return -ENOENT... i.e. I no longer need to modify __of_clk_get_by_name(). All I need is a simple wrapper just as you have outlined above. > > btw, do we need to add of_clk_get_by_name_optional()? I only added it > > as a counterpart to of_clk_get_by_name(), but it may not be needed. > > I don't need it. Given that it is easy to add when someone has a need, I'd say, > skip it for now. I'm wondering if we actually need clk_get_optional(). For me at least, I just want devm_clk_get_optional(). That would get rid of the arch patches. Thanks Phil
next prev parent reply other threads:[~2018-11-19 12:53 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-16 14:59 [PATCH v6 0/6] clk: Add functions to get optional clocks Phil Edworthy 2018-11-16 14:59 ` Phil Edworthy 2018-11-16 14:59 ` [PATCH v6 1/6] clk: Add of_clk_get_by_name_optional() function Phil Edworthy 2018-11-16 14:59 ` Phil Edworthy 2018-11-16 16:01 ` Uwe Kleine-König 2018-11-16 16:01 ` Uwe Kleine-König 2018-11-16 16:01 ` Uwe Kleine-König 2018-11-16 16:11 ` Uwe Kleine-König 2018-11-16 16:11 ` Uwe Kleine-König 2018-11-16 16:11 ` Uwe Kleine-König 2018-11-19 10:41 ` Phil Edworthy 2018-11-19 10:41 ` Phil Edworthy 2018-11-19 10:41 ` Phil Edworthy 2018-11-19 10:46 ` Uwe Kleine-König 2018-11-19 10:46 ` Uwe Kleine-König 2018-11-19 10:46 ` Uwe Kleine-König 2018-11-19 12:53 ` Phil Edworthy [this message] 2018-11-19 12:53 ` Phil Edworthy 2018-11-19 12:53 ` Phil Edworthy 2018-11-19 12:58 ` Uwe Kleine-König 2018-11-19 12:58 ` Uwe Kleine-König 2018-11-19 12:58 ` Uwe Kleine-König 2018-11-19 13:41 ` Phil Edworthy 2018-11-19 13:41 ` Phil Edworthy 2018-11-19 13:41 ` Phil Edworthy 2018-11-16 14:59 ` [PATCH v6 2/6] clk: Add (devm_)clk_get_optional() functions Phil Edworthy 2018-11-16 14:59 ` Phil Edworthy 2018-11-16 14:59 ` [PATCH v6 3/6] m68k: coldfire: Add clk_get_optional() function Phil Edworthy 2018-11-29 11:54 ` Greg Ungerer 2018-11-29 12:02 ` Phil Edworthy 2018-11-29 12:02 ` Phil Edworthy 2018-11-29 12:10 ` Greg Ungerer 2018-11-29 12:10 ` Greg Ungerer 2018-11-29 16:32 ` Christoph Hellwig 2018-12-03 12:29 ` Greg Ungerer 2018-11-16 14:59 ` [PATCH v6 4/6] MIPS: AR7: " Phil Edworthy 2018-11-16 14:59 ` [PATCH v6 5/6] MIPS: Loongson 2F: " Phil Edworthy 2018-11-16 14:59 ` [PATCH v6 6/6] arch/unicore32/kernel/clock.c: " Phil Edworthy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=TY1PR01MB17695E8ED56BBC03AAA7FD8FF5D80@TY1PR01MB1769.jpnprd01.prod.outlook.com \ --to=phil.edworthy@renesas.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=geert@linux-m68k.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mturquette@baylibre.com \ --cc=sboyd@kernel.org \ --cc=u.kleine-koenig@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.