From patchwork Mon Jul 3 03:46:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aisheng Dong X-Patchwork-Id: 9821889 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0FF336035F for ; Mon, 3 Jul 2017 03:46:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E13F226212 for ; Mon, 3 Jul 2017 03:46:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D4CDC2851A; Mon, 3 Jul 2017 03:46:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1BC0426212 for ; Mon, 3 Jul 2017 03:46:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752157AbdGCDqx (ORCPT ); Sun, 2 Jul 2017 23:46:53 -0400 Received: from mail-he1eur01on0089.outbound.protection.outlook.com ([104.47.0.89]:48208 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752152AbdGCDqw (ORCPT ); Sun, 2 Jul 2017 23:46:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=VVE5aY4EZDTjZFlor2O0xrhowA2PEvh3xdie2Tdz9ss=; b=ELdH7guA7OM5OeqZEZRM74H9oA2mPiZuMVcdKjJA/eR0rCCyy7DfgHFWV1YnkE3CltpMg1Zw85429/LwRv3lFobb0gpnkBKAnmsLMJQbsCoC0fgPKIjPpM5IkdXHxDWUeuuKtJEtZrDynBZEUBYvHlqs6fQ3c6xCOfSSiPpQZmk= Received: from AM3PR04MB306.eurprd04.prod.outlook.com (10.242.113.144) by AM3PR04MB1316.eurprd04.prod.outlook.com (10.163.7.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1220.11; Mon, 3 Jul 2017 03:46:47 +0000 Received: from AM3PR04MB306.eurprd04.prod.outlook.com ([fe80::909a:ba48:a49c:c57b]) by AM3PR04MB306.eurprd04.prod.outlook.com ([fe80::909a:ba48:a49c:c57b%13]) with mapi id 15.01.1220.015; Mon, 3 Jul 2017 03:46:47 +0000 From: "A.s. Dong" To: Stephen Boyd , Dong Aisheng CC: "linux-clk@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "mturquette@baylibre.com" , "shawnguo@kernel.org" , "Anson Huang" , Jacky Bai Subject: RE: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support Thread-Topic: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support Thread-Index: AQHSzYN3F+RbVHbrJkqp4dYX0fhP6KItMq8AgAB7yYCAEMAHAIADTWEA Date: Mon, 3 Jul 2017 03:46:47 +0000 Message-ID: References: <1494856763-6543-1-git-send-email-aisheng.dong@nxp.com> <1494856763-6543-2-git-send-email-aisheng.dong@nxp.com> <20170620014512.GL4493@codeaurora.org> <20170620090815.GA6805@b29396-OptiPlex-7040> <20170701005542.GT22780@codeaurora.org> In-Reply-To: <20170701005542.GT22780@codeaurora.org> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: codeaurora.org; dkim=none (message not signed) header.d=none; codeaurora.org; dmarc=none action=none header.from=nxp.com; x-originating-ip: [199.59.231.64] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR04MB1316; 7:m66+cgHM38PYVaBqhLg12jdYLaVn5PCPBthXx5iLvFTetyuH2SAJcXUGzhQg+YGa62kv8mqQC4sMRY4NcXu22PSnbvjzdhCurc+uKkVTOB9PUmAv282dMXgm0p7LLafeEfTYWWUhDXR5AGnCaAw7jy6LZEQwTbBkrf4+hpruh1PoJ26GCb4wGDA+FhKEx/ybJn849n6sWODdDbSfxizK0thG2S85trozrTegUx997Cbhl3wHgFOzfhaWvfzH3ATyw5FCiSP7brdUqUvk9iOtRNKob256NU6W9+UuU9Z6oxCOAPlnToffx7hYFb/Hv0MowGlF4x+JVE815FeFk47vvyBRFrSFioLDzieECZnwsnbceTPLgKCm2hmjRjtvO/83538Ad2r3Wc4ZrZFUOtJwu2T4xowgxMDReUtZ/c3tAmrjk+FVpEjvDTgr5WWR6+CIjCa+3wWw0JR7nNkIsEeKX0UbGMwLahWCZacoN8cVVS50b35cj6M/Ii27jYD2TbhYq0axdXr+vm+bcC6xO9aWK/OIS2fX5sOlHlb0ZbMn2ikOaqrFSe/+JeejdG0c/zUoI1TohjlZYYKOCKynIh8vd2Cc+0tqi69kHXgLci7lZHdzHa8taxmvbyyp4ZrZrd8f198w0aVkgJsICWOxatAP3KOx1r65pl5vQs+/CzXf3VEDILCDzlz4zTRpB2YHoUFGBcf5dFk6y0pHWOZ9wQkcFgS3CpIKcFQE4w6kgFe+JKzfW4wpxtMVJNM1AFMeHZRtPoN0nqKTJm9uaVRgsH4i91/rQ+kX5lLHF1/z0vwDovI= x-forefront-antispam-report: SFV:SKI; SCL:-1SFV:NSPM; SFS:(10009020)(6009001)(39860400002)(39840400002)(39450400003)(39850400002)(39410400002)(39400400002)(377454003)(13464003)(24454002)(3660700001)(3280700002)(14454004)(50986999)(5250100002)(54356999)(93886004)(76176999)(86362001)(478600001)(2906002)(6506006)(8936002)(2900100001)(39060400002)(6436002)(229853002)(2950100002)(38730400002)(55016002)(99286003)(6246003)(25786009)(9686003)(54906002)(189998001)(7736002)(4326008)(106356001)(6116002)(102836003)(74316002)(305945005)(3846002)(33656002)(53936002)(81166006)(8676002)(7696004)(5660300001)(53546010)(66066001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR04MB1316; H:AM3PR04MB306.eurprd04.prod.outlook.com; FPR:; SPF:None; MLV:ovrnspm; PTR:InfoNoRecords; LANG:en; x-ms-office365-filtering-correlation-id: 016d3db5-6943-4f6d-5aeb-08d4c1c62145 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254075)(48565401081)(300000503095)(300135400095)(2017052603031)(201703131423075)(201703031133081)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:AM3PR04MB1316; x-ms-traffictypediagnostic: AM3PR04MB1316: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(133145235818549)(236129657087228)(9452136761055)(258649278758335)(247924648384137); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(93006095)(93001095)(3002001)(10201501046)(100000703101)(100105400095)(6055026)(6041248)(20161123564025)(20161123555025)(20161123558100)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM3PR04MB1316; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM3PR04MB1316; x-forefront-prvs: 035748864E spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jul 2017 03:46:47.0420 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR04MB1316 Sender: linux-clk-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > -----Original Message----- > From: Stephen Boyd [mailto:sboyd@codeaurora.org] > Sent: Saturday, July 01, 2017 8:56 AM > To: Dong Aisheng > Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com; > shawnguo@kernel.org; Anson Huang; Jacky Bai > Subject: Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk > support > > On 06/20, Dong Aisheng wrote: > > Hi Stephen, > > > > On Mon, Jun 19, 2017 at 06:45:12PM -0700, Stephen Boyd wrote: > > > On 05/15, Dong Aisheng wrote: > > > > --- > > > > drivers/clk/clk-divider.c | 2 ++ > > > > include/linux/clk-provider.h | 4 ++++ > > > > 2 files changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > > > > index 96386ff..f78ba7a 100644 > > > > --- a/drivers/clk/clk-divider.c > > > > +++ b/drivers/clk/clk-divider.c > > > > @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct > > > > clk_hw *hw, unsigned long parent_rate, > > > > > > > > div = _get_div(table, val, flags, divider->width); > > > > if (!div) { > > > > + if (flags & CLK_DIVIDER_ZERO_GATE) > > > > + return 0; > > > > WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO), > > > > > > Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off doesn't > > > mean the rate is 0. The divider is just disabled, so we would > > > consider the rate as whatever the parent is, which is what this code > > > does before this patch. Similarly, we don't do anything about gate > > > clocks and return a rate of 0 when they're disabled. > > > > > > > The semantic of CLK_DIVIDER_ALLOW_ZERO seems a bit different. > > > > See below definition: > > * CLK_DIVIDER_ALLOW_ZERO - Allow zero divisors. For dividers which > have > > * CLK_DIVIDER_ONE_BASED set, it is possible to end up with a zero > divisor. > > * Some hardware implementations gracefully handle this case and > allow a > > * zero divisor by not modifying their input clock > > * (divide by one / bypass). > > > > zero divisor is simply as divide by one or bypass which is supported > > by hardware. > > > > But it's not true for this hardware. > > > > If we consider the rate as whatever the parent is if divider is zero, > > we may got an issue like below: > > e.g. > > Assuming spll_bus_clk divider is 0x0 and it may be enabled by users > > directly without setting a rate first. > > > > Then the clock tree looks like: > > ... > > spll_pfd0 1 1 500210526 0 0 > > spll_pfd_sel 1 1 500210526 0 0 > > spll_sel 1 1 500210526 0 0 > > spll_bus_clk 1 1 500210526 0 0 > > > > But the spll_bus_clk clock rate actually is wrong and it's even not > > enabled, not like CLK_DIVIDER_ALLOW_ZERO which zero divider means > simply bypass. > > > > So for this case, we probably can't simply assume zero divider rate as > > its parent, it is actually set to 0 in hw, although it's something > > like gate, but a bit different from gate as the normal gate does not > > affect divider where you can keep the rate. > > > > How would you suggest for this? > > > > It seems that set_rate() and enable/disable are conflated here. > From what you describe, it sounds like the clk is considered off when the > divider value is zero, and it's on when the divider value is non-zero. > > I'd suggest you make it so this clk supports enable/disable and set_rate > with the same register. Something like, set rate when the clk is disabled > will cache the rate request and only when the clk is enabled will the > driver actually program the hardware to have the requested divider value. > Similarly, when the clk is disabled we'll write a 0 there, but when the > clk is enabled we'll restore whatever rate (divider) was chosen last. > > It does mean that recalc rate will be sort of odd, because when the clk > is off it will return 0, and when the clk is on it will return the right > rate. So to make things work, we'll need to return the cached rate in > recalc rate when the clk is off and read the hardware when the clk is on. > Probably an if register == > 0 then lookup in cache, otherwise do normal division. > Well, good suggestions to me. It makes the implementation of this clock type more comprehensive. It seems we still need keep CLK_DIVIDER_ZERO_GATE to distinguish with others. The change for recacle_rate may looks like: 0 rate as there's still no set_rate happened. If any issue, please let me know. Regards Dong Aisheng > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project --- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index f78ba7a..7364ac4 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -125,8 +125,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate, div = _get_div(table, val, flags, divider->width); if (!div) { + if ((flags & CLK_DIVIDER_ZERO_GATE) && clk_hw_is_enabled(hw)) + return divider->cached_rate; + WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO), "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n", clk_hw_get_name(hw)); And for the initial disabled state (div = 0), the calc_rate will still return