Hi Bjørn,
Sorry for my delay because I stuck on other issues.
On Mon, Aug 08, 2016 at 12:09:55PM +0200, Bjørn Mork wrote:
> joeyli <jlee@xxxxxxxx> writes:
>
[...snip]
>
> Sorry for the delay. Just back from vacation. I took a quick peek at
> the driver and I don't think you need that dump. The DSDT is completely
> irrelevant. The driver will match and load on that generic WMI UUID,
> and this code ensures that it successfully probes regardless of DSDT:
>
>
> static acpi_status __init acer_wmi_get_handle_cb(acpi_handle ah, u32 level,
> void *ctx, void **retval)
> {
> *(acpi_handle *)retval = ah;
> return AE_OK;
> }
>
> static int __init acer_wmi_get_handle(const char *name, const char *prop,
> acpi_handle *ah)
> {
> acpi_status status;
> acpi_handle handle;
>
> BUG_ON(!name || !ah);
>
> handle = NULL;
> status = acpi_get_devices(prop, acer_wmi_get_handle_cb,
> (void *)name, &handle);
>
> if (ACPI_SUCCESS(status)) {
> *ah = handle;
> return 0;
> } else {
> return -ENODEV;
> }
> }
>
>
> Since acer_wmi_get_handle_cb() doesn't actually test anything, this will
> just return an arbitrary handle regardless of the value of *prop.
>
>
OK! Understood!
> > Then I want to add HID to the norfkill_ids list in acer-wmi to avoid that
> > it loaded on your Lenovo machine like LEN0068.
>
> I see that this has been done before when this bug has been reported.
> Sorry, but I believe that solution is plain wrong. You are papering
> over a more fundamental problem.
>
> Solve the *real* problems in the driver instead:
> 1) "67C3371D-95A3-4C37-BB61-DD47B491DAAB"
> >>
> >> as can be seen:
> >>
> >> bjorn@miraculix:~$ ls -l /sys/devices/virtual/wmi/67C3371D-95A3-4C37-BB61-DD47B491DAAB
> >> total 0
> >> -r--r--r-- 1 root root 4096 Jul 14 23:10 modalias
> >> drwxr-xr-x 2 root root 0 Jul 14 23:10 power
> >> lrwxrwxrwx 1 root root 0 Jul 14 23:10 subsystem -> ../../../../class/wmi
> >> -rw-r--r-- 1 root root 4096 Jul 14 23:10 uevent
> >>
> >>
> >> But I sort of doubt there is an Acer specific device here. Maybe this
> >> is a generic accelerometer interface? Or is there something else going
> >> on?
> >>
> >> The input device created by the driver just just returns -EPERM when I
> >> try to open it, so I don't think this actually works as-is:
> >>
> >> root@miraculix:/tmp# cat /dev/input/event8
> >> cat: /dev/input/event8: Operation not permitted
> >>
> >>
> >> And it does seem a little weird that a Thinkpad should need an Acer
> >> specific platform driver anyway.... thinkpad_acpi is of course handling
> >> all(?) the Thinkpad specifics as expected.
> >>
> >>
> >> Bjørn
> >
> > Could you please check that what does the _HID of HKEY device in your DSDT?
> >
> > or please attached your acpidump data to me:
> > # acpidump > acpidump.raw
>
> Sorry for the delay. Just back from vacation. I took a quick peek at
> the driver and I don't think you need that dump. The DSDT is completely
> irrelevant. The driver will match and load on that generic WMI UUID,
> and this code ensures that it successfully probes regardless of DSDT:
>
>
> static acpi_status __init acer_wmi_get_handle_cb(acpi_handle ah, u32 level,
> void *ctx, void **retval)
> {
> *(acpi_handle *)retval = ah;
> return AE_OK;
> }
>
> static int __init acer_wmi_get_handle(const char *name, const char *prop,
> acpi_handle *ah)
> {
> acpi_status status;
> acpi_handle handle;
>
> BUG_ON(!name || !ah);
>
> handle = NULL;
> status = acpi_get_devices(prop, acer_wmi_get_handle_cb,
> (void *)name, &handle);
>
> if (ACPI_SUCCESS(status)) {
> *ah = handle;
> return 0;
> } else {
> return -ENODEV;
> }
> }
>
>
> Since acer_wmi_get_handle_cb() doesn't actually test anything, this will
> just return an arbitrary handle regardless of the value of *prop.
>
>
> > Then I want to add HID to the norfkill_ids list in acer-wmi to avoid that
> > it loaded on your Lenovo machine like LEN0068.
>
> I see that this has been done before when this bug has been reported.
> Sorry, but I believe that solution is plain wrong. You are papering
> over a more fundamental problem.
>
> Solve the *real* problems in the driver instead:
> 1) "67C3371D-95A3-4C37-BB61-DD47B491DAAB" is not an Acer specific GUID
> and should probably never have been used to match Acer specific
> devices. The fact that it happens to work for some aribitrary laptop is
> not sufficient.
>
> 2) the "get_handle" code shown above is broken. If it works for some
> specific DSDT, then that is just pure luck.
>
> These things should be fixed ASAP, and backported to stable. The driver
> as it is will probably both load and probe successfully on a large
> number of systems it has no business messing with.
>
>
> > Then, thinkpad_acpi needs to support this _HID.
>
> No. Looking further at this, I don't think so. There is no indication
> that there is any relation between that WMI ID and any of the hardware
> functions supported by the acer-wmi driver. So there is no reason to
> believe that any of the matched Lenovo laptops have the same hardware
> functions.
>
>
>
> Bjørn
>
I have a question about removed 67C3371D-95A3-4C37-BB61-DD47B491DAAB. It
already in acer-wmi a long time since Carlos contributed this driver in
2008.
Does it possible cause regression on some laptops if we removed this GUID
support in acer-wmi?
Thanks a lot!
Joey Lee
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
