| Message ID | 1432281368-88687-1-git-send-email-mika.westerberg@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 22, 2015 at 10:56:08AM +0300, Mika Westerberg wrote: > BIOS/platform may use some of the pins by themselves, such as providing SCI > (System Control Interrupt) from the embedded controller. The driver masks > all interrupts at probe time which prevents those pins from triggering > interrupts properly. > > Fix this by not masking all interrupts at probe -- it should be enough just > to clear the status register. > > Reported-by: Yu C Chen <yu.c.chen@intel.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Please ignore this patch for now. It turned out to be causing spurious interrupts on another platform. I'll need to rethink how to fix the reported issue. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 1, 2015 at 11:23 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Fri, May 22, 2015 at 10:56:08AM +0300, Mika Westerberg wrote: >> BIOS/platform may use some of the pins by themselves, such as providing SCI >> (System Control Interrupt) from the embedded controller. The driver masks >> all interrupts at probe time which prevents those pins from triggering >> interrupts properly. >> >> Fix this by not masking all interrupts at probe -- it should be enough just >> to clear the status register. >> >> Reported-by: Yu C Chen <yu.c.chen@intel.com> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Please ignore this patch for now. It turned out to be causing spurious > interrupts on another platform. > > I'll need to rethink how to fix the reported issue. Looks like a case of "embed more magic knowledge" in the driver :/ It needs to know what platform it is running on, and only leave specific bits unmasked on these specific platforms. Right? Thereby tossing all of the acpi_device_id matching and abstraction out of the window. (Yes, I hate BIOSes doing crap behind my back.) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 02, 2015 at 03:53:40PM +0200, Linus Walleij wrote: > On Mon, Jun 1, 2015 at 11:23 AM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > On Fri, May 22, 2015 at 10:56:08AM +0300, Mika Westerberg wrote: > >> BIOS/platform may use some of the pins by themselves, such as providing SCI > >> (System Control Interrupt) from the embedded controller. The driver masks > >> all interrupts at probe time which prevents those pins from triggering > >> interrupts properly. > >> > >> Fix this by not masking all interrupts at probe -- it should be enough just > >> to clear the status register. > >> > >> Reported-by: Yu C Chen <yu.c.chen@intel.com> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > Please ignore this patch for now. It turned out to be causing spurious > > interrupts on another platform. > > > > I'll need to rethink how to fix the reported issue. > > Looks like a case of "embed more magic knowledge" in the driver :/ > > It needs to know what platform it is running on, and only leave specific > bits unmasked on these specific platforms. Right? Thereby > tossing all of the acpi_device_id matching and abstraction out > of the window. That's right. We still have few options left, like using ACPI _AEI (ACPI GPIO triggered events) for this or adding GPIO interrupt support directly to the EC driver. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
TWlrYSBXZXN0ZXJiZXJnIDxtaWthLndlc3RlcmJlcmcgPGF0PiBsaW51eC5pbnRlbC5jb20+IHdyaXRlczoKCj4gCj4gT24gVHVlLCBKdW4gMDIsIDIwMTUgYXQgMDM6NTM6NDBQTSArMDIwMCwgTGludXMgV2FsbGVpaiB3cm90ZToKPiA+IE9uIE1vbiwgSnVuIDEsIDIwMTUgYXQgMTE6MjMgQU0sIE1pa2EgV2VzdGVyYmVyZwo+ID4gPG1pa2Eud2VzdGVyYmVyZyA8YXQ+IGxpbnV4LmludGVsLmNvbT4gd3JvdGU6Cj4gPiA+IE9uIEZyaSwgTWF5IDIyLCAyMDE1IGF0IDEwOjU2OjA4QU0gKzAzMDAsIE1pa2EgV2VzdGVyYmVyZyB3cm90ZToKPiA+ID4+IEJJT1MvcGxhdGZvcm0gbWF5IHVzZSBzb21lIG9mIHRoZSBwaW5zIGJ5IHRoZW1zZWx2ZXMsIHN1Y2ggYXMKcHJvdmlkaW5nIFNDSQo+ID4gPj4gKFN5c3RlbSBDb250cm9sIEludGVycnVwdCkgZnJvbSB0aGUgZW1iZWRkZWQgY29udHJvbGxlci4gVGhlIGRyaXZlciBtYXNrcwo+ID4gPj4gYWxsIGludGVycnVwdHMgYXQgcHJvYmUgdGltZSB3aGljaCBwcmV2ZW50cyB0aG9zZSBwaW5zIGZyb20gdHJpZ2dlcmluZwo+ID4gPj4gaW50ZXJydXB0cyBwcm9wZXJseS4KPiA+ID4+Cj4gPiA+PiBGaXggdGhpcyBieSBub3QgbWFza2luZyBhbGwgaW50ZXJydXB0cyBhdCBwcm9iZSAtLSBpdCBzaG91bGQgYmUKZW5vdWdoIGp1c3QKPiA+ID4+IHRvIGNsZWFyIHRoZSBzdGF0dXMgcmVnaXN0ZXIuCj4gPiA+Pgo+ID4gPj4gUmVwb3J0ZWQtYnk6IFl1IEMgQ2hlbiA8eXUuYy5jaGVuIDxhdD4gaW50ZWwuY29tPgo+ID4gPj4gU2lnbmVkLW9mZi1ieTogTWlrYSBXZXN0ZXJiZXJnIDxtaWthLndlc3RlcmJlcmcgPGF0PiBsaW51eC5pbnRlbC5jb20+Cj4gPiA+Cj4gPiA+IFBsZWFzZSBpZ25vcmUgdGhpcyBwYXRjaCBmb3Igbm93LiBJdCB0dXJuZWQgb3V0IHRvIGJlIGNhdXNpbmcgc3B1cmlvdXMKPiA+ID4gaW50ZXJydXB0cyBvbiBhbm90aGVyIHBsYXRmb3JtLgo+ID4gPgo+ID4gPiBJJ2xsIG5lZWQgdG8gcmV0aGluayBob3cgdG8gZml4IHRoZSByZXBvcnRlZCBpc3N1ZS4KPiA+IAo+ID4gTG9va3MgbGlrZSBhIGNhc2Ugb2YgImVtYmVkIG1vcmUgbWFnaWMga25vd2xlZGdlIiBpbiB0aGUgZHJpdmVyIDovCj4gPiAKPiA+IEl0IG5lZWRzIHRvIGtub3cgd2hhdCBwbGF0Zm9ybSBpdCBpcyBydW5uaW5nIG9uLCBhbmQgb25seSBsZWF2ZSBzcGVjaWZpYwo+ID4gYml0cyB1bm1hc2tlZCBvbiB0aGVzZSBzcGVjaWZpYyBwbGF0Zm9ybXMuIFJpZ2h0PyBUaGVyZWJ5Cj4gPiB0b3NzaW5nIGFsbCBvZiB0aGUgYWNwaV9kZXZpY2VfaWQgbWF0Y2hpbmcgYW5kIGFic3RyYWN0aW9uIG91dAo+ID4gb2YgdGhlIHdpbmRvdy4KPiAKPiBUaGF0J3MgcmlnaHQuCj4gCj4gV2Ugc3RpbGwgaGF2ZSBmZXcgb3B0aW9ucyBsZWZ0LCBsaWtlIHVzaW5nIEFDUEkgX0FFSSAoQUNQSSBHUElPCj4gdHJpZ2dlcmVkIGV2ZW50cykgZm9yIHRoaXMgb3IgYWRkaW5nIEdQSU8gaW50ZXJydXB0IHN1cHBvcnQgZGlyZWN0bHkgdG8KPiB0aGUgRUMgZHJpdmVyLgoKQXJlIHRoZXJlIGFueSB1cGRhdGVzIG9uIHRoaXMgcHJvYmxlbT8gSSBjYW1lIHRocm91Z2ggdGhpcyB3aGlsZSBkb2luZwpoYXJkd2FyZSBlbmFibGVtZW50IGZvciBhIGxhcHRvcCwgYW5kIHRoZSBzeW1wdG9tcyB3ZXJlIHRoZSBicmlnaHRuZXNzCmNvbnRyb2wga2V5cyBzdG9wIHdvcmtpbmcgd2hlbiB0aGlzIG1vZHVsZSB3YXMgbG9hZGVkLiBUaGlzIHBhdGNoIGZpeGVzIHRoZQpwcm9ibGVtIG9uIHRoYXQgc3BlY2lmaWMgcGxhdGZvcm0uCgpJJ20gaGFwcHkgdG8gdGVzdCBvciBoZWxwIG91dCB3aXRoIGEgYmV0dGVyIHN1aXRlZCBmaXggaWYgZ2l2ZW4gc29tZSBndWlkYW5jZS4KCkJlc3QgcmVnYXJkcywKClBTOiBwbGVhc2Uga2VlcCBtZSBjb3BpZWQgaW4gdGhlIHRocmVhZCBzaW5jZSBJIGRvIG5vdCBzaWduIGxpbnV4LWdwaW8gbm9yIExLTUwuCgotLQpKb8OjbyBQYXVsbyBSZWNoaSBWaXRhCmh0dHA6Ly9hYm91dC5tZS9qcHJ2aXRh -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mika, On Tue, Jun 2, 2015 at 4:15 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Tue, Jun 02, 2015 at 03:53:40PM +0200, Linus Walleij wrote: >> On Mon, Jun 1, 2015 at 11:23 AM, Mika Westerberg >> <mika.westerberg@linux.intel.com> wrote: >> > On Fri, May 22, 2015 at 10:56:08AM +0300, Mika Westerberg wrote: >> >> BIOS/platform may use some of the pins by themselves, such as providing SCI >> >> (System Control Interrupt) from the embedded controller. The driver masks >> >> all interrupts at probe time which prevents those pins from triggering >> >> interrupts properly. >> >> >> >> Fix this by not masking all interrupts at probe -- it should be enough just >> >> to clear the status register. >> >> >> >> Reported-by: Yu C Chen <yu.c.chen@intel.com> >> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> >> > >> > Please ignore this patch for now. It turned out to be causing spurious >> > interrupts on another platform. >> > >> > I'll need to rethink how to fix the reported issue. >> >> Looks like a case of "embed more magic knowledge" in the driver :/ >> >> It needs to know what platform it is running on, and only leave specific >> bits unmasked on these specific platforms. Right? Thereby >> tossing all of the acpi_device_id matching and abstraction out >> of the window. > > That's right. > > We still have few options left, like using ACPI _AEI (ACPI GPIO > triggered events) for this or adding GPIO interrupt support directly to > the EC driver. Did you find a way to fix this issue ? I'm seeing a similar problem on a laptop where this masks the interrupt used for ACPI events (brightness, lid, battery). Regards, Anisse -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 16, 2016 at 06:12:40PM +0200, Anisse Astier wrote: > Hi Mika, > > On Tue, Jun 2, 2015 at 4:15 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > On Tue, Jun 02, 2015 at 03:53:40PM +0200, Linus Walleij wrote: > >> On Mon, Jun 1, 2015 at 11:23 AM, Mika Westerberg > >> <mika.westerberg@linux.intel.com> wrote: > >> > On Fri, May 22, 2015 at 10:56:08AM +0300, Mika Westerberg wrote: > >> >> BIOS/platform may use some of the pins by themselves, such as providing SCI > >> >> (System Control Interrupt) from the embedded controller. The driver masks > >> >> all interrupts at probe time which prevents those pins from triggering > >> >> interrupts properly. > >> >> > >> >> Fix this by not masking all interrupts at probe -- it should be enough just > >> >> to clear the status register. > >> >> > >> >> Reported-by: Yu C Chen <yu.c.chen@intel.com> > >> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > >> > > >> > Please ignore this patch for now. It turned out to be causing spurious > >> > interrupts on another platform. > >> > > >> > I'll need to rethink how to fix the reported issue. > >> > >> Looks like a case of "embed more magic knowledge" in the driver :/ > >> > >> It needs to know what platform it is running on, and only leave specific > >> bits unmasked on these specific platforms. Right? Thereby > >> tossing all of the acpi_device_id matching and abstraction out > >> of the window. > > > > That's right. > > > > We still have few options left, like using ACPI _AEI (ACPI GPIO > > triggered events) for this or adding GPIO interrupt support directly to > > the EC driver. > > Did you find a way to fix this issue ? I'm seeing a similar problem on a > laptop where this masks the interrupt used for ACPI events (brightness, > lid, battery). I seem to have forgotten this completely :-/ Can you send me output of /sys/kernel/debug/pinctrl/INT33FF:*/pins for that particular EC pin? In addition if you apply this patch do you see that ACPI events start working? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 17, 2016 at 10:13 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Tue, Aug 16, 2016 at 06:12:40PM +0200, Anisse Astier wrote: >> Hi Mika, >> >> Did you find a way to fix this issue ? I'm seeing a similar problem on a >> laptop where this masks the interrupt used for ACPI events (brightness, >> lid, battery). > > I seem to have forgotten this completely :-/ > > Can you send me output of /sys/kernel/debug/pinctrl/INT33FF:*/pins for > that particular EC pin? > > In addition if you apply this patch do you see that ACPI events start > working? From what I've seen it's in the north range, I don't know which pin in particular it is yet. If the interrupts aren't masked for the north community, ACPI events start working. # cat /sys/kernel/debug/pinctrl/INT33FF\:01/pins registered pins: 59 pin 0 (GPIO_DFX_0) GPIO ctrl0 0x00118102 ctrl1 0x05c00000 pin 1 (GPIO_DFX_3) GPIO ctrl0 0x2c018100 ctrl1 0x05c00000 pin 2 (GPIO_DFX_7) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 3 (GPIO_DFX_1) GPIO ctrl0 0x18118100 ctrl1 0x05c00000 pin 4 (GPIO_DFX_5) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 5 (GPIO_DFX_4) GPIO ctrl0 0x00118102 ctrl1 0x05c00000 pin 6 (GPIO_DFX_8) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 7 (GPIO_DFX_2) GPIO ctrl0 0x00118100 ctrl1 0x05c00000 pin 8 (GPIO_DFX_6) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 15 (GPIO_SUS0) GPIO ctrl0 0x3c018201 ctrl1 0x05c00001 pin 16 (SEC_GPIO_SUS10) GPIO ctrl0 0x00118100 ctrl1 0x05c00000 pin 17 (GPIO_SUS3) GPIO ctrl0 0x4c118100 ctrl1 0x05c00000 pin 18 (GPIO_SUS7) GPIO ctrl0 0xfc918201 ctrl1 0x05c00001 pin 19 (GPIO_SUS1) mode 6 ctrl0 0x00160301 ctrl1 0x05c00000 pin 20 (GPIO_SUS5) mode 1 ctrl0 0x00910200 ctrl1 0x05c00000 pin 21 (SEC_GPIO_SUS11) GPIO ctrl0 0x5c118100 ctrl1 0x05c00000 pin 22 (GPIO_SUS4) mode 6 ctrl0 0x00960301 ctrl1 0x05c00000 pin 23 (SEC_GPIO_SUS8) mode 1 ctrl0 0x00910300 ctrl1 0x05c00000 pin 24 (GPIO_SUS2) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 25 (GPIO_SUS6) GPIO ctrl0 0xec918201 ctrl1 0x05c00001 pin 26 (CX_PREQ_B) mode 1 ctrl0 0x00a10301 ctrl1 0x05c00000 pin 27 (SEC_GPIO_SUS9) GPIO ctrl0 0x00918102 ctrl1 0x05c00000 pin 30 (TRST_B) mode 1 ctrl0 0x00a10300 ctrl1 0x05c00000 pin 31 (TCK) mode 1 ctrl0 0x00210300 ctrl1 0x05c00000 pin 32 (PROCHOT_B) mode 1 ctrl0 0x00010301 ctrl1 0x05c00060 pin 33 (SVIDO_DATA) mode 0 ctrl0 0x00000300 ctrl1 0x05c00020 pin 34 (TMS) mode 1 ctrl0 0x00a10301 ctrl1 0x05c00000 pin 35 (CX_PRDY_B_2) mode 1 ctrl0 0x00a10301 ctrl1 0x05c00000 pin 36 (TDO_2) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 37 (CX_PRDY_B) mode 1 ctrl0 0x00a10301 ctrl1 0x05c00000 pin 38 (SVIDO_ALERT_B) mode 1 ctrl0 0x00010301 ctrl1 0x05c00040 pin 39 (TDO) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 40 (SVIDO_CLK) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 41 (TDI) mode 1 ctrl0 0x00a10301 ctrl1 0x05c00000 pin 45 (GP_CAMERASB_05) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 46 (GP_CAMERASB_02) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 47 (GP_CAMERASB_08) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 48 (GP_CAMERASB_00) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 49 (GP_CAMERASB_06) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 50 (GP_CAMERASB_10) GPIO ctrl0 0x00118100 ctrl1 0x05c00000 pin 51 (GP_CAMERASB_03) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 52 (GP_CAMERASB_09) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 53 (GP_CAMERASB_01) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 54 (GP_CAMERASB_07) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 55 (GP_CAMERASB_11) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 56 (GP_CAMERASB_04) GPIO ctrl0 0x00118100 ctrl1 0x05c00020 pin 60 (PANEL0_BKLTEN) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 61 (HV_DDI0_HPD) mode 1 ctrl0 0x03110301 ctrl1 0x05c00020 pin 62 (HV_DDI2_DDC_SDA) mode 3 ctrl0 0x00930301 ctrl1 0x05c00000 pin 63 (PANEL1_BKLTCTL) GPIO ctrl0 0x00018102 ctrl1 0x05c00000 pin 64 (HV_DDI1_HPD) mode 1 ctrl0 0x03010300 ctrl1 0x05c00020 pin 65 (PANEL0_BKLTCTL) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 66 (HV_DDI0_DDC_SDA) mode 1 ctrl0 0x00910301 ctrl1 0x05c00000 pin 67 (HV_DDI2_DDC_SCL) mode 3 ctrl0 0x00930301 ctrl1 0x05c00000 pin 68 (HV_DDI2_HPD) mode 1 ctrl0 0x03110300 ctrl1 0x05c00020 pin 69 (PANEL1_VDDEN) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 70 (PANEL1_BKLTEN) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 pin 71 (HV_DDI0_DDC_SCL) mode 1 ctrl0 0x00910301 ctrl1 0x05c00000 pin 72 (PANEL0_VDDEN) mode 1 ctrl0 0x00010300 ctrl1 0x05c00000 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index 82f691eeeec4..c9c0257a370c 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -1417,8 +1417,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) offset += range->npins; } - /* Mask and clear all interrupts */ - chv_writel(0, pctrl->regs + CHV_INTMASK); + /* Clear all interrupts */ chv_writel(0xffff, pctrl->regs + CHV_INTSTAT); ret = gpiochip_irqchip_add(chip, &chv_gpio_irqchip, 0,
BIOS/platform may use some of the pins by themselves, such as providing SCI (System Control Interrupt) from the embedded controller. The driver masks all interrupts at probe time which prevents those pins from triggering interrupts properly. Fix this by not masking all interrupts at probe -- it should be enough just to clear the status register. Reported-by: Yu C Chen <yu.c.chen@intel.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pinctrl/intel/pinctrl-cherryview.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)