Re: [PATCH 0/6] ACPI: button: minor cleanups

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Wednesday 06 May 2009 11:20:52 am Bjorn Helgaas wrote:
> On Wednesday 06 May 2009 09:13:07 am Thomas Renninger wrote:
> > Below is the outcome of the patch which fixes a fixed feature button
> > s2ram issue.
> > 
> > This does not work anymore since Bjoern's patches.
> > Comparing the with the HID of the acpi device looks ugly.
> > 
> > Shall I revive the fixed feature vs GPE button types?
> > ACPI_BUTTON_TYPE_POWERF and ACPI_BUTTON_TYPE_SLEEPF?
> 
> I don't understand exactly how I broke this.

What tree are you testing?  In Linus's upstream tree, 373cfc360e
adds the .notify method, and at the same time, it removes
acpi_button_notify_fixed().

Your patch below, which fixes the problem, shows that you already
have the .notify method, and yet you apparently *also* have the
acpi_button_notify_fixed() function.  That makes me suspect that
you're using a tree with a merge error.

Bjorn

> > @@ -85,6 +86,7 @@ static struct acpi_driver acpi_button_driver = {
> >  	.ops = {
> >  		.add = acpi_button_add,
> >  		.resume = acpi_button_resume,
> > +		.suspend = acpi_button_suspend,
> >  		.remove = acpi_button_remove,
> >  		.notify = acpi_button_notify,
> >  	},
> > @@ -281,8 +283,41 @@ static int acpi_button_resume(struct acpi_device *device)
> >  {
> >  	struct acpi_button *button = acpi_driver_data(device);
> >  
> > -	if (button->type == ACPI_BUTTON_TYPE_LID)
> > +	if (!button)
> > +		return -EINVAL;
> > +	switch (button->type) {
> > +	case ACPI_BUTTON_TYPE_LID:
> >  		return acpi_lid_send_state(device);
> > +	case ACPI_BUTTON_TYPE_SLEEPF:
> > +		return acpi_install_fixed_event_handler(
> > +					ACPI_EVENT_SLEEP_BUTTON,
> > +					acpi_button_notify_fixed, button);
> > +
> > +	case ACPI_BUTTON_TYPE_POWERF:
> > +		return acpi_install_fixed_event_handler(
> > +					ACPI_EVENT_POWER_BUTTON,
> > +					acpi_button_notify_fixed, button);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int acpi_button_suspend(struct acpi_device *device, pm_message_t state)
> > +{
> > +	struct acpi_button *button;
> > +	if (!device)
> > +		return -EINVAL;
> > +	button = acpi_driver_data(device);
> > +	if (!button)
> > +		return -EINVAL;
> > +	switch (button->type) {
> > +	case ACPI_BUTTON_TYPE_SLEEPF:
> > +		return acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
> > +					       acpi_button_notify_fixed);
> > +
> > +	case ACPI_BUTTON_TYPE_POWERF:
> > +		return acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
> > +					       acpi_button_notify_fixed);
> > +	}
> >  	return 0;
> >  }
> >  
> > 
> 
> 
>