alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] initial clkdev cleanups
@ 2015-03-02 17:05 Russell King - ARM Linux
  2015-03-02 17:06 ` [PATCH 01/10] media: omap3isp: remove unused clkdev Russell King
                   ` (10 more replies)
  0 siblings, 11 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2015-03-02 17:05 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Andrew Lunn, Jaroslav Kysela, Jason Cooper, Laurent Pinchart,
	Liam Girdwood, Mark Brown, Mauro Carvalho Chehab, Mike Turquette,
	Roland Stigge, Sebastian Hesselbarth, Stephen Boyd, Takashi Iwai,
	Tony Lindgren

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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 22:33   ` Laurent Pinchart
  2015-03-02 17:06 ` [PATCH 02/10] SH: use clkdev_add_table() Russell King
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Laurent Pinchart, Mauro Carvalho Chehab

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/10] SH: use clkdev_add_table()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
  2015-03-02 17:06 ` [PATCH 01/10] media: omap3isp: remove unused clkdev Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:18   ` Geert Uytterhoeven
  2015-03-02 17:06 ` [PATCH 03/10] clk: versatile: convert Integrator IM/PD-1 to " Russell King
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/10] clk: versatile: convert Integrator IM/PD-1 to use clkdev_add_table()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
  2015-03-02 17:06 ` [PATCH 01/10] media: omap3isp: remove unused clkdev Russell King
  2015-03-02 17:06 ` [PATCH 02/10] SH: use clkdev_add_table() Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:06 ` [PATCH 04/10] ARM: lpc32xx: convert " Russell King
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Mike Turquette, Stephen Boyd

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/10] ARM: lpc32xx: convert to use clkdev_add_table()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 03/10] clk: versatile: convert Integrator IM/PD-1 to " Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:06 ` [PATCH 05/10] clkdev: add clkdev_create() helper Russell King
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Roland Stigge

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 04/10] ARM: lpc32xx: convert " Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:22   ` Geert Uytterhoeven
  2015-03-02 19:07   ` Stephen Boyd
  2015-03-02 17:06 ` [PATCH 06/10] ASOC: migor: use clkdev_create() Russell King
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/10] ASOC: migor: use clkdev_create()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (4 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 05/10] clkdev: add clkdev_create() helper Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:29   ` Mark Brown
  2015-03-02 17:06 ` [PATCH 07/10] clk: s2mps11: " Russell King
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/10] clk: s2mps11: use clkdev_create()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (5 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 06/10] ASOC: migor: use clkdev_create() Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:06 ` [PATCH 08/10] ARM: orion: " Russell King
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Mike Turquette, Stephen Boyd

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/10] ARM: orion: use clkdev_create()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (6 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 07/10] clk: s2mps11: " Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-03 10:25   ` Thomas Petazzoni
  2015-03-02 17:06 ` [PATCH 09/10] ARM: omap2: " Russell King
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/10] ARM: omap2: use clkdev_create()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (7 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 08/10] ARM: orion: " Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-02 17:06 ` [PATCH 10/10] ARM: omap2: use clkdev_add_alias() Russell King
  2015-03-02 21:50 ` [PATCH 00/10] initial clkdev cleanups Stephen Boyd
  10 siblings, 0 replies; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Tony Lindgren

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/10] ARM: omap2: use clkdev_add_alias()
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (8 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 09/10] ARM: omap2: " Russell King
@ 2015-03-02 17:06 ` Russell King
  2015-03-03  0:13   ` Tony Lindgren
  2015-03-02 21:50 ` [PATCH 00/10] initial clkdev cleanups Stephen Boyd
  10 siblings, 1 reply; 29+ messages in thread
From: Russell King @ 2015-03-02 17:06 UTC (permalink / raw)
  To: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh
  Cc: Tony Lindgren

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/10] SH: use clkdev_add_table()
  2015-03-02 17:06 ` [PATCH 02/10] SH: use clkdev_add_table() Russell King
@ 2015-03-02 17:18   ` Geert Uytterhoeven
  0 siblings, 0 replies; 29+ messages in thread
From: Geert Uytterhoeven @ 2015-03-02 17:18 UTC (permalink / raw)
  To: Russell King
  Cc: ALSA Development Mailing List, linux-arm-kernel,
	Linux Media Mailing List, linux-omap, Linux-sh list

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 17:06 ` [PATCH 05/10] clkdev: add clkdev_create() helper Russell King
@ 2015-03-02 17:22   ` Geert Uytterhoeven
  2015-03-02 17:46     ` Russell King - ARM Linux
  2015-03-02 19:07   ` Stephen Boyd
  1 sibling, 1 reply; 29+ messages in thread
