Mark Kettenis <mark.kette...@xs4all.nl> writes:
>> From: Dave Voutila <d...@sisu.io>
>> Date: Sun, 27 Feb 2022 07:41:47 -0500
>>
>> James Hastings <mooset...@gmail.com> writes:
>>
>> > On Sun, Oct 10, 2021 at 11:42:31PM +0200, Mark Kettenis wrote:
>> >> > Date: Sat, 9 Oct 2021 22:27:52 +0200 (CEST)
>> >> > From: Mark Kettenis <mark.kette...@xs4all.nl>
>> >> >
>> >> > > Date: Sat, 9 Oct 2021 20:55:10 +0200 (CEST)
>> >> > > From: Mark Kettenis <mark.kette...@xs4all.nl>
>> >> > >
>> >> > > This time adding support for Sunrisepoint-H and Sunrisepoint-LP.
>> >> > > Because of all the failed attempts by Intel to get their 10nm process
>> >> > > under control, this may cover Intel Mobile CPUs marketed as 6th, 7th,
>> >> > > 8th, 9th and 10th generation. So if you have a Laptop that isn't at
>> >> > > least 5 years old, give this a try if pchgpio(4) doesn't attach. This
>> >> > > may fix all sorts of issues with keyboards, touchpads or
>> >> > > suspend/resume.
>> >> > >
>> >> > > ok?
>> >> >
>> >> > Updated diff that masks unhandled interrupts like we do in amdgpio(4).
>> >>
>> >> And another update to fix a typo in the pin groups for Sunrisepoint-LP.
>>
>> I had issues with kettenis@'s diff awhile ago on my Surface Go 3 that
>> IIRC is Sunrisepoint-LP. The touchscreen defines a gpio interrupt pin in
>> ACPI that ihidev(4) or dwiic(4) or something try to use for the device
>> instead of polling mode. It results in an interrupt storm on attach and
>> I'm not sure which driver needs fixing.
>>
>> I dont have the device with me but should be able to test this later
>> tonight my time.
>
> Hopefully the change James made fixes the issue.
Yes, looks like it does! No interrupt storm as ihidev(4) configures the
gpio pin for interrupts. My go3 with an Intel i3-10100Y is working fine
with this version of the diff:
$ dmesg | grep gpio
pchgpio0 at acpi0 GPI0 addr 0xfdaf0000/0x10000 0xfdae0000/0x10000
0xfdac0000/0x10000 irq 14, 176 pins
ihidev0 at iic1 addr 0x10 gpio 103, vendor 0x4f3 product 0x2a1c, ELAN9038
One question on a hunk in the diff itself:
RCS file: /cvs/src/sys/dev/acpi/pchgpio.c,v
retrieving revision 1.10
diff -u -p -r1.10 pchgpio.c
--- dev/acpi/pchgpio.c 21 Dec 2021 20:53:46 -0000 1.10
+++ dev/acpi/pchgpio.c 27 Feb 2022 09:29:17 -0000
@@ -465,11 +537,38 @@ pchgpio_intr_establish(void *cookie, int
}
int
+pchgpio_intr_handle(struct pchgpio_softc *sc, int group, int bit)
+{
+ uint32_t enable;
+ int gpiobase, pin, handled = 0;
+ uint8_t bank, bar;
+
+ bar = sc->sc_device->groups[group].bar;
+ bank = sc->sc_device->groups[group].bank;
+ gpiobase = sc->sc_device->groups[group].gpiobase;
+
+ pin = gpiobase + bit;
+ if (sc->sc_pin_ih[pin].ih_func) {
+ sc->sc_pin_ih[pin].ih_func(sc->sc_pin_ih[pin].ih_arg);
+ handled = 1;
+ } else {
+ /* Mask unhandled interrupt */
+ enable = bus_space_read_4(sc->sc_memt[bar], sc->sc_memh[bar],
+ sc->sc_device->gpi_ie + bank * 4);
+ enable &= ~(1 << bit);
+ bus_space_write_4(sc->sc_memt[bar], sc->sc_memh[bar],
+ sc->sc_device->gpi_ie + bank * 4, enable);
+ }
+
+ return handled;
+}
+
+int
pchgpio_intr(void *arg)
{
struct pchgpio_softc *sc = arg;
uint32_t status, enable;
- int gpiobase, group, bit, pin, handled = 0;
+ int group, bit, handled = 0;
uint16_t base, limit;
uint8_t bank, bar;
@@ -478,7 +577,6 @@ pchgpio_intr(void *arg)
bank = sc->sc_device->groups[group].bank;
base = sc->sc_device->groups[group].base;
limit = sc->sc_device->groups[group].limit;
- gpiobase = sc->sc_device->groups[group].gpiobase;
status = bus_space_read_4(sc->sc_memt[bar], sc->sc_memh[bar],
sc->sc_device->gpi_is + bank * 4);
@@ -491,10 +589,8 @@ pchgpio_intr(void *arg)
continue;
for (bit = 0; bit <= (limit - base); bit++) {
- pin = gpiobase + bit;
- if (status & (1 << bit) && sc->sc_pin_ih[pin].ih_func)
-
sc->sc_pin_ih[pin].ih_func(sc->sc_pin_ih[pin].ih_arg);
- handled = 1;
+ if (status & (1 << bit))
+ handled |= pchgpio_intr_handle(sc, group, bit);
^
Isn't this --------------------------------^
returning 0 or 1? Why the bitwise operation when handled is being set to
1 immediately prior? Am I missing something here?
-dv
