Re: [PATCH 1/9 v1.02] Add Synopsys DesignWare HS USB OTG Controller driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/22/2010 03:15 PM, Fushen Chen wrote:
The DWC OTG driver module provides the initialization and cleanup
entry points for the DWC OTG USB driver.

Signed-off-by: Fushen Chen<fchen@xxxxxxx>
Signed-off-by: Mark Miesfeld<mmiesfeld@xxxxxxx>
---
  drivers/Makefile                     |    1 +
  drivers/usb/Kconfig                  |    2 +
  drivers/usb/dwc_otg/Kconfig          |   96 +++
  drivers/usb/dwc_otg/Makefile         |   13 +
  drivers/usb/dwc_otg/dwc_otg_driver.c | 1246 ++++++++++++++++++++++++++++++++++
  drivers/usb/dwc_otg/dwc_otg_driver.h |   97 +++
  drivers/usb/gadget/Kconfig           |   21 +
  drivers/usb/gadget/gadget_chips.h    |    7 +
  8 files changed, 1483 insertions(+), 0 deletions(-)
  create mode 100644 drivers/usb/dwc_otg/Kconfig
  create mode 100644 drivers/usb/dwc_otg/Makefile
  create mode 100644 drivers/usb/dwc_otg/dwc_otg_driver.c
  create mode 100644 drivers/usb/dwc_otg/dwc_otg_driver.h

diff --git a/drivers/Makefile b/drivers/Makefile
index 20dcced..f3fc7c7 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_UWB)		+= uwb/
  obj-$(CONFIG_USB_OTG_UTILS)	+= usb/otg/
  obj-$(CONFIG_USB)		+= usb/
  obj-$(CONFIG_USB_MUSB_HDRC)	+= usb/musb/
+obj-$(CONFIG_USB_DWC_OTG)	+= usb/dwc_otg/
  obj-$(CONFIG_PCI)		+= usb/
  obj-$(CONFIG_USB_GADGET)	+= usb/gadget/
  obj-$(CONFIG_SERIO)		+= input/serio/
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 6a58cb1..f48920b 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -113,6 +113,8 @@ source "drivers/usb/host/Kconfig"

  source "drivers/usb/musb/Kconfig"

+source "drivers/usb/dwc_otg/Kconfig"
+
  source "drivers/usb/class/Kconfig"

  source "drivers/usb/storage/Kconfig"
diff --git a/drivers/usb/dwc_otg/Kconfig b/drivers/usb/dwc_otg/Kconfig
new file mode 100644
index 0000000..27ae0d5
--- /dev/null
+++ b/drivers/usb/dwc_otg/Kconfig
@@ -0,0 +1,96 @@
+#
+# USB Dual Role (OTG-ready) Controller Drivers
+# for silicon based on Synopsys DesignWare IP
+#
[...]
diff --git a/drivers/usb/dwc_otg/Makefile b/drivers/usb/dwc_otg/Makefile
new file mode 100644
index 0000000..337ff81
--- /dev/null
+++ b/drivers/usb/dwc_otg/Makefile
@@ -0,0 +1,13 @@
+#
+# OTG infrastructure and transceiver drivers
+#
+obj-$(CONFIG_USB_DWC_OTG)	+= dwc_otg.o
+
+dwc_otg-objs := dwc_otg_driver.o dwc_otg_cil.o dwc_otg_cil_intr.o
+ifneq ($(CONFIG_DWC_DEVICE_ONLY),y)
+dwc_otg-objs += dwc_otg_hcd.o dwc_otg_hcd_intr.o \
+		dwc_otg_hcd_queue.o
+endif
+ifneq ($(CONFIG_DWC_HOST_ONLY),y)
+dwc_otg-objs += dwc_otg_pcd.o dwc_otg_pcd_intr.o
+endif
diff --git a/drivers/usb/dwc_otg/dwc_otg_driver.c b/drivers/usb/dwc_otg/dwc_otg_driver.c
new file mode 100644
index 0000000..3aae30e

Look at all those files you reference in your Makefile. Most of them don't exist. This will cause the kernel to be unbuildable and break the ability to use git bisect.

One way to remedy this situation would be to make your Kconfig changes the last patch in the series.

Also the subject line for all nine patches seems to be identical, yet the patches are distinct. Perhaps you could find better subject lines.

The very last patch contains exactly one file (dwc_otg_regs.h), but this file is required by most of the preceding patches. This indicates that the ordering of the patches and the way that the files were distributed among the patches could improve. Could you just fold most of the file addition patches into a single patch?

Or if that is untenable, put the core files in one patch, and then maybe hcd and pcd seperatly.

This patch contains many lines that are indented with spaces instead of tabs. How did it manage to pass checkpatch.pl formatted like that?

And finally I would like to suggest taking all the glue-logic functions in dwc_otg_driver.c and putting them in a separate file (perhaps something like dwc_otg_amppc.c or something like that). It could be that initially you just rename dwc_otg_driver.c to dwc_otg_amppc.c. That would make it easy for me to then add my dwc_otg_octeon.c and use the driver with OCTEON (in arch/mips/cavium-octeon).

See: http://marc.info/?l=linux-mips&m=125502126531841&w=2


Thanks,
David Daney
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux