| Message ID | 1575451330-11112-2-git-send-email-peng.fan@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] gpio: mvebu: use platform_irq_count | expand |
On Wed, Dec 04, 2019 at 09:24:39AM +0000, Peng Fan wrote: > From: Peng Fan <peng.fan@nxp.com> > > platform_irq_count() is the more generic way (independent of > device trees) to determine the count of available interrupts. So > use this instead. > > As platform_irq_count() might return an error code (which > of_irq_count doesn't) some additional handling is necessary. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V3: > Use %pe Great. Note that with %pe there is a dependency on commit 57f5677e535b ("printf: add support for printing symbolic error names") which was applied during the current merge window. > V2: > Update commit log, and add err handling > Not tested, just code inspection > > > drivers/gpio/gpio-bcm-kona.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > index 4122683eb1f9..baee8c3f06ad 100644 > --- a/drivers/gpio/gpio-bcm-kona.c > +++ b/drivers/gpio/gpio-bcm-kona.c > @@ -19,7 +19,6 @@ > #include <linux/io.h> > #include <linux/gpio/driver.h> > #include <linux/of_device.h> > -#include <linux/of_irq.h> > #include <linux/init.h> > #include <linux/irqdomain.h> > #include <linux/irqchip/chained_irq.h> > @@ -586,11 +585,18 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev) > > kona_gpio->gpio_chip = template_chip; > chip = &kona_gpio->gpio_chip; > - kona_gpio->num_bank = of_irq_count(dev->of_node); > - if (kona_gpio->num_bank == 0) { > + ret = platform_irq_count(pdev); > + if (!ret) { > dev_err(dev, "Couldn't determine # GPIO banks\n"); > return -ENOENT; > + } else if (ret < 0) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Couldn't determine GPIO banks: (%pe)\n", > + ERR_PTR(ret)); I'd say drop either the colon or the parenthesis. Best regards Uwe
> Subject: Re: [PATCH v3 2/2] gpio: bcm-kona: use platform_irq_count > > On Wed, Dec 04, 2019 at 09:24:39AM +0000, Peng Fan wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > platform_irq_count() is the more generic way (independent of device > > trees) to determine the count of available interrupts. So use this > > instead. > > > > As platform_irq_count() might return an error code (which of_irq_count > > doesn't) some additional handling is necessary. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V3: > > Use %pe > > Great. Note that with %pe there is a dependency on commit 57f5677e535b > ("printf: add support for printing symbolic error names") which was applied > during the current merge window. > > > V2: > > Update commit log, and add err handling Not tested, just code > > inspection > > > > > > drivers/gpio/gpio-bcm-kona.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..baee8c3f06ad > 100644 > > --- a/drivers/gpio/gpio-bcm-kona.c > > +++ b/drivers/gpio/gpio-bcm-kona.c > > @@ -19,7 +19,6 @@ > > #include <linux/io.h> > > #include <linux/gpio/driver.h> > > #include <linux/of_device.h> > > -#include <linux/of_irq.h> > > #include <linux/init.h> > > #include <linux/irqdomain.h> > > #include <linux/irqchip/chained_irq.h> @@ -586,11 +585,18 @@ static > > int bcm_kona_gpio_probe(struct platform_device *pdev) > > > > kona_gpio->gpio_chip = template_chip; > > chip = &kona_gpio->gpio_chip; > > - kona_gpio->num_bank = of_irq_count(dev->of_node); > > - if (kona_gpio->num_bank == 0) { > > + ret = platform_irq_count(pdev); > > + if (!ret) { > > dev_err(dev, "Couldn't determine # GPIO banks\n"); > > return -ENOENT; > > + } else if (ret < 0) { > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, "Couldn't determine GPIO banks: (%pe)\n", > > + ERR_PTR(ret)); > > I'd say drop either the colon or the parenthesis. Like this? + dev_err(dev, "Couldn't determine GPIO banks: %pe\n", Does it really matter? Thanks, Peng. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. > pengutronix.de%2F&data=02%7C01%7Cpeng.fan%40nxp.com%7C627e1 > 95be7f0467580f908d778a20d60%7C686ea1d3bc2b4c6fa92cd99c5c301635% > 7C0%7C0%7C637110509717538199&sdata=3FuRYio8G3jtly1hWuGhJGg > zqaT742y%2B2H6xOwKMvJ8%3D&reserved=0 |
śr., 4 gru 2019 o 11:09 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Wed, Dec 04, 2019 at 09:24:39AM +0000, Peng Fan wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > platform_irq_count() is the more generic way (independent of > > device trees) to determine the count of available interrupts. So > > use this instead. > > > > As platform_irq_count() might return an error code (which > > of_irq_count doesn't) some additional handling is necessary. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V3: > > Use %pe > > Great. Note that with %pe there is a dependency on commit 57f5677e535b > ("printf: add support for printing symbolic error names") which was > applied during the current merge window. > Why would %pe be better in this case? The function returned an int - why convert it to a pointer? Bart > > V2: > > Update commit log, and add err handling > > Not tested, just code inspection > > > > > > drivers/gpio/gpio-bcm-kona.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > > index 4122683eb1f9..baee8c3f06ad 100644 > > --- a/drivers/gpio/gpio-bcm-kona.c > > +++ b/drivers/gpio/gpio-bcm-kona.c > > @@ -19,7 +19,6 @@ > > #include <linux/io.h> > > #include <linux/gpio/driver.h> > > #include <linux/of_device.h> > > -#include <linux/of_irq.h> > > #include <linux/init.h> > > #include <linux/irqdomain.h> > > #include <linux/irqchip/chained_irq.h> > > @@ -586,11 +585,18 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev) > > > > kona_gpio->gpio_chip = template_chip; > > chip = &kona_gpio->gpio_chip; > > - kona_gpio->num_bank = of_irq_count(dev->of_node); > > - if (kona_gpio->num_bank == 0) { > > + ret = platform_irq_count(pdev); > > + if (!ret) { > > dev_err(dev, "Couldn't determine # GPIO banks\n"); > > return -ENOENT; > > + } else if (ret < 0) { > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, "Couldn't determine GPIO banks: (%pe)\n", > > + ERR_PTR(ret)); > > I'd say drop either the colon or the parenthesis. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
On Wed, Dec 11, 2019 at 10:30:33AM +0100, Bartosz Golaszewski wrote: > śr., 4 gru 2019 o 11:09 Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > On Wed, Dec 04, 2019 at 09:24:39AM +0000, Peng Fan wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > platform_irq_count() is the more generic way (independent of > > > device trees) to determine the count of available interrupts. So > > > use this instead. > > > > > > As platform_irq_count() might return an error code (which > > > of_irq_count doesn't) some additional handling is necessary. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > > > > V3: > > > Use %pe > > > > Great. Note that with %pe there is a dependency on commit 57f5677e535b > > ("printf: add support for printing symbolic error names") which was > > applied during the current merge window. > > > > Why would %pe be better in this case? The function returned an int - > why convert it to a pointer? The conversion to a pointer is (currently still) needed, because there is no printk facility (yet) that consumes an int error pointer and results in the respecting code. Somewhere on my todo-list is an item to fix that, but we're not there yet and so the best option is to use %pe. Best regards Uwe
śr., 11 gru 2019 o 10:49 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Wed, Dec 11, 2019 at 10:30:33AM +0100, Bartosz Golaszewski wrote: > > śr., 4 gru 2019 o 11:09 Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> napisał(a): > > > > > > On Wed, Dec 04, 2019 at 09:24:39AM +0000, Peng Fan wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > platform_irq_count() is the more generic way (independent of > > > > device trees) to determine the count of available interrupts. So > > > > use this instead. > > > > > > > > As platform_irq_count() might return an error code (which > > > > of_irq_count doesn't) some additional handling is necessary. > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > --- > > > > > > > > V3: > > > > Use %pe > > > > > > Great. Note that with %pe there is a dependency on commit 57f5677e535b > > > ("printf: add support for printing symbolic error names") which was > > > applied during the current merge window. > > > > > > > Why would %pe be better in this case? The function returned an int - > > why convert it to a pointer? > > The conversion to a pointer is (currently still) needed, because there > is no printk facility (yet) that consumes an int error pointer and > results in the respecting code. > > Somewhere on my todo-list is an item to fix that, but we're not there > yet and so the best option is to use %pe. > Fair enough, I wasn't aware of this new format modifier. Both applied. Bartosz
diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..baee8c3f06ad 100644 --- a/drivers/gpio/gpio-bcm-kona.c +++ b/drivers/gpio/gpio-bcm-kona.c @@ -19,7 +19,6 @@ #include <linux/io.h> #include <linux/gpio/driver.h> #include <linux/of_device.h> -#include <linux/of_irq.h> #include <linux/init.h> #include <linux/irqdomain.h> #include <linux/irqchip/chained_irq.h> @@ -586,11 +585,18 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev) kona_gpio->gpio_chip = template_chip; chip = &kona_gpio->gpio_chip; - kona_gpio->num_bank = of_irq_count(dev->of_node); - if (kona_gpio->num_bank == 0) { + ret = platform_irq_count(pdev); + if (!ret) { dev_err(dev, "Couldn't determine # GPIO banks\n"); return -ENOENT; + } else if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "Couldn't determine GPIO banks: (%pe)\n", + ERR_PTR(ret)); + return ret; } + kona_gpio->num_bank = ret; + if (kona_gpio->num_bank > GPIO_MAX_BANK_NUM) { dev_err(dev, "Too many GPIO banks configured (max=%d)\n", GPIO_MAX_BANK_NUM);