* [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
@ 2019-12-18 12:43 Peng Fan
2019-12-18 12:43 ` [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count Peng Fan
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Peng Fan @ 2019-12-18 12:43 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens
Cc: linux-arm-kernel, linux-gpio, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
platform_irq_count() and platform_get_irq() 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>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index aa9dcde0f069..cc66a6429a06 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -15,7 +15,6 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
-#include <linux/of_irq.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinctrl.h>
@@ -739,7 +738,14 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
return ret;
}
- nr_irq_parent = of_irq_count(np);
+ nr_irq_parent = platform_irq_count(pdev);
+ if (nr_irq_parent < 0) {
+ if (nr_irq_parent != -EPROBE_DEFER)
+ dev_err(dev, "Couldn't determine irq count: %pe\n",
+ ERR_PTR(nr_irq_parent));
+ return nr_irq_parent;
+ }
+
spin_lock_init(&info->irq_lock);
if (!nr_irq_parent) {
@@ -776,7 +782,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
if (!girq->parents)
return -ENOMEM;
for (i = 0; i < nr_irq_parent; i++) {
- int irq = irq_of_parse_and_map(np, i);
+ int irq = platform_get_irq(pdev, i);
if (irq < 0)
continue;
--
2.16.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count
2019-12-18 12:43 [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Peng Fan
@ 2019-12-18 12:43 ` Peng Fan
2020-01-07 8:56 ` Linus Walleij
2019-12-18 12:48 ` [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Fabio Estevam
2020-01-16 8:28 ` Peng Fan
2 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2019-12-18 12:43 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens
Cc: linux-arm-kernel, linux-gpio, linux-kernel, Peng Fan
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>
---
drivers/pinctrl/sunxi/pinctrl-sun50i-h5.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h5.c b/drivers/pinctrl/sunxi/pinctrl-sun50i-h5.c
index a78d7b922ef4..31d62bbb7f43 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun50i-h5.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h5.c
@@ -19,7 +19,6 @@
#include <linux/platform_device.h>
#include <linux/of.h>
#include <linux/of_device.h>
-#include <linux/of_irq.h>
#include <linux/pinctrl/pinctrl.h>
#include "pinctrl-sunxi.h"
@@ -549,7 +548,17 @@ static const struct sunxi_pinctrl_desc sun50i_h5_pinctrl_data = {
static int sun50i_h5_pinctrl_probe(struct platform_device *pdev)
{
- switch (of_irq_count(pdev->dev.of_node)) {
+ int ret;
+
+ ret = platform_irq_count(pdev);
+ if (ret < 0) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Couldn't determine irq count: %pe\n",
+ ERR_PTR(ret));
+ return ret;
+ }
+
+ switch (ret) {
case 2:
dev_warn(&pdev->dev,
"Your device tree's pinctrl node is broken, which has no IRQ of PG bank routed.\n");
--
2.16.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count
2019-12-18 12:43 ` [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count Peng Fan
@ 2020-01-07 8:56 ` Linus Walleij
0 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2020-01-07 8:56 UTC (permalink / raw)
To: Peng Fan
Cc: jason, andrew, gregory.clement, sebastian.hesselbarth, mripard,
wens, linux-arm-kernel, linux-gpio, linux-kernel
On Wed, Dec 18, 2019 at 1:43 PM Peng Fan <peng.fan@nxp.com> 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>
This is clearly nicer code, patch applied.
(You only need to reiterate patch 1/2 if you decide to
keep working on that.)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 12:43 [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Peng Fan
2019-12-18 12:43 ` [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count Peng Fan
@ 2019-12-18 12:48 ` Fabio Estevam
2019-12-18 12:53 ` Peng Fan
2020-01-16 8:28 ` Peng Fan
2 siblings, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2019-12-18 12:48 UTC (permalink / raw)
To: Peng Fan
Cc: jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
On Wed, Dec 18, 2019 at 9:44 AM Peng Fan <peng.fan@nxp.com> wrote:
> - nr_irq_parent = of_irq_count(np);
> + nr_irq_parent = platform_irq_count(pdev);
> + if (nr_irq_parent < 0) {
> + if (nr_irq_parent != -EPROBE_DEFER)
> + dev_err(dev, "Couldn't determine irq count: %pe\n",
> + ERR_PTR(nr_irq_parent));
Why do you return ERR_PTR(nr_irq_parent) instead of simply nr_irq_parent?
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 12:48 ` [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Fabio Estevam
@ 2019-12-18 12:53 ` Peng Fan
2019-12-18 12:57 ` Fabio Estevam
0 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2019-12-18 12:53 UTC (permalink / raw)
To: Fabio Estevam
Cc: jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
> Subject: Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
>
> On Wed, Dec 18, 2019 at 9:44 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> > - nr_irq_parent = of_irq_count(np);
> > + nr_irq_parent = platform_irq_count(pdev);
> > + if (nr_irq_parent < 0) {
> > + if (nr_irq_parent != -EPROBE_DEFER)
> > + dev_err(dev, "Couldn't determine irq
> count: %pe\n",
> > + ERR_PTR(nr_irq_parent));
>
> Why do you return ERR_PTR(nr_irq_parent) instead of simply nr_irq_parent?
Please see:
https://lkml.org/lkml/2019/12/4/64
and the patch in tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/
commit/?id=cfdca14c44a79b9c9c491235a39b9fc1e520820b
Thanks,
Peng.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 12:53 ` Peng Fan
@ 2019-12-18 12:57 ` Fabio Estevam
2019-12-18 14:59 ` Andrew Lunn
0 siblings, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2019-12-18 12:57 UTC (permalink / raw)
To: Peng Fan
Cc: jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
Hi Peng,
On Wed, Dec 18, 2019 at 9:53 AM Peng Fan <peng.fan@nxp.com> wrote:
> Please see:
> https://lkml.org/lkml/2019/12/4/64
Still think it makes things more complicated than simply returning the
error code directly.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 12:57 ` Fabio Estevam
@ 2019-12-18 14:59 ` Andrew Lunn
2019-12-18 15:09 ` Fabio Estevam
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2019-12-18 14:59 UTC (permalink / raw)
To: Fabio Estevam
Cc: Peng Fan, jason, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
On Wed, Dec 18, 2019 at 09:57:57AM -0300, Fabio Estevam wrote:
> Hi Peng,
>
> On Wed, Dec 18, 2019 at 9:53 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> > Please see:
> > https://lkml.org/lkml/2019/12/4/64
+ dev_err(dev, "Couldn't determine irq count: %pe\n",
+ ERR_PTR(nr_irq_parent));
> Still think it makes things more complicated than simply returning the
> error code directly.
Hi Fabio
Look closer. This is not about returning an error, it is about
printing an error.
I think the API could better. A %ie formatter would make a lot of
sense, so avoiding the ERR_PTR().
Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 14:59 ` Andrew Lunn
@ 2019-12-18 15:09 ` Fabio Estevam
2019-12-18 15:28 ` Fabio Estevam
2020-01-08 7:52 ` Uwe Kleine-König
0 siblings, 2 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-12-18 15:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: Peng Fan, jason, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
Hi Andrew,
On Wed, Dec 18, 2019 at 12:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
> Hi Fabio
>
> Look closer. This is not about returning an error, it is about
> printing an error.
>
> I think the API could better. A %ie formatter would make a lot of
> sense, so avoiding the ERR_PTR().
Yes, I think that returning the error like:
dev_err(dev, "Couldn't determine irq count: %d\n", nr_irq_parent);
would make the code cleaner.
Maybe just a matter of taste though ;-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 15:09 ` Fabio Estevam
@ 2019-12-18 15:28 ` Fabio Estevam
2020-01-08 7:52 ` Uwe Kleine-König
1 sibling, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-12-18 15:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: Peng Fan, jason, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, linux-gpio, linux-kernel,
linux-arm-kernel
On Wed, Dec 18, 2019 at 12:09 PM Fabio Estevam <festevam@gmail.com> wrote:
> Yes, I think that returning the error like:
s/returning/printing
> dev_err(dev, "Couldn't determine irq count: %d\n", nr_irq_parent);
>
> would make the code cleaner.
>
> Maybe just a matter of taste though ;-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 15:09 ` Fabio Estevam
2019-12-18 15:28 ` Fabio Estevam
@ 2020-01-08 7:52 ` Uwe Kleine-König
1 sibling, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2020-01-08 7:52 UTC (permalink / raw)
To: Fabio Estevam
Cc: Andrew Lunn, Peng Fan, jason, linus.walleij, linux-kernel,
mripard, linux-gpio, wens, gregory.clement, linux-arm-kernel,
sebastian.hesselbarth
Hello Fabio,
On Wed, Dec 18, 2019 at 12:09:42PM -0300, Fabio Estevam wrote:
> On Wed, Dec 18, 2019 at 12:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Hi Fabio
> >
> > Look closer. This is not about returning an error, it is about
> > printing an error.
> >
> > I think the API could better. A %ie formatter would make a lot of
> > sense, so avoiding the ERR_PTR().
>
> Yes, I think that returning the error like:
>
> dev_err(dev, "Couldn't determine irq count: %d\n", nr_irq_parent);
>
> would make the code cleaner.
Are you aware of the semantic difference between
dev_err(..., "Couldn't determine irq count: %d\n", nr_irq_parent);
and
dev_err(..., "Couldn't determine irq count: %pe\n", ERR_PTR(nr_irq_parent));
? The first yields:
Couldn't determine irq count: -5
while the latter yields
Couldn't determine irq count: -EIO
which is more expressive.
I agree that having a format for printing an integer error code would be
useful. I have this on my todo-list but having some %pe with ERR_PTR
conversion would help me arguing my case.
So I would like the patch to go in with ERR_PTR even though v2 was sent
using %d today.
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] pinctrl: mvebu: armada-37xx: use use platform api
2019-12-18 12:43 [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Peng Fan
2019-12-18 12:43 ` [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count Peng Fan
2019-12-18 12:48 ` [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Fabio Estevam
@ 2020-01-16 8:28 ` Peng Fan
2020-01-23 15:06 ` Linus Walleij
2 siblings, 1 reply; 12+ messages in thread
From: Peng Fan @ 2020-01-16 8:28 UTC (permalink / raw)
To: Peng Fan, jason, andrew, gregory.clement, sebastian.hesselbarth,
linus.walleij, mripard, wens, Uwe Kleine-König
Cc: linux-arm-kernel, linux-gpio, linux-kernel
Hi Linus,
> Subject: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
Would you pick this patch up?
Per Uwe, this v1 patch use %pe is better that v2 use %d.
Thanks,
Peng.
>
> From: Peng Fan <peng.fan@nxp.com>
>
> platform_irq_count() and platform_get_irq() 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>
> ---
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index aa9dcde0f069..cc66a6429a06 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -15,7 +15,6 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> -#include <linux/of_irq.h>
> #include <linux/pinctrl/pinconf-generic.h> #include
> <linux/pinctrl/pinconf.h> #include <linux/pinctrl/pinctrl.h> @@ -739,7
> +738,14 @@ static int armada_37xx_irqchip_register(struct platform_device
> *pdev,
> return ret;
> }
>
> - nr_irq_parent = of_irq_count(np);
> + nr_irq_parent = platform_irq_count(pdev);
> + if (nr_irq_parent < 0) {
> + if (nr_irq_parent != -EPROBE_DEFER)
> + dev_err(dev, "Couldn't determine irq count: %pe\n",
> + ERR_PTR(nr_irq_parent));
> + return nr_irq_parent;
> + }
> +
> spin_lock_init(&info->irq_lock);
>
> if (!nr_irq_parent) {
> @@ -776,7 +782,7 @@ static int armada_37xx_irqchip_register(struct
> platform_device *pdev,
> if (!girq->parents)
> return -ENOMEM;
> for (i = 0; i < nr_irq_parent; i++) {
> - int irq = irq_of_parse_and_map(np, i);
> + int irq = platform_get_irq(pdev, i);
>
> if (irq < 0)
> continue;
> --
> 2.16.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
2020-01-16 8:28 ` Peng Fan
@ 2020-01-23 15:06 ` Linus Walleij
0 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2020-01-23 15:06 UTC (permalink / raw)
To: Peng Fan
Cc: jason, andrew, gregory.clement, sebastian.hesselbarth, mripard,
wens, Uwe Kleine-König, linux-arm-kernel, linux-gpio,
linux-kernel
On Thu, Jan 16, 2020 at 9:28 AM Peng Fan <peng.fan@nxp.com> wrote:
> Hi Linus,
>
> > Subject: [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api
>
> Would you pick this patch up?
>
> Per Uwe, this v1 patch use %pe is better that v2 use %d.
OK patch applied to my devel branch for v5.6.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-23 15:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 12:43 [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Peng Fan
2019-12-18 12:43 ` [PATCH 2/2] pinctrl: sunxi: sun50i-h5 use platform_irq_count Peng Fan
2020-01-07 8:56 ` Linus Walleij
2019-12-18 12:48 ` [PATCH 1/2] pinctrl: mvebu: armada-37xx: use use platform api Fabio Estevam
2019-12-18 12:53 ` Peng Fan
2019-12-18 12:57 ` Fabio Estevam
2019-12-18 14:59 ` Andrew Lunn
2019-12-18 15:09 ` Fabio Estevam
2019-12-18 15:28 ` Fabio Estevam
2020-01-08 7:52 ` Uwe Kleine-König
2020-01-16 8:28 ` Peng Fan
2020-01-23 15:06 ` Linus Walleij
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).