Patchwork [GIT,PULL] timer: vt8500: Move timer code to drivers/clocksource

login
register
mail settings
Submitter Tony Prisk
Date Jan. 14, 2013, 5:09 a.m.
Message ID <1358140171-25390-1-git-send-email-linux@prisktech.co.nz>
Download mbox
Permalink /patch/211704/
State New
Headers show

Pull-request

git://server.prisktech.co.nz/git/linuxwmt.git tags/vt8500/timer

Comments

Tony Prisk - Jan. 14, 2013, 5:09 a.m.
Hi Olof,

Not sure if I have done this correctly but this is the patch to move the
vt8500/timer.c code into drivers/clocksource.

I based it on timer/cleanup from armsoc (hopefully this is right - if not, a
few pointers in the right direction would be appreciated).

Regards
Tony P


The following changes since commit 1c2584c3a1c882fec729147a46d822522552e38c:

  ARM: sunxi: fix struct sys_timer removal (2013-01-08 10:50:43 -0800)

are available in the git repository at:

  git://server.prisktech.co.nz/git/linuxwmt.git tags/vt8500/timer

for you to fetch changes up to ff7ec345f0ece9ddbb28538b70ba0c7f0acc17dc:

  timer: vt8500: Move timer code to drivers/clocksource (2013-01-14 17:58:21 +1300)

----------------------------------------------------------------
Move arch-vt8500/timer.c to drivers/clocksource/vt8500-timer.c

----------------------------------------------------------------
Tony Prisk (1):
      timer: vt8500: Move timer code to drivers/clocksource

 arch/arm/mach-vt8500/Kconfig                       |    1 +
 arch/arm/mach-vt8500/Makefile                      |    2 +-
 arch/arm/mach-vt8500/common.h                      |    1 -
 arch/arm/mach-vt8500/vt8500.c                      |    1 +
 drivers/clocksource/Kconfig                        |    3 +++
 drivers/clocksource/Makefile                       |    1 +
 .../timer.c => drivers/clocksource/vt8500_timer.c  |    0
 include/linux/vt8500_timer.h                       |   22 ++++++++++++++++++++
 8 files changed, 29 insertions(+), 2 deletions(-)
 rename arch/arm/mach-vt8500/timer.c => drivers/clocksource/vt8500_timer.c (100%)
 create mode 100644 include/linux/vt8500_timer.h
Tony Prisk - Jan. 14, 2013, 5:13 a.m.
On Mon, 2013-01-14 at 18:09 +1300, Tony Prisk wrote:
> This patch moves arch-vt8500/timer.c into drivers/clocksource and
> updates the necessary Kconfig/Makefile options.
> 
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
>  arch/arm/mach-vt8500/Kconfig       |    1 +
>  arch/arm/mach-vt8500/Makefile      |    2 +-
>  arch/arm/mach-vt8500/common.h      |    1 -
>  arch/arm/mach-vt8500/timer.c       |  184 ------------------------------------
>  arch/arm/mach-vt8500/vt8500.c      |    1 +
>  drivers/clocksource/Kconfig        |    3 +
>  drivers/clocksource/Makefile       |    1 +
>  drivers/clocksource/vt8500_timer.c |  184 ++++++++++++++++++++++++++++++++++++
>  include/linux/vt8500_timer.h       |   22 +++++
>  9 files changed, 213 insertions(+), 186 deletions(-)
>  delete mode 100644 arch/arm/mach-vt8500/timer.c
>  create mode 100644 drivers/clocksource/vt8500_timer.c
>  create mode 100644 include/linux/vt8500_timer.h

Darn.. forgot the -m again. I'll await your feedback regarding the
basing of the patch first (and any other feedback), then I'll redo it
with the correct stats.

