Hi On 3/15/23 23:39, Armin Wolf wrote: <snip> >> As a site note, i recommend to use the get_maintainer.pl scripts under >> scripts/ to find >> any additional maintainers which should be CC-ed. Since your patch >> series touches the >> ideapad-laptop driver, its maintainer (Ike Panhc >> <ike.pan@xxxxxxxxxxxxx>) should also be >> notified about this patch series. >> > I forgot to mention that your patches have to title, please add one for the next revision. Armin, I'm not sure what you mean with this ? For me the patches have a good $subject / first line of the commit message ? Regards, Hans > > Armin Wolf > >>> 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. >>> >> You are still the author of the second patch. Also you should not send >> a patch series as >> a reply to any previous emails. >> >>>> 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? >> >> I meant that your driver should, after (!) setting up the input device >> and event handling, call >> sparse_keymap_report_event() with the code obtained from >> wmidev_evaluate_method() so that userspace knows about the initial state >> of the input device. You might also CC linux-input@xxxxxxxxxxxxxxx for >> the next patch series, so that the input driver maintainers can also >> review your patch series. >> >>>> 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. >> >> Drivers can be instantiated multiple times, and each time their probe >> callback is invoked, >> and many older WMI drivers cannot do this, so the allowlist exists. >> The section "State Container" in >> Documentation/driver-api/driver-model/design-patterns.rst >> explains how to write drivers which can be instantiated multiple times. >> >> If your driver is not a singleton, i.e. it can safely be instantiated >> multiple times, then >> you can add its WMI GUID to the allowlist. >> >
