public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: daniel.lezcano@linaro•org (Daniel Lezcano)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 04/11] clocksource/moxart: Generalise timer for use on other socs
Date: Fri, 22 Apr 2016 19:30:34 +0200	[thread overview]
Message-ID: <571A5FBA.9010204@linaro.org> (raw)
In-Reply-To: <1461225849-28074-5-git-send-email-joel@jms.id.au>

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 <joel@jms•id.au>
> ---

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);

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;
  }

@@ -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


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  parent reply	other threads:[~2016-04-22 17:30 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-14  9:47 [PATCH 0/9] Aspeed AST2400 BMC support Joel Stanley
2016-04-14  9:47 ` [PATCH 1/9] doc/devicetree: Add Aspeed and Tyan to vendor-prefixes Joel Stanley
2016-04-14  9:47 ` [PATCH 2/9] doc/devicetree: Add Aspeed VIC bindings Joel Stanley
2016-04-14  9:47 ` [PATCH 3/9] doc/devicetree: Add Aspeed clock bindings Joel Stanley
2016-04-14  9:47 ` [PATCH 4/9] clocksource/moxart: Generalise timer for use on other socs Joel Stanley
2016-04-14  9:47 ` [PATCH 5/9] irqchip: Add irq controller for Aspeed Joel Stanley
2016-04-14  9:47 ` [PATCH 6/9] drivers/clk: Add Aspeed clock driver Joel Stanley
2016-04-14  9:47 ` [PATCH 7/9] arm/dts: Add aspeed device trees Joel Stanley
2016-04-14  9:47 ` [PATCH 8/9] arm: Add Aspeed AST2400 machine Joel Stanley
2016-04-14  9:47 ` [PATCH 9/9] arm/configs: Add aspeed defconfig Joel Stanley
2016-04-21  8:03 ` [PATCH v2 00/11] Aspeed AST2400 and AST2500 BMC support Joel Stanley
2016-04-21  8:03   ` [PATCH v2 01/11] doc/devicetree: Add Aspeed and Tyan to vendor-prefixes Joel Stanley
2016-04-21  8:04   ` [PATCH v2 02/11] doc/devicetree: Add Aspeed VIC bindings Joel Stanley
2016-04-21  8:04   ` [PATCH v2 03/11] doc/devicetree: Add Aspeed clock bindings Joel Stanley
2016-04-21 11:20     ` Heiko Stübner
2016-04-27  8:31       ` Joel Stanley
2016-04-27  9:12         ` Heiko Stübner
2016-04-28  6:50           ` Joel Stanley
2016-04-28  7:25             ` Heiko Stübner
2016-04-28  8:38           ` Benjamin Herrenschmidt
2016-04-21  8:04   ` [PATCH v2 04/11] clocksource/moxart: Generalise timer for use on other socs Joel Stanley
2016-04-21  8:22     ` Arnd Bergmann
2016-04-22  1:06       ` Joel Stanley
2016-04-22 17:30     ` Daniel Lezcano [this message]
2016-04-22 23:55       ` Benjamin Herrenschmidt
2016-05-03  5:56       ` Joel Stanley
2016-05-03 13:36         ` Daniel Lezcano
2016-05-06 14:50           ` Jonas Jensen
2016-04-21  8:04   ` [PATCH v2 05/11] irqchip: Add irq controller for Aspeed Joel Stanley
2016-04-21  8:04   ` [PATCH v2 06/11] clk: Add driver for Aspeed fourth gen SoCs Joel Stanley
2016-04-21  8:04   ` [PATCH v2 07/11] clk: Add driver for Aspeed fifth " Joel Stanley
2016-04-21  8:04   ` [PATCH v2 08/11] arm/dts: Add Aspeed ast2400 device tree Joel Stanley
2016-04-21  8:25     ` Arnd Bergmann
2016-04-21  8:04   ` [PATCH v2 09/11] arm/dst: Add Aspeed ast2500 " Joel Stanley
2016-05-05 23:11     ` Xo Wang
2016-05-06  7:28       ` Joel Stanley
2016-04-21  8:04   ` [PATCH v2 10/11] arm: Add Aspeed machine Joel Stanley
2016-04-21  8:35     ` Arnd Bergmann
2016-04-21 22:28       ` Benjamin Herrenschmidt
2016-04-21 23:02         ` Benjamin Herrenschmidt
2016-04-22  5:20           ` Afzal Mohammed
2016-04-22  5:32             ` Joel Stanley
2016-04-22 16:37               ` Arnd Bergmann
2016-04-21  8:04   ` [PATCH v2 11/11] arm/configs: Add aspeed defconfig Joel Stanley
2016-04-21  8:44     ` Arnd Bergmann
2016-04-21  8:54   ` [PATCH v2 00/11] Aspeed AST2400 and AST2500 BMC support Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=571A5FBA.9010204@linaro.org \
    --to=daniel.lezcano@linaro$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox