Discussion:
[PATCHv5 0/7] usb: dwc2: Add support for dual-role
d***@public.gmane.org
2014-10-20 18:51:59 UTC
Permalink
From: Dinh Nguyen <dinguyen-***@public.gmane.org>

Hello,

This is version 5 of the patch series that combines the dwc2 gadget and host
driver into a single dual role driver. Here are the main differences from V4:

- Squashed 5 patches from V4 into patch 2. Patchset is now only 7 patches.
- Makefile moved to be the last patch in the series.
- When building for kernel modules, dwc2.ko will get built for all modes(host,
gadget, and dual-role). dwc2_platform.ko and dwc2_pci.ko will get built
for platform SOC and PCI.

For v5, the series is rebased on top of v3.18-rc1.

As usual, tested on SOCFPGA(host, gadget, and dual-role) and on Rpi-B
(host mode only).

I have pushed this series to a git repo to make it more convenient for people
to test/review.

git://git.rocketboards.org/linux-socfpga-next.git dwc2_dual_role_v5

Thanks,

Dinh Nguyen (7):
usb: dwc2: Update the gadget driver to use common dwc2_hsotg structure
usb: dwc2: Move gadget probe function into platform code
usb: dwc2: Initialize the USB core for peripheral mode
usb: dwc2: Update common interrupt handler to call gadget interrupt
handler
usb: dwc2: Add call_gadget functions for perpheral mode interrupts
usb: dwc2: gadget: Do not fail probe if there isn't a clock node
usb: dwc2: Update Kconfig to support dual-role

drivers/usb/dwc2/Kconfig | 61 ++++++----
drivers/usb/dwc2/Makefile | 32 ++---
drivers/usb/dwc2/core.c | 10 --
drivers/usb/dwc2/core.h | 192 +++++++++++++++++------------
drivers/usb/dwc2/core_intr.c | 16 ++-
drivers/usb/dwc2/gadget.c | 283 +++++++++++++------------------------------
drivers/usb/dwc2/hcd.c | 3 +-
drivers/usb/dwc2/hcd.h | 10 --
drivers/usb/dwc2/pci.c | 7 ++
drivers/usb/dwc2/platform.c | 52 ++++++++
10 files changed, 331 insertions(+), 335 deletions(-)
--
2.0.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
d***@opensource.altera.com
2014-10-20 18:52:01 UTC
Permalink
From: Dinh Nguyen <***@opensource.altera.com>

This patch will aggregate the probing of gadget/hcd driver into platform.c.
The gadget probe funtion is converted into gadget_init that is now only
responsible for gadget only initialization. All the gadget resources is now
handled by platform.c

Since the host workqueue will not get initialized if the driver is configured
for peripheral mode only. Thus we need to check for wq_otg before calling
queue_work().

Also, we move spin_lock_init to common location for both host and gadget that
is either in platform.c or pci.c.

We also ove suspend/resume code to common platform code, and update it to use
the new PM API (struct dev_pm_ops).

Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.

Signed-off-by: Dinh Nguyen <***@opensource.altera.com>
Acked-by: Paul Zimmerman <***@synopsys.com>
---
v5: Reworked by squashing the following commits into this one:
* [PATCHv4 02/12] usb: dwc2: move "samsung,s3c6400-hsotg" into common platform
* [PATCHv4 03/12] usb: dwc2: Update the gadget driver to use common dwc2_hsotg
structure
* [PATCHv4 09/12] usb: dwc2: initialize the spin_lock for both host and gadget
* [PATCHv4 10/12] usb: dwc2: Add suspend/resume for gadget
* [PATCHv4 11/12] usb: dwc2: check that the host work queue is valid
Also use IS_ENABLED instead of #if defined
---
drivers/usb/dwc2/core.h | 34 +++++++++++++++-
drivers/usb/dwc2/core_intr.c | 8 ++--
drivers/usb/dwc2/gadget.c | 97 +++++++++-----------------------------------
drivers/usb/dwc2/hcd.c | 1 -
drivers/usb/dwc2/hcd.h | 10 -----
drivers/usb/dwc2/pci.c | 1 +
drivers/usb/dwc2/platform.c | 32 +++++++++++++++
7 files changed, 91 insertions(+), 92 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5412f57..b21aace 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -667,7 +667,6 @@ struct dwc2_hsotg {
struct usb_phy *uphy;
struct s3c_hsotg_plat *plat;

- int irq;
struct clk *clk;

struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
@@ -961,4 +960,37 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
*/
extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);

+/* Gadget defines */
+#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
+extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
+extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
+extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+#else
+static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
+{ return 0; }
+#endif
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
+#else
+static inline void dwc2_set_all_params(struct dwc2_core_params *params, int value) {}
+static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
+ const struct dwc2_core_params *params)
+{ return 0; }
+#endif
+
#endif /* __DWC2_CORE_H__ */
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index c93918b..b176c2f 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -287,9 +287,11 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
* Release lock before scheduling workq as it holds spinlock during
* scheduling.
*/
- spin_unlock(&hsotg->lock);
- queue_work(hsotg->wq_otg, &hsotg->wf_otg);
- spin_lock(&hsotg->lock);
+ if (hsotg->wq_otg) {
+ spin_unlock(&hsotg->lock);
+ queue_work(hsotg->wq_otg, &hsotg->wf_otg);
+ spin_lock(&hsotg->lock);
+ }

/* Clear interrupt */
writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6611ea3..c407d33 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3404,26 +3404,21 @@ static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
}

/**
- * s3c_hsotg_probe - probe function for hsotg driver
- * @pdev: The platform information for the driver
+ * dwc2_gadget_init - init function for gadget
+ * @dwc2: The data structure for the DWC2 driver.
+ * @irq: The IRQ number for the controller.
*/
-static int s3c_hsotg_probe(struct platform_device *pdev)
+int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{
- struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
+ struct device *dev = hsotg->dev;
+ struct s3c_hsotg_plat *plat = dev->platform_data;
struct phy *phy;
struct usb_phy *uphy;
- struct device *dev = &pdev->dev;
struct s3c_hsotg_ep *eps;
- struct dwc2_hsotg *hsotg;
- struct resource *res;
int epnum;
int ret;
int i;

- hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
- if (!hsotg)
- return -ENOMEM;
-
/* Set default UTMI width */
hsotg->phyif = GUSBCFG_PHYIF16;

@@ -3431,14 +3426,14 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
* Attempt to find a generic PHY, then look for an old style
* USB PHY, finally fall back to pdata
*/
- phy = devm_phy_get(&pdev->dev, "usb2-phy");
+ phy = devm_phy_get(dev, "usb2-phy");
if (IS_ERR(phy)) {
uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
if (IS_ERR(uphy)) {
/* Fallback for pdata */
- plat = dev_get_platdata(&pdev->dev);
+ plat = dev_get_platdata(dev);
if (!plat) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"no platform data or transceiver defined\n");
return -EPROBE_DEFER;
}
@@ -3455,36 +3450,12 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->phyif = GUSBCFG_PHYIF8;
}