Regards
Tony P
Tony Prisk - Jan. 14, 2013, 5:47 a.m.
On Mon, 2013-01-14 at 18:13 +1300, Tony Prisk wrote:
> On Mon, 2013-01-14 at 18:09 +1300, Tony Prisk wrote:
> > This patch moves arch-vt8500/timer.c into drivers/clocksource and
> > updates the necessary Kconfig/Makefile options.
> > 
> > Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> > ---
> >  arch/arm/mach-vt8500/Kconfig       |    1 +
> >  arch/arm/mach-vt8500/Makefile      |    2 +-
> >  arch/arm/mach-vt8500/common.h      |    1 -
> >  arch/arm/mach-vt8500/timer.c       |  184 ------------------------------------
> >  arch/arm/mach-vt8500/vt8500.c      |    1 +
> >  drivers/clocksource/Kconfig        |    3 +
> >  drivers/clocksource/Makefile       |    1 +
> >  drivers/clocksource/vt8500_timer.c |  184 ++++++++++++++++++++++++++++++++++++
> >  include/linux/vt8500_timer.h       |   22 +++++
> >  9 files changed, 213 insertions(+), 186 deletions(-)
> >  delete mode 100644 arch/arm/mach-vt8500/timer.c
> >  create mode 100644 drivers/clocksource/vt8500_timer.c
> >  create mode 100644 include/linux/vt8500_timer.h
> 
> Darn.. forgot the -m again. I'll await your feedback regarding the
> basing of the patch first (and any other feedback), then I'll redo it
> with the correct stats.
> 
> Regards
> Tony P

Oh grr.. forget this completely. It doesn't take into account the
patches I already sent for WM8850.

I guess it needs to be based on timer/cleanup + vt8500/wm8x50.

Need a little advise on how to handle this one please :)

Regards
Tony P
Tony Prisk - Jan. 14, 2013, 5:53 a.m.
> Oh grr.. forget this completely. It doesn't take into account the
> patches I already sent for WM8850.
> 
> I guess it needs to be based on timer/cleanup + vt8500/wm8x50.
> 
> Need a little advise on how to handle this one please :)
> 
> Regards
> Tony P

Turns out the original patch applies cleanly on timer/cleanup +
vt8500/wm8x50 anyway so maybe it doesn't need to be rebased.

Let me know if I need to do anything (other than the -m to cleanup the
stats).

Regards
Tony P
Stephen Warren - Jan. 14, 2013, 4:34 p.m.
On 01/13/2013 10:09 PM, Tony Prisk wrote:
> This patch moves arch-vt8500/timer.c into drivers/clocksource and
> updates the necessary Kconfig/Makefile options.

> diff --git a/include/linux/vt8500_timer.h b/include/linux/vt8500_timer.h

> +#ifndef __VT8500_TIMER_H
> +#define __VT8500_TIMER_H
> +
> +#include <asm/mach/time.h>
> +
> +void vt8500_timer_init(void);
> +
> +#endif

Is VT8500 DT-only? If so, it'd be nice not to add this header, but
instead use CLOCKSOURCE_OF_DECLARE() inside the driver C file.
Olof Johansson - Jan. 14, 2013, 8:07 p.m.
On Mon, Jan 14, 2013 at 06:47:35PM +1300, Tony Prisk wrote:
> On Mon, 2013-01-14 at 18:13 +1300, Tony Prisk wrote:
> > On Mon, 2013-01-14 at 18:09 +1300, Tony Prisk wrote:
> > > This patch moves arch-vt8500/timer.c into drivers/clocksource and
> > > updates the necessary Kconfig/Makefile options.
> > > 
> > > Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> > > ---
> > >  arch/arm/mach-vt8500/Kconfig       |    1 +
> > >  arch/arm/mach-vt8500/Makefile      |    2 +-
> > >  arch/arm/mach-vt8500/common.h      |    1 -
> > >  arch/arm/mach-vt8500/timer.c       |  184 ------------------------------------
> > >  arch/arm/mach-vt8500/vt8500.c      |    1 +
> > >  drivers/clocksource/Kconfig        |    3 +
> > >  drivers/clocksource/Makefile       |    1 +
> > >  drivers/clocksource/vt8500_timer.c |  184 ++++++++++++++++++++++++++++++++++++
> > >  include/linux/vt8500_timer.h       |   22 +++++
> > >  9 files changed, 213 insertions(+), 186 deletions(-)
> > >  delete mode 100644 arch/arm/mach-vt8500/timer.c
> > >  create mode 100644 drivers/clocksource/vt8500_timer.c
> > >  create mode 100644 include/linux/vt8500_timer.h
> > 
> > Darn.. forgot the -m again. I'll await your feedback regarding the
> > basing of the patch first (and any other feedback), then I'll redo it
> > with the correct stats.
> > 
> > Regards
> > Tony P
> 
> Oh grr.. forget this completely. It doesn't take into account the
> patches I already sent for WM8850.
> 
> I guess it needs to be based on timer/cleanup + vt8500/wm8x50.
> 
> Need a little advise on how to handle this one please :)

