diff mbox

clk: fix clk-gpio.c with optional clock= DT property

Message ID 20160103064749.15239.59588@quark.deferred.io (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Turquette Jan. 3, 2016, 6:47 a.m. UTC
Hi Russell,

Quoting Russell King (2016-01-02 02:01:34)
> When the clock DT property is not given, of_clk_get_parent_count()
> returns -ENOENT, which then tries to allocate -2 x 4 bytes of memory,
> which of course fails, causing the whole driver to fail to create
> the clock.
> 
> This causes the SolidRun platforms to fail probing the SDHCI1 interface
> which is connected to the WiFi.
> 
> Fix this by detecting errno codes, skipping the allocation, and fixing
> of_clk_gpio_gate_delayed_register_get() to handle a NULL parent_names
> array.

Thanks for the fix. Applied to clk-next manually.

> 
> Fixes: 80eeb1f0f757 ("clk: add gpio controlled clock multiplexer")
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> This goes all the way back to 80eeb1f0f757 ("clk: add gpio controlled
> clock multiplexer") introduced in June in v4.3-rc2 - which raises the
> question why _development_ work in clk is being merged outside of the
> merge window.

That patch was merged into the clk tree in June 2015, based on 4.2-rc1.

v4.2 was release on Sunday, August 30, 2015. My pull request to Linus
containing this patch was sent on Monday, August 31, 2015. Thread
archived here:

http://lkml.kernel.org/r/<20150831192125.11508.92473@quantum>

Looks like it was merged in 4.3-rc1 as well:

$ git log v4.3-rc1..clk-for-linus-4.3
$

> 
> A rewrite of this patch will be necessary to apply to v4.3 kernels.
> 
> This applies on top of v4.4-rc6.

Alternatively we can send f66541ba02d5 "clk: gpio: Get parent clk names
in of_gpio_clk_setup()" along with your fix.

Below is the modified version of your patch.

Regards,
Mike



commit 60f7e5a537f367d3be7cff2bce13c91c10bdf451
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date:   Sat Jan 2 10:01:34 2016 +0000

    clk: fix clk-gpio.c with optional clock= DT property
    
    When the clock DT property is not given, of_clk_get_parent_count()
    returns -ENOENT, which then tries to allocate -2 x 4 bytes of memory,
    which of course fails, causing the whole driver to fail to create
    the clock.
    
    This causes the SolidRun platforms to fail probing the SDHCI1 interface
    which is connected to the WiFi.
    
    Fix this by detecting errno codes, skipping the allocation, and fixing
    of_clk_gpio_gate_delayed_register_get() to handle a NULL parent_names
    array.
    
    Fixes: 80eeb1f0f757 ("clk: add gpio controlled clock multiplexer")
    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
    Signed-off-by: Michael Turquette <mturquette@baylibre.com>

Comments

Russell King - ARM Linux Jan. 3, 2016, 10:53 a.m. UTC | #1
On Sat, Jan 02, 2016 at 10:47:49PM -0800, Michael Turquette wrote:
> Hi Russell,
> 
> Quoting Russell King (2016-01-02 02:01:34)
> > When the clock DT property is not given, of_clk_get_parent_count()
> > returns -ENOENT, which then tries to allocate -2 x 4 bytes of memory,
> > which of course fails, causing the whole driver to fail to create
> > the clock.
> > 
> > This causes the SolidRun platforms to fail probing the SDHCI1 interface
> > which is connected to the WiFi.
> > 
> > Fix this by detecting errno codes, skipping the allocation, and fixing
> > of_clk_gpio_gate_delayed_register_get() to handle a NULL parent_names
> > array.
> 
> Thanks for the fix. Applied to clk-next manually.

Thanks.

> > This goes all the way back to 80eeb1f0f757 ("clk: add gpio controlled
> > clock multiplexer") introduced in June in v4.3-rc2 - which raises the
> > question why _development_ work in clk is being merged outside of the
> > merge window.
> 
> That patch was merged into the clk tree in June 2015, based on 4.2-rc1.

Interesting... I guess git describe is buggy then:

$ git describe --contains 80eeb1f0f757
v4.3-rc2~4^2~124
$ git log v4.3-rc1..v4.3-rc2 | grep 80eeb1f0f757
$ git log v4.2..v4.3-rc1 | grep 80eeb1f0f757
commit 80eeb1f0f757c790b020d9f425bb0e824973d49c
$ git describe --help
...
       --contains
           Instead of finding the tag that predates the commit, find the tag
           that comes after the commit, and thus contains it. Automatically
           implies --tags.

> Below is the modified version of your patch.

Looks fine, thanks.
Fabio Estevam Jan. 3, 2016, 11:35 p.m. UTC | #2
On Sun, Jan 3, 2016 at 8:53 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

>> That patch was merged into the clk tree in June 2015, based on 4.2-rc1.
>
> Interesting... I guess git describe is buggy then:
>
> $ git describe --contains 80eeb1f0f757

'git tag --contains' shows the correct response.

$ git tag --contains 80eeb1f0f757
v4.3
v4.3-rc1
v4.3-rc2
v4.3-rc3
v4.3-rc4
v4.3-rc5
v4.3-rc6
v4.3-rc7
v4.4-rc1
v4.4-rc2
v4.4-rc3
v4.4-rc4
v4.4-rc5
v4.4-rc6
v4.4-rc7
diff mbox

Patch

diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
index 1767b9e..19fed65 100644
--- a/drivers/clk/clk-gpio.c
+++ b/drivers/clk/clk-gpio.c
@@ -264,8 +264,8 @@  static struct clk *of_clk_gpio_gate_delayed_register_get(const char *name,
 		const char * const *parent_names, u8 num_parents,
 		unsigned gpio, bool active_low)
 {
-	return clk_register_gpio_gate(NULL, name, parent_names[0],
-			gpio, active_low, 0);
+	return clk_register_gpio_gate(NULL, name, parent_names ?
+			parent_names[0] : NULL, gpio, active_low, 0);
 }
 
 static struct clk *of_clk_gpio_mux_delayed_register_get(const char *name,
@@ -295,15 +295,19 @@  static void __init of_gpio_clk_setup(struct device_node *node,
 	if (!data)
 		return;
 
-	parent_names = kcalloc(num_parents, sizeof(char *), GFP_KERNEL);
-	if (!parent_names) {
-		kfree(data);
-		return;
+	if (num_parents) {
+		parent_names = kcalloc(num_parents, sizeof(char *), GFP_KERNEL);
+		if (!parent_names) {
+			kfree(data);
+			return;
+		}
+
+		for (i = 0; i < num_parents; i++)
+			parent_names[i] = of_clk_get_parent_name(node, i);
+	} else {
+		parent_names = NULL;
 	}
 
-	for (i = 0; i < num_parents; i++)
-		parent_names[i] = of_clk_get_parent_name(node, i);
-
 	data->num_parents = num_parents;
 	data->parent_names = parent_names;
 	data->node = node;