Re: [PATCH 1/3] ACPI / dock: Rework the handling of notifications

[Yinghai Lu]

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>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/