Re: [PATCH v2] Skip deferred request irqs for devices known to fail
From: Andy Shevchenko
Date: Mon Aug 19 2019 - 07:52:25 EST
On Mon, Aug 19, 2019 at 09:26:37PM +1000, 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.
Thanks for an update.
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ian W MORRISON <ianwmorrison@xxxxxxxxx>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Hmm... Did I really give the tag?
Too many stuff is going on, anyway, please consider more comments below.
First of all, the subject should start from "gpiolib: acpi: " prefix.
Then, Fixes tag seems to be missed.
> +/*
> + * Run deferred acpi_gpiochip_request_irqs()
> + * but exclude devices known to fail
Missed period.
> +*/
Missed leading space (the column of stars).
> 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)
> + return 0;
The idea of positive check is exactly for...
> + else {
...getting rid of this redundant 'else' followed by unneeded level of indentation.
> + 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;
> }
--
With Best Regards,
Andy Shevchenko