* [PATCH 1/2] gpio: mvebu: use platform_irq_count @ 2019-12-03 6:04 Peng Fan 2019-12-03 6:04 ` [PATCH 2/2] gpio: bcm-kona: " Peng Fan 2019-12-12 15:29 ` [PATCH 1/2] gpio: mvebu: " Linus Walleij 0 siblings, 2 replies; 12+ messages in thread From: Peng Fan @ 2019-12-03 6:04 UTC (permalink / raw) To: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh Cc: bcm-kernel-feedback-list, u.kleine-koenig, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo, Peng Fan From: Peng Fan <peng.fan@nxp.com> Use platform_irq_count to replace of_irq_count Signed-off-by: Peng Fan <peng.fan@nxp.com> --- V1: Code inspection, not tested drivers/gpio/gpio-mvebu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index 993bbeb3c006..ecfedcdadc0d 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -46,7 +46,6 @@ #include <linux/irqdomain.h> #include <linux/mfd/syscon.h> #include <linux/of_device.h> -#include <linux/of_irq.h> #include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #include <linux/pwm.h> @@ -1102,7 +1101,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev) soc_variant = MVEBU_GPIO_SOC_VARIANT_ORION; /* Some gpio controllers do not provide irq support */ - have_irqs = of_irq_count(np) != 0; + have_irqs = platform_irq_count(pdev) != 0; mvchip = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_gpio_chip), GFP_KERNEL); -- 2.16.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 6:04 [PATCH 1/2] gpio: mvebu: use platform_irq_count Peng Fan @ 2019-12-03 6:04 ` Peng Fan 2019-12-03 6:57 ` Uwe Kleine-König 2019-12-12 15:29 ` [PATCH 1/2] gpio: mvebu: " Linus Walleij 1 sibling, 1 reply; 12+ messages in thread From: Peng Fan @ 2019-12-03 6:04 UTC (permalink / raw) To: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh Cc: bcm-kernel-feedback-list, u.kleine-koenig, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo, Peng Fan From: Peng Fan <peng.fan@nxp.com> Use platform_irq_count to replace of_irq_count Signed-off-by: Peng Fan <peng.fan@nxp.com> --- V1: Code inspection, not tested drivers/gpio/gpio-bcm-kona.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..c50721980a7c 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,7 +585,7 @@ 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); + kona_gpio->num_bank = platform_irq_count(pdev); if (kona_gpio->num_bank == 0) { dev_err(dev, "Couldn't determine # GPIO banks\n"); return -ENOENT; -- 2.16.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 6:04 ` [PATCH 2/2] gpio: bcm-kona: " Peng Fan @ 2019-12-03 6:57 ` Uwe Kleine-König 2019-12-03 7:00 ` Peng Fan 2019-12-03 9:54 ` Bartosz Golaszewski 0 siblings, 2 replies; 12+ messages in thread From: Uwe Kleine-König @ 2019-12-03 6:57 UTC (permalink / raw) To: Peng Fan Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > From: Peng Fan <peng.fan@nxp.com> > > Use platform_irq_count to replace of_irq_count > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V1: > Code inspection, not tested > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > index 4122683eb1f9..c50721980a7c 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,7 +585,7 @@ 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); > + kona_gpio->num_bank = platform_irq_count(pdev); of_irq_count returns 0 or a positive int while platform_irq_count might return a negative error code. This needs handling. Also I wonder why platform_irq_count() is better than of_irq_count() which would be good to describe in the commit log. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 6:57 ` Uwe Kleine-König @ 2019-12-03 7:00 ` Peng Fan 2019-12-03 7:15 ` Uwe Kleine-König 2019-12-03 9:54 ` Bartosz Golaszewski 1 sibling, 1 reply; 12+ messages in thread From: Peng Fan @ 2019-12-03 7:00 UTC (permalink / raw) To: Uwe Kleine-König Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Use platform_irq_count to replace of_irq_count > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V1: > > Code inspection, not tested > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..c50721980a7c > 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,7 +585,7 @@ 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); > > + kona_gpio->num_bank = platform_irq_count(pdev); > > of_irq_count returns 0 or a positive int while platform_irq_count might return > a negative error code. This needs handling. Also I wonder why > platform_irq_count() is better than of_irq_count() which would be good to > describe in the commit log. Inspired from https://lkml.org/lkml/2015/11/18/466 From Rob: " So I started looking at why you are using of_irq_count which drivers shouldn't need to. In patch 5 you use it to allocate memory to store the irq names, then use them here... " Is this ok? 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%7C36e86 > 59eca384830eee508d777be21cc%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0%7C0%7C637109530809227993&sdata=y95EHGlmw12IVVlWoiUiS > cQzO7GIBjQmnAfYrtd7P6k%3D&reserved=0 | ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 7:00 ` Peng Fan @ 2019-12-03 7:15 ` Uwe Kleine-König 2019-12-03 7:23 ` Peng Fan 0 siblings, 1 reply; 12+ messages in thread From: Uwe Kleine-König @ 2019-12-03 7:15 UTC (permalink / raw) To: Peng Fan Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo On Tue, Dec 03, 2019 at 07:00:40AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > Use platform_irq_count to replace of_irq_count > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > > > > V1: > > > Code inspection, not tested > > > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > > b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..c50721980a7c > > 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,7 +585,7 @@ 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); > > > + kona_gpio->num_bank = platform_irq_count(pdev); > > > > of_irq_count returns 0 or a positive int while platform_irq_count might return > > a negative error code. This needs handling. Also I wonder why > > platform_irq_count() is better than of_irq_count() which would be good to > > describe in the commit log. > > Inspired from https://lkml.org/lkml/2015/11/18/466 > From Rob: > " > So I started looking at why you are using of_irq_count which drivers > shouldn't need to. In patch 5 you use it to allocate memory to store > the irq names, then use them here... > " > > Is this ok? I would say something like: 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. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 7:15 ` Uwe Kleine-König @ 2019-12-03 7:23 ` Peng Fan 2019-12-03 7:38 ` Uwe Kleine-König 0 siblings, 1 reply; 12+ messages in thread From: Peng Fan @ 2019-12-03 7:23 UTC (permalink / raw) To: Uwe Kleine-König Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > On Tue, Dec 03, 2019 at 07:00:40AM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > Use platform_irq_count to replace of_irq_count > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > --- > > > > > > > > V1: > > > > Code inspection, not tested > > > > > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > > > b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..c50721980a7c > > > 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,7 +585,7 @@ 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); > > > > + kona_gpio->num_bank = platform_irq_count(pdev); > > > > > > of_irq_count returns 0 or a positive int while platform_irq_count > > > might return a negative error code. This needs handling. Also I > > > wonder why > > > platform_irq_count() is better than of_irq_count() which would be > > > good to describe in the commit log. > > > > Inspired from > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml > > .org%2Flkml%2F2015%2F11%2F18%2F466&data=02%7C01%7Cpeng.f > an%40nxp.c > > > om%7C46364bf12479463df7d308d777c09b39%7C686ea1d3bc2b4c6fa92cd9 > 9c5c3016 > > > 35%7C0%7C0%7C637109541440912984&sdata=awTvwwmo4692Hx7IJ > H%2BllEVJH7 > > ngINufoMH8UsosU%2BA%3D&reserved=0 > > From Rob: > > " > > So I started looking at why you are using of_irq_count which drivers > > shouldn't need to. In patch 5 you use it to allocate memory to store > > the irq names, then use them here... > > " > > > > Is this ok? > > I would say something like: > > 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. Thanks, how about this change? @@ -586,11 +585,15 @@ 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) { + return ret; } + kona_gpio->num_bank = ret; + if (kona_gpio->num_bank > GPIO_MAX_BANK_NUM) { 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%7C46364 > bf12479463df7d308d777c09b39%7C686ea1d3bc2b4c6fa92cd99c5c301635% > 7C0%7C0%7C637109541440922981&sdata=8zAFDgy56%2BjS4I4dUvB6 > y37AMHBu1PDdzESa6c2jUlc%3D&reserved=0 | ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 7:23 ` Peng Fan @ 2019-12-03 7:38 ` Uwe Kleine-König 2019-12-03 7:40 ` Peng Fan 0 siblings, 1 reply; 12+ messages in thread From: Uwe Kleine-König @ 2019-12-03 7:38 UTC (permalink / raw) To: Peng Fan Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo On Tue, Dec 03, 2019 at 07:23:14AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > On Tue, Dec 03, 2019 at 07:00:40AM +0000, Peng Fan wrote: > > > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > > > > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > Use platform_irq_count to replace of_irq_count > > > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > > --- > > > > > > > > > > V1: > > > > > Code inspection, not tested > > > > > > > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > > > > b/drivers/gpio/gpio-bcm-kona.c index 4122683eb1f9..c50721980a7c > > > > 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,7 +585,7 @@ 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); > > > > > + kona_gpio->num_bank = platform_irq_count(pdev); > > > > > > > > of_irq_count returns 0 or a positive int while platform_irq_count > > > > might return a negative error code. This needs handling. Also I > > > > wonder why > > > > platform_irq_count() is better than of_irq_count() which would be > > > > good to describe in the commit log. > > > > > > Inspired from > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml > > > .org%2Flkml%2F2015%2F11%2F18%2F466&data=02%7C01%7Cpeng.f > > an%40nxp.c > > > > > om%7C46364bf12479463df7d308d777c09b39%7C686ea1d3bc2b4c6fa92cd9 > > 9c5c3016 > > > > > 35%7C0%7C0%7C637109541440912984&sdata=awTvwwmo4692Hx7IJ > > H%2BllEVJH7 > > > ngINufoMH8UsosU%2BA%3D&reserved=0 > > > From Rob: > > > " > > > So I started looking at why you are using of_irq_count which drivers > > > shouldn't need to. In patch 5 you use it to allocate memory to store > > > the irq names, then use them here... > > > " > > > > > > Is this ok? > > > > I would say something like: > > > > 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. > > Thanks, how about this change? > @@ -586,11 +585,15 @@ 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) { > + return ret; This is inconsitent. In the ret==0 case you emit an error message, in the ret < 0 case you don't. I think the sensible approach would be to do: if (ret == 0) { dev_err(dev, "Couldn't determine # GPIO banks\n"); return -ENOENT; } else if (ret < 0) { if (ret != -EPROBE_DEFER) dev_err(dev, "Failed to determine count of GPIO banks (%pe)\n", ERR_PTR(ret)); return ret; } Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 7:38 ` Uwe Kleine-König @ 2019-12-03 7:40 ` Peng Fan 0 siblings, 0 replies; 12+ messages in thread From: Peng Fan @ 2019-12-03 7:40 UTC (permalink / raw) To: Uwe Kleine-König Cc: rjui, linus.walleij, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > On Tue, Dec 03, 2019 at 07:23:14AM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > > > On Tue, Dec 03, 2019 at 07:00:40AM +0000, Peng Fan wrote: > > > > > Subject: Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count > > > > > > > > > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > > > Use platform_irq_count to replace of_irq_count > > > > > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > > > --- > > > > > > > > > > > > V1: > > > > > > Code inspection, not tested > > > > > > > > > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c > > > > > > b/drivers/gpio/gpio-bcm-kona.c index > > > > > > 4122683eb1f9..c50721980a7c > > > > > 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,7 +585,7 @@ 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); > > > > > > + kona_gpio->num_bank = platform_irq_count(pdev); > > > > > > > > > > of_irq_count returns 0 or a positive int while > > > > > platform_irq_count might return a negative error code. This > > > > > needs handling. Also I wonder why > > > > > platform_irq_count() is better than of_irq_count() which would > > > > > be good to describe in the commit log. > > > > > > > > Inspired from > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > lkml .org%2Flkml%2F2015%2F11%2F18%2F466&data=02%7C01%7Cpen > g.f > > > an%40nxp.c > > > > > > > > om%7C46364bf12479463df7d308d777c09b39%7C686ea1d3bc2b4c6fa92cd9 > > > 9c5c3016 > > > > > > > > 35%7C0%7C0%7C637109541440912984&sdata=awTvwwmo4692Hx7IJ > > > H%2BllEVJH7 > > > > ngINufoMH8UsosU%2BA%3D&reserved=0 > > > > From Rob: > > > > " > > > > So I started looking at why you are using of_irq_count which > > > > drivers shouldn't need to. In patch 5 you use it to allocate > > > > memory to store the irq names, then use them here... > > > > " > > > > > > > > Is this ok? > > > > > > I would say something like: > > > > > > 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. > > > > Thanks, how about this change? > > @@ -586,11 +585,15 @@ 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) { > > + return ret; > > This is inconsitent. In the ret==0 case you emit an error message, in the ret < > 0 case you don't. I think the sensible approach would be to > do: > > if (ret == 0) { !ret should be fine, I think checkpatch might trigger warning using ret == 0. > dev_err(dev, "Couldn't determine # GPIO banks\n"); > return -ENOENT; > } else if (ret < 0) { > if (ret != -EPROBE_DEFER) > dev_err(dev, "Failed to determine count of GPIO banks > (%pe)\n", ERR_PTR(ret)); > return ret; > } ok, will use this for v2. 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%7Cab060 > 7335df3485a98e908d777c3d594%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0%7C0%7C637109555298587766&sdata=9NTBkwRTIuFBF9y97jqyr > b4Vym8FpvOTZYBrqJqEERk%3D&reserved=0 | ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] gpio: bcm-kona: use platform_irq_count 2019-12-03 6:57 ` Uwe Kleine-König 2019-12-03 7:00 ` Peng Fan @ 2019-12-03 9:54 ` Bartosz Golaszewski 1 sibling, 0 replies; 12+ messages in thread From: Bartosz Golaszewski @ 2019-12-03 9:54 UTC (permalink / raw) To: Uwe Kleine-König Cc: Peng Fan, rjui, linus.walleij, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo wt., 3 gru 2019 o 07:57 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> napisał(a): > > On Tue, Dec 03, 2019 at 06:04:27AM +0000, Peng Fan wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Use platform_irq_count to replace of_irq_count > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V1: > > Code inspection, not tested > > > > drivers/gpio/gpio-bcm-kona.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > > index 4122683eb1f9..c50721980a7c 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,7 +585,7 @@ 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); > > + kona_gpio->num_bank = platform_irq_count(pdev); > > of_irq_count returns 0 or a positive int while platform_irq_count might > return a negative error code. This needs handling. Also I wonder why > platform_irq_count() is better than of_irq_count() which would be good > to describe in the commit log. > In general I like drivers to use generic APIs whenever possible. There are a lot of drivers that use some random of_ routines just because the developer didn't know any better and it turns out we now have generic device properties and a lot of platform device helpers. Consolidation is always good. Waiting for v2. > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] gpio: mvebu: use platform_irq_count 2019-12-03 6:04 [PATCH 1/2] gpio: mvebu: use platform_irq_count Peng Fan 2019-12-03 6:04 ` [PATCH 2/2] gpio: bcm-kona: " Peng Fan @ 2019-12-12 15:29 ` Linus Walleij 2019-12-12 15:31 ` Linus Walleij 1 sibling, 1 reply; 12+ messages in thread From: Linus Walleij @ 2019-12-12 15:29 UTC (permalink / raw) To: Peng Fan Cc: rjui, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, u.kleine-koenig, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo On Tue, Dec 3, 2019 at 7:04 AM Peng Fan <peng.fan@nxp.com> wrote: > From: Peng Fan <peng.fan@nxp.com> > > Use platform_irq_count to replace of_irq_count > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V1: > Code inspection, not tested Patch applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] gpio: mvebu: use platform_irq_count 2019-12-12 15:29 ` [PATCH 1/2] gpio: mvebu: " Linus Walleij @ 2019-12-12 15:31 ` Linus Walleij 2019-12-12 16:08 ` Bartosz Golaszewski 0 siblings, 1 reply; 12+ messages in thread From: Linus Walleij @ 2019-12-12 15:31 UTC (permalink / raw) To: Peng Fan Cc: rjui, bgolaszewski, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, u.kleine-koenig, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo On Thu, Dec 12, 2019 at 4:29 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Dec 3, 2019 at 7:04 AM Peng Fan <peng.fan@nxp.com> wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > Use platform_irq_count to replace of_irq_count > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V1: > > Code inspection, not tested > > Patch applied. Oops dropped again now that I see there are comments on 2/2 that warrants a v2 of this as well. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] gpio: mvebu: use platform_irq_count 2019-12-12 15:31 ` Linus Walleij @ 2019-12-12 16:08 ` Bartosz Golaszewski 0 siblings, 0 replies; 12+ messages in thread From: Bartosz Golaszewski @ 2019-12-12 16:08 UTC (permalink / raw) To: Linus Walleij Cc: Peng Fan, rjui, f.fainelli, sbranden, thierry.reding, robh, bcm-kernel-feedback-list, u.kleine-koenig, dl-linux-imx, linux-gpio, linux-pwm, linux-kernel, Alice Guo czw., 12 gru 2019 o 16:31 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > On Thu, Dec 12, 2019 at 4:29 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > On Tue, Dec 3, 2019 at 7:04 AM Peng Fan <peng.fan@nxp.com> wrote: > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > Use platform_irq_count to replace of_irq_count > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > > > > V1: > > > Code inspection, not tested > > > > Patch applied. > > Oops dropped again now that I see there are comments on > 2/2 that warrants a v2 of this as well. > > Yours, > Linus Walleij Linus, FYI I picked up v3 of this series into my tree. Bart ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-12 16:08 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-03 6:04 [PATCH 1/2] gpio: mvebu: use platform_irq_count Peng Fan 2019-12-03 6:04 ` [PATCH 2/2] gpio: bcm-kona: " Peng Fan 2019-12-03 6:57 ` Uwe Kleine-König 2019-12-03 7:00 ` Peng Fan 2019-12-03 7:15 ` Uwe Kleine-König 2019-12-03 7:23 ` Peng Fan 2019-12-03 7:38 ` Uwe Kleine-König 2019-12-03 7:40 ` Peng Fan 2019-12-03 9:54 ` Bartosz Golaszewski 2019-12-12 15:29 ` [PATCH 1/2] gpio: mvebu: " Linus Walleij 2019-12-12 15:31 ` Linus Walleij 2019-12-12 16:08 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).