On Thu, Oct 03, 2019 at 10:51:38PM +0200, Hans de Goede wrote: > Hi, > > On 03-10-2019 22:37, John Stultz wrote: > > On Thu, Oct 3, 2019 at 2:25 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > > On 03-10-2019 01:16, John Stultz wrote: > > > > From: Yu Chen <chenyu56@xxxxxxxxxx> > > > > > > > > This patch adds notifier for drivers want to be informed of the usb role > > > > switch. > > > > > > I do not see any patches in this series actually using this new > > > notifier. > > > > > > Maybe it is best to drop this patch until we actually have in-kernel > > > users of this new API show up ? > > > > Fair point. I'm sort of taking a larger patchset and trying to break > > it up into more easily reviewable chunks, but I guess here I mis-cut. > > > > The user is the hikey960 gpio hub driver here: > > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=b06158a2d3eb00c914f12c76c93695e92d9af00f > > Hmm, that seems to tie the TypeC data-role to the power-role, which > is not going to work with role swapping. > > What is controlling the usb-role-switch, and thus ultimately > causing the notifier you are suggesting to get called ? > > Things like TYPEC_VBUS_POWER_OFF and TYPEC_VBUS_POWER_ON > really beg to be modeled as a regulator and then the > Type-C controller (using e.g. the drivers/usb/typec/tcpm/tcpm.c > framework) can use that regulator to control things. > in case of the tcpm.c framework it can then use that > regulator to implement the set_vbus callback. > > You really do not want to tie this do the usb_switch, both > because doing so ties the data and power-roles together > which is not supposed to happen and because role-swapping > requires careful timing of the VBUS on / off at different > moments then the moments when you actually set the mux/switch > for connecting the Dp/Dn lines to the host or gadget > controller. > > The usb role switch abstraction is really only intended > for the data-lines switch and should not be tied together > with other stuff. Hear, hear. -- heikki
