From patchwork Fri Apr 22 17:30:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Lezcano X-Patchwork-Id: 66490 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp846814qge; Fri, 22 Apr 2016 10:32:08 -0700 (PDT) X-Received: by 10.98.22.193 with SMTP id 184mr24652003pfw.116.1461346328387; Fri, 22 Apr 2016 10:32:08 -0700 (PDT) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id e184si2394513pfe.83.2016.04.22.10.32.08 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Apr 2016 10:32:08 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org 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 1atev0-0000EP-Q6; Fri, 22 Apr 2016 17:31:02 +0000 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ateux-0008RW-13 for linux-arm-kernel@lists.infradead.org; Fri, 22 Apr 2016 17:31:00 +0000 Received: by mail-wm0-x22d.google.com with SMTP id n3so37373455wmn.0 for ; Fri, 22 Apr 2016 10:30:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=Bu4qwftpFgmKlnYJP9i+vLbVBAbO6fru32pwEtmWlAA=; b=EJN8E1I2uzYXW0ed5tuE3c2QTnGH6a77wATA/QxMWx31WTyrqZmwAUymrCPJVqelkd aVoja6sZcG/qVzKS2cSKsv+wN7Sffeal9uby4bvJSWFYQTSMQOpxGgwfxZzUYPXHWg35 KEO4tzBKIRAHjI5+AdID4rHrGPrzRsZWjoa+w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Bu4qwftpFgmKlnYJP9i+vLbVBAbO6fru32pwEtmWlAA=; b=kyR3Zau9NtAVAyXTuVx7Y4fN0nXJyGsehDIalBUTgphJ/r2mNNXLbUOnWwiAU00yK4 TKJHckwrSWHTrrXkx1aM39/QjFaUVCuMlq6cn82URUtuH1uI48Nd2YZkAblerKyRZyec py97FU8b75UpouhFxDTknQx75UHNjK10HOQ5DXcrAiJycmbKhZ2LQu4/+V03hJtnBrFm jKx28BE+y5bZVcBeT10G/oi8tzRdHa6uuY5Gq9bzx6sQz6+8azD+7svD3wSK9AZB4jcT s2g27U3CCrpqPGzI79jbMMdDUMmpT3Z3KXx6b3OYOTEg8Q5kMQGgQyfrcGBqvqMUPzzt /QLQ== X-Gm-Message-State: AOPr4FXeGPB3wcXnpGQ5sDvQJuCSA8pMdqCoRmLM3kPkt55xwI49GNJ16bW1QDTVR9BdkSS8 X-Received: by 10.28.141.18 with SMTP id p18mr4881106wmd.57.1461346236559; Fri, 22 Apr 2016 10:30:36 -0700 (PDT) Received: from ?IPv6:2a01:e34:ed2f:f020:fddf:354b:eebb:34ca? ([2a01:e34:ed2f:f020:fddf:354b:eebb:34ca]) by smtp.googlemail.com with ESMTPSA id w79sm4355456wme.19.2016.04.22.10.30.35 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 22 Apr 2016 10:30:35 -0700 (PDT) Subject: Re: [PATCH v2 04/11] clocksource/moxart: Generalise timer for use on other socs To: Joel Stanley , linux-arm-kernel@lists.infradead.org, arnd@arndb.de References: <1461225849-28074-1-git-send-email-joel@jms.id.au> <1461225849-28074-5-git-send-email-joel@jms.id.au> From: Daniel Lezcano Message-ID: <571A5FBA.9010204@linaro.org> Date: Fri, 22 Apr 2016 19:30:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1461225849-28074-5-git-send-email-joel@jms.id.au> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160422_103059_602964_3BEEC523 X-CRM114-Status: GOOD ( 22.91 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2a00:1450:400c:c09:0:0:0:22d listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: benh@kernel.crashing.org, Thomas Gleixner , jk@ozlabs.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org On 04/21/2016 10:04 AM, Joel Stanley wrote: > The moxart timer IP is shared with another soc made by Aspeed. > Generalise the registers that differ so the same driver can be used for > both. > > As we now depend on CLKSRC_MMIO, create a Kconfig symbol for the driver > so we can express this dependency. > > Signed-off-by: Joel Stanley > --- In the future, please Cc the maintainers. > .../bindings/timer/moxa,moxart-timer.txt | 4 +- > drivers/clocksource/Kconfig | 6 ++ > drivers/clocksource/Makefile | 2 +- > drivers/clocksource/moxart_timer.c | 90 +++++++++++++++++----- > 4 files changed, 79 insertions(+), 23 deletions(-) [ ... ] > +config MOXART_TIMER > + def_bool ARCH_MOXART || ARCH_ASPEED > + depends on ARM && OF > + select CLKSRC_OF > + select CLKSRC_MMIO Refer to the other drivers to see how they are selected (eg. digicolor or mtk). [ ... ] > -#define TIMEREG_CR_1_ENABLE BIT(0) > -#define TIMEREG_CR_1_CLOCK BIT(1) > -#define TIMEREG_CR_1_INT BIT(2) > -#define TIMEREG_CR_2_ENABLE BIT(3) > -#define TIMEREG_CR_2_CLOCK BIT(4) > -#define TIMEREG_CR_2_INT BIT(5) > -#define TIMEREG_CR_3_ENABLE BIT(6) > -#define TIMEREG_CR_3_CLOCK BIT(7) > -#define TIMEREG_CR_3_INT BIT(8) > -#define TIMEREG_CR_COUNT_UP BIT(9) > - > -#define TIMER1_ENABLE (TIMEREG_CR_2_ENABLE | TIMEREG_CR_1_ENABLE) > -#define TIMER1_DISABLE (TIMEREG_CR_2_ENABLE) > +#define MOXART_CR_1_ENABLE BIT(0) > +#define MOXART_CR_1_CLOCK BIT(1) > +#define MOXART_CR_1_INT BIT(2) > +#define MOXART_CR_2_ENABLE BIT(3) > +#define MOXART_CR_2_CLOCK BIT(4) > +#define MOXART_CR_2_INT BIT(5) > +#define MOXART_CR_3_ENABLE BIT(6) > +#define MOXART_CR_3_CLOCK BIT(7) > +#define MOXART_CR_3_INT BIT(8) > +#define MOXART_CR_COUNT_UP BIT(9) > + > +#define MOXART_TIMER1_ENABLE (MOXART_CR_2_ENABLE | MOXART_CR_1_ENABLE) > +#define MOXART_TIMER1_DISABLE (MOXART_CR_2_ENABLE) > + > +/* > + * The ASpeed variant of the IP block has a different layout > + * for the control register > + */ > +#define ASPEED_CR_1_ENABLE BIT(0) > +#define ASPEED_CR_1_CLOCK BIT(1) > +#define ASPEED_CR_1_INT BIT(2) > +#define ASPEED_CR_2_ENABLE BIT(4) > +#define ASPEED_CR_2_CLOCK BIT(5) > +#define ASPEED_CR_2_INT BIT(6) > +#define ASPEED_CR_3_ENABLE BIT(8) > +#define ASPEED_CR_3_CLOCK BIT(9) > +#define ASPEED_CR_3_INT BIT(10) > + > +#define ASPEED_TIMER1_ENABLE (ASPEED_CR_2_ENABLE | ASPEED_CR_1_ENABLE) > +#define ASPEED_TIMER1_DISABLE (ASPEED_CR_2_ENABLE) You probably can remove all the unused macro definition here for both MOXART and ASPEED to have something just a couple of definition. > static void __iomem *base; > static unsigned int clock_count_per_tick; > +static unsigned int t1_disable_val, t1_enable_val; It will be cleaner to: 1. Factor out: writel(TIMER1_DISABLE, base + TIMER_CR); writel(TIMER1_ENABLE, base + TIMER_CR); @@ -83,12 +93,12 @@ static int moxart_clkevt_next_event(unsigned long cycles, { u32 u; - writel(TIMER1_DISABLE, base + TIMER_CR); + moxart_disable(evt); u = readl(base + TIMER1_BASE + REG_COUNT) - cycles; writel(u, base + TIMER1_BASE + REG_MATCH1); - writel(TIMER1_ENABLE, base + TIMER_CR); + moxart_enable(evt); return 0; } 2. Encapsulate clkevt and use container_of -static void __iomem *base; -static unsigned int clock_count_per_tick; +struct moxart_timer { + void __iomem *base; + unsigned int clock_count_per_tick; + struct clock_event_device clkevt; +}; + +static struct moxart_timer moxart_timer = { + .clkevt = { + .name = "moxart_timer", + .rating = 200, + .features = CLOCK_EVT_FEAT_PERIODIC | + CLOCK_EVT_FEAT_ONESHOT, + .set_state_shutdown = moxart_shutdown, + .set_state_periodic = moxart_set_periodic, + .set_state_oneshot = moxart_set_oneshot, + .tick_resume = moxart_set_oneshot, + .set_next_event = moxart_clkevt_next_event, + }, +}; + +static inline struct moxart_timer *clkevt_to_moxart(struct clock_event_device *evt) +{ + return container_of(evt, struct moxart_timer, clkevt); +} static inline void moxart_disable(struct clock_event_device *evt) { - writel(TIMER1_DISABLE, base + TIMER_CR); + writel(TIMER1_DISABLE, clkevt_to_moxart(evt)->base + TIMER_CR); } 3. And finally, add 't1_disable' / 't2_disable' to the structure. -- Daniel diff --git a/drivers/clocksource/moxart_timer.c b/drivers/clocksource/moxart_timer.c index 19857af..aede6f1 100644 --- a/drivers/clocksource/moxart_timer.c +++ b/drivers/clocksource/moxart_timer.c @@ -58,15 +58,25 @@ static void __iomem *base; static unsigned int clock_count_per_tick; -static int moxart_shutdown(struct clock_event_device *evt) +static inline void moxart_disable(struct clock_event_device *evt) { writel(TIMER1_DISABLE, base + TIMER_CR); +} + +static inline void moxart_enable(struct clock_event_device *evt) +{ + writel(TIMER1_ENABLE, base + TIMER_CR); +} + +static int moxart_shutdown(struct clock_event_device *evt) +{ + moxart_disable(evt); return 0; } static int moxart_set_oneshot(struct clock_event_device *evt) { - writel(TIMER1_DISABLE, base + TIMER_CR); + moxart_disable(evt); writel(~0, base + TIMER1_BASE + REG_LOAD); return 0; } @@ -74,7 +84,7 @@ static int moxart_set_oneshot(struct clock_event_device *evt) static int moxart_set_periodic(struct clock_event_device *evt) { writel(clock_count_per_tick, base + TIMER1_BASE + REG_LOAD); - writel(TIMER1_ENABLE, base + TIMER_CR); + moxart_enable(evt); return 0; }