Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Russell King]

On Mon, Sep 24, 2007 at 05:53:57PM -0500, Greg.Chandler@wellsfargo.com wrote:
> I was building a kernel for an iPaq {SA1110} and ran into this.
> 
> linux-2.6.22.7/arch/arm/mach-sa1100/generic.c:
> Has a: #include <linux/cpufreq.h>
> Then afterwards there is a: #if defined(CONFIG_CPU_FREQ_SA1100) ||
> defined(CONFIG_CPU_FREQ_SA1110)
> who's else section redefines the cpufreq_get function inhereited from
> the header....
> 
> I'm guessing no one ever ended up in the "else" section until now, and
> that the header was added some time ago and no one caught this.
> This patch worked for me to get rid of the compile time problems.  I'm
> having issues with the kernel, but as far as I can tell they are form
> the Frame buffer and not because of this.  If this assessment is correct
> {the not needing this code anymore} then please pass this along so it
> makes it into an upcoming release.
> 
> --- linux-2.6.22.7/arch/arm/mach-sa1100/generic.c.orig  2007-09-24
> 17:36:21.000000000 -0500
> +++ linux-2.6.22.7/arch/arm/mach-sa1100/generic.c       2007-09-24
> 17:40:02.000000000 -0500
> @@ -107,15 +107,6 @@ unsigned int sa11x0_getspeed(unsigned in
>         return cclk_frequency_100khz[PPCR & 0xf] * 100;
>  }
> 
> -#else
> -/*
> - * We still need to provide this so building without cpufreq works.
> - */
> -unsigned int cpufreq_get(unsigned int cpu)
> -{
> -       return cclk_frequency_100khz[PPCR & 0xf] * 100;
> -}
> -EXPORT_SYMBOL(cpufreq_get);
>  #endif
> 
>  /*

No.  That code is required - the StrongARM 1100 framebuffer driver
*needs* to know what the CPU frequency is so it can set the pixel
clock divisor.

The real problem is the silly people who added this to cpufreq.h:

#ifdef CONFIG_CPU_FREQ
unsigned int cpufreq_quick_get(unsigned int cpu);
unsigned int cpufreq_get(unsigned int cpu);
#else
static inline unsigned int cpufreq_quick_get(unsigned int cpu)
{
        return 0;
}
static inline unsigned int cpufreq_get(unsigned int cpu)
{
        return 0;
}
#endif

which utterly bogus.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Dave Jones]

On Tue, Sep 25, 2007 at 08:31:32AM +0100, Russell King wrote:
 > On Mon, Sep 24, 2007 at 05:53:57PM -0500, Greg.Chandler@wellsfargo.com wrote:
 > > I was building a kernel for an iPaq {SA1110} and ran into this.
 > > 
 > > linux-2.6.22.7/arch/arm/mach-sa1100/generic.c:
 > > Has a: #include <linux/cpufreq.h>
 > > Then afterwards there is a: #if defined(CONFIG_CPU_FREQ_SA1100) ||
 > > defined(CONFIG_CPU_FREQ_SA1110)
 > > who's else section redefines the cpufreq_get function inhereited from
 > > the header....
 > > 
 > > I'm guessing no one ever ended up in the "else" section until now, and
 > > that the header was added some time ago and no one caught this.
 > > This patch worked for me to get rid of the compile time problems.  I'm
 > > having issues with the kernel, but as far as I can tell they are form
 > > the Frame buffer and not because of this.  If this assessment is correct
 > > {the not needing this code anymore} then please pass this along so it
 > > makes it into an upcoming release.
 > > 
 > > --- linux-2.6.22.7/arch/arm/mach-sa1100/generic.c.orig  2007-09-24
 > > 17:36:21.000000000 -0500
 > > +++ linux-2.6.22.7/arch/arm/mach-sa1100/generic.c       2007-09-24
 > > 17:40:02.000000000 -0500
 > > @@ -107,15 +107,6 @@ unsigned int sa11x0_getspeed(unsigned in
 > >         return cclk_frequency_100khz[PPCR & 0xf] * 100;
 > >  }
 > > 
 > > -#else
 > > -/*
 > > - * We still need to provide this so building without cpufreq works.
 > > - */
 > > -unsigned int cpufreq_get(unsigned int cpu)
 > > -{
 > > -       return cclk_frequency_100khz[PPCR & 0xf] * 100;
 > > -}
 > > -EXPORT_SYMBOL(cpufreq_get);
 > >  #endif
 > > 
 > >  /*
 > 
 > No.  That code is required - the StrongARM 1100 framebuffer driver
 > *needs* to know what the CPU frequency is so it can set the pixel
 > clock divisor.
 > 
 > The real problem is the silly people who added this to cpufreq.h:
 > 
 > #ifdef CONFIG_CPU_FREQ
 > unsigned int cpufreq_quick_get(unsigned int cpu);
 > unsigned int cpufreq_get(unsigned int cpu);
 > #else
 > static inline unsigned int cpufreq_quick_get(unsigned int cpu)
 > {
 >         return 0;
 > }
 > static inline unsigned int cpufreq_get(unsigned int cpu)
 > {
 >         return 0;
 > }
 > #endif
 > 
 > which utterly bogus.

Which came from ...

commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
Author: Andrew Morton <akpm@linux-foundation.org>
Date:   Wed May 2 19:27:08 2007 +0200

    [PATCH] x86-64: fix x86_64-mm-sched-clock-share
    
    Fix for the following patch. Provide dummy cpufreq functions when
    CPUFREQ is not compiled in.
    
    Cc: Andi Kleen <ak@suse.de>
    Cc: Dave Jones <davej@codemonkey.org.uk>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Andi Kleen <ak@suse.de>

I don't remember seeing any problem here, so I'm not entirely sure what
this was supposed to be fixing.  Perhaps the -mm-esque patch name
will provide Andrew/Andi clues. It lacks sufficient information for
my brain to guess what the problem was.

"Fix for the following patch" is also something that really should
never be added to a git changelog too, because 'next' means absolutely
nothing to me, nor I expect 99% of changelog readers.


Cc's added.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Andrew Morton]

On Tue, 25 Sep 2007 10:36:51 -0400 Dave Jones <davej@redhat.com> wrote:

> On Tue, Sep 25, 2007 at 08:31:32AM +0100, Russell King wrote:
>  > On Mon, Sep 24, 2007 at 05:53:57PM -0500, Greg.Chandler@wellsfargo.com wrote:
>  > > I was building a kernel for an iPaq {SA1110} and ran into this.
>  > > 
>  > > linux-2.6.22.7/arch/arm/mach-sa1100/generic.c:
>  > > Has a: #include <linux/cpufreq.h>
>  > > Then afterwards there is a: #if defined(CONFIG_CPU_FREQ_SA1100) ||
>  > > defined(CONFIG_CPU_FREQ_SA1110)
>  > > who's else section redefines the cpufreq_get function inhereited from
>  > > the header....
>  > > 
>  > > I'm guessing no one ever ended up in the "else" section until now, and
>  > > that the header was added some time ago and no one caught this.
>  > > This patch worked for me to get rid of the compile time problems.  I'm
>  > > having issues with the kernel, but as far as I can tell they are form
>  > > the Frame buffer and not because of this.  If this assessment is correct
>  > > {the not needing this code anymore} then please pass this along so it
>  > > makes it into an upcoming release.
>  > > 
>  > > --- linux-2.6.22.7/arch/arm/mach-sa1100/generic.c.orig  2007-09-24
>  > > 17:36:21.000000000 -0500
>  > > +++ linux-2.6.22.7/arch/arm/mach-sa1100/generic.c       2007-09-24
>  > > 17:40:02.000000000 -0500
>  > > @@ -107,15 +107,6 @@ unsigned int sa11x0_getspeed(unsigned in
>  > >         return cclk_frequency_100khz[PPCR & 0xf] * 100;
>  > >  }
>  > > 
>  > > -#else
>  > > -/*
>  > > - * We still need to provide this so building without cpufreq works.
>  > > - */
>  > > -unsigned int cpufreq_get(unsigned int cpu)
>  > > -{
>  > > -       return cclk_frequency_100khz[PPCR & 0xf] * 100;
>  > > -}
>  > > -EXPORT_SYMBOL(cpufreq_get);
>  > >  #endif
>  > > 
>  > >  /*
>  > 
>  > No.  That code is required - the StrongARM 1100 framebuffer driver
>  > *needs* to know what the CPU frequency is so it can set the pixel
>  > clock divisor.
>  > 
>  > The real problem is the silly people who added this to cpufreq.h:
>  > 
>  > #ifdef CONFIG_CPU_FREQ
>  > unsigned int cpufreq_quick_get(unsigned int cpu);
>  > unsigned int cpufreq_get(unsigned int cpu);
>  > #else
>  > static inline unsigned int cpufreq_quick_get(unsigned int cpu)
>  > {
>  >         return 0;
>  > }
>  > static inline unsigned int cpufreq_get(unsigned int cpu)
>  > {
>  >         return 0;
>  > }
>  > #endif
>  > 
>  > which utterly bogus.
> 
> Which came from ...
> 
> commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
> Author: Andrew Morton <akpm@linux-foundation.org>
> Date:   Wed May 2 19:27:08 2007 +0200
> 
>     [PATCH] x86-64: fix x86_64-mm-sched-clock-share
>     
>     Fix for the following patch. Provide dummy cpufreq functions when
>     CPUFREQ is not compiled in.
>     
>     Cc: Andi Kleen <ak@suse.de>
>     Cc: Dave Jones <davej@codemonkey.org.uk>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Andi Kleen <ak@suse.de>
> 
> I don't remember seeing any problem here, so I'm not entirely sure what
> this was supposed to be fixing.  Perhaps the -mm-esque patch name
> will provide Andrew/Andi clues. It lacks sufficient information for
> my brain to guess what the problem was.

Oh geeze.  sched-clock-share went through about 18 different versions, was
merged, unmerged, remerged, dropped, etc.  I don't recall at what stage in
this mess the above fix was inserted, sorry.

> "Fix for the following patch" is also something that really should
> never be added to a git changelog too, because 'next' means absolutely
> nothing to me, nor I expect 99% of changelog readers.

184c44d2049c4db7ef6ec65794546954da2c6a0e should never have existed,
actually.  I intended that Andi fold it into the base patch prior to
sending it to Linus.  He normally does that, but it looks like this
one was handled as a standalone commit for some reason.

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Dave Jones]

On Tue, Sep 25, 2007 at 09:52:29AM -0700, Andrew Morton wrote:
 > On Tue, 25 Sep 2007 10:36:51 -0400 Dave Jones <davej@redhat.com> wrote:
 > > 
 > > commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
 > > Author: Andrew Morton <akpm@linux-foundation.org>
 > > Date:   Wed May 2 19:27:08 2007 +0200
 > > 
 > >     [PATCH] x86-64: fix x86_64-mm-sched-clock-share
 > >     
 > >     Fix for the following patch. Provide dummy cpufreq functions when
 > >     CPUFREQ is not compiled in.
 > >     
 > >     Cc: Andi Kleen <ak@suse.de>
 > >     Cc: Dave Jones <davej@codemonkey.org.uk>
 > >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
 > >     Signed-off-by: Andi Kleen <ak@suse.de>
 > > 
 > > I don't remember seeing any problem here, so I'm not entirely sure what
 > > this was supposed to be fixing.  Perhaps the -mm-esque patch name
 > > will provide Andrew/Andi clues. It lacks sufficient information for
 > > my brain to guess what the problem was.
 > 
 > Oh geeze.  sched-clock-share went through about 18 different versions, was
 > merged, unmerged, remerged, dropped, etc.  I don't recall at what stage in
 > this mess the above fix was inserted, sorry.
 > 
 > > "Fix for the following patch" is also something that really should
 > > never be added to a git changelog too, because 'next' means absolutely
 > > nothing to me, nor I expect 99% of changelog readers.
 > 
 > 184c44d2049c4db7ef6ec65794546954da2c6a0e should never have existed,
 > actually.  I intended that Andi fold it into the base patch prior to
 > sending it to Linus.  He normally does that, but it looks like this
 > one was handled as a standalone commit for some reason.

So lets see what happens if we revert it ?

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Andrew Morton]

On Tue, 25 Sep 2007 12:58:34 -0400 Dave Jones <davej@redhat.com> wrote:

> On Tue, Sep 25, 2007 at 09:52:29AM -0700, Andrew Morton wrote:
>  > On Tue, 25 Sep 2007 10:36:51 -0400 Dave Jones <davej@redhat.com> wrote:
>  > > 
>  > > commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
>  > > Author: Andrew Morton <akpm@linux-foundation.org>
>  > > Date:   Wed May 2 19:27:08 2007 +0200
>  > > 
>  > >     [PATCH] x86-64: fix x86_64-mm-sched-clock-share
>  > >     
>  > >     Fix for the following patch. Provide dummy cpufreq functions when
>  > >     CPUFREQ is not compiled in.
>  > >     
>  > >     Cc: Andi Kleen <ak@suse.de>
>  > >     Cc: Dave Jones <davej@codemonkey.org.uk>
>  > >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>  > >     Signed-off-by: Andi Kleen <ak@suse.de>
>  > > 
>  > > I don't remember seeing any problem here, so I'm not entirely sure what
>  > > this was supposed to be fixing.  Perhaps the -mm-esque patch name
>  > > will provide Andrew/Andi clues. It lacks sufficient information for
>  > > my brain to guess what the problem was.
>  > 
>  > Oh geeze.  sched-clock-share went through about 18 different versions, was
>  > merged, unmerged, remerged, dropped, etc.  I don't recall at what stage in
>  > this mess the above fix was inserted, sorry.
>  > 
>  > > "Fix for the following patch" is also something that really should
>  > > never be added to a git changelog too, because 'next' means absolutely
>  > > nothing to me, nor I expect 99% of changelog readers.
>  > 
>  > 184c44d2049c4db7ef6ec65794546954da2c6a0e should never have existed,
>  > actually.  I intended that Andi fold it into the base patch prior to
>  > sending it to Linus.  He normally does that, but it looks like this
>  > one was handled as a standalone commit for some reason.
> 
> So lets see what happens if we revert it ?
> 

<grep flurry>

OK, here: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc5/2.6.21-rc5-mm3/broken-out/fix-x86_64-mm-sched-clock-share.patch

So I guess what we want to do here is to revert that patch, test i386
allnoconfig and then fix up anything which breaks.

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Dave Jones]

On Tue, Sep 25, 2007 at 10:08:39AM -0700, Andrew Morton wrote:
 > On Tue, 25 Sep 2007 12:58:34 -0400 Dave Jones <davej@redhat.com> wrote:
 > 
 > > On Tue, Sep 25, 2007 at 09:52:29AM -0700, Andrew Morton wrote:
 > >  > On Tue, 25 Sep 2007 10:36:51 -0400 Dave Jones <davej@redhat.com> wrote:
 > >  > > 
 > >  > > commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
 > >  > > Author: Andrew Morton <akpm@linux-foundation.org>
 > >  > > Date:   Wed May 2 19:27:08 2007 +0200
 > >  > > 
 > >  > >     [PATCH] x86-64: fix x86_64-mm-sched-clock-share
 > >  > >     
 > >  > >     Fix for the following patch. Provide dummy cpufreq functions when
 > >  > >     CPUFREQ is not compiled in.
 > >  > >     
 > >  > >     Cc: Andi Kleen <ak@suse.de>
 > >  > >     Cc: Dave Jones <davej@codemonkey.org.uk>
 > >  > >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
 > >  > >     Signed-off-by: Andi Kleen <ak@suse.de>
 > >  > > 
 > >  > > I don't remember seeing any problem here, so I'm not entirely sure what
 > >  > > this was supposed to be fixing.  Perhaps the -mm-esque patch name
 > >  > > will provide Andrew/Andi clues. It lacks sufficient information for
 > >  > > my brain to guess what the problem was.
 > >  > 
 > >  > Oh geeze.  sched-clock-share went through about 18 different versions, was
 > >  > merged, unmerged, remerged, dropped, etc.  I don't recall at what stage in
 > >  > this mess the above fix was inserted, sorry.
 > >  > 
 > >  > > "Fix for the following patch" is also something that really should
 > >  > > never be added to a git changelog too, because 'next' means absolutely
 > >  > > nothing to me, nor I expect 99% of changelog readers.
 > >  > 
 > >  > 184c44d2049c4db7ef6ec65794546954da2c6a0e should never have existed,
 > >  > actually.  I intended that Andi fold it into the base patch prior to
 > >  > sending it to Linus.  He normally does that, but it looks like this
 > >  > one was handled as a standalone commit for some reason.
 > > 
 > > So lets see what happens if we revert it ?
 > > 
 > 
 > <grep flurry>
 > 
 > OK, here: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc5/2.6.21-rc5-mm3/broken-out/fix-x86_64-mm-sched-clock-share.patch
 > 
 > So I guess what we want to do here is to revert that patch, test i386
 > allnoconfig and then fix up anything which breaks.

Nothing breaks for me with make ARCH=i386 bzImage on my x86-64 box
(which should be the same as a native build).

The functions that complain in that patch header don't seem to actually
exist in mainline at all. (`init_sched_clock' and `call_r_s_f')
Did this patch perhaps jump the gun, and these are -mm only ?

	Dave

-- 
http://www.codemonkey.org.uk

RE: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Greg.Chandler]

Well then, now I guess I know why the FB isn't working...
Sorry to open up the can of worms this turned out to be {after reading
all the other replies first}...

 

-----Original Message-----
From: Russell King [mailto:rmk@arm.linux.org.uk] On Behalf Of Russell
King
Sent: Tuesday, September 25, 2007 2:32 AM
To: Chandler, Greg; cpufreq@lists.linux.org.uk; davej@redhat.com
Cc: linux-kernel@vger.kernel.org; dan.j.williams@intel.com
Subject: Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more}
ARM/StrongARM

On Mon, Sep 24, 2007 at 05:53:57PM -0500, Greg.Chandler@wellsfargo.com
wrote:
> I was building a kernel for an iPaq {SA1110} and ran into this.
> 
> linux-2.6.22.7/arch/arm/mach-sa1100/generic.c:
> Has a: #include <linux/cpufreq.h>
> Then afterwards there is a: #if defined(CONFIG_CPU_FREQ_SA1100) ||
> defined(CONFIG_CPU_FREQ_SA1110)
> who's else section redefines the cpufreq_get function inhereited from 
> the header....
> 
> I'm guessing no one ever ended up in the "else" section until now, and

> that the header was added some time ago and no one caught this.
> This patch worked for me to get rid of the compile time problems.  I'm

> having issues with the kernel, but as far as I can tell they are form 
> the Frame buffer and not because of this.  If this assessment is 
> correct {the not needing this code anymore} then please pass this 
> along so it makes it into an upcoming release.
> 
> --- linux-2.6.22.7/arch/arm/mach-sa1100/generic.c.orig  2007-09-24 
> 17:36:21.000000000 -0500
> +++ linux-2.6.22.7/arch/arm/mach-sa1100/generic.c       2007-09-24
> 17:40:02.000000000 -0500
> @@ -107,15 +107,6 @@ unsigned int sa11x0_getspeed(unsigned in
>         return cclk_frequency_100khz[PPCR & 0xf] * 100;  }
> 
> -#else
> -/*
> - * We still need to provide this so building without cpufreq works.
> - */
> -unsigned int cpufreq_get(unsigned int cpu) -{
> -       return cclk_frequency_100khz[PPCR & 0xf] * 100;
> -}
> -EXPORT_SYMBOL(cpufreq_get);
>  #endif
> 
>  /*

No.  That code is required - the StrongARM 1100 framebuffer driver
*needs* to know what the CPU frequency is so it can set the pixel clock
divisor.

The real problem is the silly people who added this to cpufreq.h:

#ifdef CONFIG_CPU_FREQ
unsigned int cpufreq_quick_get(unsigned int cpu); unsigned int
cpufreq_get(unsigned int cpu); #else static inline unsigned int
cpufreq_quick_get(unsigned int cpu) {
        return 0;
}
static inline unsigned int cpufreq_get(unsigned int cpu) {
        return 0;
}
#endif

which utterly bogus.

--
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Andrew Morton]

On Tue, 25 Sep 2007 13:22:55 -0400 Dave Jones <davej@redhat.com> wrote:

>  > 
>  > OK, here: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc5/2.6.21-rc5-mm3/broken-out/fix-x86_64-mm-sched-clock-share.patch
>  > 
>  > So I guess what we want to do here is to revert that patch, test i386
>  > allnoconfig and then fix up anything which breaks.
> 
> Nothing breaks for me with make ARCH=i386 bzImage on my x86-64 box
> (which should be the same as a native build).

Was that with allnoconfig?

> The functions that complain in that patch header don't seem to actually
> exist in mainline at all. (`init_sched_clock' and `call_r_s_f')
> Did this patch perhaps jump the gun, and these are -mm only ?

Could be that this patch fixed version 17 of sched-clock-share and we ended
up merging verion 56.  It was awful.

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Dave Jones]

On Tue, Sep 25, 2007 at 10:31:42AM -0700, Andrew Morton wrote:
 > On Tue, 25 Sep 2007 13:22:55 -0400 Dave Jones <davej@redhat.com> wrote:
 > 
 > >  > 
 > >  > OK, here: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc5/2.6.21-rc5-mm3/broken-out/fix-x86_64-mm-sched-clock-share.patch
 > >  > 
 > >  > So I guess what we want to do here is to revert that patch, test i386
 > >  > allnoconfig and then fix up anything which breaks.
 > > 
 > > Nothing breaks for me with make ARCH=i386 bzImage on my x86-64 box
 > > (which should be the same as a native build).
 > 
 > Was that with allnoconfig?

yeah.

 > > The functions that complain in that patch header don't seem to actually
 > > exist in mainline at all. (`init_sched_clock' and `call_r_s_f')
 > > Did this patch perhaps jump the gun, and these are -mm only ?
 > 
 > Could be that this patch fixed version 17 of sched-clock-share and we ended
 > up merging verion 56.  It was awful.

heh.

I think just reverting that change for .23 makes sense. It doesn't
seem that anything breaks by not having it there, and we know it
definitly breaks arm at least.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Dave Jones]

On Tue, Sep 25, 2007 at 10:36:51AM -0400, Dave Jones wrote:
 > On Tue, Sep 25, 2007 at 08:31:32AM +0100, Russell King wrote:
 >  > On Mon, Sep 24, 2007 at 05:53:57PM -0500, Greg.Chandler@wellsfargo.com wrote:
 >  > > I was building a kernel for an iPaq {SA1110} and ran into this.
 >  > > 
 >  > > linux-2.6.22.7/arch/arm/mach-sa1100/generic.c:
 >  > > Has a: #include <linux/cpufreq.h>
 >  > > Then afterwards there is a: #if defined(CONFIG_CPU_FREQ_SA1100) ||
 >  > > defined(CONFIG_CPU_FREQ_SA1110)
 >  > > who's else section redefines the cpufreq_get function inhereited from
 >  > > the header....
 >  > > 
 >  > > I'm guessing no one ever ended up in the "else" section until now, and
 >  > > that the header was added some time ago and no one caught this.
 >  > > This patch worked for me to get rid of the compile time problems.  I'm
 >  > > having issues with the kernel, but as far as I can tell they are form
 >  > > the Frame buffer and not because of this.  If this assessment is correct
 >  > > {the not needing this code anymore} then please pass this along so it
 >  > > makes it into an upcoming release.
 >  > > 
 >  > > --- linux-2.6.22.7/arch/arm/mach-sa1100/generic.c.orig  2007-09-24
 >  > > 17:36:21.000000000 -0500
 >  > > +++ linux-2.6.22.7/arch/arm/mach-sa1100/generic.c       2007-09-24
 >  > > 17:40:02.000000000 -0500
 >  > > @@ -107,15 +107,6 @@ unsigned int sa11x0_getspeed(unsigned in
 >  > >         return cclk_frequency_100khz[PPCR & 0xf] * 100;
 >  > >  }
 >  > > 
 >  > > -#else
 >  > > -/*
 >  > > - * We still need to provide this so building without cpufreq works.
 >  > > - */
 >  > > -unsigned int cpufreq_get(unsigned int cpu)
 >  > > -{
 >  > > -       return cclk_frequency_100khz[PPCR & 0xf] * 100;
 >  > > -}
 >  > > -EXPORT_SYMBOL(cpufreq_get);
 >  > >  #endif
 >  > > 
 >  > >  /*
 >  > 
 >  > No.  That code is required - the StrongARM 1100 framebuffer driver
 >  > *needs* to know what the CPU frequency is so it can set the pixel
 >  > clock divisor.
 >  > 
 >  > The real problem is the silly people who added this to cpufreq.h:
 >  > 
 >  > #ifdef CONFIG_CPU_FREQ
 >  > unsigned int cpufreq_quick_get(unsigned int cpu);
 >  > unsigned int cpufreq_get(unsigned int cpu);
 >  > #else
 >  > static inline unsigned int cpufreq_quick_get(unsigned int cpu)
 >  > {
 >  >         return 0;
 >  > }
 >  > static inline unsigned int cpufreq_get(unsigned int cpu)
 >  > {
 >  >         return 0;
 >  > }
 >  > #endif
 >  > 
 >  > which utterly bogus.
 > 
 > Which came from ...
 > 
 > commit 184c44d2049c4db7ef6ec65794546954da2c6a0e
 > Author: Andrew Morton <akpm@linux-foundation.org>
 > Date:   Wed May 2 19:27:08 2007 +0200
 > 
 >     [PATCH] x86-64: fix x86_64-mm-sched-clock-share
 >     
 >     Fix for the following patch. Provide dummy cpufreq functions when
 >     CPUFREQ is not compiled in.
 >     
 >     Cc: Andi Kleen <ak@suse.de>
 >     Cc: Dave Jones <davej@codemonkey.org.uk>
 >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
 >     Signed-off-by: Andi Kleen <ak@suse.de>
 > 

Following up on this from yesterday, Linus please revert the above cset.
It doesn't seem to be necessary (it was added to fix a miscompile in
'make allnoconfig' which doesn't seem to be repeatable with it reverted)
and actively breaks the ARM SA1100 framebuffer driver.

(If you'd prefer a patch reverting it, I'll send one, but I'm
 hoping that git-revert will just dtrt).

Thanks,

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 1/1] Kernel compile bug in 2.6.22.6/7 {maybe more} ARM/StrongARM

[Russell King]

On Wed, Sep 26, 2007 at 04:53:14PM -0400, Dave Jones wrote:
> Following up on this from yesterday, Linus please revert the above cset.
> It doesn't seem to be necessary (it was added to fix a miscompile in
> 'make allnoconfig' which doesn't seem to be repeatable with it reverted)
> and actively breaks the ARM SA1100 framebuffer driver.
> 
> (If you'd prefer a patch reverting it, I'll send one, but I'm
>  hoping that git-revert will just dtrt).

Dave,

Thanks for checking this out on x86, and getting it reverted.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of: