Re: Explicit status phase for DWC3

From: Dan Scally <dan.scally@ideasonboard.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	rogerq@kernel.org
Subject: Re: Explicit status phase for DWC3
Date: Thu, 2 Feb 2023 15:45:24 +0000	[thread overview]
Message-ID: <9da07e03-7cd2-cfeb-8c67-4562948aa948@ideasonboard.com> (raw)
In-Reply-To: <Y9vONL8ZyQdEVkr0@rowland.harvard.edu>

On 02/02/2023 14:52, Alan Stern wrote:
> On Thu, Feb 02, 2023 at 10:12:45AM +0000, Dan Scally wrote:
>> (+CC roger as the author of the USB_GADGET_DELAYED_STATUS mechanism)
>>
>> On 26/01/2023 23:57, Thinh Nguyen wrote:
>>> We should already have this mechanism in place to do protocol STALL.
>>> Please look into delayed_status and set halt.
>>
>> Thanks; I tried this by returning USB_GADGET_DELAYED_STATUS from the
>> function's .setup() callback and later (after userspace checks the data
>> packet) either calling usb_ep_queue() or usb_ep_set_halt() and it does seem
>> to be working. This surprises me, as my understanding was that the purpose
>> of USB_GADGET_DELAYED_STATUS  is to pause all control transfers including
>> the data phase to give the function driver enough time to queue a request
>> (and possibly only for specific requests). Regardless though I think the
>> conclusion from previous discussions on this topic (see [1] for example) was
>> that we don't want to rely on USB_GADGET_DELAYED_STATUS to do this which is
>> why I had avoided it in the first place. A colleague made a series [2] some
>> time ago that adds a flag to usb_request which function drivers can set when
>> queuing the data phase request. UDC drivers then read that flag to decide
>> whether to delay the status phase until after another usb_ep_queue(), and
>> that's what I'm trying to implement here.
>>
>>
>> [1] https://lkml.org/lkml/2018/10/10/138
>>
>> [2] https://patchwork.kernel.org/project/linux-usb/patch/20190124030228.19840-5-paul.elder@ideasonboard.com/
> I'm in favor of the explicit_status approach from [2].  In fact, there
> was a whole series of patches impementing this, and I don't think any of
> them were merged.

Yep, I'm picking that series up and want to get it merged.

> Keep in mind that there are two separate issues here:
>
> 	Status/data stage for a control-IN or 0-length control-OUT
> 	transfer.
>
> 	Status stage for a non-0-length control-OUT transfer.
>
> The USB_GADGET_DELAYED_STATUS mechanism was meant to help with the
> first, not the second.  explicit_status was meant to help with the
> second; it may be able to help with both.

Ack - thanks. That thread I linked was very informative, I wish I'd 
found it sooner!

> Alan Stern