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

[Stefan Roese <sr@denx.de>]

Hi Heiko,

sorry for the late review, but I just noticed some minor issues.

On Monday 14 September 2009 17:06:46 Heiko Schocher wrote:
> U-Boot can detect, if a ide device is present or not.
> If not and this new config option is activated, u-boot
> removes the ata node from the DTS before booting Linux,
> so the Linux IDE driver does not crash, when probing the
> device. This is needed for buggy hardware (uc101) where
> no pull down resistor is connected to the signal IDE5V_DD7.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> 
> changes since v1:
> - added comment from Wolfgang Denk, to move this to a more
>   common place, so others can also use it, and made it
>   therefore per CONFIG_OF_IDE_FIXUP selectable.
> 
> changes since v2:
> - add CONFIG_OF_IDE_FIXUP to mpc5200-common.h
> 
> changes since v3
> - correct spelling in README and commit message, as
>   Detlev suggested
> 
>  README                                     |    9 +++++++++
>  cpu/mpc5xxx/cpu.c                          |   18 ++++++++++++++++++
>  include/configs/manroland/mpc5200-common.h |    1 +
>  3 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/README b/README
> index ff4ed8b..191a122 100644
> --- a/README
> +++ b/README
> @@ -386,6 +386,15 @@ The following options need to be configured:
>  		This define fills in the correct boot CPU in the boot
>  		param header, the default value is zero if undefined.
> 
> +		CONFIG_OF_IDE_FIXUP
> +
> +		U-Boot can detect, if a ide device is present or not.
> +		If not and this config option is activated, u-boot
> +		removes the ata node from the DTS before booting Linux,
> +		so the Linux IDE driver does not crash, when probing the
> +		device. This is needed for buggy hardware (uc101) where
> +		no pull down resistor is connected to the signal IDE5V_DD7.
> +
>  - vxWorks boot parameters:
> 
>  		bootvx constructs a valid bootline using the following
> diff --git a/cpu/mpc5xxx/cpu.c b/cpu/mpc5xxx/cpu.c
> index f6258c7..a2fc323 100644
> --- a/cpu/mpc5xxx/cpu.c
> +++ 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.

>  	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));
		}

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