From patchwork Thu Jul 9 20:39:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris BREZILLON X-Patchwork-Id: 6758851 Return-Path: X-Original-To: patchwork-linux-omap@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 0690B9F38C for ; Thu, 9 Jul 2015 20:40:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DC53D20791 for ; Thu, 9 Jul 2015 20:40:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BA97E2078A for ; Thu, 9 Jul 2015 20:40:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753659AbbGIUjo (ORCPT ); Thu, 9 Jul 2015 16:39:44 -0400 Received: from down.free-electrons.com ([37.187.137.238]:58389 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753206AbbGIUjm convert rfc822-to-8bit (ORCPT ); Thu, 9 Jul 2015 16:39:42 -0400 Received: by mail.free-electrons.com (Postfix, from userid 106) id 2BE6166B; Thu, 9 Jul 2015 22:39:41 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from bbrezillon (unknown [46.189.28.236]) by mail.free-electrons.com (Postfix) with ESMTPSA id 81F551D0; Thu, 9 Jul 2015 22:39:40 +0200 (CEST) Date: Thu, 9 Jul 2015 22:39:38 +0200 From: Boris Brezillon To: Stephen Boyd Cc: Mike Turquette , linux-clk@vger.kernel.org, Jonathan Corbet , Tony Lindgren , Ralf Baechle , Emilio =?UTF-8?B?TMOzcGV6?= , Maxime Ripard , Tero Kristo , Peter De Schrijver , Prashant Gaikwad , Stephen Warren , Thierry Reding , Alexandre Courbot , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org, linux-mips@linux-mips.org, linux-tegra@vger.kernel.org Subject: Re: [PATCH v5] clk: change clk_ops' ->determine_rate() prototype Message-ID: <20150709223938.19450e75@bbrezillon> In-Reply-To: <559D66EE.4060707@codeaurora.org> References: <1436294888-25752-1-git-send-email-boris.brezillon@free-electrons.com> <20150708005748.GG30412@codeaurora.org> <20150708110005.704c49ff@bbrezillon> <559D66EE.4060707@codeaurora.org> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Stephen, On Wed, 08 Jul 2015 11:07:42 -0700 Stephen Boyd wrote: > On 07/08/2015 02:00 AM, Boris Brezillon wrote: > > Hi Stephen, > > > > On Tue, 7 Jul 2015 17:57:48 -0700 > > Stephen Boyd wrote: > > > >> On 07/07, Boris Brezillon wrote: > >>> > >>> } else { > >>> pr_err("clk: clk_composite_determine_rate function called, but no mux or rate callback set!\n"); > >>> + req->rate = 0; > >>> return 0; > >> Shouldn't this return an error now? And then assigning req->rate > >> wouldn't be necessary. Sorry I must have missed this last round. > >> > > Actually I wanted to keep the existing behavior: return a 0 rate (not > > an error) when there is no mux or rate ops. > > > > That's something we can change afterwards, but it might reveals > > new bugs if some users are checking for a 0 rate to detect errors. > > > > Ok. Care to send the patch now to do that while we're thinking about it? > We can test it out for a month or two. > Here is a patch modifying a few drivers to return errors instead of a 0 rate. Feel free to squash it in the previous one if you think this is better. Best Regards, Boris --- >8 --- From dca9c28301042cf19dad4b1e4555cdb7c1063745 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 9 Jul 2015 12:20:21 +0200 Subject: [PATCH] clk: fix some determine_rate implementations Some determine_rate implementations are not returning an error when then failed to adapt the rate according to the rate request. Fix them so that they return an error instead of silently returning 0. Signed-off-by: Boris Brezillon --- arch/mips/alchemy/common/clock.c | 4 ++++ drivers/clk/clk-composite.c | 3 +-- drivers/clk/clk.c | 15 ++++++--------- drivers/clk/hisilicon/clk-hi3620.c | 2 +- drivers/clk/mmp/clk-mix.c | 5 ++++- drivers/clk/sunxi/clk-factors.c | 6 ++++-- drivers/clk/sunxi/clk-sun6i-ar100.c | 3 +++ drivers/clk/sunxi/clk-sunxi.c | 6 ++++-- 8 files changed, 27 insertions(+), 17 deletions(-) diff --git a/arch/mips/alchemy/common/clock.c b/arch/mips/alchemy/common/clock.c index 0b4cf3e..7cc3eed 100644 --- a/arch/mips/alchemy/common/clock.c +++ b/arch/mips/alchemy/common/clock.c @@ -469,9 +469,13 @@ static int alchemy_clk_fgcs_detr(struct clk_hw *hw, } } + if (br < 0) + return br; + req->best_parent_rate = bpr; req->best_parent_hw = __clk_get_hw(bpc); req->rate = br; + return 0; } diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index 9e69f34..35ac062 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -125,8 +125,7 @@ static int clk_composite_determine_rate(struct clk_hw *hw, return mux_ops->determine_rate(mux_hw, req); } else { pr_err("clk: clk_composite_determine_rate function called, but no mux or rate callback set!\n"); - req->rate = 0; - return 0; + return -EINVAL; } } diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c182d8a..d67d9b4 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -447,24 +447,18 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req, /* if NO_REPARENT flag set, pass through to current parent */ if (core->flags & CLK_SET_RATE_NO_REPARENT) { - parent = core->parent; + best_parent = core->parent; if (core->flags & CLK_SET_RATE_PARENT) { - ret = __clk_determine_rate(parent ? parent->hw : NULL, + ret = __clk_determine_rate(best_parent ? best_parent->hw : NULL, &parent_req); if (ret) return ret; best = parent_req.rate; - req->best_parent_hw = parent->hw; - req->best_parent_rate = best; - } else if (parent) { + } else if (best_parent) { best = clk_core_get_rate_nolock(parent); - req->best_parent_hw = parent->hw; - req->best_parent_rate = best; } else { best = clk_core_get_rate_nolock(core); - req->best_parent_hw = NULL; - req->best_parent_rate = 0; } goto out; @@ -493,6 +487,9 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req, } } + if (!best_parent) + return -EINVAL; + out: if (best_parent) req->best_parent_hw = best_parent->hw; diff --git a/drivers/clk/hisilicon/clk-hi3620.c b/drivers/clk/hisilicon/clk-hi3620.c index a0674ba..c84ec86 100644 --- a/drivers/clk/hisilicon/clk-hi3620.c +++ b/drivers/clk/hisilicon/clk-hi3620.c @@ -316,7 +316,7 @@ static int mmc_clk_determine_rate(struct clk_hw *hw, req->rate = 180000000; req->best_parent_rate = 1440000000; } - return 0; + return -EINVAL; } static u32 mmc_clk_delay(u32 val, u32 para, u32 off, u32 len) diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c index 7a37432..665cb67 100644 --- a/drivers/clk/mmp/clk-mix.c +++ b/drivers/clk/mmp/clk-mix.c @@ -218,7 +218,7 @@ static int mmp_clk_mix_determine_rate(struct clk_hw *hw, parent = NULL; mix_rate_best = 0; parent_rate_best = 0; - gap_best = req->rate; + gap_best = ULONG_MAX; parent_best = NULL; if (mix->table) { @@ -262,6 +262,9 @@ static int mmp_clk_mix_determine_rate(struct clk_hw *hw, } found: + if (!parent_best) + return -EINVAL; + req->best_parent_rate = parent_rate_best; req->best_parent_hw = __clk_get_hw(parent_best); req->rate = mix_rate_best; diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c index 7a48587..94e2570 100644 --- a/drivers/clk/sunxi/clk-factors.c +++ b/drivers/clk/sunxi/clk-factors.c @@ -107,8 +107,10 @@ static int clk_factors_determine_rate(struct clk_hw *hw, } } - if (best_parent) - req->best_parent_hw = __clk_get_hw(best_parent); + if (!best_parent) + return -EINVAL; + + req->best_parent_hw = __clk_get_hw(best_parent); req->best_parent_rate = best; req->rate = best_child_rate; diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c index d70c1ea..21b076e 100644 --- a/drivers/clk/sunxi/clk-sun6i-ar100.c +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c @@ -105,6 +105,9 @@ static int ar100_determine_rate(struct clk_hw *hw, } } + if (best_rate < 0) + return best_rate; + req->rate = best_rate; return 0; diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c index d0f72a1..0e15165 100644 --- a/drivers/clk/sunxi/clk-sunxi.c +++ b/drivers/clk/sunxi/clk-sunxi.c @@ -146,8 +146,10 @@ static int sun6i_ahb1_clk_determine_rate(struct clk_hw *hw, } } - if (best_parent) - req->best_parent_hw = __clk_get_hw(best_parent); + if (!best_parent) + return -EINVAL; + + req->best_parent_hw = __clk_get_hw(best_parent); req->best_parent_rate = best; req->rate = best_child_rate;