From: Miquel Raynal <miquel.raynal@bootlin.com> To: Stephen Boyd <sboyd@kernel.org> Cc: Michael Turquette <mturquette@baylibre.com>, Russell King <linux@armlinux.org.uk>, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Antoine Tenart <antoine.tenart@bootlin.com>, Maxime Chevallier <maxime.chevallier@bootlin.com>, Gregory Clement <gregory.clement@bootlin.com>, Nadav Haklai <nadavh@marvell.com> Subject: Re: [PATCH v3 2/4] clk: core: link consumer with clock driver Date: Fri, 4 Jan 2019 16:54:06 +0100 [thread overview] Message-ID: <20190104165406.3b52399a@xps13> (raw) In-Reply-To: <154454837597.17204.11648795524314926025@swboyd.mtv.corp.google.com> Hi Stephen, Stephen Boyd <sboyd@kernel.org> wrote on Tue, 11 Dec 2018 09:12:55 -0800: > Sorry, I'm not reviewing the whole patch right now, just this one little > bit because I'm also working in the same area. > > Quoting Miquel Raynal (2018-12-04 11:24:38) > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > index 60c51871b04b..721d6b55b2fa 100644 > > --- a/include/linux/clk-provider.h > > +++ b/include/linux/clk-provider.h > > @@ -781,6 +781,8 @@ void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw); > > const char *__clk_get_name(const struct clk *clk); > > const char *clk_hw_get_name(const struct clk_hw *hw); > > struct clk_hw *__clk_get_hw(struct clk *clk); > > +void clk_link_consumer(struct device *consumer, struct clk *clk); > > +void clk_unlink_consumer(struct clk *clk); > > We shouldn't need to add these functions as far as I can tell. That's > because __clk_get() has become an internal API between clkdev.c and > clk.c that does nothing now on implementations that aren't the CCF. We > can even change this API to take a clk_hw pointer instead of a clk > pointer. > > I'd rather see us plumb a struct device and clk_hw structure down into > __clk_get() and fold it all into __clk_create_clk, possibly even > renaming __clk_create_clk to clk_hw_create_clk(). That way we can get > the calling device and clk_hw pointer in one call in the clk framework, > along with the device name and connection name, and then generate the > struct clk right there. This can simplify some code and make it easier > to extend this to associate calling devices with the clk consumer > somehow. > > Here's the diff. With this, you should be able to add and remove device > links in clk_hw_create_clk() when dev != NULL. Thanks for the help; I updated my work on top of yours, it looks ok but I need to run some more tests. However I had to tweak a parameter in one of your recent changes, you used '-1' as index in __of_clock_get() while it is not a valid value (returning an error). As in the __of_clk_get_by_name() function you removed index was just set to 0 at the top of the function, I think the below fix is valid. Thanks, Miquèl --- diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index ba4d1e06732d..f2f4f2afd28c 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -163,7 +163,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) struct clk_hw *hw; if (dev && dev->of_node) { - hw = of_clk_get_hw(dev->of_node, -1, con_id); + hw = of_clk_get_hw(dev->of_node, 0, con_id); if (!IS_ERR(hw) || PTR_ERR(hw) == -EPROBE_DEFER) return clk_hw_create_clk(dev, hw, dev_id, con_id); }
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com> To: Stephen Boyd <sboyd@kernel.org> Cc: Gregory Clement <gregory.clement@bootlin.com>, Antoine Tenart <antoine.tenart@bootlin.com>, Michael Turquette <mturquette@baylibre.com>, linux-kernel@vger.kernel.org, Russell King <linux@armlinux.org.uk>, Nadav Haklai <nadavh@marvell.com>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Maxime Chevallier <maxime.chevallier@bootlin.com>, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 2/4] clk: core: link consumer with clock driver Date: Fri, 4 Jan 2019 16:54:06 +0100 [thread overview] Message-ID: <20190104165406.3b52399a@xps13> (raw) In-Reply-To: <154454837597.17204.11648795524314926025@swboyd.mtv.corp.google.com> Hi Stephen, Stephen Boyd <sboyd@kernel.org> wrote on Tue, 11 Dec 2018 09:12:55 -0800: > Sorry, I'm not reviewing the whole patch right now, just this one little > bit because I'm also working in the same area. > > Quoting Miquel Raynal (2018-12-04 11:24:38) > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > index 60c51871b04b..721d6b55b2fa 100644 > > --- a/include/linux/clk-provider.h > > +++ b/include/linux/clk-provider.h > > @@ -781,6 +781,8 @@ void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw); > > const char *__clk_get_name(const struct clk *clk); > > const char *clk_hw_get_name(const struct clk_hw *hw); > > struct clk_hw *__clk_get_hw(struct clk *clk); > > +void clk_link_consumer(struct device *consumer, struct clk *clk); > > +void clk_unlink_consumer(struct clk *clk); > > We shouldn't need to add these functions as far as I can tell. That's > because __clk_get() has become an internal API between clkdev.c and > clk.c that does nothing now on implementations that aren't the CCF. We > can even change this API to take a clk_hw pointer instead of a clk > pointer. > > I'd rather see us plumb a struct device and clk_hw structure down into > __clk_get() and fold it all into __clk_create_clk, possibly even > renaming __clk_create_clk to clk_hw_create_clk(). That way we can get > the calling device and clk_hw pointer in one call in the clk framework, > along with the device name and connection name, and then generate the > struct clk right there. This can simplify some code and make it easier > to extend this to associate calling devices with the clk consumer > somehow. > > Here's the diff. With this, you should be able to add and remove device > links in clk_hw_create_clk() when dev != NULL. Thanks for the help; I updated my work on top of yours, it looks ok but I need to run some more tests. However I had to tweak a parameter in one of your recent changes, you used '-1' as index in __of_clock_get() while it is not a valid value (returning an error). As in the __of_clk_get_by_name() function you removed index was just set to 0 at the top of the function, I think the below fix is valid. Thanks, Miquèl --- diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index ba4d1e06732d..f2f4f2afd28c 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -163,7 +163,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) struct clk_hw *hw; if (dev && dev->of_node) { - hw = of_clk_get_hw(dev->of_node, -1, con_id); + hw = of_clk_get_hw(dev->of_node, 0, con_id); if (!IS_ERR(hw) || PTR_ERR(hw) == -EPROBE_DEFER) return clk_hw_create_clk(dev, hw, dev_id, con_id); } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-01-04 15:54 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-04 19:24 [PATCH v3 0/4] Add device links to clocks Miquel Raynal 2018-12-04 19:24 ` Miquel Raynal 2018-12-04 19:24 ` [PATCH v3 1/4] clk: core: clarify the check for runtime PM Miquel Raynal 2018-12-04 19:24 ` Miquel Raynal 2018-12-19 0:03 ` Stephen Boyd 2018-12-19 0:03 ` Stephen Boyd 2018-12-19 8:03 ` Miquel Raynal 2018-12-19 8:03 ` Miquel Raynal 2018-12-20 21:09 ` Stephen Boyd 2018-12-20 21:09 ` Stephen Boyd 2019-01-02 10:55 ` Miquel Raynal 2019-01-02 10:55 ` Miquel Raynal 2018-12-04 19:24 ` [PATCH v3 2/4] clk: core: link consumer with clock driver Miquel Raynal 2018-12-04 19:24 ` Miquel Raynal 2018-12-11 17:12 ` Stephen Boyd 2018-12-11 17:12 ` Stephen Boyd 2018-12-17 14:24 ` Miquel Raynal 2018-12-17 14:24 ` Miquel Raynal 2019-01-04 15:54 ` Miquel Raynal [this message] 2019-01-04 15:54 ` Miquel Raynal 2019-01-25 21:28 ` Stephen Boyd 2019-01-25 21:28 ` Stephen Boyd 2019-01-28 10:06 ` Miquel Raynal 2019-01-28 10:06 ` Miquel Raynal 2018-12-04 19:24 ` [PATCH v3 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks Miquel Raynal 2018-12-04 19:24 ` Miquel Raynal 2018-12-04 19:24 ` [PATCH v3 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock Miquel Raynal 2018-12-04 19:24 ` Miquel Raynal
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=20190104165406.3b52399a@xps13 \ --to=miquel.raynal@bootlin.com \ --cc=antoine.tenart@bootlin.com \ --cc=gregory.clement@bootlin.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=maxime.chevallier@bootlin.com \ --cc=mturquette@baylibre.com \ --cc=nadavh@marvell.com \ --cc=sboyd@kernel.org \ --cc=thomas.petazzoni@bootlin.com \ /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.