Re: [RFC][PATCH 2/3] usb: roles: Add usb role switch notifier.

From: Hans de Goede
Date: Fri Oct 18 2019 - 04:06:27 EST


Hi,

On 18-10-2019 07:55, John Stultz wrote:
On Wed, Oct 16, 2019 at 12:27 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
On 10/15/19 7:39 AM, John Stultz wrote:
On Thu, Oct 3, 2019 at 1:51 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
On 03-10-2019 22:37, John Stultz wrote:
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.

Thanks again for the feedback here. Sorry for the slow response. Been
reworking some of the easier changes but am starting to look at how to
address your feedback here.

What is controlling the usb-role-switch, and thus ultimately
causing the notifier you are suggesting to get called ?

The tcpm_mux_set() call via tcpm_state_machine_work()

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.

So I'm looking at the bindings and I'm not sure exactly how to tie a
regulator style driver into the tcpm for this?
Looking at the driver I just see this commented out bit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/tcpm/tcpm.c#n3075

Do you happen to have a pointer to something closer to what you are describing?

Look at the tcpm_set_vbus implementation in drivers/usb/typec/tcpm/fusb302.c
you need to do something similar in your Type-C controller driver and
export the GPIO as as a gpio-controlled regulator and tie the regulator to
the connector.

Thanks for the suggestion, I really appreciate it! One more question
though, since I'm using the tcpci_rt1711h driver, which re-uses the
somewhat sparse tcpci.c implementation, would you recommend trying to
add generic regulator support to the tcpci code or trying to extend
the implementation somehow allow the tcpci_rt1711h driver replace just
the set_vbus function?

I have the feeling that this is more of a question for Heikki.

My first instinct is: if you are using tcpci can't you put all
the hacks you need for the usb connection shared between hub
and type-c in your firmware ?

Regards,

Hans