Skip to contentSkip to conversation

usb: cdns3: misc improvements

PATCH In review Peter Chen 3 participants 8 messages Sep 1, 2020 – Nov 23, 2020

Subsystems (3)
USB Cadence USB3 DRD IP driver Cadence usbssp DRD IP driver
lore
Share & tools
lore

Peter Chen

author
#0 lore raw

cover usb: cdns3: misc improvements

Peter Chen (5):
usb: cdns3: core: quit if it uses role switch class
usb: cdns3: gadget: set fast access bit
usb: cdns3: gadget: clear the interrupt status when disconnect the
host
usb: cdns3: drd: call PHY .set_mode accordingly
usb: cdns3: gadget: move wait configuration operation

 drivers/usb/cdns3/core.c   |  4 ++++
 drivers/usb/cdns3/drd.c    |  5 +++++
 drivers/usb/cdns3/ep0.c    | 10 +++++++++-
 drivers/usb/cdns3/gadget.c | 15 ++++++++-------
 4 files changed, 26 insertions(+), 8 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 1/5 usb: cdns3: core: quit if it uses role switch class

If the board uses role switch class for switching the role, it should not depends on SoC OTG hardware siginal any more, so quit early.

Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/cdns3/core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/usb/cdns3/core.c +4 -0
index faee5ec5fc20..96c2da4e20c5 100644
@@ -280,6 +280,10 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
 	enum usb_role real_role, current_role;
 	int ret = 0;
 
+	/* Depends on role switch class */
+	if (cdns->role_sw)
+		return 0;
+
 	pm_runtime_get_sync(cdns->dev);
 
 	current_role = cdns->role;
Signature · 1 line
2.17.1
#2 lore raw

Re: [PATCH 1/5] usb: cdns3: core: quit if it uses role switch class

Peter/Felipe,

Quoted 13 lines · Peter Chen

If the board uses role switch class for switching the role, it should not depends on SoC OTG hardware siginal any more, so quit early.

Signed-off-by Peter Chen <[email protected]>
---
  drivers/usb/cdns3/core.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/usb/cdns3/core.c +0 -0
index faee5ec5fc20..96c2da4e20c5 100644

drivers/usb/cdns3/core.c int cdns3_hw_role_switch(struct cdns3 *cdns) · line 286

Quoted 7 lines
drivers/usb/cdns3/core.c +4 -0
@@ -280,6 +280,10 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
  	enum usb_role real_role, current_role;
  	int ret = 0;
 
+	/* Depends on role switch class */
+	if (cdns->role_sw)
+		return 0;
+

This breaks h/w based role switching for us.

cdsn->role_sw would always be non zero, right, which means h/w based role swithching will be bypassed for all platforms.

drivers/usb/cdns3/core.c int cdns3_hw_role_switch(struct cdns3 *cdns) · line 289

Quoted 4 lines
drivers/usb/cdns3/core.c +0 -0
  	pm_runtime_get_sync(cdns->dev);
 
  	current_role = cdns->role;

cheers,
-roger

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
author
#3 lore raw

PATCH 2/5 usb: cdns3: gadget: set fast access bit

In reply to Peter Chen

Below is the recommendation from Cadence designer:
Using this bit to be sure that PHY clock is keeping up in active state. It's good to keep Fast Access bit enabled as long as there is any access to USB register.

It is used to fix the potential ARM core hang when visit controller register after DEVDS (.pullup is cleared) is set, the threaded irq may be scheduled at that time.

Cc Pawel Laszczak <[email protected]>
Reviewed-by Jun Li <[email protected]>
Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/cdns3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/cdns3/gadget.c +3 -0
index 2551901e8470..03b54c239944 100644
@@ -2783,6 +2783,8 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev)
 	/* enable generic interrupt*/
 	writel(USB_IEN_INIT, &regs->usb_ien);
 	writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, &regs->usb_conf);
+	/*  keep Fast Access bit */
+	writel(PUSB_PWR_FST_REG_ACCESS, &priv_dev->regs->usb_pwr);
 
 	cdns3_configure_dmult(priv_dev, NULL);
 }

@@ -2866,6 +2868,7 @@ static int cdns3_gadget_udc_stop(struct usb_gadget *gadget)
 
 	/* disable interrupt for device */
 	writel(0, &priv_dev->regs->usb_ien);
+	writel(0, &priv_dev->regs->usb_pwr);
 	writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf);
 
 	return 0;
Signature · 1 line
2.17.1
author
#4 lore raw

PATCH 3/5 usb: cdns3: gadget: clear the interrupt status when disconnect the host

In reply to Peter Chen

It is meaningless to handle any interrupts after disconnecting with host

Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/cdns3/gadget.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/cdns3/gadget.c +5 -2
index 03b54c239944..829bc917df39 100644
@@ -2739,10 +2739,13 @@ static int cdns3_gadget_pullup(struct usb_gadget *gadget, int is_on)
 {
 	struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget);
 
-	if (is_on)
+	if (is_on) {
 		writel(USB_CONF_DEVEN, &priv_dev->regs->usb_conf);
-	else
+	} else {
+		writel(~0, &priv_dev->regs->ep_ists);
+		writel(~0, &priv_dev->regs->usb_ists);
 		writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf);
+	}
 
 	return 0;
 }
