From: Arnd Bergmann <arnd@arndb.de> To: Jonas Jensen <jonas.jensen@gmail.com>, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, arm@kernel.org, john.stultz@linaro.org, tglx@linutronix.de Subject: Re: [PATCH] ARM: clocksource: add support for MOXA ART SoCs Date: Tue, 18 Jun 2013 17:28:58 +0200 [thread overview] Message-ID: <201306181728.58869.arnd@arndb.de> (raw) In-Reply-To: <1371549604-7201-1-git-send-email-jonas.jensen@gmail.com> On Tuesday 18 June 2013, Jonas Jensen wrote: > This patch adds an clocksource driver for the main timer found > on MOXA ART SoCs. > > Applies to 3.10-rc1 and arm-soc/for-next (2013-06-15) > > Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com> I didn't look closely before but I agree with Thomas Petazzoni, this should be converted to clocksource/clockevent. A few other things I noticed now: > +#define APB_CLK 48000000 You are hardcoding the clock rate above, which makes it less portable than it should be. Ideally you would use clk_get_rate() on the default clock, but you don't actually implement a clk driver for your platform and probably don't need one. I don't know what others think about this, but I'd suggest just using a "clock-frequency" property in the device node to read the clock rate. > +#define TIMER_1_COUNT(base_addr) (base_addr + 0x00) > +#define TIMER_1_LOAD(base_addr) (base_addr + 0x04) > +#define TIMER_1_MATCH1(base_addr) (base_addr + 0x08) > +#define TIMER_1_MATCH2(base_addr) (base_addr + 0x0C) > + > +#define TIMER_2_COUNT(base_addr) (base_addr + 0x10) > +#define TIMER_2_LOAD(base_addr) (base_addr + 0x14) > +#define TIMER_2_MATCH1(base_addr) (base_addr + 0x18) > +#define TIMER_2_MATCH2(base_addr) (base_addr + 0x1C) > + > +#define TIMER_3_COUNT(base_addr) (base_addr + 0x20) > +#define TIMER_3_LOAD(base_addr) (base_addr + 0x24) > +#define TIMER_3_MATCH1(base_addr) (base_addr + 0x28) > +#define TIMER_3_MATCH2(base_addr) (base_addr + 0x2C) > + You actually seem to have three independent timers here, which means you can use one as the clock source and one for clock events. > +#define TIMER1_COUNT 0x0 > +#define TIMER1_LOAD 0x4 > +#define TIMER1_MATCH1 0x8 > +#define TIMER1_MATCH2 0xC > +#define TIMER2_COUNT 0x10 > +#define TIMER2_LOAD 0x14 > +#define TIMER2_MATCH1 0x18 > +#define TIMER2_MATCH2 0x1C > +#define TIMER3_COUNT 0x20 > +#define TIMER3_LOAD 0x24 > +#define TIMER3_MATCH1 0x28 > +#define TIMER3_MATCH2 0x2C > +#define TIMER_INTR_MASK 0x38 These look like duplicates from above. I'd prefer the second syntax, just do the addition of base_addr where you need it. Arnd
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] ARM: clocksource: add support for MOXA ART SoCs Date: Tue, 18 Jun 2013 17:28:58 +0200 [thread overview] Message-ID: <201306181728.58869.arnd@arndb.de> (raw) In-Reply-To: <1371549604-7201-1-git-send-email-jonas.jensen@gmail.com> On Tuesday 18 June 2013, Jonas Jensen wrote: > This patch adds an clocksource driver for the main timer found > on MOXA ART SoCs. > > Applies to 3.10-rc1 and arm-soc/for-next (2013-06-15) > > Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com> I didn't look closely before but I agree with Thomas Petazzoni, this should be converted to clocksource/clockevent. A few other things I noticed now: > +#define APB_CLK 48000000 You are hardcoding the clock rate above, which makes it less portable than it should be. Ideally you would use clk_get_rate() on the default clock, but you don't actually implement a clk driver for your platform and probably don't need one. I don't know what others think about this, but I'd suggest just using a "clock-frequency" property in the device node to read the clock rate. > +#define TIMER_1_COUNT(base_addr) (base_addr + 0x00) > +#define TIMER_1_LOAD(base_addr) (base_addr + 0x04) > +#define TIMER_1_MATCH1(base_addr) (base_addr + 0x08) > +#define TIMER_1_MATCH2(base_addr) (base_addr + 0x0C) > + > +#define TIMER_2_COUNT(base_addr) (base_addr + 0x10) > +#define TIMER_2_LOAD(base_addr) (base_addr + 0x14) > +#define TIMER_2_MATCH1(base_addr) (base_addr + 0x18) > +#define TIMER_2_MATCH2(base_addr) (base_addr + 0x1C) > + > +#define TIMER_3_COUNT(base_addr) (base_addr + 0x20) > +#define TIMER_3_LOAD(base_addr) (base_addr + 0x24) > +#define TIMER_3_MATCH1(base_addr) (base_addr + 0x28) > +#define TIMER_3_MATCH2(base_addr) (base_addr + 0x2C) > + You actually seem to have three independent timers here, which means you can use one as the clock source and one for clock events. > +#define TIMER1_COUNT 0x0 > +#define TIMER1_LOAD 0x4 > +#define TIMER1_MATCH1 0x8 > +#define TIMER1_MATCH2 0xC > +#define TIMER2_COUNT 0x10 > +#define TIMER2_LOAD 0x14 > +#define TIMER2_MATCH1 0x18 > +#define TIMER2_MATCH2 0x1C > +#define TIMER3_COUNT 0x20 > +#define TIMER3_LOAD 0x24 > +#define TIMER3_MATCH1 0x28 > +#define TIMER3_MATCH2 0x2C > +#define TIMER_INTR_MASK 0x38 These look like duplicates from above. I'd prefer the second syntax, just do the addition of base_addr where you need it. Arnd
next prev parent reply other threads:[~2013-06-18 15:29 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-06-18 10:00 [PATCH] ARM: clocksource: add support for MOXA ART SoCs Jonas Jensen 2013-06-18 10:00 ` Jonas Jensen 2013-06-18 15:14 ` Thomas Petazzoni 2013-06-18 15:14 ` Thomas Petazzoni 2013-06-18 15:28 ` Arnd Bergmann [this message] 2013-06-18 15:28 ` Arnd Bergmann 2013-06-26 14:53 ` [PATCH v2] " Jonas Jensen 2013-06-26 14:53 ` Jonas Jensen 2013-06-26 14:59 ` Jonas Jensen 2013-06-26 14:59 ` Jonas Jensen 2013-06-26 16:10 ` Uwe Kleine-König 2013-06-26 16:10 ` Uwe Kleine-König 2013-06-26 19:15 ` Linus Walleij 2013-06-26 19:15 ` Linus Walleij 2013-06-27 11:23 ` [PATCH v3] " Jonas Jensen 2013-06-27 11:23 ` Jonas Jensen 2013-06-28 13:34 ` Thomas Gleixner 2013-06-28 13:34 ` Thomas Gleixner 2013-07-01 14:02 ` [PATCH v4] " Jonas Jensen 2013-07-01 14:02 ` Jonas Jensen 2013-07-01 17:55 ` Thomas Gleixner 2013-07-01 17:55 ` Thomas Gleixner 2013-07-02 20:19 ` Linus Walleij 2013-07-02 20:19 ` Linus Walleij 2013-07-04 12:19 ` [PATCH v5] " Jonas Jensen 2013-07-04 12:19 ` Jonas Jensen 2013-07-04 21:42 ` Thomas Gleixner 2013-07-04 21:42 ` Thomas Gleixner 2013-07-05 10:05 ` Jonas Jensen 2013-07-05 10:05 ` Jonas Jensen 2013-07-05 10:21 ` Thomas Gleixner 2013-07-05 10:21 ` Thomas Gleixner 2013-07-05 11:48 ` Jonas Jensen 2013-07-05 11:48 ` Jonas Jensen 2013-07-05 10:04 ` [PATCH v6] " Jonas Jensen 2013-07-05 10:04 ` Jonas Jensen 2013-07-05 11:46 ` [PATCH v7] " Jonas Jensen 2013-07-05 11:46 ` Jonas Jensen 2013-07-16 13:52 ` Daniel Lezcano 2013-07-16 13:52 ` Daniel Lezcano 2013-07-16 14:44 ` [PATCH v8] " Jonas Jensen 2013-07-16 14:44 ` Jonas Jensen 2013-07-16 15:01 ` Daniel Lezcano 2013-07-16 15:01 ` Daniel Lezcano 2013-07-17 8:14 ` Jonas Jensen 2013-07-17 8:14 ` Jonas Jensen 2013-07-17 12:13 ` Daniel Lezcano 2013-07-17 12:13 ` Daniel Lezcano 2013-07-17 8:04 ` [PATCH v9] " Jonas Jensen 2013-07-17 8:04 ` Jonas Jensen 2013-07-19 11:12 ` [PATCH] ARM: clocksource: moxart: documentation: update device tree bindings document Jonas Jensen 2013-07-19 11:12 ` Jonas Jensen 2013-07-19 12:16 ` Daniel Lezcano 2013-07-19 12:16 ` Daniel Lezcano 2013-07-19 12:50 ` Jonas Jensen 2013-07-19 12:50 ` Jonas Jensen 2013-07-20 20:45 ` [PATCH v9] ARM: clocksource: add support for MOXA ART SoCs Linus Walleij 2013-07-20 20:45 ` Linus Walleij 2013-07-20 21:34 ` Daniel Lezcano 2013-07-20 21:34 ` Daniel Lezcano 2013-07-26 14:03 ` [PATCH] ARM: clocksource: moxart: add bitops.h include Jonas Jensen 2013-07-26 14:03 ` Jonas Jensen
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=201306181728.58869.arnd@arndb.de \ --to=arnd@arndb.de \ --cc=arm@kernel.org \ --cc=john.stultz@linaro.org \ --cc=jonas.jensen@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=thomas.petazzoni@free-electrons.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.