Improperly handled case in arch/mips/au1000/common/time.c

Improperly handled case in arch/mips/au1000/common/time.c

[Jeff Baitis]

Pete:

While struggling to get Linux up on Evolution's custom board based on the
Au1500, I discovered a poorly handled case in time.c; null interrupts are
handled should not affect the local IRQ count. (if the local IRQ count is not
decremented, tests for in_irq() fail.)

Thanks for taking a look at my patch!

-Jeff

Index: time.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/au1000/common/time.c,v
retrieving revision 1.5.2.10
diff -u -r1.5.2.10 time.c
--- time.c      25 Mar 2003 14:30:19 -0000      1.5.2.10
+++ time.c      22 Apr 2003 19:47:24 -0000
@@ -114,6 +114,7 @@
        return;
 
 null:
+       irq_exit(cpu, irq);
        ack_r4ktimer(0);
 }

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Pete Popov]

Jeff,

Thanks, I'll get to this this week. Are there other outstanding Au
patches that you have sent me and I haven't applied yet? I just got back
from vacation so I'll get to them.

Pete

On Tue, 2003-04-22 at 12:54, Jeff Baitis wrote:
> Pete:
> 
> While struggling to get Linux up on Evolution's custom board based on the
> Au1500, I discovered a poorly handled case in time.c; null interrupts are
> handled should not affect the local IRQ count. (if the local IRQ count is not
> decremented, tests for in_irq() fail.)
> 
> Thanks for taking a look at my patch!
> 
> -Jeff
> 
> Index: time.c
> ===================================================================
> RCS file: /home/cvs/linux/arch/mips/au1000/common/time.c,v
> retrieving revision 1.5.2.10
> diff -u -r1.5.2.10 time.c
> --- time.c      25 Mar 2003 14:30:19 -0000      1.5.2.10
> +++ time.c      22 Apr 2003 19:47:24 -0000
> @@ -114,6 +114,7 @@
>         return;
>  
>  null:
> +       irq_exit(cpu, irq);
>         ack_r4ktimer(0);
>  }
> 
> 

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Ralf Baechle]

On Tue, Apr 22, 2003 at 01:20:21PM -0700, Pete Popov wrote:

> Thanks, I'll get to this this week. Are there other outstanding Au
> patches that you have sent me and I haven't applied yet? I just got back
> from vacation so I'll get to them.

I dealt with a few bugs introduced by the Great Cache Hackery; aside of
that I don't think there is much pending.

  Ralf

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Jun Sun]

I think this is a good example to show benefit of code sharing.
There is no good reason for au1x00 boards of not using new time.c.
You get to write less board code, fewer bugs and future proof.

Jun

On Tue, Apr 22, 2003 at 12:54:50PM -0700, Jeff Baitis wrote:
> Pete:
> 
> While struggling to get Linux up on Evolution's custom board based on the
> Au1500, I discovered a poorly handled case in time.c; null interrupts are
> handled should not affect the local IRQ count. (if the local IRQ count is not
> decremented, tests for in_irq() fail.)
> 
> Thanks for taking a look at my patch!
> 
> -Jeff
> 
> Index: time.c
> ===================================================================
> RCS file: /home/cvs/linux/arch/mips/au1000/common/time.c,v
> retrieving revision 1.5.2.10
> diff -u -r1.5.2.10 time.c
> --- time.c      25 Mar 2003 14:30:19 -0000      1.5.2.10
> +++ time.c      22 Apr 2003 19:47:24 -0000
> @@ -114,6 +114,7 @@
>         return;
>  
>  null:
> +       irq_exit(cpu, irq);
>         ack_r4ktimer(0);
>  }
> 
> 

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Pete Popov]

On Tue, 2003-04-22 at 15:56, Jun Sun wrote:
> I think this is a good example to show benefit of code sharing.
> There is no good reason for au1x00 boards of not using new time.c.
> You get to write less board code, fewer bugs and future proof.

The I didn't use the generic time.c back then is power management. The
CP0 counter sleeps when using the 'wait' instruction, so in that case
you have to use a different counter with a rather poor resolution. The
modifications I had to make were such that they couldn't go in the
generic time.c. But that area definitely needs to be revisited and
cleaned up.

Pete

> Jun
> 
> On Tue, Apr 22, 2003 at 12:54:50PM -0700, Jeff Baitis wrote:
> > Pete:
> > 
> > While struggling to get Linux up on Evolution's custom board based on the
> > Au1500, I discovered a poorly handled case in time.c; null interrupts are
> > handled should not affect the local IRQ count. (if the local IRQ count is not
> > decremented, tests for in_irq() fail.)
> > 
> > Thanks for taking a look at my patch!
> > 
> > -Jeff
> > 
> > Index: time.c
> > ===================================================================
> > RCS file: /home/cvs/linux/arch/mips/au1000/common/time.c,v
> > retrieving revision 1.5.2.10
> > diff -u -r1.5.2.10 time.c
> > --- time.c      25 Mar 2003 14:30:19 -0000      1.5.2.10
> > +++ time.c      22 Apr 2003 19:47:24 -0000
> > @@ -114,6 +114,7 @@
> >         return;
> >  
> >  null:
> > +       irq_exit(cpu, irq);
> >         ack_r4ktimer(0);
> >  }
> > 
> > 
> 
> 

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Jun Sun]

