2014-01-31 21:29:33

by Andrew Chew

[permalink] [raw]
Subject: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

There are some differences between tegra20's timer registers and tegra30's
(and later). For one thing, the watchdogs don't seem to be present in
tegra20. Add this compatibility string in order to be able to distinguish
whether the watchdogs are there or not.

Signed-off-by: Andrew Chew <[email protected]>
---
drivers/clocksource/tegra20_timer.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index d1869f0..73cfa56 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -218,6 +218,7 @@ static void __init tegra20_init_timer(struct device_node *np)
0x1, 0x1fffffff);
}
CLOCKSOURCE_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);
+CLOCKSOURCE_OF_DECLARE(tegra30_timer, "nvidia,tegra30-timer", tegra20_init_timer);

static void __init tegra20_init_rtc(struct device_node *np)
{
--
1.8.1.5


2014-02-03 16:39:54

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

On 01/31/2014 10:29 PM, Andrew Chew wrote:
> There are some differences between tegra20's timer registers and tegra30's
> (and later). For one thing, the watchdogs don't seem to be present in
> tegra20.

"don't seem", so it is an assumption ?

> Add this compatibility string in order to be able to distinguish
> whether the watchdogs are there or not.

Sorry but I don't get the connection between declaring the tegra30_timer
and the log. Can you elaborate please ?

> Signed-off-by: Andrew Chew <[email protected]>
> ---
> drivers/clocksource/tegra20_timer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
> index d1869f0..73cfa56 100644
> --- a/drivers/clocksource/tegra20_timer.c
> +++ b/drivers/clocksource/tegra20_timer.c
> @@ -218,6 +218,7 @@ static void __init tegra20_init_timer(struct device_node *np)
> 0x1, 0x1fffffff);
> }
> CLOCKSOURCE_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);
> +CLOCKSOURCE_OF_DECLARE(tegra30_timer, "nvidia,tegra30-timer", tegra20_init_timer);
>
> static void __init tegra20_init_rtc(struct device_node *np)
> {
>


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

2014-02-03 18:55:01

by Andrew Chew

[permalink] [raw]
Subject: RE: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

> From: Daniel Lezcano [mailto:[email protected]]
> Sent: Monday, February 03, 2014 8:40 AM
> To: Andrew Chew; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat
>
> On 01/31/2014 10:29 PM, Andrew Chew wrote:
> > There are some differences between tegra20's timer registers and
> > tegra30's (and later). For one thing, the watchdogs don't seem to be
> > present in tegra20.
>
> "don't seem", so it is an assumption ?

No, this is not an assumption. It has been verified by other NVIDIA engineers
since I proposed this change.

> > Add this compatibility string in order to be able to distinguish
> > whether the watchdogs are there or not.
>
> Sorry but I don't get the connection between declaring the tegra30_timer
> and the log. Can you elaborate please ?

I don't know what you mean by "the log". Was that a typo? Anyway, I
have a watchdog driver that I intend to follow up with, that binds
with tegra30-timer. I don't want this driver to be able to bind with
tegra20-timer, because the driver won't actually work on tegra20.

Does that answer your question?
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-03 21:07:30

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

On 01/31/2014 02:29 PM, Andrew Chew wrote:
> There are some differences between tegra20's timer registers and tegra30's
> (and later). For one thing, the watchdogs don't seem to be present in
> tegra20. Add this compatibility string in order to be able to distinguish
> whether the watchdogs are there or not.

Acked-by: Stephen Warren <[email protected]>

2014-02-04 19:10:06

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

On Mon, 3 Feb 2014, Andrew Chew wrote:

> > From: Daniel Lezcano [mailto:[email protected]]
> > Sent: Monday, February 03, 2014 8:40 AM
> > To: Andrew Chew; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat
> >
> > On 01/31/2014 10:29 PM, Andrew Chew wrote:
> > > There are some differences between tegra20's timer registers and
> > > tegra30's (and later). For one thing, the watchdogs don't seem to be
> > > present in tegra20.
> >
> > "don't seem", so it is an assumption ?
>
> No, this is not an assumption. It has been verified by other NVIDIA engineers
> since I proposed this change.

So why is your changelog saying "don't seem to be" ?

> > > Add this compatibility string in order to be able to distinguish
> > > whether the watchdogs are there or not.
> >
> > Sorry but I don't get the connection between declaring the tegra30_timer
> > and the log. Can you elaborate please ?
>
> I don't know what you mean by "the log". Was that a typo? Anyway, I

Daniel refers to the changelog.

> have a watchdog driver that I intend to follow up with, that binds
> with tegra30-timer. I don't want this driver to be able to bind with
> tegra20-timer, because the driver won't actually work on tegra20.

So the changelog should say:

Tegra30 and later have more timer functionality than Tegra20, but
share the clocksource core code.

Add a separate devicetree entry which defaults to the tegra20 code
for now. This is preparatory work for adding new tegra30 specific
functionality.

Thanks,

tglx

2014-02-04 19:14:38

by Andrew Chew

[permalink] [raw]
Subject: RE: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

> > > On 01/31/2014 10:29 PM, Andrew Chew wrote:
> > > > There are some differences between tegra20's timer registers and
> > > > tegra30's (and later). For one thing, the watchdogs don't seem to
> > > > be present in tegra20.
> > >
> > > "don't seem", so it is an assumption ?
> >
> > No, this is not an assumption. It has been verified by other NVIDIA
> > engineers since I proposed this change.
>
> So why is your changelog saying "don't seem to be" ?

I updated the commit message (see V2 of this patch). I hope the new
commit message satisfies the concerns:

"There are some differences between tegra20's timer registers and tegra30's
(and later). For example, tegra30 has more timers. In addition, watchdogs are
not present in tegra20.

Add this compatibility string in order to be able to distinguish whether the
additional timers and watchdogs are there or not."

2014-02-04 20:22:52

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [PATCH v1] clocksource: tegra: Add nvidia,tegra30-timer compat

On Tue, 4 Feb 2014, Andrew Chew wrote:

> > > > On 01/31/2014 10:29 PM, Andrew Chew wrote:
> > > > > There are some differences between tegra20's timer registers and
> > > > > tegra30's (and later). For one thing, the watchdogs don't seem to
> > > > > be present in tegra20.
> > > >
> > > > "don't seem", so it is an assumption ?
> > >
> > > No, this is not an assumption. It has been verified by other NVIDIA
> > > engineers since I proposed this change.
> >
> > So why is your changelog saying "don't seem to be" ?
>
> I updated the commit message (see V2 of this patch). I hope the new
> commit message satisfies the concerns:
>
> "There are some differences between tegra20's timer registers and tegra30's
> (and later). For example, tegra30 has more timers. In addition, watchdogs are
> not present in tegra20.
>
> Add this compatibility string in order to be able to distinguish whether the
> additional timers and watchdogs are there or not."

Yup, that's way more understandable.