Re: [PATCH] Fixes return issues in uic_init_one

[Josh Boyer]

On Mon, Jun 30, 2014 at 12:02 AM, Nicholas Krause <xerofoify@xxxxxxxxx> 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 <xerofoify@xxxxxxxxx>

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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/