On 28. 09. 22, 8:20, Jiri Slaby wrote:
> Hi,
> 
> On 12. 07. 22, 4:00, Chuanhong Guo wrote:
>> IRQ override isn't needed on modern AMD Zen systems.
>> There's an active low keyboard IRQ on AMD Ryzen 6000 and it will stay
>> this way on newer platforms. This IRQ override breaks keyboards for
>> almost all Ryzen 6000 laptops currently on the market.
>>
>> Skip this IRQ override for all AMD Zen platforms because this IRQ
>> override is supposed to be a workaround for buggy ACPI DSDT and we can't
>> have a long list of all future AMD CPUs/Laptops in the kernel code.
>> If a device with buggy ACPI DSDT shows up, a separated list containing
>> just them should be created.
> 
> This breaks pads on IdeaPad 5 Flex:
> https://bugzilla.suse.com/show_bug.cgi?id=1203794
> 
>  > [    1.058135] hid-generic 0020:1022:0001.0001: hidraw0: SENSOR HUB 
> HID v0.00 Device [hid-amdsfh 1022:0001] on pcie_mp2_amd
>  > [    2.038937] i2c_designware AMDI0010:00: controller timed out
>  > [    2.146627] i2c_designware AMDI0010:03: controller timed out
>  > [    6.166859] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
>  > [    8.279604] i2c_designware AMDI0010:03: controller timed out
>  > [   12.310897] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
>  > [   14.429372] i2c_designware AMDI0010:03: controller timed out
>  > [   18.462629] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
>  > [   20.579183] i2c_designware AMDI0010:03: controller timed out
>  > [   24.598703] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
>  > [   25.629071] i2c_hid_acpi i2c-MSFT0001:00: can't add hid device: -61
>  > [   25.629430] i2c_hid_acpi: probe of i2c-MSFT0001:00 failed with 
> error -61
> 
> The diff of good and bad dmesgs:
> -ACPI: IRQ 10 override to edge, high
> -ACPI: IRQ 6 override to edge, high
> 
> The diff of /proc/interrupts:
>       6: ...  IR-IO-APIC    [-6-fasteoi-]    {+6-edge+}      AMDI0010:03
>      10: ...  IR-IO-APIC   [-10-fasteoi-]   {+10-edge+}      AMDI0010:00
> 
> And:
>    i2c_designware: /devices/platform/AMDI0010:00
>    i2c_designware: /devices/platform/AMDI0010:03
> 
> 
> So the if needs to be fine-tuned, apparently. Maybe introduce some list 
> as suggested in the commit log. Based on the below?

Something like the attached. It's:
1) untested yet
2) contains more debug messaging
3) contains both cases for ACPI_ACTIVE_* as I don't know the original 
polarity

I don't know how widely this is spread -- maybe it would be worth a 
commandline parameter so that people can work around this until this is 
fixed by a DMI entry permanently?

> DMI says:
>    System Info: #14
>      Manufacturer: "LENOVO"
>      Product: "82RA"
>      Version: "IdeaPad Flex 5 16ALC7"
>      Serial: "PW02359K"
>      UUID: 6b2d54d9-cd80-11ec-83eb-e00af665fbac
>      Wake-up: 0x06 (Power Switch)
>    Board Info: #15
>      Manufacturer: "LENOVO"
>      Product: "LNVNB161216"
>      Version: "SDK0T76463 WIN"
>      Serial: "PW02359K"
>      Asset Tag: "No Asset Tag"
>      Type: 0x0a (Motherboard)
>      Features: 0x09
>        Hosting Board
>        Replaceable
>      Location: "Chassis Location Unknown"
>    Chassis Info: #16
>      Manufacturer: "LENOVO"
>      Version: "IdeaPad Flex 5 16ALC7"
>      Serial: "PW02359K"
>      Asset Tag: "No Asset Tag"
>      Type: 0x1f (Other)
>      Bootup State: 0x03 (Safe)
>      Power Supply State: 0x03 (Safe)
>      Thermal State: 0x01 (Other)
>      Security Status: 0x01 (Other)
> 
>> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
>> ---
>> Change sice v5: reworked
>>
>>   drivers/acpi/resource.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index c2d494784425..510cdec375c4 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -416,6 +416,16 @@ static bool acpi_dev_irq_override(u32 gsi, u8 
>> triggering, u8 polarity,
>>   {
>>       int i;
>> +#ifdef CONFIG_X86
>> +    /*
>> +     * IRQ override isn't needed on modern AMD Zen systems and
>> +     * this override breaks active low IRQs on AMD Ryzen 6000 and
>> +     * newer systems. Skip it.
>> +     */
>> +    if (boot_cpu_has(X86_FEATURE_ZEN))
>> +        return false;
>> +#endif
>> +
>>       for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
>>           const struct irq_override_cmp *entry = &skip_override_table[i];
> 
> thanks,

-- 
js