Re: [PATCH 1/2] usb: dwc3: dwc3-qcom: Find USB connector and register role switch

[Jack Pham <jackp@codeaurora.org>]

On Tue, Jun 29, 2021 at 08:23:46PM +0100, Bryan O'Donoghue wrote:
> On 29/06/2021 16:48, Bjorn Andersson wrote:
> > What's wrong with the switch that dwc3_setup_role_switch() sets up?
> > 
> > That's what I've been using in my UCSI hacking on Snapdragon 888 and it
> > seems to work...

Bjorn are you exercising dual-role or just host mode?

> A good question, which as soon as you asked it made me completely doubt if
> I'd tested the tree without the patch applied.
> 
> I reverted both on my working tree and indeed it breaks role-switch
> detection.
> 
> In TCPM the connector is a child node of TCPM
> 
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=usb-next-5.13.rcx-rb5-tcpm
> line 1396
> 
> We use a remote endpoint inside of TCPM to hook up into &usb_1 line 1303
> 
> Not entirely sure what the graph tree looks like in your USCI case but I
> guess the connector is a child node of the controller ?
> 
> But I think your question is why bother with the role-switch in dwc3-qcom
> 
> dwc3_qcom_vbus_override_enable(){} is switching bits that probably ought to
> be in the PHY but for whatever reason ended up being buried in the qcom-dwc3
> wrapper.

This. When switching dwc3 into peripheral mode we also need
dwc3_qcom_vbus_override_enable() to write those registers to manually
drive the UTMI VBUS valid signal high to the controller since our SoCs
are never physically wired to the connector's VBUS. These registers
(QSCRATCH_{HS,SS}_PHY_CTRL) however don't belong to the PHYs but are
part of our dwc3 wrapper's IO map and hence dwc3-qcom is the only
appropriate place to handle them since dwc3-qcom has over the years
paired with several different PHYs depending on SoC.

Wesley's approach in $SUBJECT was to designate dwc3-qcom itself as a
usb-role-switch-able device so that in the DT the connector graph
endpoints would wire to the dwc3-qcom device itself instead of its dwc3
core child. The idea would be for dwc3-qcom would intercept the
role_switch_set call from TCPM/UCSI, call the vbus_override_enable() and
then pass on the notification to the child to let dwc3/drd.c handle the
rest.

IIRC there had been an alternate proposal to keep the role switch
connection only at the dwc3 core but in order to handle the vbus
override business an additional notification would have needed to be
done either from the usb_role_switch class driver or as an "upcall" from
dwc3 core -> glue. (found it, it was by you Bryan [1])

[1] https://lore.kernel.org/linux-usb/20200311191501.8165-7-bryan.odonoghue@linaro.org/

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project