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 Content-Length: 3971 Lines: 77 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 | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/