Skip to content

Commit ca876c7

Browse files
bentisslinusw
authored andcommitted
gpiolib-acpi: make sure we trigger edge events at least once on boot
On some systems using edge triggered ACPI Event Interrupts, the initial state at boot is not setup by the firmware, instead relying on the edge irq event handler running at least once to setup the initial state. 2 known examples of this are: 1) The Surface 3 has its _LID state controlled by an ACPI operation region triggered by a GPIO event: OperationRegion (GPOR, GeneralPurposeIo, Zero, One) Field (GPOR, ByteAcc, NoLock, Preserve) { Connection ( GpioIo (Shared, PullNone, 0x0000, 0x0000, IoRestrictionNone, "\\_SB.GPO0", 0x00, ResourceConsumer, , ) { // Pin list 0x004C } ), HELD, 1 } Method (_E4C, 0, Serialized) // _Exx: Edge-Triggered GPE { If ((HELD == One)) { ^^LID.LIDB = One } Else { ^^LID.LIDB = Zero Notify (LID, 0x80) // Status Change } Notify (^^PCI0.SPI1.NTRG, One) // Device Check } Currently, the state of LIDB is wrong until the user actually closes or open the cover. We need to trigger the GPIO event once to update the internal ACPI state. Coincidentally, this also enables the Surface 2 integrated HID sensor hub which also requires an ACPI gpio operation region to start initialization. 2) Various Bay Trail based tablets come with an external USB mux and TI T1210B USB phy to enable USB gadget mode. The mux is controlled by a GPIO which is controlled by an edge triggered ACPI Event Interrupt which monitors the micro-USB ID pin. When the tablet is connected to a PC (or no cable is plugged in), the ID pin is high and the tablet should be in gadget mode. But the GPIO controlling the mux is initialized by the firmware so that the USB data lines are muxed to the host controller. This means that if the user wants to use gadget mode, the user needs to first plug in a host-cable to force the ID pin low and then unplug it and connect the tablet to a PC, to get the ACPI event handler to run and switch the mux to device mode, This commit fixes both by running the event-handler once on boot. Note that the running of the event-handler is done from a late_initcall, this is done because the handler AML code may rely on OperationRegions registered by other builtin drivers. This avoids errors like these: [ 0.133026] ACPI Error: No handler for Region [XSCG] ((____ptrval____)) [GenericSerialBus] (20180531/evregion-132) [ 0.133036] ACPI Error: Region GenericSerialBus (ID=9) has no handler (20180531/exfldio-265) [ 0.133046] ACPI Error: Method parse/execution failed \_SB.GPO2._E12, AE_NOT_EXIST (20180531/psparse-516) Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> [hdegoede: Document BYT USB mux reliance on initial trigger] [hdegoede: Run event handler from a late_initcall, rather then immediately] Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
1 parent acb1872 commit ca876c7

File tree

1 file changed

+55
-1
lines changed

1 file changed

+55
-1
lines changed

drivers/gpio/gpiolib-acpi.c

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
struct acpi_gpio_event {
2727
struct list_head node;
28+
struct list_head initial_sync_list;
2829
acpi_handle handle;
2930
unsigned int pin;
3031
unsigned int irq;
@@ -50,6 +51,9 @@ struct acpi_gpio_chip {
5051
struct list_head events;
5152
};
5253

54+
static LIST_HEAD(acpi_gpio_initial_sync_list);
55+
static DEFINE_MUTEX(acpi_gpio_initial_sync_list_lock);
56+
5357
static int acpi_gpiochip_find(struct gpio_chip *gc, void *data)
5458
{
5559
if (!gc->parent)
@@ -85,6 +89,21 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
8589
return gpiochip_get_desc(chip, pin);
8690
}
8791

92+
static void acpi_gpio_add_to_initial_sync_list(struct acpi_gpio_event *event)
93+
{
94+
mutex_lock(&acpi_gpio_initial_sync_list_lock);
95+
list_add(&event->initial_sync_list, &acpi_gpio_initial_sync_list);
96+
mutex_unlock(&acpi_gpio_initial_sync_list_lock);
97+
}
98+
99+
static void acpi_gpio_del_from_initial_sync_list(struct acpi_gpio_event *event)
100+
{
101+
mutex_lock(&acpi_gpio_initial_sync_list_lock);
102+
if (!list_empty(&event->initial_sync_list))
103+
list_del_init(&event->initial_sync_list);
104+
mutex_unlock(&acpi_gpio_initial_sync_list_lock);
105+
}
106+
88107
static irqreturn_t acpi_gpio_irq_handler(int irq, void *data)
89108
{
90109
struct acpi_gpio_event *event = data;
@@ -136,7 +155,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
136155
irq_handler_t handler = NULL;
137156
struct gpio_desc *desc;
138157
unsigned long irqflags;
139-
int ret, pin, irq;
158+
int ret, pin, irq, value;
140159

141160
if (!acpi_gpio_get_irq_resource(ares, &agpio))
142161
return AE_OK;
@@ -167,6 +186,8 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
167186

168187
gpiod_direction_input(desc);
169188

189+
value = gpiod_get_value(desc);
190+
170191
ret = gpiochip_lock_as_irq(chip, pin);
171192
if (ret) {
172193
dev_err(chip->parent, "Failed to lock GPIO as interrupt\n");
@@ -208,6 +229,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
208229
event->irq = irq;
209230
event->pin = pin;
210231
event->desc = desc;
232+
INIT_LIST_HEAD(&event->initial_sync_list);
211233

212234
ret = request_threaded_irq(event->irq, NULL, handler, irqflags,
213235
"ACPI:Event", event);
@@ -222,6 +244,18 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
222244
enable_irq_wake(irq);
223245

224246
list_add_tail(&event->node, &acpi_gpio->events);
247+
248+
/*
249+
* Make sure we trigger the initial state of the IRQ when using RISING
250+
* or FALLING. Note we run the handlers on late_init, the AML code
251+
* may refer to OperationRegions from other (builtin) drivers which
252+
* may be probed after us.
253+
*/
254+
if (handler == acpi_gpio_irq_handler &&
255+
(((irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
256+
((irqflags & IRQF_TRIGGER_FALLING) && value == 0)))
257+
acpi_gpio_add_to_initial_sync_list(event);
258+
225259
return AE_OK;
226260

227261
fail_free_event:
@@ -294,6 +328,8 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
294328
list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) {
295329
struct gpio_desc *desc;
296330

331+
acpi_gpio_del_from_initial_sync_list(event);
332+
297333
if (irqd_is_wakeup_set(irq_get_irq_data(event->irq)))
298334
disable_irq_wake(event->irq);
299335

@@ -1158,3 +1194,21 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
11581194

11591195
return con_id == NULL;
11601196
}
1197+
1198+
/* Sync the initial state of handlers after all builtin drivers have probed */
1199+
static int acpi_gpio_initial_sync(void)
1200+
{
1201+
struct acpi_gpio_event *event, *ep;
1202+
1203+
mutex_lock(&acpi_gpio_initial_sync_list_lock);
1204+
list_for_each_entry_safe(event, ep, &acpi_gpio_initial_sync_list,
1205+
initial_sync_list) {
1206+
acpi_evaluate_object(event->handle, NULL, NULL, NULL);
1207+
list_del_init(&event->initial_sync_list);
1208+
}
1209+
mutex_unlock(&acpi_gpio_initial_sync_list_lock);
1210+
1211+
return 0;
1212+
}
1213+
/* We must use _sync so that this runs after the first deferred_probe run */
1214+
late_initcall_sync(acpi_gpio_initial_sync);

0 commit comments

Comments
 (0)