On 09/17/2014 02:47 PM, Dinh Nguyen wrote:
Hi Robert,
On 9/12/14, 7:13 AM, Robert Baldyga wrote:
Hi Dinh,
On 08/26/2014 06:19 PM, dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote:
From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
Add the proper init calls for either host, gadget or both in platform.c
Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
Acked-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
---
drivers/usb/dwc2/core.h | 13 +++++++++++++
drivers/usb/dwc2/gadget.c | 2 +-
drivers/usb/dwc2/platform.c | 28 ++++++++++++++++++++++++----
3 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f55e62d..3a49a00 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -960,6 +960,19 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
*/
extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);
+/* Gadget defines */
+#if defined(CONFIG_USB_DWC2_PERIPHERAL) || defined(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
+extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
Function s3c_hsotg_core_init() is used only inside file gadget.c so
exporting it makes no sense. By the way it should be static.
Yes, I agree here. Fixed up in v5.
+extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+#else
+static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
+static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
+{ return 0; }
It also makes no sense to have this functions declared if you don't have
to use them. They are called in one place in code, inside
dwc2_driver_probe() function, so you can rather use if defined() there.
I'm not sure I agree here. This is necessary for the current runtime
implementation of the role initialization. This is probably relevant
with your next 2 comments.
+#endif
+
#if defined(CONFIG_USB_DWC2_HOST) || defined(CONFIG_USB_DWC2_DUAL_ROLE)
/**
* dwc2_hcd_get_frame_number() - Returns current frame number
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 96f868f..efa68a0 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3572,7 +3572,7 @@ err_clk:
* s3c_hsotg_remove - remove function for hsotg driver
* @pdev: The platform information for the driver
*/
-static int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
+int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
{
usb_del_gadget_udc(&hsotg->gadget);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index dd2f8f5..2871f351 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -92,7 +92,14 @@ static int dwc2_driver_remove(struct platform_device *dev)
{
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
- dwc2_hcd_remove(hsotg);
+ if (IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL))
+ s3c_hsotg_remove(hsotg);
+ else if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
+ dwc2_hcd_remove(hsotg);
+ else { /* dual role */
+ dwc2_hcd_remove(hsotg);
+ s3c_hsotg_remove(hsotg);
+ }
Why don't make this checks compile-time?
Do you have a reason for a compile-time versus runtime here? It just
seems that from a few discussion threads on lkml that there is a general
biased towards using IS_ENABLED() as it looks a bit cleaner than
littering the code with a bunch of #ifdefs.
With typical compiler optimization, if (IS_ENABLED(...)) *is* a
compile-time check. Yet, it allows the code within the if block body to
be parsed, so that even if the code doesn't make it into the binary, it
still gets syntax checking etc.
--
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
