linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Joel Pepper <joel.pepper@rwth-aachen.de>
Cc: Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, Paul Elder <paul.elder@pitt.edu>
Subject: usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
Date: Wed, 21 Mar 2018 11:45:00 +0200	[thread overview]
Message-ID: <7499230.529cF5ADUy@avalon> (raw)

Hi Joel,

Thank you for the patch.

On Tuesday, 20 March 2018 22:28:53 EET Joel Pepper wrote:
> - Add bFrameIndex as a UVCG_FRAME_ATTR_RO for each frame size.
> - Automatically assign ascending bFrameIndex to each frame in a format.
> 
> Before all "bFrameindex" attributes were set to "1" with no way to
> configure the gadget otherwise. This resulted in the host always
> negotiating for bFrameIndex 1 (i.e. the first framesize of the gadget).
> After the negotiation the host driver will set the user or application
> selected framesize, while the gadget is actually set to the first
> framesize.
> 
> Now, when the containing format is linked into the streaming header,
> iterate over all child frame descriptors and assign ascending indices.
> The automatically assigned indices can be read from the new read only
> bFrameIndex configsfs attribute in each frame descriptor item.
> 
> v2: Add the new attribute to both MJPEG and uncompressed frame descriptors
> in Documentation/ABI, with note that it was added only in a later
> kernel version
> 
> v3: Changed from simply allowing user to set the value for bFrameIndex to
> automatically assigning correct distinct frame indexes. Changed
> bFrameIndex from RW to RO

I like the new commit message, but it would be nice if you could also update 
the patch contents ;-)

(By the way the subject line should have mentioned "[PATCH v3]")

> Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
> ---
>  Documentation/ABI/testing/configfs-usb-gadget-uvc | 17 +++++++++++++++++
>  drivers/usb/gadget/function/uvc_configfs.c        |  3 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 1ba0d0f..d435cf7
> 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -194,6 +194,14 @@ Description:	Specific MJPEG frame descriptors
>  		bmCapabilities		- still image support, fixed frame-rate
>  					support
> 
> +Date:		Mar 2018
> +KernelVersion:	4.16
> +
> +		bFrameIndex		- unique id for this framedescriptor;
> +					if using multiple framedescriptors for
> +					same format, user needs to set distinct
> +					value for each frame descriptor
> +
>  What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/
uncompressed
>  Date:		Dec 2014
>  KernelVersion:	4.0
> @@ -241,6 +249,15 @@ Description:	Specific uncompressed frame descriptors
>  		bmCapabilities		- still image support, fixed frame-rate
>  					support
> 
> +Date:           Mar 2018
> +KernelVersion:  4.16
> +
> +                bFrameIndex             - unique id for this
> framedescriptor; +                                        if using multiple
> framedescriptors for +                                        same format,
> user needs to set distinct +                                        value
> for each frame descriptor +
> +
>  What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/header
>  Date:		Dec 2014
>  KernelVersion:	4.0
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c
> b/drivers/usb/gadget/function/uvc_configfs.c index c9b8cc4a..5966d65 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
> 
>  UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
>  		noop_conversion, 8);
> +UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
> +		noop_conversion, 8);
>  UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
>  UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
>  UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32,
> 32); @@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval,
> dwFrameInterval);
> 
>  static struct configfs_attribute *uvcg_frame_attrs[] = {
>  	&uvcg_frame_attr_bm_capabilities,
> +	&uvcg_frame_attr_b_frame_index,
>  	&uvcg_frame_attr_w_width,
>  	&uvcg_frame_attr_w_height,
>  	&uvcg_frame_attr_dw_min_bit_rate,

             reply	other threads:[~2018-03-21  9:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21  9:45 Laurent Pinchart [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-03-21 10:01 usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first Laurent Pinchart
2018-03-20 20:28 Joel Pepper
2018-03-19 15:22 Greg Kroah-Hartman
2018-03-19 10:55 Joel Pepper

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=7499230.529cF5ADUy@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel.pepper@rwth-aachen.de \
    --cc=linux-usb@vger.kernel.org \
    --cc=paul.elder@pitt.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).