| Message ID | 20191022210128.12042-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: intel_int0002_vgpio: Pass irqchip when adding gpiochip | expand |
On Wed, Oct 23, 2019 at 12:03 AM Linus Walleij <linus.walleij@linaro.org> 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. This driver requests the IRQ directly in the driver > so it needs to pass a NULL parent handler. We may revisit this > code later and pull reqular shared IRQ handler into > gpiolib, so leave a FIXME. > Thanks! Code looks fine to me, though I will wait for Hans to confirm it doesn't break anything. > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/platform/x86/intel_int0002_vgpio.c | 28 +++++++++++----------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c > index af233b7b77f2..f14e2c5f9da5 100644 > --- a/drivers/platform/x86/intel_int0002_vgpio.c > +++ b/drivers/platform/x86/intel_int0002_vgpio.c > @@ -164,8 +164,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> */ > @@ -192,15 +192,11 @@ static int int0002_probe(struct platform_device *pdev) > chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1; > chip->irq.init_valid_mask = int0002_init_irq_valid_mask; > > - ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL); > - if (ret) { > - dev_err(dev, "Error adding gpio chip: %d\n", ret); > - return ret; > - } > - > /* > - * We manually request the irq here instead of passing a flow-handler > + * We directly request the irq here instead of passing a flow-handler > * to gpiochip_set_chained_irqchip, because the irq is shared. > + * FIXME: augment this if we managed to pull handling of shared > + * IRQs into gpiolib. > */ > ret = devm_request_irq(dev, irq, int0002_irq, > IRQF_SHARED, "INT0002", chip); > @@ -209,17 +205,21 @@ static int int0002_probe(struct platform_device *pdev) > return ret; > } > > - irq_chip = (struct irq_chip *)cpu_id->driver_data; > + girq = &chip->irq; > + girq->chip = (struct irq_chip *)cpu_id->driver_data; > + /* This let us handle the parent IRQ in the driver */ > + girq->parent_handler = NULL; > + girq->num_parents = 0; > + girq->parents = NULL; > + girq->default_type = IRQ_TYPE_NONE; > + girq->handler = handle_edge_irq; > > - ret = gpiochip_irqchip_add(chip, irq_chip, 0, handle_edge_irq, > - IRQ_TYPE_NONE); > + ret = devm_gpiochip_add_data(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); > - > device_init_wakeup(dev, true); > return 0; > } > -- > 2.21.0 >
Hi, On 22-10-2019 23:01, 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. This driver requests the IRQ directly in the driver > so it needs to pass a NULL parent handler. We may revisit this > code later and pull reqular shared IRQ handler into > gpiolib, so leave a FIXME. > > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> I've tested this on a CHT device which supports wake by USB keyboard through the INT0002 vGPIO device. Everything still works fine: Tested-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/platform/x86/intel_int0002_vgpio.c | 28 +++++++++++----------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c > index af233b7b77f2..f14e2c5f9da5 100644 > --- a/drivers/platform/x86/intel_int0002_vgpio.c > +++ b/drivers/platform/x86/intel_int0002_vgpio.c > @@ -164,8 +164,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> */ > @@ -192,15 +192,11 @@ static int int0002_probe(struct platform_device *pdev) > chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1; > chip->irq.init_valid_mask = int0002_init_irq_valid_mask; > > - ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL); > - if (ret) { > - dev_err(dev, "Error adding gpio chip: %d\n", ret); > - return ret; > - } > - > /* > - * We manually request the irq here instead of passing a flow-handler > + * We directly request the irq here instead of passing a flow-handler > * to gpiochip_set_chained_irqchip, because the irq is shared. > + * FIXME: augment this if we managed to pull handling of shared > + * IRQs into gpiolib. > */ > ret = devm_request_irq(dev, irq, int0002_irq, > IRQF_SHARED, "INT0002", chip); > @@ -209,17 +205,21 @@ static int int0002_probe(struct platform_device *pdev) > return ret; > } > > - irq_chip = (struct irq_chip *)cpu_id->driver_data; > + girq = &chip->irq; > + girq->chip = (struct irq_chip *)cpu_id->driver_data; > + /* This let us handle the parent IRQ in the driver */ > + girq->parent_handler = NULL; > + girq->num_parents = 0; > + girq->parents = NULL; > + girq->default_type = IRQ_TYPE_NONE; > + girq->handler = handle_edge_irq; > > - ret = gpiochip_irqchip_add(chip, irq_chip, 0, handle_edge_irq, > - IRQ_TYPE_NONE); > + ret = devm_gpiochip_add_data(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); > - > device_init_wakeup(dev, true); > return 0; > } >
diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c index af233b7b77f2..f14e2c5f9da5 100644 --- a/drivers/platform/x86/intel_int0002_vgpio.c +++ b/drivers/platform/x86/intel_int0002_vgpio.c @@ -164,8 +164,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> */ @@ -192,15 +192,11 @@ static int int0002_probe(struct platform_device *pdev) chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1; chip->irq.init_valid_mask = int0002_init_irq_valid_mask; - ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL); - if (ret) { - dev_err(dev, "Error adding gpio chip: %d\n", ret); - return ret; - } - /* - * We manually request the irq here instead of passing a flow-handler + * We directly request the irq here instead of passing a flow-handler * to gpiochip_set_chained_irqchip, because the irq is shared. + * FIXME: augment this if we managed to pull handling of shared + * IRQs into gpiolib. */ ret = devm_request_irq(dev, irq, int0002_irq, IRQF_SHARED, "INT0002", chip); @@ -209,17 +205,21 @@ static int int0002_probe(struct platform_device *pdev) return ret; } - irq_chip = (struct irq_chip *)cpu_id->driver_data; + girq = &chip->irq; + girq->chip = (struct irq_chip *)cpu_id->driver_data; + /* This let us handle the parent IRQ in the driver */ + girq->parent_handler = NULL; + girq->num_parents = 0; + girq->parents = NULL; + girq->default_type = IRQ_TYPE_NONE; + girq->handler = handle_edge_irq; - ret = gpiochip_irqchip_add(chip, irq_chip, 0, handle_edge_irq, - IRQ_TYPE_NONE); + ret = devm_gpiochip_add_data(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); - device_init_wakeup(dev, true); return 0; }
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. This driver requests the IRQ directly in the driver so it needs to pass a NULL parent handler. We may revisit this code later and pull reqular shared IRQ handler into gpiolib, so leave a FIXME. Cc: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/platform/x86/intel_int0002_vgpio.c | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-)