RE: [PATCH v2 2/3] clocksource: tegra: Define timer bases in headerfile

From: Andrew Chew
Date: Tue Feb 04 2014 - 13:55:38 EST


> From: Daniel Lezcano [mailto:daniel.lezcano@xxxxxxxxxx]
> Sent: Monday, February 03, 2014 11:55 PM
> To: Andrew Chew; tglx@xxxxxxxxxxxxx; swarren@xxxxxxxxxxxxx;
> thierry.reding@xxxxxxxxx; rob@xxxxxxxxxxx; grant.likely@xxxxxxxxxx;
> robh+dt@xxxxxxxxxx; abrestic@xxxxxxxxxxxx; dgreid@xxxxxxxxxxxx;
> katierh@xxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; linux-
> watchdog@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 2/3] clocksource: tegra: Define timer bases in header
> file
>
> On 02/04/2014 01:17 AM, Andrew Chew wrote:
> > Added timers that are present in tegra30 and later, that are NOT in tegra20.
> >
> > Also, some of these timer bases are needed in the tegra watchdog
> > driver, so separate them out into a header file that both the
> > clocksource driver and the watchdog driver can share them.
> >
> > Signed-off-by: Andrew Chew <achew@xxxxxxxxxx>
>
> When reading the patch 3/3, I don't see any define reused from this header
> except TEGRA30_TIMER_WDT_BASE which is only used for the watchdog.
> May be I missed something but I don't see any definition shared and thus I
> don't see the point of creating this header file.

I guess the point is to make other potential drivers that make use of the timer
registers aware that this particular timer (timer 5) is provisioned for the watchdog
driver. Right now, you're right, there really isn't much that's shared...the only
thing really is the base address of timer 5 (and its associated ID)...the timer bases
are kind of all over the place, so it's not straightforward to calculate the ID from the
base address.

But I believe the thought at the time (from Stephen Warren's suggestion) is to have
a central place where this stuff is defined so that it's more clear what timers are
provisioned for what purposes. It just happens that today, the only users of
the timers, other than clocksource, is watchdog, as far as I can tell.

> > +++ b/include/clocksource/tegra_timer.h
> > @@ -0,0 +1,43 @@
> > +/*
> > + * Copyright (C) 2010 Google, Inc.
> > + *
> > + * Author:
> > + * Colin Cross <ccross@xxxxxxxxxx>

The contents of this new header file are largely just a cut and paste of the corresponding
snippet in tegra20-timer.c, so I thought I'd carry over the license verbatim. I didn't author
this really...I just moved it around. What should I do instead?
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸ ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðà nÆàþY&—