[alsa-devel] [PATCH 00/10] initial clkdev cleanups
Here's some initial clkdev cleanups. These are targetted for the next merge window, and while the initial patches can be merged independently, I'd prefer to keep the series together as further work on solving the problems which unique struct clk's has introduced is needed.
The initial cleanups are more about using the correct clkdev function than anything else: there's no point interating over a array of clk_lookup structs, adding each one in turn when we have had a function which does this since forever.
I'm also killing a chunk of seemingly unused code in the omap3isp driver.
Lastly, I'm introducing a clkdev_create() helper, which combines the clkdev_alloc() + clkdev_add() pattern which keeps cropping up.
Individual patches copied to appropriate people, but they will all appear on the mailing lists.
arch/arm/mach-lpc32xx/clock.c | 5 +-- arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c | 12 ++----- arch/arm/mach-omap2/omap_device.c | 24 +++++-------- arch/arm/plat-orion/common.c | 6 +--- arch/sh/kernel/cpu/sh4a/clock-sh7734.c | 3 +- arch/sh/kernel/cpu/sh4a/clock-sh7757.c | 4 +-- arch/sh/kernel/cpu/sh4a/clock-sh7785.c | 4 +-- arch/sh/kernel/cpu/sh4a/clock-sh7786.c | 4 +-- arch/sh/kernel/cpu/sh4a/clock-shx3.c | 4 +-- drivers/clk/clk-s2mps11.c | 4 +-- drivers/clk/clkdev.c | 52 +++++++++++++++++++++------- drivers/clk/versatile/clk-impd1.c | 4 +-- drivers/media/platform/omap3isp/isp.c | 18 ---------- drivers/media/platform/omap3isp/isp.h | 1 - include/linux/clkdev.h | 3 ++ include/media/omap3isp.h | 6 ---- sound/soc/sh/migor.c | 3 +- 17 files changed, 68 insertions(+), 89 deletions(-)
No merged platform supplies xclks via platform data. As we want to slightly change the clkdev interface, rather than fixing this unused code, remove it instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/media/platform/omap3isp/isp.c | 18 ------------------ drivers/media/platform/omap3isp/isp.h | 1 - include/media/omap3isp.h | 6 ------ 3 files changed, 25 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index deca80903c3a..4d8078b9d010 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -281,7 +281,6 @@ static const struct clk_init_data isp_xclk_init_data = {
static int isp_xclk_init(struct isp_device *isp) { - struct isp_platform_data *pdata = isp->pdata; struct clk_init_data init; unsigned int i;
@@ -311,20 +310,6 @@ static int isp_xclk_init(struct isp_device *isp) xclk->clk = clk_register(NULL, &xclk->hw); if (IS_ERR(xclk->clk)) return PTR_ERR(xclk->clk); - - if (pdata->xclks[i].con_id == NULL && - pdata->xclks[i].dev_id == NULL) - continue; - - xclk->lookup = kzalloc(sizeof(*xclk->lookup), GFP_KERNEL); - if (xclk->lookup == NULL) - return -ENOMEM; - - xclk->lookup->con_id = pdata->xclks[i].con_id; - xclk->lookup->dev_id = pdata->xclks[i].dev_id; - xclk->lookup->clk = xclk->clk; - - clkdev_add(xclk->lookup); }
return 0; @@ -339,9 +324,6 @@ static void isp_xclk_cleanup(struct isp_device *isp)
if (!IS_ERR(xclk->clk)) clk_unregister(xclk->clk); - - if (xclk->lookup) - clkdev_drop(xclk->lookup); } }
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index cfdfc8714b6b..d41c98bbdfe7 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -122,7 +122,6 @@ enum isp_xclk_id { struct isp_xclk { struct isp_device *isp; struct clk_hw hw; - struct clk_lookup *lookup; struct clk *clk; enum isp_xclk_id id;
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h index 398279dd1922..a9798525d01e 100644 --- a/include/media/omap3isp.h +++ b/include/media/omap3isp.h @@ -152,13 +152,7 @@ struct isp_v4l2_subdevs_group { } bus; /* gcc < 4.6.0 chokes on anonymous union initializers */ };
-struct isp_platform_xclk { - const char *dev_id; - const char *con_id; -}; - struct isp_platform_data { - struct isp_platform_xclk xclks[2]; struct isp_v4l2_subdevs_group *subdevs; void (*set_constraints)(struct isp_device *isp, bool enable); };
Hi Russell,
On Monday 02 March 2015 17:06:06 Russell King wrote:
No merged platform supplies xclks via platform data. As we want to slightly change the clkdev interface, rather than fixing this unused code, remove it instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
There are quite a few out of tree users that I know of that might be impacted. On the other hand, out of tree isn't an excuse, and OMAP3 platforms should move to DT. The good news is that DT support for the omap3isp driver is about to get submitted, and hopefully merged in v4.1. I thus have no objection to this patch.
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/media/platform/omap3isp/isp.c | 18 ------------------ drivers/media/platform/omap3isp/isp.h | 1 - include/media/omap3isp.h | 6 ------ 3 files changed, 25 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index deca80903c3a..4d8078b9d010 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -281,7 +281,6 @@ static const struct clk_init_data isp_xclk_init_data = {
static int isp_xclk_init(struct isp_device *isp) {
- struct isp_platform_data *pdata = isp->pdata; struct clk_init_data init; unsigned int i;
@@ -311,20 +310,6 @@ static int isp_xclk_init(struct isp_device *isp) xclk->clk = clk_register(NULL, &xclk->hw); if (IS_ERR(xclk->clk)) return PTR_ERR(xclk->clk);
if (pdata->xclks[i].con_id == NULL &&
pdata->xclks[i].dev_id == NULL)
continue;
xclk->lookup = kzalloc(sizeof(*xclk->lookup), GFP_KERNEL);
if (xclk->lookup == NULL)
return -ENOMEM;
xclk->lookup->con_id = pdata->xclks[i].con_id;
xclk->lookup->dev_id = pdata->xclks[i].dev_id;
xclk->lookup->clk = xclk->clk;
clkdev_add(xclk->lookup);
}
return 0;
@@ -339,9 +324,6 @@ static void isp_xclk_cleanup(struct isp_device *isp)
if (!IS_ERR(xclk->clk)) clk_unregister(xclk->clk);
if (xclk->lookup)
}clkdev_drop(xclk->lookup);
}
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index cfdfc8714b6b..d41c98bbdfe7 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -122,7 +122,6 @@ enum isp_xclk_id { struct isp_xclk { struct isp_device *isp; struct clk_hw hw;
- struct clk_lookup *lookup; struct clk *clk; enum isp_xclk_id id;
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h index 398279dd1922..a9798525d01e 100644 --- a/include/media/omap3isp.h +++ b/include/media/omap3isp.h @@ -152,13 +152,7 @@ struct isp_v4l2_subdevs_group { } bus; /* gcc < 4.6.0 chokes on anonymous union initializers */ };
-struct isp_platform_xclk {
- const char *dev_id;
- const char *con_id;
-};
struct isp_platform_data {
- struct isp_platform_xclk xclks[2]; struct isp_v4l2_subdevs_group *subdevs; void (*set_constraints)(struct isp_device *isp, bool enable);
};
Hi Laurent and Russell,
On Tue, Mar 03, 2015 at 12:33:44AM +0200, Laurent Pinchart wrote:
Hi Russell,
On Monday 02 March 2015 17:06:06 Russell King wrote:
No merged platform supplies xclks via platform data. As we want to slightly change the clkdev interface, rather than fixing this unused code, remove it instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
There are quite a few out of tree users that I know of that might be impacted. On the other hand, out of tree isn't an excuse, and OMAP3 platforms should move to DT. The good news is that DT support for the omap3isp driver is about to get submitted, and hopefully merged in v4.1. I thus have no objection to this patch.
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
I first thought it wouldn't conflict, but apparently it does. The conflicting patches are here:
URL:http://vihersipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=commitdiff;h=a56c38208ee9200e57421b60b770fb8249935b95 URL:http://vihersipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=commitdiff;h=72374c7a69a12afc76f220ef4de983be4583f164 URL:http://vihersipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=commitdiff;h=0f0b86d64a555e308079f985812b011866e2c8f0
Tne entire set:
I haven't sent that to linux-media yet but I'll do that during the coming couple of days.
(Combining replies...)
On Tue, Mar 03, 2015 at 12:53:37AM +0200, Sakari Ailus wrote:
Hi Laurent and Russell,
On Tue, Mar 03, 2015 at 12:33:44AM +0200, Laurent Pinchart wrote:
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
As other changes will depend on this, I'd prefer not to. The whole "make clk_get() return a unique struct clk" wasn't well tested, and several places broke - and currently clk_add_alias() is broken as a result of that.
I'm trying to get to the longer term solution, where clkdev internally uses a struct clk_hw pointer rather than a struct clk pointer, and I want to clean stuff up first.
If omap3isp needs to keep this code, then so be it - I'll come up with a different patch improving its use of clkdev instead.
Hi Russell,
On Monday 02 March 2015 23:54:35 Russell King - ARM Linux wrote:
(Combining replies...)
On Tue, Mar 03, 2015 at 12:53:37AM +0200, Sakari Ailus wrote:
Hi Laurent and Russell,
On Tue, Mar 03, 2015 at 12:33:44AM +0200, Laurent Pinchart wrote:
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
As other changes will depend on this, I'd prefer not to. The whole "make clk_get() return a unique struct clk" wasn't well tested, and several places broke - and currently clk_add_alias() is broken as a result of that.
I'm trying to get to the longer term solution, where clkdev internally uses a struct clk_hw pointer rather than a struct clk pointer, and I want to clean stuff up first.
If omap3isp needs to keep this code, then so be it - I'll come up with a different patch improving its use of clkdev instead.
I'm totally fine with removing clkdev from the omap3isp driver if that's easier for you, I'm just concerned about the merge conflict that will result.
Hi Laurent,
On Tue, Mar 03, 2015 at 02:39:14AM +0200, Laurent Pinchart wrote:
Hi Russell,
On Monday 02 March 2015 23:54:35 Russell King - ARM Linux wrote:
(Combining replies...)
On Tue, Mar 03, 2015 at 12:53:37AM +0200, Sakari Ailus wrote:
Hi Laurent and Russell,
On Tue, Mar 03, 2015 at 12:33:44AM +0200, Laurent Pinchart wrote:
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
As other changes will depend on this, I'd prefer not to. The whole "make clk_get() return a unique struct clk" wasn't well tested, and several places broke - and currently clk_add_alias() is broken as a result of that.
I'm trying to get to the longer term solution, where clkdev internally uses a struct clk_hw pointer rather than a struct clk pointer, and I want to clean stuff up first.
If omap3isp needs to keep this code, then so be it - I'll come up with a different patch improving its use of clkdev instead.
I'm totally fine with removing clkdev from the omap3isp driver if that's easier for you, I'm just concerned about the merge conflict that will result.
There actually appears to be one more dependency, so four patches in total.
What we could also do is to rebase the omap3isp DT support set on top of Russell's single patch. This way there probably would be no merge conflict, assuming the patches are exactly the same, and you have no other patches changing the omap3isp driver.
Alternatively we could postpone the DT support for the omap3isp, but I'd rather want to avoid that.
Hi Russell,
On Mon, Mar 02, 2015 at 11:54:35PM +0000, Russell King - ARM Linux wrote:
(Combining replies...)
On Tue, Mar 03, 2015 at 12:53:37AM +0200, Sakari Ailus wrote:
Hi Laurent and Russell,
On Tue, Mar 03, 2015 at 12:33:44AM +0200, Laurent Pinchart wrote:
Sakari, does it conflict with the omap3isp DT support ? If so, how would you prefer to resolve the conflict ? Russell, would it be fine to merge this through Mauro's tree ?
As other changes will depend on this, I'd prefer not to. The whole "make clk_get() return a unique struct clk" wasn't well tested, and several places broke - and currently clk_add_alias() is broken as a result of that.
I'm trying to get to the longer term solution, where clkdev internally uses a struct clk_hw pointer rather than a struct clk pointer, and I want to clean stuff up first.
If omap3isp needs to keep this code, then so be it - I'll come up with a different patch improving its use of clkdev instead.
I discussed this with Laurent and the two options we thought are
- You provide a stable branch on which I can rebase the patches, in order to avoid a merge conflict or
- We ignore the conflict and let Stephen Rothwell handle it. The conflict itself is relatively simple to resolve.
We have always had an efficient way of registering a table of clock lookups - it's called clkdev_add_table(). However, some people seem to really love writing inefficient and unnecessary code.
Convert SH to use the correct interface.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/sh/kernel/cpu/sh4a/clock-sh7734.c | 3 +-- arch/sh/kernel/cpu/sh4a/clock-sh7757.c | 4 ++-- arch/sh/kernel/cpu/sh4a/clock-sh7785.c | 4 ++-- arch/sh/kernel/cpu/sh4a/clock-sh7786.c | 4 ++-- arch/sh/kernel/cpu/sh4a/clock-shx3.c | 4 ++-- 5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7734.c b/arch/sh/kernel/cpu/sh4a/clock-sh7734.c index 1fdf1ee672de..7f54bf2f453d 100644 --- a/arch/sh/kernel/cpu/sh4a/clock-sh7734.c +++ b/arch/sh/kernel/cpu/sh4a/clock-sh7734.c @@ -246,8 +246,7 @@ int __init arch_clk_init(void) for (i = 0; i < ARRAY_SIZE(main_clks); i++) ret |= clk_register(main_clks[i]);
- for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
if (!ret) ret = sh_clk_div4_register(div4_clks, ARRAY_SIZE(div4_clks), diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7757.c b/arch/sh/kernel/cpu/sh4a/clock-sh7757.c index 9a28fdb36387..e40ec2c97ad1 100644 --- a/arch/sh/kernel/cpu/sh4a/clock-sh7757.c +++ b/arch/sh/kernel/cpu/sh4a/clock-sh7757.c @@ -141,8 +141,8 @@ int __init arch_clk_init(void)
for (i = 0; i < ARRAY_SIZE(clks); i++) ret |= clk_register(clks[i]); - for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
if (!ret) ret = sh_clk_div4_register(div4_clks, ARRAY_SIZE(div4_clks), diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c index 17d0ea55a5a2..8eb6e62340c9 100644 --- a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c +++ b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c @@ -164,8 +164,8 @@ int __init arch_clk_init(void)
for (i = 0; i < ARRAY_SIZE(clks); i++) ret |= clk_register(clks[i]); - for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
if (!ret) ret = sh_clk_div4_register(div4_clks, ARRAY_SIZE(div4_clks), diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c index bec2a83f1ba5..5e50e7ebeff0 100644 --- a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c +++ b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c @@ -179,8 +179,8 @@ int __init arch_clk_init(void)
for (i = 0; i < ARRAY_SIZE(clks); i++) ret |= clk_register(clks[i]); - for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
if (!ret) ret = sh_clk_div4_register(div4_clks, ARRAY_SIZE(div4_clks), diff --git a/arch/sh/kernel/cpu/sh4a/clock-shx3.c b/arch/sh/kernel/cpu/sh4a/clock-shx3.c index 9a49a44f6f94..605221d1448a 100644 --- a/arch/sh/kernel/cpu/sh4a/clock-shx3.c +++ b/arch/sh/kernel/cpu/sh4a/clock-shx3.c @@ -138,8 +138,8 @@ int __init arch_clk_init(void)
for (i = 0; i < ARRAY_SIZE(clks); i++) ret |= clk_register(clks[i]); - for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
if (!ret) ret = sh_clk_div4_register(div4_clks, ARRAY_SIZE(div4_clks),
On Mon, Mar 2, 2015 at 6:06 PM, Russell King rmk+kernel@arm.linux.org.uk wrote:
We have always had an efficient way of registering a table of clock lookups - it's called clkdev_add_table(). However, some people seem to really love writing inefficient and unnecessary code.
Convert SH to use the correct interface.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Thanks, looks good.
Acked-by: Geert Uytterhoeven geert+renesas@glider.be
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
We have always had an efficient way of registering a table of clock lookups - it's called clkdev_add_table(). However, some people seem to really love writing inefficient and unnecessary code.
Convert Integrator IM-PD/1 to use the correct interface.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/versatile/clk-impd1.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clk/versatile/clk-impd1.c b/drivers/clk/versatile/clk-impd1.c index 1cc1330dc570..13e912e132d2 100644 --- a/drivers/clk/versatile/clk-impd1.c +++ b/drivers/clk/versatile/clk-impd1.c @@ -89,7 +89,6 @@ void integrator_impd1_clk_init(void __iomem *base, unsigned int id) struct impd1_clk *imc; struct clk *clk; struct clk *pclk; - int i;
if (id > 3) { pr_crit("no more than 4 LMs can be attached\n"); @@ -150,8 +149,7 @@ void integrator_impd1_clk_init(void __iomem *base, unsigned int id) imc->clks[13] = clkdev_alloc(pclk, "apb_pclk", "lm%x:00600", id); imc->clks[14] = clkdev_alloc(clk, NULL, "lm%x:00600", id);
- for (i = 0; i < ARRAY_SIZE(imc->clks); i++) - clkdev_add(imc->clks[i]); + clkdev_add_table(imc->clks, ARRAY_SIZE(imc->clks)); } EXPORT_SYMBOL_GPL(integrator_impd1_clk_init);
We have always had an efficient way of registering a table of clock lookups - it's called clkdev_add_table(). However, some people seem to really love writing inefficient and unnecessary code.
Convert LPC32xx to use the correct interface.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/mach-lpc32xx/clock.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-lpc32xx/clock.c b/arch/arm/mach-lpc32xx/clock.c index dd5d6f532e8c..661c8f4b2310 100644 --- a/arch/arm/mach-lpc32xx/clock.c +++ b/arch/arm/mach-lpc32xx/clock.c @@ -1238,10 +1238,7 @@ static struct clk_lookup lookups[] = {
static int __init clk_init(void) { - int i; - - for (i = 0; i < ARRAY_SIZE(lookups); i++) - clkdev_add(&lookups[i]); + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
/* * Setup muxed SYSCLK for HCLK PLL base -this selects the
Add a helper to allocate and add a clk_lookup structure. This can not only be used in several places in clkdev.c to simplify the code, but more importantly, can be used by callers of the clkdev code to simplify their clkdev creation and registration.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/clkdev.c | 52 ++++++++++++++++++++++++++++++++++++++------------ include/linux/clkdev.h | 3 +++ 2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 043fd3633373..611b9acbad78 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -294,6 +294,19 @@ vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, return &cla->cl; }
+static struct clk_lookup * +vclkdev_create((struct clk *clk, const char *con_id, const char *dev_fmt, + va_list ap) +{ + struct clk_lookup *cl; + + cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); + if (cl) + clkdev_add(cl); + + return cl; +} + struct clk_lookup * __init_refok clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) { @@ -308,6 +321,28 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) } EXPORT_SYMBOL(clkdev_alloc);
+/** + * clkdev_create - allocate and add a clkdev lookup structure + * @clk: struct clk to associate with all clk_lookups + * @con_id: connection ID string on device + * @dev_fmt: format string describing device name + * + * Returns a clk_lookup structure, which can be later unregistered and + * freed. + */ +struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id, + const char *dev_fmt, ...) +{ + struct clk_lookup *cl; + va_list ap; + + va_start(ap, dev_fmt); + cl = vclkdev_create(clk, con_id, dev_fmt, ap); + va_end(ap); + + return cl; +} + int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, struct device *dev) { @@ -317,12 +352,10 @@ int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, if (IS_ERR(r)) return PTR_ERR(r);
- l = clkdev_alloc(r, alias, alias_dev_name); + l = vclkdev_create(r, alias, "%s", alias_dev_name); clk_put(r); - if (!l) - return -ENODEV; - clkdev_add(l); - return 0; + + return l ? 0 : -ENODEV; } EXPORT_SYMBOL(clk_add_alias);
@@ -362,15 +395,10 @@ int clk_register_clkdev(struct clk *clk, const char *con_id, return PTR_ERR(clk);
va_start(ap, dev_fmt); - cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); + cl = vclkdev_create(clk, con_id, dev_fmt, ap); va_end(ap);
- if (!cl) - return -ENOMEM; - - clkdev_add(cl); - - return 0; + return cl ? 0 : -ENOMEM; } EXPORT_SYMBOL(clk_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h index 94bad77eeb4a..6f32f6d8b6ee 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -37,6 +37,9 @@ struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id, void clkdev_add(struct clk_lookup *cl); void clkdev_drop(struct clk_lookup *cl);
+struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id, + const char *dev_fmt, ...); + void clkdev_add_table(struct clk_lookup *, size_t); int clk_add_alias(const char *, const char *, char *, struct device *);
On Mon, Mar 2, 2015 at 6:06 PM, Russell King rmk+kernel@arm.linux.org.uk wrote:
--- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -37,6 +37,9 @@ struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id, void clkdev_add(struct clk_lookup *cl); void clkdev_drop(struct clk_lookup *cl);
+struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id,
const char *dev_fmt, ...);
__printf(3, 4)
While you're at it, can you please also add the __printf attribute to clkdev_alloc() and clk_register_clkdev()?
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
On Mon, Mar 02, 2015 at 06:22:31PM +0100, Geert Uytterhoeven wrote:
On Mon, Mar 2, 2015 at 6:06 PM, Russell King rmk+kernel@arm.linux.org.uk wrote:
--- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -37,6 +37,9 @@ struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id, void clkdev_add(struct clk_lookup *cl); void clkdev_drop(struct clk_lookup *cl);
+struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id,
const char *dev_fmt, ...);
__printf(3, 4)
While you're at it, can you please also add the __printf attribute to clkdev_alloc() and clk_register_clkdev()?
What's the behaviour of __printf() with a NULL format string? The clkdev interfaces permit that, normal printf() doesn't.
On 03/02/15 09:06, Russell King wrote:
Add a helper to allocate and add a clk_lookup structure. This can not only be used in several places in clkdev.c to simplify the code, but more importantly, can be used by callers of the clkdev code to simplify their clkdev creation and registration.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
drivers/clk/clkdev.c | 52 ++++++++++++++++++++++++++++++++++++++------------ include/linux/clkdev.h | 3 +++ 2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 043fd3633373..611b9acbad78 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -294,6 +294,19 @@ vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, return &cla->cl; }
+static struct clk_lookup * +vclkdev_create((struct clk *clk, const char *con_id, const char *dev_fmt,
- va_list ap)
+{
- struct clk_lookup *cl;
- cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
- if (cl)
clkdev_add(cl);
- return cl;
+}
struct clk_lookup * __init_refok clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) { @@ -308,6 +321,28 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) } EXPORT_SYMBOL(clkdev_alloc);
+/**
- clkdev_create - allocate and add a clkdev lookup structure
- @clk: struct clk to associate with all clk_lookups
- @con_id: connection ID string on device
- @dev_fmt: format string describing device name
- Returns a clk_lookup structure, which can be later unregistered and
- freed.
And returns NULL on failure? Any reason why we don't return an error pointer on failure?
On Mon, Mar 02, 2015 at 11:07:58AM -0800, Stephen Boyd wrote:
On 03/02/15 09:06, Russell King wrote:
Add a helper to allocate and add a clk_lookup structure. This can not only be used in several places in clkdev.c to simplify the code, but more importantly, can be used by callers of the clkdev code to simplify their clkdev creation and registration.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
drivers/clk/clkdev.c | 52 ++++++++++++++++++++++++++++++++++++++------------ include/linux/clkdev.h | 3 +++ 2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 043fd3633373..611b9acbad78 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -294,6 +294,19 @@ vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, return &cla->cl; }
+static struct clk_lookup * +vclkdev_create((struct clk *clk, const char *con_id, const char *dev_fmt,
- va_list ap)
+{
- struct clk_lookup *cl;
- cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
- if (cl)
clkdev_add(cl);
- return cl;
+}
struct clk_lookup * __init_refok clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) { @@ -308,6 +321,28 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) } EXPORT_SYMBOL(clkdev_alloc);
+/**
- clkdev_create - allocate and add a clkdev lookup structure
- @clk: struct clk to associate with all clk_lookups
- @con_id: connection ID string on device
- @dev_fmt: format string describing device name
- Returns a clk_lookup structure, which can be later unregistered and
- freed.
And returns NULL on failure? Any reason why we don't return an error pointer on failure?
Why should it when it's only error is "no memory" ? It follows the clkdev_alloc() and memory allocator pattern.
It'd also make the error handling in places like clk_add_alias() more difficult (how that happened, I don't know...) though that could probably be fixed as no one seems to bother checking the return value... maybe that's a reason to make it return void ;)
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/soc/sh/migor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/sh/migor.c b/sound/soc/sh/migor.c index 82f582344fe7..672bcd4c252b 100644 --- a/sound/soc/sh/migor.c +++ b/sound/soc/sh/migor.c @@ -162,12 +162,11 @@ static int __init migor_init(void) if (ret < 0) return ret;
- siumckb_lookup = clkdev_alloc(&siumckb_clk, "siumckb_clk", NULL); + siumckb_lookup = clkdev_create(&siumckb_clk, "siumckb_clk", NULL); if (!siumckb_lookup) { ret = -ENOMEM; goto eclkdevalloc; } - clkdev_add(siumckb_lookup);
/* Port number used on this machine: port B */ migor_snd_device = platform_device_alloc("soc-audio", 1);
On Mon, Mar 02, 2015 at 05:06:32PM +0000, Russell King wrote:
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Acked-by: Mark Brown broonie@kernel.org
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/clk-s2mps11.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c index bfa1e64e267d..9b13a303d3f8 100644 --- a/drivers/clk/clk-s2mps11.c +++ b/drivers/clk/clk-s2mps11.c @@ -242,14 +242,12 @@ static int s2mps11_clk_probe(struct platform_device *pdev) goto err_reg; }
- s2mps11_clk->lookup = clkdev_alloc(s2mps11_clk->clk, + s2mps11_clk->lookup = clkdev_create(s2mps11_clk->clk, s2mps11_name(s2mps11_clk), NULL); if (!s2mps11_clk->lookup) { ret = -ENOMEM; goto err_lup; } - - clkdev_add(s2mps11_clk->lookup); }
for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/plat-orion/common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c index f5b00f41c4f6..2235081a04ee 100644 --- a/arch/arm/plat-orion/common.c +++ b/arch/arm/plat-orion/common.c @@ -28,11 +28,7 @@ void __init orion_clkdev_add(const char *con_id, const char *dev_id, struct clk *clk) { - struct clk_lookup *cl; - - cl = clkdev_alloc(clk, con_id, dev_id); - if (cl) - clkdev_add(cl); + clkdev_create(clk, con_id, "%s", dev_id); }
/* Create clkdev entries for all orion platforms except kirkwood.
Dear Russell King,
On Mon, 02 Mar 2015 17:06:42 +0000, Russell King wrote:
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
arch/arm/plat-orion/common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c index f5b00f41c4f6..2235081a04ee 100644 --- a/arch/arm/plat-orion/common.c +++ b/arch/arm/plat-orion/common.c @@ -28,11 +28,7 @@ void __init orion_clkdev_add(const char *con_id, const char *dev_id, struct clk *clk) {
- struct clk_lookup *cl;
- cl = clkdev_alloc(clk, con_id, dev_id);
- if (cl)
clkdev_add(cl);
- clkdev_create(clk, con_id, "%s", dev_id);
}
/* Create clkdev entries for all orion platforms except kirkwood.
Looks good, but instead of having orion_clkdev_add() being just an alias for clkdev_create(), what about going ahead and simply reoving orion_clkdev_add() entirely? Something like the below patch (not even compile tested) :
diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c index 0d1a892..ec00183 100644 --- a/arch/arm/mach-dove/common.c +++ b/arch/arm/mach-dove/common.c @@ -109,28 +109,28 @@ static void __init dove_clk_init(void) gephy = dove_register_gate("gephy", "tclk", CLOCK_GATING_BIT_GIGA_PHY); ge = dove_register_gate("ge", "gephy", CLOCK_GATING_BIT_GBE);
- orion_clkdev_add(NULL, "orion_spi.0", tclk); - orion_clkdev_add(NULL, "orion_spi.1", tclk); - orion_clkdev_add(NULL, "orion_wdt", tclk); - orion_clkdev_add(NULL, "mv64xxx_i2c.0", tclk); - - orion_clkdev_add(NULL, "orion-ehci.0", usb0); - orion_clkdev_add(NULL, "orion-ehci.1", usb1); - orion_clkdev_add(NULL, "mv643xx_eth_port.0", ge); - orion_clkdev_add(NULL, "sata_mv.0", sata); - orion_clkdev_add("0", "pcie", pex0); - orion_clkdev_add("1", "pcie", pex1); - orion_clkdev_add(NULL, "sdhci-dove.0", sdio0); - orion_clkdev_add(NULL, "sdhci-dove.1", sdio1); - orion_clkdev_add(NULL, "orion_nand", nand); - orion_clkdev_add(NULL, "cafe1000-ccic.0", camera); - orion_clkdev_add(NULL, "mvebu-audio.0", i2s0); - orion_clkdev_add(NULL, "mvebu-audio.1", i2s1); - orion_clkdev_add(NULL, "mv_crypto", crypto); - orion_clkdev_add(NULL, "dove-ac97", ac97); - orion_clkdev_add(NULL, "dove-pdma", pdma); - orion_clkdev_add(NULL, MV_XOR_NAME ".0", xor0); - orion_clkdev_add(NULL, MV_XOR_NAME ".1", xor1); + clkdev_create(tclk, NULL, "%s", "orion_spi.0"); + clkdev_create(tclk, NULL, "%s", "orion_spi.1"); + clkdev_create(tclk, NULL, "%s", "orion_wdt"); + clkdev_create(tclk, NULL, "%s", "mv64xxx_i2c.0"); + + clkdev_create(usb0, NULL, "%s", "orion-ehci.0"); + clkdev_create(usb1, NULL, "%s", "orion-ehci.1"); + clkdev_create(ge, NULL, "%s", "mv643xx_eth_port.0"); + clkdev_create(sata, NULL, "%s", "sata_mv.0"); + clkdev_create(pex0, "0", "%s", "pcie"); + clkdev_create(pex1, "1", "%s", "pcie"); + clkdev_create(sdio0, NULL, "%s", "sdhci-dove.0"); + clkdev_create(sdio1, NULL, "%s", "sdhci-dove.1"); + clkdev_create(nand, NULL, "%s", "orion_nand"); + clkdev_create(camera, NULL, "%s", "cafe1000-ccic.0"); + clkdev_create(i2s0, NULL, "%s", "mvebu-audio.0"); + clkdev_create(i2s1, NULL, "%s", "mvebu-audio.1"); + clkdev_create(crypto, NULL, "%s", "mv_crypto"); + clkdev_create(ac97, NULL, "%s", "dove-ac97"); + clkdev_create(pdma, NULL, "%s", "dove-pdma"); + clkdev_create(xor0, NULL, "%s", MV_XOR_NAME ".0"); + clkdev_create(xor1, NULL, "%s", MV_XOR_NAME ".1"); }
/***************************************************************************** diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c index f5b00f4..6ac3549 100644 --- a/arch/arm/plat-orion/common.c +++ b/arch/arm/plat-orion/common.c @@ -24,31 +24,20 @@ #include <mach/bridge-regs.h> #include <plat/common.h>
-/* Create a clkdev entry for a given device/clk */ -void __init orion_clkdev_add(const char *con_id, const char *dev_id, - struct clk *clk) -{ - struct clk_lookup *cl; - - cl = clkdev_alloc(clk, con_id, dev_id); - if (cl) - clkdev_add(cl); -} - /* Create clkdev entries for all orion platforms except kirkwood. Kirkwood has gated clocks for some of its peripherals, so creates its own clkdev entries. For all the other orion devices, create clkdev entries to the tclk. */ void __init orion_clkdev_init(struct clk *tclk) { - orion_clkdev_add(NULL, "orion_spi.0", tclk); - orion_clkdev_add(NULL, "orion_spi.1", tclk); - orion_clkdev_add(NULL, MV643XX_ETH_NAME ".0", tclk); - orion_clkdev_add(NULL, MV643XX_ETH_NAME ".1", tclk); - orion_clkdev_add(NULL, MV643XX_ETH_NAME ".2", tclk); - orion_clkdev_add(NULL, MV643XX_ETH_NAME ".3", tclk); - orion_clkdev_add(NULL, "orion_wdt", tclk); - orion_clkdev_add(NULL, MV64XXX_I2C_CTLR_NAME ".0", tclk); + clkdev_create(tclk, NULL, "%s", "orion_spi.0"); + clkdev_create(tclk, NULL, "%s", "orion_spi.1"); + clkdev_create(tclk, NULL, "%s", MV643XX_ETH_NAME ".0"); + clkdev_create(tclk, NULL, "%s", MV643XX_ETH_NAME ".1"); + clkdev_create(tclk, NULL, "%s", MV643XX_ETH_NAME ".2"); + clkdev_create(tclk, NULL, "%s", MV643XX_ETH_NAME ".3"); + clkdev_create(tclk, NULL, "%s", "orion_wdt"); + clkdev_create(tclk, NULL, "%s", MV64XXX_I2C_CTLR_NAME ".0"); }
/* Fill in the resources structure and link it into the platform diff --git a/arch/arm/plat-orion/include/plat/common.h b/arch/arm/plat-orion/include/plat/common.h index d9a24f6..7a06b6b 100644 --- a/arch/arm/plat-orion/include/plat/common.h +++ b/arch/arm/plat-orion/include/plat/common.h @@ -106,8 +106,5 @@ void __init orion_crypto_init(unsigned long mapbase, unsigned long sram_size, unsigned long irq);
-void __init orion_clkdev_add(const char *con_id, const char *dev_id, - struct clk *clk); - void __init orion_clkdev_init(struct clk *tclk); #endif
Rather than open coding the clkdev allocation, initialisation and addition, use the clkdev_create() helper.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c b/arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c index 85e0b0c06718..b64d717bfab6 100644 --- a/arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c +++ b/arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c @@ -232,14 +232,12 @@ void omap2xxx_clkt_vps_init(void) struct clk_hw_omap *hw = NULL; struct clk *clk; const char *parent_name = "mpu_ck"; - struct clk_lookup *lookup = NULL;
omap2xxx_clkt_vps_late_init(); omap2xxx_clkt_vps_check_bootloader_rates();
hw = kzalloc(sizeof(*hw), GFP_KERNEL); - lookup = kzalloc(sizeof(*lookup), GFP_KERNEL); - if (!hw || !lookup) + if (!hw) goto cleanup; init.name = "virt_prcm_set"; init.ops = &virt_prcm_set_ops; @@ -249,15 +247,9 @@ void omap2xxx_clkt_vps_init(void) hw->hw.init = &init;
clk = clk_register(NULL, &hw->hw); - - lookup->dev_id = NULL; - lookup->con_id = "cpufreq_ck"; - lookup->clk = clk; - - clkdev_add(lookup); + clkdev_create(clk, "cpufreq_ck", NULL); return; cleanup: kfree(hw); - kfree(lookup); } #endif
When creating aliases of existing clkdev clocks, use clkdev_add_alias() isntead of open coding the lookup and clk_lookup creation.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/mach-omap2/omap_device.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c index be9541e18650..521c32e7778e 100644 --- a/arch/arm/mach-omap2/omap_device.c +++ b/arch/arm/mach-omap2/omap_device.c @@ -47,7 +47,7 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, const char *clk_name) { struct clk *r; - struct clk_lookup *l; + int rc;
if (!clk_alias || !clk_name) return; @@ -62,21 +62,15 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, return; }
- r = clk_get(NULL, clk_name); - if (IS_ERR(r)) { - dev_err(&od->pdev->dev, - "clk_get for %s failed\n", clk_name); - return; + rc = clk_add_alias(clk_alias, dev_name(&od->pdev->dev), clk_name, NULL); + if (rc) { + if (rc == -ENODEV || rc == -ENOMEM) + dev_err(&od->pdev->dev, + "clkdev_alloc for %s failed\n", clk_alias); + else + dev_err(&od->pdev->dev, + "clk_get for %s failed\n", clk_name); } - - l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev)); - if (!l) { - dev_err(&od->pdev->dev, - "clkdev_alloc for %s failed\n", clk_alias); - return; - } - - clkdev_add(l); }
/**
* Russell King rmk+kernel@arm.linux.org.uk [150302 09:10]:
When creating aliases of existing clkdev clocks, use clkdev_add_alias() isntead of open coding the lookup and clk_lookup creation.
Gave this series a quick try but I get these build errors:
arch/arm/mach-omap2/omap_device.c: In function ‘_add_clkdev’: arch/arm/mach-omap2/omap_device.c:65:58: warning: passing argument 3 of ‘clk_add_alias’ discards ‘const’ qualifier from pointer target type rc = clk_add_alias(clk_alias, dev_name(&od->pdev->dev), clk_name, NULL); ^ In file included from arch/arm/mach-omap2/omap_device.c:34:0: include/linux/clkdev.h:44:5: note: expected ‘char *’ but argument is of type ‘const char *’ int clk_add_alias(const char *, const char *, char *, struct device *); ^ drivers/clk/clkdev.c:298:16: error: expected declaration specifiers or ‘...’ before ‘(’ token vclkdev_create((struct clk *clk, const char *con_id, const char *dev_fmt, ^ drivers/clk/clkdev.c:322:92: error: storage class specified for parameter ‘__crc_clkdev_alloc’ EXPORT_SYMBOL(clkdev_alloc); ^ drivers/clk/clkdev.c:322:1: warning: ‘weak’ attribute ignored [-Wattributes] EXPORT_SYMBOL(clkdev_alloc); ^ drivers/clk/clkdev.c:322:1: warning: ‘externally_visible’ attribute ignored [-Wattributes] drivers/clk/clkdev.c:322:161: error: storage class specified for parameter ‘__kcrctab_clkdev_alloc’ EXPORT_SYMBOL(clkdev_alloc); ^ drivers/clk/clkdev.c:322:1: warning: ‘__used__’ attribute ignored [-Wattributes] EXPORT_SYMBOL(clkdev_alloc); ^ drivers/clk/clkdev.c:322:161: error: section attribute not allowed for ‘__kcrctab_clkdev_alloc’ EXPORT_SYMBOL(clkdev_alloc); ^ drivers/clk/clkdev.c:322:279: error: expected ‘;’, ‘,’ or ‘)’ before ‘=’ token EXPORT_SYMBOL(clkdev_alloc);
drivers/clk/clkdev.c:274:1: warning: ‘vclkdev_alloc’ defined but not used [-Wunused-function] vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt,
Regards,
Tony
On 03/02/15 09:05, Russell King - ARM Linux wrote:
Here's some initial clkdev cleanups. These are targetted for the next merge window, and while the initial patches can be merged independently, I'd prefer to keep the series together as further work on solving the problems which unique struct clk's has introduced is needed.
The initial cleanups are more about using the correct clkdev function than anything else: there's no point interating over a array of clk_lookup structs, adding each one in turn when we have had a function which does this since forever.
The clkdev_add_table() conversions look good.
I'm also killing a chunk of seemingly unused code in the omap3isp driver.
Lastly, I'm introducing a clkdev_create() helper, which combines the clkdev_alloc() + clkdev_add() pattern which keeps cropping up.
We already have a solution to that problem with clk_register_clkdev(). Andy has done some work to make clk_register_clkdev() return a struct clk_lookup pointer[1]. Maybe we can do that instead of introducing a new clkdev_create() function. There is some benefit to having a new function though so that we can avoid a flag day, although it looks like the flag day is small in this case so it might not actually matter.
[1] https://www.marc.info/?l=linux-kernel&m=142469226512289
On Mon, 2015-03-02 at 13:50 -0800, Stephen Boyd wrote:
On 03/02/15 09:05, Russell King - ARM Linux wrote:
Here's some initial clkdev cleanups. These are targetted for the next merge window, and while the initial patches can be merged independently, I'd prefer to keep the series together as further work on solving the problems which unique struct clk's has introduced is needed.
I'm also killing a chunk of seemingly unused code in the omap3isp driver.
Lastly, I'm introducing a clkdev_create() helper, which combines the clkdev_alloc() + clkdev_add() pattern which keeps cropping up.
We already have a solution to that problem with clk_register_clkdev(). Andy has done some work to make clk_register_clkdev() return a struct clk_lookup pointer[1]. Maybe we can do that instead of introducing a new clkdev_create() function. There is some benefit to having a new function though so that we can avoid a flag day, although it looks like the flag day is small in this case so it might not actually matter.
Agree with Stephen, why should we have the second function doing the same? Just name changing?
I think you may just incorporate that patch into your series.
participants (10)
-
Andy Shevchenko
-
Geert Uytterhoeven
-
Laurent Pinchart
-
Mark Brown
-
Russell King
-
Russell King - ARM Linux
-
Sakari Ailus
-
Stephen Boyd
-
Thomas Petazzoni
-
Tony Lindgren