[PATCH] clk: gpio: Really allow an optional clock= DT property

[PATCH] clk: gpio: Really allow an optional clock= DT property

[Stephen Boyd]

We mis-merged the original patch from Russell here and so the
patch went almost all the way, except that we still failed to
probe when there wasn't a clocks property in the DT node. Allow
that case by making a negative value from
of_clk_get_parent_count() into "no parents", like the original
patch did.

Fixes: 7ed88aa2efa5 ("clk: fix clk-gpio.c with optional clock= DT property")
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clk/clk-gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
index 19fed65587e8..7b09a265d79f 100644
--- a/drivers/clk/clk-gpio.c
+++ b/drivers/clk/clk-gpio.c
@@ -289,7 +289,7 @@ static void __init of_gpio_clk_setup(struct device_node *node,
 
 	num_parents = of_clk_get_parent_count(node);
 	if (num_parents < 0)
-		return;
+		num_parents = 0;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: gpio: Really allow an optional clock= DT property

[Michael Turquette]

On Thu, Feb 18, 2016 at 7:12 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>
> We mis-merged the original patch from Russell here and so the
> patch went almost all the way, except that we still failed to
> probe when there wasn't a clocks property in the DT node. Allow
> that case by making a negative value from
> of_clk_get_parent_count() into "no parents", like the original
> patch did.
>
> Fixes: 7ed88aa2efa5 ("clk: fix clk-gpio.c with optional clock= DT property")
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

Ack.

Regards,
Mike

> ---
>  drivers/clk/clk-gpio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
> index 19fed65587e8..7b09a265d79f 100644
> --- a/drivers/clk/clk-gpio.c
> +++ b/drivers/clk/clk-gpio.c
> @@ -289,7 +289,7 @@ static void __init of_gpio_clk_setup(struct device_node *node,
>
>         num_parents = of_clk_get_parent_count(node);
>         if (num_parents < 0)
> -               return;
> +               num_parents = 0;
>
>         data = kzalloc(sizeof(*data), GFP_KERNEL);
>         if (!data)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>



-- 
Michael Turquette
CEO
BayLibre - At the Heart of Embedded Linux
http://baylibre.com/

Re: [PATCH] clk: gpio: Really allow an optional clock= DT property

[Russell King - ARM Linux]

On Thu, Feb 18, 2016 at 07:12:05PM -0800, Stephen Boyd wrote:
> We mis-merged the original patch from Russell here and so the
> patch went almost all the way, except that we still failed to
> probe when there wasn't a clocks property in the DT node. Allow
> that case by making a negative value from
> of_clk_get_parent_count() into "no parents", like the original
> patch did.

NAK.  I'm not testing this patch, when I've already spent more than
enough time (a) developing the original patch and testing that, and
(b) re-diagnosing what's going wrong, and re-fixing the fuck up,
and testing that fix.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] clk: gpio: Really allow an optional clock= DT property

[Russell King - ARM Linux]

On Thu, Feb 18, 2016 at 07:12:05PM -0800, Stephen Boyd wrote:
> We mis-merged the original patch from Russell here and so the
> patch went almost all the way, except that we still failed to
> probe when there wasn't a clocks property in the DT node. Allow
> that case by making a negative value from
> of_clk_get_parent_count() into "no parents", like the original
> patch did.

So you apply it anyway, and ignore all further discussion.  I've
no idea what kind of politics is going on here, but it has to stop.

I'm also still waiting for that apology from Mike - never apply a
modified patch without stating in the commit log that it was modified.
It's basically fraud.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] clk: gpio: Really allow an optional clock= DT property

[Michael Turquette]

Hi Russell,

Quoting Russell King - ARM Linux (2016-02-23 03:30:59)
> On Thu, Feb 18, 2016 at 07:12:05PM -0800, Stephen Boyd wrote:
> > We mis-merged the original patch from Russell here and so the
> > patch went almost all the way, except that we still failed to
> > probe when there wasn't a clocks property in the DT node. Allow
> > that case by making a negative value from
> > of_clk_get_parent_count() into "no parents", like the original
> > patch did.
> 
> So you apply it anyway, and ignore all further discussion.  I've
> no idea what kind of politics is going on here, but it has to stop.

Stephen fixed the bug, and I didn't ask you to test because you made it
clear that you did not have the time (understandable). Stephen
reproduced the issue locally and insured that his version of the fix
does actually fix it.

He is an equal co-maintainer so of course I support him to merge the
fix, plus I find his version more readable.

> 
> I'm also still waiting for that apology from Mike - never apply a

No thematics please. When you wrongly accused me of merging code outside
of the merge window[0] did I ask for an apology?

> modified patch without stating in the commit log that it was modified.
> It's basically fraud.

An oversight for which we are both equally guilty. The "change" was the
resolution of a merge conflict, and I posted the resolved version of the
patch to the thread[0]. You said, "Looks fine, thanks."

In summary: bugs happen. Let's please just move on and get back to work.

[0] http://lkml.kernel.org/r/<20160103105319.GB5779@n2100.arm.linux.org.uk>

Regards,
Mike

> 
> -- 
> RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH] clk: gpio: Really allow an optional clock= DT property

