[PATCH 00/10] initial clkdev cleanups

[PATCH 00/10] initial clkdev cleanups

[Russell King - ARM Linux]

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(-)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 01/10] media: omap3isp: remove unused clkdev

[Russell King]

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);
 };
-- 
1.8.3.1

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Laurent Pinchart]

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);
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Sakari Ailus]

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:

<URL:http://vihersipuli.retiisi.org.uk/cgi-bin/gitweb.cgi?p=~sailus/linux.git;a=shortlog;h=refs/heads/rm696-051-upstream>

I haven't sent that to linux-media yet but I'll do that during the coming
couple of days.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Russell King - ARM Linux]

(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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Laurent Pinchart]

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.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Sakari Ailus]

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.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH 01/10] media: omap3isp: remove unused clkdev

[Sakari Ailus]

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.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH 02/10] SH: use clkdev_add_table()

[Russell King]

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),
-- 
1.8.3.1

Re: [PATCH 02/10] SH: use clkdev_add_table()

[Geert Uytterhoeven]

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

[PATCH 03/10] clk: versatile: convert Integrator IM/PD-1 to use clkdev_add_table()

[Russell King]

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);
 
-- 
1.8.3.1

[PATCH 04/10] ARM: lpc32xx: convert to use clkdev_add_table()

[Russell King]

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
-- 
1.8.3.1

[PATCH 05/10] clkdev: add clkdev_create() helper

[Russell King]

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 *);
 
-- 
1.8.3.1

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Geert Uytterhoeven]

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

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Geert Uytterhoeven]

On Mon, Mar 2, 2015 at 6:46 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> 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.

As expected: no warning.
Verified with gcc 4.1.2 and 4.8.2.

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

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Stephen Boyd]

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?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Russell King - ARM Linux]

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 ;)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 05/10] clkdev: add clkdev_create() helper

[Stephen Boyd]

On 03/02/15 13:01, Russell King - ARM Linux wrote:
> 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 ;)
>

Ok, fair enough. Right now clk_add_alias() leaks if a driver decides to
add an alias and then fail probe for something like probe deferral. We
should probably make that return the clk_lookup structure too so that
drivers can clean up.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 06/10] ASOC: migor: use clkdev_create()

[Russell King]

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);
-- 
1.8.3.1

Re: [PATCH 06/10] ASOC: migor: use clkdev_create()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

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>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 07/10] clk: s2mps11: use clkdev_create()

[Russell King]

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++) {
-- 
1.8.3.1

[PATCH 08/10] ARM: orion: use clkdev_create()

[Russell King]

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.
-- 
1.8.3.1

Re: [PATCH 08/10] ARM: orion: use clkdev_create()

[Thomas Petazzoni]

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



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 09/10] ARM: omap2: use clkdev_create()

[Russell King]

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
-- 
1.8.3.1

[PATCH 10/10] ARM: omap2: use clkdev_add_alias()

[Russell King]

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);
 }
 
 /**
-- 
1.8.3.1

Re: [PATCH 10/10] ARM: omap2: use clkdev_add_alias()

[Tony Lindgren]

* 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 00/10] initial clkdev cleanups

[Stephen Boyd]

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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 00/10] initial clkdev cleanups

[Andy Shevchenko]

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.

> [1] https://www.marc.info/?l=linux-kernel&m=142469226512289

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.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy