Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Fri, Sep 26, 2008 at 10:24:40AM +0200, sbabic-ynQEQJNshbs@public.gmane.org wrote:
> From: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> 
> Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>

One comment below.

> ---
>  drivers/spi/mpc52xx_psc_spi.c |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
> index 25eda71..5d233e7 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -545,8 +550,13 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
>  		id = *psc_nump + 1;
>  	}
>  
> -	return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> -					irq_of_parse_and_map(op->node, 0), id);
> +	if ((ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> +					irq_of_parse_and_map(op->node, 0), id)))
> +		return ret;

Preferred form is:

	ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
					irq_of_parse_and_map(op->node, 0), id);
	if (ret)
		return ret;

Burying the assignment inside the if() makes the code harder to read and
understand.

Otherwise, this looks right to me.

Thanks,
g.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/