As there is nothing private, add linux-acpi in the discussion. :) On Fri, 2008-03-07 at 19:25 +0800, Ângelo Miguel Arrifano wrote: > On Fri, 7 Mar 2008 14:27:13 +0800 > "Zhang, Rui" <rui.zhang@xxxxxxxxx> wrote: > > > Hi, Angelo, > > > > For a typical ACPI device driver, it usually works like this: > > 1. Register the ACPI device driver is the module_init > > 2. Enable the device, create the sys I/F in the drivers .add method. > > 3. Remove the device I/F in .remove method. > > 4. unregister the ACPI device driver in module_exit. > > > > With your patch applied, the sys I/F will be created even if no > PNP0C32 device exists in the ACPI namespace. IMO, moving this piece of > code to the .add method will be much better. > > That's right, but the sys I/F being created are not in a per-device > basis. I mean: > > Suppose PNP0C32 has three devices: > ABTN > BBTN > CBTN You mean three devces with _HID(PNP0C32), or three devices enumerated under a PNP0C32 device? could you please attach the acpidump output? > The sys files are created for PNP0C32 and not for every [ABC]BTN > device. > So, moving the > sys I/F code into .add, AFAIK, will get the sys files created several > times which is not > the purpose. .add method is called for each PNP0C32 device,not for each button. so in this case, I think it will only create two sys files... > > The sys files created are "buttons" and "pressed_button". > "buttons" displays all devices (buttons) under PNP0C32 for, IMHO, > easier enumeration by > userspace; instead of having a file for each device. > "pressed_buttons" displays the button used to turn on/resume the > computer. > > But you are still right, the driver loads successfully (although > displaying "none" on "buttons" sys file) when no PNP0C32 is present as > we can see on this user post which doesn't have PNP0C32 at all: > http://sourceforge.net/forum/forum.php?thread_id=1959640&forum_id=754264 > > On the code below, shouldn't (status < 0) be true when no PNP0C32 is > present? > > /* ACPI driver register */ > status = acpi_bus_register_driver(&quickstart_acpi_driver); > if (status < 0) > return -ENODEV; > > This was supposed to exit with -ENODEV before creating sys I/F. No, acpi_bus_register_driver returns 0 if no matched device is found, in order to support device hotplug. thanks, rui -- 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