| Message ID | 1409928798-31895-2-git-send-email-alvin.chen@intel.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, 2014-09-05 at 07:53 -0700, Weike Chen wrote: > The Synopsys DesignWare APB GPIO driver only supports open firmware devices. > But, like Intel Quark X1000 SOC, which has a single PCI function exporting > a GPIO and an I2C controller, it is a Multifunction device. This patch is > to enable the current Synopsys DesignWare APB GPIO driver to support the > Multifunction device which exports the designware GPIO controller. Few comments below. > > Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.com> > Reviewed-by: Shevchenko, Andriy <andriy.shevchenko@intel.com> > Signed-off-by: Weike Chen <alvin.chen@intel.com> > --- > drivers/gpio/Kconfig | 1 - > drivers/gpio/gpio-dwapb.c | 233 +++++++++++++++++++++++------- > include/linux/platform_data/gpio-dwapb.h | 32 ++++ > 3 files changed, 211 insertions(+), 55 deletions(-) > create mode 100644 include/linux/platform_data/gpio-dwapb.h > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 9de1515..8250a44 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -136,7 +136,6 @@ config GPIO_DWAPB > tristate "Synopsys DesignWare APB GPIO driver" > select GPIO_GENERIC > select GENERIC_IRQ_CHIP > - depends on OF_GPIO > help > Say Y or M here to build support for the Synopsys DesignWare APB > GPIO block. > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index d6618a6..f2264a2 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -21,6 +21,7 @@ > #include <linux/of_irq.h> > #include <linux/platform_device.h> > #include <linux/spinlock.h> > +#include <linux/platform_data/gpio-dwapb.h> > > #define GPIO_SWPORTA_DR 0x00 > #define GPIO_SWPORTA_DDR 0x04 > @@ -84,11 +85,10 @@ static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs) > writel(v, gpio->regs + GPIO_INT_POLARITY); > } > > -static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) > +static u32 _dwapb_irq_handler(struct dwapb_gpio *gpio) What about dwapb_do_irq() ? > { > - struct dwapb_gpio *gpio = irq_get_handler_data(irq); > - struct irq_chip *chip = irq_desc_get_chip(desc); > u32 irq_status = readl_relaxed(gpio->regs + GPIO_INTSTATUS); > + u32 ret = irq_status; > > while (irq_status) { > int hwirq = fls(irq_status) - 1; > @@ -102,6 +102,16 @@ static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) > dwapb_toggle_trigger(gpio, hwirq); > } > > + return ret; > +} > + > +static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) > +{ > + struct dwapb_gpio *gpio = irq_get_handler_data(irq); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + > + _dwapb_irq_handler(gpio); > + > if (chip->irq_eoi) > chip->irq_eoi(irq_desc_get_irq_data(desc)); > } > @@ -207,22 +217,26 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type) > return 0; > } > > +static irqreturn_t dwapb_irq_handler_mfd(int irq, void *dev_id) > +{ > + u32 worked; > + struct dwapb_gpio *gpio = (struct dwapb_gpio *)dev_id; No need to cast explicitly from void *. > + > + worked = _dwapb_irq_handler(gpio); > + > + return worked ? IRQ_HANDLED : IRQ_NONE; > +} > + > static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > - struct dwapb_gpio_port *port) > + struct dwapb_gpio_port *port, > + struct dwapb_port_property *pp) > { > struct gpio_chip *gc = &port->bgc.gc; > - struct device_node *node = gc->of_node; > - struct irq_chip_generic *irq_gc; > + struct device_node *node = pp->node; > + struct irq_chip_generic *irq_gc = NULL; > unsigned int hwirq, ngpio = gc->ngpio; > struct irq_chip_type *ct; > - int err, irq, i; > - > - irq = irq_of_parse_and_map(node, 0); > - if (!irq) { > - dev_warn(gpio->dev, "no irq for bank %s\n", > - port->bgc.gc.of_node->full_name); > - return; > - } > + int err, i; > > gpio->domain = irq_domain_add_linear(node, ngpio, > &irq_generic_chip_ops, gpio); > @@ -269,8 +283,24 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > irq_gc->chip_types[1].type = IRQ_TYPE_EDGE_BOTH; > irq_gc->chip_types[1].handler = handle_edge_irq; > > - irq_set_chained_handler(irq, dwapb_irq_handler); > - irq_set_handler_data(irq, gpio); > + if (!pp->irq_shared) { > + irq_set_chained_handler(pp->irq, dwapb_irq_handler); > + irq_set_handler_data(pp->irq, 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, > + dwapb_irq_handler_mfd, > + IRQF_SHARED, "gpio-dwapb-mfd", gpio); > + if (err) { > + dev_err(gpio->dev, "error requesting IRQ\n"); > + irq_domain_remove(gpio->domain); > + gpio->domain = NULL; > + return; > + } > + } > > for (hwirq = 0 ; hwirq < ngpio ; hwirq++) > irq_create_mapping(gpio->domain, hwirq); > @@ -296,57 +326,42 @@ static void dwapb_irq_teardown(struct dwapb_gpio *gpio) > } > > static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, > - struct device_node *port_np, > + struct dwapb_port_property *pp, > unsigned int offs) > { > struct dwapb_gpio_port *port; > - u32 port_idx, ngpio; > void __iomem *dat, *set, *dirout; > int err; > > - if (of_property_read_u32(port_np, "reg", &port_idx) || > - port_idx >= DWAPB_MAX_PORTS) { > - dev_err(gpio->dev, "missing/invalid port index for %s\n", > - port_np->full_name); > - return -EINVAL; > - } > - > port = &gpio->ports[offs]; > port->gpio = gpio; > > - if (of_property_read_u32(port_np, "snps,nr-gpios", &ngpio)) { > - dev_info(gpio->dev, "failed to get number of gpios for %s\n", > - port_np->full_name); > - ngpio = 32; > - } > - > - dat = gpio->regs + GPIO_EXT_PORTA + (port_idx * GPIO_EXT_PORT_SIZE); > - set = gpio->regs + GPIO_SWPORTA_DR + (port_idx * GPIO_SWPORT_DR_SIZE); > + dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE); > + set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE); > dirout = gpio->regs + GPIO_SWPORTA_DDR + > - (port_idx * GPIO_SWPORT_DDR_SIZE); > + (pp->idx * GPIO_SWPORT_DDR_SIZE); > > err = bgpio_init(&port->bgc, gpio->dev, 4, dat, set, NULL, dirout, > NULL, false); > if (err) { > dev_err(gpio->dev, "failed to init gpio chip for %s\n", > - port_np->full_name); > + pp->name); > return err; > } > > - port->bgc.gc.ngpio = ngpio; > - port->bgc.gc.of_node = port_np; > +#ifdef CONFIG_OF_GPIO Do we really need this #ifdef ? of_node will be NULL anyway, or I missed something? > + port->bgc.gc.of_node = pp->node; > +#endif > + port->bgc.gc.ngpio = pp->ngpio; > + port->bgc.gc.base = pp->gpio_base; > > - /* > - * Only port A can provide interrupts in all configurations of the IP. > - */ > - if (port_idx == 0 && > - of_property_read_bool(port_np, "interrupt-controller")) > - dwapb_configure_irqs(gpio, port); > + if (pp->irq) irq == 0 is a valid hwirq (hardware irq) number. Yes, there is unlikely we have it somewhere, but still it's possible. And yes, IRQ framework doesn't work with virq == 0 (*virtual* irq), but accepts hwirq == 0. I recommend to use int type for irq line number, and recognize negative value (usually -1) as no irq needed / found. > + dwapb_configure_irqs(gpio, port, pp); > > err = gpiochip_add(&port->bgc.gc); > if (err) > dev_err(gpio->dev, "failed to register gpiochip for %s\n", > - port_np->full_name); > + pp->name); > else > port->is_registered = true; > > @@ -362,25 +377,130 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio) > gpiochip_remove(&gpio->ports[m].bgc.gc); > } > > +#ifdef CONFIG_OF_GPIO > + > +static struct dwapb_platform_data * > +dwapb_gpio_get_pdata_of(struct device *dev) > +{ > + struct device_node *node, *port_np; > + struct dwapb_platform_data *pdata; > + struct dwapb_port_property *pp; > + int nports; > + int i; > + > + node = dev->of_node; > + if (!node) > + return ERR_PTR(-ENODEV); > + > + nports = of_get_child_count(node); > + if (nports == 0) > + return ERR_PTR(-ENODEV); > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return ERR_PTR(-ENOMEM); > + > + pdata->properties = devm_kcalloc(dev, nports, sizeof(*pp), GFP_KERNEL); > + if (!pdata->properties) > + return ERR_PTR(-ENOMEM); > + > + pdata->nports = nports; > + > + i = 0; > + for_each_child_of_node(node, port_np) { > + pp = &pdata->properties[i++]; > + pp->node = port_np; > + > + if (of_property_read_u32(port_np, "reg", &pp->idx) || > + pp->idx >= DWAPB_MAX_PORTS) { > + dev_err(dev, "missing/invalid port index for %s\n", > + port_np->full_name); > + return ERR_PTR(-EINVAL); > + } > + > + if (of_property_read_u32(port_np, "snps,nr-gpios", > + &pp->ngpio)) { > + dev_info(dev, "failed to get number of gpios for %s\n", > + port_np->full_name); > + pp->ngpio = 32; > + } > + > + /* > + * Only port A can provide interrupts in all configurations of > + * the IP. > + */ > + if (pp->idx == 0 && > + of_property_read_bool(port_np, "interrupt-controller")) { > + pp->irq = irq_of_parse_and_map(port_np, 0); > + if (!pp->irq) { > + dev_warn(dev, "no irq for bank %s\n", > + port_np->full_name); > + } > + } else { > + pp->irq = 0; > + } > + > + pp->irq_shared = false; > + pp->gpio_base = -1; > + pp->name = port_np->full_name; > + } > + > + return pdata; > +} > + > +static inline void dwapb_free_pdata_of(struct device *dev, > + struct dwapb_platform_data *pdata) > +{ > + if (!pdata) > + return; > + if (pdata->properties) > + devm_kfree(dev, pdata->properties); > + devm_kfree(dev, pdata); > +} > + > +#else > + > +static inline struct dwapb_platform_data * > +dwapb_gpio_get_pdata_of(struct device *dev) > +{ > + return ERR_PTR(-ENODEV); > +} > + > +static inline void dwapb_free_pdata_of(struct device *dev, > + struct dwapb_platform_data *pdata) > +{ > +} > + > +#endif > + > static int dwapb_gpio_probe(struct platform_device *pdev) > { > + int i; > + bool is_of; is_pdata_alloc (it might be not only OF case in future). > struct resource *res; > struct dwapb_gpio *gpio; > - struct device_node *np; > int err; > - unsigned int offs = 0; > + struct device *dev = &pdev->dev; > + struct dwapb_platform_data *pdata = dev_get_platdata(dev); > + > + if (!pdata) { (*) > + pdata = dwapb_gpio_get_pdata_of(dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + is_of = true; > + } else { > + is_of = false; Instead of above three lines, how about bool is_pdata_alloc = !pdata; And (*) if (is_pdata_alloc) ... > + } > + if (!pdata->nports) > + return -ENODEV; > > gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL); > if (!gpio) > return -ENOMEM; > gpio->dev = &pdev->dev; > + gpio->nr_ports = pdata->nports; > > - gpio->nr_ports = of_get_child_count(pdev->dev.of_node); > - if (!gpio->nr_ports) { > - err = -EINVAL; > - goto out_err; > - } > - gpio->ports = devm_kzalloc(&pdev->dev, gpio->nr_ports * > + gpio->ports = devm_kcalloc(&pdev->dev, gpio->nr_ports, > sizeof(*gpio->ports), GFP_KERNEL); > if (!gpio->ports) { > err = -ENOMEM; > @@ -394,13 +514,18 @@ static int dwapb_gpio_probe(struct platform_device *pdev) > goto out_err; > } > > - for_each_child_of_node(pdev->dev.of_node, np) { > - err = dwapb_gpio_add_port(gpio, np, offs++); > + for (i = 0; i < gpio->nr_ports; i++) { > + err = dwapb_gpio_add_port(gpio, &pdata->properties[i], i); > if (err) > goto out_unregister; > } > platform_set_drvdata(pdev, gpio); > > + if (is_of) { > + dwapb_free_pdata_of(dev, pdata); > + pdata = NULL; Besides that pdata assignment is probably redundant, you may use plain kmalloc/kfree and avoid unnecessary devm_* calls. > + } > + > return 0; > > out_unregister: > diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h > new file mode 100644 > index 0000000..28702c8 > --- /dev/null > +++ b/include/linux/platform_data/gpio-dwapb.h > @@ -0,0 +1,32 @@ > +/* > + * Copyright(c) 2014 Intel Corporation. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#ifndef GPIO_DW_APB_H > +#define GPIO_DW_APB_H > + > +struct dwapb_port_property { > + struct device_node *node; > + const char *name; > + unsigned int idx; > + unsigned int ngpio; > + unsigned int gpio_base; > + unsigned int irq; > + bool irq_shared; > +}; > + > +struct dwapb_platform_data { > + struct dwapb_port_property *properties; > + unsigned int nports; > +}; > + > +#endif -- Andy Shevchenko <andriy.shevchenko@intel.com> Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
> > -static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) > > +static u32 _dwapb_irq_handler(struct dwapb_gpio *gpio) > > What about dwapb_do_irq() ? OK, I will improve it. > > +static irqreturn_t dwapb_irq_handler_mfd(int irq, void *dev_id) { > > + u32 worked; > > + struct dwapb_gpio *gpio = (struct dwapb_gpio *)dev_id; > > No need to cast explicitly from void *. OK. > > - port->bgc.gc.ngpio = ngpio; > > - port->bgc.gc.of_node = port_np; > > +#ifdef CONFIG_OF_GPIO > > Do we really need this #ifdef ? > of_node will be NULL anyway, or I missed something? Yes, otherwise, can't compile it. Please refer 'struct gpio_chip', 'gc.of_node' is in OF_GPIO micro also. > > + if (pp->irq) > > irq == 0 is a valid hwirq (hardware irq) number. Yes, there is unlikely we have it > somewhere, but still it's possible. And yes, IRQ framework doesn't work with > virq == 0 (*virtual* irq), but accepts hwirq == 0. I recommend to use int type for > irq line number, and recognize negative value (usually -1) as no irq needed / > found. Understand. But if you refer the original code, you can see: irq = irq_of_parse_and_map(node, 0); If (!irq) { ...... return; } From above code, if irq=0, it indicates irq is not supported for OF devices. If we use '-1' to indicate irq is not supported. To make OF work, then our code should be: irq = irq_of_parse_and_map(node, 0); If (!irq) { pp->irq = -1; return; } else { pp->irq = irq; } Then the code looks strange. How do you think? > > + bool is_of; > > is_pdata_alloc (it might be not only OF case in future). > OK > > + if (!pdata) { > > (*) > > + pdata = dwapb_gpio_get_pdata_of(dev); > > + if (IS_ERR(pdata)) > > + return PTR_ERR(pdata); > > > + is_of = true; > > + } else { > > + is_of = false; > > Instead of above three lines, how about > bool is_pdata_alloc = !pdata; > > And (*) if (is_pdata_alloc) ... > OK. I will improve this part. > > + if (is_of) { > > + dwapb_free_pdata_of(dev, pdata); > > + pdata = NULL; > > Besides that pdata assignment is probably redundant, you may use plain > kmalloc/kfree and avoid unnecessary devm_* calls. > OK.
On Friday 05 September 2014 07:53:16 Weike Chen wrote: > > - irq_set_chained_handler(irq, dwapb_irq_handler); > - irq_set_handler_data(irq, gpio); > + if (!pp->irq_shared) { > + irq_set_chained_handler(pp->irq, dwapb_irq_handler); > + irq_set_handler_data(pp->irq, 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, > + dwapb_irq_handler_mfd, > + IRQF_SHARED, "gpio-dwapb-mfd", gpio); > + if (err) { > + dev_err(gpio->dev, "error requesting IRQ\n"); > + irq_domain_remove(gpio->domain); > + gpio->domain = NULL; > + return; > + } > + } > I think this need some better documentation. Why is it safe to use devm_request_irq rather than irq_set_chained_handler here? > > +#ifdef CONFIG_OF_GPIO > + > +static struct dwapb_platform_data * > +dwapb_gpio_get_pdata_of(struct device *dev) > +{ > + struct device_node *node, *port_np; > + struct dwapb_platform_data *pdata; > + struct dwapb_port_property *pp; > + int nports; > + int i; > + > + node = dev->of_node; > + if (!node) > + return ERR_PTR(-ENODEV); Please replace the #ifdef above with if (!IS_ENABLED(CONFIG_OF_GPIO) || !node) here so get you proper compile-time coverage of the DT code path. Arnd -- 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
T24gRnJpLCAyMDE0LTA5LTA1IGF0IDEwOjIwICswMDAwLCBDaGVuLCBBbHZpbiB3cm90ZToNCj4g PiA+IC0JcG9ydC0+YmdjLmdjLm5ncGlvID0gbmdwaW87DQo+ID4gPiAtCXBvcnQtPmJnYy5nYy5v Zl9ub2RlID0gcG9ydF9ucDsNCj4gPiA+ICsjaWZkZWYgQ09ORklHX09GX0dQSU8NCj4gPiANCj4g PiBEbyB3ZSByZWFsbHkgbmVlZCB0aGlzICNpZmRlZiA/DQo+ID4gb2Zfbm9kZSB3aWxsIGJlIE5V TEwgYW55d2F5LCBvciBJIG1pc3NlZCBzb21ldGhpbmc/DQo+IFllcywgb3RoZXJ3aXNlLCBjYW4n dCBjb21waWxlIGl0LiBQbGVhc2UgcmVmZXIgJ3N0cnVjdCBncGlvX2NoaXAnLCAnZ2Mub2Zfbm9k ZScgaXMgaW4gT0ZfR1BJTyBtaWNybyBhbHNvLg0KDQpBaCwgb2theS4gVGh1cywgaXQgZGVwZW5k cyB0byBMaW51cyBvcGluaW9uLCBzaW5jZSBJLCBmb3IgZXhhbXBsZSwgd291bGQNCmxpa2UgdG8g c2VlIHRoaXMgZmllbGQgcHJlc2VudCBpbiB0aGUgc3RydWN0dXJlIGluZGVwZW5kZW50bHkgb2YN Ck9GX0dQSU8uDQoNCj4gDQo+ID4gPiArCWlmIChwcC0+aXJxKQ0KPiA+IA0KPiA+IGlycSA9PSAw IGlzIGEgdmFsaWQgaHdpcnEgKGhhcmR3YXJlIGlycSkgbnVtYmVyLiBZZXMsIHRoZXJlIGlzIHVu bGlrZWx5IHdlIGhhdmUgaXQNCj4gPiBzb21ld2hlcmUsIGJ1dCBzdGlsbCBpdCdzIHBvc3NpYmxl LiBBbmQgeWVzLCBJUlEgZnJhbWV3b3JrIGRvZXNuJ3Qgd29yayB3aXRoDQo+ID4gdmlycSA9PSAw ICgqdmlydHVhbCogaXJxKSwgYnV0IGFjY2VwdHMgaHdpcnEgPT0gMC4gSSByZWNvbW1lbmQgdG8g dXNlIGludCB0eXBlIGZvcg0KPiA+IGlycSBsaW5lIG51bWJlciwgYW5kIHJlY29nbml6ZSBuZWdh dGl2ZSB2YWx1ZSAodXN1YWxseSAtMSkgYXMgbm8gaXJxIG5lZWRlZCAvDQo+ID4gZm91bmQuDQo+ IFVuZGVyc3RhbmQuIEJ1dCBpZiB5b3UgcmVmZXIgdGhlIG9yaWdpbmFsIGNvZGUsIHlvdSBjYW4g c2VlOg0KPiBpcnEgPSBpcnFfb2ZfcGFyc2VfYW5kX21hcChub2RlLCAwKTsNCj4gSWYgKCFpcnEp IHsNCj4gLi4uLi4uDQo+IHJldHVybjsNCj4gfQ0KPiBGcm9tIGFib3ZlIGNvZGUsIGlmIGlycT0w LCBpdCBpbmRpY2F0ZXMgaXJxIGlzIG5vdCBzdXBwb3J0ZWQgZm9yIE9GIGRldmljZXMuIElmIHdl IHVzZSAnLTEnIHRvIGluZGljYXRlIGlycSBpcyBub3Qgc3VwcG9ydGVkLiBUbyBtYWtlIE9GIHdv cmssIHRoZW4gb3VyIGNvZGUgc2hvdWxkIGJlOg0KDQpZZXMsIGxpa2UgSSBzYWlkIGFib3ZlLiBZ b3UgaW50cm9kdWNlIGh3IGlycSBpbiB0aGUgcGRhdGEsIHdoaWNoIGNvdWxkDQpiZSAwLg0KDQoN Cj4gaXJxID0gaXJxX29mX3BhcnNlX2FuZF9tYXAobm9kZSwgMCk7DQo+IElmICghaXJxKSB7DQo+ IHBwLT5pcnEgPSAtMTsNCj4gcmV0dXJuOw0KPiB9IGVsc2Ugew0KPiBwcC0+aXJxID0gaXJxOw0K PiB9DQo+IFRoZW4gdGhlIGNvZGUgbG9va3Mgc3RyYW5nZS4NCj4gDQo+IEhvdyBkbyB5b3UgdGhp bms/DQoNCklmIEkgdW5kZXJzdG9vZCBjb3JyZWN0bHkgeW91IG1lc3NlZCB1cCB3aXRoIGh3aXJx IHZzLiB2aXJxLg0KT3RoZXJ3aXNlIHlvdSBoYXZlIG1lbnRpb24gdGhhdCB5b3UgYXJlIHVzaW5n IHZpcnEgZXZlcnl3aGVyZSAoSSBndWVzcw0KeW91IG1heSByZW5hbWUgdGhlIGZpZWxkIGluIHRo ZSBzdHJ1Y3R1cmUpLCBidXQgaW4gdGhpcyBjYXNlIHRoZSBmaWVsZA0KaW4gdGhlIHBsYXRmb3Jt X2RhdGEgbG9va3MgYSBiaXQgc3RyYW5nZS4gTGludXMsIHdoYXQgZG8geW91IHRoaW5rPw0KDQoN ClAuUy4gUGxlYXNlLCByZW1vdmUgbXkgUmV2aWV3ZWQtYnkgdGFnIHNpbmNlIGNvZGUgaXMgY2hh bmdlZCBlbm91Z2guDQoNCi0tIA0KQW5keSBTaGV2Y2hlbmtvIDxhbmRyaXkuc2hldmNoZW5rb0Bp bnRlbC5jb20+DQpJbnRlbCBGaW5sYW5kIE95DQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KSW50ZWwgRmlubGFuZCBP eQpSZWdpc3RlcmVkIEFkZHJlc3M6IFBMIDI4MSwgMDAxODEgSGVsc2lua2kgCkJ1c2luZXNzIElk ZW50aXR5IENvZGU6IDAzNTc2MDYgLSA0IApEb21pY2lsZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUt bWFpbCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlh bCBmb3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocykuIEFueSByZXZp ZXcgb3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5 b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2Vu ZGVyIGFuZCBkZWxldGUgYWxsIGNvcGllcy4K -- 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 Friday 05 September 2014 12:02:01 Shevchenko, Andriy wrote: > > irq = irq_of_parse_and_map(node, 0); > > If (!irq) { > > pp->irq = -1; > > return; > > } else { > > pp->irq = irq; > > } > > Then the code looks strange. > > > > How do you think? > > If I understood correctly you messed up with hwirq vs. virq. > Otherwise you have mention that you are using virq everywhere (I guess > you may rename the field in the structure), but in this case the field > in the platform_data looks a bit strange. The field in platform_data should be the mapped virtual irq number, it makes no sense to use the hwirq unless you also add a pointer to the domain in which that hwirq exists. Also the output of irq_of_parse_and_map() is a mapped irq, as the name suggests. Arnd -- 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 2014-09-05 13:50:06 [+0200], Arnd Bergmann wrote: > On Friday 05 September 2014 07:53:16 Weike Chen wrote: > > > > - irq_set_chained_handler(irq, dwapb_irq_handler); > > - irq_set_handler_data(irq, gpio); > > + if (!pp->irq_shared) { > > + irq_set_chained_handler(pp->irq, dwapb_irq_handler); > > + irq_set_handler_data(pp->irq, 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, > > + dwapb_irq_handler_mfd, > > + IRQF_SHARED, "gpio-dwapb-mfd", gpio); > > + if (err) { > > + dev_err(gpio->dev, "error requesting IRQ\n"); > > + irq_domain_remove(gpio->domain); > > + gpio->domain = NULL; > > + return; > > + } > > + } > > > > I think this need some better documentation. Why is it safe to use > devm_request_irq rather than irq_set_chained_handler here? Usually it is preferred to use irq_set_chained_handler() for the chained handler so the handler does not show up in /proc/interrupts. This requires an exclusive non-shared handler which is not the case on the intel platform. So they have to use devm_request_irq() instead. Sebastian -- 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
> > > > > > - irq_set_chained_handler(irq, dwapb_irq_handler); > > > - irq_set_handler_data(irq, gpio); > > > + if (!pp->irq_shared) { > > > + irq_set_chained_handler(pp->irq, dwapb_irq_handler); > > > + irq_set_handler_data(pp->irq, 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, > > > + dwapb_irq_handler_mfd, > > > + IRQF_SHARED, "gpio-dwapb-mfd", gpio); > > > + if (err) { > > > + dev_err(gpio->dev, "error requesting IRQ\n"); > > > + irq_domain_remove(gpio->domain); > > > + gpio->domain = NULL; > > > + return; > > > + } > > > + } > > > > > > > I think this need some better documentation. Why is it safe to use > > devm_request_irq rather than irq_set_chained_handler here? > > Usually it is preferred to use irq_set_chained_handler() for the chained handler > so the handler does not show up in /proc/interrupts. > This requires an exclusive non-shared handler which is not the case on the intel > platform. So they have to use devm_request_irq() instead. > Yes, for Intel Quark, it has a single PCI function exporting a GPIO and I2C controller, and the irq is shared by GPIO and I2C, so we need shared irq as the comments said. -- 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
> > > > +#ifdef CONFIG_OF_GPIO > > + > > +static struct dwapb_platform_data * > > +dwapb_gpio_get_pdata_of(struct device *dev) { > > + struct device_node *node, *port_np; > > + struct dwapb_platform_data *pdata; > > + struct dwapb_port_property *pp; > > + int nports; > > + int i; > > + > > + node = dev->of_node; > > + if (!node) > > + return ERR_PTR(-ENODEV); > > Please replace the #ifdef above with > > if (!IS_ENABLED(CONFIG_OF_GPIO) || !node) > > here so get you proper compile-time coverage of the DT code path. OK, I will improve 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 Friday 05 September 2014 12:02:01 Shevchenko, Andriy wrote: > > > irq = irq_of_parse_and_map(node, 0); If (!irq) { > > > pp->irq = -1; > > > return; > > > } else { > > > pp->irq = irq; > > > } > > > Then the code looks strange. > > > > > > How do you think? > > > > If I understood correctly you messed up with hwirq vs. virq. > > Otherwise you have mention that you are using virq everywhere (I guess > > you may rename the field in the structure), but in this case the field > > in the platform_data looks a bit strange. > > The field in platform_data should be the mapped virtual irq number, it makes no > sense to use the hwirq unless you also add a pointer to the domain in which > that hwirq exists. > > Also the output of irq_of_parse_and_map() is a mapped irq, as the name > suggests. > I agree with Arnd. Here, the 'irq' is 'virq'. Andriy, you may be confused by the code like 'irq_create_mapping'. For Quark case, it has 8 GPIO pins, and each pin can trigger interrupt, but all these interrupts are triggered by PCI irq which is shared. The 'irq' in pdata is PCI irq. As all GPIO interrupts connect to the PCI irq, once the GPIO interrupt is triggered, and the PCI irq handler will be called 'dwapb_irq_handler_mfd'. And in 'dwapb_do_irq', it will read the interrupt register to see the interrupt is triggered by which GPIO pin, and 'irq_create_mapping' is for this case. . -- 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
T24gVHVlLCAyMDE0LTA5LTA5IGF0IDAxOjUwICswMDAwLCBDaGVuLCBBbHZpbiB3cm90ZToNCj4g PiBPbiBGcmlkYXkgMDUgU2VwdGVtYmVyIDIwMTQgMTI6MDI6MDEgU2hldmNoZW5rbywgQW5kcml5 IHdyb3RlOg0KPiA+ID4gPiBpcnEgPSBpcnFfb2ZfcGFyc2VfYW5kX21hcChub2RlLCAwKTsgSWYg KCFpcnEpIHsNCj4gPiA+ID4gcHAtPmlycSA9IC0xOw0KPiA+ID4gPiByZXR1cm47DQo+ID4gPiA+ IH0gZWxzZSB7DQo+ID4gPiA+IHBwLT5pcnEgPSBpcnE7DQo+ID4gPiA+IH0NCj4gPiA+ID4gVGhl biB0aGUgY29kZSBsb29rcyBzdHJhbmdlLg0KPiA+ID4gPg0KPiA+ID4gPiBIb3cgZG8geW91IHRo aW5rPw0KPiA+ID4NCj4gPiA+IElmIEkgdW5kZXJzdG9vZCBjb3JyZWN0bHkgeW91IG1lc3NlZCB1 cCB3aXRoIGh3aXJxIHZzLiB2aXJxLg0KPiA+ID4gT3RoZXJ3aXNlIHlvdSBoYXZlIG1lbnRpb24g dGhhdCB5b3UgYXJlIHVzaW5nIHZpcnEgZXZlcnl3aGVyZSAoSSBndWVzcw0KPiA+ID4geW91IG1h eSByZW5hbWUgdGhlIGZpZWxkIGluIHRoZSBzdHJ1Y3R1cmUpLCBidXQgaW4gdGhpcyBjYXNlIHRo ZSBmaWVsZA0KPiA+ID4gaW4gdGhlIHBsYXRmb3JtX2RhdGEgbG9va3MgYSBiaXQgc3RyYW5nZS4N Cj4gPiANCj4gPiBUaGUgZmllbGQgaW4gcGxhdGZvcm1fZGF0YSBzaG91bGQgYmUgdGhlIG1hcHBl ZCB2aXJ0dWFsIGlycSBudW1iZXIsIGl0IG1ha2VzIG5vDQo+ID4gc2Vuc2UgdG8gdXNlIHRoZSBo d2lycSB1bmxlc3MgeW91IGFsc28gYWRkIGEgcG9pbnRlciB0byB0aGUgZG9tYWluIGluIHdoaWNo DQo+ID4gdGhhdCBod2lycSBleGlzdHMuDQo+ID4gDQo+ID4gQWxzbyB0aGUgb3V0cHV0IG9mIGly cV9vZl9wYXJzZV9hbmRfbWFwKCkgaXMgYSBtYXBwZWQgaXJxLCBhcyB0aGUgbmFtZQ0KPiA+IHN1 Z2dlc3RzLg0KPiA+IA0KPiBJIGFncmVlIHdpdGggQXJuZC4gSGVyZSwgdGhlICdpcnEnIGlzICd2 aXJxJy4NCj4gQW5kcml5LCB5b3UgbWF5IGJlIGNvbmZ1c2VkIGJ5IHRoZSBjb2RlIGxpa2UgJ2ly cV9jcmVhdGVfbWFwcGluZycuIEZvciBRdWFyayBjYXNlLCBpdCBoYXMgOCBHUElPIHBpbnMsIGFu ZCBlYWNoIHBpbiBjYW4gdHJpZ2dlcg0KPiBpbnRlcnJ1cHQsIGJ1dCBhbGwgdGhlc2UgaW50ZXJy dXB0cyBhcmUgdHJpZ2dlcmVkIGJ5IFBDSSBpcnEgd2hpY2ggaXMgc2hhcmVkLiBUaGUgJ2lycScg aW4gcGRhdGEgaXMgUENJIGlycS4gQXMgYWxsIEdQSU8gaW50ZXJydXB0cyBjb25uZWN0IHRvIHRo ZSBQQ0kNCj4gaXJxLCBvbmNlIHRoZSBHUElPIGludGVycnVwdCBpcyB0cmlnZ2VyZWQsIGFuZCB0 aGUgUENJIGlycSBoYW5kbGVyIHdpbGwgYmUgY2FsbGVkICdkd2FwYl9pcnFfaGFuZGxlcl9tZmQn LiBBbmQgaW4gJ2R3YXBiX2RvX2lycScsIGl0IHdpbGwgcmVhZCB0aGUNCj4gaW50ZXJydXB0IHJl Z2lzdGVyIHRvIHNlZSB0aGUgaW50ZXJydXB0IGlzIHRyaWdnZXJlZCBieSB3aGljaCBHUElPIHBp biwgYW5kICdpcnFfY3JlYXRlX21hcHBpbmcnIGlzIGZvciB0aGlzIGNhc2UuDQoNClRoYW5rcyBm b3IgY2xhcmlmaWNhdGlvbi4gSSB0YWtlIGJhY2sgbXkgaW5pdGlhbCBjb21tZW50IHRoZW4uDQoN Ci0tIA0KQW5keSBTaGV2Y2hlbmtvIDxhbmRyaXkuc2hldmNoZW5rb0BpbnRlbC5jb20+DQpJbnRl bCBGaW5sYW5kIE95DQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KSW50ZWwgRmlubGFuZCBPeQpSZWdpc3RlcmVkIEFk ZHJlc3M6IFBMIDI4MSwgMDAxODEgSGVsc2lua2kgCkJ1c2luZXNzIElkZW50aXR5IENvZGU6IDAz NTc2MDYgLSA0IApEb21pY2lsZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUtbWFpbCBhbmQgYW55IGF0 dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlhbCBmb3IKdGhlIHNvbGUg dXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocykuIEFueSByZXZpZXcgb3IgZGlzdHJpYnV0 aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5b3UgYXJlIG5vdCB0aGUg aW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2VuZGVyIGFuZCBkZWxldGUg YWxsIGNvcGllcy4K -- 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/gpio/Kconfig b/drivers/gpio/Kconfig index 9de1515..8250a44 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -136,7 +136,6 @@ config GPIO_DWAPB tristate "Synopsys DesignWare APB GPIO driver" select GPIO_GENERIC select GENERIC_IRQ_CHIP - depends on OF_GPIO help Say Y or M here to build support for the Synopsys DesignWare APB GPIO block. diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index d6618a6..f2264a2 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -21,6 +21,7 @@ #include <linux/of_irq.h> #include <linux/platform_device.h> #include <linux/spinlock.h> +#include <linux/platform_data/gpio-dwapb.h> #define GPIO_SWPORTA_DR 0x00 #define GPIO_SWPORTA_DDR 0x04 @@ -84,11 +85,10 @@ static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs) writel(v, gpio->regs + GPIO_INT_POLARITY); } -static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) +static u32 _dwapb_irq_handler(struct dwapb_gpio *gpio) { - struct dwapb_gpio *gpio = irq_get_handler_data(irq); - struct irq_chip *chip = irq_desc_get_chip(desc); u32 irq_status = readl_relaxed(gpio->regs + GPIO_INTSTATUS); + u32 ret = irq_status; while (irq_status) { int hwirq = fls(irq_status) - 1; @@ -102,6 +102,16 @@ static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) dwapb_toggle_trigger(gpio, hwirq); } + return ret; +} + +static void dwapb_irq_handler(u32 irq, struct irq_desc *desc) +{ + struct dwapb_gpio *gpio = irq_get_handler_data(irq); + struct irq_chip *chip = irq_desc_get_chip(desc); + + _dwapb_irq_handler(gpio); + if (chip->irq_eoi) chip->irq_eoi(irq_desc_get_irq_data(desc)); } @@ -207,22 +217,26 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type) return 0; } +static irqreturn_t dwapb_irq_handler_mfd(int irq, void *dev_id) +{ + u32 worked; + struct dwapb_gpio *gpio = (struct dwapb_gpio *)dev_id; + + worked = _dwapb_irq_handler(gpio); + + return worked ? IRQ_HANDLED : IRQ_NONE; +} + static void dwapb_configure_irqs(struct dwapb_gpio *gpio, - struct dwapb_gpio_port *port) + struct dwapb_gpio_port *port, + struct dwapb_port_property *pp) { struct gpio_chip *gc = &port->bgc.gc; - struct device_node *node = gc->of_node; - struct irq_chip_generic *irq_gc; + struct device_node *node = pp->node; + struct irq_chip_generic *irq_gc = NULL; unsigned int hwirq, ngpio = gc->ngpio; struct irq_chip_type *ct; - int err, irq, i; - - irq = irq_of_parse_and_map(node, 0); - if (!irq) { - dev_warn(gpio->dev, "no irq for bank %s\n", - port->bgc.gc.of_node->full_name); - return; - } + int err, i; gpio->domain = irq_domain_add_linear(node, ngpio, &irq_generic_chip_ops, gpio); @@ -269,8 +283,24 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, irq_gc->chip_types[1].type = IRQ_TYPE_EDGE_BOTH; irq_gc->chip_types[1].handler = handle_edge_irq; - irq_set_chained_handler(irq, dwapb_irq_handler); - irq_set_handler_data(irq, gpio); + if (!pp->irq_shared) { + irq_set_chained_handler(pp->irq, dwapb_irq_handler); + irq_set_handler_data(pp->irq, 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, + dwapb_irq_handler_mfd, + IRQF_SHARED, "gpio-dwapb-mfd", gpio); + if (err) { + dev_err(gpio->dev, "error requesting IRQ\n"); + irq_domain_remove(gpio->domain); + gpio->domain = NULL; + return; + } + } for (hwirq = 0 ; hwirq < ngpio ; hwirq++) irq_create_mapping(gpio->domain, hwirq); @@ -296,57 +326,42 @@ static void dwapb_irq_teardown(struct dwapb_gpio *gpio) } static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, - struct device_node *port_np, + struct dwapb_port_property *pp, unsigned int offs) { struct dwapb_gpio_port *port; - u32 port_idx, ngpio; void __iomem *dat, *set, *dirout; int err; - if (of_property_read_u32(port_np, "reg", &port_idx) || - port_idx >= DWAPB_MAX_PORTS) { - dev_err(gpio->dev, "missing/invalid port index for %s\n", - port_np->full_name); - return -EINVAL; - } - port = &gpio->ports[offs]; port->gpio = gpio; - if (of_property_read_u32(port_np, "snps,nr-gpios", &ngpio)) { - dev_info(gpio->dev, "failed to get number of gpios for %s\n", - port_np->full_name); - ngpio = 32; - } - - dat = gpio->regs + GPIO_EXT_PORTA + (port_idx * GPIO_EXT_PORT_SIZE); - set = gpio->regs + GPIO_SWPORTA_DR + (port_idx * GPIO_SWPORT_DR_SIZE); + dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE); + set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE); dirout = gpio->regs + GPIO_SWPORTA_DDR + - (port_idx * GPIO_SWPORT_DDR_SIZE); + (pp->idx * GPIO_SWPORT_DDR_SIZE); err = bgpio_init(&port->bgc, gpio->dev, 4, dat, set, NULL, dirout, NULL, false); if (err) { dev_err(gpio->dev, "failed to init gpio chip for %s\n", - port_np->full_name); + pp->name); return err; } - port->bgc.gc.ngpio = ngpio; - port->bgc.gc.of_node = port_np; +#ifdef CONFIG_OF_GPIO + port->bgc.gc.of_node = pp->node; +#endif + port->bgc.gc.ngpio = pp->ngpio; + port->bgc.gc.base = pp->gpio_base; - /* - * Only port A can provide interrupts in all configurations of the IP. - */ - if (port_idx == 0 && - of_property_read_bool(port_np, "interrupt-controller")) - dwapb_configure_irqs(gpio, port); + if (pp->irq) + dwapb_configure_irqs(gpio, port, pp); err = gpiochip_add(&port->bgc.gc); if (err) dev_err(gpio->dev, "failed to register gpiochip for %s\n", - port_np->full_name); + pp->name); else port->is_registered = true; @@ -362,25 +377,130 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio) gpiochip_remove(&gpio->ports[m].bgc.gc); } +#ifdef CONFIG_OF_GPIO + +static struct dwapb_platform_data * +dwapb_gpio_get_pdata_of(struct device *dev) +{ + struct device_node *node, *port_np; + struct dwapb_platform_data *pdata; + struct dwapb_port_property *pp; + int nports; + int i; + + node = dev->of_node; + if (!node) + return ERR_PTR(-ENODEV); + + nports = of_get_child_count(node); + if (nports == 0) + return ERR_PTR(-ENODEV); + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + pdata->properties = devm_kcalloc(dev, nports, sizeof(*pp), GFP_KERNEL); + if (!pdata->properties) + return ERR_PTR(-ENOMEM); + + pdata->nports = nports; + + i = 0; + for_each_child_of_node(node, port_np) { + pp = &pdata->properties[i++]; + pp->node = port_np; + + if (of_property_read_u32(port_np, "reg", &pp->idx) || + pp->idx >= DWAPB_MAX_PORTS) { + dev_err(dev, "missing/invalid port index for %s\n", + port_np->full_name); + return ERR_PTR(-EINVAL); + } + + if (of_property_read_u32(port_np, "snps,nr-gpios", + &pp->ngpio)) { + dev_info(dev, "failed to get number of gpios for %s\n", + port_np->full_name); + pp->ngpio = 32; + } + + /* + * Only port A can provide interrupts in all configurations of + * the IP. + */ + if (pp->idx == 0 && + of_property_read_bool(port_np, "interrupt-controller")) { + pp->irq = irq_of_parse_and_map(port_np, 0); + if (!pp->irq) { + dev_warn(dev, "no irq for bank %s\n", + port_np->full_name); + } + } else { + pp->irq = 0; + } + + pp->irq_shared = false; + pp->gpio_base = -1; + pp->name = port_np->full_name; + } + + return pdata; +} + +static inline void dwapb_free_pdata_of(struct device *dev, + struct dwapb_platform_data *pdata) +{ + if (!pdata) + return; + if (pdata->properties) + devm_kfree(dev, pdata->properties); + devm_kfree(dev, pdata); +} + +#else + +static inline struct dwapb_platform_data * +dwapb_gpio_get_pdata_of(struct device *dev) +{ + return ERR_PTR(-ENODEV); +} + +static inline void dwapb_free_pdata_of(struct device *dev, + struct dwapb_platform_data *pdata) +{ +} + +#endif + static int dwapb_gpio_probe(struct platform_device *pdev) { + int i; + bool is_of; struct resource *res; struct dwapb_gpio *gpio; - struct device_node *np; int err; - unsigned int offs = 0; + struct device *dev = &pdev->dev; + struct dwapb_platform_data *pdata = dev_get_platdata(dev); + + if (!pdata) { + pdata = dwapb_gpio_get_pdata_of(dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + is_of = true; + } else { + is_of = false; + } + if (!pdata->nports) + return -ENODEV; gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL); if (!gpio) return -ENOMEM; gpio->dev = &pdev->dev; + gpio->nr_ports = pdata->nports; - gpio->nr_ports = of_get_child_count(pdev->dev.of_node); - if (!gpio->nr_ports) { - err = -EINVAL; - goto out_err; - } - gpio->ports = devm_kzalloc(&pdev->dev, gpio->nr_ports * + gpio->ports = devm_kcalloc(&pdev->dev, gpio->nr_ports, sizeof(*gpio->ports), GFP_KERNEL); if (!gpio->ports) { err = -ENOMEM; @@ -394,13 +514,18 @@ static int dwapb_gpio_probe(struct platform_device *pdev) goto out_err; } - for_each_child_of_node(pdev->dev.of_node, np) { - err = dwapb_gpio_add_port(gpio, np, offs++); + for (i = 0; i < gpio->nr_ports; i++) { + err = dwapb_gpio_add_port(gpio, &pdata->properties[i], i); if (err) goto out_unregister; } platform_set_drvdata(pdev, gpio); + if (is_of) { + dwapb_free_pdata_of(dev, pdata); + pdata = NULL; + } + return 0; out_unregister: diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h new file mode 100644 index 0000000..28702c8 --- /dev/null +++ b/include/linux/platform_data/gpio-dwapb.h @@ -0,0 +1,32 @@ +/* + * Copyright(c) 2014 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#ifndef GPIO_DW_APB_H +#define GPIO_DW_APB_H + +struct dwapb_port_property { + struct device_node *node; + const char *name; + unsigned int idx; + unsigned int ngpio; + unsigned int gpio_base; + unsigned int irq; + bool irq_shared; +}; + +struct dwapb_platform_data { + struct dwapb_port_property *properties; + unsigned int nports; +}; + +#endif