Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757571AbaFSLA0 (ORCPT ); Thu, 19 Jun 2014 07:00:26 -0400 Received: from mail-yh0-f51.google.com ([209.85.213.51]:55221 "EHLO mail-yh0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751875AbaFSLAU convert rfc822-to-8bit (ORCPT ); Thu, 19 Jun 2014 07:00:20 -0400 MIME-Version: 1.0 In-Reply-To: <53A2BBD0.3050106@linaro.org> References: <1403167145-5267-1-git-send-email-amit.daniel@samsung.com> <53A2A936.3070109@linaro.org> <53A2BBD0.3050106@linaro.org> Date: Thu, 19 Jun 2014 16:30:19 +0530 X-Google-Sender-Auth: OK8SBl5AJTPVKBrLlcUBJ_g0ojA Message-ID: Subject: Re: [PATCH v2] clocksource: exynos-mct: Register the timer for stable udelay From: amit daniel kachhap To: Daniel Lezcano Cc: "linux-samsung-soc@vger.kernel.org" , Kukjin Kim , Thomas Gleixner , LAK , "linux-kernel@vger.kernel.org" , David Riley , Doug Anderson , Tomasz Figa Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 19, 2014 at 4:00 PM, Daniel Lezcano wrote: > On 06/19/2014 12:21 PM, amit daniel kachhap wrote: >> >> On Thu, Jun 19, 2014 at 2:41 PM, Daniel Lezcano >> wrote: >>> >>> On 06/19/2014 10:39 AM, Amit Daniel Kachhap wrote: >>>> >>>> >>>> This patch register the exynos mct clocksource as the current timer >>>> as it has constant clock rate. This will generate correct udelay for the >>>> exynos platform and avoid using unnecessary calibrated jiffies. This >>>> change >>>> has been tested on exynos5420 based board and udelay is very close to >>>> expected. >>>> >>>> Signed-off-by: Amit Daniel Kachhap >>>> --- >>>> Changes in V2: >>>> * Added #defines for ARM and ARM64 as pointed by Doug Anderson. >>>> >>>> Patches from David Riley confirmed that udelay is broken in exynos5420. >>>> Link to those patches are, >>>> 1) https://patchwork.kernel.org/patch/4344911/ >>>> 2) https://patchwork.kernel.org/patch/4344881/ >>>> >>>> drivers/clocksource/exynos_mct.c | 15 +++++++++++++++ >>>> 1 file changed, 15 insertions(+) >>>> >>>> diff --git a/drivers/clocksource/exynos_mct.c >>>> b/drivers/clocksource/exynos_mct.c >>>> index f71d55f..02927e2 100644 >>>> --- a/drivers/clocksource/exynos_mct.c >>>> +++ b/drivers/clocksource/exynos_mct.c >>>> @@ -195,10 +195,25 @@ static u64 notrace exynos4_read_sched_clock(void) >>>> return exynos4_frc_read(&mct_frc); >>>> } >>>> >>>> +static struct delay_timer exynos4_delay_timer; >>>> + >>>> +static unsigned long exynos4_read_current_timer(void) >>>> +{ >>>> +#ifdef ARM >>>> + return __raw_readl(reg_base + EXYNOS4_MCT_G_CNT_L); >>>> +#else /* ARM64, etc */ >>>> + return exynos4_frc_read(&mct_frc); >>>> +#endif >>>> +} >>>> + >>> >>> >>> >>> There isn't another solution than that ? macros definitions in C file are >>> avoided as much as possible. >> >> I also didn't want to use macros but used as a last option. you want >> me to put more comments here? >> Or something like below is also possible for checking the size of >> (unsigned long) in runtime. >> >> unsigned long x; >> unsigned int size = (char *)(&x + 1) - (char *)(&x); >> if (size == 4) >> return __raw_readl(reg_base + EXYNOS4_MCT_G_CNT_L); >> else >> return exynos4_frc_read(&mct_frc); >> >> But this involves extra computation which should not be used for time >> critical functions. >> Any suggestion? > > > AFAIU, Doug may change the exynos4_frc_read to be 32 bits. So may not need > those macros as 'exynos4_frc_read' could be used instead. right, its better to wait till those patches are out. > > Doug may confirm this. > > >>>> static void __init exynos4_clocksource_init(void) >>>> { >>>> exynos4_mct_frc_start(); >>>> >>>> + exynos4_delay_timer.read_current_timer = >>>> &exynos4_read_current_timer; >>> >>> >>> >>> &exynos4_read_current_timer ? >> >> Any issue in the naming? > > > No problem with the naming. As Tomasz pointed also, you are passing '&' for > the function pointer. yes my mistake. > > >>>> + exynos4_delay_timer.freq = clk_rate; >>>> + register_current_timer_delay(&exynos4_delay_timer); >>>> + >>>> if (clocksource_register_hz(&mct_frc, clk_rate)) >>>> panic("%s: can't register clocksource\n", >>>> mct_frc.name); >>>> >>>> >>> >>> >>> -- >>> Linaro.org │ Open source software for ARM SoCs >>> >>> Follow Linaro: Facebook | >>> Twitter | >>> Blog >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> linux-samsung-soc" >>> in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: Facebook | > Twitter | > Blog > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/