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

[Roger Quadros <rogerq@kernel.org>]

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?

e.g.
> usb start
> dfu 0

-- 
cheers,
-roger