On Tue, Apr 22, 2003 at 04:00:39PM -0700, Pete Popov wrote:
> On Tue, 2003-04-22 at 15:56, Jun Sun wrote:
> > I think this is a good example to show benefit of code sharing.
> > There is no good reason for au1x00 boards of not using new time.c.
> > You get to write less board code, fewer bugs and future proof.
> 
> The I didn't use the generic time.c back then is power management. 

Oh, I see.  My ignorance ... :)

> The
> CP0 counter sleeps when using the 'wait' instruction, so in that case
> you have to use a different counter with a rather poor resolution. The
> modifications I had to make were such that they couldn't go in the
> generic time.c. But that area definitely needs to be revisited and
> cleaned up.
>

Yeah, there might be something interesting to do here.

Jun

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Dan Malek]

Pete Popov wrote:

> .... The
> modifications I had to make were such that they couldn't go in the
> generic time.c. But that area definitely needs to be revisited and
> cleaned up.

I've got a bunch of updates to the Au1xxx timer stuff.  I'll send a patch.


	-- Dan

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Ralf Baechle]

On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:

> I think this is a good example to show benefit of code sharing.
> There is no good reason for au1x00 boards of not using new time.c.
> You get to write less board code, fewer bugs and future proof.

There are just three configurations left using CONFIG_OLD_TIME_C:

 - EV64120 which I guess can be considered abandonded
 - Momenco Ocelot (Matthew, feel like you have time to take care of
   this?)
 - RM200  (semi-maintained, my turn to fix it ...)

Seems like it's time to get rid of CONFIG_OLD_TIME_C.

  Ralf

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Jun Sun]

On Wed, Apr 23, 2003 at 01:07:27AM +0200, Ralf Baechle wrote:
> On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:
> 
> > I think this is a good example to show benefit of code sharing.
> > There is no good reason for au1x00 boards of not using new time.c.
> > You get to write less board code, fewer bugs and future proof.
> 
> There are just three configurations left using CONFIG_OLD_TIME_C:
> 
>  - EV64120 which I guess can be considered abandonded
>  - Momenco Ocelot (Matthew, feel like you have time to take care of
>    this?)
>  - RM200  (semi-maintained, my turn to fix it ...)
> 
> Seems like it's time to get rid of CONFIG_OLD_TIME_C.
>

If we were going to get rid of CONFIG_OLD_TIME_C, I propose
to make CONFIG_NEW_TIME_C as the default and therefore removed as well.
And make other boards using private time_init() functions to use 
CONFIG_HAVE_PRIVATE_TIME. 

... in the spirit of encouraging code sharing. :)

Jun

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Pete Popov]

On Tue, 2003-04-22 at 16:13, Jun Sun wrote:
> On Wed, Apr 23, 2003 at 01:07:27AM +0200, Ralf Baechle wrote:
> > On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:
> > 
> > > I think this is a good example to show benefit of code sharing.
> > > There is no good reason for au1x00 boards of not using new time.c.
> > > You get to write less board code, fewer bugs and future proof.
> > 
> > There are just three configurations left using CONFIG_OLD_TIME_C:
> > 
> >  - EV64120 which I guess can be considered abandonded
> >  - Momenco Ocelot (Matthew, feel like you have time to take care of
> >    this?)
> >  - RM200  (semi-maintained, my turn to fix it ...)
> > 
> > Seems like it's time to get rid of CONFIG_OLD_TIME_C.
> >
> 
> If we were going to get rid of CONFIG_OLD_TIME_C, I propose
> to make CONFIG_NEW_TIME_C as the default and therefore removed as well.
> And make other boards using private time_init() functions to use 
> CONFIG_HAVE_PRIVATE_TIME. 
> 
> ... in the spirit of encouraging code sharing. :)

How about just extending the time api so that "have private time" can
just override some functions instead?  That way even then you'll have
some code sharing, even if it's not 100%.

Pete

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Jun Sun]

On Tue, Apr 22, 2003 at 04:18:40PM -0700, Pete Popov wrote:
> On Tue, 2003-04-22 at 16:13, Jun Sun wrote:
> > On Wed, Apr 23, 2003 at 01:07:27AM +0200, Ralf Baechle wrote:
> > > On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:
> > > 
> > > > I think this is a good example to show benefit of code sharing.
> > > > There is no good reason for au1x00 boards of not using new time.c.
> > > > You get to write less board code, fewer bugs and future proof.
> > > 
> > > There are just three configurations left using CONFIG_OLD_TIME_C:
> > > 
> > >  - EV64120 which I guess can be considered abandonded
> > >  - Momenco Ocelot (Matthew, feel like you have time to take care of
> > >    this?)
> > >  - RM200  (semi-maintained, my turn to fix it ...)
> > > 
> > > Seems like it's time to get rid of CONFIG_OLD_TIME_C.
> > >
> > 
> > If we were going to get rid of CONFIG_OLD_TIME_C, I propose
> > to make CONFIG_NEW_TIME_C as the default and therefore removed as well.
> > And make other boards using private time_init() functions to use 
> > CONFIG_HAVE_PRIVATE_TIME. 
> > 
> > ... in the spirit of encouraging code sharing. :)
> 
> How about just extending the time api so that "have private time" can
> just override some functions instead?  

