Re: [PATCH] usb: roles: intel_xhci: Don't use PMIC for port type identification

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

 



On Fri, Apr 20, 2018 at 11:16:05AM +0200, Hans de Goede wrote:
> 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.

That does not tell you anything. You need to check the status of the
device to know if it's there or not:

        % grep . /sys/bus/acpi/devices/INT33FE*/status

> 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.

Well, INT33F4 has DOS Device Name set to "XPOWER PMIC Controller". It
does not give any more hints regarding the USB connector.

I only used INT33FE quite simply because it is the HID used in the
driver that populates the device for fusb302.

But I can see now that _HID INT33FE is in practice used as the
identifier for a "type" of devices instead of identifier for specific
IP. That to be honest is pretty scary. _CID could have been used for
that purpose, but every IP should have its own _HID. It's of course
too late to complain about that.

INT33FE does not indeed seem like usable for this case.

> 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.

In these cases we don't really need the mux. Actually, we probable
should not even register the driver for it in these cases.

> 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.

Fair enough.

> 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?

The other way arround. The user-space control is not enabled when it
should be.

It's somekind of an automotive platform. I don't have the ACPI dump,
nor access to one, but I can ask for it. Right now I just know that
INT33F4 is not enabled on that platform, but user space control is
needed.

Braswells use CHT xHCI PCI ID AFAIK, so I checked the ACPI tables I
have for them. There was no node for INT33F4 at all. It could be that
they are using a Braswell based board.

In any case, it seems to be really difficult to reliable determine the
cases where we need the userspace control. Perhaps we should just
enable the control always with this mux driver after all? It's not
ideal when the port is Type-C port, but I don't think there is any
real risk of, for example, the user being able to break his/her board
with it at least.


Cheers,

-- 
heikki
--
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



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

  Powered by Linux