[5/6] usb: musb: gadget: implement send_response

[5/6] usb: musb: gadget: implement send_response

[Paul Elder]

This patch implements a mechanism to signal the MUSB driver to reply to
a control OUT request with STALL or ACK.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/usb/musb/musb_gadget_ep0.c | 41 ++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
index 91a5027b5c1f..f0ed1f7472a3 100644
--- a/drivers/usb/musb/musb_gadget_ep0.c
+++ b/drivers/usb/musb/musb_gadget_ep0.c
@@ -458,6 +458,25 @@ __acquires(musb->lock)
 	return handled;
 }
 
+static int ep0_send_response(struct musb *musb, bool stall)
+{
+	void __iomem *regs = musb->control_ep->regs;
+	u16 ackpend;
+
+	if (musb->ep0_state != MUSB_EP0_STAGE_RX &&
+	    musb->ep0_state != MUSB_EP0_STAGE_STATUSIN)
+		return -EINVAL;
+
+	ackpend = MUSB_CSR0_P_DATAEND
+		| MUSB_CSR0_P_SVDRXPKTRDY
+		| (stall ? MUSB_CSR0_P_SENDSTALL : 0);
+
+	musb_ep_select(musb->mregs, 0);
+	musb_writew(regs, MUSB_CSR0, ackpend);
+
+	return 0;
+}
+
 /* we have an ep0out data packet
  * Context:  caller holds controller lock
  */
@@ -466,10 +485,13 @@ static void ep0_rxstate(struct musb *musb)
 	void __iomem		*regs = musb->control_ep->regs;
 	struct musb_request	*request;
 	struct usb_request	*req;
+	struct usb_ep		*ep;
 	u16			count, csr;
+	bool			last_packet = false;
 
 	request = next_ep0_request(musb);
 	req = &request->request;
+	ep = &request->ep->end_point;
 
 	/* read packet and ack; or stall because of gadget driver bug:
 	 * should have provided the rx buffer before setup() returned.
@@ -492,6 +514,7 @@ static void ep0_rxstate(struct musb *musb)
 		if (count < 64 || req->actual == req->length) {
 			musb->ep0_state = MUSB_EP0_STAGE_STATUSIN;
 			csr |= MUSB_CSR0_P_DATAEND;
+			last_packet = true;
 		} else
 			req = NULL;
 	} else
@@ -508,6 +531,10 @@ static void ep0_rxstate(struct musb *musb)
 			return;
 		musb->ackpend = 0;
 	}
+
+	if (last_packet && ep->delayed_status)
+		return;
+
 	musb_ep_select(musb->mregs, 0);
 	musb_writew(regs, MUSB_CSR0, csr);
 }
@@ -991,6 +1018,19 @@ static int musb_g_ep0_dequeue(struct usb_ep *ep, struct usb_request *req)
 	return -EINVAL;
 }
 
+static int musb_g_ep0_send_response(struct usb_ep *e, bool stall)
+{
+	struct musb_ep *ep = to_musb_ep(e);
+	struct musb *musb = ep->musb;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&musb->lock, flags);
+	ret = ep0_send_response(musb, stall);
+	spin_unlock_irqrestore(&musb->lock, flags);
+	return ret;
+}
+
 static int musb_g_ep0_halt(struct usb_ep *e, int value)
 {
 	struct musb_ep		*ep;
@@ -1059,4 +1099,5 @@ const struct usb_ep_ops musb_g_ep0_ops = {
 	.queue		= musb_g_ep0_queue,
 	.dequeue	= musb_g_ep0_dequeue,
 	.set_halt	= musb_g_ep0_halt,
+	.send_response  = musb_g_ep0_send_response,
 };

[5/6] usb: musb: gadget: implement send_response

[Bin Liu]

Hi,

On Tue, Oct 09, 2018 at 10:49:02PM -0400, Paul Elder wrote:
> This patch implements a mechanism to signal the MUSB driver to reply to
> a control OUT request with STALL or ACK.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The patch looks good to me, here is my Acked-by:

Acked-by: Bin Liu <b-liu@ti.com>

but I am unable to test this patch set - the current Greg's usb-next
tree gives deadlock error between composite_disconnect() and
usb_function_deactivate() when loading g_webcam.ko on BeagleboneBlack.
The error happens before applying this patch set.

Regards,
-Bin.

[5/6] usb: musb: gadget: implement send_response

[Paul Elder]

Hi Bin,

On Thu, Oct 11, 2018 at 11:07:46AM -0500, Bin Liu wrote:
> Hi,
> 
> On Tue, Oct 09, 2018 at 10:49:02PM -0400, Paul Elder wrote:
> > This patch implements a mechanism to signal the MUSB driver to reply to
> > a control OUT request with STALL or ACK.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> The patch looks good to me, here is my Acked-by:
> 
> Acked-by: Bin Liu <b-liu@ti.com>
> 
> but I am unable to test this patch set - the current Greg's usb-next
> tree gives deadlock error between composite_disconnect() and
> usb_function_deactivate() when loading g_webcam.ko on BeagleboneBlack.
> The error happens before applying this patch set.

We don't use g_webcam anymore, because it doesn't offer the flexibility
that configfs does (for example, only one function can be configured with
g_webcam). There are no features that g_webcam offers that configfs doesn't.

I was unable to reproduce the deadlock that you mention on Greg's
usb-next tree. Which commit were you on?
I did, however, get the deadlock that you mention upon *killing* the
userspace application providing the stream, not when loading g_webcam.ko.

Here is a sample script for setting up a UVC gadget through configfs
(I haven't tested this exact script; I extracted the functional
components):

CONFIGFS="/sys/kernel/config"
GADGET="$CONFIGFS/usb_gadget"
VID="0x0525"
PID="0xa4a2"
SERIAL="0123456789"
MANUF=$(hostname)
PRODUCT="UVC Gadget"
UDC=`ls /sys/class/udc`

cd $GADGET/g1
echo $VID > idVendor
echo $PID > idProduct

mkdir -p strings/0x409
echo $SERIAL > strings/0x409/serialnumber
echo $MANUF > strings/0x409/manufacturer
echo $PRODUCT > strings/0x409/product

mkdir configs/c.1
mkdir configs/c.1/strings/0x409

# create uvc
CONFIG="configs/c.1"
FUNCTION="uvc.0"

mkdir functions/$FUNCTION

# create frame 640x360 uncompressed
WIDTH=640
HEIGHT=360

wdir=functions/$FUNCTION/streaming/uncompressed/u/${HEIGHT}p

mkdir $wdir
echo $WIDTH > $wdir/wWidth
echo $HEIGHT > $wdir/wHeight
echo $(( $WIDTH * $HEIGHT * 2 )) > $wdir/dwMaxVideoFrameBufferSize
cat <<EOF > $wdir/dwFrameInterval
666666
100000
5000000
EOF
# end create frame

mkdir functions/$FUNCTION/streaming/header/h
cd functions/$FUNCTION/streaming/header/h
ln -s ../../uncompressed/u
cd ../../class/fs
ln -s ../../header/h
cd ../../class/hs
ln -s ../../header/h
cd ../../../control
mkdir header/h
ln -s header/h class/fs
ln -s header/h class/ss
cd ../../../

# Set the packet size: uvc gadget max size is 3k...
echo 3072 > functions/$FUNCTION/streaming_maxpacket
echo 2048 > functions/$FUNCTION/streaming_maxpacket
echo 1024 > functions/$FUNCTION/streaming_maxpacket

ln -s functions/$FUNCTION configs/c.1
# end create uvc

echo $UDC > UDC


Thanks,

Paul