Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
> 
>             Method (_LID, 0, NotSerialized)  // _LID: Lid Status
>             {
>                 If ((STAS & One))
>                 {
>                     Local0 = One
>                     PBCG |= 0x05000000
>                     HMCG |= 0x05000000
>                 }
>                 Else
>                 {
>                     Local0 = Zero
>                     PBCG &= 0xF0FFFFFF
>                     HMCG &= 0xF0FFFFFF
>                 }
> 
>                 ^^PCI0.GFX0.CLID = Local0
>                 Return (Local0)
>             }
> 
> The problem here is the accesses to the PBCG and HMCG, these are the
> pinconf0 registers for the power, resp. the home button GPIO,
> e.g. PBCG is declared as:
> 
>             OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
>             Field (PWBT, DWordAcc, NoLock, Preserve)
>             {
>                 PBCG,   32,
>                 PBV1,   32,
>                 PBSA,   32,
>                 PBV2,   32
>             }
> 
> Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
> the offset for the pin used for the powerbutton.
> 
> The problem here is this line in _LID:
>                     PBCG |= 0x05000000
> 
> This changes the trigger flags of the GPIO, changing when it generates
> interrupts. Note it does not clear the original flags. Linux uses an
> edge triggered interrupt on both positive and negative edges. This |=
> adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
> interrupt which fires both when low and high, iow it simply always
> fires leading to an interrupt storm, the tablet immediately waking up
> from suspend again, etc.
> 
> There is nothing we can do to fix this, except for a DSDT override,
> which the user needs to do manually. The only thing we can do is
> never call _LID, which requires disabling the lid-switch functionality
> altogether.
> 
> This commit adds a quirk for this, as no lid-switch function is better
> then the interrupt storm. A user manually applying a DSDT override can
> also override the quirk on the kernel cmdline.
> 

Fair enough, thanks for the quirk.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  drivers/acpi/button.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index d27b01c0323d..b758b45737f5 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -77,6 +77,19 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids);
>  
>  /* Please keep this list sorted alphabetically by vendor and model */
>  static const struct dmi_system_id dmi_lid_quirks[] = {
> +	{
> +		/*
> +		 * Acer Switch 10 SW5-012. _LID method messes with home and
> +		 * power button GPIO IRQ settings causing an interrupt storm on
> +		 * both GPIOs. This is unfixable without a DSDT override, so we
> +		 * have to disable the lid-switch functionality altogether :|
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
> +		},
> +		.driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED,
> +	},
>  	{
>  		/*
>  		 * Asus T200TA, _LID keeps reporting closed after every second
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko