Re: [PATCH] HID: amd_sfh: Enable operating mode

On 6/19/25 00:01, Mario Limonciello wrote:

On 6/18/2025 12:00 PM, Eric Naim wrote:

quoted

On 6/17/25 03:46, Limonciello, Mario wrote:

quoted

On 6/14/25 1:51 AM, Eric Naim wrote:

quoted

On 6/13/25 21:46, Mario Limonciello wrote:

quoted

On 6/9/2025 3:21 PM, Mario Limonciello wrote:

quoted

+Denis

On 5/27/2025 4:10 AM, Basavaraj Natikar wrote:

quoted

Add changes to enable operating modes in the driver to allow the FW to
activate and retrieve data from relevant sensors. This enables the FW to
take necessary actions based on the operating modes.

Co-developed-by: Akshata MukundShetty <redacted>
Signed-off-by: Akshata MukundShetty <redacted>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

Comparing this to the series that was submitted by Denis [1] I notice that the main tangible difference is that this isn't exported into the HID descriptor.  So how does userspace know the current operating mode with this patch?

Link: https://lore.kernel.org/linux-input/20250309194934.1759953-2- benato.denis96@gmail.com/ [1]

Recently a bug report came in: https://bugzilla.kernel.org/show_bug.cgi?id=220224

It was very weird because it advertised a tablet mode switch which I believe comes from the patch that I linked being added to a downstream kernel.  So there appears to be problems with the descriptor from that patch.

Considering this, I think what you're doing is fine for now; appears firmware is notified on tablet mode.

Unfortunately this version of the patch is also causing problems for the user above. Attached is their journal log with the kernel traces. See [1] for the GitHub issue.

I looked through the patch and I have a theory on the problem.  Is the
issue that MAX_HID_DEVICES wasn't incremented?

Because previously there was only "6" kinda before and now this is a 7th.

This incremental diff would fix it if so.

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h

b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
index 1c91be8daeddf..7452b03029538 100644

--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h

@@ -11,7 +11,7 @@

   #ifndef AMDSFH_HID_H
   #define AMDSFH_HID_H

-#define MAX_HID_DEVICES                6
+#define MAX_HID_DEVICES                7
   #define AMD_SFH_HID_VENDOR     0x1022
   #define AMD_SFH_HID_PRODUCT    0x0001

Hi Mario,

This patch seems to work on top of patch in the main post here. Any other steps to proceed?

Eric,

That's great news!

I think Akshata just needs to spin a second version of the patch squashing it in.

Thanks,

One more to add to that I suppose, I had another user test this patch that was using Denis's patch, and their device has working tablet mode switching too with this patch.

Akshata, Basavaraj,

feel free to add

Tested-by: Eric Naim <redacted>

if its applicable for the next revision of the patch.

-- 
Regards,
  Eric