[v2,0/7] Add camera access keys, IdeaPad driver improvements

  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
  

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