- hsotg->dev = dev;
-
- hsotg->clk = devm_clk_get(&pdev->dev, "otg");
+ hsotg->clk = devm_clk_get(dev, "otg");
if (IS_ERR(hsotg->clk)) {
dev_err(dev, "cannot get otg clock\n");
return PTR_ERR(hsotg->clk);
}

- platform_set_drvdata(pdev, hsotg);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- hsotg->regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(hsotg->regs)) {
- ret = PTR_ERR(hsotg->regs);
- goto err_clk;
- }
-
- ret = platform_get_irq(pdev, 0);
- if (ret < 0) {
- dev_err(dev, "cannot find IRQ\n");
- goto err_clk;
- }
-
- spin_lock_init(&hsotg->lock);
-
- hsotg->irq = ret;
-
- dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
-
hsotg->gadget.max_speed = USB_SPEED_HIGH;
hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
hsotg->gadget.name = dev_name(dev);
@@ -3501,7 +3472,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
+ dev_err(dev, "failed to request supplies: %d\n", ret);
goto err_clk;
}

@@ -3520,7 +3491,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_hw_cfg(hsotg);
s3c_hsotg_init(hsotg);

- ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
+ ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
dev_name(dev), hsotg);
if (ret < 0) {
s3c_hsotg_phy_disable(hsotg);
@@ -3572,7 +3543,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
+ dev_err(dev, "failed to disable supplies: %d\n", ret);
goto err_ep_mem;
}

@@ -3597,15 +3568,14 @@ err_clk:

return ret;
}
+EXPORT_SYMBOL_GPL(dwc2_gadget_init);

/**
* s3c_hsotg_remove - remove function for hsotg driver
* @pdev: The platform information for the driver
*/
-static int s3c_hsotg_remove(struct platform_device *pdev)
+int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
-
usb_del_gadget_udc(&hsotg->gadget);

s3c_hsotg_delete_debug(hsotg);
@@ -3619,10 +3589,10 @@ static int s3c_hsotg_remove(struct platform_device *pdev)

return 0;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_remove);

-static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
+int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

@@ -3648,10 +3618,10 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)

return ret;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_suspend);

-static int s3c_hsotg_resume(struct platform_device *pdev)
+int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;

@@ -3672,31 +3642,4 @@ static int s3c_hsotg_resume(struct platform_device *pdev)