The normal way to handle these kind of dependencies is to base them on merges
of the needed branches. Based on the later email, you only seem to need
timer/cleanup, but if you would have needed the other one, then you'd merge
that on top of timer/cleanup, and then add your patches.

Of course, ideally you would do the cleanup, then add the wm8x50 features,
but in reality work doesn't always pan out that way, so you end up with
cleanups that depend on including new features in the same (sweeping)
cleanup since they have already been merged. That's when things sometimes
get hairy, and we need to start a second cleanup branch that's "after"
the feature branch in the sequence of topics. But it should be rare,
and in your case it seems like it wasn't needed.


-Olof
Olof Johansson - Jan. 14, 2013, 10:16 p.m.
On Mon, Jan 14, 2013 at 06:09:30PM +1300, Tony Prisk wrote:
> Hi Olof,
> 
> Not sure if I have done this correctly but this is the patch to move the
> vt8500/timer.c code into drivers/clocksource.
> 
> I based it on timer/cleanup from armsoc (hopefully this is right - if not, a
> few pointers in the right direction would be appreciated).
> 
> Regards
> Tony P
> 
> 
> The following changes since commit 1c2584c3a1c882fec729147a46d822522552e38c:
> 
>   ARM: sunxi: fix struct sys_timer removal (2013-01-08 10:50:43 -0800)
> 
> are available in the git repository at:
> 
>   git://server.prisktech.co.nz/git/linuxwmt.git tags/vt8500/timer

Thanks, pulled and merged into next/cleanup.


-Olof
Tony Prisk - Jan. 15, 2013, 4:12 a.m.
On Mon, 2013-01-14 at 12:07 -0800, Olof Johansson wrote:
> On Mon, Jan 14, 2013 at 06:47:35PM +1300, Tony Prisk wrote:
> > On Mon, 2013-01-14 at 18:13 +1300, Tony Prisk wrote:
> > > On Mon, 2013-01-14 at 18:09 +1300, Tony Prisk wrote:
> > > > This patch moves arch-vt8500/timer.c into drivers/clocksource and
> > > > updates the necessary Kconfig/Makefile options.
> > > > 
> > > > Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> > > > ---
> > > >  arch/arm/mach-vt8500/Kconfig       |    1 +
> > > >  arch/arm/mach-vt8500/Makefile      |    2 +-
> > > >  arch/arm/mach-vt8500/common.h      |    1 -
> > > >  arch/arm/mach-vt8500/timer.c       |  184 ------------------------------------
> > > >  arch/arm/mach-vt8500/vt8500.c      |    1 +
> > > >  drivers/clocksource/Kconfig        |    3 +
> > > >  drivers/clocksource/Makefile       |    1 +
> > > >  drivers/clocksource/vt8500_timer.c |  184 ++++++++++++++++++++++++++++++++++++
> > > >  include/linux/vt8500_timer.h       |   22 +++++
> > > >  9 files changed, 213 insertions(+), 186 deletions(-)
> > > >  delete mode 100644 arch/arm/mach-vt8500/timer.c
> > > >  create mode 100644 drivers/clocksource/vt8500_timer.c
> > > >  create mode 100644 include/linux/vt8500_timer.h
> > > 
> > > Darn.. forgot the -m again. I'll await your feedback regarding the
> > > basing of the patch first (and any other feedback), then I'll redo it
> > > with the correct stats.
> > > 
> > > Regards
> > > Tony P
> > 
> > Oh grr.. forget this completely. It doesn't take into account the
> > patches I already sent for WM8850.
> > 
> > I guess it needs to be based on timer/cleanup + vt8500/wm8x50.
> > 
> > Need a little advise on how to handle this one please :)
> 
> The normal way to handle these kind of dependencies is to base them on merges
> of the needed branches. Based on the later email, you only seem to need
> timer/cleanup, but if you would have needed the other one, then you'd merge
> that on top of timer/cleanup, and then add your patches.
> 
> Of course, ideally you would do the cleanup, then add the wm8x50 features,
> but in reality work doesn't always pan out that way, so you end up with
> cleanups that depend on including new features in the same (sweeping)
> cleanup since they have already been merged. That's when things sometimes
> get hairy, and we need to start a second cleanup branch that's "after"
> the feature branch in the sequence of topics. But it should be rare,
> and in your case it seems like it wasn't needed.
> 
> 
> -Olof
> 

