Message ID | 20200512184513.86883-4-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers | show |
Series | [v1,1/4] gpio: dwapb: avoid error message for optional IRQ | expand |
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20200512]
[cannot apply to linus/master v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-a003-20200512 (attached as .config)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':
>> drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq'
pdata->properties->has_irq = true;
^~
vim +219 drivers/mfd/intel_quark_i2c_gpio.c
60ae5b9f5cdd80 Raymond Tan 2015-02-02 189
60ae5b9f5cdd80 Raymond Tan 2015-02-02 190 static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
60ae5b9f5cdd80 Raymond Tan 2015-02-02 191 {
60ae5b9f5cdd80 Raymond Tan 2015-02-02 192 struct dwapb_platform_data *pdata;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 193 struct resource *res = (struct resource *)cell->resources;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 194 struct device *dev = &pdev->dev;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 195
60ae5b9f5cdd80 Raymond Tan 2015-02-02 196 res[INTEL_QUARK_IORES_MEM].start =
60ae5b9f5cdd80 Raymond Tan 2015-02-02 197 pci_resource_start(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80 Raymond Tan 2015-02-02 198 res[INTEL_QUARK_IORES_MEM].end =
60ae5b9f5cdd80 Raymond Tan 2015-02-02 199 pci_resource_end(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80 Raymond Tan 2015-02-02 200
60ae5b9f5cdd80 Raymond Tan 2015-02-02 201 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
60ae5b9f5cdd80 Raymond Tan 2015-02-02 202 if (!pdata)
60ae5b9f5cdd80 Raymond Tan 2015-02-02 203 return -ENOMEM;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 204
60ae5b9f5cdd80 Raymond Tan 2015-02-02 205 /* For intel quark x1000, it has only one port: portA */
60ae5b9f5cdd80 Raymond Tan 2015-02-02 206 pdata->nports = INTEL_QUARK_GPIO_NPORTS;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 207 pdata->properties = devm_kcalloc(dev, pdata->nports,
60ae5b9f5cdd80 Raymond Tan 2015-02-02 208 sizeof(*pdata->properties),
60ae5b9f5cdd80 Raymond Tan 2015-02-02 209 GFP_KERNEL);
60ae5b9f5cdd80 Raymond Tan 2015-02-02 210 if (!pdata->properties)
60ae5b9f5cdd80 Raymond Tan 2015-02-02 211 return -ENOMEM;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 212
60ae5b9f5cdd80 Raymond Tan 2015-02-02 213 /* Set the properties for portA */
4ba8cfa79f44a9 Jiang Qiu 2016-04-28 214 pdata->properties->fwnode = NULL;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 215 pdata->properties->idx = 0;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 216 pdata->properties->ngpio = INTEL_QUARK_MFD_NGPIO;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 217 pdata->properties->gpio_base = INTEL_QUARK_MFD_GPIO_BASE;
e6ca26abd37606 Phil Edworthy 2018-04-26 218 pdata->properties->irq[0] = pdev->irq;
e6ca26abd37606 Phil Edworthy 2018-04-26 @219 pdata->properties->has_irq = true;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 220 pdata->properties->irq_shared = true;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 221
60ae5b9f5cdd80 Raymond Tan 2015-02-02 222 cell->platform_data = pdata;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 223 cell->pdata_size = sizeof(*pdata);
60ae5b9f5cdd80 Raymond Tan 2015-02-02 224
60ae5b9f5cdd80 Raymond Tan 2015-02-02 225 return 0;
60ae5b9f5cdd80 Raymond Tan 2015-02-02 226 }
60ae5b9f5cdd80 Raymond Tan 2015-02-02 227
:::::: The code at line 219 was first introduced by commit
:::::: e6ca26abd37606ba4864f20c85d3fe4a2173b93f gpio: dwapb: Add support for 1 interrupt per port A GPIO
:::::: TO: Phil Edworthy <phil.edworthy@renesas.com>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, May 13, 2020 at 06:53:09AM +0800, kbuild test robot wrote: > Hi Andy, > > I love your patch! Yet something to improve: > > [auto build test ERROR on gpio/for-next] > [also build test ERROR on next-20200512] > [cannot apply to linus/master v5.7-rc5] > [if your patch is applied to the wrong git tree, please drop us a note to help > improve the system. BTW, we also suggest to use '--base' option to specify the > base tree in git format-patch, please see https://stackoverflow.com/a/37406982] > > url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227 > base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next > config: x86_64-randconfig-a003-20200512 (attached as .config) > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install x86_64 cross compiling tool for clang build > # apt-get install binutils-x86-64-linux-gnu > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kbuild test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup': > >> drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq' > pdata->properties->has_irq = true; > ^~ Indeed. I have that removed by other WIP patch in my tree. Thanks!
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20200512]
[cannot apply to linus/master v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':
>> drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq'
pdata->properties->has_irq = true;
^~
# https://github.com/0day-ci/linux/commit/f8686f0bb4ff7fdded832484a559f1454a2a51d2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f8686f0bb4ff7fdded832484a559f1454a2a51d2
vim +219 drivers/mfd/intel_quark_i2c_gpio.c
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 189
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 190 static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 191 {
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 192 struct dwapb_platform_data *pdata;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 193 struct resource *res = (struct resource *)cell->resources;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 194 struct device *dev = &pdev->dev;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 195
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 196 res[INTEL_QUARK_IORES_MEM].start =
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 197 pci_resource_start(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 198 res[INTEL_QUARK_IORES_MEM].end =
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 199 pci_resource_end(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 200
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 201 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 202 if (!pdata)
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 203 return -ENOMEM;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 204
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 205 /* For intel quark x1000, it has only one port: portA */
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 206 pdata->nports = INTEL_QUARK_GPIO_NPORTS;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 207 pdata->properties = devm_kcalloc(dev, pdata->nports,
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 208 sizeof(*pdata->properties),
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 209 GFP_KERNEL);
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 210 if (!pdata->properties)
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 211 return -ENOMEM;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 212
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 213 /* Set the properties for portA */
4ba8cfa79f44a948 Jiang Qiu 2016-04-28 214 pdata->properties->fwnode = NULL;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 215 pdata->properties->idx = 0;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 216 pdata->properties->ngpio = INTEL_QUARK_MFD_NGPIO;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 217 pdata->properties->gpio_base = INTEL_QUARK_MFD_GPIO_BASE;
e6ca26abd37606ba Phil Edworthy 2018-04-26 218 pdata->properties->irq[0] = pdev->irq;
e6ca26abd37606ba Phil Edworthy 2018-04-26 @219 pdata->properties->has_irq = true;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 220 pdata->properties->irq_shared = true;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 221
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 222 cell->platform_data = pdata;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 223 cell->pdata_size = sizeof(*pdata);
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 224
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 225 return 0;
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 226 }
60ae5b9f5cdd80c5 Raymond Tan 2015-02-02 227
:::::: The code at line 219 was first introduced by commit
:::::: e6ca26abd37606ba4864f20c85d3fe4a2173b93f gpio: dwapb: Add support for 1 interrupt per port A GPIO
:::::: TO: Phil Edworthy <phil.edworthy@renesas.com>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, May 12, 2020 at 09:45:13PM +0300, Andy Shevchenko wrote: > has_irq member of struct dwapb_port_property is used only in one place, > so, make it local test instead and remove from the structure. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Serge Semin <fancer.lancer@gmail.com> > --- > drivers/gpio/gpio-dwapb.c | 12 +++++++----- > include/linux/platform_data/gpio-dwapb.h | 1 - > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 2975f2d369874a..87834adccc4534 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -366,6 +366,11 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > irq_hw_number_t hwirq; > int err, i; > > + if (memchr_inv(pp->irq, 0, sizeof(pp->irq)) == NULL) { > + dev_warn(gpio->dev, "no IRQ for port%d\n", pp->idx); > + return; > + } > + Ah, that's why you added the memchr_inv() method in patch 2. So to move it to dwapb_configure_irqs() at this point. Anyway I still think, that it would be better to leave the has_irq initialization in the loop there, but here you could just remove that assignment. For this patch: Reviewed-by: Serge Semin <fancer.lancer@gmail.com> I'll test the whole series up when you send v2. Our hardware is equipped with two DW APB GPIO IPs with Port A enabled for each. One of them is connected to an interrupt controller by a single line. -Sergey > gpio->domain = irq_domain_create_linear(fwnode, ngpio, > &irq_generic_chip_ops, gpio); > if (!gpio->domain) > @@ -501,7 +506,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, > if (pp->idx == 0) > port->gc.set_config = dwapb_gpio_set_config; > > - if (pp->has_irq) > + /* Only port A can provide interrupts in all configurations of the IP */ > + if (pp->idx == 0) > dwapb_configure_irqs(gpio, port, pp); > > err = gpiochip_add_data(&port->gc, port); > @@ -553,10 +559,6 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, > > pp->irq[j] = err; > } > - > - pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL; > - if (!pp->has_irq) > - dev_warn(dev, "no irq for port%d\n", pp->idx); > } > > static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev) > diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h > index 3c606c450d0596..ff1be737bad6aa 100644 > --- a/include/linux/platform_data/gpio-dwapb.h > +++ b/include/linux/platform_data/gpio-dwapb.h > @@ -12,7 +12,6 @@ struct dwapb_port_property { > unsigned int ngpio; > unsigned int gpio_base; > int irq[32]; > - bool has_irq; > bool irq_shared; > }; > > -- > 2.26.2 >
On Sun, May 17, 2020 at 05:26:30PM +0300, Serge Semin wrote: > On Tue, May 12, 2020 at 09:45:13PM +0300, Andy Shevchenko wrote: > > has_irq member of struct dwapb_port_property is used only in one place, > > so, make it local test instead and remove from the structure. ... > > + if (memchr_inv(pp->irq, 0, sizeof(pp->irq)) == NULL) { > > + dev_warn(gpio->dev, "no IRQ for port%d\n", pp->idx); > > + return; > > + } > > + > > Ah, that's why you added the memchr_inv() method in patch 2. So to move it > to dwapb_configure_irqs() at this point. Yes. > Anyway I still think, that it would be > better to leave the has_irq initialization in the loop there, but here you could > just remove that assignment. I think you noticed that I don't like to ping-pong in the series, for what you propose it would be something like original --->>> if (pp->irq[j] >= 0) pp->has_irq = true; after patch 2 (if your suggestion applied) --->>> if (irq > 0) { pp->irq[j] = irq; pp->has->irq = true; } after this patch --->>> if (irq > 0) pp->irq[j] = irq; I prefer not to do this. OTOH, I guess it might work if we leave original conditional separate to assignment of IRQ itself (with '>= 0' -> '> 0' being replaced). I'll look what can I do here. > For this patch: > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com> Thank you! > I'll test the whole series up when you send v2. Our hardware is equipped with > two DW APB GPIO IPs with Port A enabled for each. One of them is connected to an > interrupt controller by a single line.
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index 2975f2d369874a..87834adccc4534 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -366,6 +366,11 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, irq_hw_number_t hwirq; int err, i; + if (memchr_inv(pp->irq, 0, sizeof(pp->irq)) == NULL) { + dev_warn(gpio->dev, "no IRQ for port%d\n", pp->idx); + return; + } + gpio->domain = irq_domain_create_linear(fwnode, ngpio, &irq_generic_chip_ops, gpio); if (!gpio->domain) @@ -501,7 +506,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, if (pp->idx == 0) port->gc.set_config = dwapb_gpio_set_config; - if (pp->has_irq) + /* Only port A can provide interrupts in all configurations of the IP */ + if (pp->idx == 0) dwapb_configure_irqs(gpio, port, pp); err = gpiochip_add_data(&port->gc, port); @@ -553,10 +559,6 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, pp->irq[j] = err; } - - pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL; - if (!pp->has_irq) - dev_warn(dev, "no irq for port%d\n", pp->idx); } static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev) diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h index 3c606c450d0596..ff1be737bad6aa 100644 --- a/include/linux/platform_data/gpio-dwapb.h +++ b/include/linux/platform_data/gpio-dwapb.h @@ -12,7 +12,6 @@ struct dwapb_port_property { unsigned int ngpio; unsigned int gpio_base; int irq[32]; - bool has_irq; bool irq_shared; };
has_irq member of struct dwapb_port_property is used only in one place, so, make it local test instead and remove from the structure. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Serge Semin <fancer.lancer@gmail.com> --- drivers/gpio/gpio-dwapb.c | 12 +++++++----- include/linux/platform_data/gpio-dwapb.h | 1 - 2 files changed, 7 insertions(+), 6 deletions(-)