Hi Saranya, Heikki,
On 07-09-18 10:18, Heikki Krogerus wrote:
+Hans
On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.gopal@xxxxxxxxx wrote:
From: Saranya Gopal <saranya.gopal@xxxxxxxxx>
This patch adds static DRD mode for host/device
mode switch. This fixes the issue where device
mode was not working after DUT switches to host
mode with 3.0 OTG connector.
What is the DUT?
Anyways this patch cannot go upstream as is, it will break
DRD switching on many Cherry Trail devices, on Cherry Trail
devices the role is often controlled through firmware using
these 2 ACPI methods called from an edge triggered _AEI
handler:
Scope (PCI0)
{
OperationRegion (XOP1, SystemMemory, BR0X, 0x80F8)
Field (XOP1, DWordAcc, NoLock, Preserve)
{
Offset (0x80D4),
, 11,
BT11, 1,
, 8,
BT20, 1,
BT21, 1,
Offset (0x80D7),
BT24, 1
}
Method (CDRH, 1, Serialized)
{
If (DAMT == Zero)
{
BT20 = Zero
If (Arg0 == Zero)
{
BT24 = Zero
}
Else
{
BT24 = One
}
BT11 = One
BT21 = One
}
Else
{
// same thing through IOSF side bus
}
}
Method (CDRD, 1, Serialized)
{
If (DAMT == Zero)
{
BT11 = One
BT20 = One
BT21 = One
If (Arg0 == Zero)
{
BT24 = Zero
}
Else
{
BT24 = One
}
}
Else
{
// same thing through IOSF side bus
}
}
Note that these only control the SW_IDPIN (bit 20) to
change the role and rely on SW_SWITCH_ENABLE (bit 16)
and DRD_CONFIG (bit 0:1) to be all 0.
This patch as suggested breaks these ACPI methods.
Note that even though the mux is changed by firmware the
intel_xhci_usb_set_role() may (and typically will) still
run on these devices, there are 2 ways this can happen:
1) On many devices the firmware only switches between
the host and none roles (it does not set the SW_VBUS_VALID
bit), because it is targeting Windows.
This means that device mode cannot work because the
designware dwc3 controller will not operate as a device
without the SW_VBUS_VALID bit set.
To fix this the axp288_extcon code monitors vbus changes
and when the vbus becomes present it checks if the firmware
put the role-switch in the none role and if it did it changes
the role to device, calling intel_xhci_usb_set_role()
2) The user may manually change the role through sysfs, to
deal with otg-charging hubs (ACA devices) which this hardware
typically cannot detect.
If the proposed change is necessary to fix things on some non
Cherry Trail hardware, you could do a new version of this
patch which only does this on affected hardware.
Regards,
Hans
Signed-off-by: Saranya Gopal <saranya.gopal@xxxxxxxxx>
Signed-off-by: M Balaji <m.balaji@xxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
---
changes since V5: Corrected the name format in From and Signed-off-by
changes since V4: Removed change-Id
changes since V3: Added Reviewed-by Sathyanarayanan tag
changes since V2: Incorporated Heikki's review comments and added Reviewed-by Heikki tag
changes since V1: none
drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index dad2d19..0d1ea82 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -25,6 +25,9 @@
#define SW_VBUS_VALID BIT(24)
#define SW_IDPIN_EN BIT(21)
#define SW_IDPIN BIT(20)
+#define SW_SWITCH_EN_CFG0 BIT(16)
+#define SW_DRD_STATIC_HOST_CFG0 1
+#define SW_DRD_STATIC_DEV_CFG0 2
#define DUAL_ROLE_CFG1 0x6c
#define HOST_MODE BIT(29)
@@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+ val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0);
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+ val &= ~SW_DRD_STATIC_DEV_CFG0;
+ val |= SW_DRD_STATIC_HOST_CFG0;
break;
case USB_ROLE_DEVICE:
val |= SW_IDPIN;
val |= SW_VBUS_VALID;
+ val &= ~SW_DRD_STATIC_HOST_CFG0;
+ val |= SW_DRD_STATIC_DEV_CFG0;
break;
}
- val |= SW_IDPIN_EN;
+ val |= SW_IDPIN_EN | SW_SWITCH_EN_CFG0;
writel(val, data->base + DUAL_ROLE_CFG0);
--
2.7.4
Thanks,