Just to clarify what I did (and to make sure it was as you understood
it):

#1) I wrote the patch on top of timer/cleanup. This is the branch the
patch was written for.

#2) I then pulled timer/cleanup and merged vt8500/wm8x50 on top, then
reapplied the patch from #1 - it applied cleanly.

What I have just realised is that you might?? get a conflict when you
merge vt8500/wm8x50 on top of wherever this patch ends up due to the few
lines at the top of arch-vt8500/Kconfig having changed (the addition of
SELECT VT8500_TIMER). This should be trivial to fix (I assume).

Regards
Tony P
Tony Prisk - Jan. 15, 2013, 4:53 a.m.
On Mon, 2013-01-14 at 09:34 -0700, Stephen Warren wrote:
> On 01/13/2013 10:09 PM, Tony Prisk wrote:
> > This patch moves arch-vt8500/timer.c into drivers/clocksource and
> > updates the necessary Kconfig/Makefile options.
> 
> > diff --git a/include/linux/vt8500_timer.h b/include/linux/vt8500_timer.h
> 
> > +#ifndef __VT8500_TIMER_H
> > +#define __VT8500_TIMER_H
> > +
> > +#include <asm/mach/time.h>
> > +
> > +void vt8500_timer_init(void);
> > +
> > +#endif
> 
> Is VT8500 DT-only? If so, it'd be nice not to add this header, but
> instead use CLOCKSOURCE_OF_DECLARE() inside the driver C file.

Agreed - I didn't like the header when I added it but I didn't know of
another way and based in on the sunxi code.

Unfortunately Olof already pulled it into arm-soc, so I will try get
another patch done to undo it :)

Regards
Tony P
Olof Johansson - Jan. 15, 2013, 5:59 a.m.
On Mon, Jan 14, 2013 at 8:53 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
> On Mon, 2013-01-14 at 09:34 -0700, Stephen Warren wrote:
>> On 01/13/2013 10:09 PM, Tony Prisk wrote:
>> > This patch moves arch-vt8500/timer.c into drivers/clocksource and
>> > updates the necessary Kconfig/Makefile options.
>>
>> > diff --git a/include/linux/vt8500_timer.h b/include/linux/vt8500_timer.h
>>
>> > +#ifndef __VT8500_TIMER_H
>> > +#define __VT8500_TIMER_H
>> > +
>> > +#include <asm/mach/time.h>
>> > +
>> > +void vt8500_timer_init(void);
>> > +
>> > +#endif
>>
>> Is VT8500 DT-only? If so, it'd be nice not to add this header, but
>> instead use CLOCKSOURCE_OF_DECLARE() inside the driver C file.
>
> Agreed - I didn't like the header when I added it but I didn't know of
> another way and based in on the sunxi code.
>
> Unfortunately Olof already pulled it into arm-soc, so I will try get
> another patch done to undo it :)

Ack, I saw the comments but somehow blanked when it came to pulling
it. Incremental patches on top are fine though.

