Hello,
This patchset contains fixes for dwc2 gadget driver. It touches PHY,
FIFO configuration, initialization sequence and adds many other small fixes.
Best regards
Robert Baldyga
Samsung R&D Institute Poland
Andrzej Pietrasiewicz (1):
usb: dwc2/gadget: Fix comment text
Kamil Debski (3):
usb: dwc2/gadget: fix phy disable sequence
usb: dwc2/gadget: fix phy initialization sequence
usb: dwc2/gadget: move phy bus legth initialization
Marek Szyprowski (5):
usb: dwc2/gadget: hide some not really needed debug messages
usb: dwc2/gadget: ensure that all fifos have correct memory buffers
usb: dwc2/gadget: break infinite loop in endpoint disable code
usb: dwc2/gadget: do not call disconnect method in pullup
usb: dwc2/gadget: delay enabling irq once hardware is configured
properly
Robert Baldyga (2):
usb: dwc2/gadget: assign TX FIFO dynamically
usb: dwc2/gadget: disable clock when it's not needed
drivers/usb/dwc2/core.h | 2 +
drivers/usb/dwc2/gadget.c | 150 ++++++++++++++++++++++++++++------------------
2 files changed, 93 insertions(+), 59 deletions(-)
--
1.9.1
From: Kamil Debski <[email protected]>
When the driver is removed s3c_hsotg_phy_disable is called three times
instead of once. This results in decreasing of the phy reference counter
below zero and thus consecutive inserts of the module fails.
This patch removes calls to s3c_hsotg_phy_disable from s3c_hsotg_remove
and s3c_hsotg_udc_stop.
s3c_hsotg_udc_stop is called from udc-core.c only after
usb_gadget_disconnect, which in turn calls s3c_hsotg_pullup, which
already calls s3c_hsotg_phy_disable.
s3c_hsotg_remove must be called only after udc_stop, so there is no
point in disabling phy once again there.
Signed-off-by: Kamil Debski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index f3c56a2..ccef3a7 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2898,8 +2898,6 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
spin_lock_irqsave(&hsotg->lock, flags);
- s3c_hsotg_phy_disable(hsotg);
-
if (!driver)
hsotg->driver = NULL;
@@ -3586,7 +3584,6 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
usb_gadget_unregister_driver(hsotg->driver);
}
- s3c_hsotg_phy_disable(hsotg);
if (hsotg->phy)
phy_exit(hsotg->phy);
clk_disable_unprepare(hsotg->clk);
--
1.9.1
From: Kamil Debski <[email protected]>
In the Generic PHY Framework a NULL phy is considered to be a valid phy
thus the "if (hsotg->phy)" check does not give us the information whether
the Generic PHY Framework is used.
In addition to the above this patch also removes phy_init from probe and
phy_exit from remove. This is not necessary when init/exit is done in the
s3c_hsotg_phy_enable/disable functions.
Signed-off-by: Kamil Debski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ccef3a7..70eab95 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2748,13 +2748,14 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
- if (hsotg->phy) {
- phy_init(hsotg->phy);
- phy_power_on(hsotg->phy);
- } else if (hsotg->uphy)
+ if (hsotg->uphy)
usb_phy_init(hsotg->uphy);
- else if (hsotg->plat->phy_init)
+ else if (hsotg->plat && hsotg->plat->phy_init)
hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
+ else {
+ phy_init(hsotg->phy);
+ phy_power_on(hsotg->phy);
+ }
}
/**
@@ -2768,13 +2769,14 @@ static void s3c_hsotg_phy_disable(struct s3c_hsotg *hsotg)
{
struct platform_device *pdev = to_platform_device(hsotg->dev);
- if (hsotg->phy) {
- phy_power_off(hsotg->phy);
- phy_exit(hsotg->phy);
- } else if (hsotg->uphy)
+ if (hsotg->uphy)
usb_phy_shutdown(hsotg->uphy);
- else if (hsotg->plat->phy_exit)
+ else if (hsotg->plat && hsotg->plat->phy_exit)
hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
+ else {
+ phy_power_off(hsotg->phy);
+ phy_exit(hsotg->phy);
+ }
}
/**
@@ -3489,9 +3491,6 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
if (hsotg->phy && (phy_get_bus_width(phy) == 8))
hsotg->phyif = GUSBCFG_PHYIF8;
- if (hsotg->phy)
- phy_init(hsotg->phy);
-
/* usb phy enable */
s3c_hsotg_phy_enable(hsotg);
@@ -3584,8 +3583,6 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
usb_gadget_unregister_driver(hsotg->driver);
}
- if (hsotg->phy)
- phy_exit(hsotg->phy);
clk_disable_unprepare(hsotg->clk);
return 0;
--
1.9.1
From: Kamil Debski <[email protected]>
This patch moves the part of code that initializes the PHY bus width.
This results in simpler code and removes the need to check whether
the Generic PHY Framework is used.
Signed-off-by: Kamil Debski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 70eab95..fc27b4c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3395,6 +3395,9 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ /* Set default UTMI width */
+ hsotg->phyif = GUSBCFG_PHYIF16;
+
/*
* Attempt to find a generic PHY, then look for an old style
* USB PHY, finally fall back to pdata
@@ -3413,8 +3416,15 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->plat = plat;
} else
hsotg->uphy = uphy;
- } else
+ } else {
hsotg->phy = phy;
+ /*
+ * If using the generic PHY framework, check if the PHY bus
+ * width is 8-bit and set the phyif appropriately.
+ */
+ if (phy_get_bus_width(phy) == 8)
+ hsotg->phyif = GUSBCFG_PHYIF8;
+ }
hsotg->dev = dev;
@@ -3481,16 +3491,6 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
goto err_supplies;
}
- /* Set default UTMI width */
- hsotg->phyif = GUSBCFG_PHYIF16;
-
- /*
- * If using the generic PHY framework, check if the PHY bus
- * width is 8-bit and set the phyif appropriately.
- */
- if (hsotg->phy && (phy_get_bus_width(phy) == 8))
- hsotg->phyif = GUSBCFG_PHYIF8;
-
/* usb phy enable */
s3c_hsotg_phy_enable(hsotg);
--
1.9.1
From: Andrzej Pietrasiewicz <[email protected]>
Adjust the debug text to the name of the printed variable.
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index fc27b4c..35b4890 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2935,7 +2935,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
struct s3c_hsotg *hsotg = to_hsotg(gadget);
unsigned long flags = 0;
- dev_dbg(hsotg->dev, "%s: is_in: %d\n", __func__, is_on);
+ dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);
spin_lock_irqsave(&hsotg->lock, flags);
if (is_on) {
--
1.9.1
From: Marek Szyprowski <[email protected]>
Some DWC2/s3c-hsotg debug messages are really useless for typical user,
so hide them behind dev_dbg().
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 35b4890..95b6dcb 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2568,7 +2568,7 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep)
u32 epctrl_reg;
u32 ctrl;
- dev_info(hsotg->dev, "%s(ep %p)\n", __func__, ep);
+ dev_dbg(hsotg->dev, "%s(ep %p)\n", __func__, ep);
if (ep == &hsotg->eps[0].ep) {
dev_err(hsotg->dev, "%s: called for ep0\n", __func__);
@@ -2626,7 +2626,7 @@ static int s3c_hsotg_ep_dequeue(struct usb_ep *ep, struct usb_request *req)
struct s3c_hsotg *hs = hs_ep->parent;
unsigned long flags;
- dev_info(hs->dev, "ep_dequeue(%p,%p)\n", ep, req);
+ dev_dbg(hs->dev, "ep_dequeue(%p,%p)\n", ep, req);
spin_lock_irqsave(&hs->lock, flags);
--
1.9.1
From: Marek Szyprowski <[email protected]>
This patch fixes possible freeze caused by infinite loop in interrupt
context.
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 21d21de..2220882 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1652,6 +1652,7 @@ static void s3c_hsotg_txfifo_flush(struct s3c_hsotg *hsotg, unsigned int idx)
dev_err(hsotg->dev,
"%s: timeout flushing fifo (GRSTCTL=%08x)\n",
__func__, val);
+ break;
}
udelay(1);
--
1.9.1
From: Marek Szyprowski <[email protected]>
This leads to potential spinlock recursion in composite framework, other
udc drivers also don't call it directly from pullup method.
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 2220882..def4900 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2945,7 +2945,6 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
s3c_hsotg_phy_enable(hsotg);
s3c_hsotg_core_init(hsotg);
} else {
- s3c_hsotg_disconnect(hsotg);
s3c_hsotg_phy_disable(hsotg);
}
--
1.9.1
From: Marek Szyprowski <[email protected]>
This patch fixes kernel panic/interrupt storm/etc issues if bootloader
left s3c-hsotg module in enabled state. Now interrupt handler is enabled
only after proper configuration of hardware registers.
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index def4900..3435711 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3459,13 +3459,6 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
hsotg->irq = ret;
- ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
- dev_name(dev), hsotg);
- if (ret < 0) {
- dev_err(dev, "cannot claim IRQ\n");
- goto err_clk;
- }
-
dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
hsotg->gadget.max_speed = USB_SPEED_HIGH;
@@ -3503,6 +3496,17 @@ 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,
+ 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) {
--
1.9.1
Because we have not enough memory to have each TX FIFO of size at least 3072
bytes (the maximum single packet size), we create four FIFOs of lenght 1024,
and four of length 3072 bytes, and assing them to endpoints dynamically
according to maxpacket size value of given endpoint.
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 48 +++++++++++++++++++++++++++++++++--------------
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 067390e..23f7e86 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -197,6 +197,7 @@ struct s3c_hsotg {
int fifo_mem;
unsigned int dedicated_fifos:1;
unsigned char num_of_eps;
+ u32 fifo_map;
struct dentry *debug_root;
struct dentry *debug_file;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 3435711..2a7c014 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -184,14 +184,26 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
/* start at the end of the GNPTXFSIZ, rounded up */
addr = 2048 + 1024;
- size = 768;
/*
* currently we allocate TX FIFOs for all possible endpoints,
* and assume that they are all the same size.
*/
- for (ep = 1; ep <= 15; ep++) {
+ /* 256*4=1024 bytes FIFO length */
+ size = 256;
+ for (ep = 1; ep <= 4; ep++) {
+ val = addr;
+ val |= size << FIFOSIZE_DEPTH_SHIFT;
+ WARN_ONCE(addr + size > hsotg->fifo_mem,
+ "insufficient fifo memory");
+ addr += size;
+
+ writel(val, hsotg->regs + DPTXFSIZN(ep));
+ }
+ /* 768*4=3072 bytes FIFO length */
+ size = 768;
+ for (ep = 5; ep <= 8; ep++) {
val = addr;
val |= size << FIFOSIZE_DEPTH_SHIFT;
WARN_ONCE(addr + size > hsotg->fifo_mem,
@@ -2440,6 +2452,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
u32 epctrl;
u32 mps;
int dir_in;
+ int i, val, size;
int ret = 0;
dev_dbg(hsotg->dev,
@@ -2512,17 +2525,8 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
break;
case USB_ENDPOINT_XFER_INT:
- if (dir_in) {
- /*
- * Allocate our TxFNum by simply using the index
- * of the endpoint for the moment. We could do
- * something better if the host indicates how
- * many FIFOs we are expecting to use.
- */
-
+ if (dir_in)
hs_ep->periodic = 1;
- epctrl |= DXEPCTL_TXFNUM(index);
- }
epctrl |= DXEPCTL_EPTYPE_INTERRUPT;
break;
@@ -2536,8 +2540,24 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
* if the hardware has dedicated fifos, we must give each IN EP
* a unique tx-fifo even if it is non-periodic.
*/
- if (dir_in && hsotg->dedicated_fifos)
- epctrl |= DXEPCTL_TXFNUM(index);
+ if (dir_in && hsotg->dedicated_fifos) {
+ size = hs_ep->ep.maxpacket*hs_ep->mc;
+ for (i = 1; i < 8; ++i) {
+ if (hsotg->fifo_map & (1<<i))
+ continue;
+ val = readl(hsotg->regs + DPTXFSIZN(i));
+ val = (val >> FIFOSIZE_DEPTH_SHIFT)*4;
+ if (val < size)
+ continue;
+ hsotg->fifo_map |= 1<<i;
+ break;
+ }
+
+ if (i == 8)
+ return -ENOMEM;
+
+ epctrl |= DXEPCTL_TXFNUM(i);
+ }
/* for non control endpoints, set PID to D0 */
if (index)
--
1.9.1
When device is stopped or suspended clock is not needed so we
can disable it for this time.
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/gadget.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 2a7c014..0523bc3 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2884,6 +2884,8 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
hsotg->gadget.dev.of_node = hsotg->dev->of_node;
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+ clk_enable(hsotg->clk);
+
ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
if (ret) {
@@ -2932,6 +2934,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+ clk_disable(hsotg->clk);
+
return 0;
}
@@ -2963,8 +2967,10 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
spin_lock_irqsave(&hsotg->lock, flags);
if (is_on) {
s3c_hsotg_phy_enable(hsotg);
+ clk_enable(hsotg->clk);
s3c_hsotg_core_init(hsotg);
} else {
+ clk_disable(hsotg->clk);
s3c_hsotg_phy_disable(hsotg);
}
@@ -3640,6 +3646,7 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
+ clk_disable(hsotg->clk);
}
return ret;
@@ -3654,6 +3661,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
if (hsotg->driver) {
dev_info(hsotg->dev, "resuming usb gadget %s\n",
hsotg->driver->driver.name);
+
+ clk_enable(hsotg->clk);
ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
}
--
1.9.1
From: Marek Szyprowski <[email protected]>
Print warning if FIFOs are configured in such a way that they don't fit
into the SPRAM available on the s3c hsotg module.
Signed-off-by: Marek Szyprowski <[email protected]>
Signed-off-by: Robert Baldyga <[email protected]>
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 15 ++++++++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..067390e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -194,6 +194,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
u32 phyif;
+ int fifo_mem;
unsigned int dedicated_fifos:1;
unsigned char num_of_eps;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 95b6dcb..21d21de 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
for (ep = 1; ep <= 15; ep++) {
val = addr;
val |= size << FIFOSIZE_DEPTH_SHIFT;
+ WARN_ONCE(addr + size > hsotg->fifo_mem,
+ "insufficient fifo memory");
addr += size;
writel(val, hsotg->regs + DPTXFSIZN(ep));
@@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
*/
static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
{
- u32 cfg2, cfg4;
+ u32 cfg2, cfg3, cfg4;
/* check hardware configuration */
cfg2 = readl(hsotg->regs + 0x48);
hsotg->num_of_eps = (cfg2 >> 10) & 0xF;
- dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
+ cfg3 = readl(hsotg->regs + 0x4C);
+ hsotg->fifo_mem = (cfg3 >> 16);
cfg4 = readl(hsotg->regs + 0x50);
hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
- dev_info(hsotg->dev, "%s fifos\n",
- hsotg->dedicated_fifos ? "dedicated" : "shared");
+ dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
+ hsotg->num_of_eps,
+ hsotg->dedicated_fifos ? "dedicated" : "shared",
+ hsotg->fifo_mem);
}
/**
@@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_phy_enable(hsotg);
s3c_hsotg_corereset(hsotg);
- s3c_hsotg_init(hsotg);
s3c_hsotg_hw_cfg(hsotg);
+ s3c_hsotg_init(hsotg);
/* hsotg->num_of_eps holds number of EPs other than ep0 */
--
1.9.1
> From: Robert Baldyga [mailto:[email protected]]
> Sent: Monday, June 23, 2014 12:51 AM
>
> From: Marek Szyprowski <[email protected]>
>
> Print warning if FIFOs are configured in such a way that they don't fit
> into the SPRAM available on the s3c hsotg module.
>
> Signed-off-by: Marek Szyprowski <[email protected]>
> Signed-off-by: Robert Baldyga <[email protected]>
> ---
> drivers/usb/dwc2/core.h | 1 +
> drivers/usb/dwc2/gadget.c | 15 ++++++++++-----
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 1efd10c..067390e 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -194,6 +194,7 @@ struct s3c_hsotg {
> struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
>
> u32 phyif;
> + int fifo_mem;
> unsigned int dedicated_fifos:1;
> unsigned char num_of_eps;
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 95b6dcb..21d21de 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
> for (ep = 1; ep <= 15; ep++) {
> val = addr;
> val |= size << FIFOSIZE_DEPTH_SHIFT;
> + WARN_ONCE(addr + size > hsotg->fifo_mem,
> + "insufficient fifo memory");
> addr += size;
>
> writel(val, hsotg->regs + DPTXFSIZN(ep));
> @@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
> */
> static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
> {
> - u32 cfg2, cfg4;
> + u32 cfg2, cfg3, cfg4;
> /* check hardware configuration */
>
> cfg2 = readl(hsotg->regs + 0x48);
> hsotg->num_of_eps = (cfg2 >> 10) & 0xF;
>
> - dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
> + cfg3 = readl(hsotg->regs + 0x4C);
> + hsotg->fifo_mem = (cfg3 >> 16);
>
> cfg4 = readl(hsotg->regs + 0x50);
> hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
>
> - dev_info(hsotg->dev, "%s fifos\n",
> - hsotg->dedicated_fifos ? "dedicated" : "shared");
> + dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
> + hsotg->num_of_eps,
> + hsotg->dedicated_fifos ? "dedicated" : "shared",
> + hsotg->fifo_mem);
> }
>
> /**
> @@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> s3c_hsotg_phy_enable(hsotg);
>
> s3c_hsotg_corereset(hsotg);
> - s3c_hsotg_init(hsotg);
> s3c_hsotg_hw_cfg(hsotg);
> + s3c_hsotg_init(hsotg);
This last hunk looks like it is not related to the rest of the patch? If
so, I think it should be a separate patch.
--
Paul
> From: Robert Baldyga [mailto:[email protected]]
> Sent: Monday, June 23, 2014 12:51 AM
>
> Because we have not enough memory to have each TX FIFO of size at least 3072
> bytes (the maximum single packet size), we create four FIFOs of lenght 1024,
> and four of length 3072 bytes, and assing them to endpoints dynamically
> according to maxpacket size value of given endpoint.
>
> Signed-off-by: Robert Baldyga <[email protected]>
> ---
> drivers/usb/dwc2/core.h | 1 +
> drivers/usb/dwc2/gadget.c | 48 +++++++++++++++++++++++++++++++++--------------
> 2 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 067390e..23f7e86 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -197,6 +197,7 @@ struct s3c_hsotg {
> int fifo_mem;
> unsigned int dedicated_fifos:1;
> unsigned char num_of_eps;
> + u32 fifo_map;
>
> struct dentry *debug_root;
> struct dentry *debug_file;
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 3435711..2a7c014 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -184,14 +184,26 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>
> /* start at the end of the GNPTXFSIZ, rounded up */
> addr = 2048 + 1024;
> - size = 768;
>
> /*
> * currently we allocate TX FIFOs for all possible endpoints,
> * and assume that they are all the same size.
> */
I think this comment is no longer correct after this patch, right? In
that case, it should be deleted, or better yet replaced with the text
from the commit message.
--
Paul
> - for (ep = 1; ep <= 15; ep++) {
> + /* 256*4=1024 bytes FIFO length */
> + size = 256;
> + for (ep = 1; ep <= 4; ep++) {
> + val = addr;
> + val |= size << FIFOSIZE_DEPTH_SHIFT;
> + WARN_ONCE(addr + size > hsotg->fifo_mem,
> + "insufficient fifo memory");
> + addr += size;
> +
> + writel(val, hsotg->regs + DPTXFSIZN(ep));
> + }
> + /* 768*4=3072 bytes FIFO length */
> + size = 768;
> + for (ep = 5; ep <= 8; ep++) {
> val = addr;
> val |= size << FIFOSIZE_DEPTH_SHIFT;
> WARN_ONCE(addr + size > hsotg->fifo_mem,
> @@ -2440,6 +2452,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> u32 epctrl;
> u32 mps;
> int dir_in;
> + int i, val, size;
> int ret = 0;
>
> dev_dbg(hsotg->dev,
> @@ -2512,17 +2525,8 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> break;
>
> case USB_ENDPOINT_XFER_INT:
> - if (dir_in) {
> - /*
> - * Allocate our TxFNum by simply using the index
> - * of the endpoint for the moment. We could do
> - * something better if the host indicates how
> - * many FIFOs we are expecting to use.
> - */
> -
> + if (dir_in)
> hs_ep->periodic = 1;
> - epctrl |= DXEPCTL_TXFNUM(index);
> - }
>
> epctrl |= DXEPCTL_EPTYPE_INTERRUPT;
> break;
> @@ -2536,8 +2540,24 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> * if the hardware has dedicated fifos, we must give each IN EP
> * a unique tx-fifo even if it is non-periodic.
> */
> - if (dir_in && hsotg->dedicated_fifos)
> - epctrl |= DXEPCTL_TXFNUM(index);
> + if (dir_in && hsotg->dedicated_fifos) {
> + size = hs_ep->ep.maxpacket*hs_ep->mc;
> + for (i = 1; i < 8; ++i) {
> + if (hsotg->fifo_map & (1<<i))
> + continue;
> + val = readl(hsotg->regs + DPTXFSIZN(i));
> + val = (val >> FIFOSIZE_DEPTH_SHIFT)*4;
> + if (val < size)
> + continue;
> + hsotg->fifo_map |= 1<<i;
> + break;
> + }
> +
> + if (i == 8)
> + return -ENOMEM;
> +
> + epctrl |= DXEPCTL_TXFNUM(i);
> + }
>
> /* for non control endpoints, set PID to D0 */
> if (index)
> --
> 1.9.1
> From: Robert Baldyga [mailto:[email protected]]
> Sent: Monday, June 23, 2014 12:51 AM
>
> Hello,
>
> This patchset contains fixes for dwc2 gadget driver. It touches PHY,
> FIFO configuration, initialization sequence and adds many other small fixes.
Hi Robert,
I have a couple of questions/suggestions on patches #6 and #10. Other
than that,
for the entire series,
Acked-by: Paul Zimmerman <[email protected]>
I think we should also have Felipe's ACK for the parts that concern the
PHY framework and the gadget API.
--
Paul
Hi Robert,
When I trace the s3c-gadget driver, I found that the driver did not use
DMA mode because of the 32bit align problem. Have you solved this
problem ? I think just do one more time copy when meet the unalign
buffer address can make the DMA mode work correctly. Does any body have
more suggestions on this issue?
on 2014/6/23 15:51, Robert Baldyga wrote:
> Hello,
>
> This patchset contains fixes for dwc2 gadget driver. It touches PHY,
> FIFO configuration, initialization sequence and adds many other small fixes.
>
> Best regards
> Robert Baldyga
> Samsung R&D Institute Poland
>
> Andrzej Pietrasiewicz (1):
> usb: dwc2/gadget: Fix comment text
>
> Kamil Debski (3):
> usb: dwc2/gadget: fix phy disable sequence
> usb: dwc2/gadget: fix phy initialization sequence
> usb: dwc2/gadget: move phy bus legth initialization
>
> Marek Szyprowski (5):
> usb: dwc2/gadget: hide some not really needed debug messages
> usb: dwc2/gadget: ensure that all fifos have correct memory buffers
> usb: dwc2/gadget: break infinite loop in endpoint disable code
> usb: dwc2/gadget: do not call disconnect method in pullup
> usb: dwc2/gadget: delay enabling irq once hardware is configured
> properly
>
> Robert Baldyga (2):
> usb: dwc2/gadget: assign TX FIFO dynamically
> usb: dwc2/gadget: disable clock when it's not needed
>
> drivers/usb/dwc2/core.h | 2 +
> drivers/usb/dwc2/gadget.c | 150 ++++++++++++++++++++++++++++------------------
> 2 files changed, 93 insertions(+), 59 deletions(-)
>
--
--------
Roy Li @ Rockchip
Hello,
On 2014-06-23 20:40, Paul Zimmerman wrote:
>> From: Robert Baldyga [mailto:[email protected]]
>> Sent: Monday, June 23, 2014 12:51 AM
>>
>> From: Marek Szyprowski <[email protected]>
>>
>> Print warning if FIFOs are configured in such a way that they don't fit
>> into the SPRAM available on the s3c hsotg module.
>>
>> Signed-off-by: Marek Szyprowski <[email protected]>
>> Signed-off-by: Robert Baldyga <[email protected]>
>> ---
>> drivers/usb/dwc2/core.h | 1 +
>> drivers/usb/dwc2/gadget.c | 15 ++++++++++-----
>> 2 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 1efd10c..067390e 100644
>> --- a/drivers/usb/dwc2/core.h
>> +++ b/drivers/usb/dwc2/core.h
>> @@ -194,6 +194,7 @@ struct s3c_hsotg {
>> struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
>>
>> u32 phyif;
>> + int fifo_mem;
>> unsigned int dedicated_fifos:1;
>> unsigned char num_of_eps;
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 95b6dcb..21d21de 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>> for (ep = 1; ep <= 15; ep++) {
>> val = addr;
>> val |= size << FIFOSIZE_DEPTH_SHIFT;
>> + WARN_ONCE(addr + size > hsotg->fifo_mem,
>> + "insufficient fifo memory");
>> addr += size;
>>
>> writel(val, hsotg->regs + DPTXFSIZN(ep));
>> @@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
>> */
>> static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
>> {
>> - u32 cfg2, cfg4;
>> + u32 cfg2, cfg3, cfg4;
>> /* check hardware configuration */
>>
>> cfg2 = readl(hsotg->regs + 0x48);
>> hsotg->num_of_eps = (cfg2 >> 10) & 0xF;
>>
>> - dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
>> + cfg3 = readl(hsotg->regs + 0x4C);
>> + hsotg->fifo_mem = (cfg3 >> 16);
>>
>> cfg4 = readl(hsotg->regs + 0x50);
>> hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
>>
>> - dev_info(hsotg->dev, "%s fifos\n",
>> - hsotg->dedicated_fifos ? "dedicated" : "shared");
>> + dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
>> + hsotg->num_of_eps,
>> + hsotg->dedicated_fifos ? "dedicated" : "shared",
>> + hsotg->fifo_mem);
>> }
>>
>> /**
>> @@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
>> s3c_hsotg_phy_enable(hsotg);
>>
>> s3c_hsotg_corereset(hsotg);
>> - s3c_hsotg_init(hsotg);
>> s3c_hsotg_hw_cfg(hsotg);
>> + s3c_hsotg_init(hsotg);
> This last hunk looks like it is not related to the rest of the patch? If
> so, I think it should be a separate patch.
s3c_hsotg_hw_cfg() only reads hw configuration and some values read there (fifo_mem)
are used in s3c_hsotg_init_fifo(), which is called from s3c_hsotg_init(). This chunk
really belongs to this patch, although it might not be easy to notice it at the first
glance.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland