| Message ID | 20221029120311.11152-1-erayorcunus@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add camera access keys, IdeaPad driver improvements | expand |
On 10/29/22 20:03, Eray Orçunus wrote: > Nowadays many laptops have camera access keys, yet there is no usage codes > mapped to them, even though it's introduced in HUTRR72. Start point of > this patch series was adding it and making IdeaPads send it to userspace. > But later I discovered that camera_power attribute of ideapad-laptop > driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with > that. I managed to find a way to check whether an IdeaPad supports > camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose > it to sysfs so userspace will know that it can't toggle camera access via > camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE. > > Along the way I discovered that old IdeaPads, like S10-3, may not be able > to toggle their touchpad as a regression of a commit aimed for newer > IdeaPads, so I reverted it. > > Also I noticed that I can get/set the state of my keyboard light, > so one of the patches also adds supports for this kind of keyboard lights, > which I call "partially supported keyboard lights". I expect that commit > to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more. > Currently only tested on 520-15IKB. Thanks. Also test on my ideapad s410 and it looks good. Acked-by: Ike Panhc <ike.pan@canonical.com>
On 11/08/22 06:56, Ike Panhc wrote: > On 10/29/22 20:03, Eray Orçunus wrote: > > Nowadays many laptops have camera access keys, yet there is no usage codes > > mapped to them, even though it's introduced in HUTRR72. Start point of > > this patch series was adding it and making IdeaPads send it to userspace. > > But later I discovered that camera_power attribute of ideapad-laptop > > driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with > > that. I managed to find a way to check whether an IdeaPad supports > > camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose > > it to sysfs so userspace will know that it can't toggle camera access via > > camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE. > > > > Along the way I discovered that old IdeaPads, like S10-3, may not be able > > to toggle their touchpad as a regression of a commit aimed for newer > > IdeaPads, so I reverted it. > > > > Also I noticed that I can get/set the state of my keyboard light, > > so one of the patches also adds supports for this kind of keyboard lights, > > which I call "partially supported keyboard lights". I expect that commit > > to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more. > > Currently only tested on 520-15IKB. > > Thanks. Also test on my ideapad s410 and it looks good. > > Acked-by: Ike Panhc <ike.pan@canonical.com> Thank you :) I need some advice since I'm new in here, sadly another patch has been merged to ideapad-laptop along the way and currently it's not possible to merge patch #7, does that mean I should send v3 of my patch series? And whom should I wait for merge, x86 platform drivers maintainers? I think that is the only subsystem whose maintainers haven't replied yet. -eray
Hi Eray, Sorry for the long silence, I have not done any pdx86 patch review the last 2 weeks due to personal circumstances. On 11/9/22 13:58, Eray Orçunus wrote: > On 11/08/22 06:56, Ike Panhc wrote: >> On 10/29/22 20:03, Eray Orçunus wrote: >>> Nowadays many laptops have camera access keys, yet there is no usage codes >>> mapped to them, even though it's introduced in HUTRR72. Start point of >>> this patch series was adding it and making IdeaPads send it to userspace. >>> But later I discovered that camera_power attribute of ideapad-laptop >>> driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with >>> that. I managed to find a way to check whether an IdeaPad supports >>> camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose >>> it to sysfs so userspace will know that it can't toggle camera access via >>> camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE. >>> >>> Along the way I discovered that old IdeaPads, like S10-3, may not be able >>> to toggle their touchpad as a regression of a commit aimed for newer >>> IdeaPads, so I reverted it. >>> >>> Also I noticed that I can get/set the state of my keyboard light, >>> so one of the patches also adds supports for this kind of keyboard lights, >>> which I call "partially supported keyboard lights". I expect that commit >>> to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more. >>> Currently only tested on 520-15IKB. >> >> Thanks. Also test on my ideapad s410 and it looks good. >> >> Acked-by: Ike Panhc <ike.pan@canonical.com> > > > Thank you :) > > I need some advice since I'm new in here, sadly another patch has been > merged to ideapad-laptop along the way and currently it's not possible to > merge patch #7, does that mean I should send v3 of my patch series? No that is not necessary, I can rework it to apply on top of the other patch. But TBH I think we really need to work on a different solution for the problem with the touchpad issues with ideapad-laptop we cannot just keep adding touchpad and/or DMI ids because the driver is breaking touchpad functionality left and right. I will send out an email after this one to all authors of recent patches which all do "priv->features.touchpad_ctrl_via_ec = 0" in some way. With a request to gather some more info of why exactly this is necessary and to see if we cannot come up with a more generic fix. > And whom should I wait for merge, x86 platform drivers maintainers? I'm the x86 platform drivers maintainer. I believe it makes sense to merge this series through the x86 platform drivers git tree. I need to coordinate the merging of patch 2/7 with wDmitry (the input subsystem maintainer) I'll send him an email about this. After that I can likely merge patches 2-6. For the touchpad patches I would first like to get a better handle on how to fix things more generic. Specifically patch 1/7 will cause priv->features.touchpad_ctrl_via_ec to get set to 1 on more models and since that is causing issues I don't think that is a good idea (even though the patch does make sense) and for 7/7 I hope to come up with something more generic. If you can run the tests from the touchpad mail soon that would really help! Note I do plan to send 7/7 out as a fix for 6.1 if we run out of time wrt coming up with a recent fix. Getting at least some fix out the door is also why I already merged the other patch using the DMI ids. > I think that is the only subsystem whose maintainers haven't replied yet. Correct, but I have replied now :) Regards, Hans
Hi! On 11/9/22 19:38, Hans de Goede wrote: > Hi Eray, > > Sorry for the long silence, I have not done any pdx86 patch review > the last 2 weeks due to personal circumstances. Oh, I wasn't even aware I had to wait for pdx86 review, and Ike Panhc just sent his Acked-By anyway, no problem at all. > On 11/9/22 13:58, Eray Orçunus wrote: > > On 11/08/22 06:56, Ike Panhc wrote: > >> > >> Thanks. Also test on my ideapad s410 and it looks good. > >> > >> Acked-by: Ike Panhc <ike.pan@canonical.com> > > > > > > Thank you :) > > > > I need some advice since I'm new in here, sadly another patch has been > > merged to ideapad-laptop along the way and currently it's not possible to > > merge patch #7, does that mean I should send v3 of my patch series? > > No that is not necessary, I can rework it to apply on top of the other > patch. Oh, that's great, thank you. > For the touchpad patches I would first like to get > a better handle on how to fix things more generic. > > Specifically patch 1/7 will cause priv->features.touchpad_ctrl_via_ec > to get set to 1 on more models and since that is causing issues > I don't think that is a good idea (even though the patch does > make sense) and for 7/7 I hope to come up with something > more generic. > > If you can run the tests from the touchpad mail soon that > would really help! That sounds great! I will try to help as much as I can. And yeah, I couldn't guess patch 1 can cause a regression on some IdeaPads. > > I think that is the only subsystem whose maintainers haven't replied yet. > > Correct, but I have replied now :) Hehe, this reply was very informative, thank you :) Best, Eray
Nowadays many laptops have camera access keys, yet there is no usage codes mapped to them, even though it's introduced in HUTRR72. Start point of this patch series was adding it and making IdeaPads send it to userspace. But later I discovered that camera_power attribute of ideapad-laptop driver on my IdeaPad 520-15IKB doesn't work, so I can't toggle it with that. I managed to find a way to check whether an IdeaPad supports camera_power attribute (which sends VPCCMD_W_CAMERA to EC), don't expose it to sysfs so userspace will know that it can't toggle camera access via camera_power, in my case, after receiving KEY_CAMERA_ACCESS_TOGGLE. Along the way I discovered that old IdeaPads, like S10-3, may not be able to toggle their touchpad as a regression of a commit aimed for newer IdeaPads, so I reverted it. Also I noticed that I can get/set the state of my keyboard light, so one of the patches also adds supports for this kind of keyboard lights, which I call "partially supported keyboard lights". I expect that commit to add keyboard light support for 520-15IKB, 330-17ICH, 5 (15) and more. Currently only tested on 520-15IKB. --- Changes in v2: - Added Dmitry Torokhov's Acked-By to patch 2 - Applied Barnabás Pőcze's recommendations to patch 5: - strncmp -> strstarts - static global "CAM" string -> inlined "CAM" string - move new variables to the scope they're used, and order them - Added patch 7, which removes "touchpad" attr for SYNA2B33 Eray Orçunus (7): Revert "platform/x86: ideapad-laptop: check for touchpad support in _CFG" HID: add mapping for camera access keys platform/x86: ideapad-laptop: Report KEY_CAMERA_ACCESS_TOGGLE instead of KEY_CAMERA platform/x86: ideapad-laptop: Add new _CFG bit numbers for future use platform/x86: ideapad-laptop: Expose camera_power only if supported platform/x86: ideapad-laptop: Keyboard backlight support for more IdeaPads platform/x86: ideapad-laptop: Don't expose touchpad attr on IdeaPads with SYNA2B33 drivers/hid/hid-debug.c | 3 + drivers/hid/hid-input.c | 3 + drivers/platform/x86/ideapad-laptop.c | 170 ++++++++++++++++++++++--- include/uapi/linux/input-event-codes.h | 3 + 4 files changed, 162 insertions(+), 17 deletions(-) base-commit: d9db04c1dec6189413701c52b9498a7a56c96445