From: Geert Uytterhoeven @ 2015-03-02 17:22 UTC (permalink / raw)
  To: Russell King
  Cc: ALSA Development Mailing List, linux-arm-kernel,
	Linux Media Mailing List, linux-omap, Linux-sh list

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 06/10] ASOC: migor: use clkdev_create()
  2015-03-02 17:06 ` [PATCH 06/10] ASOC: migor: use clkdev_create() Russell King
@ 2015-03-02 17:29   ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2015-03-02 17:29 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh,
	Liam Girdwood, Jaroslav Kysela, Takashi Iwai

[-- 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 --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 17:22   ` Geert Uytterhoeven
@ 2015-03-02 17:46     ` Russell King - ARM Linux
  2015-03-02 20:36       ` Geert Uytterhoeven
  0 siblings, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2015-03-02 17:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: ALSA Development Mailing List, linux-arm-kernel,
	Linux Media Mailing List, linux-omap, Linux-sh list

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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 17:06 ` [PATCH 05/10] clkdev: add clkdev_create() helper Russell King
  2015-03-02 17:22   ` Geert Uytterhoeven
@ 2015-03-02 19:07   ` Stephen Boyd
  2015-03-02 21:01     ` Russell King - ARM Linux
  1 sibling, 1 reply; 29+ messages in thread
From: Stephen Boyd @ 2015-03-02 19:07 UTC (permalink / raw)
  To: Russell King, alsa-devel, linux-arm-kernel, linux-media,
	linux-omap, linux-sh

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 17:46     ` Russell King - ARM Linux
@ 2015-03-02 20:36       ` Geert Uytterhoeven
  0 siblings, 0 replies; 29+ messages in thread
From: Geert Uytterhoeven @ 2015-03-02 20:36 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: ALSA Development Mailing List, linux-arm-kernel,
	Linux Media Mailing List, linux-omap, Linux-sh list

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 19:07   ` Stephen Boyd
@ 2015-03-02 21:01     ` Russell King - ARM Linux
  2015-03-02 21:54       ` Stephen Boyd
  0 siblings, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2015-03-02 21:01 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh

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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 00/10] initial clkdev cleanups
  2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
                   ` (9 preceding siblings ...)
  2015-03-02 17:06 ` [PATCH 10/10] ARM: omap2: use clkdev_add_alias() Russell King
@ 2015-03-02 21:50 ` Stephen Boyd
  2015-03-03 15:56   ` Andy Shevchenko
  10 siblings, 1 reply; 29+ messages in thread
From: Stephen Boyd @ 2015-03-02 21:50 UTC (permalink / raw)
  To: Russell King - ARM Linux, alsa-devel, linux-arm-kernel,
	linux-media, linux-omap, linux-sh, Andy Shevchenko
  Cc: Roland Stigge, Andrew Lunn, Mike Turquette, Jason Cooper,
	Mauro Carvalho Chehab, Takashi Iwai, Liam Girdwood,
	Jaroslav Kysela, Tony Lindgren, Mark Brown, Laurent Pinchart,
	Sebastian Hesselbarth

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 05/10] clkdev: add clkdev_create() helper
  2015-03-02 21:01     ` Russell King - ARM Linux
@ 2015-03-02 21:54       ` Stephen Boyd
  0 siblings, 0 replies; 29+ messages in thread
From: Stephen Boyd @ 2015-03-02 21:54 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 17:06 ` [PATCH 01/10] media: omap3isp: remove unused clkdev Russell King
@ 2015-03-02 22:33   ` Laurent Pinchart
  2015-03-02 22:53     ` Sakari Ailus
  0 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2015-03-02 22:33 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh,
	Mauro Carvalho Chehab, Sakari Ailus

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 22:33   ` Laurent Pinchart
@ 2015-03-02 22:53     ` Sakari Ailus
  2015-03-02 23:54       ` Russell King - ARM Linux
  0 siblings, 1 reply; 29+ messages in thread
From: Sakari Ailus @ 2015-03-02 22:53 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Russell King, alsa-devel, linux-arm-kernel, linux-media,
	linux-omap, linux-sh, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 22:53     ` Sakari Ailus
@ 2015-03-02 23:54       ` Russell King - ARM Linux
  2015-03-03  0:39         ` Laurent Pinchart
  2015-03-03 23:09         ` Sakari Ailus
  0 siblings, 2 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2015-03-02 23:54 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Laurent Pinchart, alsa-devel, linux-arm-kernel, linux-media,
	linux-omap, linux-sh, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 10/10] ARM: omap2: use clkdev_add_alias()
  2015-03-02 17:06 ` [PATCH 10/10] ARM: omap2: use clkdev_add_alias() Russell King
