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

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@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@f0000000/ata@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@denx.de