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

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 29 Jun 14:23 CDT 2021, 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...
> > 
> > Regards,
> > Bjorn
> 
> 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
> 

That's expected, because the dwc3 role switching logic registers on
&usb_1_dwc3 instead of &usb_1.

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

No, the connector currently a child of a completely different rpmsg
device, i.e. in system terms it's the same as your design. One thing
detecting the change in parameters and then invoking the change on the
dwc3 controller (not the qcom variant thereof).

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

I missed this part when I wired up my role switcher and things works
fine, but I presume there's a good reason for the vbus override and it's
not unlikely that I missed something in my testing.

I do however think that it would be appropriate to use the generic dwc3
role switcher and then make that somehow poke the vbus_override in the
qcom instance.

> Certainly we want that for qcs405 if you remember - I'm assuming for the
> sm8x50 too.
> 
> Even if we shouldn't twiddle these bits on sm8x50 I believe its wanted on
> qcs405.
> 

Can you remind me about the purpose of these bits?

Regards,
Bjorn

> I'm open to correction on that one though
> 
> ---
> bod