Signature · 1 line
2.17.1
author
#5 lore raw

PATCH 4/5 usb: cdns3: drd: call PHY .set_mode accordingly

In reply to Peter Chen

Some PHYs may need to enter related mode, and do some settings.

Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/cdns3/drd.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/cdns3/drd.c +5 -0
index 5f2685cadf5e..31fddf84f226 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/iopoll.h>
 #include <linux/usb/otg.h>
+#include <linux/phy/phy.h>
 
 #include "gadget.h"
 #include "drd.h"

@@ -145,6 +146,7 @@ int cdns3_drd_host_on(struct cdns3 *cdns)
 	if (ret)
 		dev_err(cdns->dev, "timeout waiting for xhci_ready\n");
 
+	phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_HOST);
 	return ret;
 }
 

@@ -164,6 +166,7 @@ void cdns3_drd_host_off(struct cdns3 *cdns)
 	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
 				  !(val & OTGSTATE_HOST_STATE_MASK),
 				  1, 2000000);
+	phy_set_mode(cdns->usb3_phy, PHY_MODE_INVALID);
 }
 
 /**

@@ -190,6 +193,7 @@ int cdns3_drd_gadget_on(struct cdns3 *cdns)
 		return ret;
 	}
 
+	phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_DEVICE);
 	return 0;
 }
 

@@ -213,6 +217,7 @@ void cdns3_drd_gadget_off(struct cdns3 *cdns)
 	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
 				  !(val & OTGSTATE_DEV_STATE_MASK),
 				  1, 2000000);
+	phy_set_mode(cdns->usb3_phy, PHY_MODE_INVALID);
 }
 
 /**
Signature · 1 line
2.17.1
author
#6 lore raw

PATCH 5/5 usb: cdns3: gadget: move wait configuration operation

In reply to Peter Chen

After commit f4cfe5ce607d ("usb: cdns3: gadget: improve the set_configuration handling"), the software will inform the hardware the request has finished at cdns3_ep0_complete_setup. The configuration set bit is only set after request has finished, so it needs to move waiting operation after that. Meanwhile, if it is timeout, it will show warning message and return error.

Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/cdns3/ep0.c    | 10 +++++++++-
 drivers/usb/cdns3/gadget.c |  5 -----
 2 files changed, 9 insertions(+), 6 deletions(-)
drivers/usb/cdns3/ep0.c +9 -1
index d9779abc65b2..4761c852d9c4 100644
@@ -717,9 +717,17 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
 
 	/* send STATUS stage. Should be called only for SET_CONFIGURATION */
 	if (priv_dev->ep0_stage == CDNS3_STATUS_STAGE) {
+		u32 val;
+
 		cdns3_select_ep(priv_dev, 0x00);
 		cdns3_set_hw_configuration(priv_dev);
 		cdns3_ep0_complete_setup(priv_dev, 0, 1);
+		/* wait until configuration set */
+		ret = readl_poll_timeout_atomic(&priv_dev->regs->usb_sts, val,
+					  val & USB_STS_CFGSTS_MASK, 1, 100);
+		if (ret == -ETIMEDOUT)
+			dev_warn(priv_dev->dev, "timeout for waiting configuration set\n");
+
 		request->actual = 0;
 		priv_dev->status_completion_no_call = true;
 		priv_dev->pending_status_request = request;

@@ -731,7 +739,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
 		 * ep0_queue is back.
 		 */
 		queue_work(system_freezable_wq, &priv_dev->pending_status_wq);
-		return 0;
+		return ret;
 	}
 
 	if (!list_empty(&priv_ep->pending_req_list)) {
drivers/usb/cdns3/gadget.c +0 -5
index 829bc917df39..2ea4d30e1828 100644
@@ -1309,7 +1309,6 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev)
 {
 	struct cdns3_endpoint *priv_ep;
 	struct usb_ep *ep;
-	int val;
 
 	if (priv_dev->hw_configured_flag)
 		return;

@@ -1319,10 +1318,6 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev)
 	cdns3_set_register_bit(&priv_dev->regs->usb_conf,
 			       USB_CONF_U1EN | USB_CONF_U2EN);
 
-	/* wait until configuration set */
-	readl_poll_timeout_atomic(&priv_dev->regs->usb_sts, val,
-				  val & USB_STS_CFGSTS_MASK, 1, 100);
-
 	priv_dev->hw_configured_flag = 1;
 
 	list_for_each_entry(ep, &priv_dev->gadget.ep_list, ep_list) {
Signature · 1 line
2.17.1
#7 lore raw
In reply to Peter Chen
Quoted 9 lines · Peter Chen

Peter Chen (5):
usb: cdns3: core: quit if it uses role switch class
usb: cdns3: gadget: set fast access bit
usb: cdns3: gadget: clear the interrupt status when disconnect the
host
usb: cdns3: drd: call PHY .set_mode accordingly
usb: cdns3: gadget: move wait configuration operation

Quoted 7 lines
 drivers/usb/cdns3/core.c   |  4 ++++
 drivers/usb/cdns3/drd.c    |  5 +++++
 drivers/usb/cdns3/ep0.c    | 10 +++++++++-
 drivers/usb/cdns3/gadget.c | 15 ++++++++-------
 4 files changed, 26 insertions(+), 8 deletions(-)

--

A gentle ping.

Peter