Re: [PATCH]libata-acpi: add ACPI _PSx method

Shaohua Li
2007-11-05T01:26:12Z

On Fri, 2007-11-02 at 12:16 +0100, Rafael J. Wysocki wrote:
> >
> > Index: linux/drivers/ata/libata-acpi.c
> > ===================================================================
> > --- linux.orig/drivers/ata/libata-acpi.c 2007-11-01 10:54:25.000000000 +0800
> > +++ linux/drivers/ata/libata-acpi.c 2007-11-02 09:14:12.000000000 +0800
> > @@ -652,6 +652,35 @@ void ata_acpi_on_resume(struct ata_port
> > }
> >
> > /**
> > + * ata_acpi_set_state - set the port power state
> > + * @ap: target ATA port
> > + * @state: state, on/off
> > + *
> > + * This function executes the _PS0/_PS3 ACPI method to set the power state.
> > + * ACPI spec requires _PS0 when IDE power on and _PS3 when power off
> > + */
> > +void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
>
> I wouldn't use pm_message_t here.
>
> We're seriously considering removing it in the future and the caller passes
> constants explicitly anyway.
>
> IMO, it might be cleaner to define two separate functions, one for switching
> to D0 and one for switching to D3, and call them as appropriate. Then, you
> could get rid of some conditionals below.
It doesn't make sense to add a new one just for ata_acpi and we don't
misuse pm_message_t here. If you will remove it, lets clean up it then.

Thanks,
Shaohua