Re: [PATCH 2/2] platform/x86: Add driver for Yoga Tablet Mode switch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 10, 2023 at 2:28 AM Armin Wolf <W_Armin@xxxxxx> wrote:
>
> Hi,
>
> according to the embedded MOF data, the method id for retrieving the mode data
> is 0x1, not 0xAB.

Thanks! I can see from your earlier email in the other thread that
this is right. Strangely, when I tested 0xAB had almost exactly the
same behavior as 0x01. I think you're right though, I have updated my
local copy to 0x01.

I have also fixed the missing cleanup calls and

> Using wmi_evaluate_method() is deprecated as WMI GUIDs are not guaranteed to be
> unique for a given machine. I think both WMI GUIDs should be handled by separate
> drivers, and the driver for the event GUID uses a notifier call chain to inform
> the driver for the data GUID that the usage mode changed, which then handles the
> input device stuff.
>
> This way the driver does not rely on deprecated WMI APIs, and everything would
> still work should Lenovo decide to duplicate some WMI GUIDs.

After reading many times, I believe I understand this and can figure
out the implementation. How should I attribute the commits? Should
this be a 3rd patch in a v2 patch series with myself as the author? Or
should these drivers be introduced in one commit without the
intermediate single driver that uses the deprecated API?

Also have I correctly set Gergo as the commit author for this PATCH
2/2 email like Hans said to? I wasn't sure if I had it right and I
could fix it in the v2 series.

> acpi_dev_put() is missing.

Thanks! Not sure why I thought it was okay to delete lenovo_ymc_remove
but I have added that back in with the input_unregister_device call.

> Maybe it would be beneficial to allow userspace to get the current usage mode without having
> to wait for an WMI event. This way, userspace could still react to situations like the device
> already being in tablet mode when this driver is loaded.

I'm not following how to implement this, not familiar enough with WMI.
Could you explain more?

> If the drivers handling the event and data GUIDs are fine with being instantiated multiple
> times, then adding the WMI GUIDs to the allow_duplicates[] list in drivers/platform/x86/wmi.c
> will allow the WMI driver core to handle duplicated event and data GUIDs.

Is there an easy way to test if it's fine to run multiple copies?
Currently testing by compiling the module with an inlined
ideapad-laptop.h out of the kernel tree and using the insmod command
to load it.




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux