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/