Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 8BIT
Subject: RE: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Date: Tue, 22 Mar 2005 20:13:13 +0800
Message-ID: <16A54BF5D6E14E4D916CE26C9AD3057501731439@pdsmsx402.ccr.corp.intel.com>
Thread-Topic: 2.6.12-rc1-mm1: Kernel BUG at pci:389
Thread-Index: AcUuzmZCC5QCVGVVQlK2s3YBI6SL1QACL7IA
From: "Li, Shaohua" <shaohua.li@intel.com>
To: "Pavel Machek" <pavel@ucw.cz>
Cc: "Andrew Morton" <akpm@osdl.org>, <rjw@sisk.pl>,
       "lkml" <linux-kernel@vger.kernel.org>,
       "Brown, Len" <len.brown@intel.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2272
Lines: 64

>
>> > 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/