bay: Exit if notify handler cannot be installed
PATCH 1/1 bay: Exit if notify handler cannot be installed
If acpi_install_notify_handler() for a bay device fails, the bay driver is superfluous. Most likely, another driver (like libata) is already caring about this device anyway. Furthermore, register_hotplug_dock_device(acpi_handle) from the dock driver must not be called twice with the same handler. So clean up and exit.
---
drivers/acpi/bay.c +1 -0
@@ -311,6 +311,7 @@ static int bay_add(acpi_handle handle, int id) bay_notify, new_bay);if (ACPI_FAILURE (status)) { printk(KERN_ERR PREFIX "Error installing bay notify handler\n"); + goto bay_add_err; }/* if we are on a dock station, we should register for dock
Quoted 5 lines · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the bay driver is superfluous. Most likely, another driver (like libata) is already caring about this device anyway. Furthermore,register_hotplug_dock_device(acpi_handle)from the dock driver must not be called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
Regards,
Holger
Quoted 4 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
Quoted 8 lines
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver mustnot be
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Thanks,
Shaohua
Quoted 8 lines · Shaohua Li
Quoted 7 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver must
Quoted 7 lines
Quoted 5 lines
not be
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Yes, I know, but couldn't it be helpful on systems not using libata?
Regards,
Holger
Quoted 10 lines · Holger Macht
Quoted 9 lines · Shaohua Li
Quoted 8 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver mustnot be
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Yes, I know, but couldn't it be helpful on systems not using libata?
Also, libata has to be patched to execute ACPI _EJ0 (not a huge issue, I know) or calling bay's eject_removable drive method.
Regards,
Holger
Quoted 15 lines · Holger Macht
Quoted 14 lines · Holger Macht
Quoted 13 lines · Shaohua Li
Quoted 12 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver mustnot be
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Yes, I know, but couldn't it be helpful on systems not using libata?
Also, libata has to be patched to execute ACPI
_EJ0(not a huge issue, I know) or calling bay'seject_removabledrive method.
It *is* a huge issue. I think that currently, bay handling on thinkpads is broken because of it, and users that expect it to just work could well fry their hardware if they're the sort of people who don't LOOK at the bay light before they yank stuff from it :-)
This is not a bug report, but a head's up. I am at 2.6.23, so I didn't test for the bug yet.
Anyway, when you bind to an ACPI node, you forbid anything else from doing it. This means it is now your problem to handle **ALL** capabilities of that node. This means libata MUST handle ejection, or it MUST NOT bind to an ACPI node.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 29 lines · Henrique de Moraes Holschuh
Quoted 15 lines · Holger Macht
Quoted 14 lines · Holger Macht
Quoted 13 lines · Shaohua Li
Quoted 12 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver mustnot be
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Yes, I know, but couldn't it be helpful on systems not using libata?
Also, libata has to be patched to execute ACPI
_EJ0(not a huge issue, I know) or calling bay'seject_removabledrive method.It *is* a huge issue. I think that currently, bay handling on thinkpads is broken because of it, and users that expect it to just work could well fry their hardware if they're the sort of people who don't LOOK at the bay light before they yank stuff from it :-)
Quoted 7 lines
This is not a bug report, but a head's up. I am at 2.6.23, so I didn't test for the bug yet.
Anyway, when you bind to an ACPI node, you forbid anything else from doing it. This means it is now your problem to handle **ALL** capabilities of that node. This means libata MUST handle ejection, or it MUST NOT bind to an ACPI node.
I fully agree. With "not a huge issue", I rather meant that coding this two lines of code is pretty easy ;-)
Regards,
Holger
Quoted 15 lines · Holger Macht
Quoted 14 lines · Holger Macht
Quoted 13 lines · Shaohua Li
Quoted 12 lines · Holger Macht
Quoted 1 line · Holger Macht
If
acpi_install_notify_handler()for a bay device fails, the baydriver is
Quoted 1 line
superfluous. Most likely, another driver (like libata) is already
caring
Quoted 2 lines
about this device anyway. Furthermore,
register_hotplug_dock_device(acpi_handle)from the dock driver mustnot be
Quoted 1 line
called twice with the same handler. So clean up and exit.
The patch needs some more work. I'll send an updated one as soon as ready.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Yes, I know, but couldn't it be helpful on systems not using libata?
Also, libata has to be patched to execute ACPI
_EJ0(not a huge issue, I know) or calling bay'seject_removabledrive method.
I have a patch in above bugzilla to handle it.
Thanks,
Shaohua
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 7 lines · On Tue, 6 May 2008 12:18:46 -0300 Henrique de Moraes Holschuh <[email protected]>…
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The bay driver needs to become a service that hands out bay events to all the other drivers for that to happen.
Quoted 6 lines · Alan Cox
Quoted 4 lines · On Tue, 6 May 2008 12:18:46 -0300 Henrique de Moraes Holschuh <[email protected]>…
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Quoted 6 lines
Quoted 3 lines
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The bay driver needs to become a service that hands out bay events to all the other drivers for that to happen.
Seems sensible. Any pointers? I wouldn't know how to do it (and actually, right now I am busy working on rfkill for some stuff thinkpad-acpi needs), but I could try to tack bay after rfkill, if nobody beats me to it (hint!)
BTW: dock handling might share these issues as well.
The problem has a bit of a hard edge, though: if nobody binds to an ACPI node that has an EJ0 subnode, bay needs to do it. But if someone wants to, bay should give the node up, and somehow help that someone handle the ejection stuff.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 4 lines
The problem has a bit of a hard edge, though: if nobody binds to an ACPI node that has an EJ0 subnode, bay needs to do it. But if someone wants to, bay should give the node up, and somehow help that someone handle the ejection stuff.
A notifier chain should do all that is needed. If bay is loaded anyway then ATA and other users can register with the bay driver and it can pass an event to a notifier chain so that anyone who wants that event can act on it (and if nobody does it can do its own stuff)
Alan
Quoted 14 lines · Henrique de Moraes Holschuh
Quoted 6 lines · Alan Cox
Quoted 4 lines · On Tue, 6 May 2008 12:18:46 -0300 Henrique de Moraes Holschuh <[email protected]>…
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Quoted 6 lines
Quoted 3 lines
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The bay driver needs to become a service that hands out bay events to all the other drivers for that to happen.
Quoted 6 lines
Seems sensible. Any pointers? I wouldn't know how to do it (and actually, right now I am busy working on rfkill for some stuff thinkpad-acpi needs), but I could try to tack bay after rfkill, if nobody beats me to it (hint!)
BTW: dock handling might share these issues as well.
For pointers, the dock driver should be a good starting point. It already handles this. For example, libata just calls register_hotplug_dock_device(handle, callback) and gets notified.
Regards,
Holger
Quoted 5 lines
The problem has a bit of a hard edge, though: if nobody binds to an ACPI node that has an EJ0 subnode, bay needs to do it. But if someone wants to, bay should give the node up, and somehow help that someone handle the ejection stuff.
dock does this:
acpi event --> dock calls all registered handlers from other drivers --> --> dock executes _DCK
Regards,
Holger
Quoted 11 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Yes, it still does with my patch.
Regards,
Holger
Quoted 11 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Isn't this the job of acpi dock driver?
Quoted 5 lines · Shaohua Li
Quoted 4 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
Quoted 8 lines
Quoted 7 lines
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Isn't this the job of acpi dock driver?
No, but I actually fail to see why do we even care about the difference from a dock to a bay. Dock *could* be made to handle both.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 10 lines · Henrique de Moraes Holschuh
Quoted 9 lines · Shaohua Li
Quoted 8 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Isn't this the job of acpi dock driver?
No, but I actually fail to see why do we even care about the difference from a dock to a bay. Dock *could* be made to handle both.
It appears some systems haven't a dock but a bay. In a thinkpad, you could hotplug cdrom/battery/harddisk in the slot of cdrom, but it's not a dock. libata should handle it well (but it doesn't handle _EJ0 currently, I have a patch in above bugzilla).
But you are right, acpi dock driver should be extended to support bay. I'll give it a try.
Thanks,
Shaohua
Quoted 18 lines · Shaohua Li
Quoted 10 lines · Henrique de Moraes Holschuh
Quoted 9 lines · Shaohua Li
Quoted 8 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Isn't this the job of acpi dock driver?
No, but I actually fail to see why do we even care about the difference from a dock to a bay. Dock *could* be made to handle both.
Quoted 7 lines
It appears some systems haven't a dock but a bay. In a thinkpad, you could hotplug cdrom/battery/harddisk in the slot of cdrom, but it's not a dock. libata should handle it well (but it doesn't handle
_EJ0currently, I have a patch in above bugzilla).But you are right, acpi dock driver should be extended to support bay. I'll give it a try.
Thanks. Here's what I know about bays (and docks). I expect you know most, if not all of what I'll write, but what the heck, it is good to have these things in the clear :-)
1. Bay and dock notifications are NOT standard. You need to feed them to a notification chain AND generate uevents, and expect a reply before you can actually do something.
There are two big classes of machines here:
1a) those which notify you that the bay/dock is BEING ejected 1b) those which notify you the user wants to eject, and wait for
a command to power down the bay/dock and let the user know she can remove the device.
Thinkpads are on the 1b class. See (4) below.
2. Even if a bay can take different types of devices, the same bay may appear as a number of different EJ0 handlers. In that case, the firmware is to know which handler to use based on which type of device is inside the bay. Thinkpads do this.
3. IMO, "eject" really should be a device property, and not something tied to a bay. For docks, this is a lot more difficult to do right, since a dock has multiple devices (and usually at least one BUS, and often more than one. Thinkpads have PCI, PCIe and USB buses through the dock) behind it, so it is probably best to have dock ejects as a property of a "dock" platform device, and bay ejects as a property of whatever device is inside the bay. This should be an userspace AND kernel API.
4. bay ejection notifications can take two forms (see (1) above):
4a) device removal (we already have this)
4b) device removal REQUEST
Thinkpads should do 4b, then IF AND ONLY IF the user or a kernel platform driver uses (3) to command the ejection, they do 4a.
Some laptops will just do 4a, doing (3) before the user yanks the device requires the user to not be an yahoo (i.e. just like USB pen-drives).
And for laptops that do 4b for docks, one should probably replicate the "request undock" notification for every device inside the dock, and not just for the dock device.
What value you get from an ACPI NOTIFY for 4a or 4b is NOT standard, you will need a platform device OR userspace to interpret it for you.
5. bay and dock notifications (at least on thinkpads) are issued to the ACPI node of the real device, not to the APCI EJ0 node. This is what is causing issues between bay and libata (both want to register a notify hook to the same node, and ACPICA allows only one driver per node).
I am not really sure the "ACPI driver model" is the right way to tie bay functionality to the system. A hook and notifier system inside ACPICA to implement it as a service of the ACPI sybsystem might work better.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Forgot one detail:
Quoted 4 lines · Henrique de Moraes Holschuh
2. Even if a bay can take different types of devices, the same bay may appear as a number of different EJ0 handlers. In that case, the firmware is to know which handler to use based on which type of device is inside the bay. Thinkpads do this.
But calling EJ0 on any of the devices that might fit the bay will eject whatever is in the bay, regardless of it being the "active device" in the bay or not.
This is why bay can be used in a thinkpad to eject BAT1 (a battery), their EJ0 handlers do exactly the same thing. You don't get the bay request eject and bay ejected events since bay.c doesn't tie itself to the battery ACPI node, but you can eject it.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 77 lines · Henrique de Moraes Holschuh
Quoted 18 lines · Shaohua Li
Quoted 10 lines · Henrique de Moraes Holschuh
Quoted 9 lines · Shaohua Li
Quoted 8 lines · Henrique de Moraes Holschuh
Quoted 2 lines · Shaohua Li
The bay driver is duplicated with libata, I thought we should delete it. See bug http://bugzilla.kernel.org/show_bug.cgi?id=9526
The bay driver is currently useless, BUT it should handle a lot of stuff libata won't, such as bay batteries, bay floppies, and anything else in a bay that is not a hard disk.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
The fact that the driver currently looks only after disks is just a bug. It should, in fact, bind to any ejectable device not already handled by a different driver.
Isn't this the job of acpi dock driver?
No, but I actually fail to see why do we even care about the difference from a dock to a bay. Dock *could* be made to handle both.
Quoted 7 lines
It appears some systems haven't a dock but a bay. In a thinkpad, you could hotplug cdrom/battery/harddisk in the slot of cdrom, but it's not a dock. libata should handle it well (but it doesn't handle
_EJ0currently, I have a patch in above bugzilla).But you are right, acpi dock driver should be extended to support bay. I'll give it a try.
Thanks. Here's what I know about bays (and docks). I expect you know most, if not all of what I'll write, but what the heck, it is good to have these things in the clear :-)
1. Bay and dock notifications are NOT standard. You need to feed them to a notification chain AND generate uevents, and expect a reply before you can actually do something.
There are two big classes of machines here:
1a) those which notify you that the bay/dock is BEING ejected 1b) those which notify you the user wants to eject, and wait for
a command to power down the bay/dock and let the user know she can remove the device.Thinkpads are on the 1b class. See (4) below.
2. Even if a bay can take different types of devices, the same bay may appear as a number of different EJ0 handlers. In that case, the firmware is to know which handler to use based on which type of device is inside the bay. Thinkpads do this.
3. IMO, "eject" really should be a device property, and not something tied to a bay. For docks, this is a lot more difficult to do right, since a dock has multiple devices (and usually at least one BUS, and often more than one. Thinkpads have PCI, PCIe and USB buses through the dock) behind it, so it is probably best to have dock ejects as a property of a "dock" platform device, and bay ejects as a property of whatever device is inside the bay. This should be an userspace AND kernel API.
4. bay ejection notifications can take two forms (see (1) above):
4a) device removal (we already have this)
4b) device removal REQUESTThinkpads should do 4b, then IF AND ONLY IF the user or a kernel platform driver uses (3) to command the ejection, they do 4a.
Some laptops will just do 4a, doing (3) before the user yanks the device requires the user to not be an yahoo (i.e. just like USB pen-drives).
And for laptops that do 4b for docks, one should probably replicate the "request undock" notification for every device inside the dock, and not just for the dock device.
What value you get from an ACPI NOTIFY for 4a or 4b is NOT standard, you will need a platform device OR userspace to interpret it for you.
Quoted 9 lines
5. bay and dock notifications (at least on thinkpads) are issued to the ACPI node of the real device, not to the APCI EJ0 node. This is what is causing issues between bay and libata (both want to register a notify hook to the same node, and ACPICA allows only one driver per node).
I am not really sure the "ACPI driver model" is the right way to tie bay functionality to the system. A hook and notifier system inside ACPICA to implement it as a service of the ACPI sybsystem might work better.
Please also consider the following thread:
http://www.spinics.net/lists/linux-ide/msg22927.html
Regards,
Holger
Quoted 4 lines · Henrique de Moraes Holschuh
1a) those which notify you that the bay/dock is BEING ejected 1b) those which notify you the user wants to eject, and wait for
a command to power down the bay/dock and let the user know she can remove the device.
I believe that docks are always in 1b, though bays may be in either. The only docks I've seen without an eject request aren't ACPI docks at all, so fall outside the scope of this.
Signature · 1 line
Matthew Garrett | [email protected]