Re: [PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role switch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Saranya,

On 29-08-19 12:42, 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@xxxxxxxxx>
Signed-off-by: Balaji Manoharan <m.balaji@xxxxxxxxx>

Unfortunately this patch conflicts with a patch to
drivers/usb/roles/intel-xhci-usb-role-switch.c from Heikki
which is already in -next, see:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=devprop

And specifically this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=devprop&id=d2a90ebb65536a6deeb9daf5aa8e0700e8cbb43a

So you need to rebase on op of that branch and then the subsys
maintainers need to figure out how to merge this, I guess
the usb-next tree can merge Rafael's devprop branch for this?

I've manually resolved the conflict locally and  given this new version
a test-run on a Cherry Trail device and things still work as they should
there, so with the conflict fixed this series is:

Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Regards,

Hans


---
  changes since v3: Initialized drd_config variable to fix warning
  changes since v2: Revised SoB tags
  changes since v1: Added drd_config to avoid multiple if checks
                    Other minor changes suggested by Hans
drivers/usb/roles/intel-xhci-usb-role-switch.c | 27 ++++++++++++++++++++++++--
  1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 277de96..88d0416 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)
@@ -45,6 +53,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
  	unsigned long timeout;
  	acpi_status status;
  	u32 glk, val;
+	u32 drd_config = DRD_CONFIG_DYNAMIC;
/*
  	 * On many CHT devices ACPI event (_AEI) handlers read / modify /
@@ -59,24 +68,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 +167,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);




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux