| Message ID | 1523968977-4241-1-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Tue, Apr 17, 2018 at 7:42 AM, Phil Edworthy <phil.edworthy@renesas.com> wrote: > The DesignWare GPIO IP can be configured for either 1 interrupt or 1 > per GPIO in port A, but the driver currently only supports 1 interrupt. > See the DesignWare DW_apb_gpio Databook description of the > 'GPIO_INTR_IO' parameter. > > This change allows the driver to work with up to 32 interrupts, it will > get as many interrupts as specified in the DT 'interrupts' property. > It doesn't do anything clever with the different interrupts, it just calls > the same handler used for single interrupt hardware. > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > One point to mention is that I have made it possible for users to have > unconncted interrupts by specifying holes in the list of interrupts. This is > done by supporting the interrupts-extended DT prop. > However, I have no use for this and had to hack some test case for this. > Perhaps the driver should support 1 interrupt or all GPIOa as interrupts? > > v4: > - Use of_irq_get() instead of of_irq_parse_one()+irq_create_of_mapping() > v3: > - Rolled mfd: intel_quark_i2c_gpio fix into this patch to avoid bisect problems > v2: > - Replaced interrupt-mask DT prop with support for the interrupts-extended > prop. This means replacing the call to irq_of_parse_and_map() with calls > to of_irq_parse_one() and irq_create_of_mapping(). > > Note: There are a few *code* lines over 80 chars, but this is just guidance, > right? Especially as there are already some lines over 80 chars. > > snps:gpio fix > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > .../devicetree/bindings/gpio/snps-dwapb-gpio.txt | 9 +++-- > drivers/gpio/gpio-dwapb.c | 42 +++++++++++++++++----- > drivers/mfd/intel_quark_i2c_gpio.c | 3 +- > include/linux/platform_data/gpio-dwapb.h | 3 +- > 4 files changed, 44 insertions(+), 13 deletions(-) Reviewed-by: Rob Herring <robh@kernel.org>
On Tue, 17 Apr 2018, Phil Edworthy wrote: > The DesignWare GPIO IP can be configured for either 1 interrupt or 1 > per GPIO in port A, but the driver currently only supports 1 interrupt. > See the DesignWare DW_apb_gpio Databook description of the > 'GPIO_INTR_IO' parameter. > > This change allows the driver to work with up to 32 interrupts, it will > get as many interrupts as specified in the DT 'interrupts' property. > It doesn't do anything clever with the different interrupts, it just calls > the same handler used for single interrupt hardware. > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > One point to mention is that I have made it possible for users to have > unconncted interrupts by specifying holes in the list of interrupts. This is > done by supporting the interrupts-extended DT prop. > However, I have no use for this and had to hack some test case for this. > Perhaps the driver should support 1 interrupt or all GPIOa as interrupts? > > v4: > - Use of_irq_get() instead of of_irq_parse_one()+irq_create_of_mapping() > v3: > - Rolled mfd: intel_quark_i2c_gpio fix into this patch to avoid bisect problems > v2: > - Replaced interrupt-mask DT prop with support for the interrupts-extended > prop. This means replacing the call to irq_of_parse_and_map() with calls > to of_irq_parse_one() and irq_create_of_mapping(). > > Note: There are a few *code* lines over 80 chars, but this is just guidance, > right? Especially as there are already some lines over 80 chars. > > snps:gpio fix > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > .../devicetree/bindings/gpio/snps-dwapb-gpio.txt | 9 +++-- > drivers/gpio/gpio-dwapb.c | 42 +++++++++++++++++----- > drivers/mfd/intel_quark_i2c_gpio.c | 3 +- If Linus, is happy with the GPIO implementation, then the changes in MFD look fine: Acked-by: Lee Jones <lee.jones@linaro.org>
diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt index 4a75da7..3c1118b 100644 --- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt @@ -26,8 +26,13 @@ controller. the second encodes the triger flags encoded as described in Documentation/devicetree/bindings/interrupt-controller/interrupts.txt - interrupt-parent : The parent interrupt controller. -- interrupts : The interrupt to the parent controller raised when GPIOs - generate the interrupts. +- interrupts : The interrupts to the parent controller raised when GPIOs + generate the interrupts. If the controller provides one combined interrupt + for all GPIOs, specify a single interrupt. If the controller provides one + interrupt for each GPIO, provide a list of interrupts that correspond to each + of the GPIO pins. When specifying multiple interrupts, if any are unconnected, + use the interrupts-extended property to specify the interrupts and set the + interrupt controller handle for unused interrupts to 0. - snps,nr-gpios : The number of pins in the port, a single cell. - resets : Reset line for the controller. diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index 226977f..7a1022f 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -441,14 +441,19 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, irq_gc->chip_types[1].handler = handle_edge_irq; if (!pp->irq_shared) { - irq_set_chained_handler_and_data(pp->irq, dwapb_irq_handler, - gpio); + int i; + + for (i = 0; i < pp->ngpio; i++) { + if (pp->irq[i]) + irq_set_chained_handler_and_data(pp->irq[i], + dwapb_irq_handler, gpio); + } } else { /* * Request a shared IRQ since where MFD would have devices * using the same irq pin */ - err = devm_request_irq(gpio->dev, pp->irq, + err = devm_request_irq(gpio->dev, pp->irq[0], dwapb_irq_handler_mfd, IRQF_SHARED, "gpio-dwapb-mfd", gpio); if (err) { @@ -524,7 +529,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, if (pp->idx == 0) port->gc.set_config = dwapb_gpio_set_config; - if (pp->irq) + if (pp->has_irq) dwapb_configure_irqs(gpio, port, pp); err = gpiochip_add_data(&port->gc, port); @@ -535,7 +540,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, port->is_registered = true; /* Add GPIO-signaled ACPI event support */ - if (pp->irq) + if (pp->has_irq) acpi_gpiochip_request_interrupts(&port->gc); return err; @@ -601,13 +606,32 @@ dwapb_gpio_get_pdata(struct device *dev) if (dev->of_node && pp->idx == 0 && fwnode_property_read_bool(fwnode, "interrupt-controller")) { - pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0); - if (!pp->irq) + struct device_node *np = to_of_node(fwnode); + unsigned int j; + + /* + * The IP has configuration options to allow a single + * combined interrupt or one per gpio. If one per gpio, + * some might not be used. + */ + for (j = 0; j < pp->ngpio; j++) { + int irq = of_irq_get(np, j); + if (irq < 0) + continue; + + pp->irq[j] = irq; + pp->has_irq = true; + } + + if (!pp->has_irq) dev_warn(dev, "no irq for port%d\n", pp->idx); } - if (has_acpi_companion(dev) && pp->idx == 0) - pp->irq = platform_get_irq(to_platform_device(dev), 0); + if (has_acpi_companion(dev) && pp->idx == 0) { + pp->irq[0] = platform_get_irq(to_platform_device(dev), 0); + if (pp->irq[0]) + pp->has_irq = true; + } pp->irq_shared = false; pp->gpio_base = -1; diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c index 90e35de..5bddb84 100644 --- a/drivers/mfd/intel_quark_i2c_gpio.c +++ b/drivers/mfd/intel_quark_i2c_gpio.c @@ -233,7 +233,8 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell) pdata->properties->idx = 0; pdata->properties->ngpio = INTEL_QUARK_MFD_NGPIO; pdata->properties->gpio_base = INTEL_QUARK_MFD_GPIO_BASE; - pdata->properties->irq = pdev->irq; + pdata->properties->irq[0] = pdev->irq; + pdata->properties->has_irq = true; pdata->properties->irq_shared = true; cell->platform_data = pdata; diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h index 2dc7f4a..5a52d69 100644 --- a/include/linux/platform_data/gpio-dwapb.h +++ b/include/linux/platform_data/gpio-dwapb.h @@ -19,7 +19,8 @@ struct dwapb_port_property { unsigned int idx; unsigned int ngpio; unsigned int gpio_base; - unsigned int irq; + unsigned int irq[32]; + bool has_irq; bool irq_shared; };