From patchwork Fri Feb 19 03:07:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 8355791 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 917FAC0553 for ; Fri, 19 Feb 2016 03:09:52 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AB5B6203EC for ; Fri, 19 Feb 2016 03:09:51 +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 B199F203E6 for ; Fri, 19 Feb 2016 03:09:50 +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 1aWbQB-0001EB-Nb; Fri, 19 Feb 2016 03:07:55 +0000 Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aWbQ9-0001DT-DJ for linux-arm-kernel@lists.infradead.org; Fri, 19 Feb 2016 03:07:54 +0000 Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id D681D611BB; Fri, 19 Feb 2016 03:07:32 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id CA105611D0; Fri, 19 Feb 2016 03:07:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from localhost (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 3EDE46023D; Fri, 19 Feb 2016 03:07:32 +0000 (UTC) Date: Thu, 18 Feb 2016 19:07:31 -0800 From: Stephen Boyd To: Russell King - ARM Linux Subject: Re: [PATCH] clk: fix clk-gpio.c with optional clock= DT property Message-ID: <20160219030731.GS4847@codeaurora.org> References: <20160217230529.GM19428@n2100.arm.linux.org.uk> <20160218000747.2278.46405@quark.deferred.io> <20160218005503.GO19428@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160218005503.GO19428@n2100.arm.linux.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) X-Virus-Scanned: ClamAV using ClamSMTP X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160218_190753_514810_8D2D16E3 X-CRM114-Status: GOOD ( 23.89 ) X-Spam-Score: -1.9 (-) 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-Virus-Scanned: ClamAV using ClamSMTP 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. ---8<--- 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) Maybe we should change of_clk_get_parent_count() to return an unsigned int as well. I'm not sure why anyone would care whether or not a node has "clocks" or not as a property. It seems that all the callers of this function want to know how many parents there are. In fact, some TI drivers assign the return value from this function directly to clk_init_data::num_parents which is an unsigned value. That's horribly broken if there isn't a proper DT node. So how about we apply this patch for the next release? We could also change the return type to unsigned so that people don't get the wrong idea. That type of change will be a bit larger though because we'll have to remove impossible < 0 checks in drivers; not a huge deal but slightly annoying. I can do that legwork tomorrow. ----8<--- diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 67cd2f116c3b..cdf18f76acb0 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3020,9 +3020,21 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec) return __of_clk_get_from_provider(clkspec, NULL, __func__); } +/** + * of_clk_get_parent_count() - Count the number of clocks a device node has + * @np: device node to count + * + * Returns: The number of clocks that are possible parents of this node + */ int of_clk_get_parent_count(struct device_node *np) { - return of_count_phandle_with_args(np, "clocks", "#clock-cells"); + int count; + + count = of_count_phandle_with_args(np, "clocks", "#clock-cells"); + if (count < 0) + return 0; + + return count; } EXPORT_SYMBOL_GPL(of_clk_get_parent_count);