Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
@ 2019-08-27 20:28 Hans de Goede
  2019-08-28 11:37 ` Mika Westerberg
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hans de Goede @ 2019-08-27 20:28 UTC (permalink / raw)
  To: Mika Westerberg, Andy Shevchenko, Bartosz Golaszewski, Linus Walleij
  Cc: Hans de Goede, linux-gpio, linux-acpi, stable, Daniel Drake,
	Ian W MORRISON

Another day; another DSDT bug we need to workaround...

Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
at least once on boot") we call _AEI edge handlers at boot.

In some rare cases this causes problems. One example of this is the Minix
Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
and pasted code for dealing with Micro USB-B connector host/device role
switching, while the mini PC does not even have a micro-USB connector.
This code, which should not be there, messes with the DDC data pin from
the HDMI connector (switching it to GPIO mode) breaking HDMI support.

To avoid problems like this, this commit adds a new
gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
allows disabling the running of _AEI edge event handlers at boot.

The default value is -1/auto which uses a DMI based blacklist, the initial
version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.

Cc: stable@vger.kernel.org
Cc: Daniel Drake <drake@endlessm.com>
Cc: Ian W MORRISON <ianwmorrison@gmail.com>
Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Use a module_param instead of __setup
- Do DMI check only once from a postcore_initcall
---
 drivers/gpio/gpiolib-acpi.c | 42 +++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 39f2f9035c11..bda28eb82c3f 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -7,6 +7,7 @@
  *          Mika Westerberg <mika.westerberg@linux.intel.com>
  */
 
+#include <linux/dmi.h>
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
@@ -19,6 +20,11 @@
 
 #include "gpiolib.h"
 
+static int run_edge_events_on_boot = -1;
+module_param(run_edge_events_on_boot, int, 0444);
+MODULE_PARM_DESC(run_edge_events_on_boot,
+		 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
+
 /**
  * struct acpi_gpio_event - ACPI GPIO event handler data
  *
@@ -170,10 +176,13 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
 	event->irq_requested = true;
 
 	/* Make sure we trigger the initial state of edge-triggered IRQs */
-	value = gpiod_get_raw_value_cansleep(event->desc);
-	if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
-	    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
-		event->handler(event->irq, event);
+	if (run_edge_events_on_boot &&
+	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
+		value = gpiod_get_raw_value_cansleep(event->desc);
+		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
+		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
+			event->handler(event->irq, event);
+	}
 }
 
 static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
@@ -1283,3 +1292,28 @@ static int acpi_gpio_handle_deferred_request_irqs(void)
 }
 /* We must use _sync so that this runs after the first deferred_probe run */
 late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
+
+static const struct dmi_system_id run_edge_events_on_boot_blacklist[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
+		}
+	},
+	{} /* Terminating entry */
+};
+
+static int acpi_gpio_setup_params(void)
+{
+	if (run_edge_events_on_boot < 0) {
+		if (dmi_check_system(run_edge_events_on_boot_blacklist))
+			run_edge_events_on_boot = 0;
+		else
+			run_edge_events_on_boot = 1;
+	}
+
+	return 0;
+}
+
+/* Directly after dmi_setup() which runs as core_initcall() */
+postcore_initcall(acpi_gpio_setup_params);
-- 
2.23.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-27 20:28 [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist Hans de Goede
@ 2019-08-28 11:37 ` Mika Westerberg
  2019-08-28 11:37 ` Andy Shevchenko
  2019-09-11  9:48 ` Linus Walleij
  2 siblings, 0 replies; 7+ messages in thread
From: Mika Westerberg @ 2019-08-28 11:37 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Andy Shevchenko, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-acpi, stable, Daniel Drake, Ian W MORRISON

