From patchwork Wed Jul 2 04:35:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shawn Guo X-Patchwork-Id: 4463481 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 42EC0BEEAA for ; Wed, 2 Jul 2014 04:37:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 11A92202FE for ; Wed, 2 Jul 2014 04:37:54 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 20F40202DD for ; Wed, 2 Jul 2014 04:37:52 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1X2CGp-0003X8-0i; Wed, 02 Jul 2014 04:35:47 +0000 Received: from mail-by2lp0239.outbound.protection.outlook.com ([207.46.163.239] helo=na01-by2-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1X2CGm-0003Un-06 for linux-arm-kernel@lists.infradead.org; Wed, 02 Jul 2014 04:35:44 +0000 Received: from BLUPR03MB248.namprd03.prod.outlook.com (10.255.213.26) by BLUPR03MB002.namprd03.prod.outlook.com (10.255.208.36) with Microsoft SMTP Server (TLS) id 15.0.980.8; Wed, 2 Jul 2014 04:35:20 +0000 Received: from BY2PR03CA067.namprd03.prod.outlook.com (10.141.249.40) by BLUPR03MB248.namprd03.prod.outlook.com (10.255.213.26) with Microsoft SMTP Server (TLS) id 15.0.980.8; Wed, 2 Jul 2014 04:35:19 +0000 Received: from BL2FFO11FD055.protection.gbl (2a01:111:f400:7c09::159) by BY2PR03CA067.outlook.office365.com (2a01:111:e400:2c5d::40) with Microsoft SMTP Server (TLS) id 15.0.974.11 via Frontend Transport; Wed, 2 Jul 2014 04:35:18 +0000 Received: from az84smr01.freescale.net (192.88.158.2) by BL2FFO11FD055.mail.protection.outlook.com (10.173.161.183) with Microsoft SMTP Server (TLS) id 15.0.969.12 via Frontend Transport; Wed, 2 Jul 2014 04:35:17 +0000 Received: from dragon ([10.192.185.81]) by az84smr01.freescale.net (8.14.3/8.14.0) with ESMTP id s624ZCOe031652; Tue, 1 Jul 2014 21:35:13 -0700 Date: Wed, 2 Jul 2014 12:35:09 +0800 From: Shawn Guo To: Fabio Estevam Subject: Re: [PATCH 4/5] ARM: imx: clk-gate2: Use post decrement for share_count Message-ID: <20140702043508.GA16176@dragon> References: <1404194129-25543-1-git-send-email-festevam@gmail.com> <1404194129-25543-4-git-send-email-festevam@gmail.com> <20140701115252.GM14471@dragon> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:192.88.158.2; CTRY:US; IPV:CAL; IPV:NLI; EFV:NLI; SFV:NSPM; SFS:(6009001)(199002)(377454003)(189002)(51704005)(24454002)(87936001)(99396002)(50986999)(83072002)(85852003)(23726002)(76482001)(50466002)(21056001)(74502001)(97736001)(97756001)(76176999)(106466001)(54356999)(81156004)(107046002)(105606002)(4396001)(33656002)(68736004)(46406003)(79102001)(57986006)(33716001)(93886003)(85306003)(104016002)(95666004)(69596002)(92566001)(92726001)(81342001)(19580395003)(86362001)(46102001)(74662001)(83506001)(77982001)(31966008)(84676001)(26826002)(102836001)(6806004)(19580405001)(64706001)(20776003)(47776003)(83322001)(81542001)(80022001)(44976005)(1411001)(309714004); DIR:OUT; SFP:; SCL:1; SRVR:BLUPR03MB248; H:az84smr01.freescale.net; FPR:; MLV:ovrnspm; PTR:InfoDomainNonexistent; MX:1; LANG:en; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID: X-Forefront-PRVS: 0260457E99 Received-SPF: Fail (: domain of freescale.com does not designate 192.88.158.2 as permitted sender) receiver=; client-ip=192.88.158.2; helo=az84smr01.freescale.net; Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=Shawn.Guo@freescale.com; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID: X-OriginatorOrg: freescale.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140701_213544_114123_2D313E2A X-CRM114-Status: GOOD ( 23.77 ) X-Spam-Score: -1.4 (-) Cc: Fabio Estevam , Nicolin Chen , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 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=-1.9 required=5.0 tests=BAYES_00, T_RP_MATCHES_RCVD, 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 Tue, Jul 01, 2014 at 02:44:40PM -0300, Fabio Estevam wrote: > On Tue, Jul 1, 2014 at 8:52 AM, Shawn Guo wrote: > > >> --- a/arch/arm/mach-imx/clk-gate2.c > >> +++ b/arch/arm/mach-imx/clk-gate2.c > >> @@ -67,7 +67,7 @@ static void clk_gate2_disable(struct clk_hw *hw) > >> > >> spin_lock_irqsave(gate->lock, flags); > >> > >> - if (gate->share_count && --(*gate->share_count) > 0) > >> + if (gate->share_count && (*gate->share_count)-- > 0) > > > > The change makes no sense. Let's say that clk_gate2_disable() gets > > called with share_count being 1. In this case, we should access > > register to gate the clock, right? > > If share_count is 1 it means that someone else is using the clock and > we can't disable it. You do not really know it's someone else or itself. > > Please try running the series without this patch. When the extern > audio clock is enabled, share_count is 1. Later the the spdif clock > (the one that is shared with extern audio clock) is disabled by the > CCF as it is not used, which makes the extern audio clock to be also > disabled, which is not what we want. > > What would you suggest? What about the patch below? Shawn ----8<------------- From 0ed4b3edc661c63f86c914ea3c6deb3af3438151 Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 2 Jul 2014 11:32:06 +0800 Subject: [PATCH] ARM: imx: fix shared gate clock to have its own enable count Let's say clock A and B are two gate clocks that share the same register bit in hardware. Therefore they should be registered as shared gate clocks with imx_clk_gate2_shared(). In the current implementation, clk_enable(A) call will have share_count become 1. If clk_disable(B) is called right after that, the register bit will be cleared to gate off the clocks. This is unexpected. The cause for that is there is no enable count tracking for clock A and B respectively. The patch fixes the issue by adding enable_count into clk_gate2, and tracks it prior to share_count in .enable and .disable. Also, .is_enabled is fixed to report enable state instead of hardware state in case of shared gate clock. Reported-by: Fabio Estevam Cc: Fixes: f9f28cdf2167 ("ARM: imx: add shared gate clock support") Signed-off-by: Shawn Guo --- arch/arm/mach-imx/clk-gate2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/arch/arm/mach-imx/clk-gate2.c b/arch/arm/mach-imx/clk-gate2.c index 4ba587da89d2..c5dca7cdbbb1 100644 --- a/arch/arm/mach-imx/clk-gate2.c +++ b/arch/arm/mach-imx/clk-gate2.c @@ -33,6 +33,7 @@ struct clk_gate2 { u8 bit_idx; u8 flags; spinlock_t *lock; + unsigned int enable_count; unsigned int *share_count; }; @@ -46,6 +47,9 @@ static int clk_gate2_enable(struct clk_hw *hw) spin_lock_irqsave(gate->lock, flags); + if (gate->enable_count++ > 0) + goto out; + if (gate->share_count && (*gate->share_count)++ > 0) goto out; @@ -67,6 +71,9 @@ static void clk_gate2_disable(struct clk_hw *hw) spin_lock_irqsave(gate->lock, flags); + if (--gate->enable_count > 0) + goto out; + if (gate->share_count && --(*gate->share_count) > 0) goto out; @@ -83,6 +90,13 @@ static int clk_gate2_is_enabled(struct clk_hw *hw) u32 reg; struct clk_gate2 *gate = to_clk_gate2(hw); + /* + * In case this is a shared gate, we cannot just report the hardware + * state but its own enable state. + */ + if (gate->share_count) + return !!gate->enable_count; + reg = readl(gate->reg); if (((reg >> gate->bit_idx) & 1) == 1)