Re: [PATCH 2/2 v2] sysdev,mv64x60: initialization of mv64x60 ethernet, serial and I2C

[Dale Farnsworth <dale@farnsworth.org>]

On Mon, Apr 21, 2008 at 10:37:14AM -0700, Remi Machet wrote:
> If one of the devices of the mv64x60 init fails, the remaining 
> devices are not initialized => This patch change the code to 
> display an error and continue the initialization.
> 
> Signed-off-by: Remi Machet (rmachet@slac.stanford.edu)
> ---
> This is the second part of the re-submission of my patch of 4/17/2008
> titled "[PATCH] sysdev,mv64x60: initialization of mv64x60 ethernet, 
> serial and I2C"
> 
> --- a/arch/powerpc/sysdev/mv64x60_dev.c
> +++ b/arch/powerpc/sysdev/mv64x60_dev.c
> @@ -431,9 +431,14 @@ static int __init mv64x60_device_setup(void)
>  	int err;
>  
>  	id = 0;
> -	for_each_compatible_node(np, "serial", "marvell,mv64360-mpsc")
> -		if ((err = mv64x60_mpsc_device_setup(np, id++)))
> -			goto error;
> +	for_each_compatible_node(np, "serial", "marvell,mv64360-mpsc") {
> +		err = mv64x60_mpsc_device_setup(np, id++);
> +		if (err) {
> +			printk(KERN_ERR "Failed to initialize MV64x60 " \

No need for the backslash continuation above.  Also below.

> +					"serial device %s: error %d.\n",
> +					np->full_name, err);
> +		};

You don't need the braces around the single printk statement.

> +	};

No need for the semi-colon following closing brace, also below.

>  
>  	id = 0;
>  	id2 = 0;
> @@ -441,38 +446,46 @@ static int __init mv64x60_device_setup(void)
>  		pdev = mv64x60_eth_register_shared_pdev(np, id++);
>  		if (IS_ERR(pdev)) {
>  			err = PTR_ERR(pdev);
> -			goto error;
> -		}
> +			printk(KERN_ERR "Failed to initialize MV64x60 " \
> +					"network block %s: error %d.\n",
> +					np->full_name, err);
> +			continue;
> +		};
>  		for_each_child_of_node(np, np2) {
>  			if (!of_device_is_compatible(np2,
>  					"marvell,mv64360-eth"))
>  				continue;
>  			err = mv64x60_eth_device_setup(np2, id2++, pdev);
>  			if (err) {
> -				of_node_put(np2);
> -				goto error;
> +				printk(KERN_ERR "Failed to initialize " \
> +						"MV64x60 network device %s: " \
> +						"error %d.\n",
> +						np2->full_name, err);
>  			}
>  		}
>  	}
>  
>  	id = 0;
> -	for_each_compatible_node(np, "i2c", "marvell,mv64360-i2c")
> -		if ((err = mv64x60_i2c_device_setup(np, id++)))
> -			goto error;
> +	for_each_compatible_node(np, "i2c", "marvell,mv64360-i2c") {
> +		err = mv64x60_i2c_device_setup(np, id++);
> +		if (err) {
> +			printk(KERN_ERR "Failed to initialize MV64x60 I2C " \
> +					"bus %s: error %d.\n",
> +					np->full_name, err);
> +		};
> +	};
>  
>  	/* support up to one watchdog timer */
>  	np = of_find_compatible_node(np, NULL, "marvell,mv64360-wdt");
>  	if (np) {
>  		if ((err = mv64x60_wdt_device_setup(np, id)))
> -			goto error;
> +			printk(KERN_ERR "Failed to initialize MV64x60 " \
> +					"Watchdog %s: error %d.\n",
> +					np->full_name, err);
>  		of_node_put(np);
>  	}
>  
>  	return 0;
> -
> -error:
> -	of_node_put(np);
> -	return err;
>  }
>  arch_initcall(mv64x60_device_setup);