Hi Heikki,
On 20-04-18 10:06, Heikki Krogerus wrote:
This will add an array of known USB Type-C Port devices that
will be used as a blacklist for enabling userspace-control,
and remove the PMIC ACPI HID which was used for the same
purpose.
It turns out that on some CHT based platforms the X-Powers
PMIC is handled in firmware. The ACPI HID for it is
therefore not usable for determining the port type. The
driver now searches for known USB Type-C port devices
instead.
Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
---
Hi Hans,
So it seems that we can't rely on the PMIC. This is my proposal for a
fix. I'm in practice just using blacklist instead of whitelist for
identifying the port type.
Let me know if it's OK to you.
I'm afraid that this is not going to work, almost all CHT devices
(and some BYT devices to) define an INT33FE device, here is a quick grep
on a directory where I store dsdt files from various CHT and BYT devices:
[hans@shalem ~]$ ls /home/archive/hwinfo | wc -l
64
[hans@shalem ~]$ ack -l INT33FE /home/archive/hwinfo/*/dsdt.dsl | wc -l
36
So 36 out of 64 devices define an INT33FE device and at least half
of the devices there are BYT.
The INT33FE device is described as a "XPOWER Battery Device", so I'm
not sure why you are checking for this to determine if the Type-C
connector is controlled by firmware.
I have a bunch of CHT devices with a Type-C connector:
GPD win:
--------
This uses a Whiskey Cove PMIC combined with FUSB302 Type-C controller,
pi3usb30532. Which is all driven by the tcpm code.
Chuwi Hi8 Pro and Chuwi Vi8 Plus:
---------------------------------
These use a Type-C connector as a glorified micro-usb connector,
the Hi8 Pro supports Superspeed using a hardwired asmedia switch
to deal with upright / upside-down insertion. The Vi8 Plus is
USB2 only.
Host/device mode is determine by treating the Cc pin as an id-pin
(it has a gpio connected which is either low or high depending
on the Cc being pulled up/down).
These use an AXP288 PMIC and use the USB2 lines to BC-1.2 charger
detection there is no PD and no 3A / 1.5A / 0.5A detection based
on the Cc pull up resistor, resulting in charging at 0.5 A
when using a Type-C charger which does not do BC1.2 on its
USB2 lines.
As said these really treat the Type-C connector as a micro-sub
connector, so we need userspace control to be able to switch
to host mode when using an otg charging-hub (so that the user can
still use devices attached to the hub while charging).
Asus T100HA and Lenovo Ideapad Miix 320:
----------------------------------------
These have an USB host only Type-C connector, which does
do Superspeed (likely contain a fixed switch to deal with upright /
upside-down insertion).
These cannot charge at all through the Type-C connector (they
do turn of the 5v boost when used in device mode), theoretically
the port may work in device mode (depending on which lanes they
used), but the device-mode USB controller is disabled by the BIOS
with no option to change it.
I also have 5 different CHT devices with a micro-usb connector:
-Cube iWork8 Air
-Jumper Ezpad mini 3
-Pipo W2s
-Teclast X80 pro
-Lenovo Ideapad Miix 310
All of which use an AXP288 PMIC and can do charging, host and
device mode through their micro-usb connector with the
exception of the Lenovo Ideapad Miix 310 where the micro-usb
is host mode only (like the Type-C on the 320) and there is
a separate power-barrel connector for charging.
All these define an INT33FE device, although at least on
the Cube and Lenovo ones the INT33FE's device _STA returns 0,
so your check should not match, but on others the _STA for
the INT33FE device does return 0x0f.
TL;DR: Checking the INT33FE device to determine if a Type-C
connector is present is not a good way to handle this.
Can you describe the device on which you are seeing
userspace control being enabled even though it should not be
enabled and maybe mail me an ACPI dump for it?
Regards,
Hans
Thanks,
---
.../usb/roles/intel-xhci-usb-role-switch.c | 40 +++++++++++--------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index de72eedb762e..1696d1f25769 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -38,19 +38,26 @@ struct intel_xhci_usb_data {
void __iomem *base;
};
-struct intel_xhci_acpi_match {
- const char *hid;
- int hrv;
+static const struct acpi_device_id typec_port_devices[] = {
+ { "PNP0CA0", 0 }, /* UCSI */
+ { "INT33FE", 0 }, /* CHT USB Type-C PHY */
+ { },
};
-/*
- * ACPI IDs for PMICs which do not support separate data and power role
- * detection (USB ACA detection for micro USB OTG), we allow userspace to
- * change the role manually on these.
- */
-static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
- { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */
-};
+static acpi_status intel_xhci_userspace_ctrl(acpi_handle handle, u32 level,
+ void *ctx, void **ret)
+{
+ struct acpi_device *adev;
+
+ if (acpi_bus_get_device(handle, &adev))
+ return AE_OK;
+
+ /* Don't allow userspace-control with Type-C ports */
+ if (!acpi_match_device_ids(adev, typec_port_devices))
+ return AE_ACCESS;
+
+ return AE_OK;
+}
static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
{
@@ -137,7 +144,7 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct intel_xhci_usb_data *data;
struct resource *res;
- int i;
+ acpi_status status;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -148,10 +155,11 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
if (!data->base)
return -ENOMEM;
- for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
- if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
- allow_userspace_ctrl_ids[i].hrv))
- sw_desc.allow_userspace_control = true;
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, intel_xhci_userspace_ctrl,
+ NULL, NULL, NULL);
+ if (ACPI_SUCCESS(status))
+ sw_desc.allow_userspace_control = true;
platform_set_drvdata(pdev, data);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
