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

[Heiko Schocher <hs@denx.de>]

Hello Stefan,

Stefan Roese wrote:
> Hi Heiko,
> 
> sorry for the late review, but I just noticed some minor issues.

No problem, thanks for reviewing.

> 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.

include/ide.h would be the right place for it I think, or?

>>  	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?

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany