From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ying.Liu@freescale.com (Liu Ying) Date: Thu, 12 Feb 2015 18:39:45 +0800 Subject: [PATCH RFC v9 01/20] clk: divider: Correct parent clk round rate if no bestdiv is normally found In-Reply-To: <20150212093356.GR12209@pengutronix.de> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <1423720903-24806-2-git-send-email-Ying.Liu@freescale.com> <20150212093356.GR12209@pengutronix.de> Message-ID: <20150212103944.GA1290@victor> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Feb 12, 2015 at 10:33:56AM +0100, Sascha Hauer wrote: > On Thu, Feb 12, 2015 at 02:01:24PM +0800, Liu Ying wrote: > > If no best divider is normally found, we will try to use the maximum divider. > > We should not set the parent clock rate to be 1Hz by force for being rounded. > > Instead, we should take the maximum divider as a base and calculate a correct > > parent clock rate for being rounded. > > Please add an explanation why you think the current code is wrong and > what this actually fixes, maybe an example? The MIPI DSI panel's pixel clock rate is 26.4MHz and it's derived from PLL5 on the MX6DL SabreSD board. These are the clock tree summaries with or without the patch applied: 1) With the patch applied: pll5_bypass_src 1 1 24000000 0 0 pll5 1 1 844800048 0 0 pll5_bypass 1 1 844800048 0 0 pll5_video 1 1 844800048 0 0 pll5_post_div 1 1 211200012 0 0 pll5_video_div 1 1 211200012 0 0 ipu1_di0_pre_sel 1 1 211200012 0 0 ipu1_di0_pre 1 1 26400002 0 0 ipu1_di0_sel 1 1 26400002 0 0 ipu1_di0 1 1 26400002 0 0 2) Without the patch applied: pll5_bypass_src 1 1 24000000 0 0 pll5 1 1 648000000 0 0 pll5_bypass 1 1 648000000 0 0 pll5_video 1 1 648000000 0 0 pll5_post_div 1 1 162000000 0 0 pll5_video_div 1 1 40500000 0 0 ipu1_di0_pre_sel 1 1 40500000 0 0 ipu1_di0_pre 1 1 20250000 0 0 ipu1_di0_sel 1 1 20250000 0 0 ipu1_di0 1 1 20250000 0 0 > > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > > index c0a842b..f641d4b 100644 > > --- a/drivers/clk/clk-divider.c > > +++ b/drivers/clk/clk-divider.c > > @@ -311,7 +311,8 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, > > > > if (!bestdiv) { > > bestdiv = _get_maxdiv(divider); > > - *best_parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), 1); > > + *best_parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), > > + MULT_ROUND_UP(rate, bestdiv)); > > When getting into the if(!bestdiv) it means that the lowest possible > rate we can archieve is still higher than the target rate, so setting > the parent rate as low as possible seems sane to me. Why do you think > this is wrong? In which case this even makes a difference? We still should take the little left chance to get a closest target clock rate we want(26.4MHz, in the example case above), so it looks that the lowest parent clock rate for being rounded should be MULT_ROUND_UP(rate, bestdiv) instead of 1Hz. Regards, Liu Ying > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liu Ying Subject: Re: [PATCH RFC v9 01/20] clk: divider: Correct parent clk round rate if no bestdiv is normally found Date: Thu, 12 Feb 2015 18:39:45 +0800 Message-ID: <20150212103944.GA1290@victor> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <1423720903-24806-2-git-send-email-Ying.Liu@freescale.com> <20150212093356.GR12209@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20150212093356.GR12209@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sascha Hauer Cc: stefan.wahren@i2se.com, devicetree@vger.kernel.org, linux@arm.linux.org.uk, mturquette@linaro.org, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, a.hajda@samsung.com, kernel@pengutronix.de, andy.yan@rock-chips.com, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org T24gVGh1LCBGZWIgMTIsIDIwMTUgYXQgMTA6MzM6NTZBTSArMDEwMCwgU2FzY2hhIEhhdWVyIHdy b3RlOgo+IE9uIFRodSwgRmViIDEyLCAyMDE1IGF0IDAyOjAxOjI0UE0gKzA4MDAsIExpdSBZaW5n IHdyb3RlOgo+ID4gSWYgbm8gYmVzdCBkaXZpZGVyIGlzIG5vcm1hbGx5IGZvdW5kLCB3ZSB3aWxs IHRyeSB0byB1c2UgdGhlIG1heGltdW0gZGl2aWRlci4KPiA+IFdlIHNob3VsZCBub3Qgc2V0IHRo ZSBwYXJlbnQgY2xvY2sgcmF0ZSB0byBiZSAxSHogYnkgZm9yY2UgZm9yIGJlaW5nIHJvdW5kZWQu Cj4gPiBJbnN0ZWFkLCB3ZSBzaG91bGQgdGFrZSB0aGUgbWF4aW11bSBkaXZpZGVyIGFzIGEgYmFz ZSBhbmQgY2FsY3VsYXRlIGEgY29ycmVjdAo+ID4gcGFyZW50IGNsb2NrIHJhdGUgZm9yIGJlaW5n IHJvdW5kZWQuCj4gCj4gUGxlYXNlIGFkZCBhbiBleHBsYW5hdGlvbiB3aHkgeW91IHRoaW5rIHRo ZSBjdXJyZW50IGNvZGUgaXMgd3JvbmcgYW5kCj4gd2hhdCB0aGlzIGFjdHVhbGx5IGZpeGVzLCBt YXliZSBhbiBleGFtcGxlPwoKVGhlIE1JUEkgRFNJIHBhbmVsJ3MgcGl4ZWwgY2xvY2sgcmF0ZSBp cyAyNi40TUh6IGFuZCBpdCdzIGRlcml2ZWQgZnJvbSBQTEw1IG9uCnRoZSBNWDZETCBTYWJyZVNE IGJvYXJkLgoKVGhlc2UgYXJlIHRoZSBjbG9jayB0cmVlIHN1bW1hcmllcyB3aXRoIG9yIHdpdGhv dXQgdGhlIHBhdGNoIGFwcGxpZWQ6CjEpIFdpdGggdGhlIHBhdGNoIGFwcGxpZWQ6CnBsbDVfYnlw YXNzX3NyYyAgICAgICAgICAgICAgICAgICAgICAgMSAgICAgICAgICAgIDEgICAgMjQwMDAwMDAg ICAgICAgICAgMCAwCiAgIHBsbDUgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgMSAgICAg ICAgICAgIDEgICA4NDQ4MDAwNDggICAgICAgICAgMCAwCiAgICAgIHBsbDVfYnlwYXNzICAgICAg ICAgICAgICAgICAgICAgMSAgICAgICAgICAgIDEgICA4NDQ4MDAwNDggICAgICAgICAgMCAwCiAg ICAgICAgIHBsbDVfdmlkZW8gICAgICAgICAgICAgICAgICAgMSAgICAgICAgICAgIDEgICA4NDQ4 MDAwNDggICAgICAgICAgMCAwCiAgICAgICAgICAgIHBsbDVfcG9zdF9kaXYgICAgICAgICAgICAg MSAgICAgICAgICAgIDEgICAyMTEyMDAwMTIgICAgICAgICAgMCAwCiAgICAgICAgICAgICAgIHBs bDVfdmlkZW9fZGl2ICAgICAgICAgICAxICAgICAgICAgICAgMSAgIDIxMTIwMDAxMiAgICAgICAg MCAwCiAgICAgICAgICAgICAgICAgIGlwdTFfZGkwX3ByZV9zZWwgICAgICAgICAgIDEgICAgICAg ICAgICAxICAgMjExMjAwMDEyICAgMCAwCiAgICAgICAgICAgICAgICAgICAgIGlwdTFfZGkwX3By ZSAgICAgICAgICAgMSAgICAgICAgICAgIDEgICAgMjY0MDAwMDIgICAgMCAwCiAgICAgICAgICAg ICAgICAgICAgICAgIGlwdTFfZGkwX3NlbCAgICAgICAgICAgMSAgICAgICAgICAgIDEgICAgMjY0 MDAwMDIgMCAwCiAgICAgICAgICAgICAgICAgICAgICAgICAgIGlwdTFfZGkwICAgICAgICAgICAx ICAgICAgICAgICAgMSAgICAyNjQwMDAwMiAgMCAwCgoyKSBXaXRob3V0IHRoZSBwYXRjaCBhcHBs aWVkOgpwbGw1X2J5cGFzc19zcmMgICAgICAgICAgICAgICAgICAgICAgIDEgICAgICAgICAgICAx ICAgIDI0MDAwMDAwICAgICAgICAgIDAgMAogICBwbGw1ICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIDEgICAgICAgICAgICAxICAgNjQ4MDAwMDAwICAgICAgICAgIDAgMAogICAgICBwbGw1 X2J5cGFzcyAgICAgICAgICAgICAgICAgICAgIDEgICAgICAgICAgICAxICAgNjQ4MDAwMDAwICAg ICAgICAgIDAgMAogICAgICAgICBwbGw1X3ZpZGVvICAgICAgICAgICAgICAgICAgIDEgICAgICAg ICAgICAxICAgNjQ4MDAwMDAwICAgICAgICAgIDAgMAogICAgICAgICAgICBwbGw1X3Bvc3RfZGl2 ICAgICAgICAgICAgIDEgICAgICAgICAgICAxICAgMTYyMDAwMDAwICAgICAgICAgIDAgMAogICAg ICAgICAgICAgICBwbGw1X3ZpZGVvX2RpdiAgICAgICAgICAgMSAgICAgICAgICAgIDEgICAgNDA1 MDAwMDAgICAgICAgIDAgMAogICAgICAgICAgICAgICAgICBpcHUxX2RpMF9wcmVfc2VsICAgICAg ICAgICAxICAgICAgICAgICAgMSAgICA0MDUwMDAwMCAgIDAgMAogICAgICAgICAgICAgICAgICAg ICBpcHUxX2RpMF9wcmUgICAgICAgICAgIDEgICAgICAgICAgICAxICAgIDIwMjUwMDAwICAgIDAg MAogICAgICAgICAgICAgICAgICAgICAgICBpcHUxX2RpMF9zZWwgICAgICAgICAgIDEgICAgICAg ICAgICAxICAgIDIwMjUwMDAwIDAgMAogICAgICAgICAgICAgICAgICAgICAgICAgICBpcHUxX2Rp MCAgICAgICAgICAgMSAgICAgICAgICAgIDEgICAgMjAyNTAwMDAgIDAgMAoKPiAKPiA+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2Nsay9jbGstZGl2aWRlci5jIGIvZHJpdmVycy9jbGsvY2xrLWRpdmlk ZXIuYwo+ID4gaW5kZXggYzBhODQyYi4uZjY0MWQ0YiAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMv Y2xrL2Nsay1kaXZpZGVyLmMKPiA+ICsrKyBiL2RyaXZlcnMvY2xrL2Nsay1kaXZpZGVyLmMKPiA+ IEBAIC0zMTEsNyArMzExLDggQEAgc3RhdGljIGludCBjbGtfZGl2aWRlcl9iZXN0ZGl2KHN0cnVj dCBjbGtfaHcgKmh3LCB1bnNpZ25lZCBsb25nIHJhdGUsCj4gPiAgCj4gPiAgCWlmICghYmVzdGRp dikgewo+ID4gIAkJYmVzdGRpdiA9IF9nZXRfbWF4ZGl2KGRpdmlkZXIpOwo+ID4gLQkJKmJlc3Rf cGFyZW50X3JhdGUgPSBfX2Nsa19yb3VuZF9yYXRlKF9fY2xrX2dldF9wYXJlbnQoaHctPmNsayks IDEpOwo+ID4gKwkJKmJlc3RfcGFyZW50X3JhdGUgPSBfX2Nsa19yb3VuZF9yYXRlKF9fY2xrX2dl dF9wYXJlbnQoaHctPmNsayksCj4gPiArCQkJCQkJTVVMVF9ST1VORF9VUChyYXRlLCBiZXN0ZGl2 KSk7Cj4gCj4gV2hlbiBnZXR0aW5nIGludG8gdGhlIGlmKCFiZXN0ZGl2KSBpdCBtZWFucyB0aGF0 IHRoZSBsb3dlc3QgcG9zc2libGUKPiByYXRlIHdlIGNhbiBhcmNoaWV2ZSBpcyBzdGlsbCBoaWdo ZXIgdGhhbiB0aGUgdGFyZ2V0IHJhdGUsIHNvIHNldHRpbmcKPiB0aGUgcGFyZW50IHJhdGUgYXMg bG93IGFzIHBvc3NpYmxlIHNlZW1zIHNhbmUgdG8gbWUuIFdoeSBkbyB5b3UgdGhpbmsKPiB0aGlz IGlzIHdyb25nPyBJbiB3aGljaCBjYXNlIHRoaXMgZXZlbiBtYWtlcyBhIGRpZmZlcmVuY2U/CgpX ZSBzdGlsbCBzaG91bGQgdGFrZSB0aGUgbGl0dGxlIGxlZnQgY2hhbmNlIHRvIGdldCBhIGNsb3Nl c3QgdGFyZ2V0IGNsb2NrCnJhdGUgd2Ugd2FudCgyNi40TUh6LCBpbiB0aGUgZXhhbXBsZSBjYXNl IGFib3ZlKSwgc28gaXQgbG9va3MgdGhhdCB0aGUgbG93ZXN0CnBhcmVudCBjbG9jayByYXRlIGZv ciBiZWluZyByb3VuZGVkIHNob3VsZCBiZSBNVUxUX1JPVU5EX1VQKHJhdGUsIGJlc3RkaXYpCmlu c3RlYWQgb2YgMUh6LgoKUmVnYXJkcywKTGl1IFlpbmcKCj4gCj4gU2FzY2hhCj4gCj4gLS0gCj4g UGVuZ3V0cm9uaXggZS5LLiAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgICAgICAgICAg ICAgICAgICAgICAgICAgIHwKPiBJbmR1c3RyaWFsIExpbnV4IFNvbHV0aW9ucyAgICAgICAgICAg ICAgICAgfCBodHRwOi8vd3d3LnBlbmd1dHJvbml4LmRlLyAgfAo+IFBlaW5lciBTdHIuIDYtOCwg MzExMzcgSGlsZGVzaGVpbSwgR2VybWFueSB8IFBob25lOiArNDktNTEyMS0yMDY5MTctMCAgICB8 Cj4gQW10c2dlcmljaHQgSGlsZGVzaGVpbSwgSFJBIDI2ODYgICAgICAgICAgIHwgRmF4OiAgICs0 OS01MTIxLTIwNjkxNy01NTU1IHwKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755455AbbBLKfA (ORCPT ); Thu, 12 Feb 2015 05:35:00 -0500 Received: from mail-by2on0118.outbound.protection.outlook.com ([207.46.100.118]:12527 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752254AbbBLKe6 (ORCPT ); Thu, 12 Feb 2015 05:34:58 -0500 Date: Thu, 12 Feb 2015 18:39:45 +0800 From: Liu Ying To: Sascha Hauer CC: , , , , , , , , , , Subject: Re: [PATCH RFC v9 01/20] clk: divider: Correct parent clk round rate if no bestdiv is normally found Message-ID: <20150212103944.GA1290@victor> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <1423720903-24806-2-git-send-email-Ying.Liu@freescale.com> <20150212093356.GR12209@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20150212093356.GR12209@pengutronix.de> User-Agent: Mutt/1.5.23 (2014-03-12) X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=Ying.Liu@freescale.com; lists.infradead.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(51704005)(2950100001)(15975445007)(97756001)(57986006)(110136001)(33656002)(86362001)(575784001)(23726002)(92566002)(50986999)(54356999)(77096005)(33716001)(87936001)(76176999)(106466001)(19580395003)(6806004)(77156002)(62966003)(85426001)(83506001)(46102003)(104016003)(46406003)(105606002)(50466002)(47776003)(217873001);DIR:OUT;SFP:1102;SCL:1;SRVR:CY1PR0301MB0634;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;MLV:sfv;LANG:en; X-Microsoft-Antispam: UriScan:; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0634; X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004);SRVR:CY1PR0301MB0634; X-Forefront-PRVS: 0485417665 X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0634; X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Feb 2015 10:34:55.4851 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0301MB0634 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 12, 2015 at 10:33:56AM +0100, Sascha Hauer wrote: > On Thu, Feb 12, 2015 at 02:01:24PM +0800, Liu Ying wrote: > > If no best divider is normally found, we will try to use the maximum divider. > > We should not set the parent clock rate to be 1Hz by force for being rounded. > > Instead, we should take the maximum divider as a base and calculate a correct > > parent clock rate for being rounded. > > Please add an explanation why you think the current code is wrong and > what this actually fixes, maybe an example? The MIPI DSI panel's pixel clock rate is 26.4MHz and it's derived from PLL5 on the MX6DL SabreSD board. These are the clock tree summaries with or without the patch applied: 1) With the patch applied: pll5_bypass_src 1 1 24000000 0 0 pll5 1 1 844800048 0 0 pll5_bypass 1 1 844800048 0 0 pll5_video 1 1 844800048 0 0 pll5_post_div 1 1 211200012 0 0 pll5_video_div 1 1 211200012 0 0 ipu1_di0_pre_sel 1 1 211200012 0 0 ipu1_di0_pre 1 1 26400002 0 0 ipu1_di0_sel 1 1 26400002 0 0 ipu1_di0 1 1 26400002 0 0 2) Without the patch applied: pll5_bypass_src 1 1 24000000 0 0 pll5 1 1 648000000 0 0 pll5_bypass 1 1 648000000 0 0 pll5_video 1 1 648000000 0 0 pll5_post_div 1 1 162000000 0 0 pll5_video_div 1 1 40500000 0 0 ipu1_di0_pre_sel 1 1 40500000 0 0 ipu1_di0_pre 1 1 20250000 0 0 ipu1_di0_sel 1 1 20250000 0 0 ipu1_di0 1 1 20250000 0 0 > > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > > index c0a842b..f641d4b 100644 > > --- a/drivers/clk/clk-divider.c > > +++ b/drivers/clk/clk-divider.c > > @@ -311,7 +311,8 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, > > > > if (!bestdiv) { > > bestdiv = _get_maxdiv(divider); > > - *best_parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), 1); > > + *best_parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), > > + MULT_ROUND_UP(rate, bestdiv)); > > When getting into the if(!bestdiv) it means that the lowest possible > rate we can archieve is still higher than the target rate, so setting > the parent rate as low as possible seems sane to me. Why do you think > this is wrong? In which case this even makes a difference? We still should take the little left chance to get a closest target clock rate we want(26.4MHz, in the example case above), so it looks that the lowest parent clock rate for being rounded should be MULT_ROUND_UP(rate, bestdiv) instead of 1Hz. Regards, Liu Ying > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |