Hi,
On 10/21/20 12:29 AM, Samuel Čavoj wrote:
> Hi,
>
> naturally I notice this right after I send the patch, but my whitespace
> is wrong. Time to set a pre-commit hook up. I guess that means a v4,
> unless you would fix it on your end? It's just a single line.
>
> Sorry about all the noise,
> Sam
>
> On 21.10.2020 00:09, Samuel Čavoj wrote:
>> @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>> }
>> }
>>
>> + if (asus->driver->quirks->use_lid_flip_devid) {
>> + result = asus_wmi_get_devstate_simple(asus,
>> ASUS_WMI_DEVID_LID_FLIP);
>> + if (result < 0)
>
> Right ^here.
No worries I will fix this when merging it.
>
>> + asus->driver->quirks->use_lid_flip_devid = 0;
>> + if (result >= 0) {
>> + input_set_capability(asus->inputdev, EV_SW,
>> SW_TABLET_MODE);
>> + input_report_switch(asus->inputdev, SW_TABLET_MODE,
>> result);
>> + } else if (result == -ENODEV) {
>> + pr_err("This device has lid_flip quirk but got ENODEV
>> checking it. This is a bug.");
>> + } else {
>> + pr_err("Error checking for lid-flip: %d\n", result);
>> + }
>> + }
>
>> + if (asus->driver->quirks->use_lid_flip_devid) {
>> + result = asus_wmi_get_devstate_simple(asus,
>> ASUS_WMI_DEVID_LID_FLIP);
>> + if (result < 0)
>> + asus->driver->quirks->use_lid_flip_devid = 0;
>
> I had the idea of doing this, not sure if it is actually a good idea
> though. The idea is that it would prevent querying the device later
> during runtime, if the first get_devstate fails. Also I assume doing a
> input_report_switch without a corresponding input_set_capability is not
> great either, this would prevent that, if for some reason the
> get_devstate fails in the beginning but later fixes itself.
>
> However, I can also imagine that writing to the quirks structure is
> frowned upon, please tell if that is the case. Thanks.
The quirk_entry structs are not defined const; and they are already written
to in other places (e.g. the wapf module param handling). So this is fine.
Note FWIW, that calling input_report_switch() with a switch which
has not been declared before through input_set_capability() before
is allowed and is simply treated as a no-op. But just skipping the
handling of the event alltogether is a bit cleaner, so your approach is
fine.
Regards,
Hans
