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

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@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)); + } } #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@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)); + } } #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */

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

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

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

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

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

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

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
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)); + } + +#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@f0000000" #define OF_TBCLK (bd->bi_busfreq / 4) #define OF_STDOUT_PATH "/soc5200@f0000000/serial@2000" +#define CONFIG_OF_IDE_FIXUP
#endif /* __MANROLAND_MPC52XX__COMMON_H */

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

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

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@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@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)); }
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@denx.de

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

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

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

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

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

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]

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
participants (6)
-
Detlev Zundel
-
Heiko Schocher
-
Heiko Schocher
-
Jerry Van Baren
-
Stefan Roese
-
Wolfgang Denk