diff mbox

[RFC,5/5] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

Message ID 1481612990-23409-6-git-send-email-john.stultz@linaro.org
State New
Headers show

Commit Message

John Stultz Dec. 13, 2016, 7:09 a.m. UTC 
From: Chen Yu <chenyu56@huawei.com>


The Hi6220's usb controller is limited in that it does not
support "Split Transactions", so it does not support communicating
with low-speed and full-speed devices behind a high-speed hub.

Thus it requires a quirk so that we can manually drop the usb
speed when low/full-speed are attached, and bump back to high
speed when they are removed.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu <chenyu56@huawei.com>

[jstultz: Reworked to simplify the patch, and made
 commit log to be more specific about the issue]
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
v2:
* Fix build issue reported by kbuildbot
* Rework to avoid using new dts entry suggested by RobH
* Further tweaks from Chen Yu to try to address comments from
  John Youn
* Further simplified logic
---
 drivers/usb/dwc2/core.h     |  6 +++++
 drivers/usb/dwc2/hcd.c      | 60 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/platform.c |  1 +
 3 files changed, 67 insertions(+)

-- 
2.7.4

Comments

Alan Stern Dec. 13, 2016, 1:24 p.m. UTC | #1 
On Mon, 12 Dec 2016, John Stultz wrote:

> From: Chen Yu <chenyu56@huawei.com>

> 

> The Hi6220's usb controller is limited in that it does not

> support "Split Transactions", so it does not support communicating

> with low-speed and full-speed devices behind a high-speed hub.

> 

> Thus it requires a quirk so that we can manually drop the usb

> speed when low/full-speed are attached, and bump back to high

> speed when they are removed.


Just out of curiosity (I know nothing about this hardware), what 
happens if there is a high-speed hub plugged into the host controller 
and both a high-speed and a full-speed device plugged into the hub?  

Do you end up forcing the high-speed device to run at full speed?

Alan Stern
John Stultz Dec. 13, 2016, 7:23 p.m. UTC | #2 
On Tue, Dec 13, 2016 at 5:24 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 12 Dec 2016, John Stultz wrote:

>

>> From: Chen Yu <chenyu56@huawei.com>

>>

>> The Hi6220's usb controller is limited in that it does not

>> support "Split Transactions", so it does not support communicating

>> with low-speed and full-speed devices behind a high-speed hub.

>>

>> Thus it requires a quirk so that we can manually drop the usb

>> speed when low/full-speed are attached, and bump back to high

>> speed when they are removed.

>

> Just out of curiosity (I know nothing about this hardware), what


If your interested in details, page 12 of the pdf here has some details:
https://github.com/96boards/documentation/blob/master/ConsumerEdition/HiKey/AdditionalDocs/HiKey_Hardware_User_Manual_Rev0.2.pdf

There's also schematics for the board available (if you are interested
in that sort of stuff). You can find the USB bits on Page 4 here:
https://github.com/96boards/documentation/blob/master/ConsumerEdition/HiKey/HardwareDocs/HiKey_schematics_LeMaker_version_Rev_A1.pdf


> happens if there is a high-speed hub plugged into the host controller

> and both a high-speed and a full-speed device plugged into the hub?

>

> Do you end up forcing the high-speed device to run at full speed?


Yes. It drops back to full-speed.

thanks
-john
diff mbox

Patch

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 963ea1b..de1fa0c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -417,6 +417,11 @@  enum dwc2_ep0_state {
  *			needed.
  *			0 - No (default)
  *			1 - Yes
+ * @change_speed_quirk: Change speed configuration to DWC2_SPEED_PARAM_FULL
+ *                      while full&low speed device connect. And change speed
+ *                      back to DWC2_SPEED_PARAM_HIGH while device is gone.
+ *			0 - No (default)
+ *			1 - Yes
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -457,6 +462,7 @@  struct dwc2_core_params {
 	int uframe_sched;
 	int external_id_pin_ctl;
 	int hibernation;
+	int change_speed_quirk;
 };
 
 /**
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 008f5bc..7cf8d8e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4873,6 +4873,61 @@  static void _dwc2_hcd_clear_tt_buffer_complete(struct usb_hcd *hcd,
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 }
 
+/*
+ * HPRT0_SPD_HIGH_SPEED: high speed
+ * HPRT0_SPD_FULL_SPEED: full speed
+ */
+static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (hsotg->core_params->speed == speed)
+		return;
+
+	hsotg->core_params->speed = speed;
+	queue_work(hsotg->wq_otg, &hsotg->wf_otg);
+}
+
+static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (!hsotg->core_params->change_speed_quirk)
+		return;
+
+	/*
+	 * On removal, set speed to default high-speed.
+	 */
+	if (udev->parent && udev->parent->speed > USB_SPEED_UNKNOWN &&
+	    udev->parent->speed < USB_SPEED_HIGH) {
+		dev_info(hsotg->dev, "Set speed to default high-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+	}
+}
+
+static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (!hsotg->core_params->change_speed_quirk)
+		return 0;
+
+	if (udev->speed == USB_SPEED_HIGH) {
+		dev_info(hsotg->dev, "Set speed to high-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+	} else if ((udev->speed == USB_SPEED_FULL ||
+				udev->speed == USB_SPEED_LOW)) {
+		/*
+		 * Change speed setting to full-speed if there's
+		 * a full-speed or low-speed device plugged in.
+		 */
+		dev_info(hsotg->dev, "Set speed to full-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
+	}
+
+	return 0;
+}
+
 static struct hc_driver dwc2_hc_driver = {
 	.description = "dwc2_hsotg",
 	.product_desc = "DWC OTG Controller",
@@ -5028,6 +5083,11 @@  int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 			dev_warn(hsotg->dev, "can't set coherent DMA mask\n");
 	}
 
+	if (hsotg->core_params->change_speed_quirk) {
+		dwc2_hc_driver.free_dev = dwc2_free_dev;
+		dwc2_hc_driver.reset_device = dwc2_reset_device;
+	}
+
 	hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev));
 	if (!hcd)
 		goto error1;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 8e1728b..17eb5fd 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -85,6 +85,7 @@  static const struct dwc2_core_params params_hi6220 = {
 	.uframe_sched			= 0,
 	.external_id_pin_ctl		= -1,
 	.hibernation			= -1,
+	.change_speed_quirk		= 1,
 };
 
 static const struct dwc2_core_params params_bcm2835 = {