From patchwork Fri Feb 19 09:39:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 8358691 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CBB609F38B for ; Fri, 19 Feb 2016 09:42:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D0A38202F0 for ; Fri, 19 Feb 2016 09:42:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A545E20115 for ; Fri, 19 Feb 2016 09:42:14 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aWhYJ-0006Dn-9f; Fri, 19 Feb 2016 09:40:43 +0000 Received: from pandora.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aWhY9-00052U-9i for linux-arm-kernel@lists.infradead.org; Fri, 19 Feb 2016 09:40:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=YYMQiSxz68XeAG15q8cILXmHlUTzMwUgJ6kWFWqHxFU=; b=lOXjwKGK/aupbZuZJGf9hNga+5wfXcACW39Yz+eHT08yoSLE4GteXcrEEoAKDq4i7B5CqIRpiwdoa+QBO3J9xxBsrFlyEVIiI8S8RPV6ck/4Ux3LxBynCUtAUWoxw6SWa5IfYmaUsp1c1Lfa2uZIDnBKUVACr9o8BJ5nwg6HPb4=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:46034) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1aWhXg-0006JM-J7; Fri, 19 Feb 2016 09:40:04 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1aWhXb-0001Bz-Ov; Fri, 19 Feb 2016 09:39:59 +0000 Date: Fri, 19 Feb 2016 09:39:59 +0000 From: Russell King - ARM Linux To: Stephen Boyd Subject: Re: [PATCH] clk: fix clk-gpio.c with optional clock= DT property Message-ID: <20160219093959.GV19428@n2100.arm.linux.org.uk> References: <20160217230529.GM19428@n2100.arm.linux.org.uk> <20160218000747.2278.46405@quark.deferred.io> <20160218005503.GO19428@n2100.arm.linux.org.uk> <20160219030731.GS4847@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160219030731.GS4847@codeaurora.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160219_014034_329192_F6794555 X-CRM114-Status: GOOD ( 20.05 ) X-Spam-Score: -4.3 (----) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Michael Turquette , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Feb 18, 2016 at 07:07:31PM -0800, Stephen Boyd wrote: > On 02/18, Russell King - ARM Linux wrote: > > This is even explained in the very first sentence of my commit > > log: > > > > | 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, > > ^^^^^^^^^^^^^^^ > > > > -ENOENT is a negative number. Now look at the patch which totally > > rejects the clock if of_clk_get_parent_count() returns any negative > > number... > > > > I assume, therefore, that you did not *even* read my commit log... > > > > Ok. So the simplest fix is to just do this? I'll write up some > commit text and get this into the rc. I'm sorry, I've had enough of this crappy maintainer behaviour over this issue. There's a simple solution: (a) making a patch which undoes the broken commit and gives the result from my _tested_ patch which is _known_ to work, rather than coming up with yet another potentially broken solution. (b) someone apologising for modifying a patch without mentioning in the commit log that it was modified, where the modification makes the patch no longer match the commit log, and effectively making the patch totally useless. (a) is easy: Apply that with an apology and I'll be happy. Do something else and I'm not going to be happy, because it means more work for me. And yes, yet again, I've tested the above solution, and it works. Of course it works, it's my original solution, which was tested at the time. The moral here is: do _not_ modify other peoples tested patches, or if you do either _test_ them yourself or ask for them to be tested before committing. And if you modify a patch, _say so_ in the commit text. And yes, in this case, it's _easy_ for you to test. You don't need hardware other than a free gpio pin to come up with a DT fragment to insert in a test platforms DT. There is *no* excuse for this mess. diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index 19fed65587e8..05cca9298f44 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c @@ -287,15 +287,12 @@ static void __init of_gpio_clk_setup(struct device_node *node, const char **parent_names; int i, num_parents; - num_parents = of_clk_get_parent_count(node); - if (num_parents < 0) - return; - data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) return; - if (num_parents) { + num_parents = of_clk_get_parent_count(node); + if (num_parents > 0) { parent_names = kcalloc(num_parents, sizeof(char *), GFP_KERNEL); if (!parent_names) { kfree(data);