All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Elder <paul.elder@ideasonboard.com>
To: Bin Liu <b-liu@ti.com>,
	laurent.pinchart@ideasonboard.com,
	kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	balbi@kernel.org, stern@rowland.harvard.edu, rogerq@ti.com
Subject: Re: [PATCH 5/6] usb: musb: gadget: implement send_response
Date: Wed, 31 Oct 2018 19:26:20 -0400	[thread overview]
Message-ID: <20181031232620.GA13896@emerald.amanokami.net> (raw)
In-Reply-To: <20181011160746.GA8763@uda0271908>

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

WARNING: multiple messages have this Message-ID (diff)
From: Paul Elder <paul.elder@ideasonboard.com>
To: Bin Liu <b-liu@ti.com>,
	laurent.pinchart@ideasonboard.com,
	kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	balbi@kernel.org, stern@rowland.harvard.edu, rogerq@ti.com
Subject: [5/6] usb: musb: gadget: implement send_response
Date: Wed, 31 Oct 2018 19:26:20 -0400	[thread overview]
Message-ID: <20181031232620.GA13896@emerald.amanokami.net> (raw)

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

  reply	other threads:[~2018-10-31 23:26 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10  2:48 [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Paul Elder
2018-10-10  2:48 ` [PATCH 1/6] usb: uvc: include videodev2.h in g_uvc.h Paul Elder
2018-10-10  2:48   ` [1/6] " Paul Elder
2018-10-10 13:42   ` [PATCH 1/6] " Laurent Pinchart
2018-10-10 13:42     ` [1/6] " Laurent Pinchart
2018-10-10  2:48 ` [PATCH 2/6] usb: gadget: uvc: enqueue usb request in setup handler for control OUT Paul Elder
2018-10-10  2:48   ` [2/6] " Paul Elder
2018-10-10  2:49 ` [PATCH 3/6] usb: gadget: uvc: package setup and data for control OUT requests Paul Elder
2018-10-10  2:49   ` [3/6] " Paul Elder
2018-10-10  2:49 ` [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage Paul Elder
2018-10-10  2:49   ` [4/6] " Paul Elder
2018-10-11 16:10   ` [PATCH 4/6] " Bin Liu
2018-10-11 16:10     ` [4/6] " Bin Liu
2018-10-17 23:45     ` [PATCH 4/6] " Laurent Pinchart
2018-10-17 23:45       ` [4/6] " Laurent Pinchart
2018-10-18 12:46       ` [PATCH 4/6] " Bin Liu
2018-10-18 12:46         ` [4/6] " Bin Liu
2018-10-18 14:07       ` [PATCH 4/6] " Alan Stern
2018-10-18 14:07         ` [4/6] " Alan Stern
2018-11-01 23:40         ` [PATCH 4/6] " Paul Elder
2018-11-01 23:40           ` [4/6] " Paul Elder
2018-11-02 12:44           ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 12:44             ` [4/6] " Laurent Pinchart
     [not found]             ` <87h8gzy5y7.fsf@linux.intel.com>
2018-11-02 14:36               ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 14:36                 ` [4/6] " Laurent Pinchart
2018-11-02 16:18                 ` [PATCH 4/6] " Alan Stern
2018-11-02 16:18                   ` [4/6] " Alan Stern
2018-11-02 17:10                   ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 17:10                     ` [4/6] " Laurent Pinchart
2018-11-02 19:46                     ` [PATCH 4/6] " Alan Stern
2018-11-02 19:46                       ` [4/6] " Alan Stern
2018-11-06 11:24                       ` [PATCH 4/6] " Felipe Balbi
2018-11-06 11:24                         ` [4/6] " Felipe Balbi
2018-11-06 15:01                         ` [PATCH 4/6] " Alan Stern
2018-11-06 15:01                           ` [4/6] " Alan Stern
2018-11-07  6:53                           ` [PATCH 4/6] " Felipe Balbi
2018-11-07  6:53                             ` [4/6] " Felipe Balbi
2018-11-06 11:17                     ` [PATCH 4/6] " Felipe Balbi
2018-11-06 11:17                       ` [4/6] " Felipe Balbi
2018-11-06 14:51                       ` [PATCH 4/6] " Alan Stern
2018-11-06 14:51                         ` [4/6] " Alan Stern
2018-11-07  7:00                         ` [PATCH 4/6] " Felipe Balbi
2018-11-07  7:00                           ` [4/6] " Felipe Balbi
2018-11-07 16:23                           ` [PATCH 4/6] " Alan Stern
2018-11-07 16:23                             ` [4/6] " Alan Stern
2018-12-14  3:47                             ` [PATCH 4/6] " Paul Elder
2018-12-14  3:47                               ` [4/6] " Paul Elder
2018-12-14 15:35                               ` [PATCH 4/6] " Alan Stern
2018-12-14 15:35                                 ` [4/6] " Alan Stern
2018-10-10  2:49 ` [PATCH 5/6] usb: musb: gadget: implement send_response Paul Elder
2018-10-10  2:49   ` [5/6] " Paul Elder
2018-10-11 16:07   ` [PATCH 5/6] " Bin Liu
2018-10-11 16:07     ` [5/6] " Bin Liu
2018-10-31 23:26     ` Paul Elder [this message]
2018-10-31 23:26       ` Paul Elder
2018-10-10  2:49 ` [PATCH 6/6] usb: gadget: uvc: allow ioctl to send response in status stage Paul Elder
2018-10-10  2:49   ` [6/6] " Paul Elder
2018-10-10 12:57 ` [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Laurent Pinchart
2018-10-11 19:31 ` Bin Liu
2018-10-17 23:42   ` Laurent Pinchart
2018-10-18 12:40     ` Bin Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181031232620.GA13896@emerald.amanokami.net \
    --to=paul.elder@ideasonboard.com \
    --cc=b-liu@ti.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rogerq@ti.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.