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

[Shaohua Li <shaohua.li@intel.com>]

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