[U-Boot-Users] Patch generalizing IDE Reset function for MPC5200

The following patch generalises the use of PSC1.4 for IDE Reset. The idea behind is that most probably many 5200 designs will be similiar to IceCube, _if_ they use the IDE interface. Using the otherwise quite useless PSC1.4 for UDE-Reset seems an obvious choice then.
Instead of continuosly adding more and more boards into the #if defined () chain, one define CONFIG_IDE_RESET_USE_PSC1_4 can be added to the board's configuration file.
If someone else uses another I/O port, that can be easyly added in a similiar way.
This patch patches cpu/mpc5xxx/ide.c AND include/config/IceCube.h.
Thanks, Pierre, for the good work, the driver works fine on our board.

In message 00ee01c3ff80$3b641f60$644ba8c0@alb.sub.de you wrote:
The following patch generalises the use of PSC1.4 for IDE Reset. The idea behind is that most probably many 5200 designs will be similiar to IceCube,
Such assumptions are know to fail on the very next board that will be added.
Instead of continuosly adding more and more boards into the #if defined () chain, one define CONFIG_IDE_RESET_USE_PSC1_4 can be added to the board's configuration file.
Urggh. What an ugly name.
If someone else uses another I/O port, that can be easyly added in a similiar way.
Yes, and we end up "adding more and more boards into the "#define CONFIG_IDE_RESET_USE_this" and "#define CONFIG_IDE_RESET_USE_that" chain.
Sorry, this code is ugly and I reject it.
-#if defined (CONFIG_ICECUBE) && defined (CONFIG_IDE_RESET) +#if defined (CONFIG_IDE_RESET_USE_PSC1_4) /* Configure PSC1_4 as GPIO output for ATA reset */ *(vu_long *) MPC5XXX_WU_GPIO_DATA |= GPIO_PSC1_4; *(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC1_4; *(vu_long *) MPC5XXX_WU_GPIO_DIR |= GPIO_PSC1_4; -#endif /* defined (CONFIG_ICECUBE) && defined (CONFIG_IDE_RESET) */ +#endif /* defined (CONFIG_IDE_RESET_USE_PSC1_4) */
How about changing this into something like??
include/configs/???.h: ... #define CONFIG_IDE_RESET_PIN GPIO_PSC1_4
... #ifdef CONFIG_IDE_RESET_PIN /* Configure IDE Reset pin as GPIO output for ATA reset */ *(vu_long *) MPC5XXX_WU_GPIO_DATA |= CONFIG_IDE_RESET_PIN; ... #endif ...
Best regards,
Wolfgang Denk

Hello,
if a real genuine generalization is wanted, the ide_set_reset () function must be moved to the board specific files.
"Generally" it cannot be assumed that a GPIO Port is used in a non-inverting way to perform a IDE_Reset.
Therefore I will now move "void ide_set_reset (int idereset)" to our top5200.c and leave the mpc5xxx/ide.c untouched.
best regards Reinhard
----- Original Message -----
The following patch generalises the use of PSC1.4 for IDE Reset. The
idea
behind is that most probably many 5200 designs will be similiar to
IceCube,
Such assumptions are know to fail on the very next board that will be added.
Instead of continuosly adding more and more boards into the #if defined
()
chain, one define CONFIG_IDE_RESET_USE_PSC1_4 can be added to the board's configuration
file.
Urggh. What an ugly name.
If someone else uses another I/O port, that can be easyly added in a similiar way.
Yes, and we end up "adding more and more boards into the "#define CONFIG_IDE_RESET_USE_this" and "#define CONFIG_IDE_RESET_USE_that" chain.
Sorry, this code is ugly and I reject it.
.....
How about changing this into something like??
include/configs/???.h: ... #define CONFIG_IDE_RESET_PIN GPIO_PSC1_4
THAT assumption will fail on the first board that does not use a GPIO pin for reset, or uses one that is inverted.
participants (2)
-
Reinhard Meyer
-
Wolfgang Denk