Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753054AbcCJTJ5 (ORCPT ); Thu, 10 Mar 2016 14:09:57 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33848 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750942AbcCJTJs (ORCPT ); Thu, 10 Mar 2016 14:09:48 -0500 MIME-Version: 1.0 In-Reply-To: <1457077474-124064-2-git-send-email-qiujiang@huawei.com> References: <1457077474-124064-1-git-send-email-qiujiang@huawei.com> <1457077474-124064-2-git-send-email-qiujiang@huawei.com> Date: Thu, 10 Mar 2016 13:09:46 -0600 Message-ID: Subject: Re: [PATCH v5 1/3] gpio: designware: convert device node to fwnode From: Alan Tull To: qiujiang Cc: Linus Walleij , Alexandre Courbot , mika.westerberg@linux.intel.com, andy.shevchenko@gmail.com, linux-kernel , "linux-gpio@vger.kernel.org" , linux-acpi@vger.kernel.org, linuxarm@huawei.com, haifeng.wei@huawei.com, charles.chenxin@huawei.com, Alan Tull Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6656 Lines: 164 On Fri, Mar 4, 2016 at 1:44 AM, qiujiang wrote: > This patch converts device node to fwnode in > dwapb_port_property for designware gpio driver, > so as to provide a unified data structure for DT > and ACPI bindings. > > Acked-by: Andy Shevchenko > Signed-off-by: qiujiang > --- > drivers/gpio/gpio-dwapb.c | 43 +++++++++++++++----------------- > drivers/mfd/intel_quark_i2c_gpio.c | 2 +- > include/linux/platform_data/gpio-dwapb.h | 2 +- > 3 files changed, 22 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 597de1e..49f6e5d 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -290,14 +291,14 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > struct dwapb_port_property *pp) > { > struct gpio_chip *gc = &port->gc; > - struct device_node *node = pp->node; > + struct fwnode_handle *fwnode = pp->fwnode; > struct irq_chip_generic *irq_gc = NULL; > unsigned int hwirq, ngpio = gc->ngpio; > struct irq_chip_type *ct; > int err, i; > > - gpio->domain = irq_domain_add_linear(node, ngpio, > - &irq_generic_chip_ops, gpio); > + gpio->domain = irq_domain_create_linear(fwnode, ngpio, > + &irq_generic_chip_ops, gpio); > if (!gpio->domain) > return; > > @@ -415,7 +416,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, > } > > #ifdef CONFIG_OF_GPIO > - port->gc.of_node = pp->node; > + port->gc.of_node = is_of_node(pp->fwnode) ? > + to_of_node(pp->fwnode) : NULL; > #endif > port->gc.ngpio = pp->ngpio; > port->gc.base = pp->gpio_base; > @@ -449,17 +451,13 @@ static void dwapb_gpio_unregister(struct dwapb_gpio *gpio) > static struct dwapb_platform_data * > dwapb_gpio_get_pdata_of(struct device *dev) > { > - struct device_node *node, *port_np; > + struct fwnode_handle *fwnode; > struct dwapb_platform_data *pdata; > struct dwapb_port_property *pp; > int nports; > int i; > > - node = dev->of_node; > - if (!IS_ENABLED(CONFIG_OF_GPIO) || !node) > - return ERR_PTR(-ENODEV); > - > - nports = of_get_child_count(node); > + nports = device_get_child_node_count(dev); > if (nports == 0) > return ERR_PTR(-ENODEV); > > @@ -474,21 +472,19 @@ dwapb_gpio_get_pdata_of(struct device *dev) > pdata->nports = nports; > > i = 0; > - for_each_child_of_node(node, port_np) { > + device_for_each_child_node(dev, fwnode) { > pp = &pdata->properties[i++]; > - pp->node = port_np; > + pp->fwnode = fwnode; > > - if (of_property_read_u32(port_np, "reg", &pp->idx) || > + if (fwnode_property_read_u32(fwnode, "reg", &pp->idx) || > pp->idx >= DWAPB_MAX_PORTS) { > - dev_err(dev, "missing/invalid port index for %s\n", > - port_np->full_name); > + dev_err(dev, "missing/invalid port index\n"); > return ERR_PTR(-EINVAL); > } > > - if (of_property_read_u32(port_np, "snps,nr-gpios", > + if (fwnode_property_read_u32(fwnode, "snps,nr-gpios", > &pp->ngpio)) { > - dev_info(dev, "failed to get number of gpios for %s\n", > - port_np->full_name); > + dev_info(dev, "failed to get number of gpios\n"); > pp->ngpio = 32; > } > > @@ -496,18 +492,19 @@ dwapb_gpio_get_pdata_of(struct device *dev) > * 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 (dev->of_node && pp->idx == 0 && > + of_property_read_bool(to_of_node(fwnode), > + "interrupt-controller")) { Hi Qiujiang, Is there a reason to use "of_property_read_bool" here instead of "device_property_read_bool" or similar? Alan > + pp->irq = irq_of_parse_and_map(to_of_node(fwnode), 0); > if (!pp->irq) { > dev_warn(dev, "no irq for bank %s\n", > - port_np->full_name); > + to_of_node(fwnode)->full_name); > } > } > > pp->irq_shared = false; > pp->gpio_base = -1; > - pp->name = port_np->full_name; > + pp->name = to_of_node(fwnode)->full_name; > } > > return pdata; > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c > index 0421374..265bd3c 100644 > --- a/drivers/mfd/intel_quark_i2c_gpio.c > +++ b/drivers/mfd/intel_quark_i2c_gpio.c > @@ -227,7 +227,7 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell) > return -ENOMEM; > > /* Set the properties for portA */ > - pdata->properties->node = NULL; > + pdata->properties->fwnode = NULL; > pdata->properties->name = "intel-quark-x1000-gpio-portA"; > pdata->properties->idx = 0; > pdata->properties->ngpio = INTEL_QUARK_MFD_NGPIO; > diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h > index 28702c8..c5bd1f2 100644 > --- a/include/linux/platform_data/gpio-dwapb.h > +++ b/include/linux/platform_data/gpio-dwapb.h > @@ -15,7 +15,7 @@ > #define GPIO_DW_APB_H > > struct dwapb_port_property { > - struct device_node *node; > + struct fwnode_handle *fwnode; > const char *name; > unsigned int idx; > unsigned int ngpio; > -- > 1.9.1 >