[U-Boot] [PATCH v4 2/2] mucmc52, uc101: delete ata@3a00 node, if no CF card is detected

[Stefan Roese <sr@denx.de>]

On Monday 14 September 2009 17:26:45 Heiko Schocher wrote:
> >> +++ b/cpu/mpc5xxx/cpu.c
> >> @@ -125,6 +125,9 @@ void ft_cpu_setup(void *blob, bd_t *bd)
> >>  	uchar enetaddr[6];
> >>  	char * eth_path = "/" OF_SOC "/ethernet at 3000";
> >>  #endif
> >> +#if defined(CONFIG_OF_IDE_FIXUP)
> >> +	extern block_dev_desc_t ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
> >> +#endif
> >
> > Why do you declare this global variable inside of the function? Perhaps
> > it would be better to move this declaration into some header.
> 
> include/ide.h would be the right place for it I think, or?

Yes, I think so too.

> >>  	do_fixup_by_path_u32(blob, cpu_path, "timebase-frequency", OF_TBCLK,
> >> 1); do_fixup_by_path_u32(blob, cpu_path, "bus-frequency",
> >> bd->bi_busfreq, 1); @@ -137,6 +140,21 @@ void ft_cpu_setup(void *blob,
> >> bd_t *bd)
> >>  	do_fixup_by_path(blob, eth_path, "mac-address", enetaddr, 6, 0);
> >>  	do_fixup_by_path(blob, eth_path, "local-mac-address", enetaddr, 6, 0);
> >>  #endif
> >> +#if defined(CONFIG_OF_IDE_FIXUP)
> >> +	if (ide_dev_desc[0].type == DEV_TYPE_UNKNOWN) {
> >> +		/* NO CF card detected -> delete ata node in DTS */
> >> +		int nodeoffset = 0;
> >> +		char nodename[] = "/soc5200 at f0000000/ata at 3a00";
> >> +
> >> +		nodeoffset = fdt_path_offset (blob, nodename);
> >> +		if (nodeoffset >= 0) {
> >> +			fdt_del_node(blob, nodeoffset);
> >> +		} else
> >> +			printf("%s: cannot find %s node err:%s\n",
> >> +				__func__, nodename, fdt_strerror(nodeoffset));
> >
> > Incorrect multi-line parentheses:
> >
> > 		if (nodeoffset >= 0) {
> > 			fdt_del_node(blob, nodeoffset);
> > 		} else {
> > 			printf("%s: cannot find %s node err:%s\n",
> > 				__func__, nodename, fdt_strerror(nodeoffset));
> > 		}
> 
>                 if (nodeoffset >= 0)
>                         fdt_del_node(blob, nodeoffset);
>                 else
>                         printf("%s: cannot find %s node err:%s\n",
>                                 __func__, nodename,
>  fdt_strerror(nodeoffset));
> 
> Should be right, or?

No. IIRC, then when one of the statements is a multi-line statement, both 
statements of the if/else struct should have the parentheses.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de