Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi,
On Friday, 25 of March 2005 01:49, you wrote:
]--snip--[
> > > I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and
> > > definitely required for S3). Unclear if it's ok for S4, so please try
> > > revert the patch.
> >
> > 2.6.11-rc1-mm1 with the patch reverted works fine. :-)
> So just remove the pci_enable/disable_device call in the driver makes
> the system work?
I'm a bit confused. :-) I'm not sure if the patch that I have reverted is
related
to pci_enable/disable_device. It's this one:
diff -Naru a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
--- a/drivers/acpi/pci_link.c 2005-03-24 04:57:27 -08:00
+++ b/drivers/acpi/pci_link.c 2005-03-24 04:57:27 -08:00
@@ -72,10 +72,12 @@
u8 active; /* Current IRQ */
u8 edge_level; /* All IRQs */
u8 active_high_low;/* All IRQs */
- u8 initialized;
u8 resource_type;
u8 possible_count;
u8 possible[ACPI_PCI_LINK_MAX_POSSIBLE];
+ u8 initialized:1;
+ u8 suspend_resume:1;
+ u8 reserved:6;
};
struct acpi_pci_link {
@@ -530,6 +532,10 @@
ACPI_FUNCTION_TRACE("acpi_pci_link_allocate");
+ if (link->irq.suspend_resume) {
+ acpi_pci_link_set(link, link->irq.active);
+ link->irq.suspend_resume = 0;
+ }
if (link->irq.initialized)
return_VALUE(0);
@@ -713,38 +719,24 @@
return_VALUE(result);
}
-
-static int
-acpi_pci_link_resume (
- struct acpi_pci_link*link)
-{
- ACPI_FUNCTION_TRACE("acpi_pci_link_resume");
-
- if (link->irq.active && link->irq.initialized)
- return_VALUE(acpi_pci_link_set(link, link->irq.active));
- else
- return_VALUE(0);
-}
-
-
static int
-irqrouter_resume(
- struct sys_device *dev)
+irqrouter_suspend(
+ struct sys_device *dev,
+ u32 state)
{
struct list_head*node = NULL;
struct acpi_pci_link*link = NULL;
- ACPI_FUNCTION_TRACE("irqrouter_resume");
+ ACPI_FUNCTION_TRACE("irqrouter_suspend");
list_for_each(node, _link.entries) {
-
link = list_entry(node, struct acpi_pci_link, node);
if (!link) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid link
context\n"));
continue;
}
-
- acpi_pci_link_resume(link);
+ if (link->irq.active && link->irq.initialized)
+ link->irq.suspend_resume = 1;
}
return_VALUE(0);
}
@@ -856,7 +848,7 @@
static struct sysdev_class irqrouter_sysdev_class = {
set_kset_name("irqrouter"),
-.resume = irqrouter_resume,
+.suspend = irqrouter_suspend,
};
Greets,
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi,
On Friday, 25 of March 2005 01:49, you wrote:
]--snip--[
I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and
definitely required for S3). Unclear if it's ok for S4, so please try
revert the patch.
2.6.11-rc1-mm1 with the patch reverted works fine. :-)
So just remove the pci_enable/disable_device call in the driver makes
the system work?
I'm a bit confused. :-) I'm not sure if the patch that I have reverted is
related
to pci_enable/disable_device. It's this one:
diff -Naru a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
--- a/drivers/acpi/pci_link.c 2005-03-24 04:57:27 -08:00
+++ b/drivers/acpi/pci_link.c 2005-03-24 04:57:27 -08:00
@@ -72,10 +72,12 @@
u8 active; /* Current IRQ */
u8 edge_level; /* All IRQs */
u8 active_high_low;/* All IRQs */
- u8 initialized;
u8 resource_type;
u8 possible_count;
u8 possible[ACPI_PCI_LINK_MAX_POSSIBLE];
+ u8 initialized:1;
+ u8 suspend_resume:1;
+ u8 reserved:6;
};
struct acpi_pci_link {
@@ -530,6 +532,10 @@
ACPI_FUNCTION_TRACE(acpi_pci_link_allocate);
+ if (link-irq.suspend_resume) {
+ acpi_pci_link_set(link, link-irq.active);
+ link-irq.suspend_resume = 0;
+ }
if (link-irq.initialized)
return_VALUE(0);
@@ -713,38 +719,24 @@
return_VALUE(result);
}
-
-static int
-acpi_pci_link_resume (
- struct acpi_pci_link*link)
-{
- ACPI_FUNCTION_TRACE(acpi_pci_link_resume);
-
- if (link-irq.active link-irq.initialized)
- return_VALUE(acpi_pci_link_set(link, link-irq.active));
- else
- return_VALUE(0);
-}
-
-
static int
-irqrouter_resume(
- struct sys_device *dev)
+irqrouter_suspend(
+ struct sys_device *dev,
+ u32 state)
{
struct list_head*node = NULL;
struct acpi_pci_link*link = NULL;
- ACPI_FUNCTION_TRACE(irqrouter_resume);
+ ACPI_FUNCTION_TRACE(irqrouter_suspend);
list_for_each(node, acpi_link.entries) {
-
link = list_entry(node, struct acpi_pci_link, node);
if (!link) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR, Invalid link
context\n));
continue;
}
-
- acpi_pci_link_resume(link);
+ if (link-irq.active link-irq.initialized)
+ link-irq.suspend_resume = 1;
}
return_VALUE(0);
}
@@ -856,7 +848,7 @@
static struct sysdev_class irqrouter_sysdev_class = {
set_kset_name(irqrouter),
-.resume = irqrouter_resume,
+.suspend = irqrouter_suspend,
};
Greets,
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll Alice's Adventures in Wonderland
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Thu, 2005-03-24 at 21:42, Rafael J. Wysocki wrote: > Hi, > > On Thursday, 24 of March 2005 02:27, Li Shaohua wrote: > > On Thu, 2005-03-24 at 09:03, Len Brown wrote: > > > On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: > > > > Hi, > > > > > > > > On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: > > > > > Hi! > > > > > > > > > > > > > > Will this do it for the moment? > > > > > > > > > > > > > > > > Its certainly better. > > > > > > > > > > > > > > With the Len's patch applied I have to unload the modules: > > > > > > > > > > > > > > ohci_hcd > > > > > > > ehci_hcd > > > > > > > yenta_socket > > > > > > > > > > > > > > before suspend as each of them hangs the box solid during > > > either > > > > > > > suspend or resume. Moreover, when I tried to load the > > > ehci_hcd > > > > > > > module back after resume, it hanged the box solid too. > > > > > > Is this failure with suspend to RAM or to disk? > > > > > > How about if you try this patch? > > > > > > http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] > > > > > > patch -Rp1 from 2.6.12-rc1-mm1 and see if it stops being broken > > > or patch -Np1 to 2.6.12-rc and see if it starts being broken. > > > > > > This one removes an earlier attempt at resuming PCI links -- now > > > putting the onus on the drivers to be properly written > > > to release and acquire their interrupt for a successful > > > suspend/resume. > > > > > > > > > In theory, this is taken care of something like this: > > > driver.resume > > > pci_enable_device > > > pci_enable_device_bars > > > pcibios_enable_device > > > pcibios_enable_irq > > > acpi_pci_irq_enable > > > > > > but if the patch above makes a difference, then theory != practice:-) > > It looks like that. ;-) > > > > I'd believe that ohci_hcd and ehci_hcd are fragile since glancing > > > at their lengthy .resume routines it isn't immediately obvious > > > that they do this. But yenta_dev_resume has a pci_enable_device(), > > > so that failure may be less straightforward. > > > > > > cheers, > > > -Len > > > > > > ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled > > > boot, that would help -- for it will show if we're even using pci > > > interrupt links (and programming them) for these devices on this box. > > Yes, we changed the behavior of device suspend/resume. Every PCI device > > should call 'pci_disable_device' at suspend and call 'pci_enable_device' > > at resume. It fixes a bug and more important thing is it's safer (Eg. it > > disable interrupts, bus master and etc). > > I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and > > definitely required for S3). Unclear if it's ok for S4, so please try > > revert the patch. > > 2.6.11-rc1-mm1 with the patch reverted works fine. :-) So just remove the pci_enable/disable_device call in the driver makes the system work? Strange, I tried them on two laptops (one HP nx5000, and one Toshiba M2N), both works (no hang, and USB mouse works after S3/S4. I didn't try yenta, since I have no pc card) for S3/S4. Is it possible it's another bug or just because of different BIOS? Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Thursday, 24 of March 2005 02:03, Len Brown wrote: > On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: ]-- snip --[ > I'd believe that ohci_hcd and ehci_hcd are fragile since glancing > at their lengthy .resume routines it isn't immediately obvious > that they do this. But yenta_dev_resume has a pci_enable_device(), > so that failure may be less straightforward. > > cheers, > -Len > > ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled > boot, that would help -- for it will show if we're even using pci > interrupt links (and programming them) for these devices on this box. The dmesg output is at: http://www.sisk.pl/kernel/050325/2.6.11-rc1-dmesg.log Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll "Alice's Adventures in Wonderland" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Thursday, 24 of March 2005 02:27, Li Shaohua wrote: > On Thu, 2005-03-24 at 09:03, Len Brown wrote: > > On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: > > > Hi, > > > > > > On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: > > > > Hi! > > > > > > > > > > > > Will this do it for the moment? > > > > > > > > > > > > > > Its certainly better. > > > > > > > > > > > > With the Len's patch applied I have to unload the modules: > > > > > > > > > > > > ohci_hcd > > > > > > ehci_hcd > > > > > > yenta_socket > > > > > > > > > > > > before suspend as each of them hangs the box solid during > > either > > > > > > suspend or resume. Moreover, when I tried to load the > > ehci_hcd > > > > > > module back after resume, it hanged the box solid too. > > > > Is this failure with suspend to RAM or to disk? > > > > How about if you try this patch? > > > > http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] > > > > patch -Rp1 from 2.6.12-rc1-mm1 and see if it stops being broken > > or patch -Np1 to 2.6.12-rc and see if it starts being broken. > > > > This one removes an earlier attempt at resuming PCI links -- now > > putting the onus on the drivers to be properly written > > to release and acquire their interrupt for a successful > > suspend/resume. > > > > > > In theory, this is taken care of something like this: > > driver.resume > > pci_enable_device > > pci_enable_device_bars > > pcibios_enable_device > > pcibios_enable_irq > > acpi_pci_irq_enable > > > > but if the patch above makes a difference, then theory != practice:-) It looks like that. ;-) > > I'd believe that ohci_hcd and ehci_hcd are fragile since glancing > > at their lengthy .resume routines it isn't immediately obvious > > that they do this. But yenta_dev_resume has a pci_enable_device(), > > so that failure may be less straightforward. > > > > cheers, > > -Len > > > > ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled > > boot, that would help -- for it will show if we're even using pci > > interrupt links (and programming them) for these devices on this box. > Yes, we changed the behavior of device suspend/resume. Every PCI device > should call 'pci_disable_device' at suspend and call 'pci_enable_device' > at resume. It fixes a bug and more important thing is it's safer (Eg. it > disable interrupts, bus master and etc). > I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and > definitely required for S3). Unclear if it's ok for S4, so please try > revert the patch. 2.6.11-rc1-mm1 with the patch reverted works fine. :-) Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll "Alice's Adventures in Wonderland" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! > > You can't put -ENODEV into pci_power_t ... but maybe we should create > > PCI_ERROR and pass it in cases like this one? > That makes sense, please do it. Added: #define PCI_POWER_ERROR ((pci_power_t __force) -1) Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! You can't put -ENODEV into pci_power_t ... but maybe we should create PCI_ERROR and pass it in cases like this one? That makes sense, please do it. Added: #define PCI_POWER_ERROR ((pci_power_t __force) -1) Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Thursday, 24 of March 2005 02:27, Li Shaohua wrote: On Thu, 2005-03-24 at 09:03, Len Brown wrote: On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: Hi, On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Is this failure with suspend to RAM or to disk? How about if you try this patch? http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] patch -Rp1 from 2.6.12-rc1-mm1 and see if it stops being broken or patch -Np1 to 2.6.12-rc and see if it starts being broken. This one removes an earlier attempt at resuming PCI links -- now putting the onus on the drivers to be properly written to release and acquire their interrupt for a successful suspend/resume. In theory, this is taken care of something like this: driver.resume pci_enable_device pci_enable_device_bars pcibios_enable_device pcibios_enable_irq acpi_pci_irq_enable but if the patch above makes a difference, then theory != practice:-) It looks like that. ;-) I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. Yes, we changed the behavior of device suspend/resume. Every PCI device should call 'pci_disable_device' at suspend and call 'pci_enable_device' at resume. It fixes a bug and more important thing is it's safer (Eg. it disable interrupts, bus master and etc). I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and definitely required for S3). Unclear if it's ok for S4, so please try revert the patch. 2.6.11-rc1-mm1 with the patch reverted works fine. :-) Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll Alice's Adventures in Wonderland - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Thursday, 24 of March 2005 02:03, Len Brown wrote: On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: ]-- snip --[ I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. The dmesg output is at: http://www.sisk.pl/kernel/050325/2.6.11-rc1-dmesg.log Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll Alice's Adventures in Wonderland - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Thu, 2005-03-24 at 21:42, Rafael J. Wysocki wrote: Hi, On Thursday, 24 of March 2005 02:27, Li Shaohua wrote: On Thu, 2005-03-24 at 09:03, Len Brown wrote: On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: Hi, On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Is this failure with suspend to RAM or to disk? How about if you try this patch? http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] patch -Rp1 from 2.6.12-rc1-mm1 and see if it stops being broken or patch -Np1 to 2.6.12-rc and see if it starts being broken. This one removes an earlier attempt at resuming PCI links -- now putting the onus on the drivers to be properly written to release and acquire their interrupt for a successful suspend/resume. In theory, this is taken care of something like this: driver.resume pci_enable_device pci_enable_device_bars pcibios_enable_device pcibios_enable_irq acpi_pci_irq_enable but if the patch above makes a difference, then theory != practice:-) It looks like that. ;-) I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. Yes, we changed the behavior of device suspend/resume. Every PCI device should call 'pci_disable_device' at suspend and call 'pci_enable_device' at resume. It fixes a bug and more important thing is it's safer (Eg. it disable interrupts, bus master and etc). I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and definitely required for S3). Unclear if it's ok for S4, so please try revert the patch. 2.6.11-rc1-mm1 with the patch reverted works fine. :-) So just remove the pci_enable/disable_device call in the driver makes the system work? Strange, I tried them on two laptops (one HP nx5000, and one Toshiba M2N), both works (no hang, and USB mouse works after S3/S4. I didn't try yenta, since I have no pc card) for S3/S4. Is it possible it's another bug or just because of different BIOS? Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Tue, 2005-03-22 at 20:20, Pavel Machek wrote:
> Hi!
>
> > >> > Yes, but it is needed. There are many drivers, and they look at
> > >> > numerical value of PMSG_*. I'm proceeding in steps. I hopefully
> > killed
> > >> > all direct accesses to the constants, and will switch constants
> to
> > >> > something else... But that is going to be tommorow (need some
> > sleep).
> > >> The patches are going to acquire correct PCI device sleep state
> for
> > >> suspend/resume. We discussed the issue several months ago. My
> plan is
> > we
> > >> first introduce 'platform_pci_set_power_state', then merge the
> > >> 'platform_pci_choose_state' patch after Pavel's pm_message_t
> > conversion
> > >> finished. Maybe Len mislead my comments.
> > >>
> > >> Anyway for the callback, my intend is platform_pci_choose_state
> > accept
> > >> the pm_message_t parameter, and it return an 'int', since
> platform
> > >> method possibly failed and then pci_choose_state translate the
> return
> > >> value to pci_power_t.
> > >
> > >You can't just retype around like that. You may want it take
> > >pci_power_t * as an argument, and then return 0/-ENODEV or
> something
> > >like that. But you can't retype between int and pm_message_t...
> > No, taking pci_power_t as an argument is meaningless. For ACPI, we
> > should know the exact sleep state, pm_message_t will tell us. But
> I'm ok
> > to let it return a pci_power_t, and the failure case returns
> > -ENODEV.
>
> You can't put -ENODEV into pci_power_t ... but maybe we should create
> PCI_ERROR and pass it in cases like this one?
That makes sense, please do it.
>
> > >> > Could you just revert those two patches? First one is very
> > >> > wrong. Second one might be fixed, but... See comments below.
> > >> I think the platform_pci_set_power_state should be ok, did you
> see it
> > >> causes oops?
> > >
> > >No its just ugly and uses __force in "creative" way. That one can
> be
> > >recovered.
> > Do you mean this?
> >
> > > + static int state_conv[] = {
> > > + [0] = 0,
> > > + [1] = 1,
> > > + [2] = 2,
> > > + [3] = 3,
> > > + [4] = 3
> > > + };
> > > + int acpi_state = state_conv[(int __force) state];
> >
> > The table should be
> > [PCI_D0] = 0,
> >
> > I'm not sure, but then could we use state_conv[state] directly? It
> seems
>
> I think so. Of course it is wrong, but it is less wrong than forcing
> it to integer than index, without using macros at all.
>
> Or perhaps you should do
>
> switch (state) {
> case PCI_D0: ...
> }
>
> ...and handle default case somehow.
That's ok for me. I'll change it later.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Thu, 2005-03-24 at 09:03, Len Brown wrote: > On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: > > Hi, > > > > On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: > > > Hi! > > > > > > > > > > Will this do it for the moment? > > > > > > > > > > > > Its certainly better. > > > > > > > > > > With the Len's patch applied I have to unload the modules: > > > > > > > > > > ohci_hcd > > > > > ehci_hcd > > > > > yenta_socket > > > > > > > > > > before suspend as each of them hangs the box solid during > either > > > > > suspend or resume. Moreover, when I tried to load the > ehci_hcd > > > > > module back after resume, it hanged the box solid too. > > Is this failure with suspend to RAM or to disk? > > How about if you try this patch? > > http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] > > patch -Rp1 from 2.6.12-rc1-mm and see if it stops being broken > or patch -Np1 to 2.6.12-rc and see if it starts being broken. > > This one removes an earlier attempt at resuming PCI links -- now > putting the onus on the drivers to be properly written > to release and acquire their interrupt for a successful > suspend/resume. > > > In theory, this is taken care of something like this: > driver.resume > pci_enable_device > pci_enable_device_bars > pcibios_enable_device > pcibios_enable_irq > acpi_pci_irq_enable > > but if the patch above makes a difference, then theory != practice:-) > > I'd believe that ohci_hcd and ehci_hcd are fragile since glancing > at their lengthy .resume routines it isn't immediately obvious > that they do this. But yenta_dev_resume has a pci_enable_device(), > so that failure may be less straightforward. > > cheers, > -Len > > ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled > boot, that would help -- for it will show if we're even using pci > interrupt links (and programming them) for these devices on this box. Yes, we changed the behavior of device suspend/resume. Every PCI device should call 'pci_disable_device' at suspend and call 'pci_enable_device' at resume. It fixes a bug and more important thing is it's safer (Eg. it disable interrupts, bus master and etc). I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and definitely required for S3). Unclear if it's ok for S4, so please try revert the patch. Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: > Hi, > > On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: > > Hi! > > > > > > > > Will this do it for the moment? > > > > > > > > > > Its certainly better. > > > > > > > > With the Len's patch applied I have to unload the modules: > > > > > > > > ohci_hcd > > > > ehci_hcd > > > > yenta_socket > > > > > > > > before suspend as each of them hangs the box solid during either > > > > suspend or resume. Moreover, when I tried to load the ehci_hcd > > > > module back after resume, it hanged the box solid too. Is this failure with suspend to RAM or to disk? How about if you try this patch? http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] patch -Rp1 from 2.6.12-rc1-mm and see if it stops being broken or patch -Np1 to 2.6.12-rc and see if it starts being broken. This one removes an earlier attempt at resuming PCI links -- now putting the onus on the drivers to be properly written to release and acquire their interrupt for a successful suspend/resume. In theory, this is taken care of something like this: driver.resume pci_enable_device pci_enable_device_bars pcibios_enable_device pcibios_enable_irq acpi_pci_irq_enable but if the patch above makes a difference, then theory != practice:-) I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: > Hi! > > > > > > Will this do it for the moment? > > > > > > > > Its certainly better. > > > > > > With the Len's patch applied I have to unload the modules: > > > > > > ohci_hcd > > > ehci_hcd > > > yenta_socket > > > > > > before suspend as each of them hangs the box solid during either > > > suspend or resume. Moreover, when I tried to load the ehci_hcd > > > module back after resume, it hanged the box solid too. > > > > This behavior is apparently caused by the call to pci_write_config_word() > > with > > pmcsr = 0 in drivers/pci/pci.c:pci_set_power_state(). > > > > Well, I don't think I can do anything more about it myself. :-) > > Can you just revert those two patches from Len, and see what happens? Reverting them doesn't change anything, so there's something else that breaks things, apparently. Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll "Alice's Adventures in Wonderland" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi! > > > > Will this do it for the moment? > > > > > > Its certainly better. > > > > With the Len's patch applied I have to unload the modules: > > > > ohci_hcd > > ehci_hcd > > yenta_socket > > > > before suspend as each of them hangs the box solid during either > > suspend or resume. Moreover, when I tried to load the ehci_hcd > > module back after resume, it hanged the box solid too. > > This behavior is apparently caused by the call to pci_write_config_word() with > pmcsr = 0 in drivers/pci/pci.c:pci_set_power_state(). > > Well, I don't think I can do anything more about it myself. :-) Can you just revert those two patches from Len, and see what happens? Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Tuesday, 22 of March 2005 22:49, Rafael J. Wysocki wrote: > Hi, > > On Tuesday, 22 of March 2005 12:01, Pavel Machek wrote: > > Hi! > > > > > Will this do it for the moment? > > > > Its certainly better. > > With the Len's patch applied I have to unload the modules: > > ohci_hcd > ehci_hcd > yenta_socket > > before suspend as each of them hangs the box solid during either > suspend or resume. Moreover, when I tried to load the ehci_hcd > module back after resume, it hanged the box solid too. This behavior is apparently caused by the call to pci_write_config_word() with pmcsr = 0 in drivers/pci/pci.c:pci_set_power_state(). Well, I don't think I can do anything more about it myself. :-) Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll "Alice's Adventures in Wonderland" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Tuesday, 22 of March 2005 22:49, Rafael J. Wysocki wrote: Hi, On Tuesday, 22 of March 2005 12:01, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. This behavior is apparently caused by the call to pci_write_config_word() with pmcsr = 0 in drivers/pci/pci.c:pci_set_power_state(). Well, I don't think I can do anything more about it myself. :-) Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll Alice's Adventures in Wonderland - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. This behavior is apparently caused by the call to pci_write_config_word() with pmcsr = 0 in drivers/pci/pci.c:pci_set_power_state(). Well, I don't think I can do anything more about it myself. :-) Can you just revert those two patches from Len, and see what happens? Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: Hi, On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Is this failure with suspend to RAM or to disk? How about if you try this patch? http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] patch -Rp1 from 2.6.12-rc1-mm and see if it stops being broken or patch -Np1 to 2.6.12-rc and see if it starts being broken. This one removes an earlier attempt at resuming PCI links -- now putting the onus on the drivers to be properly written to release and acquire their interrupt for a successful suspend/resume. In theory, this is taken care of something like this: driver.resume pci_enable_device pci_enable_device_bars pcibios_enable_device pcibios_enable_irq acpi_pci_irq_enable but if the patch above makes a difference, then theory != practice:-) I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: resume regression [update] (was: Re:2.6.12-rc1-mm1: Kernel BUG at pci:389)
On Thu, 2005-03-24 at 09:03, Len Brown wrote: On Wed, 2005-03-23 at 18:49, Rafael J. Wysocki wrote: Hi, On Wednesday, 23 of March 2005 23:39, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Is this failure with suspend to RAM or to disk? How about if you try this patch? http://linux-acpi.bkbits.net:8080/to-akpm/[EMAIL PROTECTED] patch -Rp1 from 2.6.12-rc1-mm and see if it stops being broken or patch -Np1 to 2.6.12-rc and see if it starts being broken. This one removes an earlier attempt at resuming PCI links -- now putting the onus on the drivers to be properly written to release and acquire their interrupt for a successful suspend/resume. In theory, this is taken care of something like this: driver.resume pci_enable_device pci_enable_device_bars pcibios_enable_device pcibios_enable_irq acpi_pci_irq_enable but if the patch above makes a difference, then theory != practice:-) I'd believe that ohci_hcd and ehci_hcd are fragile since glancing at their lengthy .resume routines it isn't immediately obvious that they do this. But yenta_dev_resume has a pci_enable_device(), so that failure may be less straightforward. cheers, -Len ps. if point me to a full dmesg -s64000 from 2.6.12-rc1 acpi-enabled boot, that would help -- for it will show if we're even using pci interrupt links (and programming them) for these devices on this box. Yes, we changed the behavior of device suspend/resume. Every PCI device should call 'pci_disable_device' at suspend and call 'pci_enable_device' at resume. It fixes a bug and more important thing is it's safer (Eg. it disable interrupts, bus master and etc). I actually added such calls in uhci, ehci and yenta. It's ok for S3 (and definitely required for S3). Unclear if it's ok for S4, so please try revert the patch. Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Tue, 2005-03-22 at 20:20, Pavel Machek wrote:
Hi!
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully
killed
all direct accesses to the constants, and will switch constants
to
something else... But that is going to be tommorow (need some
sleep).
The patches are going to acquire correct PCI device sleep state
for
suspend/resume. We discussed the issue several months ago. My
plan is
we
first introduce 'platform_pci_set_power_state', then merge the
'platform_pci_choose_state' patch after Pavel's pm_message_t
conversion
finished. Maybe Len mislead my comments.
Anyway for the callback, my intend is platform_pci_choose_state
accept
the pm_message_t parameter, and it return an 'int', since
platform
method possibly failed and then pci_choose_state translate the
return
value to pci_power_t.
You can't just retype around like that. You may want it take
pci_power_t * as an argument, and then return 0/-ENODEV or
something
like that. But you can't retype between int and pm_message_t...
No, taking pci_power_t as an argument is meaningless. For ACPI, we
should know the exact sleep state, pm_message_t will tell us. But
I'm ok
to let it return a pci_power_t, and the failure case returns
-ENODEV.
You can't put -ENODEV into pci_power_t ... but maybe we should create
PCI_ERROR and pass it in cases like this one?
That makes sense, please do it.
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
I think the platform_pci_set_power_state should be ok, did you
see it
causes oops?
No its just ugly and uses __force in creative way. That one can
be
recovered.
Do you mean this?
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
I'm not sure, but then could we use state_conv[state] directly? It
seems
I think so. Of course it is wrong, but it is less wrong than forcing
it to integer than index, without using macros at all.
Or perhaps you should do
switch (state) {
case PCI_D0: ...
}
...and handle default case somehow.
That's ok for me. I'll change it later.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
2.6.12-rc1-mm1: resume regression (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Tuesday, 22 of March 2005 12:01, Pavel Machek wrote: > Hi! > > > Will this do it for the moment? > > Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll "Alice's Adventures in Wonderland" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
pm_message_t to struct conversion [was Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389]
Hi!
> to Linus when he reappears and then I'll duck for cover and let you guys
> sort it out ;)
There should be little reason for taking cover, that patches were just
anotating types... BTW this is how switch to pm_message_t is going to
look. If you are developing something pm-related, you should probably
do it against this tree.
Pavel
--- clean/drivers/base/power/resume.c 2004-12-25 13:34:59.0 +0100
+++ linux/drivers/base/power/resume.c 2005-03-22 12:20:53.0 +0100
@@ -41,7 +41,7 @@
list_add_tail(entry, _active);
up(_list_sem);
- if (!dev->power.prev_state)
+ if (!dev->power.prev_state.event)
resume_device(dev);
down(_list_sem);
put_device(dev);
--- clean/drivers/base/power/runtime.c 2005-01-12 11:07:39.0 +0100
+++ linux/drivers/base/power/runtime.c 2005-03-22 12:20:53.0 +0100
@@ -13,10 +13,10 @@
static void runtime_resume(struct device * dev)
{
dev_dbg(dev, "resuming\n");
- if (!dev->power.power_state)
+ if (!dev->power.power_state.event)
return;
if (!resume_device(dev))
- dev->power.power_state = 0;
+ dev->power.power_state = PMSG_ON;
}
@@ -49,10 +49,10 @@
int error = 0;
down(_sem);
- if (dev->power.power_state == state)
+ if (dev->power.power_state.event == state.event)
goto Done;
- if (dev->power.power_state)
+ if (dev->power.power_state.event)
runtime_resume(dev);
if (!(error = suspend_device(dev, state)))
--- clean/drivers/base/power/shutdown.c 2004-08-15 19:14:55.0 +0200
+++ linux/drivers/base/power/shutdown.c 2005-03-22 12:20:53.0 +0100
@@ -29,7 +29,8 @@
dev->driver->shutdown(dev);
return 0;
}
- return dpm_runtime_suspend(dev, dev->detach_state);
+ /* FIXME */
+ return dpm_runtime_suspend(dev, PMSG_FREEZE);
}
--- clean/drivers/base/power/suspend.c 2005-01-12 11:07:39.0 +0100
+++ linux/drivers/base/power/suspend.c 2005-03-22 12:20:53.0 +0100
@@ -43,7 +43,7 @@
dev->power.prev_state = dev->power.power_state;
- if (dev->bus && dev->bus->suspend && !dev->power.power_state)
+ if (dev->bus && dev->bus->suspend && (!dev->power.power_state.event))
error = dev->bus->suspend(dev, state);
return error;
--- clean/drivers/base/power/sysfs.c2004-08-15 19:14:55.0 +0200
+++ linux/drivers/base/power/sysfs.c2005-03-22 12:20:53.0 +0100
@@ -26,19 +26,20 @@
static ssize_t state_show(struct device * dev, char * buf)
{
- return sprintf(buf, "%u\n", dev->power.power_state);
+ return sprintf(buf, "%u\n", dev->power.power_state.event);
}
static ssize_t state_store(struct device * dev, const char * buf, size_t n)
{
- u32 state;
+ pm_message_t state;
char * rest;
int error = 0;
- state = simple_strtoul(buf, , 10);
+ state.event = simple_strtoul(buf, , 10);
+// state.flags = PFL_RUNTIME;
if (*rest)
return -EINVAL;
- if (state)
+ if (state.event)
error = dpm_runtime_suspend(dev, state);
else
dpm_runtime_resume(dev);
--- clean/drivers/ide/ide.c 2005-03-19 00:31:23.0 +0100
+++ linux/drivers/ide/ide.c 2005-03-22 12:20:53.0 +0100
@@ -1390,7 +1390,7 @@
rq.special =
rq.pm =
rqpm.pm_step = ide_pm_state_start_suspend;
- rqpm.pm_state = state;
+ rqpm.pm_state = state.event;
return ide_do_drive_cmd(drive, , ide_wait);
}
@@ -1409,7 +1409,7 @@
rq.special =
rq.pm =
rqpm.pm_step = ide_pm_state_start_resume;
- rqpm.pm_state = 0;
+ rqpm.pm_state = PM_EVENT_ON;
return ide_do_drive_cmd(drive, , ide_head_wait);
}
--- clean/drivers/pci/pci.c 2005-03-19 00:31:43.0 +0100
+++ linux/drivers/pci/pci.c 2005-03-22 12:20:53.0 +0100
@@ -312,22 +312,24 @@
/**
* pci_choose_state - Choose the power state of a PCI device
* @dev: PCI device to be suspended
- * @state: target sleep state for the whole system
+ * @state: target sleep state for the whole system. This is the value
+ * that is passed to suspend() function.
*
* Returns PCI power state suitable for given device and given system
* message.
*/
-pci_power_t pci_choose_state(struct pci_dev *dev, u32 state)
+pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
{
- if (!pci_find_capability(dev, PCI_CAP_ID_PM))
+ switch (state.event) {
+ case PM_EVENT_ON:
return PCI_D0;
-
- switch (state) {
- case 0: return PCI_D0;
- case 2: return PCI_D2;
- case 3: return PCI_D3hot;
-
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
> >> > Yes, but it is needed. There are many drivers, and they look at
> >> > numerical value of PMSG_*. I'm proceeding in steps. I hopefully
> killed
> >> > all direct accesses to the constants, and will switch constants to
> >> > something else... But that is going to be tommorow (need some
> sleep).
> >> The patches are going to acquire correct PCI device sleep state for
> >> suspend/resume. We discussed the issue several months ago. My plan is
> we
> >> first introduce 'platform_pci_set_power_state', then merge the
> >> 'platform_pci_choose_state' patch after Pavel's pm_message_t
> conversion
> >> finished. Maybe Len mislead my comments.
> >>
> >> Anyway for the callback, my intend is platform_pci_choose_state
> accept
> >> the pm_message_t parameter, and it return an 'int', since platform
> >> method possibly failed and then pci_choose_state translate the return
> >> value to pci_power_t.
> >
> >You can't just retype around like that. You may want it take
> >pci_power_t * as an argument, and then return 0/-ENODEV or something
> >like that. But you can't retype between int and pm_message_t...
> No, taking pci_power_t as an argument is meaningless. For ACPI, we
> should know the exact sleep state, pm_message_t will tell us. But I'm ok
> to let it return a pci_power_t, and the failure case returns
> -ENODEV.
You can't put -ENODEV into pci_power_t ... but maybe we should create
PCI_ERROR and pass it in cases like this one?
> >> > Could you just revert those two patches? First one is very
> >> > wrong. Second one might be fixed, but... See comments below.
> >> I think the platform_pci_set_power_state should be ok, did you see it
> >> causes oops?
> >
> >No its just ugly and uses __force in "creative" way. That one can be
> >recovered.
> Do you mean this?
>
> > + static int state_conv[] = {
> > + [0] = 0,
> > + [1] = 1,
> > + [2] = 2,
> > + [3] = 3,
> > + [4] = 3
> > + };
> > + int acpi_state = state_conv[(int __force) state];
>
> The table should be
> [PCI_D0] = 0,
>
> I'm not sure, but then could we use state_conv[state] directly? It seems
I think so. Of course it is wrong, but it is less wrong than forcing
it to integer than index, without using macros at all.
Or perhaps you should do
switch (state) {
case PCI_D0: ...
}
...and handle default case somehow.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: 2.6.12-rc1-mm1: Kernel BUG at pci:389
>
>> > Yes, but it is needed. There are many drivers, and they look at
>> > numerical value of PMSG_*. I'm proceeding in steps. I hopefully
killed
>> > all direct accesses to the constants, and will switch constants to
>> > something else... But that is going to be tommorow (need some
sleep).
>> The patches are going to acquire correct PCI device sleep state for
>> suspend/resume. We discussed the issue several months ago. My plan is
we
>> first introduce 'platform_pci_set_power_state', then merge the
>> 'platform_pci_choose_state' patch after Pavel's pm_message_t
conversion
>> finished. Maybe Len mislead my comments.
>>
>> Anyway for the callback, my intend is platform_pci_choose_state
accept
>> the pm_message_t parameter, and it return an 'int', since platform
>> method possibly failed and then pci_choose_state translate the return
>> value to pci_power_t.
>
>You can't just retype around like that. You may want it take
>pci_power_t * as an argument, and then return 0/-ENODEV or something
>like that. But you can't retype between int and pm_message_t...
No, taking pci_power_t as an argument is meaningless. For ACPI, we
should know the exact sleep state, pm_message_t will tell us. But I'm ok
to let it return a pci_power_t, and the failure case returns -ENODEV.
>
>Plus that function should have a documentation somewhere!
I will add it.
>
>> > Could you just revert those two patches? First one is very
>> > wrong. Second one might be fixed, but... See comments below.
>> I think the platform_pci_set_power_state should be ok, did you see it
>> causes oops?
>
>No its just ugly and uses __force in "creative" way. That one can be
>recovered.
Do you mean this?
> + static int state_conv[] = {
> + [0] = 0,
> + [1] = 1,
> + [2] = 2,
> + [3] = 3,
> + [4] = 3
> + };
> + int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
I'm not sure, but then could we use state_conv[state] directly? It seems
wrong to me (the array accepts a pci_power_t as index?)
Thanks,
Shaohua
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
> Will this do it for the moment?
Its certainly better.
What about
> > > +static int acpi_pci_set_power_state(struct pci_dev *dev,
> > pci_power_t state)
> > > +{
> > > + acpi_handle handle = DEVICE_ACPI_HANDLE(>dev);
> > > + static int state_conv[] = {
> > > + [0] = 0,
> > > + [1] = 1,
> > > + [2] = 2,
> > > + [3] = 3,
> > > + [4] = 3
> > > + };
> > > + int acpi_state = state_conv[(int __force) state];
...this force? Then platform_pci_choose_state should not be NULL by
default and acpi_pci_choose_state should really have some more
reasonable calling convention.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! > > Yes, but it is needed. There are many drivers, and they look at > > numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed > > all direct accesses to the constants, and will switch constants to > > something else... But that is going to be tommorow (need some sleep). > The patches are going to acquire correct PCI device sleep state for > suspend/resume. We discussed the issue several months ago. My plan is we > first introduce 'platform_pci_set_power_state', then merge the > 'platform_pci_choose_state' patch after Pavel's pm_message_t conversion > finished. Maybe Len mislead my comments. > > Anyway for the callback, my intend is platform_pci_choose_state accept > the pm_message_t parameter, and it return an 'int', since platform > method possibly failed and then pci_choose_state translate the return > value to pci_power_t. You can't just retype around like that. You may want it take pci_power_t * as an argument, and then return 0/-ENODEV or something like that. But you can't retype between int and pm_message_t... Plus that function should have a documentation somewhere! > > Could you just revert those two patches? First one is very > > wrong. Second one might be fixed, but... See comments below. > I think the platform_pci_set_power_state should be ok, did you see it > causes oops? No its just ugly and uses __force in "creative" way. That one can be recovered. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! > And they are both "dangerous" -- they introduce new and untested > functionality while I'm trying to transition from int to > pm_message_t. They also affect all the drivers. Actually, there's one even more severe problem with platform_pci_choose_state... If we are doing freeze for swsusp snapshot (or freeze for kexec or something similar, that ACPI does not know about), it is very wrong to ask ACPI to tell us power levels for devices. ACPI does not even know about those states, it can not tell us anything meaningfull. So if this hook is to be reintroduced, it should go down in the function, and only trigger for ACPI S3 and ACPI S1 cases. Maybe for swsusp/plaform (== ACPI S4). But I'd prefer the hook to go away for now, it clearly needs infrastructure that is not yet there, and provides nothing. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! And they are both dangerous -- they introduce new and untested functionality while I'm trying to transition from int to pm_message_t. They also affect all the drivers. Actually, there's one even more severe problem with platform_pci_choose_state... If we are doing freeze for swsusp snapshot (or freeze for kexec or something similar, that ACPI does not know about), it is very wrong to ask ACPI to tell us power levels for devices. ACPI does not even know about those states, it can not tell us anything meaningfull. So if this hook is to be reintroduced, it should go down in the function, and only trigger for ACPI S3 and ACPI S1 cases. Maybe for swsusp/plaform (== ACPI S4). But I'd prefer the hook to go away for now, it clearly needs infrastructure that is not yet there, and provides nothing. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi! Yes, but it is needed. There are many drivers, and they look at numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed all direct accesses to the constants, and will switch constants to something else... But that is going to be tommorow (need some sleep). The patches are going to acquire correct PCI device sleep state for suspend/resume. We discussed the issue several months ago. My plan is we first introduce 'platform_pci_set_power_state', then merge the 'platform_pci_choose_state' patch after Pavel's pm_message_t conversion finished. Maybe Len mislead my comments. Anyway for the callback, my intend is platform_pci_choose_state accept the pm_message_t parameter, and it return an 'int', since platform method possibly failed and then pci_choose_state translate the return value to pci_power_t. You can't just retype around like that. You may want it take pci_power_t * as an argument, and then return 0/-ENODEV or something like that. But you can't retype between int and pm_message_t... Plus that function should have a documentation somewhere! Could you just revert those two patches? First one is very wrong. Second one might be fixed, but... See comments below. I think the platform_pci_set_power_state should be ok, did you see it causes oops? No its just ugly and uses __force in creative way. That one can be recovered. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
Will this do it for the moment?
Its certainly better.
What about
+static int acpi_pci_set_power_state(struct pci_dev *dev,
pci_power_t state)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev-dev);
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
...this force? Then platform_pci_choose_state should not be NULL by
default and acpi_pci_choose_state should really have some more
reasonable calling convention.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully
killed
all direct accesses to the constants, and will switch constants to
something else... But that is going to be tommorow (need some
sleep).
The patches are going to acquire correct PCI device sleep state for
suspend/resume. We discussed the issue several months ago. My plan is
we
first introduce 'platform_pci_set_power_state', then merge the
'platform_pci_choose_state' patch after Pavel's pm_message_t
conversion
finished. Maybe Len mislead my comments.
Anyway for the callback, my intend is platform_pci_choose_state
accept
the pm_message_t parameter, and it return an 'int', since platform
method possibly failed and then pci_choose_state translate the return
value to pci_power_t.
You can't just retype around like that. You may want it take
pci_power_t * as an argument, and then return 0/-ENODEV or something
like that. But you can't retype between int and pm_message_t...
No, taking pci_power_t as an argument is meaningless. For ACPI, we
should know the exact sleep state, pm_message_t will tell us. But I'm ok
to let it return a pci_power_t, and the failure case returns -ENODEV.
Plus that function should have a documentation somewhere!
I will add it.
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
I think the platform_pci_set_power_state should be ok, did you see it
causes oops?
No its just ugly and uses __force in creative way. That one can be
recovered.
Do you mean this?
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
I'm not sure, but then could we use state_conv[state] directly? It seems
wrong to me (the array accepts a pci_power_t as index?)
Thanks,
Shaohua
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully
killed
all direct accesses to the constants, and will switch constants to
something else... But that is going to be tommorow (need some
sleep).
The patches are going to acquire correct PCI device sleep state for
suspend/resume. We discussed the issue several months ago. My plan is
we
first introduce 'platform_pci_set_power_state', then merge the
'platform_pci_choose_state' patch after Pavel's pm_message_t
conversion
finished. Maybe Len mislead my comments.
Anyway for the callback, my intend is platform_pci_choose_state
accept
the pm_message_t parameter, and it return an 'int', since platform
method possibly failed and then pci_choose_state translate the return
value to pci_power_t.
You can't just retype around like that. You may want it take
pci_power_t * as an argument, and then return 0/-ENODEV or something
like that. But you can't retype between int and pm_message_t...
No, taking pci_power_t as an argument is meaningless. For ACPI, we
should know the exact sleep state, pm_message_t will tell us. But I'm ok
to let it return a pci_power_t, and the failure case returns
-ENODEV.
You can't put -ENODEV into pci_power_t ... but maybe we should create
PCI_ERROR and pass it in cases like this one?
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
I think the platform_pci_set_power_state should be ok, did you see it
causes oops?
No its just ugly and uses __force in creative way. That one can be
recovered.
Do you mean this?
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
I'm not sure, but then could we use state_conv[state] directly? It seems
I think so. Of course it is wrong, but it is less wrong than forcing
it to integer than index, without using macros at all.
Or perhaps you should do
switch (state) {
case PCI_D0: ...
}
...and handle default case somehow.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
pm_message_t to struct conversion [was Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389]
Hi!
to Linus when he reappears and then I'll duck for cover and let you guys
sort it out ;)
There should be little reason for taking cover, that patches were just
anotating types... BTW this is how switch to pm_message_t is going to
look. If you are developing something pm-related, you should probably
do it against this tree.
Pavel
--- clean/drivers/base/power/resume.c 2004-12-25 13:34:59.0 +0100
+++ linux/drivers/base/power/resume.c 2005-03-22 12:20:53.0 +0100
@@ -41,7 +41,7 @@
list_add_tail(entry, dpm_active);
up(dpm_list_sem);
- if (!dev-power.prev_state)
+ if (!dev-power.prev_state.event)
resume_device(dev);
down(dpm_list_sem);
put_device(dev);
--- clean/drivers/base/power/runtime.c 2005-01-12 11:07:39.0 +0100
+++ linux/drivers/base/power/runtime.c 2005-03-22 12:20:53.0 +0100
@@ -13,10 +13,10 @@
static void runtime_resume(struct device * dev)
{
dev_dbg(dev, resuming\n);
- if (!dev-power.power_state)
+ if (!dev-power.power_state.event)
return;
if (!resume_device(dev))
- dev-power.power_state = 0;
+ dev-power.power_state = PMSG_ON;
}
@@ -49,10 +49,10 @@
int error = 0;
down(dpm_sem);
- if (dev-power.power_state == state)
+ if (dev-power.power_state.event == state.event)
goto Done;
- if (dev-power.power_state)
+ if (dev-power.power_state.event)
runtime_resume(dev);
if (!(error = suspend_device(dev, state)))
--- clean/drivers/base/power/shutdown.c 2004-08-15 19:14:55.0 +0200
+++ linux/drivers/base/power/shutdown.c 2005-03-22 12:20:53.0 +0100
@@ -29,7 +29,8 @@
dev-driver-shutdown(dev);
return 0;
}
- return dpm_runtime_suspend(dev, dev-detach_state);
+ /* FIXME */
+ return dpm_runtime_suspend(dev, PMSG_FREEZE);
}
--- clean/drivers/base/power/suspend.c 2005-01-12 11:07:39.0 +0100
+++ linux/drivers/base/power/suspend.c 2005-03-22 12:20:53.0 +0100
@@ -43,7 +43,7 @@
dev-power.prev_state = dev-power.power_state;
- if (dev-bus dev-bus-suspend !dev-power.power_state)
+ if (dev-bus dev-bus-suspend (!dev-power.power_state.event))
error = dev-bus-suspend(dev, state);
return error;
--- clean/drivers/base/power/sysfs.c2004-08-15 19:14:55.0 +0200
+++ linux/drivers/base/power/sysfs.c2005-03-22 12:20:53.0 +0100
@@ -26,19 +26,20 @@
static ssize_t state_show(struct device * dev, char * buf)
{
- return sprintf(buf, %u\n, dev-power.power_state);
+ return sprintf(buf, %u\n, dev-power.power_state.event);
}
static ssize_t state_store(struct device * dev, const char * buf, size_t n)
{
- u32 state;
+ pm_message_t state;
char * rest;
int error = 0;
- state = simple_strtoul(buf, rest, 10);
+ state.event = simple_strtoul(buf, rest, 10);
+// state.flags = PFL_RUNTIME;
if (*rest)
return -EINVAL;
- if (state)
+ if (state.event)
error = dpm_runtime_suspend(dev, state);
else
dpm_runtime_resume(dev);
--- clean/drivers/ide/ide.c 2005-03-19 00:31:23.0 +0100
+++ linux/drivers/ide/ide.c 2005-03-22 12:20:53.0 +0100
@@ -1390,7 +1390,7 @@
rq.special = args;
rq.pm = rqpm;
rqpm.pm_step = ide_pm_state_start_suspend;
- rqpm.pm_state = state;
+ rqpm.pm_state = state.event;
return ide_do_drive_cmd(drive, rq, ide_wait);
}
@@ -1409,7 +1409,7 @@
rq.special = args;
rq.pm = rqpm;
rqpm.pm_step = ide_pm_state_start_resume;
- rqpm.pm_state = 0;
+ rqpm.pm_state = PM_EVENT_ON;
return ide_do_drive_cmd(drive, rq, ide_head_wait);
}
--- clean/drivers/pci/pci.c 2005-03-19 00:31:43.0 +0100
+++ linux/drivers/pci/pci.c 2005-03-22 12:20:53.0 +0100
@@ -312,22 +312,24 @@
/**
* pci_choose_state - Choose the power state of a PCI device
* @dev: PCI device to be suspended
- * @state: target sleep state for the whole system
+ * @state: target sleep state for the whole system. This is the value
+ * that is passed to suspend() function.
*
* Returns PCI power state suitable for given device and given system
* message.
*/
-pci_power_t pci_choose_state(struct pci_dev *dev, u32 state)
+pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
{
- if (!pci_find_capability(dev, PCI_CAP_ID_PM))
+ switch (state.event) {
+ case PM_EVENT_ON:
return PCI_D0;
-
- switch (state) {
- case 0: return PCI_D0;
- case 2: return PCI_D2;
- case 3: return PCI_D3hot;
-
2.6.12-rc1-mm1: resume regression (was: Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389)
Hi, On Tuesday, 22 of March 2005 12:01, Pavel Machek wrote: Hi! Will this do it for the moment? Its certainly better. With the Len's patch applied I have to unload the modules: ohci_hcd ehci_hcd yenta_socket before suspend as each of them hangs the box solid during either suspend or resume. Moreover, when I tried to load the ehci_hcd module back after resume, it hanged the box solid too. Greets, Rafael -- - Would you tell me, please, which way I ought to go from here? - That depends a good deal on where you want to get to. -- Lewis Carroll Alice's Adventures in Wonderland - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Mon, Mar 21, 2005 at 06:27:33PM -0800, Andrew Morton wrote: > OK, well unless someone has objections I'll just send all these > > swsusp-add-missing-refrigerator-calls.patch > suspend-to-ram-update-videotxt-with-more-systems.patch > pm-remove-obsolete-pm_-from-vtc.patch > swsusp-small-updates.patch > swsusp-1-1-kill-swsusp_restore.patch > fix-pm_message_t-in-generic-code.patch > fix-u32-vs-pm_message_t-in-usb.patch > more-pm_message_t-fixes.patch > fix-u32-vs-pm_message_t-confusion-in-oss.patch > fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch > fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch > fix-u32-vs-pm_message_t-confusion-in-mmc.patch > fix-u32-vs-pm_message_t-confusion-in-serials.patch > fix-u32-vs-pm_message_t-in-macintosh.patch > fix-u32-vs-pm_message_t-confusion-in-agp.patch > > to Linus when he reappears and then I'll duck for cover and let you guys > sort it out ;) No objection from me, that's probably the best way for this to get into the tree. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Will this do it for the moment?
If so, lets use it until Pavel's flag-day is over -- when we'll send an
updated patch.
thanks,
-Len
= drivers/pci/pci-acpi.c 1.4 vs edited =
--- 1.4/drivers/pci/pci-acpi.c 2005-03-03 04:28:23 -05:00
+++ edited/drivers/pci/pci-acpi.c 2005-03-21 22:59:39 -05:00
@@ -237,19 +237,8 @@
static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t state)
{
- char dstate_str[] = "_S0D";
- acpi_status status;
- unsigned long val;
- struct device *dev = >dev;
+ /* TBD */
- /* Fixme: the check is wrong after pm_message_t is a struct */
- if ((state >= PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
- return -EINVAL;
- dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
- status = acpi_evaluate_integer(DEVICE_ACPI_HANDLE(dev), dstate_str,
- NULL, );
- if (ACPI_SUCCESS(status))
- return val;
return -ENODEV;
}
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek <[EMAIL PROTECTED]> wrote: > > > Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send > > that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 > > based tree then I can look after it until things get flushed out. > > Could you just revert those two patches? First one is very > wrong. Second one might be fixed, but... See comments below. I could revert them locally, but that wouldn't gain us much. Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the best. Perhaps I should just jam everything-from-Pavel into Linus's tree as soon as he returns and then we can fix up the downstream fallout in the various bk trees? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Tue, 2005-03-22 at 09:35, Pavel Machek wrote:
> Hi!
>
> > > and that says:
> > >
> > > #define PMSG_FREEZE ((__force pm_message_t) 3)
> > >
> > > ... I certainly have _FREEZE defined as 1 in my local tree, but I
> do
> > > not see that change in -mm yet.
> >
> > Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
> >
> > #define PMSG_FREEZE ((__force pm_message_t) 3)
> > #define PMSG_SUSPEND((__force pm_message_t) 3)
> > #define PMSG_ON ((__force pm_message_t) 0)
> >
> > which looks odd.
>
> Yes, but it is needed. There are many drivers, and they look at
> numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed
> all direct accesses to the constants, and will switch constants to
> something else... But that is going to be tommorow (need some sleep).
The patches are going to acquire correct PCI device sleep state for
suspend/resume. We discussed the issue several months ago. My plan is we
first introduce 'platform_pci_set_power_state', then merge the
'platform_pci_choose_state' patch after Pavel's pm_message_t conversion
finished. Maybe Len mislead my comments.
Anyway for the callback, my intend is platform_pci_choose_state accept
the pm_message_t parameter, and it return an 'int', since platform
method possibly failed and then pci_choose_state translate the return
value to pci_power_t.
> > > I reproduced it here.. I do not know who introduced
> > > platform_pci_choose_state, but it is *very* wrong. It returns
> > > it. Should it return pci_power_t? It probably should to match
> > > pci_choose_state, but that int is retyped to pm_message_t. Oops.
> >
> > That change came from Len. I've appended the two relevant patches
> below.
> >
> > So hm. We have incompatible changes in flight. That doesn't happen
> very
> > often.
> >
> > Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and
> send
> > that to Len and myself? If that fixup is not suitable for a
> 2.6.12-rc1
> > based tree then I can look after it until things get flushed out.
>
> Could you just revert those two patches? First one is very
> wrong. Second one might be fixed, but... See comments below.
I think the platform_pci_set_power_state should be ok, did you see it
causes oops?
>
> And they are both "dangerous" -- they introduce new and untested
> functionality while I'm trying to transition from int to
> pm_message_t. They also affect all the drivers.
>
> Len, please Cc me on patches that affect suspend.
>
> > @@ -17,6 +17,7 @@
> > #include
> >
> > #include
> > +#include "pci.h"
>
>
> Should be ?
I suppose it's not exported out side of PCI, so I used 'pci.h'
>
> > +static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t
> state)
> > +{
>
> Should return pci_power_t, probably.
Should return int as I said above.
>
> > + char dstate_str[] = "_S0D";
> > + acpi_status status;
> > + unsigned long val;
> > + struct device *dev = >dev;
> > +
> > + /* Fixme: the check is wrong after pm_message_t is a struct */
>
> Exactly.
>
> > + if ((state >= PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
>
> PM_SUSPEND_MAX and friends is going to disappear.
Yep, this should be fixed.
>
> > + return -EINVAL;
> > + dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
>
> Ugh, assumes numerical values of states actually meaning anything. It
> definitely should not. Should be switch(state.event), but that code
> is not merged, yet => I'll send code that switches pm_message_t to
> struct, tommorow. But it may compile-time break some obscure
> drivers...
>
> > diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > --- a/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
> > +++ b/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
> > @@ -253,6 +253,24 @@
> > return -ENODEV;
> > }
> >
> > +static int acpi_pci_set_power_state(struct pci_dev *dev,
> pci_power_t state)
> > +{
> > + acpi_handle handle = DEVICE_ACPI_HANDLE(>dev);
> > + static int state_conv[] = {
> > + [0] = 0,
> > + [1] = 1,
> > + [2] = 2,
> > + [3] = 3,
> > + [4] = 3
> > + };
> > + int acpi_state = state_conv[(int __force) state];
>
> The table should be
> [PCI_D0] = 0,
> ...
Ok, please revert the 'platform_pci_choose_pci' patch, I will add it
after Pavel's conversion is finished. Or after Pavel's is done, I can
send a quick fix.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Mon, Mar 21, 2005 at 05:06:23PM -0800, Andrew Morton wrote: > # drivers/pci/pci-acpi.c > # 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +46 -1 > # add platform_pci_choose_state() > # > diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > --- a/drivers/pci/pci-acpi.c 2005-03-21 17:01:44 -08:00 > +++ b/drivers/pci/pci-acpi.c 2005-03-21 17:01:44 -08:00 > @@ -1,6 +1,6 @@ > /* > * File:pci-acpi.c > - * Purpose: Provide PCI support in ACPI > + * Purpose: Provde PCI support in ACPI Oops. Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Po 21-03-05 17:52:32, Andrew Morton wrote: > Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send > > > that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 > > > based tree then I can look after it until things get flushed out. > > > > Could you just revert those two patches? First one is very > > wrong. Second one might be fixed, but... See comments below. > > I could revert them locally, but that wouldn't gain us much. You mean that Len has to revert them or revert is "ineffective"? > Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the best. > > Perhaps I should just jam everything-from-Pavel into Linus's tree as soon > as he returns and then we can fix up the downstream fallout in the various > bk trees? Yes, that would help a lot. I was waiting with "turn-pm_message_t-into-struct" until all pm_message_t patches reached Linus so that there's not a mess "in flight". Len's patch pretty much depends on pm_message_t already being converted... (and I'd prefer it to wait a while, so we can see which problems were introduced by conversion and which are due to ACPI BIOS bugs). Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek <[EMAIL PROTECTED]> wrote: > > On Po 21-03-05 17:52:32, Andrew Morton wrote: > > Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > > Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send > > > > that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 > > > > based tree then I can look after it until things get flushed out. > > > > > > Could you just revert those two patches? First one is very > > > wrong. Second one might be fixed, but... See comments below. > > > > I could revert them locally, but that wouldn't gain us much. > > You mean that Len has to revert them or revert is "ineffective"? The patches are in Len's tree. > > Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the > > best. > > > > Perhaps I should just jam everything-from-Pavel into Linus's tree as soon > > as he returns and then we can fix up the downstream fallout in the various > > bk trees? > > Yes, that would help a lot. I was waiting with > "turn-pm_message_t-into-struct" until all pm_message_t patches reached > Linus so that there's not a mess "in flight". Len's patch pretty much > depends on pm_message_t already being converted... (and I'd prefer it > to wait a while, so we can see which problems were introduced by > conversion and which are due to ACPI BIOS bugs). OK, well unless someone has objections I'll just send all these swsusp-add-missing-refrigerator-calls.patch suspend-to-ram-update-videotxt-with-more-systems.patch pm-remove-obsolete-pm_-from-vtc.patch swsusp-small-updates.patch swsusp-1-1-kill-swsusp_restore.patch fix-pm_message_t-in-generic-code.patch fix-u32-vs-pm_message_t-in-usb.patch more-pm_message_t-fixes.patch fix-u32-vs-pm_message_t-confusion-in-oss.patch fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch fix-u32-vs-pm_message_t-confusion-in-mmc.patch fix-u32-vs-pm_message_t-confusion-in-serials.patch fix-u32-vs-pm_message_t-in-macintosh.patch fix-u32-vs-pm_message_t-confusion-in-agp.patch to Linus when he reappears and then I'll duck for cover and let you guys sort it out ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
> > and that says:
> >
> > #define PMSG_FREEZE ((__force pm_message_t) 3)
> >
> > ... I certainly have _FREEZE defined as 1 in my local tree, but I do
> > not see that change in -mm yet.
>
> Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
>
> #define PMSG_FREEZE ((__force pm_message_t) 3)
> #define PMSG_SUSPEND((__force pm_message_t) 3)
> #define PMSG_ON ((__force pm_message_t) 0)
>
> which looks odd.
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed
all direct accesses to the constants, and will switch constants to
something else... But that is going to be tommorow (need some sleep).
> > I reproduced it here.. I do not know who introduced
> > platform_pci_choose_state, but it is *very* wrong. It returns
> > it. Should it return pci_power_t? It probably should to match
> > pci_choose_state, but that int is retyped to pm_message_t. Oops.
>
> That change came from Len. I've appended the two relevant patches below.
>
> So hm. We have incompatible changes in flight. That doesn't happen very
> often.
>
> Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send
> that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1
> based tree then I can look after it until things get flushed out.
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
And they are both "dangerous" -- they introduce new and untested
functionality while I'm trying to transition from int to
pm_message_t. They also affect all the drivers.
Len, please Cc me on patches that affect suspend.
> @@ -17,6 +17,7 @@
> #include
>
> #include
> +#include "pci.h"
Should be ?
> +static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t state)
> +{
Should return pci_power_t, probably.
> + char dstate_str[] = "_S0D";
> + acpi_status status;
> + unsigned long val;
> + struct device *dev = >dev;
> +
> + /* Fixme: the check is wrong after pm_message_t is a struct */
Exactly.
> + if ((state >= PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
PM_SUSPEND_MAX and friends is going to disappear.
> + return -EINVAL;
> + dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
Ugh, assumes numerical values of states actually meaning anything. It
definitely should not. Should be switch(state.event), but that code
is not merged, yet => I'll send code that switches pm_message_t to
struct, tommorow. But it may compile-time break some obscure drivers...
> diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> --- a/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
> +++ b/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
> @@ -253,6 +253,24 @@
> return -ENODEV;
> }
>
> +static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
> +{
> + acpi_handle handle = DEVICE_ACPI_HANDLE(>dev);
> + static int state_conv[] = {
> + [0] = 0,
> + [1] = 1,
> + [2] = 2,
> + [3] = 3,
> + [4] = 3
> + };
> + int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
...
and then it should not need __force.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek <[EMAIL PROTECTED]> wrote:
>
> Hi!
>
> > > On Monday, 21 of March 2005 11:51, you wrote:
> > > >
> > > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
> > >
> > > I get the following BUG every time I try to suspend my box to disk.
> >
> > Pavel, that's the BUG() in pci_choose_state(). I did have some
> > reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
> > some incompatible intent in there.
> >
> > I dunno why pci_choose_state() is saying that it received PCI_D1, when
> > prepare_devices() is passing down PMSG_FREEZE?
>
> Uf, I don't know what version that was.. I think I have
>
> VERSION = 2
> PATCHLEVEL = 6
> SUBLEVEL = 12
> EXTRAVERSION =-rc1-mm1
yes, the report was against 2.6.12-rc1-mm1.
> and that says:
>
> #define PMSG_FREEZE ((__force pm_message_t) 3)
>
> ... I certainly have _FREEZE defined as 1 in my local tree, but I do
> not see that change in -mm yet.
Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
#define PMSG_FREEZE ((__force pm_message_t) 3)
#define PMSG_SUSPEND((__force pm_message_t) 3)
#define PMSG_ON ((__force pm_message_t) 0)
which looks odd.
> Possibly pm.h changes went in faster than pci.c or something like
> that?
grep says that 2.6.12-rc1-mm1 has these patches from you:
fix-suspend-resume-on-via-velocity.patch
x86-fix-esp-corruption-cpu-bug-take-2.patch
swsusp-add-missing-refrigerator-calls.patch
suspend-to-ram-update-videotxt-with-more-systems.patch
pm-remove-obsolete-pm_-from-vtc.patch
swsusp-small-updates.patch
swsusp-1-1-kill-swsusp_restore.patch
pcmcia-id_table-for-orinoco_cs.patch
fix-pm_message_t-in-generic-code.patch
fix-u32-vs-pm_message_t-in-usb.patch
more-pm_message_t-fixes.patch
fix-u32-vs-pm_message_t-confusion-in-oss.patch
fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch
fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch
fix-u32-vs-pm_message_t-confusion-in-mmc.patch
fix-u32-vs-pm_message_t-confusion-in-serials.patch
fix-u32-vs-pm_message_t-in-macintosh.patch
fix-u32-vs-pm_message_t-confusion-in-agp.patch
> I reproduced it here.. I do not know who introduced
> platform_pci_choose_state, but it is *very* wrong. It returns
> it. Should it return pci_power_t? It probably should to match
> pci_choose_state, but that int is retyped to pm_message_t. Oops.
That change came from Len. I've appended the two relevant patches below.
So hm. We have incompatible changes in flight. That doesn't happen very
often.
Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send
that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1
based tree then I can look after it until things get flushed out.
(Len, platform_pci_set_power_state shouldn't be initialised to NULL).
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/19 00:15:48-05:00 [EMAIL PROTECTED]
# [ACPI] PCI can now get suspend state from firmware
#
# pci_choose_state() can now call
# platform_pci_choose_state()
# and ACPI can answer
#
# http://bugzilla.kernel.org/show_bug.cgi?id=4277
#
# Signed-off-by: David Shaohua Li <[EMAIL PROTECTED]>
# Signed-off-by: Len Brown <[EMAIL PROTECTED]>
#
# drivers/pci/pci.h
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +3 -0
# add platform_pci_choose_state()
#
# drivers/pci/pci.c
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +7 -0
# add platform_pci_choose_state()
#
# drivers/pci/pci-acpi.c
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +46 -1
# add platform_pci_choose_state()
#
diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
--- a/drivers/pci/pci-acpi.c2005-03-21 17:01:44 -08:00
+++ b/drivers/pci/pci-acpi.c2005-03-21 17:01:44 -08:00
@@ -1,6 +1,6 @@
/*
* File: pci-acpi.c
- * Purpose:Provide PCI support in ACPI
+ * Purpose:Provde PCI support in ACPI
*
* Copyright (C) 2005 David Shaohua Li <[EMAIL PROTECTED]>
* Copyright (C) 2004 Tom Long Nguyen <[EMAIL PROTECTED]>
@@ -17,6 +17,7 @@
#include
#include
+#include "pci.h"
static u32 ctrlset_buf[3] = {0, 0, 0};
static u32 global_ctrlsets = 0;
@@ -209,6 +210,49 @@
}
EXPORT_SYMBOL(pci_osc_control_set);
+/*
+ * _SxD returns the D-state with the highest power
+ * (lowest D-state number) supported in the S-state "x".
+ *
+ * If the devices does not have a _PRW
+ * (Power Resources for Wake) supporting system wakeup from "x"
+ * then the OS is free to choose a lower power (higher number
+ * D-state) than the return value from _SxD.
+ *
+ * But if _PRW is enabled at S-state "x", the OS
+ * must not choose a power lower than _SxD --
+ * unless the device has an _SxW method specifying
+ * the lowest power (highest D-state number) the device
+ * may enter while still able to wake the system.
+ *
+ * ie. depending on global OS policy:
+ *
+ * if (_PRW at S-state x)
+ * choose from highest power _SxD to lowest power _SxW
+ * else // no _PRW at S-state x
+ * choose
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
> > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
> >
> > I get the following BUG every time I try to suspend my box to disk.
>
> Pavel, that's the BUG() in pci_choose_state(). I did have some
> reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
> some incompatible intent in there.
>
> I dunno why pci_choose_state() is saying that it received PCI_D1, when
> prepare_devices() is passing down PMSG_FREEZE?
This works it around:
--- clean-mm/drivers/pci/pci.c 2005-03-21 11:39:32.0 +0100
+++ linux-mm/drivers/pci/pci.c 2005-03-22 01:41:48.0 +0100
@@ -376,11 +376,13 @@
if (!pci_find_capability(dev, PCI_CAP_ID_PM))
return PCI_D0;
+#if 0
if (platform_pci_choose_state) {
ret = platform_pci_choose_state(dev, state);
if (ret >= 0)
state = ret;
}
+#endif
switch (state) {
case 0: return PCI_D0;
case 3: return PCI_D3hot;
platform_pci_choose_state is very wrong, and it would be nice to just
revert the patch that introduced it. pm_message_t is going to became a
structure, and I don't want to have another place to fixup.
Hmm, it looks like I should do switch to the structure *now* so that
pm_message_t becomes incompatible with int and people can't get it
wrong...
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
> > On Monday, 21 of March 2005 11:51, you wrote:
> > >
> > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
> >
> > I get the following BUG every time I try to suspend my box to disk.
>
> Pavel, that's the BUG() in pci_choose_state(). I did have some
> reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
> some incompatible intent in there.
>
> I dunno why pci_choose_state() is saying that it received PCI_D1, when
> prepare_devices() is passing down PMSG_FREEZE?
Uf, I don't know what version that was.. I think I have
VERSION = 2
PATCHLEVEL = 6
SUBLEVEL = 12
EXTRAVERSION =-rc1-mm1
and that says:
#define PMSG_FREEZE ((__force pm_message_t) 3)
... I certainly have _FREEZE defined as 1 in my local tree, but I do
not see that change in -mm yet.
Possibly pm.h changes went in faster than pci.c or something like
that?
I reproduced it here.. I do not know who introduced
platform_pci_choose_state, but it is *very* wrong. It returns
it. Should it return pci_power_t? It probably should to match
pci_choose_state, but that int is retyped to pm_message_t. Oops.
Pavel
>
>
> > Greets,
> > Rafael
> >
> >
> > Stopping tasks:
> > ===|
> > Freeing memory... done (66711 pages freed)
> > They asked me for state 1
> > --- [cut here ] - [please bite here ] -
> > Kernel BUG at pci:389
> > invalid operand: [1]
> > CPU 0
> > Modules linked in: usbserial parport_pc lp parport thermal processor fan
> > button battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
> > Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
> > RIP: 0010:[] {pci_choose_state+96}
> > RSP: :810020fbfd78 EFLAGS: 00010292
> > RAX: 001d RBX: 0001 RCX:
> > RDX: RSI: 44e0 RDI: 8041d140
> > RBP: 81002fc151c0 R08: R09: 81002a535c48
> > R10: R11: R12: 81002fc151c0
> > R13: 0003 R14: R15: 0080
> > FS: 2b28b800() GS:8055c840() knlGS:
> > CS: 0010 DS: ES: CR0: 8005003b
> > CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
> > Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
> > 810020d527e0)
> > Stack: 81002c349628 81002c349628 8032218a
> >81002fc149a8 81002fc15230
> >8048f680 0003
> > Call Trace:{usb_hcd_pci_suspend+74}
> > {pci_device_suspend+30}
> >{suspend_device+50}
> > {device_suspend+129}
> >{prepare_devices+11}
> > {pm_suspend_disk+21}
> >{enter_state+70}
> > {state_store+109}
> >{subsys_attr_store+31}
> > {sysfs_write_file+204}
> >{vfs_write+233} {sys_write+83}
> >{system_call+126}
> >
> > Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
> > RIP {pci_choose_state+96} RSP
> >
> >
> >
> > --
> > - Would you tell me, please, which way I ought to go from here?
> > - That depends a good deal on where you want to get to.
> > -- Lewis Carroll "Alice's Adventures in Wonderland"
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
"Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
>
> Hi,
>
> On Monday, 21 of March 2005 11:51, you wrote:
> >
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
>
> I get the following BUG every time I try to suspend my box to disk.
Pavel, that's the BUG() in pci_choose_state(). I did have some
reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
some incompatible intent in there.
I dunno why pci_choose_state() is saying that it received PCI_D1, when
prepare_devices() is passing down PMSG_FREEZE?
> Greets,
> Rafael
>
>
> Stopping tasks:
> ===|
> Freeing memory... done (66711 pages freed)
> They asked me for state 1
> --- [cut here ] - [please bite here ] -
> Kernel BUG at pci:389
> invalid operand: [1]
> CPU 0
> Modules linked in: usbserial parport_pc lp parport thermal processor fan
> button battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
> Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
> RIP: 0010:[] {pci_choose_state+96}
> RSP: :810020fbfd78 EFLAGS: 00010292
> RAX: 001d RBX: 0001 RCX:
> RDX: RSI: 44e0 RDI: 8041d140
> RBP: 81002fc151c0 R08: R09: 81002a535c48
> R10: R11: R12: 81002fc151c0
> R13: 0003 R14: R15: 0080
> FS: 2b28b800() GS:8055c840() knlGS:
> CS: 0010 DS: ES: CR0: 8005003b
> CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
> Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
> 810020d527e0)
> Stack: 81002c349628 81002c349628 8032218a
>81002fc149a8 81002fc15230
>8048f680 0003
> Call Trace:{usb_hcd_pci_suspend+74}
> {pci_device_suspend+30}
>{suspend_device+50}
> {device_suspend+129}
>{prepare_devices+11}
> {pm_suspend_disk+21}
>{enter_state+70} {state_store+109}
>{subsys_attr_store+31}
> {sysfs_write_file+204}
>{vfs_write+233} {sys_write+83}
>{system_call+126}
>
> Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
> RIP {pci_choose_state+96} RSP
>
>
>
> --
> - Would you tell me, please, which way I ought to go from here?
> - That depends a good deal on where you want to get to.
> -- Lewis Carroll "Alice's Adventures in Wonderland"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi,
On Monday, 21 of March 2005 11:51, you wrote:
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Greets,
Rafael
Stopping tasks:
===|
Freeing memory... done (66711 pages freed)
They asked me for state 1
--- [cut here ] - [please bite here ] -
Kernel BUG at pci:389
invalid operand: [1]
CPU 0
Modules linked in: usbserial parport_pc lp parport thermal processor fan button
battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
RIP: 0010:[] {pci_choose_state+96}
RSP: :810020fbfd78 EFLAGS: 00010292
RAX: 001d RBX: 0001 RCX:
RDX: RSI: 44e0 RDI: 8041d140
RBP: 81002fc151c0 R08: R09: 81002a535c48
R10: R11: R12: 81002fc151c0
R13: 0003 R14: R15: 0080
FS: 2b28b800() GS:8055c840() knlGS:
CS: 0010 DS: ES: CR0: 8005003b
CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
810020d527e0)
Stack: 81002c349628 81002c349628 8032218a
81002fc149a8 81002fc15230
8048f680 0003
Call Trace:{usb_hcd_pci_suspend+74}
{pci_device_suspend+30}
{suspend_device+50}
{device_suspend+129}
{prepare_devices+11}
{pm_suspend_disk+21}
{enter_state+70} {state_store+109}
{subsys_attr_store+31}
{sysfs_write_file+204}
{vfs_write+233} {sys_write+83}
{system_call+126}
Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
RIP {pci_choose_state+96} RSP
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi,
On Monday, 21 of March 2005 11:51, you wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Greets,
Rafael
Stopping tasks:
===|
Freeing memory... done (66711 pages freed)
They asked me for state 1
--- [cut here ] - [please bite here ] -
Kernel BUG at pci:389
invalid operand: [1]
CPU 0
Modules linked in: usbserial parport_pc lp parport thermal processor fan button
battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
RIP: 0010:[80283a70] 80283a70{pci_choose_state+96}
RSP: :810020fbfd78 EFLAGS: 00010292
RAX: 001d RBX: 0001 RCX:
RDX: RSI: 44e0 RDI: 8041d140
RBP: 81002fc151c0 R08: R09: 81002a535c48
R10: R11: R12: 81002fc151c0
R13: 0003 R14: R15: 0080
FS: 2b28b800() GS:8055c840() knlGS:
CS: 0010 DS: ES: CR0: 8005003b
CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
810020d527e0)
Stack: 81002c349628 81002c349628 8032218a
81002fc149a8 81002fc15230
8048f680 0003
Call Trace:8032218a{usb_hcd_pci_suspend+74}
8028519e{pci_device_suspend+30}
802ee3d2{suspend_device+50}
802ee4f1{device_suspend+129}
80166ceb{prepare_devices+11}
80167095{pm_suspend_disk+21}
80164206{enter_state+70} 8016442d{state_store+109}
801f275f{subsys_attr_store+31}
801f2c1c{sysfs_write_file+204}
8019c6c9{vfs_write+233} 8019c863{sys_write+83}
8010f092{system_call+126}
Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
RIP 80283a70{pci_choose_state+96} RSP 810020fbfd78
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll Alice's Adventures in Wonderland
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
Hi,
On Monday, 21 of March 2005 11:51, you wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Pavel, that's the BUG() in pci_choose_state(). I did have some
reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
some incompatible intent in there.
I dunno why pci_choose_state() is saying that it received PCI_D1, when
prepare_devices() is passing down PMSG_FREEZE?
Greets,
Rafael
Stopping tasks:
===|
Freeing memory... done (66711 pages freed)
They asked me for state 1
--- [cut here ] - [please bite here ] -
Kernel BUG at pci:389
invalid operand: [1]
CPU 0
Modules linked in: usbserial parport_pc lp parport thermal processor fan
button battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
RIP: 0010:[80283a70] 80283a70{pci_choose_state+96}
RSP: :810020fbfd78 EFLAGS: 00010292
RAX: 001d RBX: 0001 RCX:
RDX: RSI: 44e0 RDI: 8041d140
RBP: 81002fc151c0 R08: R09: 81002a535c48
R10: R11: R12: 81002fc151c0
R13: 0003 R14: R15: 0080
FS: 2b28b800() GS:8055c840() knlGS:
CS: 0010 DS: ES: CR0: 8005003b
CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
810020d527e0)
Stack: 81002c349628 81002c349628 8032218a
81002fc149a8 81002fc15230
8048f680 0003
Call Trace:8032218a{usb_hcd_pci_suspend+74}
8028519e{pci_device_suspend+30}
802ee3d2{suspend_device+50}
802ee4f1{device_suspend+129}
80166ceb{prepare_devices+11}
80167095{pm_suspend_disk+21}
80164206{enter_state+70} 8016442d{state_store+109}
801f275f{subsys_attr_store+31}
801f2c1c{sysfs_write_file+204}
8019c6c9{vfs_write+233} 8019c863{sys_write+83}
8010f092{system_call+126}
Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
RIP 80283a70{pci_choose_state+96} RSP 810020fbfd78
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll Alice's Adventures in Wonderland
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
On Monday, 21 of March 2005 11:51, you wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Pavel, that's the BUG() in pci_choose_state(). I did have some
reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
some incompatible intent in there.
I dunno why pci_choose_state() is saying that it received PCI_D1, when
prepare_devices() is passing down PMSG_FREEZE?
Uf, I don't know what version that was.. I think I have
VERSION = 2
PATCHLEVEL = 6
SUBLEVEL = 12
EXTRAVERSION =-rc1-mm1
and that says:
#define PMSG_FREEZE ((__force pm_message_t) 3)
... I certainly have _FREEZE defined as 1 in my local tree, but I do
not see that change in -mm yet.
Possibly pm.h changes went in faster than pci.c or something like
that?
I reproduced it here.. I do not know who introduced
platform_pci_choose_state, but it is *very* wrong. It returns
it. Should it return pci_power_t? It probably should to match
pci_choose_state, but that int is retyped to pm_message_t. Oops.
Pavel
Greets,
Rafael
Stopping tasks:
===|
Freeing memory... done (66711 pages freed)
They asked me for state 1
--- [cut here ] - [please bite here ] -
Kernel BUG at pci:389
invalid operand: [1]
CPU 0
Modules linked in: usbserial parport_pc lp parport thermal processor fan
button battery ac soundcore snd_page_alloc ipt_TOS ipt_LOG ipt_limit v
Pid: 9141, comm: do_acpi_sleep Not tainted 2.6.12-rc1-mm1
RIP: 0010:[80283a70] 80283a70{pci_choose_state+96}
RSP: :810020fbfd78 EFLAGS: 00010292
RAX: 001d RBX: 0001 RCX:
RDX: RSI: 44e0 RDI: 8041d140
RBP: 81002fc151c0 R08: R09: 81002a535c48
R10: R11: R12: 81002fc151c0
R13: 0003 R14: R15: 0080
FS: 2b28b800() GS:8055c840() knlGS:
CS: 0010 DS: ES: CR0: 8005003b
CR2: 2aac2000 CR3: 1dd8a000 CR4: 06e0
Process do_acpi_sleep (pid: 9141, threadinfo 810020fbe000, task
810020d527e0)
Stack: 81002c349628 81002c349628 8032218a
81002fc149a8 81002fc15230
8048f680 0003
Call Trace:8032218a{usb_hcd_pci_suspend+74}
8028519e{pci_device_suspend+30}
802ee3d2{suspend_device+50}
802ee4f1{device_suspend+129}
80166ceb{prepare_devices+11}
80167095{pm_suspend_disk+21}
80164206{enter_state+70}
8016442d{state_store+109}
801f275f{subsys_attr_store+31}
801f2c1c{sysfs_write_file+204}
8019c6c9{vfs_write+233} 8019c863{sys_write+83}
8010f092{system_call+126}
Code: 0f 0b 7a 3e 3e 80 ff ff ff ff 85 01 31 d2 66 90 48 8b 5c 24
RIP 80283a70{pci_choose_state+96} RSP 810020fbfd78
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll Alice's Adventures in Wonderland
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Pavel, that's the BUG() in pci_choose_state(). I did have some
reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
some incompatible intent in there.
I dunno why pci_choose_state() is saying that it received PCI_D1, when
prepare_devices() is passing down PMSG_FREEZE?
This works it around:
--- clean-mm/drivers/pci/pci.c 2005-03-21 11:39:32.0 +0100
+++ linux-mm/drivers/pci/pci.c 2005-03-22 01:41:48.0 +0100
@@ -376,11 +376,13 @@
if (!pci_find_capability(dev, PCI_CAP_ID_PM))
return PCI_D0;
+#if 0
if (platform_pci_choose_state) {
ret = platform_pci_choose_state(dev, state);
if (ret = 0)
state = ret;
}
+#endif
switch (state) {
case 0: return PCI_D0;
case 3: return PCI_D3hot;
platform_pci_choose_state is very wrong, and it would be nice to just
revert the patch that introduced it. pm_message_t is going to became a
structure, and I don't want to have another place to fixup.
Hmm, it looks like I should do switch to the structure *now* so that
pm_message_t becomes incompatible with int and people can't get it
wrong...
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek [EMAIL PROTECTED] wrote:
Hi!
On Monday, 21 of March 2005 11:51, you wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc1/2.6.12-rc1-mm1/
I get the following BUG every time I try to suspend my box to disk.
Pavel, that's the BUG() in pci_choose_state(). I did have some
reject-fixing to do on that wrt a change in Greg's tree, so maybe there was
some incompatible intent in there.
I dunno why pci_choose_state() is saying that it received PCI_D1, when
prepare_devices() is passing down PMSG_FREEZE?
Uf, I don't know what version that was.. I think I have
VERSION = 2
PATCHLEVEL = 6
SUBLEVEL = 12
EXTRAVERSION =-rc1-mm1
yes, the report was against 2.6.12-rc1-mm1.
and that says:
#define PMSG_FREEZE ((__force pm_message_t) 3)
... I certainly have _FREEZE defined as 1 in my local tree, but I do
not see that change in -mm yet.
Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
#define PMSG_FREEZE ((__force pm_message_t) 3)
#define PMSG_SUSPEND((__force pm_message_t) 3)
#define PMSG_ON ((__force pm_message_t) 0)
which looks odd.
Possibly pm.h changes went in faster than pci.c or something like
that?
grep says that 2.6.12-rc1-mm1 has these patches from you:
fix-suspend-resume-on-via-velocity.patch
x86-fix-esp-corruption-cpu-bug-take-2.patch
swsusp-add-missing-refrigerator-calls.patch
suspend-to-ram-update-videotxt-with-more-systems.patch
pm-remove-obsolete-pm_-from-vtc.patch
swsusp-small-updates.patch
swsusp-1-1-kill-swsusp_restore.patch
pcmcia-id_table-for-orinoco_cs.patch
fix-pm_message_t-in-generic-code.patch
fix-u32-vs-pm_message_t-in-usb.patch
more-pm_message_t-fixes.patch
fix-u32-vs-pm_message_t-confusion-in-oss.patch
fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch
fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch
fix-u32-vs-pm_message_t-confusion-in-mmc.patch
fix-u32-vs-pm_message_t-confusion-in-serials.patch
fix-u32-vs-pm_message_t-in-macintosh.patch
fix-u32-vs-pm_message_t-confusion-in-agp.patch
I reproduced it here.. I do not know who introduced
platform_pci_choose_state, but it is *very* wrong. It returns
it. Should it return pci_power_t? It probably should to match
pci_choose_state, but that int is retyped to pm_message_t. Oops.
That change came from Len. I've appended the two relevant patches below.
So hm. We have incompatible changes in flight. That doesn't happen very
often.
Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send
that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1
based tree then I can look after it until things get flushed out.
(Len, platform_pci_set_power_state shouldn't be initialised to NULL).
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/19 00:15:48-05:00 [EMAIL PROTECTED]
# [ACPI] PCI can now get suspend state from firmware
#
# pci_choose_state() can now call
# platform_pci_choose_state()
# and ACPI can answer
#
# http://bugzilla.kernel.org/show_bug.cgi?id=4277
#
# Signed-off-by: David Shaohua Li [EMAIL PROTECTED]
# Signed-off-by: Len Brown [EMAIL PROTECTED]
#
# drivers/pci/pci.h
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +3 -0
# add platform_pci_choose_state()
#
# drivers/pci/pci.c
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +7 -0
# add platform_pci_choose_state()
#
# drivers/pci/pci-acpi.c
# 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +46 -1
# add platform_pci_choose_state()
#
diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
--- a/drivers/pci/pci-acpi.c2005-03-21 17:01:44 -08:00
+++ b/drivers/pci/pci-acpi.c2005-03-21 17:01:44 -08:00
@@ -1,6 +1,6 @@
/*
* File: pci-acpi.c
- * Purpose:Provide PCI support in ACPI
+ * Purpose:Provde PCI support in ACPI
*
* Copyright (C) 2005 David Shaohua Li [EMAIL PROTECTED]
* Copyright (C) 2004 Tom Long Nguyen [EMAIL PROTECTED]
@@ -17,6 +17,7 @@
#include acpi/acpi_bus.h
#include linux/pci-acpi.h
+#include pci.h
static u32 ctrlset_buf[3] = {0, 0, 0};
static u32 global_ctrlsets = 0;
@@ -209,6 +210,49 @@
}
EXPORT_SYMBOL(pci_osc_control_set);
+/*
+ * _SxD returns the D-state with the highest power
+ * (lowest D-state number) supported in the S-state x.
+ *
+ * If the devices does not have a _PRW
+ * (Power Resources for Wake) supporting system wakeup from x
+ * then the OS is free to choose a lower power (higher number
+ * D-state) than the return value from _SxD.
+ *
+ * But if _PRW is enabled at S-state x, the OS
+ * must not choose a power lower than _SxD --
+ * unless the device has an _SxW method specifying
+ * the lowest power (highest D-state number) the device
+ * may enter while still able to wake the system.
+ *
+ * ie. depending on global OS policy:
+ *
+ * if (_PRW at S-state x)
+ * choose from highest power _SxD to lowest power _SxW
+ * else // no _PRW at S-state x
+ * choose highest power _SxD or any lower power
+ *
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Hi!
and that says:
#define PMSG_FREEZE ((__force pm_message_t) 3)
... I certainly have _FREEZE defined as 1 in my local tree, but I do
not see that change in -mm yet.
Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
#define PMSG_FREEZE ((__force pm_message_t) 3)
#define PMSG_SUSPEND((__force pm_message_t) 3)
#define PMSG_ON ((__force pm_message_t) 0)
which looks odd.
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed
all direct accesses to the constants, and will switch constants to
something else... But that is going to be tommorow (need some sleep).
I reproduced it here.. I do not know who introduced
platform_pci_choose_state, but it is *very* wrong. It returns
it. Should it return pci_power_t? It probably should to match
pci_choose_state, but that int is retyped to pm_message_t. Oops.
That change came from Len. I've appended the two relevant patches below.
So hm. We have incompatible changes in flight. That doesn't happen very
often.
Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send
that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1
based tree then I can look after it until things get flushed out.
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
And they are both dangerous -- they introduce new and untested
functionality while I'm trying to transition from int to
pm_message_t. They also affect all the drivers.
Len, please Cc me on patches that affect suspend.
@@ -17,6 +17,7 @@
#include acpi/acpi_bus.h
#include linux/pci-acpi.h
+#include pci.h
Should be linux/pci.h?
+static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t state)
+{
Should return pci_power_t, probably.
+ char dstate_str[] = _S0D;
+ acpi_status status;
+ unsigned long val;
+ struct device *dev = pdev-dev;
+
+ /* Fixme: the check is wrong after pm_message_t is a struct */
Exactly.
+ if ((state = PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
PM_SUSPEND_MAX and friends is going to disappear.
+ return -EINVAL;
+ dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
Ugh, assumes numerical values of states actually meaning anything. It
definitely should not. Should be switch(state.event), but that code
is not merged, yet = I'll send code that switches pm_message_t to
struct, tommorow. But it may compile-time break some obscure drivers...
diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
--- a/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
+++ b/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
@@ -253,6 +253,24 @@
return -ENODEV;
}
+static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev-dev);
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
...
and then it should not need __force.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek [EMAIL PROTECTED] wrote: On Po 21-03-05 17:52:32, Andrew Morton wrote: Pavel Machek [EMAIL PROTECTED] wrote: Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 based tree then I can look after it until things get flushed out. Could you just revert those two patches? First one is very wrong. Second one might be fixed, but... See comments below. I could revert them locally, but that wouldn't gain us much. You mean that Len has to revert them or revert is ineffective? The patches are in Len's tree. Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the best. Perhaps I should just jam everything-from-Pavel into Linus's tree as soon as he returns and then we can fix up the downstream fallout in the various bk trees? Yes, that would help a lot. I was waiting with turn-pm_message_t-into-struct until all pm_message_t patches reached Linus so that there's not a mess in flight. Len's patch pretty much depends on pm_message_t already being converted... (and I'd prefer it to wait a while, so we can see which problems were introduced by conversion and which are due to ACPI BIOS bugs). OK, well unless someone has objections I'll just send all these swsusp-add-missing-refrigerator-calls.patch suspend-to-ram-update-videotxt-with-more-systems.patch pm-remove-obsolete-pm_-from-vtc.patch swsusp-small-updates.patch swsusp-1-1-kill-swsusp_restore.patch fix-pm_message_t-in-generic-code.patch fix-u32-vs-pm_message_t-in-usb.patch more-pm_message_t-fixes.patch fix-u32-vs-pm_message_t-confusion-in-oss.patch fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch fix-u32-vs-pm_message_t-confusion-in-mmc.patch fix-u32-vs-pm_message_t-confusion-in-serials.patch fix-u32-vs-pm_message_t-in-macintosh.patch fix-u32-vs-pm_message_t-confusion-in-agp.patch to Linus when he reappears and then I'll duck for cover and let you guys sort it out ;) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Po 21-03-05 17:52:32, Andrew Morton wrote: Pavel Machek [EMAIL PROTECTED] wrote: Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 based tree then I can look after it until things get flushed out. Could you just revert those two patches? First one is very wrong. Second one might be fixed, but... See comments below. I could revert them locally, but that wouldn't gain us much. You mean that Len has to revert them or revert is ineffective? Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the best. Perhaps I should just jam everything-from-Pavel into Linus's tree as soon as he returns and then we can fix up the downstream fallout in the various bk trees? Yes, that would help a lot. I was waiting with turn-pm_message_t-into-struct until all pm_message_t patches reached Linus so that there's not a mess in flight. Len's patch pretty much depends on pm_message_t already being converted... (and I'd prefer it to wait a while, so we can see which problems were introduced by conversion and which are due to ACPI BIOS bugs). Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Mon, Mar 21, 2005 at 05:06:23PM -0800, Andrew Morton wrote: # drivers/pci/pci-acpi.c # 2005/03/19 00:15:24-05:00 [EMAIL PROTECTED] +46 -1 # add platform_pci_choose_state() # diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c --- a/drivers/pci/pci-acpi.c 2005-03-21 17:01:44 -08:00 +++ b/drivers/pci/pci-acpi.c 2005-03-21 17:01:44 -08:00 @@ -1,6 +1,6 @@ /* * File:pci-acpi.c - * Purpose: Provide PCI support in ACPI + * Purpose: Provde PCI support in ACPI Oops. Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Tue, 2005-03-22 at 09:35, Pavel Machek wrote:
Hi!
and that says:
#define PMSG_FREEZE ((__force pm_message_t) 3)
... I certainly have _FREEZE defined as 1 in my local tree, but I
do
not see that change in -mm yet.
Both 2.6.12-rc1-mm1 and 2.6.12-rc1 have:
#define PMSG_FREEZE ((__force pm_message_t) 3)
#define PMSG_SUSPEND((__force pm_message_t) 3)
#define PMSG_ON ((__force pm_message_t) 0)
which looks odd.
Yes, but it is needed. There are many drivers, and they look at
numerical value of PMSG_*. I'm proceeding in steps. I hopefully killed
all direct accesses to the constants, and will switch constants to
something else... But that is going to be tommorow (need some sleep).
The patches are going to acquire correct PCI device sleep state for
suspend/resume. We discussed the issue several months ago. My plan is we
first introduce 'platform_pci_set_power_state', then merge the
'platform_pci_choose_state' patch after Pavel's pm_message_t conversion
finished. Maybe Len mislead my comments.
Anyway for the callback, my intend is platform_pci_choose_state accept
the pm_message_t parameter, and it return an 'int', since platform
method possibly failed and then pci_choose_state translate the return
value to pci_power_t.
I reproduced it here.. I do not know who introduced
platform_pci_choose_state, but it is *very* wrong. It returns
it. Should it return pci_power_t? It probably should to match
pci_choose_state, but that int is retyped to pm_message_t. Oops.
That change came from Len. I've appended the two relevant patches
below.
So hm. We have incompatible changes in flight. That doesn't happen
very
often.
Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and
send
that to Len and myself? If that fixup is not suitable for a
2.6.12-rc1
based tree then I can look after it until things get flushed out.
Could you just revert those two patches? First one is very
wrong. Second one might be fixed, but... See comments below.
I think the platform_pci_set_power_state should be ok, did you see it
causes oops?
And they are both dangerous -- they introduce new and untested
functionality while I'm trying to transition from int to
pm_message_t. They also affect all the drivers.
Len, please Cc me on patches that affect suspend.
@@ -17,6 +17,7 @@
#include acpi/acpi_bus.h
#include linux/pci-acpi.h
+#include pci.h
Should be linux/pci.h?
I suppose it's not exported out side of PCI, so I used 'pci.h'
+static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t
state)
+{
Should return pci_power_t, probably.
Should return int as I said above.
+ char dstate_str[] = _S0D;
+ acpi_status status;
+ unsigned long val;
+ struct device *dev = pdev-dev;
+
+ /* Fixme: the check is wrong after pm_message_t is a struct */
Exactly.
+ if ((state = PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
PM_SUSPEND_MAX and friends is going to disappear.
Yep, this should be fixed.
+ return -EINVAL;
+ dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
Ugh, assumes numerical values of states actually meaning anything. It
definitely should not. Should be switch(state.event), but that code
is not merged, yet = I'll send code that switches pm_message_t to
struct, tommorow. But it may compile-time break some obscure
drivers...
diff -Nru a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
--- a/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
+++ b/drivers/pci/pci-acpi.c 2005-03-21 17:02:38 -08:00
@@ -253,6 +253,24 @@
return -ENODEV;
}
+static int acpi_pci_set_power_state(struct pci_dev *dev,
pci_power_t state)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev-dev);
+ static int state_conv[] = {
+ [0] = 0,
+ [1] = 1,
+ [2] = 2,
+ [3] = 3,
+ [4] = 3
+ };
+ int acpi_state = state_conv[(int __force) state];
The table should be
[PCI_D0] = 0,
...
Ok, please revert the 'platform_pci_choose_pci' patch, I will add it
after Pavel's conversion is finished. Or after Pavel's is done, I can
send a quick fix.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Pavel Machek [EMAIL PROTECTED] wrote: Could I suggest that you prepare a fixup against 2.6.12-rc1-mm1 and send that to Len and myself? If that fixup is not suitable for a 2.6.12-rc1 based tree then I can look after it until things get flushed out. Could you just revert those two patches? First one is very wrong. Second one might be fixed, but... See comments below. I could revert them locally, but that wouldn't gain us much. Greg hasn't taken the pm_message_t patches yet. Perhaps that's for the best. Perhaps I should just jam everything-from-Pavel into Linus's tree as soon as he returns and then we can fix up the downstream fallout in the various bk trees? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Will this do it for the moment?
If so, lets use it until Pavel's flag-day is over -- when we'll send an
updated patch.
thanks,
-Len
= drivers/pci/pci-acpi.c 1.4 vs edited =
--- 1.4/drivers/pci/pci-acpi.c 2005-03-03 04:28:23 -05:00
+++ edited/drivers/pci/pci-acpi.c 2005-03-21 22:59:39 -05:00
@@ -237,19 +237,8 @@
static int acpi_pci_choose_state(struct pci_dev *pdev, pm_message_t state)
{
- char dstate_str[] = _S0D;
- acpi_status status;
- unsigned long val;
- struct device *dev = pdev-dev;
+ /* TBD */
- /* Fixme: the check is wrong after pm_message_t is a struct */
- if ((state = PM_SUSPEND_MAX) || !DEVICE_ACPI_HANDLE(dev))
- return -EINVAL;
- dstate_str[2] += state; /* _S1D, _S2D, _S3D, _S4D */
- status = acpi_evaluate_integer(DEVICE_ACPI_HANDLE(dev), dstate_str,
- NULL, val);
- if (ACPI_SUCCESS(status))
- return val;
return -ENODEV;
}
Re: 2.6.12-rc1-mm1: Kernel BUG at pci:389
On Mon, Mar 21, 2005 at 06:27:33PM -0800, Andrew Morton wrote: OK, well unless someone has objections I'll just send all these swsusp-add-missing-refrigerator-calls.patch suspend-to-ram-update-videotxt-with-more-systems.patch pm-remove-obsolete-pm_-from-vtc.patch swsusp-small-updates.patch swsusp-1-1-kill-swsusp_restore.patch fix-pm_message_t-in-generic-code.patch fix-u32-vs-pm_message_t-in-usb.patch more-pm_message_t-fixes.patch fix-u32-vs-pm_message_t-confusion-in-oss.patch fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch fix-u32-vs-pm_message_t-confusion-in-mmc.patch fix-u32-vs-pm_message_t-confusion-in-serials.patch fix-u32-vs-pm_message_t-in-macintosh.patch fix-u32-vs-pm_message_t-confusion-in-agp.patch to Linus when he reappears and then I'll duck for cover and let you guys sort it out ;) No objection from me, that's probably the best way for this to get into the tree. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
