Patchwork [v2,07/10] of/irq: Propagate errors in of_irq_to_resource_table()

login
register
mail settings
Submitter Thierry Reding
Date Sept. 18, 2013, 1:24 p.m.
Message ID <1379510692-32435-8-git-send-email-treding@nvidia.com>
Download mbox | patch
Permalink /patch/275732/
State Not Applicable
Headers show

Comments

Thierry Reding - Sept. 18, 2013, 1:24 p.m.
Now that all helpers return precise error codes, this function can
propagate these errors to the caller properly.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- return 0 on success or a negative error code on failure
- convert callers to new calling convention

 arch/mips/lantiq/irq.c       |  2 +-
 arch/mips/lantiq/xway/gptu.c |  6 ++++--
 drivers/of/irq.c             | 14 ++++++++------
 drivers/tty/serial/lantiq.c  |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)
Ralf Baechle - Sept. 18, 2013, 2:23 p.m.
On Wed, Sep 18, 2013 at 03:24:49PM +0200, Thierry Reding wrote:

For the MIPS bits:

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf
Rob Herring - Sept. 22, 2013, 9:08 p.m.
On Wed, Sep 18, 2013 at 8:24 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> Now that all helpers return precise error codes, this function can
> propagate these errors to the caller properly.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - return 0 on success or a negative error code on failure
> - convert callers to new calling convention

[snip]

> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index e4f38c0..6d7f824 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -397,18 +397,20 @@ int of_irq_count(struct device_node *dev)
>   * @res: array of resources to fill in
>   * @nr_irqs: the number of IRQs (and upper bound for num of @res elements)

You are effectively changing this to require an exact match rather
than an upper bound. That seems to be okay since that is what all the
callers want, but the documentation should be updated.

>   *
> - * Returns the size of the filled in table (up to @nr_irqs).
> + * Returns 0 on success or a negative error code on failure.
>   */
>  int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
>                 int nr_irqs)
>  {
> -       int i;
> +       int i, ret;
>
> -       for (i = 0; i < nr_irqs; i++, res++)
> -               if (!of_irq_to_resource(dev, i, res))

The error handling here needs to be updated in the previous patch.

> -                       break;
> +       for (i = 0; i < nr_irqs; i++, res++) {
> +               ret = of_irq_to_resource(dev, i, res);
> +               if (ret < 0)
> +                       return ret;
> +       }
>
> -       return i;
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(of_irq_to_resource_table);
>
Thierry Reding - Sept. 23, 2013, 8:36 a.m.
On Sun, Sep 22, 2013 at 04:08:26PM -0500, Rob Herring wrote:
> On Wed, Sep 18, 2013 at 8:24 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > Now that all helpers return precise error codes, this function can
> > propagate these errors to the caller properly.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v2:
> > - return 0 on success or a negative error code on failure
> > - convert callers to new calling convention
> 
> [snip]
> 
> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> > index e4f38c0..6d7f824 100644
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -397,18 +397,20 @@ int of_irq_count(struct device_node *dev)
> >   * @res: array of resources to fill in
> >   * @nr_irqs: the number of IRQs (and upper bound for num of @res elements)
> 
> You are effectively changing this to require an exact match rather
> than an upper bound. That seems to be okay since that is what all the
> callers want, but the documentation should be updated.

Done.

> >   *
> > - * Returns the size of the filled in table (up to @nr_irqs).
> > + * Returns 0 on success or a negative error code on failure.
> >   */
> >  int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
> >                 int nr_irqs)
> >  {
> > -       int i;
> > +       int i, ret;
> >
> > -       for (i = 0; i < nr_irqs; i++, res++)
> > -               if (!of_irq_to_resource(dev, i, res))
> 
> The error handling here needs to be updated in the previous patch.

Yes, you're right.

Thanks,
Thierry

Patch

diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c
index eb3e186..5bb7ee6 100644
--- a/arch/mips/lantiq/irq.c
+++ b/arch/mips/lantiq/irq.c
@@ -389,7 +389,7 @@  int __init icu_of_init(struct device_node *node, struct device_node *parent)
 
 		ret = of_irq_to_resource_table(eiu_node,
 						ltq_eiu_irq, exin_avail);
-		if (ret != exin_avail)
+		if (ret < 0)
 			panic("failed to load external irq resources\n");
 
 		if (request_mem_region(res.start, resource_size(&res),
diff --git a/arch/mips/lantiq/xway/gptu.c b/arch/mips/lantiq/xway/gptu.c
index 850821d..0c4b134 100644
--- a/arch/mips/lantiq/xway/gptu.c
+++ b/arch/mips/lantiq/xway/gptu.c
@@ -137,10 +137,12 @@  static int gptu_probe(struct platform_device *pdev)
 {
 	struct clk *clk;
 	struct resource *res;
+	int ret;
 
-	if (of_irq_to_resource_table(pdev->dev.of_node, irqres, 6) != 6) {
+	ret = of_irq_to_resource_table(pdev->dev.of_node, irqres, 6);
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to get IRQ list\n");
-		return -EINVAL;
+		return ret;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index e4f38c0..6d7f824 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -397,18 +397,20 @@  int of_irq_count(struct device_node *dev)
  * @res: array of resources to fill in
  * @nr_irqs: the number of IRQs (and upper bound for num of @res elements)
  *
- * Returns the size of the filled in table (up to @nr_irqs).
+ * Returns 0 on success or a negative error code on failure.
  */
 int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
 		int nr_irqs)
 {
-	int i;
+	int i, ret;
 
-	for (i = 0; i < nr_irqs; i++, res++)
-		if (!of_irq_to_resource(dev, i, res))
-			break;
+	for (i = 0; i < nr_irqs; i++, res++) {
+		ret = of_irq_to_resource(dev, i, res);
+		if (ret < 0)
+			return ret;
+	}
 
-	return i;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(of_irq_to_resource_table);
 
diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 88d01e0..e59efdc 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -686,7 +686,7 @@  lqasc_probe(struct platform_device *pdev)
 
 	mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ret = of_irq_to_resource_table(node, irqres, 3);
-	if (!mmres || (ret != 3)) {
+	if (!mmres || (ret < 0)) {
 		dev_err(&pdev->dev,
 			"failed to get memory/irq for serial port\n");
 		return -ENODEV;