return ret;
}
-
-#ifdef CONFIG_OF
-static const struct of_device_id s3c_hsotg_of_ids[] = {
- { .compatible = "samsung,s3c6400-hsotg", },
- { .compatible = "snps,dwc2", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
-#endif
-
-static struct platform_driver s3c_hsotg_driver = {
- .driver = {
- .name = "s3c-hsotg",
- .owner = THIS_MODULE,
- .of_match_table = of_match_ptr(s3c_hsotg_of_ids),
- },
- .probe = s3c_hsotg_probe,
- .remove = s3c_hsotg_remove,
- .suspend = s3c_hsotg_suspend,
- .resume = s3c_hsotg_resume,
-};
-
-module_platform_driver(s3c_hsotg_driver);
-
-MODULE_DESCRIPTION("Samsung S3C USB High-speed/OtG device");
-MODULE_AUTHOR("Ben Dooks <***@simtec.co.uk>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:s3c-hsotg");
+EXPORT_SYMBOL_GPL(s3c_hsotg_resume);
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 0a0e6f0..4a3cce0 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2839,7 +2839,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,

hcd->has_tt = 1;

- spin_lock_init(&hsotg->lock);
((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
hsotg->priv = hcd;

diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index a12bb15..e69a843 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -668,9 +668,6 @@ extern irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg);
*/
extern void dwc2_hcd_stop(struct dwc2_hsotg *hsotg);

-extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
-
/**
* dwc2_hcd_is_b_host() - Returns 1 if core currently is acting as B host,
* and 0 otherwise
@@ -680,13 +677,6 @@ extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
extern int dwc2_hcd_is_b_host(struct dwc2_hsotg *hsotg);

/**
- * dwc2_hcd_get_frame_number() - Returns current frame number
- *
- * @hsotg: The DWC2 HCD
- */
-extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
-
-/**
* dwc2_hcd_dump_state() - Dumps hsotg state
*
* @hsotg: The DWC2 HCD
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index c291fca..6d33ecf 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -141,6 +141,7 @@ static int dwc2_driver_probe(struct pci_dev *dev,

pci_set_master(dev);

+ spin_lock_init(&hsotg->lock);
retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
if (retval) {
pci_disable_device(dev);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 121dbda..5783ed0 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -121,6 +121,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);

dwc2_hcd_remove(hsotg);
+ s3c_hsotg_remove(hsotg);

return 0;
}
@@ -129,6 +130,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = &params_bcm2835 },
{ .compatible = "rockchip,rk3066-usb", .data = &params_rk3066 },
{ .compatible = "snps,dwc2", .data = NULL },
+ { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
@@ -204,6 +206,10 @@ static int dwc2_driver_probe(struct platform_device *dev)

hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);

+ spin_lock_init(&hsotg->lock);
+ retval = dwc2_gadget_init(hsotg, irq);
+ if (retval)
+ return retval;
retval = dwc2_hcd_init(hsotg, irq, params);
if (retval)
return retval;
@@ -213,10 +219,36 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
}

+static int dwc2_suspend(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_suspend(dwc2);
+ return ret;
+}
+
+static int dwc2_resume(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_resume(dwc2);
+
+ return ret;
+}
+
+static const struct dev_pm_ops dwc2_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(dwc2_suspend, dwc2_resume)
+};
+
static struct platform_driver dwc2_platform_driver = {
.driver = {
.name = dwc2_driver_name,
.of_match_table = dwc2_of_match_table,
+ .pm = &dwc2_dev_pm_ops,
},
.probe = dwc2_driver_probe,
.remove = dwc2_driver_remove,
--
2.0.3
Bartlomiej Zolnierkiewicz
2014-10-22 11:16:03 UTC
Permalink
Hi,
Post by d***@opensource.altera.com
This patch will aggregate the probing of gadget/hcd driver into platform.c.
The gadget probe funtion is converted into gadget_init that is now only
responsible for gadget only initialization. All the gadget resources is now
handled by platform.c
Since the host workqueue will not get initialized if the driver is configured
for peripheral mode only. Thus we need to check for wq_otg before calling
queue_work().
Also, we move spin_lock_init to common location for both host and gadget that
is either in platform.c or pci.c.
We also ove suspend/resume code to common platform code, and update it to use
the new PM API (struct dev_pm_ops).
Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
This patch seems to break bisectability. It moves all the gadget probing
to platform.c but Kconfig/Makefile are not updated (platform.c will be
compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by d***@opensource.altera.com
---
* [PATCHv4 02/12] usb: dwc2: move "samsung,s3c6400-hsotg" into common platform
* [PATCHv4 03/12] usb: dwc2: Update the gadget driver to use common dwc2_hsotg
structure
* [PATCHv4 09/12] usb: dwc2: initialize the spin_lock for both host and gadget
* [PATCHv4 10/12] usb: dwc2: Add suspend/resume for gadget
* [PATCHv4 11/12] usb: dwc2: check that the host work queue is valid
Also use IS_ENABLED instead of #if defined
---
drivers/usb/dwc2/core.h | 34 +++++++++++++++-
drivers/usb/dwc2/core_intr.c | 8 ++--
drivers/usb/dwc2/gadget.c | 97 +++++++++-----------------------------------
drivers/usb/dwc2/hcd.c | 1 -
drivers/usb/dwc2/hcd.h | 10 -----
drivers/usb/dwc2/pci.c | 1 +
drivers/usb/dwc2/platform.c | 32 +++++++++++++++
7 files changed, 91 insertions(+), 92 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5412f57..b21aace 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -667,7 +667,6 @@ struct dwc2_hsotg {
struct usb_phy *uphy;
struct s3c_hsotg_plat *plat;
- int irq;
struct clk *clk;
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
@@ -961,4 +960,37 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
*/
extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);
+/* Gadget defines */
+#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
+extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
+extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
+extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+#else
+static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
+{ return 0; }
+static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
+{ return 0; }
+#endif
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
+extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
+#else
+static inline void dwc2_set_all_params(struct dwc2_core_params *params, int value) {}
+static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
+ const struct dwc2_core_params *params)
+{ return 0; }
+#endif
+
#endif /* __DWC2_CORE_H__ */
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index c93918b..b176c2f 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -287,9 +287,11 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
* Release lock before scheduling workq as it holds spinlock during
* scheduling.
*/
- spin_unlock(&hsotg->lock);
- queue_work(hsotg->wq_otg, &hsotg->wf_otg);
- spin_lock(&hsotg->lock);
+ if (hsotg->wq_otg) {
+ spin_unlock(&hsotg->lock);
+ queue_work(hsotg->wq_otg, &hsotg->wf_otg);
+ spin_lock(&hsotg->lock);
+ }
/* Clear interrupt */
writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6611ea3..c407d33 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3404,26 +3404,21 @@ static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
}
/**
- * s3c_hsotg_probe - probe function for hsotg driver
+ * dwc2_gadget_init - init function for gadget
*/
-static int s3c_hsotg_probe(struct platform_device *pdev)
+int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{
- struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
+ struct device *dev = hsotg->dev;
+ struct s3c_hsotg_plat *plat = dev->platform_data;
struct phy *phy;
struct usb_phy *uphy;
- struct device *dev = &pdev->dev;
struct s3c_hsotg_ep *eps;
- struct dwc2_hsotg *hsotg;
- struct resource *res;
int epnum;
int ret;
int i;
- hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
- if (!hsotg)
- return -ENOMEM;
-
/* Set default UTMI width */
hsotg->phyif = GUSBCFG_PHYIF16;
@@ -3431,14 +3426,14 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
* Attempt to find a generic PHY, then look for an old style
* USB PHY, finally fall back to pdata
*/
- phy = devm_phy_get(&pdev->dev, "usb2-phy");
+ phy = devm_phy_get(dev, "usb2-phy");
if (IS_ERR(phy)) {
uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
if (IS_ERR(uphy)) {
/* Fallback for pdata */
- plat = dev_get_platdata(&pdev->dev);
+ plat = dev_get_platdata(dev);
if (!plat) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"no platform data or transceiver defined\n");
return -EPROBE_DEFER;
}
@@ -3455,36 +3450,12 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->phyif = GUSBCFG_PHYIF8;
}
- hsotg->dev = dev;
-
- hsotg->clk = devm_clk_get(&pdev->dev, "otg");
+ hsotg->clk = devm_clk_get(dev, "otg");
if (IS_ERR(hsotg->clk)) {
dev_err(dev, "cannot get otg clock\n");
return PTR_ERR(hsotg->clk);
}
- platform_set_drvdata(pdev, hsotg);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- hsotg->regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(hsotg->regs)) {
- ret = PTR_ERR(hsotg->regs);
- goto err_clk;
- }
-
- ret = platform_get_irq(pdev, 0);
- if (ret < 0) {
- dev_err(dev, "cannot find IRQ\n");
- goto err_clk;
- }
-
- spin_lock_init(&hsotg->lock);
-
- hsotg->irq = ret;
-
- dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
-
hsotg->gadget.max_speed = USB_SPEED_HIGH;
hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
hsotg->gadget.name = dev_name(dev);
@@ -3501,7 +3472,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
+ dev_err(dev, "failed to request supplies: %d\n", ret);
goto err_clk;
}
@@ -3520,7 +3491,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_hw_cfg(hsotg);
s3c_hsotg_init(hsotg);
- ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
+ ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
dev_name(dev), hsotg);
if (ret < 0) {
s3c_hsotg_phy_disable(hsotg);
@@ -3572,7 +3543,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
- dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
+ dev_err(dev, "failed to disable supplies: %d\n", ret);
goto err_ep_mem;
}
return ret;
}
+EXPORT_SYMBOL_GPL(dwc2_gadget_init);
/**
* s3c_hsotg_remove - remove function for hsotg driver
*/
-static int s3c_hsotg_remove(struct platform_device *pdev)
+int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
-
usb_del_gadget_udc(&hsotg->gadget);
s3c_hsotg_delete_debug(hsotg);
@@ -3619,10 +3589,10 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
return 0;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_remove);
-static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
+int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;
@@ -3648,10 +3618,10 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
return ret;
}
+EXPORT_SYMBOL_GPL(s3c_hsotg_suspend);
-static int s3c_hsotg_resume(struct platform_device *pdev)
+int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
{
- struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
unsigned long flags;
int ret = 0;
@@ -3672,31 +3642,4 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
return ret;
}
-
-#ifdef CONFIG_OF
-static const struct of_device_id s3c_hsotg_of_ids[] = {
- { .compatible = "samsung,s3c6400-hsotg", },
- { .compatible = "snps,dwc2", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
-#endif
-
-static struct platform_driver s3c_hsotg_driver = {
- .driver = {
- .name = "s3c-hsotg",
- .owner = THIS_MODULE,
- .of_match_table = of_match_ptr(s3c_hsotg_of_ids),
- },
- .probe = s3c_hsotg_probe,
- .remove = s3c_hsotg_remove,
- .suspend = s3c_hsotg_suspend,
- .resume = s3c_hsotg_resume,
-};
-
-module_platform_driver(s3c_hsotg_driver);
-
-MODULE_DESCRIPTION("Samsung S3C USB High-speed/OtG device");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:s3c-hsotg");
+EXPORT_SYMBOL_GPL(s3c_hsotg_resume);
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 0a0e6f0..4a3cce0 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2839,7 +2839,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
hcd->has_tt = 1;
- spin_lock_init(&hsotg->lock);
((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
hsotg->priv = hcd;
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index a12bb15..e69a843 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -668,9 +668,6 @@ extern irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg);
*/
extern void dwc2_hcd_stop(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
-
/**
* dwc2_hcd_is_b_host() - Returns 1 if core currently is acting as B host,
* and 0 otherwise
@@ -680,13 +677,6 @@ extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
extern int dwc2_hcd_is_b_host(struct dwc2_hsotg *hsotg);
/**
- * dwc2_hcd_get_frame_number() - Returns current frame number
- *
- */
-extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
-
-/**
* dwc2_hcd_dump_state() - Dumps hsotg state
*
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index c291fca..6d33ecf 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -141,6 +141,7 @@ static int dwc2_driver_probe(struct pci_dev *dev,
pci_set_master(dev);
+ spin_lock_init(&hsotg->lock);
retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
if (retval) {
pci_disable_device(dev);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 121dbda..5783ed0 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -121,6 +121,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
dwc2_hcd_remove(hsotg);
+ s3c_hsotg_remove(hsotg);
return 0;
}
@@ -129,6 +130,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = &params_bcm2835 },
{ .compatible = "rockchip,rk3066-usb", .data = &params_rk3066 },
{ .compatible = "snps,dwc2", .data = NULL },
+ { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
@@ -204,6 +206,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
+ spin_lock_init(&hsotg->lock);
+ retval = dwc2_gadget_init(hsotg, irq);
+ if (retval)
+ return retval;
retval = dwc2_hcd_init(hsotg, irq, params);
if (retval)
return retval;
@@ -213,10 +219,36 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
}
+static int dwc2_suspend(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_suspend(dwc2);
+ return ret;
+}
+
+static int dwc2_resume(struct device *dev)
+{
+ struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (dwc2_is_device_mode(dwc2))
+ ret = s3c_hsotg_resume(dwc2);
+
+ return ret;
+}
+
+static const struct dev_pm_ops dwc2_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(dwc2_suspend, dwc2_resume)
+};
+
static struct platform_driver dwc2_platform_driver = {
.driver = {
.name = dwc2_driver_name,
.of_match_table = dwc2_of_match_table,
+ .pm = &dwc2_dev_pm_ops,
},
.probe = dwc2_driver_probe,
.remove = dwc2_driver_remove,
Paul Zimmerman
2014-10-22 20:54:00 UTC
Permalink
Sent: Wednesday, October 22, 2014 4:16 AM
Post by d***@opensource.altera.com
This patch will aggregate the probing of gadget/hcd driver into platform.c.
The gadget probe funtion is converted into gadget_init that is now only
responsible for gadget only initialization. All the gadget resources is now
handled by platform.c
Since the host workqueue will not get initialized if the driver is configured
for peripheral mode only. Thus we need to check for wq_otg before calling
queue_work().
Also, we move spin_lock_init to common location for both host and gadget that
is either in platform.c or pci.c.
We also ove suspend/resume code to common platform code, and update it to use
the new PM API (struct dev_pm_ops).
Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
This patch seems to break bisectability. It moves all the gadget probing
to platform.c but Kconfig/Makefile are not updated (platform.c will be
compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).
It doesn't break the compile, I already tested it. It does break the
operation of the driver until patch #7 is applied, but I think that's
OK in the middle of a patch series. I think it's a bit much to expect
the driver to keep working at each step of a patch series like this.
--
Paul
d***@opensource.altera.com
2014-10-20 18:52:00 UTC
Permalink
This post might be inappropriate. Click to display it.
d***@opensource.altera.com
2014-10-20 18:52:02 UTC
Permalink
From: Dinh Nguyen <***@opensource.altera.com>

Initialize the USB driver to peripheral mode when a B-Device connector
is attached.

Signed-off-by: Dinh Nguyen <***@opensource.altera.com>
Acked-by: Paul Zimmerman <***@synopsys.com>
---
v5: move the export of s3c_hsotg_core_init into this patch
---
drivers/usb/dwc2/core.h | 2 ++
drivers/usb/dwc2/gadget.c | 2 +-
drivers/usb/dwc2/hcd.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index b21aace..de1f357 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -966,6 +966,7 @@ extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
+extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
#else
static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
{ return 0; }
@@ -975,6 +976,7 @@ static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
{ return 0; }
static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{ return 0; }
+static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
#endif

#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c407d33..6793ca8 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2125,7 +2125,7 @@ static int s3c_hsotg_corereset(struct dwc2_hsotg *hsotg)
*
* Issue a soft reset to the core, and await the core finishing it.
*/
-static void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
+void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
{
s3c_hsotg_corereset(hsotg);

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 4a3cce0..44c609f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1371,6 +1371,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
+ s3c_hsotg_core_init(hsotg);
} else {
/* A-Device connector (Host Mode) */
dev_dbg(hsotg->dev, "connId A\n");
--
2.0.3
d***@opensource.altera.com
2014-10-20 18:52:03 UTC
Permalink
From: Dinh Nguyen <***@opensource.altera.com>

Make dwc2_handle_common_intr call the gadget interrupt function when operating
in peripheral mode. Remove the spinlock functions in s3c_hsotg_irq as
dwc2_handle_common_intr() already has the spinlocks.

Move the registeration of the IRQ to common code for platform and PCI.

Remove duplicate interrupt conditions that was in gadget, as those are handled
by dwc2 common interrupt handler.

Signed-off-by: Dinh Nguyen <***@opensource.altera.com>
Acked-by: Paul Zimmerman <***@synopsys.com>
---
v5: remove individual devm_request_irq from gadget and hcd, and place a
single devm_request_irq in platform and pci.
v2: Keep interrupt handler for host and peripheral modes separate
---
drivers/usb/dwc2/core.c | 10 --------
drivers/usb/dwc2/core.h | 3 +++
drivers/usb/dwc2/core_intr.c | 3 +++
drivers/usb/dwc2/gadget.c | 57 ++------------------------------------------
drivers/usb/dwc2/pci.c | 6 +++++
drivers/usb/dwc2/platform.c | 9 +++++++
6 files changed, 23 insertions(+), 65 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index d926945..7605850b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -458,16 +458,6 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy, int irq)
/* Clear the SRP success bit for FS-I2c */
hsotg->srp_success = 0;

- if (irq >= 0) {
- dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
- irq);
- retval = devm_request_irq(hsotg->dev, irq,
- dwc2_handle_common_intr, IRQF_SHARED,
- dev_name(hsotg->dev), hsotg);
- if (retval)
- return retval;
- }
-
/* Enable common interrupts */
dwc2_enable_common_interrupts(hsotg);

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index de1f357..ba488ec 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -967,6 +967,7 @@ extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
extern void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2);
+irqreturn_t s3c_hsotg_irq(int irq, void *pw);
#else
static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
{ return 0; }
@@ -977,6 +978,8 @@ static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{ return 0; }
static inline void s3c_hsotg_core_init(struct dwc2_hsotg *dwc2) {}
+static inline irqreturn_t s3c_hsotg_irq(int irq, void *pw)
+{ return IRQ_HANDLED; }
#endif

#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index b176c2f..b0c14e0 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -474,6 +474,9 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)

spin_lock(&hsotg->lock);

+ if (dwc2_is_device_mode(hsotg))
+ retval = s3c_hsotg_irq(irq, dev);
+
gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
retval = IRQ_HANDLED;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6793ca8..6ffbfc2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2257,14 +2257,13 @@ void s3c_hsotg_core_init(struct dwc2_hsotg *hsotg)
* @irq: The IRQ number triggered
* @pw: The pw value when registered the handler.
*/
-static irqreturn_t s3c_hsotg_irq(int irq, void *pw)
+irqreturn_t s3c_hsotg_irq(int irq, void *pw)
{
struct dwc2_hsotg *hsotg = pw;
int retry_count = 8;
u32 gintsts;
u32 gintmsk;

- spin_lock(&hsotg->lock);
irq_retry:
gintsts = readl(hsotg->regs + GINTSTS);
gintmsk = readl(hsotg->regs + GINTMSK);
@@ -2274,33 +2273,12 @@ irq_retry:

gintsts &= gintmsk;

- if (gintsts & GINTSTS_OTGINT) {
- u32 otgint = readl(hsotg->regs + GOTGINT);
-
- dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);
-
- writel(otgint, hsotg->regs + GOTGINT);
- }
-
- if (gintsts & GINTSTS_SESSREQINT) {
- dev_dbg(hsotg->dev, "%s: SessReqInt\n", __func__);
- writel(GINTSTS_SESSREQINT, hsotg->regs + GINTSTS);
- }
-
if (gintsts & GINTSTS_ENUMDONE) {
writel(GINTSTS_ENUMDONE, hsotg->regs + GINTSTS);

s3c_hsotg_irq_enumdone(hsotg);
}

- if (gintsts & GINTSTS_CONIDSTSCHNG) {
- dev_dbg(hsotg->dev, "ConIDStsChg (DSTS=0x%08x, GOTCTL=%08x)\n",
- readl(hsotg->regs + DSTS),
- readl(hsotg->regs + GOTGCTL));
-
- writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
- }
-
if (gintsts & (GINTSTS_OEPINT | GINTSTS_IEPINT)) {
u32 daint = readl(hsotg->regs + DAINT);
u32 daintmsk = readl(hsotg->regs + DAINTMSK);
@@ -2381,25 +2359,6 @@ irq_retry:
s3c_hsotg_handle_rx(hsotg);
}

- if (gintsts & GINTSTS_MODEMIS) {
- dev_warn(hsotg->dev, "warning, mode mismatch triggered\n");
- writel(GINTSTS_MODEMIS, hsotg->regs + GINTSTS);
- }
-
- if (gintsts & GINTSTS_USBSUSP) {
- dev_info(hsotg->dev, "GINTSTS_USBSusp\n");
- writel(GINTSTS_USBSUSP, hsotg->regs + GINTSTS);
-
- call_gadget(hsotg, suspend);
- }
-
- if (gintsts & GINTSTS_WKUPINT) {
- dev_info(hsotg->dev, "GINTSTS_WkUpIn\n");
- writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
-
- call_gadget(hsotg, resume);
- }
-
if (gintsts & GINTSTS_ERLYSUSP) {
dev_dbg(hsotg->dev, "GINTSTS_ErlySusp\n");
writel(GINTSTS_ERLYSUSP, hsotg->regs + GINTSTS);
@@ -2435,10 +2394,9 @@ irq_retry:
if (gintsts & IRQ_RETRY_MASK && --retry_count > 0)
goto irq_retry;

- spin_unlock(&hsotg->lock);
-
return IRQ_HANDLED;
}
+EXPORT_SYMBOL(s3c_hsotg_irq);

/**
* s3c_hsotg_ep_enable - enable the given endpoint
@@ -3491,17 +3449,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
s3c_hsotg_hw_cfg(hsotg);
s3c_hsotg_init(hsotg);

- ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
- dev_name(dev), hsotg);
- if (ret < 0) {
- s3c_hsotg_phy_disable(hsotg);
- clk_disable_unprepare(hsotg->clk);
- regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
- hsotg->supplies);
- dev_err(dev, "cannot claim IRQ\n");
- goto err_clk;
- }
-
/* hsotg->num_of_eps holds number of EPs other than ep0 */

if (hsotg->num_of_eps == 0) {
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index 6d33ecf..a4e724b 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -141,6 +141,12 @@ static int dwc2_driver_probe(struct pci_dev *dev,

pci_set_master(dev);

+ retval = devm_request_irq(hsotg->dev, dev->irq,
+ dwc2_handle_common_intr, IRQF_SHARED,
+ dev_name(hsotg->dev), hsotg);
+ if (retval)
+ return retval;
+
spin_lock_init(&hsotg->lock);
retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
if (retval) {
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 5783ed0..72f32f7 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -196,6 +196,15 @@ static int dwc2_driver_probe(struct platform_device *dev)
return irq;
}

+ dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
+ irq);
+
+ retval = devm_request_irq(hsotg->dev, irq,
+ dwc2_handle_common_intr, IRQF_SHARED,
+ dev_name(hsotg->dev), hsotg);
+ if (retval)
+ return retval;
+
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
hsotg->regs = devm_ioremap_resource(&dev->dev, res);
if (IS_ERR(hsotg->regs))
--
2.0.3
d***@opensource.altera.com
2014-10-20 18:52:04 UTC
Permalink
From: Dinh Nguyen <***@opensource.altera.com>

Update the dwc2 wakeup and suspend interrupt functions to use call_gadget
when the IP is in peripheral mode.

Signed-off-by: Dinh Nguyen <***@opensource.altera.com>
Acked-by: Paul Zimmerman <***@synopsys.com>
---
drivers/usb/dwc2/core_intr.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index b0c14e0..1240875 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -339,6 +339,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
}
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;
+ call_gadget(hsotg, resume);
} else {
if (hsotg->lx_state != DWC2_L1) {
u32 pcgcctl = readl(hsotg->regs + PCGCTL);
@@ -399,6 +400,7 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
"DSTS.Suspend Status=%d HWCFG4.Power Optimize=%d\n",
!!(dsts & DSTS_SUSPSTS),
hsotg->hw_params.power_optimized);
+ call_gadget(hsotg, suspend);
} else {
if (hsotg->op_state == OTG_STATE_A_PERIPHERAL) {
dev_dbg(hsotg->dev, "a_peripheral->a_host\n");
--
2.0.3
d***@opensource.altera.com
2014-10-20 18:52:05 UTC
Permalink
From: Dinh Nguyen <***@opensource.altera.com>

Since the dwc2 hcd driver is currently not looking for a clock node during
init, we should not completely fail if there isn't a clock provided.
For dual-role mode, we will only fail init for a non-clock node error. We
then update the HCD to only call gadget funtions if there is a proper clock
node.

Signed-off-by: Dinh Nguyen <***@opensource.altera.com>
---
v5: reworked to not access gadget functions from the hcd.
---
drivers/usb/dwc2/core.h | 3 +--
drivers/usb/dwc2/core_intr.c | 9 ++++++---
drivers/usb/dwc2/hcd.c | 3 ++-
drivers/usb/dwc2/platform.c | 19 +++++++++++++++----
4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index ba488ec..8794c08 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -660,6 +660,7 @@ struct dwc2_hsotg {
#endif
#endif /* CONFIG_USB_DWC2_HOST || CONFIG_USB_DWC2_DUAL_ROLE */

+ struct clk *clk;
#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
/* Gadget structures */
struct usb_gadget_driver *driver;
@@ -667,8 +668,6 @@ struct dwc2_hsotg {
struct usb_phy *uphy;
struct s3c_hsotg_plat *plat;

- struct clk *clk;
-
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];

u32 phyif;
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 1240875..1608037 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -339,7 +339,8 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
}
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;
- call_gadget(hsotg, resume);
+ if (!IS_ERR(hsotg->clk))
+ call_gadget(hsotg, resume);
} else {
if (hsotg->lx_state != DWC2_L1) {
u32 pcgcctl = readl(hsotg->regs + PCGCTL);
@@ -400,7 +401,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
"DSTS.Suspend Status=%d HWCFG4.Power Optimize=%d\n",
!!(dsts & DSTS_SUSPSTS),
hsotg->hw_params.power_optimized);
- call_gadget(hsotg, suspend);
+ if (!IS_ERR(hsotg->clk))
+ call_gadget(hsotg, suspend);
} else {
if (hsotg->op_state == OTG_STATE_A_PERIPHERAL) {
dev_dbg(hsotg->dev, "a_peripheral->a_host\n");
@@ -477,7 +479,8 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
spin_lock(&hsotg->lock);

if (dwc2_is_device_mode(hsotg))
- retval = s3c_hsotg_irq(irq, dev);
+ if (!IS_ERR(hsotg->clk))
+ retval = s3c_hsotg_irq(irq, dev);

gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 44c609f..fa49c72 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1371,7 +1371,8 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
- s3c_hsotg_core_init(hsotg);
+ if (!IS_ERR(hsotg->clk))
+ s3c_hsotg_core_init(hsotg);
} else {
/* A-Device connector (Host Mode) */
dev_dbg(hsotg->dev, "connId A\n");
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 72f32f7..77c8417 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -217,8 +217,17 @@ static int dwc2_driver_probe(struct platform_device *dev)

spin_lock_init(&hsotg->lock);
retval = dwc2_gadget_init(hsotg, irq);
- if (retval)
- return retval;
+ if (retval) {
+ /*
+ * We will not fail the driver initialization for dual-role
+ * if no clock node is supplied. However, all gadget
+ * functionality will be disabled if a clock node is not
+ * provided. Host functionality will continue.
+ * TO-DO: make clock node a requirement for the HCD.
+ */
+ if (!IS_ERR(hsotg->clk))
+ return retval;
+ }
retval = dwc2_hcd_init(hsotg, irq, params);
if (retval)
return retval;
@@ -234,7 +243,8 @@ static int dwc2_suspend(struct device *dev)
int ret = 0;

if (dwc2_is_device_mode(dwc2))
- ret = s3c_hsotg_suspend(dwc2);
+ if (!IS_ERR(dwc2->clk))
+ ret = s3c_hsotg_suspend(dwc2);
return ret;
}

@@ -244,7 +254,8 @@ static int dwc2_resume(struct device *dev)
int ret = 0;

if (dwc2_is_device_mode(dwc2))
- ret = s3c_hsotg_resume(dwc2);
+ if (!IS_ERR(dwc2->clk))
+ ret = s3c_hsotg_resume(dwc2);

return ret;
}
--
2.0.3
d***@public.gmane.org
2014-10-20 18:52:06 UTC
Permalink
From: Dinh Nguyen <dinguyen-***@public.gmane.org>

Update DWC2 kconfig and makefile to support dual-role mode. The platform
file will always get compiled for the case where the controller is directly
connected to the CPU. So for loadable modules, dwc2.ko is built for host,
peripheral, and dual-role mode. The PCI bus interface will be called
dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.

Signed-off-by: Dinh Nguyen <dinguyen-***@public.gmane.org>
Acked-by: Paul Zimmerman <paulz-HKixBCOQz3hWk0Htik3J/***@public.gmane.org>
---
v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
get built for kernel modules.
v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
config options.
v2: Remove reference to dwc2_gadget
---
drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
2 files changed, 53 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.

if USB_DWC2

+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
+endchoice

config USB_DWC2_PLATFORM
bool "DWC2 Platform"
- depends on USB_DWC2_HOST
default USB_DWC2_HOST
- help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ default y
+ help
+ The Designware USB2.0 platform interface module for
+ controllers directly connected to the CPU.

config USB_DWC2_PCI
bool "DWC2 PCI"
@@ -44,15 +66,6 @@ config USB_DWC2_PCI
The Designware USB2.0 PCI interface module for controllers
connected to a PCI bus. This is only used for host mode.

-comment "Gadget mode requires USB Gadget support to be enabled"
-
-config USB_DWC2_PERIPHERAL
- tristate "Gadget only mode"
- depends on USB_GADGET
- help
- The Designware USB2.0 high-speed gadget controller
- integrated into many SoCs.
-
config USB_DWC2_DEBUG
bool "Enable Debugging Messages"
help
diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
index b73d2a5..2175d93 100644
--- a/drivers/usb/dwc2/Makefile
+++ b/drivers/usb/dwc2/Makefile
@@ -1,28 +1,28 @@
ccflags-$(CONFIG_USB_DWC2_DEBUG) += -DDEBUG
ccflags-$(CONFIG_USB_DWC2_VERBOSE) += -DVERBOSE_DEBUG

-obj-$(CONFIG_USB_DWC2_HOST) += dwc2.o
+obj-$(CONFIG_USB_DWC2) += dwc2.o
dwc2-y := core.o core_intr.o
-dwc2-y += hcd.o hcd_intr.o
-dwc2-y += hcd_queue.o hcd_ddma.o
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_HOST) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += hcd.o hcd_intr.o
+ dwc2-y += hcd_queue.o hcd_ddma.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_PERIPHERAL) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += gadget.o
+endif

# NOTE: The previous s3c-hsotg peripheral mode only driver has been moved to
# this location and renamed gadget.c. When building for dynamically linked
-# modules, dwc2_gadget.ko will get built for peripheral mode. For host mode,
-# the core module will be dwc2.ko, the PCI bus interface module will called
-# dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
-# At present the host and gadget driver will be separate drivers, but there
-# are plans in the near future to create a dual-role driver.
+# modules, dwc2.ko will get built for host mode, peripheral mode, and dual-role
+# mode. The PCI bus interface module will called dwc2_pci.ko and the platform
+# interface module will be called dwc2_platform.ko.

ifneq ($(CONFIG_USB_DWC2_PCI),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_pci.o
+ obj-$(CONFIG_USB_DWC2) += dwc2_pci.o
dwc2_pci-y := pci.o
endif

-ifneq ($(CONFIG_USB_DWC2_PLATFORM),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_platform.o
- dwc2_platform-y := platform.o
-endif
Paul Bolle
2014-10-20 19:42:31 UTC
Permalink
Post by d***@public.gmane.org
Update DWC2 kconfig and makefile to support dual-role mode. The platform
file will always get compiled for the case where the controller is directly
connected to the CPU. So for loadable modules, dwc2.ko is built for host,
peripheral, and dual-role mode. The PCI bus interface will be called
dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
---
v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
get built for kernel modules.
v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
config options.
v2: Remove reference to dwc2_gadget
---
drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
2 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
(Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
that?)
Post by d***@public.gmane.org
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.
if USB_DWC2
+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
Juggling kconfig tristate logic is still rather hard for me, but don't
the above three "if" rules all evaluate to non-zero if both USB and
USB_GADGET are 'm'? If that's correct perhaps USB_DWC2_DUAL_ROLE should
be the last default (assuming the last default wins, that is).

Besides, will the default USB_DWC2_PERIPHERAL ever trigger as !USB can't
be true at this point, can it?
Post by d***@public.gmane.org
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
+endchoice
(I wonder how the dependencies of these three config entries interact
with the three defaults of this choice. Since we're dealing with three
options here this requires a piece of paper, a pencil, and clear mind to
figure out. Maybe I'll try that in a few days...)
Post by d***@public.gmane.org
config USB_DWC2_PLATFORM
bool "DWC2 Platform"
- depends on USB_DWC2_HOST
default USB_DWC2_HOST
- help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ default y
You now have to default lines. Which one wins?
Post by d***@public.gmane.org
+ help
+ The Designware USB2.0 platform interface module for
+ controllers directly connected to the CPU.
config USB_DWC2_PCI
bool "DWC2 PCI"
@@ -44,15 +66,6 @@ config USB_DWC2_PCI
The Designware USB2.0 PCI interface module for controllers
connected to a PCI bus. This is only used for host mode.
-comment "Gadget mode requires USB Gadget support to be enabled"
-
-config USB_DWC2_PERIPHERAL
- tristate "Gadget only mode"
- depends on USB_GADGET
- help
- The Designware USB2.0 high-speed gadget controller
- integrated into many SoCs.
-
config USB_DWC2_DEBUG
bool "Enable Debugging Messages"
help
diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
index b73d2a5..2175d93 100644
Paul Bolle
Dinh Nguyen
2014-10-21 20:47:36 UTC
Permalink
Post by Paul Bolle
Post by d***@public.gmane.org
Update DWC2 kconfig and makefile to support dual-role mode. The platform
file will always get compiled for the case where the controller is directly
connected to the CPU. So for loadable modules, dwc2.ko is built for host,
peripheral, and dual-role mode. The PCI bus interface will be called
dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
---
v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
get built for kernel modules.
v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
config options.
v2: Remove reference to dwc2_gadget
---
drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
2 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
(Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
that?)
Because USB is for Host-Side support. DWC2 driver should only get built
for when USB is enabled. I think you're getting USB and USB_SUPPORT
mixed up.

In addition, thanks to your comment, I realized that DWC2 should also be
available to build when USB_GADGET is enabled. A patch has been sent:

http://marc.info/?l=linux-usb&m=141392377124818&w=2
Post by Paul Bolle
Post by d***@public.gmane.org
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.
if USB_DWC2
+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
Juggling kconfig tristate logic is still rather hard for me, but don't
the above three "if" rules all evaluate to non-zero if both USB and
USB_GADGET are 'm'? If that's correct perhaps USB_DWC2_DUAL_ROLE should
be the last default (assuming the last default wins, that is).
As the way it is, the functionality is correct. As this is the same as
DWC3's Kconfig, perhaps Felipe can comment if something doesn't seem
correct.
Post by Paul Bolle
Besides, will the default USB_DWC2_PERIPHERAL ever trigger as !USB can't
be true at this point, can it?
Yes it can. USB is for Host-side support, so you can have a condition
where you just want to build for GADGET and !USB.
Post by Paul Bolle
Post by d***@public.gmane.org
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
+endchoice
(I wonder how the dependencies of these three config entries interact
with the three defaults of this choice. Since we're dealing with three
options here this requires a piece of paper, a pencil, and clear mind to
figure out. Maybe I'll try that in a few days...)
Post by d***@public.gmane.org
config USB_DWC2_PLATFORM
bool "DWC2 Platform"
- depends on USB_DWC2_HOST
default USB_DWC2_HOST
- help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ default y
You now have to default lines. Which one wins?
Yes, "default y" should be removed.


Thanks,
Dinh
Paul Zimmerman
2014-10-22 18:45:08 UTC
Permalink
Sent: Tuesday, October 21, 2014 1:48 PM
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.
Maybe "For all modes (host, gadget and dual-role), there will be an
additional module named dwc2.ko." That would be clearer.
if USB_DWC2
+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
Shouldn't this be "This option requires USB_GADGET to be enabled"? It
doesn't have to be built-in.
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
Ditto.

Once you fix these, plus the extraneous "default y" that Paul Bolle
pointed out, you can add
Paul Bolle
2014-10-22 20:27:07 UTC
Permalink
Post by Dinh Nguyen
Post by Paul Bolle
(Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig)
even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is
that?)
Because USB is for Host-Side support. DWC2 driver should only get built
for when USB is enabled. I think you're getting USB and USB_SUPPORT
mixed up.
No, I don't think I did. Because in drivers/usb/Kconfig we see
if USB

[...]

endif

[...]

source "drivers/usb/dwc2/Kconfig"

[...]
Post by Dinh Nguyen
In addition, thanks to your comment, I realized that DWC2 should also be
http://marc.info/?l=linux-usb&m=141392377124818&w=2
I haven't checked, but doesn't this mean USB_DWC2 could just depend on
USB_SUPPORT?


Paul Bolle
Bartlomiej Zolnierkiewicz
2014-10-22 12:25:46 UTC
Permalink
Hi,
Post by d***@public.gmane.org
Update DWC2 kconfig and makefile to support dual-role mode. The platform
file will always get compiled for the case where the controller is directly
connected to the CPU. So for loadable modules, dwc2.ko is built for host,
peripheral, and dual-role mode. The PCI bus interface will be called
dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
---
v5: dwc2.ko for all modes along with dwc2_platform.ko and dwc2_pci.ko will
get built for kernel modules.
v3: Add USB_GADGET=y and USB_GADGET=USB_DWC2 for peripheral and dual-role
config options.
v2: Remove reference to dwc2_gadget
---
drivers/usb/dwc2/Kconfig | 61 ++++++++++++++++++++++++++++-------------------
drivers/usb/dwc2/Makefile | 32 ++++++++++++-------------
2 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index f93807b..1ea702e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -1,5 +1,5 @@
config USB_DWC2
- bool "DesignWare USB2 DRD Core Support"
+ tristate "DesignWare USB2 DRD Core Support"
depends on USB
help
Say Y here if your system has a Dual Role Hi-Speed USB
@@ -10,31 +10,53 @@ config USB_DWC2
bus interface module (if you have a PCI bus system) will be
called dwc2_pci.ko, and the platform interface module (for
controllers directly connected to the CPU) will be called
- dwc2_platform.ko. For gadget mode, there will be a single
- module called dwc2_gadget.ko.
-
- NOTE: The s3c-hsotg driver is now renamed to dwc2_gadget. The
- host and gadget drivers are still currently separate drivers.
- There are plans to merge the dwc2_gadget driver with the dwc2
- host driver in the near future to create a dual-role driver.
+ dwc2_platform.ko. For all modes(host, gadget and dual-role), there
+ will be a single module called dwc2.ko.
if USB_DWC2
+choice
+ bool "DWC2 Mode Selection"
+ default USB_DWC2_DUAL_ROLE if (USB && USB_GADGET)
+ default USB_DWC2_HOST if (USB && !USB_GADGET)
+ default USB_DWC2_PERIPHERAL if (!USB && USB_GADGET)
+
config USB_DWC2_HOST
- tristate "Host only mode"
+ bool "Host only mode"
depends on USB
help
The Designware USB2.0 high-speed host controller
- integrated into many SoCs.
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Host-only mode.
+
+comment "Gadget/Dual-role mode requires USB Gadget support to be enabled"
+
+config USB_DWC2_PERIPHERAL
+ bool "Gadget only mode"
+ depends on USB_GADGET=y || USB_GADGET=USB_DWC2
+ help
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs. Select this option if you want the
+ driver to operate in Peripheral-only mode. This option requires
+ USB_GADGET=y.
+
+config USB_DWC2_DUAL_ROLE
+ bool "Dual Role mode"
+ depends on (USB=y || USB=USB_DWC2) && (USB_GADGET=y || USB_GADGET=USB_DWC2)
+ help
+ Select this option if you want the driver to work in a dual-role
+ mode. In this mode both host and gadget features are enabled, and
+ the role will be determined by the cable that gets plugged-in. This
+ option requires USB_GADGET=y.
+endchoice
config USB_DWC2_PLATFORM
bool "DWC2 Platform"
- depends on USB_DWC2_HOST
default USB_DWC2_HOST
It should be "default USB_DWC2_HOST || USB_DWC2_PERIPHERAL" because
USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality.

Additionaly USB_DWC2_PLATFORM should be changed to tristate
(USB_DWC2_PERIPHERAL was a tristeate before your changes).

BTW It is a bit late but it would be great if you could split your
patchset on two. First one merging gadget functionality into
core/platform code and the second one adding USB_DWC2_DUAL_ROLE
functionality.
Post by d***@public.gmane.org
- help
- The Designware USB2.0 platform interface module for
- controllers directly connected to the CPU. This is only
- used for host mode.
+ default y
+ help
+ The Designware USB2.0 platform interface module for
+ controllers directly connected to the CPU.
config USB_DWC2_PCI
bool "DWC2 PCI"
@@ -44,15 +66,6 @@ config USB_DWC2_PCI
The Designware USB2.0 PCI interface module for controllers
connected to a PCI bus. This is only used for host mode.
-comment "Gadget mode requires USB Gadget support to be enabled"
-
-config USB_DWC2_PERIPHERAL
- tristate "Gadget only mode"
- depends on USB_GADGET
- help
- The Designware USB2.0 high-speed gadget controller
- integrated into many SoCs.
-
config USB_DWC2_DEBUG
bool "Enable Debugging Messages"
help
diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
index b73d2a5..2175d93 100644
--- a/drivers/usb/dwc2/Makefile
+++ b/drivers/usb/dwc2/Makefile
@@ -1,28 +1,28 @@
ccflags-$(CONFIG_USB_DWC2_DEBUG) += -DDEBUG
ccflags-$(CONFIG_USB_DWC2_VERBOSE) += -DVERBOSE_DEBUG
-obj-$(CONFIG_USB_DWC2_HOST) += dwc2.o
+obj-$(CONFIG_USB_DWC2) += dwc2.o
dwc2-y := core.o core_intr.o
-dwc2-y += hcd.o hcd_intr.o
-dwc2-y += hcd_queue.o hcd_ddma.o
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_HOST) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += hcd.o hcd_intr.o
+ dwc2-y += hcd_queue.o hcd_ddma.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_DWC2_PERIPHERAL) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
+ dwc2-y += gadget.o
+endif
# NOTE: The previous s3c-hsotg peripheral mode only driver has been moved to
# this location and renamed gadget.c. When building for dynamically linked
-# modules, dwc2_gadget.ko will get built for peripheral mode. For host mode,
-# the core module will be dwc2.ko, the PCI bus interface module will called
-# dwc2_pci.ko and the platform interface module will be called dwc2_platform.ko.
-# At present the host and gadget driver will be separate drivers, but there
-# are plans in the near future to create a dual-role driver.
+# modules, dwc2.ko will get built for host mode, peripheral mode, and dual-role
+# mode. The PCI bus interface module will called dwc2_pci.ko and the platform
+# interface module will be called dwc2_platform.ko.
ifneq ($(CONFIG_USB_DWC2_PCI),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_pci.o
+ obj-$(CONFIG_USB_DWC2) += dwc2_pci.o
dwc2_pci-y := pci.o
endif
-ifneq ($(CONFIG_USB_DWC2_PLATFORM),)
- obj-$(CONFIG_USB_DWC2_HOST) += dwc2_platform.o
- dwc2_platform-y := platform.o
-endif
-
-obj-$(CONFIG_USB_DWC2_PERIPHERAL) += dwc2_gadget.o
-dwc2_gadget-y := gadget.o
+obj-$(CONFIG_USB_DWC2) += dwc2_platform.o
Shouldn't it use CONFIG_USB_DWC2_PLATFORM instead of CONFIG_USB_DWC2?
Post by d***@public.gmane.org
+dwc2_platform-y := platform.o
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz
2014-10-22 12:29:20 UTC
Permalink
Post by Bartlomiej Zolnierkiewicz
BTW It is a bit late but it would be great if you could split your
patchset on two. First one merging gadget functionality into
core/platform code and the second one adding USB_DWC2_DUAL_ROLE
functionality.
On the second thought I think that the dual-role is needed to preserve
existing functionality (available through separate gadget and host
drivers) so please scrap the above comment.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...