> From: Dave Voutila <[email protected]> > Date: Sun, 27 Feb 2022 17:57:17 -0500 > > Mark Kettenis <[email protected]> writes: > > >> From: Dave Voutila <[email protected]> > >> Date: Sun, 27 Feb 2022 07:41:47 -0500 > >> > >> James Hastings <[email protected]> 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 <[email protected]> > >> >> > > >> >> > > Date: Sat, 9 Oct 2021 20:55:10 +0200 (CEST) > >> >> > > From: Mark Kettenis <[email protected]> > >> >> > > > >> >> > > 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?
That line immediately prior is a line that gets removed ;).
