2014-06-30 04:02:21

by Nicholas Krause

[permalink] [raw]
Subject: [PATCH] Fixes return issues in uic_init_one

This patch fixes the FIXME messages for returning a ENOMEM error
if uic is not allocated and if uic->irqhost is not allocated a
IRQ domain that is linear returns EIO.

Signed-off-by: Nicholas Krause <[email protected]>
---
arch/powerpc/sysdev/uic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/uic.c b/arch/powerpc/sysdev/uic.c
index 9203393..f95010a 100644
--- a/arch/powerpc/sysdev/uic.c
+++ b/arch/powerpc/sysdev/uic.c
@@ -239,7 +239,7 @@ static struct uic * __init uic_init_one(struct device_node *node)

uic = kzalloc(sizeof(*uic), GFP_KERNEL);
if (! uic)
- return NULL; /* FIXME: panic? */
+ return -ENOMEM;

raw_spin_lock_init(&uic->lock);
indexp = of_get_property(node, "cell-index", &len);
@@ -261,7 +261,7 @@ static struct uic * __init uic_init_one(struct device_node *node)
uic->irqhost = irq_domain_add_linear(node, NR_UIC_INTS, &uic_host_ops,
uic);
if (! uic->irqhost)
- return NULL; /* FIXME: panic? */
+ return -EIO;

/* Start with all interrupts disabled, level and non-critical */
mtdcr(uic->dcrbase + UIC_ER, 0);
--
1.9.1


2014-06-30 12:21:27

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] Fixes return issues in uic_init_one

On Mon, Jun 30, 2014 at 12:02 AM, Nicholas Krause <[email protected]> wrote:
> This patch fixes the FIXME messages for returning a ENOMEM error
> if uic is not allocated and if uic->irqhost is not allocated a
> IRQ domain that is linear returns EIO.
>
> Signed-off-by: Nicholas Krause <[email protected]>

NAK.

You didn't actually fix anything here. In fact, you changed the
semantics of what uic_init_one returns and the function that calls
doesn't change for negative return codes. You also didn't do similar
changes for the rest of the error conditions in within the same
function, making it inconsistent with itself.

NULL is the appropriate return for errors in this function. The FIXME
about panicing is because without a UIC, the boards can't service
interrupts at all. A board that doesn't work might as well panic, but
that panic is actually done in uic_init_tree appropriately. If you're
going to change anything, just remove the comment.

josh

> ---
> arch/powerpc/sysdev/uic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/uic.c b/arch/powerpc/sysdev/uic.c
> index 9203393..f95010a 100644
> --- a/arch/powerpc/sysdev/uic.c
> +++ b/arch/powerpc/sysdev/uic.c
> @@ -239,7 +239,7 @@ static struct uic * __init uic_init_one(struct device_node *node)
>
> uic = kzalloc(sizeof(*uic), GFP_KERNEL);
> if (! uic)
> - return NULL; /* FIXME: panic? */
> + return -ENOMEM;
>
> raw_spin_lock_init(&uic->lock);
> indexp = of_get_property(node, "cell-index", &len);
> @@ -261,7 +261,7 @@ static struct uic * __init uic_init_one(struct device_node *node)
> uic->irqhost = irq_domain_add_linear(node, NR_UIC_INTS, &uic_host_ops,
> uic);
> if (! uic->irqhost)
> - return NULL; /* FIXME: panic? */
> + return -EIO;
>
> /* Start with all interrupts disabled, level and non-critical */
> mtdcr(uic->dcrbase + UIC_ER, 0);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/