Dear Ajay Bhargav,
In message 1310106168-17166-4-git-send-email-ajay.bhargav@einfochips.com you wrote: ...
diff --git a/arch/arm/cpu/arm926ejs/armada100/cpu.c b/arch/arm/cpu/arm926ejs/armada100/cpu.c index c21938e..2db42a0 100644 --- a/arch/arm/cpu/arm926ejs/armada100/cpu.c +++ b/arch/arm/cpu/arm926ejs/armada100/cpu.c
...
+/* Both USB host as well as USB ETH requires this function.
- So moving it from usb eth file (usbeth/mv_u2o_ctl.c) to this common file */
+/* CHIP ID:
- Z0: 0x00a0c910
- Y0: 0x00f0c910
- */
Incorrect multiline comment style. Please fix globally.
...
--- /dev/null +++ b/arch/arm/cpu/arm926ejs/armada100/pxa168_u2h.c @@ -0,0 +1,154 @@
...
+#undef DEBUG
Please don't undefine what is not defined anyway.
+#if DEBUG +static unsigned int usb_debug = DEBUG;
NAK. "DEBUG' in U-Boot is either not defined, or defined without a specific value. This code assumes it is defined as a numeric value, which is an incorrect assumption.
+#else +#define usb_debug 0 /* gcc will remove all the debug code for us */ +#endif
+/*****************************************************************************
- The registers read/write routines
- ******************************************************************************/
+static unsigned usb_sph_get(unsigned *base, unsigned offset) +{
- return readl(base + (offset>>2));
+}
NAK. Please use C structs, and use the standard I/O accessors directly.
+static void usb_sph_set(unsigned *base, unsigned offset, unsigned value) +{
- unsigned int reg;
- if (usb_debug)
printf("base %p off %x base+off %p read %x\n", base, offset,(base + (offset>>2)), *(unsigned *)(base + (offset>>2)));
Braces needed for multiline statements. Please fix globally.
+static void usb_sph_clear(unsigned *base, unsigned offset, unsigned value)
...
+static void usb_sph_write(unsigned *base, unsigned offset, unsigned value)
NAK, see above.
- /* Turn on Main PMU clocks ACGR */
- writel(0x1EFFFF, 0xD4051024);
Please provide #defines for these magic numbers.
- /* USB clk reset */
- writel(0x18, PMUA_USB_CLK_RES_CTRL);
- writel(0x1b, PMUA_USB_CLK_RES_CTRL);
- /* enable the pull up */
- if (!cpu_is_pxa910_168()) {
if (pxa910_is_z0()) {writel((1<<20), (0xc0000004));} else {tmp = readl(0xd4207004);tmp |= (1<<20);writel(tmp, (0xd4207004));
Ditto.
...
--- a/arch/arm/include/asm/arch-armada100/cpu.h +++ b/arch/arm/include/asm/arch-armada100/cpu.h @@ -28,6 +28,36 @@ #include <asm/io.h> #include <asm/system.h>
+#define CPUID_ID 0
+#define __stringify_1(x) #x +#define __stringify(x) __stringify_1(x)
+#define read_cpuid(reg) \
- ({ \
unsigned int __val; \asm("mrc p15, 0, %0, c0, c0, " __stringify(reg) \: "=r" (__val) \: \: "cc"); \__val; \- })
+#define __cpu_is_pxa910_168(id) \
- ({ \
unsigned int _id = (id) & 0xffff; \_id == 0x9263 || _id == 0x8400; \- })
+#define cpu_is_pxa910_168() \
- ({ \
unsigned int id = read_cpuid(CPUID_ID); \__cpu_is_pxa910_168(id); \- })
CodingStyle says:
"Generally, inline functions are preferable to macros resembling functions."
...
+/* ASPEN */ +#define UTMI_REVISION 0x0 +#define UTMI_CTRL 0x4 +#define UTMI_PLL 0x8 +#define UTMI_TX 0xc +#define UTMI_RX 0x10 +#define UTMI_IVREF 0x14 +#define UTMI_T0 0x18 +#define UTMI_T1 0x1c +#define UTMI_T2 0x20 +#define UTMI_T3 0x24 +#define UTMI_T4 0x28 +#define UTMI_T5 0x2c +#define UTMI_RESERVE 0x30 +#define UTMI_USB_INT 0x34 +#define UTMI_DBG_CTL 0x38 +#define UTMI_OTG_ADDON 0x3c
etc.
NAK. Please use C structs instead.
...
--- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -46,6 +46,7 @@ COBJS-$(CONFIG_USB_EHCI_IXP4XX) += ehci-ixp.o COBJS-$(CONFIG_USB_EHCI_KIRKWOOD) += ehci-kirkwood.o COBJS-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o COBJS-$(CONFIG_USB_EHCI_VCT) += ehci-vct.o +COBJS-$(CONFIG_USB_EHCI_PXA168) += ehci-pxa168.o
Please keep lists sorted. Fix globally.
...
+/*
- Destroy the appropriate control structures corresponding
- the the EHCI host controller.
- */
+int ehci_hcd_stop(void) +{
- return 0;
+}
This is probably not sufficient to really stop the EHCI controller, which is a mandatory action here.
Best regards,
Wolfgang Denk