Sorry Stephen, I didn't mean to ignore your comments. :)


-Olof
Olof Johansson - Jan. 15, 2013, 6:03 a.m.
On Mon, Jan 14, 2013 at 8:12 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
> On Mon, 2013-01-14 at 12:07 -0800, Olof Johansson wrote:
>> On Mon, Jan 14, 2013 at 06:47:35PM +1300, Tony Prisk wrote:
>> > On Mon, 2013-01-14 at 18:13 +1300, Tony Prisk wrote:
>> > > On Mon, 2013-01-14 at 18:09 +1300, Tony Prisk wrote:
>> > > > This patch moves arch-vt8500/timer.c into drivers/clocksource and
>> > > > updates the necessary Kconfig/Makefile options.
>> > > >
>> > > > Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
>> > > > ---
>> > > >  arch/arm/mach-vt8500/Kconfig       |    1 +
>> > > >  arch/arm/mach-vt8500/Makefile      |    2 +-
>> > > >  arch/arm/mach-vt8500/common.h      |    1 -
>> > > >  arch/arm/mach-vt8500/timer.c       |  184 ------------------------------------
>> > > >  arch/arm/mach-vt8500/vt8500.c      |    1 +
>> > > >  drivers/clocksource/Kconfig        |    3 +
>> > > >  drivers/clocksource/Makefile       |    1 +
>> > > >  drivers/clocksource/vt8500_timer.c |  184 ++++++++++++++++++++++++++++++++++++
>> > > >  include/linux/vt8500_timer.h       |   22 +++++
>> > > >  9 files changed, 213 insertions(+), 186 deletions(-)
>> > > >  delete mode 100644 arch/arm/mach-vt8500/timer.c
>> > > >  create mode 100644 drivers/clocksource/vt8500_timer.c
>> > > >  create mode 100644 include/linux/vt8500_timer.h
>> > >
>> > > Darn.. forgot the -m again. I'll await your feedback regarding the
>> > > basing of the patch first (and any other feedback), then I'll redo it
>> > > with the correct stats.
>> > >
>> > > Regards
>> > > Tony P
>> >
>> > Oh grr.. forget this completely. It doesn't take into account the
>> > patches I already sent for WM8850.
>> >
>> > I guess it needs to be based on timer/cleanup + vt8500/wm8x50.
>> >
>> > Need a little advise on how to handle this one please :)
>>
>> The normal way to handle these kind of dependencies is to base them on merges
>> of the needed branches. Based on the later email, you only seem to need
>> timer/cleanup, but if you would have needed the other one, then you'd merge
>> that on top of timer/cleanup, and then add your patches.
>>
>> Of course, ideally you would do the cleanup, then add the wm8x50 features,
>> but in reality work doesn't always pan out that way, so you end up with
>> cleanups that depend on including new features in the same (sweeping)
>> cleanup since they have already been merged. That's when things sometimes
>> get hairy, and we need to start a second cleanup branch that's "after"
>> the feature branch in the sequence of topics. But it should be rare,
>> and in your case it seems like it wasn't needed.
>>
>>
>> -Olof
>>
>
> Just to clarify what I did (and to make sure it was as you understood
> it):
>
> #1) I wrote the patch on top of timer/cleanup. This is the branch the
> patch was written for.
>
> #2) I then pulled timer/cleanup and merged vt8500/wm8x50 on top, then
> reapplied the patch from #1 - it applied cleanly.
>
> What I have just realised is that you might?? get a conflict when you
> merge vt8500/wm8x50 on top of wherever this patch ends up due to the few
> lines at the top of arch-vt8500/Kconfig having changed (the addition of
> SELECT VT8500_TIMER). This should be trivial to fix (I assume).

Yeah, a couple of trivial conflicts are fine, but when they start to
stack up we normally have to push back since some of these are exposed
all the way up to Linus. We can also be a little more strategic in how
we chose to pick bases for the topic branches to avoid some of this,
which is what I intend to do this release cycle if it pans out.

In this case, there are no manual conflict resolution to be done,
everything panned out just fine as it was.


-Olof