On Tue, Aug 27, 2019 at 10:28:35PM +0200, Hans de Goede wrote:
> Another day; another DSDT bug we need to workaround...
> 
> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
> at least once on boot") we call _AEI edge handlers at boot.
> 
> In some rare cases this causes problems. One example of this is the Minix
> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
> and pasted code for dealing with Micro USB-B connector host/device role
> switching, while the mini PC does not even have a micro-USB connector.
> This code, which should not be there, messes with the DDC data pin from
> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
> 
> To avoid problems like this, this commit adds a new
> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
> allows disabling the running of _AEI edge event handlers at boot.
> 
> The default value is -1/auto which uses a DMI based blacklist, the initial
> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Drake <drake@endlessm.com>
> Cc: Ian W MORRISON <ianwmorrison@gmail.com>
> Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-27 20:28 [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist Hans de Goede
  2019-08-28 11:37 ` Mika Westerberg
@ 2019-08-28 11:37 ` Andy Shevchenko
  2019-08-28 12:20   ` Hans de Goede
  2019-09-11  9:48 ` Linus Walleij
  2 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2019-08-28 11:37 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mika Westerberg, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-acpi, stable, Daniel Drake, Ian W MORRISON

On Tue, Aug 27, 2019 at 10:28:35PM +0200, Hans de Goede wrote:
> Another day; another DSDT bug we need to workaround...
> 
> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
> at least once on boot") we call _AEI edge handlers at boot.
> 
> In some rare cases this causes problems. One example of this is the Minix
> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
> and pasted code for dealing with Micro USB-B connector host/device role
> switching, while the mini PC does not even have a micro-USB connector.
> This code, which should not be there, messes with the DDC data pin from
> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
> 
> To avoid problems like this, this commit adds a new
> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
> allows disabling the running of _AEI edge event handlers at boot.
> 
> The default value is -1/auto which uses a DMI based blacklist, the initial
> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.

Thank you!

Assuming it works for Ian,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Drake <drake@endlessm.com>
> Cc: Ian W MORRISON <ianwmorrison@gmail.com>
> Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Use a module_param instead of __setup
> - Do DMI check only once from a postcore_initcall
> ---
>  drivers/gpio/gpiolib-acpi.c | 42 +++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 39f2f9035c11..bda28eb82c3f 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -7,6 +7,7 @@
>   *          Mika Westerberg <mika.westerberg@linux.intel.com>
>   */
>  
> +#include <linux/dmi.h>
>  #include <linux/errno.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio/driver.h>
> @@ -19,6 +20,11 @@
>  
>  #include "gpiolib.h"
>  
> +static int run_edge_events_on_boot = -1;
> +module_param(run_edge_events_on_boot, int, 0444);
> +MODULE_PARM_DESC(run_edge_events_on_boot,
> +		 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
> +
>  /**
>   * struct acpi_gpio_event - ACPI GPIO event handler data
>   *
> @@ -170,10 +176,13 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
>  	event->irq_requested = true;
>  
>  	/* Make sure we trigger the initial state of edge-triggered IRQs */
> -	value = gpiod_get_raw_value_cansleep(event->desc);
> -	if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
> -	    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
> -		event->handler(event->irq, event);
> +	if (run_edge_events_on_boot &&
> +	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> +		value = gpiod_get_raw_value_cansleep(event->desc);
> +		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
> +		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
> +			event->handler(event->irq, event);
> +	}
>  }
>  
>  static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
> @@ -1283,3 +1292,28 @@ static int acpi_gpio_handle_deferred_request_irqs(void)
>  }
>  /* We must use _sync so that this runs after the first deferred_probe run */
>  late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
> +
> +static const struct dmi_system_id run_edge_events_on_boot_blacklist[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
> +		}
> +	},
> +	{} /* Terminating entry */
> +};
> +
> +static int acpi_gpio_setup_params(void)
> +{
> +	if (run_edge_events_on_boot < 0) {
> +		if (dmi_check_system(run_edge_events_on_boot_blacklist))
> +			run_edge_events_on_boot = 0;
> +		else
> +			run_edge_events_on_boot = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Directly after dmi_setup() which runs as core_initcall() */
> +postcore_initcall(acpi_gpio_setup_params);
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-28 11:37 ` Andy Shevchenko
@ 2019-08-28 12:20   ` Hans de Goede
  2019-08-28 12:21     ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2019-08-28 12:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mika Westerberg, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-acpi, stable, Daniel Drake, Ian W MORRISON

Hi,

On 28-08-19 13:37, Andy Shevchenko wrote:
> On Tue, Aug 27, 2019 at 10:28:35PM +0200, Hans de Goede wrote:
>> Another day; another DSDT bug we need to workaround...
>>
>> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
>> at least once on boot") we call _AEI edge handlers at boot.
>>
>> In some rare cases this causes problems. One example of this is the Minix
>> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
>> and pasted code for dealing with Micro USB-B connector host/device role
>> switching, while the mini PC does not even have a micro-USB connector.
>> This code, which should not be there, messes with the DDC data pin from
>> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
>>
>> To avoid problems like this, this commit adds a new
>> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
>> allows disabling the running of _AEI edge event handlers at boot.
>>
>> The default value is -1/auto which uses a DMI based blacklist, the initial
>> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.
> 
> Thank you!
> 
> Assuming it works for Ian,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Note I have access to a Minix Neo Z83-4 myself now and I did test that
this fixes it and that passing gpiolib_acpi.run_edge_events_on_boot=0
breaks HDMI again (so the option works).

Regards,

Hans


> 
>>
>> Cc: stable@vger.kernel.org
>> Cc: Daniel Drake <drake@endlessm.com>
>> Cc: Ian W MORRISON <ianwmorrison@gmail.com>
>> Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
>> Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
>> Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> - Use a module_param instead of __setup
>> - Do DMI check only once from a postcore_initcall
>> ---
>>   drivers/gpio/gpiolib-acpi.c | 42 +++++++++++++++++++++++++++++++++----
>>   1 file changed, 38 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>> index 39f2f9035c11..bda28eb82c3f 100644
>> --- a/drivers/gpio/gpiolib-acpi.c
>> +++ b/drivers/gpio/gpiolib-acpi.c
>> @@ -7,6 +7,7 @@
>>    *          Mika Westerberg <mika.westerberg@linux.intel.com>
>>    */
>>   
>> +#include <linux/dmi.h>
>>   #include <linux/errno.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/gpio/driver.h>
>> @@ -19,6 +20,11 @@
>>   
>>   #include "gpiolib.h"
>>   
>> +static int run_edge_events_on_boot = -1;
>> +module_param(run_edge_events_on_boot, int, 0444);
>> +MODULE_PARM_DESC(run_edge_events_on_boot,
>> +		 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
>> +
>>   /**
>>    * struct acpi_gpio_event - ACPI GPIO event handler data
>>    *
>> @@ -170,10 +176,13 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
>>   	event->irq_requested = true;
>>   
>>   	/* Make sure we trigger the initial state of edge-triggered IRQs */
>> -	value = gpiod_get_raw_value_cansleep(event->desc);
>> -	if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>> -	    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>> -		event->handler(event->irq, event);
>> +	if (run_edge_events_on_boot &&
>> +	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>> +		value = gpiod_get_raw_value_cansleep(event->desc);
>> +		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>> +		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>> +			event->handler(event->irq, event);
>> +	}
>>   }
>>   
>>   static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
>> @@ -1283,3 +1292,28 @@ static int acpi_gpio_handle_deferred_request_irqs(void)
>>   }
>>   /* We must use _sync so that this runs after the first deferred_probe run */
>>   late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
>> +
>> +static const struct dmi_system_id run_edge_events_on_boot_blacklist[] = {
>> +	{
>> +		.matches = {
>> +			DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
>> +		}
>> +	},
>> +	{} /* Terminating entry */
>> +};
>> +
>> +static int acpi_gpio_setup_params(void)
>> +{
>> +	if (run_edge_events_on_boot < 0) {
>> +		if (dmi_check_system(run_edge_events_on_boot_blacklist))
>> +			run_edge_events_on_boot = 0;
>> +		else
>> +			run_edge_events_on_boot = 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* Directly after dmi_setup() which runs as core_initcall() */
>> +postcore_initcall(acpi_gpio_setup_params);
>> -- 
>> 2.23.0
>>
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-28 12:20   ` Hans de Goede
@ 2019-08-28 12:21     ` Hans de Goede
  2019-08-28 13:16       ` Ian W MORRISON
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2019-08-28 12:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mika Westerberg, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-acpi, stable, Daniel Drake, Ian W MORRISON

Hi,

On 28-08-19 14:20, Hans de Goede wrote:
> Hi,
> 
> On 28-08-19 13:37, Andy Shevchenko wrote:
>> On Tue, Aug 27, 2019 at 10:28:35PM +0200, Hans de Goede wrote:
>>> Another day; another DSDT bug we need to workaround...
>>>
>>> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
>>> at least once on boot") we call _AEI edge handlers at boot.
>>>
>>> In some rare cases this causes problems. One example of this is the Minix
>>> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
>>> and pasted code for dealing with Micro USB-B connector host/device role
>>> switching, while the mini PC does not even have a micro-USB connector.
>>> This code, which should not be there, messes with the DDC data pin from
>>> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
>>>
>>> To avoid problems like this, this commit adds a new
>>> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
>>> allows disabling the running of _AEI edge event handlers at boot.
>>>
>>> The default value is -1/auto which uses a DMI based blacklist, the initial
>>> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.
>>
>> Thank you!
>>
>> Assuming it works for Ian,
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Note I have access to a Minix Neo Z83-4 myself now and I did test that
> this fixes it and that passing gpiolib_acpi.run_edge_events_on_boot=0
> breaks HDMI again (so the option works).

Erm that should be gpiolib_acpi.run_edge_events_on_boot=1 (not 0) breaks
HDMI.

Regards,

Hans



>>> Cc: stable@vger.kernel.org
>>> Cc: Daniel Drake <drake@endlessm.com>
>>> Cc: Ian W MORRISON <ianwmorrison@gmail.com>
>>> Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
>>> Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
>>> Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> - Use a module_param instead of __setup
>>> - Do DMI check only once from a postcore_initcall
>>> ---
>>>   drivers/gpio/gpiolib-acpi.c | 42 +++++++++++++++++++++++++++++++++----
>>>   1 file changed, 38 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>>> index 39f2f9035c11..bda28eb82c3f 100644
>>> --- a/drivers/gpio/gpiolib-acpi.c
>>> +++ b/drivers/gpio/gpiolib-acpi.c
>>> @@ -7,6 +7,7 @@
>>>    *          Mika Westerberg <mika.westerberg@linux.intel.com>
>>>    */
>>> +#include <linux/dmi.h>
>>>   #include <linux/errno.h>
>>>   #include <linux/gpio/consumer.h>
>>>   #include <linux/gpio/driver.h>
>>> @@ -19,6 +20,11 @@
>>>   #include "gpiolib.h"
>>> +static int run_edge_events_on_boot = -1;
>>> +module_param(run_edge_events_on_boot, int, 0444);
>>> +MODULE_PARM_DESC(run_edge_events_on_boot,
>>> +         "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
>>> +
>>>   /**
>>>    * struct acpi_gpio_event - ACPI GPIO event handler data
>>>    *
>>> @@ -170,10 +176,13 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
>>>       event->irq_requested = true;
>>>       /* Make sure we trigger the initial state of edge-triggered IRQs */
>>> -    value = gpiod_get_raw_value_cansleep(event->desc);
>>> -    if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>>> -        ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>>> -        event->handler(event->irq, event);
>>> +    if (run_edge_events_on_boot &&
>>> +        (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>> +        value = gpiod_get_raw_value_cansleep(event->desc);
>>> +        if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>>> +            ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>>> +            event->handler(event->irq, event);
>>> +    }
>>>   }
>>>   static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
>>> @@ -1283,3 +1292,28 @@ static int acpi_gpio_handle_deferred_request_irqs(void)
>>>   }
>>>   /* We must use _sync so that this runs after the first deferred_probe run */
>>>   late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
>>> +
>>> +static const struct dmi_system_id run_edge_events_on_boot_blacklist[] = {
>>> +    {
>>> +        .matches = {
>>> +            DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
>>> +        }
>>> +    },
>>> +    {} /* Terminating entry */
>>> +};
>>> +
>>> +static int acpi_gpio_setup_params(void)
>>> +{
>>> +    if (run_edge_events_on_boot < 0) {
>>> +        if (dmi_check_system(run_edge_events_on_boot_blacklist))
>>> +            run_edge_events_on_boot = 0;
>>> +        else
>>> +            run_edge_events_on_boot = 1;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/* Directly after dmi_setup() which runs as core_initcall() */
>>> +postcore_initcall(acpi_gpio_setup_params);
>>> -- 
>>> 2.23.0
>>>
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-28 12:21     ` Hans de Goede
@ 2019-08-28 13:16       ` Ian W MORRISON
  0 siblings, 0 replies; 7+ messages in thread
From: Ian W MORRISON @ 2019-08-28 13:16 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Andy Shevchenko, Mika Westerberg, Bartosz Golaszewski,
	Linus Walleij, linux-gpio, linux-acpi, stable, Daniel Drake

On Wed, 28 Aug 2019 at 22:21, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 28-08-19 14:20, Hans de Goede wrote:
> > Hi,
> >
> > On 28-08-19 13:37, Andy Shevchenko wrote:
> >> On Tue, Aug 27, 2019 at 10:28:35PM +0200, Hans de Goede wrote:
> >>> Another day; another DSDT bug we need to workaround...
> >>>
> >>> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
> >>> at least once on boot") we call _AEI edge handlers at boot.
> >>>
> >>> In some rare cases this causes problems. One example of this is the Minix
> >>> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
> >>> and pasted code for dealing with Micro USB-B connector host/device role
> >>> switching, while the mini PC does not even have a micro-USB connector.
> >>> This code, which should not be there, messes with the DDC data pin from
> >>> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
> >>>
> >>> To avoid problems like this, this commit adds a new
> >>> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
> >>> allows disabling the running of _AEI edge event handlers at boot.
> >>>
> >>> The default value is -1/auto which uses a DMI based blacklist, the initial
> >>> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.
> >>
> >> Thank you!
> >>
> >> Assuming it works for Ian,
> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Note I have access to a Minix Neo Z83-4 myself now and I did test that
> > this fixes it and that passing gpiolib_acpi.run_edge_events_on_boot=0
> > breaks HDMI again (so the option works).
>
> Erm that should be gpiolib_acpi.run_edge_events_on_boot=1 (not 0) breaks
> HDMI.
>

Many thanks again Hans. I've also tested the patch including the various command
line options on my MINIX Z83-4 and they work fine.

Best regards,
Ian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist
  2019-08-27 20:28 [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist Hans de Goede
  2019-08-28 11:37 ` Mika Westerberg
  2019-08-28 11:37 ` Andy Shevchenko
@ 2019-09-11  9:48 ` Linus Walleij
  2 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2019-09-11  9:48 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mika Westerberg, Andy Shevchenko, Bartosz Golaszewski,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List, stable,
	Daniel Drake, Ian W MORRISON

On Tue, Aug 27, 2019 at 9:28 PM Hans de Goede <hdegoede@redhat.com> wrote:

> Another day; another DSDT bug we need to workaround...
>
> Since commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events
> at least once on boot") we call _AEI edge handlers at boot.
>
> In some rare cases this causes problems. One example of this is the Minix
> Neo Z83-4 mini PC, this device has a clear DSDT bug where it has some copy
> and pasted code for dealing with Micro USB-B connector host/device role
> switching, while the mini PC does not even have a micro-USB connector.
> This code, which should not be there, messes with the DDC data pin from
> the HDMI connector (switching it to GPIO mode) breaking HDMI support.
>
> To avoid problems like this, this commit adds a new
> gpiolib_acpi.run_edge_events_on_boot kernel commandline option, which
> allows disabling the running of _AEI edge event handlers at boot.
>
> The default value is -1/auto which uses a DMI based blacklist, the initial
> version of this blacklist contains the Neo Z83-4 fixing the HDMI breakage.
>
> Cc: stable@vger.kernel.org
> Cc: Daniel Drake <drake@endlessm.com>
> Cc: Ian W MORRISON <ianwmorrison@gmail.com>
> Reported-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Suggested-by: Ian W MORRISON <ianwmorrison@gmail.com>
> Fixes: ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Use a module_param instead of __setup
> - Do DMI check only once from a postcore_initcall

Patch applied for fixes, thanks for working this out,
sorry for slow reply.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 20:28 [PATCH v2] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist Hans de Goede
2019-08-28 11:37 ` Mika Westerberg
2019-08-28 11:37 ` Andy Shevchenko
2019-08-28 12:20   ` Hans de Goede
2019-08-28 12:21     ` Hans de Goede
2019-08-28 13:16       ` Ian W MORRISON
2019-09-11  9:48 ` Linus Walleij

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org linux-acpi@archiver.kernel.org
	public-inbox-index linux-acpi


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/ public-inbox