diff mbox series

[v2,2/2] usb: roles: intel: Enable static DRD mode for role switch

Message ID 1567004932-16967-3-git-send-email-saranya.gopal@intel.com (mailing list archive)
State New, archived
Headers show
Series usb: roles: intel: Use static mode by default | expand

Commit Message

Saranya Gopal Aug. 28, 2019, 3:08 p.m. UTC 
Enable static DRD mode in Intel platforms which guarantees
successful role switch all the time. This fixes issues like
software role switch failure after cold boot and issue with
role switch when USB 3.0 cable is used. But, do not enable
static DRD mode for Cherrytrail devices which rely on firmware
for role switch.

Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
Signed-off-by: Balaji Manoharan <m.balaji@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/roles/intel-xhci-usb-role-switch.c | 28 +++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Comments

Heikki Krogerus Aug. 28, 2019, 4:06 p.m. UTC | #1 
On Wed, Aug 28, 2019 at 08:38:52PM +0530, Saranya Gopal wrote:
> Enable static DRD mode in Intel platforms which guarantees
> successful role switch all the time. This fixes issues like
> software role switch failure after cold boot and issue with
> role switch when USB 3.0 cable is used. But, do not enable
> static DRD mode for Cherrytrail devices which rely on firmware
> for role switch.
> 
> Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
> Signed-off-by: Balaji Manoharan <m.balaji@intel.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Ditto. Can't use my SoB anymore.

Note. Since I only proposed using the device property, in this patch
you should also not use my "Suggested-by" tag. So I think it is OK to
use it in the first patch of this series, but not in this second patch.

thanks,
Saranya Gopal Aug. 28, 2019, 4:58 p.m. UTC | #2 
> On Wed, Aug 28, 2019 at 08:38:52PM +0530, Saranya Gopal wrote:
> > Enable static DRD mode in Intel platforms which guarantees
> > successful role switch all the time. This fixes issues like
> > software role switch failure after cold boot and issue with
> > role switch when USB 3.0 cable is used. But, do not enable
> > static DRD mode for Cherrytrail devices which rely on firmware
> > for role switch.
> >
> > Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
> > Signed-off-by: Balaji Manoharan <m.balaji@intel.com>
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> Ditto. Can't use my SoB anymore.
> 
> Note. Since I only proposed using the device property, in this patch
> you should also not use my "Suggested-by" tag. So I think it is OK to
> use it in the first patch of this series, but not in this second patch.

Sure, let me remove the tag and resend the patches.
> 
> thanks,
> 
> --
> heikki
diff mbox series

Patch

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 277de96..a903bef 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -19,6 +19,7 @@ 
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/usb/role.h>
 
 /* register definition */
@@ -26,6 +27,12 @@ 
 #define SW_VBUS_VALID			BIT(24)
 #define SW_IDPIN_EN			BIT(21)
 #define SW_IDPIN			BIT(20)
+#define SW_SWITCH_EN			BIT(16)
+
+#define DRD_CONFIG_DYNAMIC		0
+#define DRD_CONFIG_STATIC_HOST		1
+#define DRD_CONFIG_STATIC_DEVICE	2
+#define DRD_CONFIG_MASK			3
 
 #define DUAL_ROLE_CFG1			0x6c
 #define HOST_MODE			BIT(29)
@@ -37,6 +44,7 @@ 
 struct intel_xhci_usb_data {
 	struct usb_role_switch *role_sw;
 	void __iomem *base;
+	bool enable_sw_switch;
 };
 
 static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
@@ -44,7 +52,7 @@  static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
 	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
 	unsigned long timeout;
 	acpi_status status;
-	u32 glk, val;
+	u32 glk, val, drd_config;
 
 	/*
 	 * On many CHT devices ACPI event (_AEI) handlers read / modify /
@@ -59,24 +67,35 @@  static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
 
 	pm_runtime_get_sync(dev);
 
-	/* Set idpin value as requested */
+	/*
+	 * Set idpin value as requested.
+	 * Since some devices rely on firmware setting DRD_CONFIG and
+	 * SW_SWITCH_EN bits to be zero for role switch,
+	 * do not set these bits for those devices.
+	 */
 	val = readl(data->base + DUAL_ROLE_CFG0);
 	switch (role) {
 	case USB_ROLE_NONE:
 		val |= SW_IDPIN;
 		val &= ~SW_VBUS_VALID;
+		drd_config = DRD_CONFIG_DYNAMIC;
 		break;
 	case USB_ROLE_HOST:
 		val &= ~SW_IDPIN;
 		val &= ~SW_VBUS_VALID;
+		drd_config = DRD_CONFIG_STATIC_HOST;
 		break;
 	case USB_ROLE_DEVICE:
 		val |= SW_IDPIN;
 		val |= SW_VBUS_VALID;
+		drd_config = DRD_CONFIG_STATIC_DEVICE;
 		break;
 	}
 	val |= SW_IDPIN_EN;
-
+	if (data->enable_sw_switch) {
+		val &= ~DRD_CONFIG_MASK;
+		val |= SW_SWITCH_EN | drd_config;
+	}
 	writel(val, data->base + DUAL_ROLE_CFG0);
 
 	acpi_release_global_lock(glk);
@@ -147,6 +166,9 @@  static int intel_xhci_usb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
+	data->enable_sw_switch = !device_property_read_bool(dev,
+						"sw_switch_disable");
+
 	data->role_sw = usb_role_switch_register(dev, &sw_desc);
 	if (IS_ERR(data->role_sw))
 		return PTR_ERR(data->role_sw);