From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 37E6DC56202 for ; Wed, 18 Nov 2020 01:50:05 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A7E3124248 for ; Wed, 18 Nov 2020 01:50:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="gMynjB2n"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="GFbG78XC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A7E3124248 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:Date:To:From:Subject:References: In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=snklWOQZWrlo/L91WwYNS/V7IALObZeD36aScHjCIjQ=; b=gMynjB2ny+DlSfLCwB8i/+zMA mWe6o1KLArMW6MLNRFFNJbCFetZDCvqFiciKoPLdGAM+IWayr0xKMpaIxuH0eMhNqlZWmCNcHeV9s dA2X2cVup7HyhBcfsyzG/jJzodfjK1Uq0qwbDoyYi/Wm15Pi5rAwSL2BgmzCOSWR1q6p2v8pzMzWd rXZ99jWSk5lciEc8f5nayLD3y1DDoVLcRSY1kX8mIKAkusZ3vSmWu6B+ocLbh+KdPJB0mXUqsQ8TZ RCwO8pIwfVIfUD7EhAulmAOSd+bjGvWScZS0wVfkAOdn8jJkt6xCQQHZDlzx/of/UJSxMgo/9A7Hx ySU9dRDoQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kfCbI-0002Ia-U6; Wed, 18 Nov 2020 01:49:36 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kfCbF-0002HU-HT for linux-arm-kernel@lists.infradead.org; Wed, 18 Nov 2020 01:49:34 +0000 Received: from kernel.org (unknown [104.132.1.79]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 44ECA241A7; Wed, 18 Nov 2020 01:49:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1605664172; bh=M6DfrIdy95Ghs9mCFBL8Ngh2V0KHo7z65Isd9IgYS3s=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=GFbG78XCZWOi7mQ937XcvONvdNNLDydGM450RdgNzpN3caAYSRmfTf1XerRwsaW4v /Dq4pOCMsDsgx6QaqwNie1Vh5xT3xu/K5iXqy4/kNzP3h9O6j7MiQ10Rd/fbCJ/XCm WxDk55uYFKWryDlqwJqlGunKSUMUCm0OLHbiZZSk= MIME-Version: 1.0 In-Reply-To: <24d975ca-1942-5f7f-ae89-7b572f48812c@microchip.com> References: <1604655988-353-1-git-send-email-claudiu.beznea@microchip.com> <1604655988-353-7-git-send-email-claudiu.beznea@microchip.com> <160538849947.60232.12002724470272520124@swboyd.mtv.corp.google.com> <24d975ca-1942-5f7f-ae89-7b572f48812c@microchip.com> Subject: Re: [PATCH v4 06/11] clk: at91: clk-sam9x60-pll: allow runtime changes for pll From: Stephen Boyd To: Claudiu.Beznea@microchip.com, Ludovic.Desroches@microchip.com, Nicolas.Ferre@microchip.com, alexandre.belloni@bootlin.com, mturquette@baylibre.com, robh+dt@kernel.org Date: Tue, 17 Nov 2020 17:49:30 -0800 Message-ID: <160566417078.60232.18106288530854376790@swboyd.mtv.corp.google.com> User-Agent: alot/0.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201117_204933_736519_F21EFF52 X-CRM114-Status: GOOD ( 31.47 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eugen.Hristev@microchip.com, devicetree@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Claudiu.Beznea@microchip.com (2020-11-16 03:24:54) > > > On 14.11.2020 23:14, Stephen Boyd wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Quoting Claudiu Beznea (2020-11-06 01:46:23) > >> diff --git a/drivers/clk/at91/clk-sam9x60-pll.c b/drivers/clk/at91/clk-sam9x60-pll.c > >> index 78f458a7b2ef..6fe5d8530a0c 100644 > >> --- a/drivers/clk/at91/clk-sam9x60-pll.c > >> +++ b/drivers/clk/at91/clk-sam9x60-pll.c > >> @@ -225,8 +225,51 @@ static int sam9x60_frac_pll_set_rate(struct clk_hw *hw, unsigned long rate, > >> unsigned long parent_rate) > >> { > >> struct sam9x60_pll_core *core = to_sam9x60_pll_core(hw); > >> + struct sam9x60_frac *frac = to_sam9x60_frac(core); > >> + struct regmap *regmap = core->regmap; > >> + unsigned long irqflags, clkflags = clk_hw_get_flags(hw); > >> + unsigned int val, cfrac, cmul; > >> + long ret; > >> + > >> + ret = sam9x60_frac_pll_compute_mul_frac(core, rate, parent_rate, true); > >> + if (ret <= 0 || (clkflags & CLK_SET_RATE_GATE)) > > > > Is this function being called when the clk is enabled and it has the > > CLK_SET_RATE_GATE flag set? > > Yes, this function could be called when CLK_SET_RATE_GATE is set. > On SAMA7G5 there are multiple PLL blocks of the same type. All these PLLs > are controlled by clk-sam9x60-pll.c driver. One of this PLL block fed the > CPU who's frequency could be changed at run time. At the same time there > are PLLs that fed hardware block not glitch free aware or that we don't > want to allow the rate change (this is the case of SAM9X60's CPU PLL, or > the DDR PLL on SAMA7G5). > > I'm confused why this driver needs to check > > this flag. > > Because we have multiple PLLs of the same type, some of them feed hardware > blocks that are glitch free aware of these PLLs' frequencies changes, some > feed hardware blocks that are not glitch free aware of PLLs' frequencies > changes or for some of them we don't want the frequency changes at all. Can we have different clk_ops for the different types of PLLs? It looks like the flag is being used to overload this function to do different things depending on how the flags are set. What happens if we decide to change the semantics of this clk flag? How does it map to this driver? Ideally this driver shouldn't need to worry about this flag, at least not in this function, except to figure out if it should do something different like not write the value to the hardware or something like that. The flag indicates to the clk framework that this clk should be gated when clk_set_rate() is called on it. The driver should be able to figure out that the clk is disabled by reading the hardware here and checking the enable state, or it could just have different clk_ops for the different type of PLL and do something different without checking the flag. Either way, checking the flag looks wrong. > >> - .c = 1, > >> + .f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE, > > > > Please indicate why clks are critical. > > Sure! I'll do it in next version. I chose it like this because they are > feeding critical parts of the system like CPU or memory. > > > Whenever the CLK_IS_CRITICAL flag > > is used we should have a comment indicating why. > > I was not aware of this rule. I'll update the code accordingly. Sorry. I should put a document comment next to the flag. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel