Hi everybody,
Here's my suggestions : - for the USB drivers, the constants CONFIG_USB_CDMFDC5xxx and CONFIG_USB_GPSCFG5xxx must be defined in the board configuration file - for the IDE driver, the reset routine is board specific, then the good place for it is the board directory.
The attached patch fixes this points for the Lite5200 board. (Reinhard: it includes your fixes for compiler warnings in usb_ohci.c).
CHANGELOG: * Patch by Pierre AUBERT, 02 Mar 2004 cleanups on IDE and USB drivers for MPC5200
Wolfgang Denk wrote:
In message 00ef01c3ff80$3bf22e90$644ba8c0@alb.sub.de you wrote:
- This spatch removes the compiler warnings (integer made from pointer
without cast)
- it generalizes the Clock Divider Values and the Port Config for USB
if CONFIG_USB_CDMFDC5xxx and CONFIG_USB_GPSCFG5xxx are NOT defined, nothing changes.
Please provide a CHANGELOG entry and a description of these #define's for the README.
@@ -1532,10 +1532,19 @@ int usb_lowlevel_init(void) {
/* Set the USB Clock */ +#if defined (CONFIG_USB_CDMFDC5xxx)
- *(vu_long *)MPC5XXX_CDM_48_FDC = CONFIG_USB_CDMFDC5xxx;
+#else *(vu_long *)MPC5XXX_CDM_48_FDC = 0x0001bbbb;
- *(vu_long *)MPC5XXX_GPS_PORT_CONFIG &= ~0x00800000;
+#endif
- /* remove all USB bits first before ORing in ours */
- *(vu_long *)MPC5XXX_GPS_PORT_CONFIG &= ~0x00807000; /* Activate USB port */
+#if defined (CONFIG_USB_GPSCFG5xxx)
- *(vu_long *)MPC5XXX_GPS_PORT_CONFIG |= CONFIG_USB_GPSCFG5xxx;
+#else *(vu_long *)MPC5XXX_GPS_PORT_CONFIG |= 0x00001000; +#endif
If you think it is likely that these nubers may need adjustment for other boards, then we should #defined them for ALL boards in the config file and get rid of the "#if defined()" mess here.
Your patch adds no code tat actually uses these constants, so basicly all we do is create dead code.
That's why I reject the patch as is. Please feel free to address the 3 issues listed above and resubmit.
Best regards,
Wolfgang Denk
Best regards
diff -Naur u-boot/board/icecube/icecube.c u-boot-paub/board/icecube/icecube.c --- u-boot/board/icecube/icecube.c 2004-01-06 23:38:16.000000000 +0100 +++ u-boot-paub/board/icecube/icecube.c 2004-03-02 09:20:14.000000000 +0100 @@ -207,3 +207,28 @@ pci_mpc5xxx_init(&hose); } #endif + +#if defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET) + +#define GPIO_PSC1_4 0x01000000ul + +void init_ide_reset (void) +{ + printf ("init_ide_reset\n"); + + /* 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; +} + +void ide_set_reset (int idereset) +{ + printf ("ide_reset(%d)\n", idereset); + if (idereset) { + *(vu_long *) MPC5XXX_WU_GPIO_DATA &= ~GPIO_PSC1_4; + } else { + *(vu_long *) MPC5XXX_WU_GPIO_DATA |= GPIO_PSC1_4; + } +} +#endif /* defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET) */ diff -Naur u-boot/cpu/mpc5xxx/ide.c u-boot-paub/cpu/mpc5xxx/ide.c --- u-boot/cpu/mpc5xxx/ide.c 2004-02-27 09:20:54.000000000 +0100 +++ u-boot-paub/cpu/mpc5xxx/ide.c 2004-03-02 09:16:06.000000000 +0100 @@ -29,7 +29,9 @@
#define CALC_TIMING(t) (t + period - 1) / period
-#define GPIO_PSC1_4 0x01000000ul +#ifdef CONFIG_IDE_RESET +extern void init_ide_reset (void); +#endif
int ide_preinit (void) { @@ -70,24 +72,10 @@
*(vu_long *) MPC5XXX_ATA_PIO2 = reg;
-#if defined (CONFIG_ICECUBE) && defined (CONFIG_IDE_RESET) - /* 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) */ +#ifdef CONFIG_IDE_RESET + init_ide_reset (); +#endif /* CONFIG_IDE_RESET */
return (0); } - -#if defined (CONFIG_ICECUBE) && defined (CONFIG_IDE_RESET) -void ide_set_reset (int idereset) -{ - if (idereset) { - *(vu_long *) MPC5XXX_WU_GPIO_DATA &= ~GPIO_PSC1_4; - } else { - *(vu_long *) MPC5XXX_WU_GPIO_DATA |= GPIO_PSC1_4; - } -} -#endif /* defined (CONFIG_ICECUBE) && defined (CONFIG_IDE_RESET) */ #endif /* CFG_CMD_IDE */ diff -Naur u-boot/cpu/mpc5xxx/usb_ohci.c u-boot-paub/cpu/mpc5xxx/usb_ohci.c --- u-boot/cpu/mpc5xxx/usb_ohci.c 2004-02-27 00:46:22.000000000 +0100 +++ u-boot-paub/cpu/mpc5xxx/usb_ohci.c 2004-03-02 08:40:39.000000000 +0100 @@ -488,7 +488,7 @@ if (ohci->ed_controltail == NULL) { writel (ed, &ohci->regs->ed_controlhead); } else { - ohci->ed_controltail->hwNextED = ohci_cpu_to_le32 (ed); + ohci->ed_controltail->hwNextED = (int) ohci_cpu_to_le32 (ed); } ed->ed_prev = ohci->ed_controltail; if (!ohci->ed_controltail && !ohci->ed_rm_list[0] && @@ -504,7 +504,7 @@ if (ohci->ed_bulktail == NULL) { writel (ed, &ohci->regs->ed_bulkhead); } else { - ohci->ed_bulktail->hwNextED = ohci_cpu_to_le32 (ed); + ohci->ed_bulktail->hwNextED = (int) ohci_cpu_to_le32 (ed); } ed->ed_prev = ohci->ed_bulktail; if (!ohci->ed_bulktail && !ohci->ed_rm_list[0] && @@ -598,7 +598,7 @@ ed->hwINFO = ohci_cpu_to_le32 (OHCI_ED_SKIP); /* skip ed */ /* dummy td; end of td list for ed */ td = td_alloc (usb_dev); - ed->hwTailP = ohci_cpu_to_le32 (td); + ed->hwTailP = (int) ohci_cpu_to_le32 (td); ed->hwHeadP = ed->hwTailP; ed->state = ED_UNLINK; ed->type = usb_pipetype (pipe); @@ -656,12 +656,12 @@ data = 0;
td->hwINFO = ohci_cpu_to_le32 (info); - td->hwCBP = ohci_cpu_to_le32 (data); + td->hwCBP = (int) ohci_cpu_to_le32 (data); if (data) - td->hwBE = ohci_cpu_to_le32 (data + len - 1); + td->hwBE = (int) ohci_cpu_to_le32 (data + len - 1); else td->hwBE = 0; - td->hwNextTD = ohci_cpu_to_le32 (td_pt); + td->hwNextTD = (int) ohci_cpu_to_le32 (td_pt); td->hwPSW [0] = ohci_cpu_to_le16 (((__u32)data & 0x0FFF) | 0xE000);
/* append to queue */ @@ -1532,10 +1532,12 @@ {
/* Set the USB Clock */ - *(vu_long *)MPC5XXX_CDM_48_FDC = 0x0001bbbb; - *(vu_long *)MPC5XXX_GPS_PORT_CONFIG &= ~0x00800000; + *(vu_long *)MPC5XXX_CDM_48_FDC = CONFIG_USB_CDMFDC5xxx; + + /* remove all USB bits first before ORing in ours */ + *(vu_long *)MPC5XXX_GPS_PORT_CONFIG &= ~0x00807000; /* Activate USB port */ - *(vu_long *)MPC5XXX_GPS_PORT_CONFIG |= 0x00001000; + *(vu_long *)MPC5XXX_GPS_PORT_CONFIG |= CONFIG_USB_GPSCFG5xxx;
memset (&gohci, 0, sizeof (ohci_t)); memset (&urb_priv, 0, sizeof (urb_priv_t)); diff -Naur u-boot/include/configs/IceCube.h u-boot-paub/include/configs/IceCube.h --- u-boot/include/configs/IceCube.h 2004-02-27 09:20:55.000000000 +0100 +++ u-boot-paub/include/configs/IceCube.h 2004-03-02 09:24:31.000000000 +0100 @@ -302,6 +302,13 @@ #define CFG_RESET_ADDRESS 0xff000000
/*----------------------------------------------------------------------- + * USB stuff + *----------------------------------------------------------------------- + */ +#define CONFIG_USB_CDMFDC5xxx 0x0001bbbb +#define CONFIG_USB_GPSCFG5xxx 0x00001000 + +/*----------------------------------------------------------------------- * IDE/ATA stuff Supports IDE harddisk *----------------------------------------------------------------------- */