Patch ca876c7483b6 "gpiolib-acpi: make sure we trigger edge events at
least once on boot" causes the MINIX family of mini PCs to fail to boot
resulting in a "black screen".
This patch excludes MINIX devices from executing this trigger in order
to successfully boot.
Cc: [email protected]
Signed-off-by: Ian W MORRISON <[email protected]>
---
drivers/gpio/gpiolib-acpi.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 259cf6ab969b..8d855dc9b020 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -16,9 +16,21 @@
#include <linux/interrupt.h>
#include <linux/mutex.h>
#include <linux/pinctrl/pinctrl.h>
+#include <linux/dmi.h>
#include "gpiolib.h"
+static const struct dmi_system_id skip_deferred_request_irqs_table[] = {
+ {
+ .ident = "MINIX Z83-4",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "MINIX"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
+ },
+ },
+ {}
+};
+
/**
* struct acpi_gpio_event - ACPI GPIO event handler data
*
@@ -1219,18 +1231,24 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
}
/* Run deferred acpi_gpiochip_request_irqs() */
+/* but exclude devices known to fail */
static int acpi_gpio_handle_deferred_request_irqs(void)
{
struct acpi_gpio_chip *acpi_gpio, *tmp;
+ const struct dmi_system_id *dmi_id;
- mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
- list_for_each_entry_safe(acpi_gpio, tmp,
+ dmi_id = dmi_first_match(skip_deferred_request_irqs_table);
+
+ if (! dmi_id) {
+ mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+ list_for_each_entry_safe(acpi_gpio, tmp,
&acpi_gpio_deferred_req_irqs_list,
deferred_req_irqs_list_entry)
- acpi_gpiochip_request_irqs(acpi_gpio);
+ acpi_gpiochip_request_irqs(acpi_gpio);
- acpi_gpio_deferred_req_irqs_done = true;
- mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+ acpi_gpio_deferred_req_irqs_done = true;
+ mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+ }
return 0;
}
--
2.17.1
On Fri, Mar 22, 2019 at 10:05:15PM +1100, Ian W MORRISON wrote:
Thanks for the patch, my comments below.
> Patch ca876c7483b6 "gpiolib-acpi: make sure we trigger edge events at
> least once on boot" causes the MINIX family of mini PCs to fail to boot
> resulting in a "black screen".
> This patch excludes MINIX devices from executing this trigger in order
> to successfully boot.
Hmm... Feels like this is symptomatic healing.
Hans, do you have anything in mind about this case?
> #include <linux/interrupt.h>
> #include <linux/mutex.h>
> #include <linux/pinctrl/pinctrl.h>
> +#include <linux/dmi.h>
This should be in order.
> /* Run deferred acpi_gpiochip_request_irqs() */
> +/* but exclude devices known to fail */
/*
* This should be done in the similar style
* as for multi-line comments. Like this one.
*/
> + dmi_id = dmi_first_match(skip_deferred_request_irqs_table);
> +
Redundant blank line.
> + if (! dmi_id) {
No space here, however, better to write positive conditional.
--
With Best Regards,
Andy Shevchenko
Hi,
On 3/22/19 12:05 PM, Ian W MORRISON wrote:
> Patch ca876c7483b6 "gpiolib-acpi: make sure we trigger edge events at
> least once on boot" causes the MINIX family of mini PCs to fail to boot
> resulting in a "black screen".
>
> This patch excludes MINIX devices from executing this trigger in order
> to successfully boot.
>
> Cc: [email protected]
> Signed-off-by: Ian W MORRISON <[email protected]>
IMHO we need to root-cause this problem a bit more before applying this
kludge.
Can you provide an ACPI dump of one of the affected machines ?
Regards,
Hans
> ---
> drivers/gpio/gpiolib-acpi.c | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 259cf6ab969b..8d855dc9b020 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -16,9 +16,21 @@
> #include <linux/interrupt.h>
> #include <linux/mutex.h>
> #include <linux/pinctrl/pinctrl.h>
> +#include <linux/dmi.h>
>
> #include "gpiolib.h"
>
> +static const struct dmi_system_id skip_deferred_request_irqs_table[] = {
> + {
> + .ident = "MINIX Z83-4",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "MINIX"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
> + },
> + },
> + {}
> +};
> +
> /**
> * struct acpi_gpio_event - ACPI GPIO event handler data
> *
> @@ -1219,18 +1231,24 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
> }
>
> /* Run deferred acpi_gpiochip_request_irqs() */
> +/* but exclude devices known to fail */
> static int acpi_gpio_handle_deferred_request_irqs(void)
> {
> struct acpi_gpio_chip *acpi_gpio, *tmp;
> + const struct dmi_system_id *dmi_id;
>
> - mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
> - list_for_each_entry_safe(acpi_gpio, tmp,
> + dmi_id = dmi_first_match(skip_deferred_request_irqs_table);
> +
> + if (! dmi_id) {
> + mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
> + list_for_each_entry_safe(acpi_gpio, tmp,
> &acpi_gpio_deferred_req_irqs_list,
> deferred_req_irqs_list_entry)
> - acpi_gpiochip_request_irqs(acpi_gpio);
> + acpi_gpiochip_request_irqs(acpi_gpio);
>
> - acpi_gpio_deferred_req_irqs_done = true;
> - mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
> + acpi_gpio_deferred_req_irqs_done = true;
> + mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
> + }
>
> return 0;
> }
>
Hi Ian, et. al.,
On 23-03-19 04:39, Ian W MORRISON wrote:
> Hi Hans,
>
>> IMHO we need to root-cause this problem a bit more before applying this
>> kludge.
>>
>> Can you provide an ACPI dump of one of the affected machines ?
>>
>
> Attached is an ACPI dump.
First of all sorry for taking way too long to get back to you on this.
So I've taken a look at all the _AEI code in the DSDT, a whole bunch of
it seems copy and pasted from various tablets, but nothing really
stands out as being a likely cause of this.
As such I guess we may need to go with the blacklist patch you suggested
which sucks, but having these devices not boot sucks even harder.
I guess this problem did not magically fix it self in the mean time
(with newer kernels) ?
Can you resubmit your patch with Andy's review remarks addressed?
In case you've lost Andy's reply I will reproduce the review remarks
below.
Regards,
Hans
p.s.
Andy's review remarks as promised:
> #include <linux/interrupt.h>
> #include <linux/mutex.h>
> #include <linux/pinctrl/pinctrl.h>
> +#include <linux/dmi.h>
This should be in order.
> /* Run deferred acpi_gpiochip_request_irqs() */
> +/* but exclude devices known to fail */
/*
* This should be done in the similar style
* as for multi-line comments. Like this one.
*/
> + dmi_id = dmi_first_match(skip_deferred_request_irqs_table);
> +
Redundant blank line.
> + if (! dmi_id) {
No space here, however, better to write positive conditional.
Hi Hans and everyone,
On Mon, 19 Aug 2019 at 04:59, Hans de Goede <[email protected]> wrote:
>
> Hi Ian, et. al.,
>
> As such I guess we may need to go with the blacklist patch you suggested
> which sucks, but having these devices not boot sucks even harder.
>
> I guess this problem did not magically fix it self in the mean time
> (with newer kernels) ?
>
Unfortunately it didn't 'self-fix' with later kernels.
> Can you resubmit your patch with Andy's review remarks addressed?
>
> In case you've lost Andy's reply I will reproduce the review remarks
> below.
>
> Regards,
>
> Hans
>
Resubmitted as requested.
Many thanks and best regards,
Ian