| Message ID | 20160911080506.GO15313@lahna.fi.intel.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 11, 2016 at 11:05:06AM +0300, Mika Westerberg wrote: > On Fri, Sep 09, 2016 at 11:58:32AM +0300, Mika Westerberg wrote: > > On Fri, Sep 09, 2016 at 04:23:58PM +0800, Phidias Chiang wrote: > > > > Only other place where we touch INTMASK register is > > chv_gpio_irq_mask_unmask(). Can you add some debug there to find out the > > caller? > > Something like this: > > diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c > index 0fe8fad..95fa3b1 100644 > --- a/drivers/pinctrl/intel/pinctrl-cherryview.c > +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c > @@ -1357,6 +1357,11 @@ static void chv_gpio_irq_mask_unmask(struct irq_data *d, bool mask) > value |= BIT(intr_line); > chv_writel(value, pctrl->regs + CHV_INTMASK); > > + if (printk_ratelimit()) { > + dev_info(pctrl->dev, "%smask pin %u intmask 0x%08x\n", > + mask ? "" : "un", pin, readl(pctrl->regs + CHV_INTMASK)); > + } > + > raw_spin_unlock_irqrestore(&chv_lock, flags); > } > With printk_ratelimit(): [ 2.058485] cherryview-pinctrl INT33FF:00: INTMASK0: 0x00000006 [ 2.058513] cherryview-pinctrl INT33FF:00: mask pin 0 intmask 0x00000006 [ 2.058533] cherryview-pinctrl INT33FF:00: mask pin 1 intmask 0x00000006 [ 2.058551] cherryview-pinctrl INT33FF:00: mask pin 2 intmask 0x00000006 [ 2.058569] cherryview-pinctrl INT33FF:00: mask pin 3 intmask 0x00000006 [ 2.058587] cherryview-pinctrl INT33FF:00: mask pin 4 intmask 0x00000006 [ 2.058604] cherryview-pinctrl INT33FF:00: mask pin 5 intmask 0x00000006 [ 2.058623] cherryview-pinctrl INT33FF:00: mask pin 6 intmask 0x00000006 [ 2.058641] cherryview-pinctrl INT33FF:00: mask pin 7 intmask 0x00000006 [ 2.058663] cherryview-pinctrl INT33FF:00: mask pin 15 intmask 0x00000006 [ 2.059401] cherryview-pinctrl INT33FF:00: INTMASK1: 0x00000000 [ 2.059551] cherryview-pinctrl INT33FF:01: INTMASK0: 0x00004000 [ 2.061272] cherryview-pinctrl INT33FF:01: INTMASK1: 0x00000000 [ 2.061516] cherryview-pinctrl INT33FF:02: INTMASK0: 0x00000000 [ 2.061906] cherryview-pinctrl INT33FF:02: INTMASK1: 0x00000000 [ 2.062116] cherryview-pinctrl INT33FF:03: INTMASK0: 0x00000000 [ 2.062956] cherryview-pinctrl INT33FF:03: INTMASK1: 0x00000000 w/o printk_ratelimit(): http://pastebin.com/dLDELDB4 The main difference is the intmask turns to 0x4 on pin 75 and 0x0 afterwards for INT33FF:00. And same thing happened with :01 on pin 25 -- 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, Sep 12, 2016 at 02:56:56PM +0800, Phidias Chiang wrote: > On Sun, Sep 11, 2016 at 11:05:06AM +0300, Mika Westerberg wrote: > > On Fri, Sep 09, 2016 at 11:58:32AM +0300, Mika Westerberg wrote: > > > On Fri, Sep 09, 2016 at 04:23:58PM +0800, Phidias Chiang wrote: > > > > > > Only other place where we touch INTMASK register is > > > chv_gpio_irq_mask_unmask(). Can you add some debug there to find out the > > > caller? > > > > Something like this: > > > > diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c > > index 0fe8fad..95fa3b1 100644 > > --- a/drivers/pinctrl/intel/pinctrl-cherryview.c > > +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c > > @@ -1357,6 +1357,11 @@ static void chv_gpio_irq_mask_unmask(struct irq_data *d, bool mask) > > value |= BIT(intr_line); > > chv_writel(value, pctrl->regs + CHV_INTMASK); > > > > + if (printk_ratelimit()) { > > + dev_info(pctrl->dev, "%smask pin %u intmask 0x%08x\n", > > + mask ? "" : "un", pin, readl(pctrl->regs + CHV_INTMASK)); > > + } > > + > > raw_spin_unlock_irqrestore(&chv_lock, flags); > > } > > > > With printk_ratelimit(): > > [ 2.058485] cherryview-pinctrl INT33FF:00: INTMASK0: 0x00000006 > [ 2.058513] cherryview-pinctrl INT33FF:00: mask pin 0 intmask 0x00000006 > [ 2.058533] cherryview-pinctrl INT33FF:00: mask pin 1 intmask 0x00000006 > [ 2.058551] cherryview-pinctrl INT33FF:00: mask pin 2 intmask 0x00000006 > [ 2.058569] cherryview-pinctrl INT33FF:00: mask pin 3 intmask 0x00000006 > [ 2.058587] cherryview-pinctrl INT33FF:00: mask pin 4 intmask 0x00000006 > [ 2.058604] cherryview-pinctrl INT33FF:00: mask pin 5 intmask 0x00000006 > [ 2.058623] cherryview-pinctrl INT33FF:00: mask pin 6 intmask 0x00000006 > [ 2.058641] cherryview-pinctrl INT33FF:00: mask pin 7 intmask 0x00000006 > [ 2.058663] cherryview-pinctrl INT33FF:00: mask pin 15 intmask 0x00000006 > [ 2.059401] cherryview-pinctrl INT33FF:00: INTMASK1: 0x00000000 > [ 2.059551] cherryview-pinctrl INT33FF:01: INTMASK0: 0x00004000 > [ 2.061272] cherryview-pinctrl INT33FF:01: INTMASK1: 0x00000000 > [ 2.061516] cherryview-pinctrl INT33FF:02: INTMASK0: 0x00000000 > [ 2.061906] cherryview-pinctrl INT33FF:02: INTMASK1: 0x00000000 > [ 2.062116] cherryview-pinctrl INT33FF:03: INTMASK0: 0x00000000 > [ 2.062956] cherryview-pinctrl INT33FF:03: INTMASK1: 0x00000000 > > w/o printk_ratelimit(): > http://pastebin.com/dLDELDB4 > > The main difference is the intmask turns to 0x4 on pin 75 and 0x0 > afterwards for INT33FF:00. And same thing happened with :01 on pin 25 OK, I see what is going on now. When I changed handle_simple_irq to handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the handler is uninstalled and masks the line. If you change handle_bad_irq to handle_simple_irq, in call to gpiochip_irqchip_add(), does it work then? -- 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, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: > > OK, I see what is going on now. When I changed handle_simple_irq to > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the > handler is uninstalled and masks the line. > > If you change handle_bad_irq to handle_simple_irq, in call to > gpiochip_irqchip_add(), does it work then? Yes it does :), thank you for the support! -- 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, Sep 12, 2016 at 09:04:44PM +0800, Phidias Chiang wrote: > On Mon, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: > > > > OK, I see what is going on now. When I changed handle_simple_irq to > > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the > > handler is uninstalled and masks the line. > > > > If you change handle_bad_irq to handle_simple_irq, in call to > > gpiochip_irqchip_add(), does it work then? > > Yes it does :), thank you for the support! Thanks for testing. So we need to use handle_simple_irq here instead. Linus, do you see any problems with that? -- 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, Sep 12, 2016 at 3:11 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Mon, Sep 12, 2016 at 09:04:44PM +0800, Phidias Chiang wrote: >> On Mon, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: >> > >> > OK, I see what is going on now. When I changed handle_simple_irq to >> > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the >> > handler is uninstalled and masks the line. >> > >> > If you change handle_bad_irq to handle_simple_irq, in call to >> > gpiochip_irqchip_add(), does it work then? >> >> Yes it does :), thank you for the support! > > Thanks for testing. > > So we need to use handle_simple_irq here instead. > > Linus, do you see any problems with that? I need to see the patch in its context with a commit message, I can't figure it out from the thread. handle_simple_irq() is for something generic not level- or edge-triggered. If you support specific triggers only, it should not be used. Nominally assigning handle_bad_irq() until a specific edge or level is requested is the right thing to do, since the IRQ is really not configured for anything at all and hence has undefined behaviour. But write a patch and involve the irqchip people I guess? 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, Sep 13, 2016 at 11:18:49AM +0200, Linus Walleij wrote: > On Mon, Sep 12, 2016 at 3:11 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > On Mon, Sep 12, 2016 at 09:04:44PM +0800, Phidias Chiang wrote: > >> On Mon, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: > >> > > >> > OK, I see what is going on now. When I changed handle_simple_irq to > >> > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the > >> > handler is uninstalled and masks the line. > >> > > >> > If you change handle_bad_irq to handle_simple_irq, in call to > >> > gpiochip_irqchip_add(), does it work then? > >> > >> Yes it does :), thank you for the support! > > > > Thanks for testing. > > > > So we need to use handle_simple_irq here instead. > > > > Linus, do you see any problems with that? > > I need to see the patch in its context with a commit message, > I can't figure it out from the thread. > > handle_simple_irq() is for something generic not level- or > edge-triggered. If you support specific triggers only, it > should not be used. > > Nominally assigning handle_bad_irq() until a specific > edge or level is requested is the right thing to do, since > the IRQ is really not configured for anything at all and > hence has undefined behaviour. For Cherryview/Braswell some interrupts are actually configured by the BIOS but they are routed directly to the I/O-APIC and are supposed to be handled without involvement of the GPIO driver (an example of this is the ACPI SCI interrupt). However, INTMASK GPIO register can still be used to mask the interrupt in question. So when we specify handle_bad_irq as handler the IRQ core thinks the handler is being uninstalled and masks the interrupt. > But write a patch and involve the irqchip people I guess? OK, I'll do this. Thanks. -- 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, Sep 13, 2016 at 11:33 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Tue, Sep 13, 2016 at 11:18:49AM +0200, Linus Walleij wrote: >> On Mon, Sep 12, 2016 at 3:11 PM, Mika Westerberg >> <mika.westerberg@linux.intel.com> wrote: >> > On Mon, Sep 12, 2016 at 09:04:44PM +0800, Phidias Chiang wrote: >> >> On Mon, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: >> >> > >> >> > OK, I see what is going on now. When I changed handle_simple_irq to >> >> > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the >> >> > handler is uninstalled and masks the line. >> >> > >> >> > If you change handle_bad_irq to handle_simple_irq, in call to >> >> > gpiochip_irqchip_add(), does it work then? >> >> >> >> Yes it does :), thank you for the support! >> > >> > Thanks for testing. >> > >> > So we need to use handle_simple_irq here instead. >> > >> > Linus, do you see any problems with that? >> >> I need to see the patch in its context with a commit message, >> I can't figure it out from the thread. >> >> handle_simple_irq() is for something generic not level- or >> edge-triggered. If you support specific triggers only, it >> should not be used. >> >> Nominally assigning handle_bad_irq() until a specific >> edge or level is requested is the right thing to do, since >> the IRQ is really not configured for anything at all and >> hence has undefined behaviour. > > For Cherryview/Braswell some interrupts are actually configured by the > BIOS but they are routed directly to the I/O-APIC and are supposed to be > handled without involvement of the GPIO driver (an example of this is > the ACPI SCI interrupt). However, INTMASK GPIO register can still be > used to mask the interrupt in question. A-ha! But why are you registering a irqdomain entry for an interrupt that cannot be used, hm? > So when we specify handle_bad_irq as handler the IRQ core thinks the > handler is being uninstalled and masks the interrupt. You should not register any handle for it. In fact, IMO the irqdomain should reject it being mapped, as it is not for the kernel to use. Check: drivers/irqchip/irq-vic.c We supply a u32 to the driver from the device tree named "valid-mask". This has bits set to 1 for the valid (to be mapped) IRQs and zero for those we may not touch. So in our vic_irqdomain_map() call we have: /* Skip invalid IRQs, only register handlers for the real ones */ if (!(v->valid_sources & (1 << hwirq))) return -EPERM; So it can never be mapped. Also notice: /* create an IRQ mapping for each valid IRQ */ for (i = 0; i < fls(valid_sources); i++) if (valid_sources & (1 << i)) irq_create_mapping(v->domain, i); So we only create mappings where there are valid IRQs, skipping over any "holes" in the mapping. We should do something like this. To make this play nicely with gpiolib_irqchip_add() I suggest adding a bitmap valid_mask to struct gpio_chip() in include/linux/gpio/driver.h inside the CONFIG_GPIOLIB_IRQCHIP I guess u32 bitmap[MAX_IRQS_FOR_A_GPIO_CHIP]; Then augment the generic GPIO IRQCHIP helpers in drivers/gpio/gpiolib.c per above so that the invalid IRQs can't be mapped. The driver would just: /* Mark line N as invalid: used by BIOS */ set_bit(&chip->valid_mask, N); Before calling gpiolib_irqchip_add(). This has the downside of roofing the number of lines that can be flagged as valid/invalid, but I'm open to more advanced ideas on this, but the check needs to be fast in the irqdomain. 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, Sep 13, 2016 at 02:22:25PM +0200, Linus Walleij wrote: > On Tue, Sep 13, 2016 at 11:33 AM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > On Tue, Sep 13, 2016 at 11:18:49AM +0200, Linus Walleij wrote: > >> On Mon, Sep 12, 2016 at 3:11 PM, Mika Westerberg > >> <mika.westerberg@linux.intel.com> wrote: > >> > On Mon, Sep 12, 2016 at 09:04:44PM +0800, Phidias Chiang wrote: > >> >> On Mon, Sep 12, 2016 at 12:04:01PM +0300, Mika Westerberg wrote: > >> >> > > >> >> > OK, I see what is going on now. When I changed handle_simple_irq to > >> >> > handle_bad_irq, the IRQ core in __irq_do_set_handler() thinks the > >> >> > handler is uninstalled and masks the line. > >> >> > > >> >> > If you change handle_bad_irq to handle_simple_irq, in call to > >> >> > gpiochip_irqchip_add(), does it work then? > >> >> > >> >> Yes it does :), thank you for the support! > >> > > >> > Thanks for testing. > >> > > >> > So we need to use handle_simple_irq here instead. > >> > > >> > Linus, do you see any problems with that? > >> > >> I need to see the patch in its context with a commit message, > >> I can't figure it out from the thread. > >> > >> handle_simple_irq() is for something generic not level- or > >> edge-triggered. If you support specific triggers only, it > >> should not be used. > >> > >> Nominally assigning handle_bad_irq() until a specific > >> edge or level is requested is the right thing to do, since > >> the IRQ is really not configured for anything at all and > >> hence has undefined behaviour. > > > > For Cherryview/Braswell some interrupts are actually configured by the > > BIOS but they are routed directly to the I/O-APIC and are supposed to be > > handled without involvement of the GPIO driver (an example of this is > > the ACPI SCI interrupt). However, INTMASK GPIO register can still be > > used to mask the interrupt in question. > > A-ha! But why are you registering a irqdomain entry for an interrupt > that cannot be used, hm? Unfortunately there is no way to figure out from the hardware (or firmware) whether the interrupt is supposed to be used by the GPIO driver or something else. > > So when we specify handle_bad_irq as handler the IRQ core thinks the > > handler is being uninstalled and masks the interrupt. > > You should not register any handle for it. > > In fact, IMO the irqdomain should reject it being mapped, as it is not > for the kernel to use. > > Check: > drivers/irqchip/irq-vic.c > > We supply a u32 to the driver from the device tree named "valid-mask". > This has bits set to 1 for the valid (to be mapped) IRQs and zero > for those we may not touch. > > So in our vic_irqdomain_map() call we have: > > /* Skip invalid IRQs, only register handlers for the real ones */ > if (!(v->valid_sources & (1 << hwirq))) > return -EPERM; > > So it can never be mapped. > > Also notice: > > /* create an IRQ mapping for each valid IRQ */ > for (i = 0; i < fls(valid_sources); i++) > if (valid_sources & (1 << i)) > irq_create_mapping(v->domain, i); > > So we only create mappings where there are valid IRQs, > skipping over any "holes" in the mapping. > > We should do something like this. > > To make this play nicely with gpiolib_irqchip_add() I suggest > adding a bitmap valid_mask to struct gpio_chip() in > include/linux/gpio/driver.h > inside the CONFIG_GPIOLIB_IRQCHIP > > I guess > > u32 bitmap[MAX_IRQS_FOR_A_GPIO_CHIP]; > > Then augment the generic GPIO IRQCHIP helpers in > drivers/gpio/gpiolib.c per above so that the invalid > IRQs can't be mapped. > > The driver would just: > > /* Mark line N as invalid: used by BIOS */ > set_bit(&chip->valid_mask, N); Otherwise this probably works but it forces us to hardcode these "special" lines in the driver and I would like to avoid that if possible. -- 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, Sep 13, 2016 at 2:52 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > [Me] >> A-ha! But why are you registering a irqdomain entry for an interrupt >> that cannot be used, hm? > > Unfortunately there is no way to figure out from the hardware (or > firmware) whether the interrupt is supposed to be used by the GPIO > driver or something else. So the fact that we kept it in valid-mask in the DT was a hint: it is part of the hardware description. Isn't this (a list of what IRQs are reserved by BIOS) by sheer logic something that ACPI should provide? Or is this one of those "well we could alter ACPI tables but we can't because they already shipped so we just can't so now we need to hack around it"? Letting Linux map an interrupt it cannot access and then papering it over by using handle_simple_irq() just feels wrong to me. I would argue for associating the mask of BIOS-reserved IRQs with something in ACPI and implement the mentioned scheme to avoid even mapping them seems most logical. If we have to use handle_simple_irq() by default on all I prefer to put in a very fat comment of the type: /* * HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK * * Some interrupts are BIOS-reserved but we don't know which ones! * So we anyway map them and assign the handle_simple_irq() handle * to them, leaving them unmasked, pretending they can be used, and * pray no-one will accidentally use these GPIO IRQs. * * HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK */ 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, Sep 13, 2016 at 10:57:31PM +0200, Linus Walleij wrote: > On Tue, Sep 13, 2016 at 2:52 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > [Me] > >> A-ha! But why are you registering a irqdomain entry for an interrupt > >> that cannot be used, hm? > > > > Unfortunately there is no way to figure out from the hardware (or > > firmware) whether the interrupt is supposed to be used by the GPIO > > driver or something else. > > So the fact that we kept it in valid-mask in the DT was a hint: it is > part of the hardware description. > > Isn't this (a list of what IRQs are reserved by BIOS) by sheer logic > something that ACPI should provide? > > Or is this one of those "well we could alter ACPI tables but we can't > because they already shipped so we just can't so now we need to > hack around it"? Isn't it always the case? ;-) Once the hardware enters stores the firmware cannot be changed anymore and we get all the fun working around problems in the OS. > Letting Linux map an interrupt it cannot access and then papering it > over by using handle_simple_irq() just feels wrong to me. > > I would argue for associating the mask of BIOS-reserved IRQs with > something in ACPI and implement the mentioned scheme to avoid > even mapping them seems most logical. I'm going to re-read the hardware spec and see if there is anything we can do about this. The newer hardware (Skylake, Broxton) has a bit that tells the IRQ is routed directly to I/O-APIC but unfortunately Braswell misses that. There may be something else, though. > If we have to use handle_simple_irq() by default on all I prefer to put > in a very fat comment of the type: > > /* > * HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK > * > * Some interrupts are BIOS-reserved but we don't know which ones! > * So we anyway map them and assign the handle_simple_irq() handle > * to them, leaving them unmasked, pretending they can be used, and > * pray no-one will accidentally use these GPIO IRQs. > * > * HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK > */ OK, got it. Let me try to come up with a solution that both works and does not involve using handle_simple_irq. -- 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, Sep 14, 2016 at 10:26 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Tue, Sep 13, 2016 at 10:57:31PM +0200, Linus Walleij wrote: >> Isn't this (a list of what IRQs are reserved by BIOS) by sheer logic >> something that ACPI should provide? >> >> Or is this one of those "well we could alter ACPI tables but we can't >> because they already shipped so we just can't so now we need to >> hack around it"? > > Isn't it always the case? ;-) > > Once the hardware enters stores the firmware cannot be changed anymore > and we get all the fun working around problems in the OS. To me this is just a big proof that the ACPI design-by-committee isn't working. With Device Tree we review bindings and drivers together and then we tend to not miss stuff like this as much. But I realize as soon as I say that someone will pull out a counter-example of stupid DT bindings used in the wild and make me look stupid. >> Letting Linux map an interrupt it cannot access and then papering it >> over by using handle_simple_irq() just feels wrong to me. >> >> I would argue for associating the mask of BIOS-reserved IRQs with >> something in ACPI and implement the mentioned scheme to avoid >> even mapping them seems most logical. > > I'm going to re-read the hardware spec and see if there is anything we > can do about this. The newer hardware (Skylake, Broxton) has a bit that > tells the IRQ is routed directly to I/O-APIC but unfortunately Braswell > misses that. There may be something else, though. So as far as we can determine: (A) we are running on Braswell and (B) we are probing this driver we can conclude that (C) IRQs A,B,C are reserved by BIOS? That sounds doable? 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 Wed, Sep 14, 2016 at 02:46:01PM +0200, Linus Walleij wrote: > > I'm going to re-read the hardware spec and see if there is anything we > > can do about this. The newer hardware (Skylake, Broxton) has a bit that > > tells the IRQ is routed directly to I/O-APIC but unfortunately Braswell > > misses that. There may be something else, though. > > So as far as we can determine: > > (A) we are running on Braswell and > (B) we are probing this driver > > we can conclude that > > (C) IRQs A,B,C are reserved by BIOS? > > That sounds doable? Yes, it's doable but that requires some hard coding in the driver :-/ I'll look into it. -- 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, Sep 14, 2016 at 5:12 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Wed, Sep 14, 2016 at 02:46:01PM +0200, Linus Walleij wrote: >> > I'm going to re-read the hardware spec and see if there is anything we >> > can do about this. The newer hardware (Skylake, Broxton) has a bit that >> > tells the IRQ is routed directly to I/O-APIC but unfortunately Braswell >> > misses that. There may be something else, though. >> >> So as far as we can determine: >> >> (A) we are running on Braswell and >> (B) we are probing this driver >> >> we can conclude that >> >> (C) IRQs A,B,C are reserved by BIOS? >> >> That sounds doable? > > Yes, it's doable but that requires some hard coding in the driver :-/ From my point of view that is the lesser of two evils. We only have hard-coding (syntactic) madness over having behaviour-dependent (semantic) madness. 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 Thu, Sep 15, 2016 at 02:39:47PM +0200, Linus Walleij wrote: > On Wed, Sep 14, 2016 at 5:12 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > On Wed, Sep 14, 2016 at 02:46:01PM +0200, Linus Walleij wrote: > >> > I'm going to re-read the hardware spec and see if there is anything we > >> > can do about this. The newer hardware (Skylake, Broxton) has a bit that > >> > tells the IRQ is routed directly to I/O-APIC but unfortunately Braswell > >> > misses that. There may be something else, though. > >> > >> So as far as we can determine: > >> > >> (A) we are running on Braswell and > >> (B) we are probing this driver > >> > >> we can conclude that > >> > >> (C) IRQs A,B,C are reserved by BIOS? > >> > >> That sounds doable? > > > > Yes, it's doable but that requires some hard coding in the driver :-/ > > >From my point of view that is the lesser of two evils. > > We only have hard-coding (syntactic) madness over having > behaviour-dependent (semantic) madness. I re-read the hardware spec now and it occured to me that for north and southwest community, only the first 8 IRQs can be used as interrupts (all GPIOs which have IntSel value < 8). Rest can only trigger GPEs which are used for EC events. I'll submit patches shortly using this information and valid_mask as you suggested. -- 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 0fe8fad..95fa3b1 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -1357,6 +1357,11 @@ static void chv_gpio_irq_mask_unmask(struct irq_data *d, bool mask) value |= BIT(intr_line); chv_writel(value, pctrl->regs + CHV_INTMASK); + if (printk_ratelimit()) { + dev_info(pctrl->dev, "%smask pin %u intmask 0x%08x\n", + mask ? "" : "un", pin, readl(pctrl->regs + CHV_INTMASK)); + } + raw_spin_unlock_irqrestore(&chv_lock, flags); }