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

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

[Heiko Schocher]

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 board/mucmc52/mucmc52.c |   15 +++++++++++++++
 board/uc101/uc101.c     |   15 +++++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/board/mucmc52/mucmc52.c b/board/mucmc52/mucmc52.c
index bac49be..950798a 100644
--- a/board/mucmc52/mucmc52.c
+++ b/board/mucmc52/mucmc52.c
@@ -400,8 +400,23 @@ void pci_init_board (void)
 #endif

 #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+#include <libfdt.h>
 void ft_board_setup(void *blob, bd_t *bd)
 {
+	extern block_dev_desc_t ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
+
 	ft_cpu_setup(blob, bd);
+	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));
+	}
 }
 #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */
diff --git a/board/uc101/uc101.c b/board/uc101/uc101.c
index 4030b9d..4d39a08 100644
--- a/board/uc101/uc101.c
+++ b/board/uc101/uc101.c
@@ -373,8 +373,23 @@ void hw_watchdog_reset(void)
 #endif

 #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+#include <libfdt.h>
 void ft_board_setup(void *blob, bd_t *bd)
 {
+	extern block_dev_desc_t ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
+
 	ft_cpu_setup(blob, bd);
+	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));
+	}
 }
 #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */
-- 
1.6.0.6

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

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

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4AA755D4.1020207@denx.de> you wrote:
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>  board/mucmc52/mucmc52.c |   15 +++++++++++++++
>  board/uc101/uc101.c     |   15 +++++++++++++++
>  2 files changed, 30 insertions(+), 0 deletions(-)

Hmm... that looks like duplicated identical code to me. Maybe we can
move this into a central location instead?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To be a winner, all you need to give is all you have.

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

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <4AA755D4.1020207@denx.de> you wrote:
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>  board/mucmc52/mucmc52.c |   15 +++++++++++++++
>>  board/uc101/uc101.c     |   15 +++++++++++++++
>>  2 files changed, 30 insertions(+), 0 deletions(-)
> 
> Hmm... that looks like duplicated identical code to me. Maybe we can
> move this into a central location instead?

Hmm.. you are right, maybe cpu/mpc5xxx/cpu.c ft_cpu_setup() is
a better place for it. And we can activate this through an
CONFIG_OF_IDE_FIXUP define.

Comments?

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

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

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4AA7716C.3000003@denx.de> you wrote:
> 
> > Hmm... that looks like duplicated identical code to me. Maybe we can
> > move this into a central location instead?
> 
> Hmm.. you are right, maybe cpu/mpc5xxx/cpu.c ft_cpu_setup() is
> a better place for it. And we can activate this through an
> CONFIG_OF_IDE_FIXUP define.

Sounds like a good idea to me, as I think that other boards may suffer
from the same problem. The suggested solution would keep the common
code unchanged and still allow all boards that need this to activate
it by a simple #define in the board config file.

The patch should then
- document the new CONFIG_ variable in the README,  and
- add a description of the problem that you are fixing to the commit
  message and the README, so others who look for solutions for the
  same problem can find it.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
und das nennen sie ihren Standpunkt.

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

[Heiko Schocher]

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 didn;t crashs, 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.

 README            |    9 +++++++++
 cpu/mpc5xxx/cpu.c |   18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/README b/README
index ff4ed8b..b9364bf 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 didn;t crashs, 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

 	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));
+	}
+
+#endif
 }
 #endif

-- 
1.6.0.6

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

[Detlev Zundel]

Hi Heiko,

> 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 didn;t crashs, when probing the
                          does not crash

The same below in the actual README.


[...]

> diff --git a/README b/README

[...]

> +		so the Linux IDE driver didn;t crashs, when probing the

Here.

Cheers
  Detlev

-- 
Who is General Failure and why is he reading my hard disk?
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

[Heiko Schocher]

Hello Detlev,

Detlev Zundel 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 didn;t crashs, when probing the
>                           does not crash
> 
> The same below in the actual README.

Thanks for detecting this, patch follow

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

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

[Heiko Schocher]

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

 	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));
+	}
+
+#endif
 }
 #endif

diff --git a/include/configs/manroland/mpc5200-common.h b/include/configs/manroland/mpc5200-common.h
index 2f092b1..b29ef9b 100644
--- a/include/configs/manroland/mpc5200-common.h
+++ b/include/configs/manroland/mpc5200-common.h
@@ -225,5 +225,6 @@
 #define OF_SOC			"soc5200 at f0000000"
 #define OF_TBCLK		(bd->bi_busfreq / 4)
 #define OF_STDOUT_PATH		"/soc5200 at f0000000/serial at 2000"
+#define CONFIG_OF_IDE_FIXUP

 #endif /* __MANROLAND_MPC52XX__COMMON_H */
-- 
1.6.0.6

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

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

[Stefan Roese]

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

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

[Heiko Schocher]

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

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

[Stefan Roese]

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

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

[Heiko Schocher]

Hello Stefan,

Stefan Roese wrote:
> On Monday 14 September 2009 17:26:45 Heiko Schocher wrote:
[...]
>>>>  	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.

I see only one statement in the if and the else case ...

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

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

[Stefan Roese]

On Monday 14 September 2009 17:55:01 Heiko Schocher wrote:
> >>> 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.
> 
> I see only one statement in the if and the else case ...

Yes, but it spans over multiple (2) lines. So it's a multi-line statement. At 
least that's how I understand the coding-style docs.

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

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

[Heiko Schocher]

Hello Stefan,

Stefan Roese wrote:
> On Monday 14 September 2009 17:55:01 Heiko Schocher wrote:
>>>>> 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.
>> I see only one statement in the if and the else case ...
> 
> Yes, but it spans over multiple (2) lines. So it's a multi-line statement. At 
> least that's how I understand the coding-style docs.

Ah, okay, fixed it.

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

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

[Stefan Roese]

Hi Heiko,

and another nitpicking comment below:

On Monday 14 September 2009 17:06:46 Heiko Schocher wrote:

<snip>

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

Please use consistent styles in one file, in this case func() without a space 
before the "(".

Thanks.

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

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

[Heiko Schocher]

Hello Stefan,

Stefan Roese wrote:
> Hi Heiko,
> 
> and another nitpicking comment below:
> 
> On Monday 14 September 2009 17:06:46 Heiko Schocher wrote:
> 
> <snip>
> 
>> 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
>>
>>  	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);
> 
> Please use consistent styles in one file, in this case func() without a space 
> before the "(".

Fixed, thanks.

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

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

[Jerry Van Baren]

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.

Nitpicking your commit comment...

U-Boot can detect if an 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 probe the device and
crash. This is needed for buggy hardware (uc101) where
no pull down resistor is connected to the signal IDE5V_DD7.

Thanks,
gvb

[snip]

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

[Heiko Schocher]

Hello Jerry,

Jerry Van Baren wrote:
> 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.
> 
> Nitpicking your commit comment...
> 
> U-Boot can detect if an 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 probe the device and
> crash. This is needed for buggy hardware (uc101) where
> no pull down resistor is connected to the signal IDE5V_DD7.

Fixed, thanks!

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