Hi Linus,
On 12-08-19 15:53, Linus Walleij wrote:
We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.
For chained irqchips this is a pretty straight-forward
conversion.
Cc: Maxim Mikityanskiy <maxtram95@xxxxxxxxx>
Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
Cc: Darren Hart <dvhart@xxxxxxxxxxxxx>
Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
---
Andy please merge this into your platform tree when you
feel happy with the patch, would be great of someone
can test it on hardware as well.
So I've just tested this on a Cherry Trail machine and
the interrupt storm, fixing which is the reason the
intel_int0002_vgpio.c driver was introduced, is back:
[root@localhost ~]# cat /proc/interrupts | grep INT0002
9: 0 23429420 0 0 IO-APIC 9-fasteoi acpi, INT0002
23 million interrupts and counting, normally this is 0
at boot low 10s after a suspend/resume with wakeup by
USB keyboard.
Notice that the driver has attached itself as shared irq-handler
to the ACPI IRQ, but it seems something is going wrong with the
registration of its own IRQ and/or for some reason the ACPI
subsys is no longer attaching the ACPI event handler for the
child IRQ to it, here is a the same command on a working
kernel:
[root@localhost ~]# cat /proc/interrupts | grep INT0002
9: 0 0 0 0 IO-APIC 9-fasteoi acpi, INT0002
123: 0 0 0 0 INT0002 Virtual GPIO 2 ACPI:Event
Do I need any patches on top of 5.3-rc4 to test this patch?
Note that it is important that the single irq on the chip is
advertised as irq number 2 (so the third irq) because that
is what the ACPI event code expects:
Device (GPED)
{
Name (_ADR, Zero) // _ADR: Address
Name (_HID, "INT0002" /* Virtual GPIO Controller */) // _HID: Hardw
Name (_CID, "INT0002" /* Virtual GPIO Controller */) // _CID: Compa
Name (_DDN, "Virtual GPIO controller") // _DDN: DOS Device Name
...
Method (_AEI, 0, NotSerialized) // _AEI: ACPI Event Interrupts
{
Name (RBUF, ResourceTemplate ()
{
GpioInt (Level, ActiveHigh, ExclusiveAndWake, PullDown, 0x00
"\\_SB.GPED", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0002
}
})
Return (RBUF) /* \_SB_.GPED._AEI.RBUF */
}
Method (_L02, 0, NotSerialized) // _Lxx: Level-Triggered GPE
{
...
}
}
Anyways, this will need to be fixed before we can merge this.
Regards,
Hans
---
drivers/platform/x86/intel_int0002_vgpio.c | 29 +++++++++-------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c
index d9542c661ddc..493a97ce0b08 100644
--- a/drivers/platform/x86/intel_int0002_vgpio.c
+++ b/drivers/platform/x86/intel_int0002_vgpio.c
@@ -156,8 +156,8 @@ static int int0002_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
const struct x86_cpu_id *cpu_id;
- struct irq_chip *irq_chip;
struct gpio_chip *chip;
+ struct gpio_irq_chip *girq;
int irq, ret;
/* Menlow has a different INT0002 device? <sigh> */
@@ -186,17 +186,9 @@ static int int0002_probe(struct platform_device *pdev)
chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1;
chip->irq.need_valid_mask = true;
- ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
- if (ret) {
- dev_err(dev, "Error adding gpio chip: %d\n", ret);
- return ret;
- }
-
- bitmap_clear(chip->irq.valid_mask, 0, GPE0A_PME_B0_VIRT_GPIO_PIN);
-
/*
- * We manually request the irq here instead of passing a flow-handler
- * to gpiochip_set_chained_irqchip, because the irq is shared.
+ * We directly request the irq here instead of passing a flow-handler
+ * to the gpio irqchip, because the irq is shared.
*/
ret = devm_request_irq(dev, irq, int0002_irq,
IRQF_SHARED, "INT0002", chip);
@@ -204,17 +196,20 @@ static int int0002_probe(struct platform_device *pdev)
dev_err(dev, "Error requesting IRQ %d: %d\n", irq, ret);
return ret;
}
+ girq = &chip->irq;
+ girq->chip = (struct irq_chip *)cpu_id->driver_data;
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_edge_irq;
- irq_chip = (struct irq_chip *)cpu_id->driver_data;
-
- ret = gpiochip_irqchip_add(chip, irq_chip, 0, handle_edge_irq,
- IRQ_TYPE_NONE);
+ ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
if (ret) {
- dev_err(dev, "Error adding irqchip: %d\n", ret);
+ dev_err(dev, "Error adding gpio chip: %d\n", ret);
return ret;
}
- gpiochip_set_chained_irqchip(chip, irq_chip, irq, NULL);
+ bitmap_clear(chip->irq.valid_mask, 0, GPE0A_PME_B0_VIRT_GPIO_PIN);
return 0;
}
