Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 11 Jan 2019 03:43:46 -0500
From:   Paul Elder <paul.elder@ideasonboard.com>
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com,
        b-liu@ti.com, rogerq@ti.com, balbi@kernel.org,
        gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 0/6] usb: gadget: add mechanism to asynchronously
 validate data stage of ctrl out request
Message-ID: <20190111084346.GC32268@localhost.localdomain>
References: <20190109070856.27460-1-paul.elder@ideasonboard.com>
 <Pine.LNX.4.44L0.1901101520090.1206-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Pine.LNX.4.44L0.1901101520090.1206-100000@iolanthe.rowland.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jan 10, 2019 at 03:39:25PM -0500, Alan Stern wrote:
> On Wed, 9 Jan 2019, Paul Elder wrote:
> 
> > This patch series adds a mechanism to allow asynchronously validating
> > the data stage of a control OUT request, and for stalling or suceeding
> > the request accordingly.
> 
> One thing we haven't mentioned explicitly: What should happen when the 
> time for the status stage rolls around if the gadget driver queues a 
> non-zero length request?

Ah, yeah, I missed that.

> This can happen in a few different ways.  One obvious possibility is
> that the gadget driver sets the explicit_status flag and then submits a
> non-zero length request.  Another is that the gadget driver submits
> _two_ requests during the data stage (the second would be interpreted
> as the status-stage request).  A third is that the gadget driver
> submits a data-stage request that is too long and the excess portion is
> used for the status stage.
> 
> My feeling is that the behavior in these cases should officially be
> undefined.  Almost anything could happen: the status stage could STALL,
> it could succeed, it could NAK, or it could send a non-zero packet to
> the host.  The request could return with 0 status or an error status,
> and req->actual could take on any reasonable value.
> 
> Alternatively, the UDC driver could detect these errors and report them 
> somehow.  Maybe STALL the status stage and complete the request with 
> -EPIPE status or some such thing.
> 
> Any preferences or other ideas?

I think error detection and reporting would be useful. The question is
what action to take after that; either leave it undefined or STALL. I
think STALL would be fine, since if a non-zero length request is
submitted for a status stage, intentionally or not, it isn't part of
proper behavior and should count as an error.

> One other thing: Some UDC drivers may assume that the data stage of a 
> control transfer never spans more than a single usb_request.  Should 
> this become an official requirement?

Would the data stage of a control transfer ever need more space than a
single usb_request can contain? I know UVC doesn't; that's why we pack
it together with the setup stage data in 3/6. If so, I would think we
can make it a requirement.


Paul