Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbbHGK2Z (ORCPT ); Fri, 7 Aug 2015 06:28:25 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:33155 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751934AbbHGK2W (ORCPT ); Fri, 7 Aug 2015 06:28:22 -0400 Message-ID: <55C48841.3050902@linaro.org> Date: Fri, 07 Aug 2015 12:28:17 +0200 From: Daniel Lezcano User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Govindraj Raja , linux-kernel@vger.kernel.org, linux-mips@linux-mips.org, devicetree@vger.kernel.org CC: Thomas Gleixner , Andrew Bresticker , James Hartley , Damien Horsley , James Hogan , Ezequiel Garcia , Ezequiel Garcia Subject: Re: [PATCH v5 6/7] clocksource: Add Pistachio clocksource-only driver References: <1438860138-31718-1-git-send-email-govindraj.raja@imgtec.com> In-Reply-To: <1438860138-31718-1-git-send-email-govindraj.raja@imgtec.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4799 Lines: 170 On 08/06/2015 01:22 PM, Govindraj Raja wrote: > From: Ezequiel Garcia > > The Pistachio SoC provides four general purpose timers, and allow > to implement a clocksource driver. > > This driver can be used as a replacement for the MIPS GIC and MIPS R4K > clocksources and sched clocks, which are clocked from the CPU clock. > > Given the general purpose timers are clocked from an independent clock, > this new clocksource driver will be useful to introduce CPUFreq support > for Pistachio machines. > > Signed-off-by: Ezequiel Garcia > Signed-off-by: Govindraj Raja > --- > > changes from v4 > ---------------- > Fixes comments from Daniel as dicussed in below thread: > http://patchwork.linux-mips.org/patch/10784/ > > > drivers/clocksource/Kconfig | 5 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/time-pistachio.c | 216 +++++++++++++++++++++++++++++++++++ > 3 files changed, 222 insertions(+) > create mode 100644 drivers/clocksource/time-pistachio.c > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig [ ... ] > index 4e57730..e642825 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -111,6 +111,11 @@ config CLKSRC_LPC32XX > select CLKSRC_MMIO > select CLKSRC_OF > > +config CLKSRC_PISTACHIO > + bool > + default y if MACH_PISTACHIO You don't need to add this condition. > + select CLKSRC_OF [ ... ] > + > +struct pistachio_clocksource pcs_gpt; static. > +#define to_pistachio_clocksource(cs) \ > + container_of(cs, struct pistachio_clocksource, cs) > + > +static inline u32 gpt_readl(void __iomem *base, u32 offset, u32 gpt_id) > +{ > + return readl(base + 0x20 * gpt_id + offset); Are you sure of the address computation ? I guess it is correct but just wanted you to double check. > +} > + > +static inline void gpt_writel(void __iomem *base, u32 value, u32 offset, > + u32 gpt_id) > +{ > + writel(value, base + 0x20 * gpt_id + offset); > +} > + > +static cycle_t pistachio_clocksource_read_cycles(struct clocksource *cs) > +{ > + struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs); > + u32 counter, overflw; > + unsigned long flags; > + > + /* The counter value is only refreshed after the overflow value is read. > + * And they must be read in strict order, hence raw spin lock added. > + */ nit: extra carriage return and comment format: /* * blabla */ > + raw_spin_lock_irqsave(&pcs->lock, flags); > + overflw = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0); > + counter = gpt_readl(pcs->base, TIMER_CURRENT_VALUE, 0); > + raw_spin_unlock_irqrestore(&pcs->lock, flags); > + > + return ~(cycle_t)counter; > +} > + > +static u64 notrace pistachio_read_sched_clock(void) > +{ > + return pistachio_clocksource_read_cycles(&pcs_gpt.cs); Can you double check the u32 cast to cycle_t returning a u64 from this function ? > +} > + > +static void pistachio_clksrc_set_mode(struct clocksource *cs, int timeridx, > + int enable) > +{ > + struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs); > + u32 val; > + > + val = gpt_readl(pcs->base, TIMER_CFG, timeridx); > + if (enable) > + val |= TIMER_ME_LOCAL; > + else > + val &= ~TIMER_ME_LOCAL; > + > + gpt_writel(pcs->base, val, TIMER_CFG, timeridx); > +} > + > +static void pistachio_clksrc_enable(struct clocksource *cs, int timeridx) > +{ > + struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs); > + > + /* Disable GPT local before loading reload value */ > + pistachio_clksrc_set_mode(cs, timeridx, false); > + gpt_writel(pcs->base, RELOAD_VALUE, TIMER_RELOAD_VALUE, timeridx); > + pistachio_clksrc_set_mode(cs, timeridx, true); > +} > + > +static void pistachio_clksrc_disable(struct clocksource *cs, int timeridx) > +{ > + /* Disable GPT local */ > + pistachio_clksrc_set_mode(cs, timeridx, false); > +} > + > +static int pistachio_clocksource_enable(struct clocksource *cs) > +{ > + pistachio_clksrc_enable(cs, 0); > + return 0; > +} > + > +static void pistachio_clocksource_disable(struct clocksource *cs) > +{ > + pistachio_clksrc_disable(cs, 0); > +} > + > +/* Desirable clock source for pistachio platform */ > +struct pistachio_clocksource pcs_gpt = { static. Thanks ! -- Daniel -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/