[PATCH] bay: Exit if notify handler cannot be installed

[PATCH] bay: Exit if notify handler cannot be installed

[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.

Signed-off-by: Holger Macht <hmacht@suse.de>
---

diff --git a/drivers/acpi/bay.c b/drivers/acpi/bay.c
index d2fc941..ce9038f 100644
--- a/drivers/acpi/bay.c
+++ b/drivers/acpi/bay.c
@@ -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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Shaohua Li]

On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > 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.
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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Di 06. Mai - 17:23:31, Shaohua Li wrote:
> On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> > On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > > 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.
> 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Di 06. Mai - 11:33:16, Holger Macht wrote:
> On Di 06. Mai - 17:23:31, Shaohua Li wrote:
> > On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> > > On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > > > 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.
> > 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

On Tue, 06 May 2008, Shaohua Li wrote:
> 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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Alan Cox]

On Tue, 6 May 2008 12:18:46 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Tue, 06 May 2008, Shaohua Li wrote:
> > 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.

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

On Tue, 06 May 2008, Holger Macht wrote:
> On Di 06. Mai - 11:33:16, Holger Macht wrote:
> > On Di 06. Mai - 17:23:31, Shaohua Li wrote:
> > > On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> > > > On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > > > > 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.
> > > 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.

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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

On Tue, 06 May 2008, Alan Cox wrote:
> On Tue, 6 May 2008 12:18:46 -0300
> Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> > On Tue, 06 May 2008, Shaohua Li wrote:
> > > 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.

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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Alan Cox]

> 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Di 06. Mai - 12:24:11, Henrique de Moraes Holschuh wrote:
> On Tue, 06 May 2008, Holger Macht wrote:
> > On Di 06. Mai - 11:33:16, Holger Macht wrote:
> > > On Di 06. Mai - 17:23:31, Shaohua Li wrote:
> > > > On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> > > > > On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > > > > > 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.
> > > > 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.
> 
> 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.

I fully agree. With "not a huge issue", I rather meant that coding this
two lines of code is pretty easy ;-)

Regards,
	Holger

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Di 06. Mai - 12:18:46, Henrique de Moraes Holschuh wrote:
> On Tue, 06 May 2008, Shaohua Li wrote:
> > 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Di 06. Mai - 12:39:01, Henrique de Moraes Holschuh wrote:
> On Tue, 06 May 2008, Alan Cox wrote:
> > On Tue, 6 May 2008 12:18:46 -0300
> > Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> > > On Tue, 06 May 2008, Shaohua Li wrote:
> > > > 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.
> 
> 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

> 
> 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Shaohua Li]

On Tue, 2008-05-06 at 11:35 +0200, Holger Macht wrote:
> On Di 06. Mai - 11:33:16, Holger Macht wrote:
> > On Di 06. Mai - 17:23:31, Shaohua Li wrote:
> > > On Tue, 2008-05-06 at 17:15 +0800, Holger Macht wrote:
> > > > On Mo 05. Mai - 22:25:08, Holger Macht wrote:
> > > > > 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.
> > > 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.
I have a patch in above bugzilla to handle it.

Thanks,
Shaohua

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Shaohua Li]

On Tue, 2008-05-06 at 12:18 -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 06 May 2008, Shaohua Li wrote:
> > 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?

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

On Wed, 07 May 2008, Shaohua Li wrote:
> On Tue, 2008-05-06 at 12:18 -0300, Henrique de Moraes Holschuh wrote:
> > On Tue, 06 May 2008, Shaohua Li wrote:
> > > 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?

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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Shaohua Li]

On Tue, 2008-05-06 at 22:13 -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 07 May 2008, Shaohua Li wrote:
> > On Tue, 2008-05-06 at 12:18 -0300, Henrique de Moraes Holschuh wrote:
> > > On Tue, 06 May 2008, Shaohua Li wrote:
> > > > 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?
> 
> 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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

On Wed, 07 May 2008, Shaohua Li wrote:
> On Tue, 2008-05-06 at 22:13 -0300, Henrique de Moraes Holschuh wrote:
> > On Wed, 07 May 2008, Shaohua Li wrote:
> > > On Tue, 2008-05-06 at 12:18 -0300, Henrique de Moraes Holschuh wrote:
> > > > On Tue, 06 May 2008, Shaohua Li wrote:
> > > > > 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?
> > 
> > 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.  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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Henrique de Moraes Holschuh]

Forgot one detail:
On Wed, 07 May 2008, Henrique de Moraes Holschuh wrote:
> 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.

-- 
  "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

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Wed 07. May - 09:37:15, Henrique de Moraes Holschuh wrote:
> On Wed, 07 May 2008, Shaohua Li wrote:
> > On Tue, 2008-05-06 at 22:13 -0300, Henrique de Moraes Holschuh wrote:
> > > On Wed, 07 May 2008, Shaohua Li wrote:
> > > > On Tue, 2008-05-06 at 12:18 -0300, Henrique de Moraes Holschuh wrote:
> > > > > On Tue, 06 May 2008, Shaohua Li wrote:
> > > > > > 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?
> > > 
> > > 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.  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.
> 

Please also consider the following thread:

  http://www.spinics.net/lists/linux-ide/msg22927.html

Regards,
	Holger

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Matthew Garrett]

On Wed, May 07, 2008 at 09:37:15AM -0300, Henrique de Moraes Holschuh wrote:

>   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.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH] bay: Exit if notify handler cannot be installed

[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. This would result in an endless loop
consuming 100% of CPU. So clean up and exit.

Signed-off-by: Holger Macht <hmacht@suse.de>
---

diff --git a/drivers/acpi/bay.c b/drivers/acpi/bay.c
index d2fc941..26038c2 100644
--- a/drivers/acpi/bay.c
+++ b/drivers/acpi/bay.c
@@ -301,16 +301,20 @@ static int bay_add(acpi_handle handle, int id)
 	 */
 	pdev->dev.uevent_suppress = 0;
 
-	if (acpi_bay_add_fs(new_bay)) {
-		platform_device_unregister(new_bay->pdev);
-		goto bay_add_err;
-	}
-
 	/* register for events on this device */
 	status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
 			bay_notify, new_bay);
 	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR PREFIX "Error installing bay notify handler\n");
+		printk(KERN_INFO PREFIX "Error installing bay notify handler\n");
+		platform_device_unregister(new_bay->pdev);
+		goto bay_add_err;
+	}
+
+	if (acpi_bay_add_fs(new_bay)) {
+		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+					   bay_notify);
+		platform_device_unregister(new_bay->pdev);
+		goto bay_add_err;
 	}
 
 	/* if we are on a dock station, we should register for dock

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Matthew Garrett]

On Wed, May 21, 2008 at 12:45:47PM +0200, Holger Macht wrote:
> 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. This would result in an endless loop
> consuming 100% of CPU. So clean up and exit.

The bay driver still provides the only mechanism for calling the eject 
methods.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] bay: Exit if notify handler cannot be installed

[Holger Macht]

On Wed 21. May - 11:59:44, Matthew Garrett wrote:
> On Wed, May 21, 2008 at 12:45:47PM +0200, Holger Macht wrote:
> > 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. This would result in an endless loop
> > consuming 100% of CPU. So clean up and exit.
> 
> The bay driver still provides the only mechanism for calling the eject 
> methods.

Yes, I'll send a patch on top of the libata hotplug fixes as soon as it's
in for that. But this patch has nothing to do with this. Without it, you
get an unusable system.

Regards,
	Holger