Hi Andy,
On 26-08-19 11:11, Andy Shevchenko wrote:
On Fri, Aug 23, 2019 at 11:52:55PM +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
can be "on", "off", or "auto" (default).
In auto mode the default is on and a DMI based blacklist is used,
the initial version of this blacklist contains the Minix Neo Z83-4
fixing the HDMI being broken on this device.
+static int gpiolib_acpi_run_edge_events_on_boot = -1;
+
+static int __init gpiolib_acpi_run_edge_events_on_boot_setup(char *arg)
+{
+ if (!strcmp(arg, "on"))
+ gpiolib_acpi_run_edge_events_on_boot = 1;
+ else if (!strcmp(arg, "off"))
+ gpiolib_acpi_run_edge_events_on_boot = 0;
kstrtobool() ?
+ else if (!strcmp(arg, "auto"))
+ gpiolib_acpi_run_edge_events_on_boot = -1;
+
+ return 1;
+}
+
+__setup("gpiolib_acpi_run_edge_events_on_boot=",
+ gpiolib_acpi_run_edge_events_on_boot_setup);
Can't we use module_param() ?
The resulting option would be 'gpiolib_acpi.foo=...'
I was expecting that would not work, since gpiolib is a bool
not a tristate, but it seems that if there is no module-name
to use as prefix for module-parameters the kernel simply uses
the .c file name, so this works and yes, this is better then
using __setup, will fix for v2.
+{
+ if (gpiolib_acpi_run_edge_events_on_boot == -1) {
+ if (dmi_check_system(run_edge_events_on_boot_blacklist))
+ gpiolib_acpi_run_edge_events_on_boot = 0;
+ else
+ gpiolib_acpi_run_edge_events_on_boot = 1;
+ }
Can we run this at an initcall once and use variable instead of calling a
method below?
I was a bit worried about init ordering, but I've checked and dmi_setup()
is done as a core_initcall, so we can do this once as a postcore_initcall
which should be early enough, will fix for v2.
+ return gpiolib_acpi_run_edge_events_on_boot;
+}
+
static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
struct acpi_gpio_event *event)
{
@@ -170,10 +211,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 (acpi_gpiochip_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);
+ }
Regards,
Hans
