Re: [PATCH] usb: dwc3: Support EBC feature of DWC_usb31

[Thinh Nguyen]

Hi Greg/Manan,

On Fri, Nov 17, 2023, Thinh Nguyen wrote:
> On Thu, Nov 16, 2023, Manan Aurora wrote:
> > On Sat, Nov 11, 2023 at 4:39 AM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Nov 02, 2023, Manan Aurora wrote:
> > > > Support configuration and use of bulk endpoints in the so-called EBC
> > > > mode described in the DBC_usb31 databook (appendix E)
> > > >
> > > > Added a bit fifo_mode to usb_ep to indicate to the UDC driver that a
> > > > specific endpoint is to operate in the EBC (or equivalent) mode when
> > > > enabled
> > >
> > > This should be unique to dwc3, and it's only for bulk. I don't think
> > > usb_ep or the user of usb_ep should know this.
> > 
> > In our use case we have a function driver that configures an allocated bulk
> > endpoint to operate as an EBC EP. So the function driver already depends on the
> > feature.
> 
> This should be abstracted from the function driver. The function driver
> should not need to know about this feature.
> 
> > 
> > dwc3_ep seems like the correct place to put this field but a function
> > driver that allocates
> > EPs and configures them for this use case would need to include dwc3 headers.
> > If other vendors offer an equivalent feature this dependency would
> > become an issue.
> > 
> > Exporting a symbol from dwc3 is an easy option but dwc3 doesn't
> > currently export symbols
> > hence I tried to avoid that
> > 
> > > Also since DWC3_DEPCFG_EBC_HWO_NOWB must be set, the controller does not
> > > write back to the TRB. Did you handle how the driver would update the
> > > usb request on completion? (e.g. how much was transferred).
> > 
> > In our use case, we intend to have a link TRB and issue a startXfer
> > command. Completion
> > handling and continuing the transfer will be offloaded to dedicated
> > FIFO hardware.
> > But we can definitely rework this to disable no-writeback mode by
> > default and allow this to
> > be separately enabled
> > 
> 
> Ok.
> 

Looks like this change was applied to Greg's branch. Unless I'm missing
something, I don't think my concerns are addressed yet. Here are the
reiteration of the concerns:

1) The gadget driver should not need to know the dwc3's FIFO mode. It's
specific to dwc3 capability and should be handled in dwc3 driver only.
Usually these properties are selected in device tree bindings and not
specified through the gadget driver API.

2) This specific mode "EBC" doesn't write back to TRBs. It's not clear
how this is handled when updating the request's status. It's also only
applicable to bulk endpoint. If it's to be applied to the usb gadget
API, it's not documented fully.

Thanks,
Thinh