Re: [PATCH v3 00/12] USB Type-C device-connection, mux and switch support

[Hans de Goede]

Hi,

On 26-02-18 11:59, Andy Shevchenko wrote:
> On Mon, Feb 26, 2018 at 11:09 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > Hi All,
> >
> > Here is version 3 of Heikki's and my USB Type-C device-connection, mux and
> > switch support series. Version 2 and 3 bring various small code and style
> > fixes based on review (no major changes).
> >
> > Here is the original cover-letter of v1:
> >
> > Some devices with an USB Type-C connector have a bunch of muxes
> > behind that connector which need to be controlled by the kernel (rather
> > then having them controlled by firmware as on most devices).
> >
> > Quite a while back I submitted a patch-series to tie together these muxes
> > and the Type-C Port Manager (tcpm) code, using the then new drivers/mux
> > framework. But the way I used the mux framework went against what it was
> > designed for, so in the end that series got nowhere.
> >
> > Heikki Krogerus from Intel, who maintains the USB TYPEC subsystem, has
> > recently been working on solving the same problem for some boards he is
> > doing hardware-enablement for.
> >
> > Heikki has come up with a number of infrastructure patches for this.
> > The first one is a new device-connection framework. This solves the
> > problem of describing non bus device-links on x86 in what in my experience
> > with this problematic area is a really nice simple, clean and *generic*
> > way. This could for example in the near future also replace the custom
> > lookup code in the pwm subsys and the custom pwm_add_table() /
> > pwm_remove_table() functions.
> >
> > The other 3 patches add a framework for the different type of Type-C /
> > USB "muxes".
> >
> > Heikki and I have gone through a number of iterations of these patches
> > together and we believe these are now ready for merging. Since merging
> > infrastructure patches without users is not done and Heikki's own use-case
> > for these is not yet ready for merging, the rest of this series consists
> > of patches by me to make the Type-C connector found on some Cherry Trail
> > devices (finally) be able to actually work as an USB port and not just
> > a charge port.
> >
> > The last patch uses the new usb-role-switch framework to also do proper
> > devcie / host switching on CHT devices with a USB micro AB connector.
> > This is also a big feature for CHT users, because before this they had
> > to do a reboot to get an OTG-host cable recognized (on some devices).
> >
> > Part of this series is an usb-role-switch driver for the role-switch
> > found inside the xhci controller on e.g. CHT devices, this is currently
> > implemented as the generic xhci controller instantiating a platform
> > child-device for this, since this really is a separate chunk of HW
> > which happens to sit in the XHCI mmio space. This approach may not be
> > universally liked, given that in this new series the role-switch driver
> > is much smaller and does not have any external deps anymore we could
> > just integrate it into the xhci code if that is preferred.
> >
> > About merging this series (once everything is reviewed, etc.), there are
> > quite some interdependencies in it esp. a lot of the patches depend on
> > the first patch. Luckily patches 1-10 all apply to subsystems which are
> > maintained by Greg (most to the USB subsys). Which just leaves patches
> > 11 and 12 once 1-10 are merged. Greg, can you create an immutable branch
> > for the platform/x86 and extcon maintainers to merge once this is done?
>
> Didn't have time to comment on v2, so here we are:
> you are using in even the same file two styles, i.e. IS_ERR_OR_NULL
> vs. !x || IS_ERR(x) (and negative ones).

Good catch, I only see this in "usb: common: Small class for USB role switches",
will fix for v4.

> Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

So with the above fixed I can apply your reviewed-by to the entire series?

Regards,

Hans