Thread (18 messages) 18 messages, 4 authors, 2009-07-10

Re: [PATCH 1/2] ACPI: reintroduce acpi_device_ops .shutdown method

From: David Härdeman <david@hardeman.nu>
Date: 2009-07-01 08:21:11
Also in: linux-acpi, lkml

On Wed, July 1, 2009 01:11, Bjorn Helgaas wrote:
On Tuesday 30 June 2009 3:10:47 pm Andrew Morton wrote:
quoted
On Sat, 27 Jun 2009 07:18:31 +0200
David Härdeman [off-list ref] wrote:
quoted
This reintroduces the .shutdown method which is used by the
winbond-cir driver. A normal revert wasn't possible since there
had been other changes to include/acpi/acpi_bus.h since.
Len, Bjorn: is this OK?  Or is there some other mechanism which the
driver should have used?
I'm on vacation and don't have time to read the new winbond driver
right now, but maybe it could be changed so that wbcir_shutdown()
is an internal function called by wbcir_suspend() (as it is already)
and wbcir_remove().
I didn't want to call wbcir_shutdown from wbcir_remove since wbcir_remove
is called on rmmod and the wbcir_shutdown method programs the chip so that
wake-on-a-specific-ir-command is enabled (and I imagine that people would
expect rmmod to disable the hardware completely). I will clarify the
function names a bit in the next version of the patch...

As far as I could understand from my testing, the acpi .suspend method is
not called during shutdown which is why I needed to hook into both
.suspend and .shutdown separately.
I hate to re-introduce .shutdown when it's only used by a single
driver.  That makes me think either we have a bunch of drivers that
are buggy because they *should* have .shutdown methods but don't,
or the single user of .shutdown doesn't have a real dependency on it.
I think this single user does have a real dependency because of its
wake-from-poweroff and wake-from-suspend capability. But I could be
mistaken...
The winbond driver does not use any ACPI-specific functionality, so
it might be simpler to write it as a PNP driver (which would depend
on PNPACPI, of course).
As far as I could tell from a quick look at include/linux/pnp.h, a
pnp_driver doesn't seem to have any .shutdown methods either, so I'm not
sure how it would help?

(On a related note, it seems inconsistent to me that platform_driver has a
.shutdown method while pnp_driver and acpi_driver doesn't.)

If you disagree with acpi_driver regaining the .shutdown method I guess
the only options are to use register_reboot_notifier or to rewrite the
driver as a platform_driver (Alan Cox seemed to suggest earlier that the
driver was not a good fit for a platform driver).
Nice looking driver, by the way.  Even from a cursory glance it's
obvious that you've taken a lot of care with it.
Thank you, and have a nice vacation.

Regards,
David


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help