* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-25 14:19 [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data Dave Young
@ 2007-07-25 6:40 ` Andrew Morton
2007-07-26 4:20 ` Dave Young
2007-07-26 7:55 ` Hiroshi Miura
0 siblings, 2 replies; 8+ messages in thread
From: Andrew Morton @ 2007-07-25 6:40 UTC (permalink / raw)
To: Dave Young; +Cc: linux-kernel, Dave Jones, cpufreq
On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
> Hi,
> in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>
> ---
> arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> struct pci_dev *gx_pci = NULL;
>
> /* check if CPU is a MediaGX or a Geode. */
> - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> dprintk("error: no MediaGX/Geode processor found!\n");
> return NULL;
> }
um, I suspect it really wants to get at the current CPU. But putting a
preempt_disable() around just that code is meaningless: the current CPU
could change immediately before or after the code block. It needs deeper
fixing, methinks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
@ 2007-07-25 14:19 Dave Young
2007-07-25 6:40 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Dave Young @ 2007-07-25 14:19 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
Hi,
in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
---
arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
--- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
+++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
@@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
struct pci_dev *gx_pci = NULL;
/* check if CPU is a MediaGX or a Geode. */
- if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
- (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
+ if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
+ (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
dprintk("error: no MediaGX/Geode processor found!\n");
return NULL;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-25 6:40 ` Andrew Morton
@ 2007-07-26 4:20 ` Dave Young
2007-07-26 4:26 ` Andrew Morton
2007-07-26 7:55 ` Hiroshi Miura
1 sibling, 1 reply; 8+ messages in thread
From: Dave Young @ 2007-07-26 4:20 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Dave Jones, cpufreq
>On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
>
> > Hi,
> > in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
> >
> > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> >
> > ---
> > arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> > --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> > +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> > @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> > struct pci_dev *gx_pci = NULL;
> >
> > /* check if CPU is a MediaGX or a Geode. */
> > - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > dprintk("error: no MediaGX/Geode processor found!\n");
> > return NULL;
> > }
>
> um, I suspect it really wants to get at the current CPU. But putting a
> preempt_disable() around just that code is meaningless: the current CPU
> could change immediately before or after the code block. It needs deeper
> fixing, methinks.
The only target is to get the cpu vendor, so boot_cpu_data is enough,
the drivers/mtd/nand/cs553x_nand.c has the same usage.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-26 4:20 ` Dave Young
@ 2007-07-26 4:26 ` Andrew Morton
2007-07-26 16:46 ` Dave Jones
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2007-07-26 4:26 UTC (permalink / raw)
To: Dave Young; +Cc: linux-kernel, Dave Jones, cpufreq
On Thu, 26 Jul 2007 04:20:10 +0000 "Dave Young" <hidave.darkstar@gmail.com> wrote:
> >On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
> >
> > > Hi,
> > > in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
> > >
> > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> > >
> > > ---
> > > arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> > > --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> > > +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> > > @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> > > struct pci_dev *gx_pci = NULL;
> > >
> > > /* check if CPU is a MediaGX or a Geode. */
> > > - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > dprintk("error: no MediaGX/Geode processor found!\n");
> > > return NULL;
> > > }
> >
> > um, I suspect it really wants to get at the current CPU. But putting a
> > preempt_disable() around just that code is meaningless: the current CPU
> > could change immediately before or after the code block. It needs deeper
> > fixing, methinks.
> The only target is to get the cpu vendor, so boot_cpu_data is enough,
> the drivers/mtd/nand/cs553x_nand.c has the same usage.
I think there's some vague ambition in there to support non-identical CPUs.
In which case reading from the local CPU would make more sense. (waves
frantically at cpufreq developers).
otoh, it'll take some work I suspect. It'll need to sort out the overall
scope of "local cpu". At what point and for how long should this code pin
itself on a cpu?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-25 6:40 ` Andrew Morton
2007-07-26 4:20 ` Dave Young
@ 2007-07-26 7:55 ` Hiroshi Miura
1 sibling, 0 replies; 8+ messages in thread
From: Hiroshi Miura @ 2007-07-26 7:55 UTC (permalink / raw)
To: Andrew Morton; +Cc: Dave Young, Dave Jones, linux-kernel, cpufreq
2007/7/25, Andrew Morton <akpm@linux-foundation.org>:
> On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
>
> > Hi,
> > in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
> >
> > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> >
> > ---
> > arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> > --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> > +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> > @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> > struct pci_dev *gx_pci = NULL;
> >
> > /* check if CPU is a MediaGX or a Geode. */
> > - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > dprintk("error: no MediaGX/Geode processor found!\n");
> > return NULL;
> > }
>
> um, I suspect it really wants to get at the current CPU. But putting a
> preempt_disable() around just that code is meaningless: the current CPU
> could change immediately before or after the code block. It needs deeper
> fixing, methinks.
I think we can remove these part. It checks chipset 's pci id
(vendor id/device id) after this part as follows;
/* detect which companion chip is used */
while ((gx_pci = pci_get_device(PCI_ANY_ID, PCI_ANY_ID,
gx_pci)) != NULL) {
if ((pci_match_id(gx_chipset_tbl, gx_pci)) != NULL)
return gx_pci;
}
It drives cpu frequency through chipset register, not CPU regsiter.
It may be enough to check chipset pci id.
Hiroshi
--
HIroshi Miura
NTT DATA Corp. and IPA OSS center
miura@da-cha.org
(株)NTTデータ /(独)情報処理推進機構
三浦広志
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-26 4:26 ` Andrew Morton
@ 2007-07-26 16:46 ` Dave Jones
0 siblings, 0 replies; 8+ messages in thread
From: Dave Jones @ 2007-07-26 16:46 UTC (permalink / raw)
To: Andrew Morton; +Cc: Dave Young, linux-kernel, Dave Jones, cpufreq
On Wed, Jul 25, 2007 at 09:26:38PM -0700, Andrew Morton wrote:
> On Thu, 26 Jul 2007 04:20:10 +0000 "Dave Young" <hidave.darkstar@gmail.com> wrote:
>
> > >On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
> > >
> > > > Hi,
> > > > in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
> > > >
> > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> > > >
> > > > ---
> > > > arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> > > > --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> > > > +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> > > > @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> > > > struct pci_dev *gx_pci = NULL;
> > > >
> > > > /* check if CPU is a MediaGX or a Geode. */
> > > > - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > > - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > > + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > > + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > > dprintk("error: no MediaGX/Geode processor found!\n");
> > > > return NULL;
> > > > }
> > >
> > > um, I suspect it really wants to get at the current CPU. But putting a
> > > preempt_disable() around just that code is meaningless: the current CPU
> > > could change immediately before or after the code block. It needs deeper
> > > fixing, methinks.
> > The only target is to get the cpu vendor, so boot_cpu_data is enough,
> > the drivers/mtd/nand/cs553x_nand.c has the same usage.
>
> I think there's some vague ambition in there to support non-identical CPUs.
> In which case reading from the local CPU would make more sense. (waves
> frantically at cpufreq developers).
a non-identical CPU system with a geode + something else doesn't exist
and is very unlikely to happen.
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
@ 2007-07-26 16:46 ` Dave Jones
0 siblings, 0 replies; 8+ messages in thread
From: Dave Jones @ 2007-07-26 16:46 UTC (permalink / raw)
To: Andrew Morton; +Cc: Dave Jones, cpufreq, Dave Young, linux-kernel
On Wed, Jul 25, 2007 at 09:26:38PM -0700, Andrew Morton wrote:
> On Thu, 26 Jul 2007 04:20:10 +0000 "Dave Young" <hidave.darkstar@gmail.com> wrote:
>
> > >On 7/25/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Wed, 25 Jul 2007 14:19:05 +0000 Dave Young <hidave.darkstar@gmail.com> wrote:
> > >
> > > > Hi,
> > > > in preemptible kernel will report BUG: using smp_processor_id() in preemptible, so use boot_cpu_data instead of current_cpu_data.
> > > >
> > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> > > >
> > > > ---
> > > > arch/i386/kernel/cpu/cpufreq/gx-suspmod.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff -pur linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> > > > --- linux/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 14:11:06.000000000 +0000
> > > > +++ linux.new/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c 2007-07-25 13:57:29.000000000 +0000
> > > > @@ -181,8 +181,8 @@ static __init struct pci_dev *gx_detect_
> > > > struct pci_dev *gx_pci = NULL;
> > > >
> > > > /* check if CPU is a MediaGX or a Geode. */
> > > > - if ((current_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > > - (current_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > > + if ((boot_cpu_data.x86_vendor != X86_VENDOR_NSC) &&
> > > > + (boot_cpu_data.x86_vendor != X86_VENDOR_CYRIX)) {
> > > > dprintk("error: no MediaGX/Geode processor found!\n");
> > > > return NULL;
> > > > }
> > >
> > > um, I suspect it really wants to get at the current CPU. But putting a
> > > preempt_disable() around just that code is meaningless: the current CPU
> > > could change immediately before or after the code block. It needs deeper
> > > fixing, methinks.
> > The only target is to get the cpu vendor, so boot_cpu_data is enough,
> > the drivers/mtd/nand/cs553x_nand.c has the same usage.
>
> I think there's some vague ambition in there to support non-identical CPUs.
> In which case reading from the local CPU would make more sense. (waves
> frantically at cpufreq developers).
a non-identical CPU system with a geode + something else doesn't exist
and is very unlikely to happen.
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data
2007-07-26 16:46 ` Dave Jones
(?)
@ 2007-07-26 17:11 ` Alan Cox
-1 siblings, 0 replies; 8+ messages in thread
From: Alan Cox @ 2007-07-26 17:11 UTC (permalink / raw)
To: Dave Jones; +Cc: Andrew Morton, Dave Young, linux-kernel, Dave Jones, cpufreq
O> a non-identical CPU system with a geode + something else doesn't exist
> and is very unlikely to happen.
As the Geode isn't used SMP (and afaik isnt SMP capable) that would be a
reasonable assumption
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-07-26 17:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-25 14:19 [PATCH]gx-suspmod.c use boot_cpu_data instead of current_cpu_data Dave Young
2007-07-25 6:40 ` Andrew Morton
2007-07-26 4:20 ` Dave Young
2007-07-26 4:26 ` Andrew Morton
2007-07-26 16:46 ` Dave Jones
2007-07-26 16:46 ` Dave Jones
2007-07-26 17:11 ` Alan Cox
2007-07-26 7:55 ` Hiroshi Miura
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.