Bug 219614 - IRQ1 override needed to let keyboard work on laptop TongFang gm5ida/GM5HG0A
Summary: IRQ1 override needed to let keyboard work on laptop TongFang gm5ida/GM5HG0A
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: Input Devices (show other bugs)
Hardware: AMD Linux
: P3 normal
Assignee: drivers_input-devices
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-12-20 02:41 UTC by Stephan
Modified: 2024-12-30 17:49 UTC (History)
1 user (show)

See Also:
Kernel Version:
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Patch for GM5HG0A keyboard (1.01 KB, text/plain)
2024-12-20 02:41 UTC, Stephan
Details
DMI values relevant for this laptop (1.71 KB, text/plain)
2024-12-20 02:43 UTC, Stephan
Details
acpidump for laptop TongFang gm5ida(GM5HG0A) (1.45 MB, text/plain)
2024-12-20 02:58 UTC, Stephan
Details
dmesg from before the patch (so without IRQ1 override high) (123.33 KB, text/plain)
2024-12-24 03:57 UTC, Stephan
Details

Description Stephan 2024-12-20 02:41:57 UTC
Created attachment 307374 [details]
Patch for GM5HG0A keyboard

The keyboard from a AMD Ryzen7(8845HS) laptop bought from webshop www.skikk.eu
model "SKIKK Vanaheim 15 RTX 4060 Black"
does not work under Linux kernel 6, however 5.15.0-46 and before worked ok
(since 5.15.0-119 and beyond it does not).

In 6.13.0-rc3 the cause is code in linux-pm/drivers/acpi/resource.c,
function acpi_dev_irq_override which has has a line to return false for AMD Zen CPU's.
when function returns true, I have a working keyboard again.
After this change the dmesg will have "ACPI: IRQ 1 override to edge, high(!)"

This function also has a table irq1_edge_low_force_override, but its entries
does not match this laptop.

However laptop has sticker at back "TongFang gm5ida", so re-branded and vendor has changed DMI values DMI_SYS_VENDOR, DMI_SYS_VENDOR, DMI_SYS_VENDOR and DMI_SYS_VENDOR.


To fix I see different possible solutions:
1. Let the table lists the new vendor "SKIKK.EU" with product "Vanaheim", but this would lead to many entries if all rebranded laptops would be added.

2. See attachment for a working patch based on array DMI_OEM_STRING having "GM5HG0A", which seems a original factory value and could match many more rebranded laptops of this type.
However this would be the first entry using DMI_OEM_STRING search, which might be unwanted (and expensive to search, as the arrray can be 20 entries big?)

3. There is also a possibility of checking DMI_PRODUCT_SERIAL for prefix "GM5HG0A", however I could not find a macro to do that (eg DMI_PREFIX_MATCH).

Or is there a better solution?
Comment 1 Stephan 2024-12-20 02:43:43 UTC
Created attachment 307375 [details]
DMI values relevant for this laptop
Comment 2 Stephan 2024-12-20 02:58:04 UTC
Created attachment 307376 [details]
acpidump for laptop TongFang gm5ida(GM5HG0A)
Comment 3 Stephan 2024-12-24 03:57:18 UTC
Created attachment 307398 [details]
dmesg from before the patch (so without IRQ1 override high)
Comment 4 Hans de Goede 2024-12-27 14:49:17 UTC
Hi Stephan,

Thank you for your bug report and patch and good that you added a note to bug 217394, that is how I got here.

Before I noticed you already attached a patch I came up with this:

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index ab4c0e0b6b8e..008d42260df4 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -653,6 +653,16 @@ static const struct dmi_system_id irq1_edge_low_force_override[] = {
 			DMI_MATCH(DMI_BOARD_NAME, "GMxHGxx"),
 		},
 	},
+	{
+		/*
+		 * TongFang GM5HG0A sold as SKIKK Vanaheim
+		 * https://bugzilla.kernel.org/show_bug.cgi?id=219614
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SKIKK"),
+			DMI_MATCH(DMI_BOARD_NAME, "Vanaheim"),
+		},
+	},
 	{ }
 };
 

This was based on the assumption that DMI*MATCH() was not possible for DMI_OEM_STRING at all, but after noticing your patch and looking at the dmi_matches() code I see that that is not correct.

I have looked at the dmidecode for a few other TongFang devices and the TongFang board-name string being in the OEM strings seems to be something which is consistently true, so I think that your solution using DMI_EXACT_MATCH(DMI_OEM_STRING, "GM5HG0A") is a good solution.

I would leave out the DMI_PRODUCT_SERIAL partial match since prefixing the serial with the board-name seems to be a skikk specific thing.

Can you submit a patch for this upstream following: https://www.kernel.org/doc/html/latest/process/submitting-patches.html ?

Or shall I submit a patch with just the DMI_EXACT_MATCH(DMI_OEM_STRING, "GM5HG0A") match upstream for you ? If I submit the patch upstream, do you want to be credited in the form of:

Reported-by: Stephan <youremail@gmail.com>

? Note this will expose your youremail@gmail.com address to the entire world.
Comment 5 Stephan 2024-12-27 15:12:02 UTC
Hans,

Thank you for looking at this.

Yes I think the DMI_OEM_STRING could match more laptops, but keep in mind it is checking a list of (now) 20 entries.. so more costly.

Please.. you do the patch submit, because for me the submit process looked a bit overwhelming. And I don't know the people and what kind of code they prefer (I was already a bit confused with which comment syntax is allowed, etc)

I don't need credit for it, so you can take credit for it.

I am just glad to see Linux to be able running on more laptops.
Comment 6 Hans de Goede 2024-12-28 16:55:33 UTC
(In reply to Stephan from comment #5)
> Please.. you do the patch submit

Done:

https://lore.kernel.org/linux-acpi/20241228164845.42381-1-hdegoede@redhat.com/

> Yes I think the DMI_OEM_STRING could match more laptops, but keep in mind it
> is checking a list of (now) 20 entries.. so more costly.

About this being costly, this code only runs once at boot so that is not really a problem. But your comment did make me realize that this code calls dmi_check_system() approx. 30 times at boot, while it only really needs to be called once in case if the keyboard IRQ having a specific polarity which needs to be corrected.

So I submitted a second patch to mkae this code only call dmi_check_system() once at boot:

https://lore.kernel.org/linux-acpi/20241228165253.42584-1-hdegoede@redhat.com/
Comment 7 Stephan 2024-12-30 17:49:33 UTC
Yes.. I noticed also that the previous called dmi_check_system for IRQ that did not have any override.

Your second patch skips that and makes the boot slightly faster. ;)

Note You need to log in before you can comment on or make changes to this bug.