[PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Sjoerd Simons]

On Fri, 2024-01-12 at 14:56 +0200, Roger Quadros wrote:
> 
> 
> On 12/01/2024 13:06, Sjoerd Simons wrote:
> > On Fri, 2024-01-12 at 12:39 +0200, Roger Quadros wrote:
> > > 
> > > 
> > > On 12/01/2024 10:52, Sjoerd Simons wrote:
> > > > When dr_mode is "otg" the dwc3 is initially configured in _OTG
> > > > mode;
> > > > However in this mode the gadget functionality doesn't work
> > > > without
> > > > further configuration. To resolve that on gadget start switch
> > > > to
> > > > _DEVICE
> > > > mode globally and go back to _OTG on stop again.
> > > > 
> > > > For this the dwc3_set_mode is renamed to dwc3_core_set_mode to
> > > > avoid a
> > > > conflict with the same function exposed by xhci-dwc3
> > > 
> > > Aren't they both doing the same thing? I'd rather define them at
> > > one
> > > place
> > > i.e. dwc3/core.c.
> > 
> > They twiddle the same registers afaict indeed; but the way to get
> > there
> > is rather different. So having just one for both really needs a
> > bigger
> > amount of rework.
> > 
> > > The driver model design for dwc3 looks really broken in u-boot.
> > > 
> > > "snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by
> > > dwc3/core.c.
> > > This is probably because at the time USB host was the more needed
> > > use
> > > case at u-boot.
> > > 
> > > Ideally dwc3/core.c should claim "snps,dwc3" device and
> > > instantiate
> > > the respective Host/Device/OTG device based on the provided otg
> > > mode.
> > > 
> > > For Host/Device mode it is straight forwa
> > > dwc3/core.c does dwc3_set_mode() depending on the mode and:
> > > for host mode -> register xhci-usb driver.
> > > for device mode -> register UDC device driver.
> > > 
> > > For dual-role mode, the solution is not very clear as I don't
> > > think
> > > there is a role switch framework present
> > > 
> > > To begin with, we could probably restrict it to just device mode
> > > as most platforms were forcing it to device mode in any case as
> > > they
> > > usually have a 2nd USB port that is host only.
> > 
> > Right we don't have role switching; And if we had we'd have to add
> > support of the Type-C controller on the SK boards which determines
> > the
> > roles for this.
> > 
> > This one more minimal patch was modelled after the discussion last
> > year
> > around otg mode[0]. And similar to how the cdns3 driver handles it.
> > Essentially letting host/gadget mode be determined by the usage
> > rather
> > then the cables plugged, which is not pretty but works.
> > 
> > While I agree with you this could all be a lot nicer of u-boot did
> > "proper" OTG/role-switching; I unfortunately don't have the
> > bandwidth
> > to introduce all of that and it seems a shame to block DFU support
> > on
> > it. For reference my previous series just set the peripheral role
> > in
> > the dts which reflects your suggestion of forcing device mode. I'd
> > be
> > wary to do so in the driver because i would hope the OTG handling
> > in
> > dwc3 is there for a reason :) 
> > 
> > 
> > 0: https://lists.denx.de/pipermail/u-boot/2023-August/527709.html
> > 
> > 
> 
> In your current series, how does it work?
> What happens if user starts both host and device controllers?

Testing host functionality on the same port as used for DFU (the only
one that's otg) doesn't seem to work at all, not sure why that is. host
on usb1 (usb-a port, host only) works fine ofcourse.

practically however once the user has used a gadget it's likely they'd
have to reset the host side (or at least do a rescan) though as likely
they'll have also switched around the cable :).

> 
> e.g.
> > usb start
> > dfu 0

That seems to work fine, modulo usb0 not seeing attached devices, but
that's not a regression introduced by this patch.



-- 
Sjoerd Simons
Collabora Ltd.