On Saturday, June 29, 2013 09:12:59 AM Yinghai Lu wrote: > On Sat, Jun 29, 2013 at 4:16 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > On Friday, June 28, 2013 04:34:21 PM Yinghai Lu wrote: > >> On Fri, Jun 28, 2013 at 3:53 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > >> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > >> > > >> > The ACPI dock driver uses register_acpi_bus_notifier() which > >> > installs a notifier triggered globally for all system notifications. > >> > That first of all is inefficient, because the dock driver is only > >> > interested in notifications associated with the devices it handles, > >> > but it has to handle all system notifies for all devices. Moreover, > >> > it does that even if no docking stations are present in the system > >> > (CONFIG_ACPI_DOCK set is sufficient for that to happen). Besides, > >> > that is inconvenient, because it requires the driver to do extra work > >> > for each notification to find the target dock station object. > >> > > >> > For these reasons, rework the dock driver to install a notify > >> > handler individually for each dock station in the system using > >> > acpi_install_notify_handler(). This allows the dock station > >> > object to be passed directly to the notify handler and makes it > >> > possible to simplify the dock driver quite a bit. It also > >> > reduces the overhead related to the handling of all system > >> > notifies when CONFIG_ACPI_DOCK is set. > >> > >> original change to use register_acpi_bus_notifier, have two assumption > >> 1. two dock_station will have same handle. > > > > Well, that would mean that dock_add() might be called twice for the same handle > > and I don't see how that's possible. > > > > Moreover, even if that were possible, the loop in acpi_dock_notifier_call() > > would break after finding the *first* matching handle anyway, so > > acpi_dock_deferred_cb() wouldn't be called for the second dock station with > > the same handle, if there were two. > > related commit: > commit 6bd00a61ab63d4ceb635ae0316353c11c900b8d8 > Author: Shaohua Li <shaohua.li@xxxxxxxxx> > Date: Thu Aug 28 10:04:29 2008 +0800 > > ACPI: introduce notifier change to avoid duplicates > > The battery driver already registers notification handler. > To avoid registering notification handler again, > introduce a notifier chain in global system notifier handler > and use it in dock driver. > > so it is not two dock station have same handle. it is battery acpi driver. > > but notitifer installing is changed > > commit d94066910943837558d2a461c6766da981260bf0 > Author: Bjorn Helgaas <bjorn.helgaas@xxxxxx> > Date: Thu Apr 30 09:35:47 2009 -0600 > > ACPI: battery: use .notify method instead of installing handler directly > > This patch adds a .notify() method. The presence of .notify() causes > Linux/ACPI to manage event handlers and notify handlers on our behalf, > so we don't have to install and remove them ourselves. > > This driver apparently relies on seeing ALL notify events, not just > device-specific ones (because it used ACPI_ALL_NOTIFY). We use the > ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events. > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@xxxxxx> > CC: Alexey Starikovskiy <alexey.y.starikovskiy@xxxxxxxxxxxxxxx> > Signed-off-by: Len Brown <len.brown@xxxxxxxxx> > > > > > >> 2. acpi subsystem: non root acpi device only can have one system > >> notifier installed. > > > > No, that limitation is long gone. We removed it when we were working on ACPI > > wakeup support for runtime PM. > > looks like i misread that... > > * NOTES: The Root namespace object may have only one handler for each > * type of notify (System/Device). Device/Thermal/Processor objects > * may have one device notify handler, and multiple system notify > * handlers. > > device could have multiple system notify handlers. > > So > > Acked-by: Yinghai Lu <yinghai@xxxxxxxxxx> Thanks (and for the other ACKs too)! -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
