[PATCH] clocksource: sirf/marco+prima2: drop usage of CLOCK_TICK_RATE

[daniel.lezcano@linaro.org (Daniel Lezcano)]

On 11/26/2013 02:55 PM, Uwe Kleine-K?nig wrote:
> Hi Danial,
>
> On Thu, Nov 14, 2013 at 10:07:05AM +0100, Uwe Kleine-K?nig wrote:
>> On Wed, Nov 13, 2013 at 09:00:52PM +0100, Daniel Lezcano wrote:
>>> On 11/11/2013 09:20 PM, Uwe Kleine-K?nig wrote:
>>>> As CSR SiRF is converted to multi platform CLOCK_TICK_RATE is a dummy
>>>> value that seems to match the right value is used.
>>>> (arch/arm/mach-prima2/include/mach/timex.h which defined CLOCK_TICK_RATE
>>>> to 1000000 was removed in commit cf82e0e (ARM: sirf: enable
>>>> multiplatform support); marco used the same file.)
>>>>
>>>> To not depend on that dummy value use a local #define instead.
>>>
>>> I don't get this patch. It is to fix a compilation error ?
>> No, the problem is that CLOCK_TICK_RATE used to be defined in a platform
>> specific header <mach/timex.h>. For the ARM multiplatform stuff, this
>> was dropped and now all multiplatform kernels use 1000000. For some
>> platform (like SiRF) this happens to be correct, but actually it's pure
>> luck. Further down the road I'd like to drop defining CLOCK_TICK_RATE
>> for all platforms, so this is a preparing patch. But even independant
>> from that it feels wrong to use a dummy value that was only introduced
>> to prevent compile breakage.
>>
>> Would this change log be better:
>>
>> 	Since CSR SiRF was converted to multi platform in cf82e0e (ARM:
>> 	sirf: enable multiplatform support) the symbol CLOCK_TICK_RATE
>> 	isn't the platform specific definition any more, but a global
>> 	dummy value. There was no harm introduced in cf82e0e because the
>> 	global value happens to match the old platform specific one,
>> 	still this dummy value isn't intended to be used and will
>> 	hopefully disappear soon, so introduce a local #define and use
>> 	that instead.
>>
>> So it's not urgent, but would be a nice cleanup for 3.14-rc1.
> I'd like to depend on this patch to drop CLOCK_TICK_RATE for
> mach-prima2. Would it be ok for you when I include it in a pull request
> to the arm-soc people? If not, do you intend to take that patch, or do
> you still have objections? In that case I'd back out mach-prima2 from my
> CLOCK_TICK_RATE change.

I think it would be better to keep the macro name consistent and 
redefine it in the file with a comment.

/* over riding default value because bla bla */
#ifdef CLOCK_TICK_RATE
#undef CLOCK_TICK_RATE
#define CLOCK_TICK_RATE 100000
#endif

So people reading the code won't have to scratch their head to 
understand why CLOCK_FREQ is used instead of CLOCK_TICK_RATE. And that 
will limit the impact in the code.

Alternatively, I am wondering if that shouldn't fall into the DT, 
without the declaration in the DT, it defaults to 100000.

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