[Russell King - ARM Linux]

On Wed, Feb 24, 2016 at 01:47:06PM -0800, Michael Turquette wrote:
> Hi Russell,
> 
> Quoting Russell King - ARM Linux (2016-02-23 03:30:59)
> > On Thu, Feb 18, 2016 at 07:12:05PM -0800, Stephen Boyd wrote:
> > > We mis-merged the original patch from Russell here and so the
> > > patch went almost all the way, except that we still failed to
> > > probe when there wasn't a clocks property in the DT node. Allow
> > > that case by making a negative value from
> > > of_clk_get_parent_count() into "no parents", like the original
> > > patch did.
> > 
> > So you apply it anyway, and ignore all further discussion.  I've
> > no idea what kind of politics is going on here, but it has to stop.
> 
> Stephen fixed the bug, and I didn't ask you to test because you made it
> clear that you did not have the time (understandable). Stephen
> reproduced the issue locally and insured that his version of the fix
> does actually fix it.

Right, so let's go back shall we.

The reason this bug happened is because you changed fully tested code
as a result of a merge conflict.  The result was not tested.

Now, rather than going back to the tested patch, you maintainers decide
to create yet another change, which is also untested and merge it
immediately into mainline.

That's really insane - you had a choice to do the sane thing: you could
have gone back and looked at my patch, realised that my patch resolves
the same problem that the conflicting patch did, and therefore the
correct resolution would be to make the code look the same after both
patches have been applied, as if it would have done if only my patch
had been applied.

And the resulting code would have been tested by implication of the
fact that the original code plus my patch was what I tested here.

But no, you guys think it's better to create some other solution to the
fuck up.

> He is an equal co-maintainer so of course I support him to merge the
> fix, plus I find his version more readable.

I really detest this maintainer shite - it seems anyone can walk up and
take 

> > I'm also still waiting for that apology from Mike - never apply a
> 
> No thematics please. When you wrongly accused me of merging code outside
> of the merge window[0] did I ask for an apology?
> 
> > modified patch without stating in the commit log that it was modified.
> > It's basically fraud.
> 
> An oversight for which we are both equally guilty.

Sorry, I disagree.  I'm not guilty of doing that, because when I apply
patches, they come straight from the patch system and are applied to
my tree unmodified.  There's no intermediate modification step possible.
Any changes I want to make are done in a separate commit.

So that's another apology you now owe me.

> The "change" was the
> resolution of a merge conflict, and I posted the resolved version of the
> patch to the thread[0]. You said, "Looks fine, thanks."

Yes, it _looked_ fine for a quick read, and I didn't test it.  I
trusted you to be competent at merge resolution, but I now see that
trust was misplaced.  I'll try to remember to pay more attention to
any changes you make in the future - or I'll merge them through my
tree.

Trust is something I initially give without earning - break that
trust and you've got to work really hard to regain it.

I'll also remind you that _I_ used to look after the clk code, until
you walked onto the scene and effectively took it away from me - I
didn't say a word about that (or all the other stuff that Linaro
"took" from me without asking.)  So you won't mind if I do a bit of
that myself by merging my own fixes through my tree.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.