From patchwork Wed Nov 6 16:19:11 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 3148281 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 6E435BEEB2 for ; Wed, 6 Nov 2013 16:21:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B395520443 for ; Wed, 6 Nov 2013 16:21:13 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 242E32043C for ; Wed, 6 Nov 2013 16:21:12 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ve5qo-0006Sa-G6; Wed, 06 Nov 2013 16:21:02 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ve5qj-00050g-3i; Wed, 06 Nov 2013 16:20:57 +0000 Received: from [2002:4e20:1eda::1] (helo=caramon.arm.linux.org.uk) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ve5qf-0004zc-Pf for linux-arm-kernel@lists.infradead.org; Wed, 06 Nov 2013 16:20:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=7HBHGz93tW+rrirV8tinFcz1k8Ycru6ArpgtD4Gmdx0=; b=AhZBgkxg0jeuYJ9R2XTljvIbqEDzD+sdzweIhXUZirBYcprYMvPyCm2oemuRoSMOItA7QirVAxx2y+rOfc2Dn5wjdsSK1sGydVGfLnsBkeaeA8giMr/d21utDuu5D/tmiAQmuwkPRy3Mz+hC4QyRf0I3FzNSfWuhO/u+EDjkSzg=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:33621) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1Ve5p4-00029S-7Y; Wed, 06 Nov 2013 16:19:14 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Ve5p2-0004KV-Cd; Wed, 06 Nov 2013 16:19:12 +0000 Date: Wed, 6 Nov 2013 16:19:11 +0000 From: Russell King - ARM Linux To: Tomi Valkeinen Subject: Re: [PATCH] clk: divider: fix rate calculation for fractional rates Message-ID: <20131106161911.GA16735@n2100.arm.linux.org.uk> References: <1383736008-22764-1-git-send-email-tomi.valkeinen@ti.com> <20131106111534.GW16735@n2100.arm.linux.org.uk> <527A2C9C.4080409@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <527A2C9C.4080409@ti.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131106_112054_454910_4DF9D9D1 X-CRM114-Status: GOOD ( 23.46 ) X-Spam-Score: -1.2 (-) Cc: "Kristo, Tero" , Mike Turquette , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Nov 06, 2013 at 01:48:44PM +0200, Tomi Valkeinen wrote: > On 2013-11-06 13:15, Russell King - ARM Linux wrote: > > On Wed, Nov 06, 2013 at 01:06:48PM +0200, Tomi Valkeinen wrote: > >> This means that the following code works a bit oddly: > >> > >> rate = clk_round_rate(clk, 123428572); > >> clk_set_rate(clk, rate); > > > > You're right, but the above sequence is quite a crass thing to do. Why? > > Do you mean that you think the fix is right, but the above example > sequence is silly, or that the fix is not needed either? I think a fix _is) required, because: rate = clk_get_rate(clk); clk_set_rate(clk, rate); assert(clk_get_rate(clk) == rate); If not, there's something quite wrong. However, I am saying that the sequence you provided was nevertheless silly - I've seen it in real code in the kernel, which is why I've commented about it. > Ok, if defined like that, then the current behavior is logical. > > The comment in clk.h says "adjust a rate to the exact rate a clock can > provide", which does not contradict with what you said, but doesn't > really confirm it either. If I get "the exact rate a clock can provide" > I don't see why I can't use that exact clock rate for clk_set_rate. > Maybe the comment should be improved to state explicitly what it does. > > However, how about the following sequence: > > clk_set_rate(clk, 123428572); > rate = clk_get_rate(clk); > clk_set_rate(clk, rate); > > I didn't test that but it should result in the clock first set to > 123428571, and then to 108000000. Obviously pointless if done exactly > like that, but I don't see why the above code sequence is wrong, yet it > gives a bit surprising result. Does this help clarify it? --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -228,6 +228,20 @@ * @clk: clock source * @rate: desired clock rate in Hz * + * This answers the question "if I were to pass @rate to clk_set_rate(), + * what clock rate would I end up with?" without changing the hardware + * in any way. In other words: + * + * rate = clk_round_rate(clk, r); + * + * and: + * + * clk_set_rate(clk, r); + * rate = clk_get_rate(clk); + * + * are equivalent except the former does not modify the clock hardware + * in any way. + * * Returns rounded clock rate in Hz, or negative errno. */ long clk_round_rate(struct clk *clk, unsigned long rate);