@ 2015-03-03  0:13   ` Tony Lindgren
  0 siblings, 0 replies; 29+ messages in thread
From: Tony Lindgren @ 2015-03-03  0:13 UTC (permalink / raw)
  To: Russell King
  Cc: linux-sh, alsa-devel, linux-omap, linux-arm-kernel, linux-media

* 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 23:54       ` Russell King - ARM Linux
@ 2015-03-03  0:39         ` Laurent Pinchart
  2015-03-03 22:09           ` Sakari Ailus
  2015-03-03 23:09         ` Sakari Ailus
  1 sibling, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2015-03-03  0:39 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Sakari Ailus, alsa-devel, linux-arm-kernel, linux-media,
	linux-omap, linux-sh, Mauro Carvalho Chehab

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/10] ARM: orion: use clkdev_create()
  2015-03-02 17:06 ` [PATCH 08/10] ARM: orion: " Russell King
@ 2015-03-03 10:25   ` Thomas Petazzoni
  0 siblings, 0 replies; 29+ messages in thread
From: Thomas Petazzoni @ 2015-03-03 10:25 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, linux-arm-kernel, linux-media, linux-omap, linux-sh,
	Andrew Lunn, Jason Cooper, Sebastian Hesselbarth

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 00/10] initial clkdev cleanups
  2015-03-02 21:50 ` [PATCH 00/10] initial clkdev cleanups Stephen Boyd
@ 2015-03-03 15:56   ` Andy Shevchenko
  0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2015-03-03 15:56 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Russell King - ARM Linux, alsa-devel, linux-arm-kernel,
	linux-media, linux-omap, linux-sh, Roland Stigge, Andrew Lunn,
	Mike Turquette, Jason Cooper, Mauro Carvalho Chehab,
	Takashi Iwai, Liam Girdwood, Jaroslav Kysela, Tony Lindgren,
	Mark Brown, Laurent Pinchart, Sebastian Hesselbarth

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-03  0:39         ` Laurent Pinchart
@ 2015-03-03 22:09           ` Sakari Ailus
  0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-03-03 22:09 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Russell King - ARM Linux, alsa-devel, linux-arm-kernel,
	linux-media, linux-omap, linux-sh, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/10] media: omap3isp: remove unused clkdev
  2015-03-02 23:54       ` Russell King - ARM Linux
  2015-03-03  0:39         ` Laurent Pinchart
@ 2015-03-03 23:09         ` Sakari Ailus
  1 sibling, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2015-03-03 23:09 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Laurent Pinchart, alsa-devel, linux-arm-kernel, linux-media,
	linux-omap, linux-sh, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2015-03-03 23:09 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-02 17:05 [PATCH 00/10] initial clkdev cleanups Russell King - ARM Linux
2015-03-02 17:06 ` [PATCH 01/10] media: omap3isp: remove unused clkdev Russell King
2015-03-02 22:33   ` Laurent Pinchart
2015-03-02 22:53     ` Sakari Ailus
2015-03-02 23:54       ` Russell King - ARM Linux
2015-03-03  0:39         ` Laurent Pinchart
2015-03-03 22:09           ` Sakari Ailus
2015-03-03 23:09         ` Sakari Ailus
2015-03-02 17:06 ` [PATCH 02/10] SH: use clkdev_add_table() Russell King
2015-03-02 17:18   ` Geert Uytterhoeven
2015-03-02 17:06 ` [PATCH 03/10] clk: versatile: convert Integrator IM/PD-1 to " Russell King
2015-03-02 17:06 ` [PATCH 04/10] ARM: lpc32xx: convert " Russell King
2015-03-02 17:06 ` [PATCH 05/10] clkdev: add clkdev_create() helper Russell King
2015-03-02 17:22   ` Geert Uytterhoeven
2015-03-02 17:46     ` Russell King - ARM Linux
2015-03-02 20:36       ` Geert Uytterhoeven
2015-03-02 19:07   ` Stephen Boyd
2015-03-02 21:01     ` Russell King - ARM Linux
2015-03-02 21:54       ` Stephen Boyd
2015-03-02 17:06 ` [PATCH 06/10] ASOC: migor: use clkdev_create() Russell King
2015-03-02 17:29   ` Mark Brown
2015-03-02 17:06 ` [PATCH 07/10] clk: s2mps11: " Russell King
2015-03-02 17:06 ` [PATCH 08/10] ARM: orion: " Russell King
2015-03-03 10:25   ` Thomas Petazzoni
2015-03-02 17:06 ` [PATCH 09/10] ARM: omap2: " Russell King
2015-03-02 17:06 ` [PATCH 10/10] ARM: omap2: use clkdev_add_alias() Russell King
2015-03-03  0:13   ` Tony Lindgren
2015-03-02 21:50 ` [PATCH 00/10] initial clkdev cleanups Stephen Boyd
2015-03-03 15:56   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).