[alsa-devel] [PATCH 00/14] Fix fallout from per-user struct clk patches
Sorry for posting this soo close to the 4.1 merge window, I had completely forgotten about this chunk of work I did earlier this month.
The per-user struct clk patches rather badly broke clkdev and various other places. This was reported, but was forgotten about. Really, the per-user clk stuff should've been reverted, but we've lived with it far too long for that.
So, our only other option is to now rush these patches into 4.1 and hope for the best.
The series cleans up quite a number of places too...
arch/arm/mach-davinci/da850.c | 1 + arch/arm/mach-lpc32xx/clock.c | 5 +- arch/arm/mach-omap1/board-nokia770.c | 2 +- arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c | 12 +--- arch/arm/mach-omap2/omap_device.c | 24 +++----- arch/arm/mach-pxa/eseries.c | 1 + arch/arm/mach-pxa/lubbock.c | 1 + arch/arm/mach-pxa/tosa.c | 1 + 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 | 83 ++++++++++++++++++++-------- drivers/media/platform/omap3isp/isp.c | 18 ------ drivers/media/platform/omap3isp/isp.h | 1 - include/linux/clk.h | 27 ++++----- include/linux/clkdev.h | 6 +- include/media/omap3isp.h | 6 -- sound/soc/sh/migor.c | 3 +- 22 files changed, 108 insertions(+), 112 deletions(-)
The idea is that rate = clk_round_rate(clk, r) is equivalent to:
clk_set_rate(clk, r); rate = clk_get_rate(clk);
except that clk_round_rate() does not change the hardware in any way.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- include/linux/clk.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/linux/clk.h b/include/linux/clk.h index 8381bbfbc308..d1ac9f3ab24b 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -288,6 +288,20 @@ void devm_clk_put(struct device *dev, struct clk *clk); * @clk: clock source * @rate: desired clock rate in Hz * + * This answers the question "if I were to pass @rate to clk_set_rate(), + * what clock rate would I end up with?" without changing the hardware + * in any way. In other words: + * + * rate = clk_round_rate(clk, r); + * + * and: + * + * clk_set_rate(clk, r); + * rate = clk_get_rate(clk); + * + * are equivalent except the former does not modify the clock hardware + * in any way. + * * Returns rounded clock rate in Hz, or negative errno. */ long clk_round_rate(struct clk *clk, unsigned long rate);
We want to be able to call clkdev_add_table() from non-init code, so we need to drop the __init marker from it.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/clkdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 043fd3633373..156f2e2f972c 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -251,7 +251,7 @@ void clkdev_add(struct clk_lookup *cl) } EXPORT_SYMBOL(clkdev_add);
-void __init clkdev_add_table(struct clk_lookup *cl, size_t num) +void clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) {
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/mach-davinci/da850.c | 1 + arch/arm/mach-omap1/board-nokia770.c | 2 +- arch/arm/mach-pxa/eseries.c | 1 + arch/arm/mach-pxa/lubbock.c | 1 + arch/arm/mach-pxa/tosa.c | 1 + include/linux/clk.h | 13 ------------- 6 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c index 45ce065e7170..3b8740c083c4 100644 --- a/arch/arm/mach-davinci/da850.c +++ b/arch/arm/mach-davinci/da850.c @@ -11,6 +11,7 @@ * is licensed "as is" without any warranty of any kind, whether express * or implied. */ +#include <linux/clkdev.h> #include <linux/gpio.h> #include <linux/init.h> #include <linux/clk.h> diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c index 85089d821982..3bc59390a943 100644 --- a/arch/arm/mach-omap1/board-nokia770.c +++ b/arch/arm/mach-omap1/board-nokia770.c @@ -7,6 +7,7 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#include <linux/clkdev.h> #include <linux/irq.h> #include <linux/gpio.h> #include <linux/kernel.h> @@ -14,7 +15,6 @@ #include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/input.h> -#include <linux/clk.h> #include <linux/omapfb.h>
#include <linux/spi/spi.h> diff --git a/arch/arm/mach-pxa/eseries.c b/arch/arm/mach-pxa/eseries.c index cfb864173ce3..4427bf26ea47 100644 --- a/arch/arm/mach-pxa/eseries.c +++ b/arch/arm/mach-pxa/eseries.c @@ -10,6 +10,7 @@ * */
+#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/gpio.h> diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c index d8a1be619f21..235f2d9318c1 100644 --- a/arch/arm/mach-pxa/lubbock.c +++ b/arch/arm/mach-pxa/lubbock.c @@ -11,6 +11,7 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#include <linux/clkdev.h> #include <linux/gpio.h> #include <linux/module.h> #include <linux/kernel.h> diff --git a/arch/arm/mach-pxa/tosa.c b/arch/arm/mach-pxa/tosa.c index 7780d1faa06f..92e56d8a24d8 100644 --- a/arch/arm/mach-pxa/tosa.c +++ b/arch/arm/mach-pxa/tosa.c @@ -12,6 +12,7 @@ * */
+#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/platform_device.h> diff --git a/include/linux/clk.h b/include/linux/clk.h index d1ac9f3ab24b..c94a34a8a1b3 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -467,19 +467,6 @@ static inline void clk_disable_unprepare(struct clk *clk) clk_unprepare(clk); }
-/** - * clk_add_alias - add a new clock alias - * @alias: name for clock alias - * @alias_dev_name: device name - * @id: platform specific clock name - * @dev: device - * - * Allows using generic clock names for drivers by adding a new alias. - * Assumes clkdev, see clkdev.h for more info. - */ -int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, - struct device *dev); - struct device_node; struct of_phandle_args;
* Russell King rmk+kernel@arm.linux.org.uk [150403 10:13]:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Acked-by: Tony Lindgren tony@atomide.com
Russell King rmk+kernel@arm.linux.org.uk writes:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Tested-by: Robert Jarzmik robert.jarzmik@free.fr
Actually, this serie fixes a regression I've seen in linux-next, and which was triggering the Oops in [1] on lubbock. With your serie, the kernel boots fine.
Cheers.
-- Robert
[1] Oops without this serie on linux-next (20150213, commit b8acf73) [ 0.223403] Unable to handle kernel paging request at virtual address 32617878 [ 0.231047] pgd = c0004000 [ 0.233810] [32617878] *pgd=00000000 [ 0.237486] Internal error: Oops: f5 [#1] ARM [ 0.241881] Modules linked in: [ 0.245051] CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.19.0-next-20150213-09795-g3bd95ad-dirty #433 [ 0.255288] Hardware name: Intel DBPXA250 Development Platform (aka Lubbock) [ 0.262412] task: c3e46000 ti: c3e48000 task.ti: c3e48000 [ 0.267903] PC is at __clk_get_hw+0x8/0x10 [ 0.272150] LR is at clk_get_sys+0xd0/0x120 [ 0.276437] pc : [<c0249018>] lr : [<c0248f30>] psr: a0000053 [ 0.276437] sp : c3e49e10 ip : 00000000 fp : c3e67480 [ 0.288009] r10: 00000003 r9 : 00000001 r8 : c0807cc4 [ 0.293310] r7 : c3e755c0 r6 : c0390d0e r5 : 00000001 r4 : c3e67480 [ 0.299919] r3 : 32617870 r2 : 00000000 r1 : c0390d0e r0 : c3e75440 [ 0.306527] Flags: NzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel [ 0.313996] Control: 0000397f Table: a0004000 DAC: 00000017 [ 0.319809] Process swapper (pid: 1, stack limit = 0xc3e48190) [ 0.325710] Stack: (0xc3e49e10 to 0xc3e4a000) [ 0.330177] 9e00: c07f64c8 c3e75920 c07f64c8 c07f6a98 [ 0.338502] 9e20: c07f64b8 c07f64c8 00000131 c07f6aac 00000000 c00182e0 c03b6378 c3e79690 [ 0.346822] 9e40: c3e7afa0 00000001 00000000 ffffffed c07f64c8 c07f313c 00000000 c040a318 [ 0.355141] 9e60: 00000000 c041f738 00000000 c01e7a40 c07f64c8 c07f64fc c07f313c c01e6614 [ 0.363466] 9e80: 00000000 c07f64c8 c07f64fc c07f313c c01e676c c01e67d4 00000000 c3e49ea8 [ 0.371793] 9ea0: c07f313c c01e5030 c3e4534c c3e72870 c07f313c c07f313c c3e72ae0 00000000 [ 0.380119] 9ec0: c0802ff0 c01e5e74 c0390df8 c0390df8 00000070 c07f313c c07f1898 c07f1898 [ 0.388440] 9ee0: c080ce60 c01e6df8 00000000 00000000 c07f1898 c040a338 c3e75800 c000882c [ 0.396761] 9f00: c3ffcaaa c03bb7ee 00000000 c03e2ab0 00000000 00000000 c03e2ac0 c3e49f30 [ 0.405082] 9f20: 00000086 c3ffcb56 c3ffcb56 c0030450 60000053 c03e2b70 c03e27e0 00000086 [ 0.413396] 9f40: 00000004 00000004 00000000 00000004 00000004 c041f724 c0423ec0 c080ce60 [ 0.421719] 9f60: 00000086 c0404588 c041f738 c0404d3c 00000004 00000004 c0404588 c3e46000 [ 0.430028] 9f80: c001cb88 c3e03c14 00000000 c02f7d20 00000000 00000000 00000000 00000000 [ 0.438341] 9fa0: 00000000 c02f7d28 00000000 c000e718 00000000 00000000 00000000 00000000 [ 0.446642] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.454950] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 0.463318] [<c0249018>] (__clk_get_hw) from [<c0248f30>] (clk_get_sys+0xd0/0x120) [ 0.471122] [<c0248f30>] (clk_get_sys) from [<c00182e0>] (sa1111_probe+0x68/0x4c4) [ 0.478902] [<c00182e0>] (sa1111_probe) from [<c01e7a40>] (platform_drv_probe+0x30/0x78) [ 0.487155] [<c01e7a40>] (platform_drv_probe) from [<c01e6614>] (driver_probe_device+0xac/0x204) [ 0.496089] [<c01e6614>] (driver_probe_device) from [<c01e67d4>] (__driver_attach+0x68/0x8c) [ 0.504669] [<c01e67d4>] (__driver_attach) from [<c01e5030>] (bus_for_each_dev+0x50/0x88) [ 0.512986] [<c01e5030>] (bus_for_each_dev) from [<c01e5e74>] (bus_add_driver+0xc8/0x1c8) [ 0.521311] [<c01e5e74>] (bus_add_driver) from [<c01e6df8>] (driver_register+0x9c/0xe0) [ 0.529499] [<c01e6df8>] (driver_register) from [<c040a338>] (sa1111_init+0x20/0x30) [ 0.537393] [<c040a338>] (sa1111_init) from [<c000882c>] (do_one_initcall+0xf8/0x1cc) [ 0.545362] [<c000882c>] (do_one_initcall) from [<c0404d3c>] (kernel_init_freeable+0xe8/0x1b0) [ 0.554171] [<c0404d3c>] (kernel_init_freeable) from [<c02f7d28>] (kernel_init+0x8/0xe4) [ 0.562444] [<c02f7d28>] (kernel_init) from [<c000e718>] (ret_from_fork+0x14/0x3c) [ 0.570149] Code: 15930000 e12fff1e e3500000 15903000 (15930008) [ 0.576450] ---[ end trace cb88537fdc8fa201 ]--- [ 0.581232] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
On Sat, Apr 04, 2015 at 02:43:22PM +0200, Robert Jarzmik wrote:
Russell King rmk+kernel@arm.linux.org.uk writes:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Tested-by: Robert Jarzmik robert.jarzmik@free.fr
Actually, this serie fixes a regression I've seen in linux-next, and which was triggering the Oops in [1] on lubbock. With your serie, the kernel boots fine.
Yes, this series does fix a regression - I mentioned that the per-user clk patches broke quite a bit of this code in the cover to the series.
On 04/04/15 05:43, Robert Jarzmik wrote:
Russell King rmk+kernel@arm.linux.org.uk writes:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Tested-by: Robert Jarzmik robert.jarzmik@free.fr
Actually, this serie fixes a regression I've seen in linux-next, and which was triggering the Oops in [1] on lubbock. With your serie, the kernel boots fine.
Is this with the lubbock_defconfig? Is it a regression in 4.0-rc series or is it due to some pending -next patches interacting with the per-user clock patches? It looks like the latter because __clk_get_hw() should be inlined on lubbock_defconfig where CONFIG_COMMON_CLK=n.
Stephen Boyd sboyd@codeaurora.org writes:
On 04/04/15 05:43, Robert Jarzmik wrote:
Russell King rmk+kernel@arm.linux.org.uk writes:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Tested-by: Robert Jarzmik robert.jarzmik@free.fr
Actually, this serie fixes a regression I've seen in linux-next, and which was triggering the Oops in [1] on lubbock. With your serie, the kernel boots fine.
Is this with the lubbock_defconfig? Is it a regression in 4.0-rc series or is it due to some pending -next patches interacting with the per-user clock patches? It looks like the latter because __clk_get_hw() should be inlined on lubbock_defconfig where CONFIG_COMMON_CLK=n.
It's upon linux-next and the "not yet pulled" pxa tree merged : - http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/331119.html I think it will appear once arm-soc has been prepared for linux-next and pulled.
This specific merge window will shift pxa architecture to common clock framework, that's why I saw the regression.
Cheers.
On Mon, Apr 06, 2015 at 12:04:21PM -0700, Stephen Boyd wrote:
On 04/04/15 05:43, Robert Jarzmik wrote:
Russell King rmk+kernel@arm.linux.org.uk writes:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Tested-by: Robert Jarzmik robert.jarzmik@free.fr
Actually, this serie fixes a regression I've seen in linux-next, and which was triggering the Oops in [1] on lubbock. With your serie, the kernel boots fine.
Is this with the lubbock_defconfig? Is it a regression in 4.0-rc series or is it due to some pending -next patches interacting with the per-user clock patches? It looks like the latter because __clk_get_hw() should be inlined on lubbock_defconfig where CONFIG_COMMON_CLK=n.
It's a regression with anything that uses clk_add_alias() or which open codes the aliasing of clocks, or registration of clocks into clkdev. It's quite nasty.
The problem is that the way you updated clkdev, by adding __clk_get_hw() at clk_get() time, the struct clk which we're passing in there could well have been already clk_put()'d, and therefore freed, and no longer valid.
It's a regression ever since the per-user clk patches went in, caused _entirely_ by those patches.
On Friday 03 April 2015 10:42 PM, Russell King wrote:
clk_add_alias() is provided by clkdev, and is not part of the clk API. Howver, it is prototyped in two locations: linux/clkdev.h and linux/clk.h. This is a mess. Get rid of the redundant and unnecessary version in linux/clk.h.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
arch/arm/mach-davinci/da850.c | 1 +
For the DaVinci change:
Acked-by: Sekhar Nori nsekhar@ti.com
Thanks, Sekhar
The connection id is only passed to clk_get() which is already const. Const-ify this argument too.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/clkdev.c | 6 +++--- include/linux/clkdev.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 156f2e2f972c..5d7746d19445 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -308,10 +308,10 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) } EXPORT_SYMBOL(clkdev_alloc);
-int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, - struct device *dev) +int clk_add_alias(const char *alias, const char *alias_dev_name, + const char *con_id, struct device *dev) { - struct clk *r = clk_get(dev, id); + struct clk *r = clk_get(dev, con_id); struct clk_lookup *l;
if (IS_ERR(r)) diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h index 94bad77eeb4a..eb9b38a79b43 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -38,7 +38,7 @@ void clkdev_add(struct clk_lookup *cl); void clkdev_drop(struct clk_lookup *cl);
void clkdev_add_table(struct clk_lookup *, size_t); -int clk_add_alias(const char *, const char *, char *, struct device *); +int clk_add_alias(const char *, const char *, const char *, struct device *);
int clk_register_clkdev(struct clk *, const char *, const char *, ...); int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t);
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/clk/clkdev.c | 24 ++++++++++++++++-------- include/linux/clkdev.h | 1 + 2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 5d7746d19445..8e676eafc823 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -205,7 +205,7 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id) if (!cl) goto out;
- clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id); + clk = __clk_create_clk(cl->clk_hw, dev_id, con_id); if (IS_ERR(clk)) goto out;
@@ -243,18 +243,26 @@ void clk_put(struct clk *clk) } EXPORT_SYMBOL(clk_put);
-void clkdev_add(struct clk_lookup *cl) +static void __clkdev_add(struct clk_lookup *cl) { mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); } + +void clkdev_add(struct clk_lookup *cl) +{ + if (!cl->clk_hw) + cl->clk_hw = __clk_get_hw(cl->clk); + __clkdev_add(cl); +} EXPORT_SYMBOL(clkdev_add);
void clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) { + cl->clk_hw = __clk_get_hw(cl->clk); list_add_tail(&cl->node, &clocks); cl++; } @@ -271,7 +279,7 @@ struct clk_lookup_alloc { };
static struct clk_lookup * __init_refok -vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, +vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, va_list ap) { struct clk_lookup_alloc *cla; @@ -280,7 +288,7 @@ vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, if (!cla) return NULL;
- cla->cl.clk = clk; + cla->cl.clk_hw = hw; if (con_id) { strlcpy(cla->con_id, con_id, sizeof(cla->con_id)); cla->cl.con_id = cla->con_id; @@ -301,7 +309,7 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) va_list ap;
va_start(ap, dev_fmt); - cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); + cl = vclkdev_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap); va_end(ap);
return cl; @@ -362,7 +370,7 @@ 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_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap); va_end(ap);
if (!cl) @@ -393,8 +401,8 @@ int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num) return PTR_ERR(clk);
for (i = 0; i < num; i++, cl++) { - cl->clk = clk; - clkdev_add(cl); + cl->clk_hw = __clk_get_hw(clk); + __clkdev_add(cl); }
return 0; diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h index eb9b38a79b43..cd93b215e3af 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -22,6 +22,7 @@ struct clk_lookup { const char *dev_id; const char *con_id; struct clk *clk; + struct clk_hw *clk_hw; };
#define CLKDEV_INIT(d, n, c) \
On 04/03/15 10:12, Russell King wrote:
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Some commit text would be nice for us 5 years from now when we wonder why this patch was applied.
I suspect the commit text would go like this:
clk_add_alias() calls clk_get() followed by clk_put() but in between those two calls it saves away the struct clk pointer to a clk_lookup structure. This leaves the 'clk' member of the clk_lookup pointing at freed memory on configurations where CONFIG_COMMON_CLK=y. This is a problem because clk_get_sys() will eventually try to dereference the freed pointer by calling __clk_get_hw() on it. Fix this by saving away the struct clk_hw pointer instead of the struct clk pointer so that when we try to create a per-user struct clk in clk_get_sys() we don't dereference a junk pointer.
Now the question is does any of this matter for the 4.0 release. From what I can tell, the answer is no.
$ git grep 'clk_add_alias' v4.0-rc6 v4.0-rc6:arch/arm/mach-davinci/da850.c: clk_add_alias("async", da850_cpufreq_device.name, v4.0-rc6:arch/arm/mach-omap1/board-nokia770.c: clk_add_alias("hwa_sys_ck", NULL, "bclk", NULL); v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK48M", e740_t7l66xb_device.name, v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK3P6MI", e750_tc6393xb_device.name, v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK3P6MI", e800_tc6393xb_device.name, v4.0-rc6:arch/arm/mach-pxa/lubbock.c: clk_add_alias("SA1111_CLK", NULL, "GPIO11_CLK", NULL); v4.0-rc6:arch/arm/mach-pxa/tosa.c: clk_add_alias("CLK_CK3P6MI", tc6393xb_device.name, "GPIO11_CLK", NULL); v4.0-rc6:arch/mips/alchemy/common/clock.c: clk_add_alias(t->alias, NULL, t->base, NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ref", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ref", NULL); v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu-sh3.0", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.0", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.1", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.2", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-mtu2", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-cmt-16.0", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-cmt-32.0", "peripheral_clk", NULL); v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("sci_ick", NULL, "peripheral_clk", NULL);
All of these architectures and platforms have CONFIG_COMMON_CLK=n, so there doesn't seem to be any regression that these patches are fixing. That isn't to say the patches are bad, just that they aren't urgent for the upcoming release.
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 | 53 ++++++++++++++++++++++++++++++++++++++------------ include/linux/clkdev.h | 3 +++ 2 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 8e676eafc823..6d992d8b7c47 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -302,6 +302,19 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, return &cla->cl; }
+static struct clk_lookup * +vclkdev_create(struct clk_hw *hw, const char *con_id, const char *dev_fmt, + va_list ap) +{ + struct clk_lookup *cl; + + cl = vclkdev_alloc(hw, 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, ...) { @@ -316,6 +329,29 @@ 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_get_hw(clk), con_id, dev_fmt, ap); + va_end(ap); + + return cl; +} +EXPORT_SYMBOL_GPL(clkdev_create); + int clk_add_alias(const char *alias, const char *alias_dev_name, const char *con_id, struct device *dev) { @@ -325,12 +361,10 @@ int clk_add_alias(const char *alias, const char *alias_dev_name, if (IS_ERR(r)) return PTR_ERR(r);
- l = clkdev_alloc(r, alias, alias_dev_name); + l = clkdev_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);
@@ -370,15 +404,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_get_hw(clk), con_id, dev_fmt, ap); + cl = vclkdev_create(__clk_get_hw(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 cd93b215e3af..a240b18e86fa 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -38,6 +38,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 *, const char *, struct device *);
On 04/03/15 10:12, Russell King wrote:
@@ -316,6 +329,29 @@ 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.
Please add that this returns NULL on failure.
On Mon, Apr 06, 2015 at 01:19:33PM -0700, Stephen Boyd wrote:
On 04/03/15 10:12, Russell King wrote:
@@ -316,6 +329,29 @@ 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.
Please add that this returns NULL on failure.
Will do, but please remember that _I'm_ taking the clkdev patches through my tree, as I'm the maintainer for clkdev. Thanks.
On 04/07/15 05:43, Russell King - ARM Linux wrote:
On Mon, Apr 06, 2015 at 01:19:33PM -0700, Stephen Boyd wrote:
On 04/03/15 10:12, Russell King wrote:
@@ -316,6 +329,29 @@ 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.
Please add that this returns NULL on failure.
Will do, but please remember that _I'm_ taking the clkdev patches through my tree, as I'm the maintainer for clkdev. Thanks.
Sounds good to me. Thanks.
No merged platform supplies xclks via platform data. As we want to slightly change the clkdev interface, rather than fixing this unused code, remove it instead.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/media/platform/omap3isp/isp.c | 18 ------------------ drivers/media/platform/omap3isp/isp.h | 1 - include/media/omap3isp.h | 6 ------ 3 files changed, 25 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index deca80903c3a..4d8078b9d010 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -281,7 +281,6 @@ static const struct clk_init_data isp_xclk_init_data = {
static int isp_xclk_init(struct isp_device *isp) { - struct isp_platform_data *pdata = isp->pdata; struct clk_init_data init; unsigned int i;
@@ -311,20 +310,6 @@ static int isp_xclk_init(struct isp_device *isp) xclk->clk = clk_register(NULL, &xclk->hw); if (IS_ERR(xclk->clk)) return PTR_ERR(xclk->clk); - - if (pdata->xclks[i].con_id == NULL && - pdata->xclks[i].dev_id == NULL) - continue; - - xclk->lookup = kzalloc(sizeof(*xclk->lookup), GFP_KERNEL); - if (xclk->lookup == NULL) - return -ENOMEM; - - xclk->lookup->con_id = pdata->xclks[i].con_id; - xclk->lookup->dev_id = pdata->xclks[i].dev_id; - xclk->lookup->clk = xclk->clk; - - clkdev_add(xclk->lookup); }
return 0; @@ -339,9 +324,6 @@ static void isp_xclk_cleanup(struct isp_device *isp)
if (!IS_ERR(xclk->clk)) clk_unregister(xclk->clk); - - if (xclk->lookup) - clkdev_drop(xclk->lookup); } }
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index cfdfc8714b6b..d41c98bbdfe7 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -122,7 +122,6 @@ enum isp_xclk_id { struct isp_xclk { struct isp_device *isp; struct clk_hw hw; - struct clk_lookup *lookup; struct clk *clk; enum isp_xclk_id id;
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h index 398279dd1922..a9798525d01e 100644 --- a/include/media/omap3isp.h +++ b/include/media/omap3isp.h @@ -152,13 +152,7 @@ struct isp_v4l2_subdevs_group { } bus; /* gcc < 4.6.0 chokes on anonymous union initializers */ };
-struct isp_platform_xclk { - const char *dev_id; - const char *con_id; -}; - struct isp_platform_data { - struct isp_platform_xclk xclks[2]; struct isp_v4l2_subdevs_group *subdevs; void (*set_constraints)(struct isp_device *isp, bool enable); };
Hi Russell,
Thank you for the patch;
On Friday 03 April 2015 18:12:58 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
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with one caveat though : it conflicts with patches queued for v4.1 in the media tree. I'll post a rebased version in a reply to your e-mail. How would you like to handle the conflict ?
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);
};
On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote:
Hi Russell,
Thank you for the patch;
On Friday 03 April 2015 18:12:58 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
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with one caveat though : it conflicts with patches queued for v4.1 in the media tree. I'll post a rebased version in a reply to your e-mail. How would you like to handle the conflict ?
How bad is the conflict?
Hello Russell,
On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote:
On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote:
Hi Russell,
Thank you for the patch;
On Friday 03 April 2015 18:12:58 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
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with one caveat though : it conflicts with patches queued for v4.1 in the media tree. I'll post a rebased version in a reply to your e-mail. How would you like to handle the conflict ?
How bad is the conflict?
It's not too bad, it's mostly a context-related conflict. There are two additional lines to remove (plus the associated comment) from isp_xclk_init(), as your patch makes a loop now terminate with if (condition) continue;. Those two lines could be removed later, keeping them doesn't break anything.
On Tue, Apr 07, 2015 at 12:42:52PM +0300, Laurent Pinchart wrote:
Hello Russell,
On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote:
On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote:
Hi Russell,
Thank you for the patch;
On Friday 03 April 2015 18:12:58 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
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with one caveat though : it conflicts with patches queued for v4.1 in the media tree. I'll post a rebased version in a reply to your e-mail. How would you like to handle the conflict ?
How bad is the conflict?
It's not too bad, it's mostly a context-related conflict. There are two additional lines to remove (plus the associated comment) from isp_xclk_init(), as your patch makes a loop now terminate with if (condition) continue;. Those two lines could be removed later, keeping them doesn't break anything.
I think it's fine to take it through the media tree as the series doesn't have any dependencies on this patch. It was merely attempting to get rid of stuff so that we could move closer to clkdev dealing with a clk_hw rather than a struct clk - but I never made it that far with the series. Maybe at a later date... :)
Hi Russell,
On Tuesday 07 April 2015 13:45:36 Russell King - ARM Linux wrote:
On Tue, Apr 07, 2015 at 12:42:52PM +0300, Laurent Pinchart wrote:
On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote:
On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote:
Hi Russell,
Thank you for the patch;
On Friday 03 April 2015 18:12:58 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
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with one caveat though : it conflicts with patches queued for v4.1 in the media tree. I'll post a rebased version in a reply to your e-mail. How would you like to handle the conflict ?
How bad is the conflict?
It's not too bad, it's mostly a context-related conflict. There are two additional lines to remove (plus the associated comment) from isp_xclk_init(), as your patch makes a loop now terminate with if (condition) continue;. Those two lines could be removed later, keeping them doesn't break anything.
I think it's fine to take it through the media tree as the series doesn't have any dependencies on this patch. It was merely attempting to get rid of stuff so that we could move closer to clkdev dealing with a clk_hw rather than a struct clk - but I never made it that far with the series. Maybe at a later date... :)
:-) I'll take the patch and send a pull request.
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.
Acked-by: Geert Uytterhoeven geert+renesas@glider.be 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),
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
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.
On Fri, Apr 03, 2015 at 06:13:13PM +0100, 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
Acked-by: Andrew Lunn andrew@lunn.ch
Andrew
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
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Andrew, Russell,
On 04/04/2015 02:17, Andrew Lunn wrote:
On Fri, Apr 03, 2015 at 06:13:13PM +0100, 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
Acked-by: Andrew Lunn andrew@lunn.ch
This change makes sens however what about Thomas' comment: removing orion_clkdev_add() entirely and directly using lkdev_create() all over the place: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html
Then what would be the path for this patch?
As there is a dependency on the 6th patch of this series: "clkdev: add clkdev_create() helper" which should be merged through the clk tree, I think the best option is that this patch would be also managed by the clk tree maintainer (I added them in CC).
Thanks,
Gregory
Andrew
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
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Apr 07, 2015 at 03:20:05PM +0200, Gregory CLEMENT wrote:
Hi Andrew, Russell,
On 04/04/2015 02:17, Andrew Lunn wrote:
On Fri, Apr 03, 2015 at 06:13:13PM +0100, 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
Acked-by: Andrew Lunn andrew@lunn.ch
This change makes sens however what about Thomas' comment: removing orion_clkdev_add() entirely and directly using lkdev_create() all over the place: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html
Then what would be the path for this patch?
As there is a dependency on the 6th patch of this series: "clkdev: add clkdev_create() helper" which should be merged through the clk tree, I think the best option is that this patch would be also managed by the clk tree maintainer (I added them in CC).
Let me remind people that clkdev is *NOT* part of clk, and that I'm the maintainer for clkdev.
I'm getting rather pissed off with people taking work away from me, even when I'm named in the MAINTAINERS file. These patches are going through my tree unless there's a good reason for them not to. They are _not_ going through the clk tree.
Hi Russell,
On 07/04/2015 16:01, Russell King - ARM Linux wrote:
On Tue, Apr 07, 2015 at 03:20:05PM +0200, Gregory CLEMENT wrote:
Hi Andrew, Russell,
On 04/04/2015 02:17, Andrew Lunn wrote:
On Fri, Apr 03, 2015 at 06:13:13PM +0100, 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
Acked-by: Andrew Lunn andrew@lunn.ch
This change makes sens however what about Thomas' comment: removing orion_clkdev_add() entirely and directly using lkdev_create() all over the place: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html
Then what would be the path for this patch?
As there is a dependency on the 6th patch of this series: "clkdev: add clkdev_create() helper" which should be merged through the clk tree, I think the best option is that this patch would be also managed by the clk tree maintainer (I added them in CC).
Let me remind people that clkdev is *NOT* part of clk, and that I'm the maintainer for clkdev.
Sorry for the confusion, I quickly had a look on the MAINTAINERS file and didn't realized that the drivers/clk/clkdev.c file was not part of clk (even if actually it was mentioned).
I'm getting rather pissed off with people taking work away from me, even when I'm named in the MAINTAINERS file. These patches are going through my tree unless there's a good reason for them not to. They are _not_ going through the clk tree.
So, as you are going to take care of all the patches it is even simpler. You can take this one too: in mvebu there is no change on this file for this release so there won't be any conflict.
Thanks,
Gregory
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
* Russell King rmk+kernel@arm.linux.org.uk [150403 10:14]:
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
Acked-by: Tony Lindgren tony@atomide.com
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
When creating aliases of existing clkdev clocks, use clkdev_add_alias() isntead of open coding the lookup and clk_lookup creation.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- arch/arm/mach-omap2/omap_device.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c index be9541e18650..521c32e7778e 100644 --- a/arch/arm/mach-omap2/omap_device.c +++ b/arch/arm/mach-omap2/omap_device.c @@ -47,7 +47,7 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, const char *clk_name) { struct clk *r; - struct clk_lookup *l; + int rc;
if (!clk_alias || !clk_name) return; @@ -62,21 +62,15 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias, return; }
- r = clk_get(NULL, clk_name); - if (IS_ERR(r)) { - dev_err(&od->pdev->dev, - "clk_get for %s failed\n", clk_name); - return; + rc = clk_add_alias(clk_alias, dev_name(&od->pdev->dev), clk_name, NULL); + if (rc) { + if (rc == -ENODEV || rc == -ENOMEM) + dev_err(&od->pdev->dev, + "clkdev_alloc for %s failed\n", clk_alias); + else + dev_err(&od->pdev->dev, + "clk_get for %s failed\n", clk_name); } - - l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev)); - if (!l) { - dev_err(&od->pdev->dev, - "clkdev_alloc for %s failed\n", clk_alias); - return; - } - - clkdev_add(l); }
/**
* Russell King rmk+kernel@arm.linux.org.uk [150403 10:14]:
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
Acked-by: Tony Lindgren tony@atomide.com
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);elsedev_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
clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead.
Acked-by: Mark Brown broonie@kernel.org 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);
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++) {
On Fri, Apr 03, 2015 at 06:11:49PM +0100, Russell King - ARM Linux wrote:
Sorry for posting this soo close to the 4.1 merge window, I had completely forgotten about this chunk of work I did earlier this month.
Correction - earlier _last_ month - 1st/2nd March to be exact.
Hi Russell,
On Fri, Apr 3, 2015 at 7:11 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
Sorry for posting this soo close to the 4.1 merge window, I had completely forgotten about this chunk of work I did earlier this month.
The per-user struct clk patches rather badly broke clkdev and various other places. This was reported, but was forgotten about. Really, the per-user clk stuff should've been reverted, but we've lived with it far too long for that.
So, our only other option is to now rush these patches into 4.1 and hope for the best.
The series cleans up quite a number of places too...
Thanks for your patches!
Can you please tell which are critical fixes for regressions, and which are cleanups? It's not so obvious to me from the patch descriptions.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Sun, Apr 05, 2015 at 11:04:56AM +0200, Geert Uytterhoeven wrote:
Hi Russell,
On Fri, Apr 3, 2015 at 7:11 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
Sorry for posting this soo close to the 4.1 merge window, I had completely forgotten about this chunk of work I did earlier this month.
The per-user struct clk patches rather badly broke clkdev and various other places. This was reported, but was forgotten about. Really, the per-user clk stuff should've been reverted, but we've lived with it far too long for that.
So, our only other option is to now rush these patches into 4.1 and hope for the best.
The series cleans up quite a number of places too...
Thanks for your patches!
Can you please tell which are critical fixes for regressions, and which are cleanups? It's not so obvious to me from the patch descriptions.
The 5th one is the most important as far as fixing the regression caused by the per-user clk patches.
participants (10)
-
Andrew Lunn -
Geert Uytterhoeven -
Gregory CLEMENT -
Laurent Pinchart -
Robert Jarzmik -
Russell King -
Russell King - ARM Linux -
Sekhar Nori -
Stephen Boyd -
Tony Lindgren