Right now, the only function you _have_ to use is time_init().  All other
functions you can override (in board_timer_setup()).  And you can even
set up such that your own private routines are called first and in side which
you can call common routines.

New extension might be needed if some common features
are not accommondated by existing functions.  We can look at those 
features and see what is the way to abstract the support.

Jun

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Ralf Baechle]

On Tue, Apr 22, 2003 at 04:13:38PM -0700, Jun Sun wrote:

> If we were going to get rid of CONFIG_OLD_TIME_C, I propose
> to make CONFIG_NEW_TIME_C as the default and therefore removed as well.
> And make other boards using private time_init() functions to use 
> CONFIG_HAVE_PRIVATE_TIME. 
> 
> ... in the spirit of encouraging code sharing. :)

That's basically the old situation again, just in disguise this time.  In
practice I fear that's going to be used to keep inferior copies of some
code alive so let's see if we can avoid it.

  Ralf

Re: Improperly handled case in arch/mips/au1000/common/time.c

[Maciej W. Rozycki]

On Wed, 23 Apr 2003, Ralf Baechle wrote:

> That's basically the old situation again, just in disguise this time.  In
> practice I fear that's going to be used to keep inferior copies of some
> code alive so let's see if we can avoid it.

 Board-specific variations are unavoidable.  I'll see how to deal with
that as I want to merge the DECstation code (which already supports three
configuration variations) with the generic one.  It needs to be done for
2.5 and shouldn't hurt for 2.4, either. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

RE: Improperly handled case in arch/mips/au1000/common/time.c

[Matthew Dharm]

Umm... I might have some time in a few weeks.  Right now it's not much
of a priority for us (as a corporation).  I just have to hijack the
code from some of our other boards which already implements the new
system.

Matt

--
Matthew D. Dharm                            Senior Software Designer
Momentum Computer Inc.                      1815 Aston Ave.  Suite 107
(760) 431-8663 X-115                        Carlsbad, CA 92008-7310
Momentum Works For You                      www.momenco.com

> -----Original Message-----
> From: Ralf Baechle [mailto:ralf@linux-mips.org]
> Sent: Tuesday, April 22, 2003 4:07 PM
> To: Jun Sun
> Cc: Jeff Baitis; Pete Popov; linux-mips@linux-mips.org;
> Matthew Dharm
> Subject: Re: Improperly handled case in
> arch/mips/au1000/common/time.c
>
>
> On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:
>
> > I think this is a good example to show benefit of code sharing.
> > There is no good reason for au1x00 boards of not using new time.c.
> > You get to write less board code, fewer bugs and future proof.
>
> There are just three configurations left using CONFIG_OLD_TIME_C:
>
>  - EV64120 which I guess can be considered abandonded
>  - Momenco Ocelot (Matthew, feel like you have time to take care of
>    this?)
>  - RM200  (semi-maintained, my turn to fix it ...)
>
> Seems like it's time to get rid of CONFIG_OLD_TIME_C.
>
>   Ralf
>

RE: Improperly handled case in arch/mips/au1000/common/time.c

[Matthew Dharm]

Umm... I might have some time in a few weeks.  Right now it's not much
of a priority for us (as a corporation).  I just have to hijack the
code from some of our other boards which already implements the new
system.

Matt

--
Matthew D. Dharm                            Senior Software Designer
Momentum Computer Inc.                      1815 Aston Ave.  Suite 107
(760) 431-8663 X-115                        Carlsbad, CA 92008-7310
Momentum Works For You                      www.momenco.com

> -----Original Message-----
> From: Ralf Baechle [mailto:ralf@linux-mips.org]
> Sent: Tuesday, April 22, 2003 4:07 PM
> To: Jun Sun
> Cc: Jeff Baitis; Pete Popov; linux-mips@linux-mips.org;
> Matthew Dharm
> Subject: Re: Improperly handled case in
> arch/mips/au1000/common/time.c
>
>
> On Tue, Apr 22, 2003 at 03:56:25PM -0700, Jun Sun wrote:
>
> > I think this is a good example to show benefit of code sharing.
> > There is no good reason for au1x00 boards of not using new time.c.
> > You get to write less board code, fewer bugs and future proof.
>
> There are just three configurations left using CONFIG_OLD_TIME_C:
>
>  - EV64120 which I guess can be considered abandonded
>  - Momenco Ocelot (Matthew, feel like you have time to take care of
>    this?)
>  - RM200  (semi-maintained, my turn to fix it ...)
>
> Seems like it's time to get rid of CONFIG_OLD_TIME_C.
>
>   Ralf
>