* [4/8] usb: gadget: uvc: configfs: Add interface number attributes
@ 2018-08-01 21:42 Laurent Pinchart
0 siblings, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2018-08-01 21:42 UTC (permalink / raw)
To: kieran.bingham
Cc: linux-usb, Felipe Balbi, Joel Pepper, Andrzej Pietrasiewicz
Hi Kieran,
On Wednesday, 1 August 2018 13:07:20 EEST Kieran Bingham wrote:
> On 01/08/18 01:29, Laurent Pinchart wrote:
> > The video control and video streaming interface numbers are needed in
> > the UVC gadget userspace stack to reply to UVC requests. They are
> > hardcoded to fixed values at the moment, preventing configurations with
> > multiple functions.
> >
> > To fix this, make them dynamically discoverable by userspace through
> > read-only configfs attributes in <function>/control/bInterfaceNumber and
> > <function>/streaming/bInterfaceNumber respectively.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/usb/gadget/function/f_uvc.c | 2 +
> > drivers/usb/gadget/function/u_uvc.h | 3 ++
> > drivers/usb/gadget/function/uvc_configfs.c | 62 +++++++++++++++++++++++++
> It sounds like this is extending the userspace ABI.
>
> Do we need to document this anywhere?
> Documentation/ABI/testing/configfs-usb-gadget-uvc ?
You're right, I'll document it.
> > 3 files changed, 67 insertions(+)
[snip]
> > diff --git a/drivers/usb/gadget/function/uvc_configfs.c
> > b/drivers/usb/gadget/function/uvc_configfs.c index
> > e019ea317c7a..801117686108 100644
> > --- a/drivers/usb/gadget/function/uvc_configfs.c
> > +++ b/drivers/usb/gadget/function/uvc_configfs.c
> > @@ -687,8 +687,39 @@ static const struct uvcg_config_group_type
> > uvcg_control_class_grp_type = {>
> > * control
> > */
> >
> > +static ssize_t uvcg_default_control_b_interface_number_show(
> > + struct config_item *item, char *page)
> > +{
> > + struct config_group *group = to_config_group(item);
> > + struct mutex *su_mutex = &group->cg_subsys->su_mutex;
> > + struct config_item *opts_item;
> > + struct f_uvc_opts *opts;
> > + int result = 0;
> > +
> > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> > +
> > + opts_item = item->ci_parent;
> > + opts = to_f_uvc_opts(opts_item);
> > +
> > + mutex_lock(&opts->lock);
> > + result += sprintf(page, "%u\n", opts->control_interface);
> > + mutex_unlock(&opts->lock);
> > +
> > + mutex_unlock(su_mutex);
> > +
> > + return result;
> > +}
> > +
> > +UVC_ATTR_RO(uvcg_default_control_, b_interface_number, bInterfaceNumber);
> > +
> > +static struct configfs_attribute *uvcg_default_control_attrs[] = {
> > + &uvcg_default_control_attr_b_interface_number,
> > + NULL,
> > +};
[snip]
> > +static ssize_t uvcg_default_streaming_b_interface_number_show(
> > + struct config_item *item, char *page)
> > +{
> > + struct config_group *group = to_config_group(item);
> > + struct mutex *su_mutex = &group->cg_subsys->su_mutex;
> > + struct config_item *opts_item;
> > + struct f_uvc_opts *opts;
> > + int result = 0;
> > +
> > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> > +
> > + opts_item = item->ci_parent;
> > + opts = to_f_uvc_opts(opts_item);
> > +
> > + mutex_lock(&opts->lock);
> > + result += sprintf(page, "%u\n", opts->streaming_interface);
> > + mutex_unlock(&opts->lock);
> > +
> > + mutex_unlock(su_mutex);
> > +
> > + return result;
> > +}
>
> That looks like a lot of common boilerplate code copied for each file
> which prints a single %u.
>
> Perhaps we should convert those to a macro - but for now they look fine.
Feel free to submit patches :-) Helper functions would be event better than
macros to decrease the object size.
> > +
> > +UVC_ATTR_RO(uvcg_default_streaming_, b_interface_number,
> > bInterfaceNumber);
> > +static struct configfs_attribute
> > *uvcg_default_streaming_attrs[] = {
> > + &uvcg_default_streaming_attr_b_interface_number,
> > + NULL,
> > +};
[snip]
^ permalink raw reply [flat|nested] 3+ messages in thread* [4/8] usb: gadget: uvc: configfs: Add interface number attributes
@ 2018-08-01 10:07 Kieran Bingham
0 siblings, 0 replies; 3+ messages in thread
From: Kieran Bingham @ 2018-08-01 10:07 UTC (permalink / raw)
To: Laurent Pinchart, linux-usb
Cc: Felipe Balbi, Joel Pepper, Andrzej Pietrasiewicz
Hi Laurent,
Thank you for the patch,
On 01/08/18 01:29, Laurent Pinchart wrote:
> The video control and video streaming interface numbers are needed in
> the UVC gadget userspace stack to reply to UVC requests. They are
> hardcoded to fixed values at the moment, preventing configurations with
> multiple functions.
>
> To fix this, make them dynamically discoverable by userspace through
> read-only configfs attributes in <function>/control/bInterfaceNumber and
> <function>/streaming/bInterfaceNumber respectively.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/usb/gadget/function/f_uvc.c | 2 +
> drivers/usb/gadget/function/u_uvc.h | 3 ++
> drivers/usb/gadget/function/uvc_configfs.c | 62 ++++++++++++++++++++++++++++++
It sounds like this is extending the userspace ABI.
Do we need to document this anywhere?
Documentation/ABI/testing/configfs-usb-gadget-uvc ?
> 3 files changed, 67 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 95cb1b5f5ffe..4ea987741e6e 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -699,12 +699,14 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
> uvc_iad.bFirstInterface = ret;
> uvc_control_intf.bInterfaceNumber = ret;
> uvc->control_intf = ret;
> + opts->control_interface = ret;
>
> if ((ret = usb_interface_id(c, f)) < 0)
> goto error;
> uvc_streaming_intf_alt0.bInterfaceNumber = ret;
> uvc_streaming_intf_alt1.bInterfaceNumber = ret;
> uvc->streaming_intf = ret;
> + opts->streaming_interface = ret;
>
> /* Copy descriptors */
> f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
> diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
> index 2ed292e94fbc..5242d489e20a 100644
> --- a/drivers/usb/gadget/function/u_uvc.h
> +++ b/drivers/usb/gadget/function/u_uvc.h
> @@ -25,6 +25,9 @@ struct f_uvc_opts {
> unsigned int streaming_maxpacket;
> unsigned int streaming_maxburst;
>
> + unsigned int control_interface;
> + unsigned int streaming_interface;
> +
> /*
> * Control descriptors array pointers for full-/high-speed and
> * super-speed. They point by default to the uvc_fs_control_cls and
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index e019ea317c7a..801117686108 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -687,8 +687,39 @@ static const struct uvcg_config_group_type uvcg_control_class_grp_type = {
> * control
> */
>
> +static ssize_t uvcg_default_control_b_interface_number_show(
> + struct config_item *item, char *page)
> +{
> + struct config_group *group = to_config_group(item);
> + struct mutex *su_mutex = &group->cg_subsys->su_mutex;
> + struct config_item *opts_item;
> + struct f_uvc_opts *opts;
> + int result = 0;
> +
> + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> +
> + opts_item = item->ci_parent;
> + opts = to_f_uvc_opts(opts_item);
> +
> + mutex_lock(&opts->lock);
> + result += sprintf(page, "%u\n", opts->control_interface);
> + mutex_unlock(&opts->lock);
> +
> + mutex_unlock(su_mutex);
> +
> + return result;
> +}
> +
> +UVC_ATTR_RO(uvcg_default_control_, b_interface_number, bInterfaceNumber);
> +
> +static struct configfs_attribute *uvcg_default_control_attrs[] = {
> + &uvcg_default_control_attr_b_interface_number,
> + NULL,
> +};
> +
> static const struct uvcg_config_group_type uvcg_control_grp_type = {
> .type = {
> + .ct_attrs = uvcg_default_control_attrs,
> .ct_owner = THIS_MODULE,
> },
> .name = "control",
> @@ -2246,8 +2277,39 @@ static const struct uvcg_config_group_type uvcg_streaming_class_grp_type = {
> * streaming
> */
>
> +static ssize_t uvcg_default_streaming_b_interface_number_show(
> + struct config_item *item, char *page)
> +{
> + struct config_group *group = to_config_group(item);
> + struct mutex *su_mutex = &group->cg_subsys->su_mutex;
> + struct config_item *opts_item;
> + struct f_uvc_opts *opts;
> + int result = 0;
> +
> + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> +
> + opts_item = item->ci_parent;
> + opts = to_f_uvc_opts(opts_item);
> +
> + mutex_lock(&opts->lock);
> + result += sprintf(page, "%u\n", opts->streaming_interface);
> + mutex_unlock(&opts->lock);
> +
> + mutex_unlock(su_mutex);
> +
> + return result;
> +}
That looks like a lot of common boilerplate code copied for each file
which prints a single %u.
Perhaps we should convert those to a macro - but for now they look fine.
> +
> +UVC_ATTR_RO(uvcg_default_streaming_, b_interface_number, bInterfaceNumber);> +
> +static struct configfs_attribute *uvcg_default_streaming_attrs[] = {
> + &uvcg_default_streaming_attr_b_interface_number,
> + NULL,
> +};
> +
> static const struct uvcg_config_group_type uvcg_streaming_grp_type = {
> .type = {
> + .ct_attrs = uvcg_default_streaming_attrs,
> .ct_owner = THIS_MODULE,
> },
> .name = "streaming",
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [4/8] usb: gadget: uvc: configfs: Add interface number attributes
@ 2018-08-01 0:29 Laurent Pinchart
0 siblings, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2018-08-01 0:29 UTC (permalink / raw)
To: linux-usb
Cc: Felipe Balbi, Joel Pepper, Kieran Bingham, Andrzej Pietrasiewicz
The video control and video streaming interface numbers are needed in
the UVC gadget userspace stack to reply to UVC requests. They are
hardcoded to fixed values at the moment, preventing configurations with
multiple functions.
To fix this, make them dynamically discoverable by userspace through
read-only configfs attributes in <function>/control/bInterfaceNumber and
<function>/streaming/bInterfaceNumber respectively.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/usb/gadget/function/f_uvc.c | 2 +
drivers/usb/gadget/function/u_uvc.h | 3 ++
drivers/usb/gadget/function/uvc_configfs.c | 62 ++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 95cb1b5f5ffe..4ea987741e6e 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -699,12 +699,14 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
uvc_iad.bFirstInterface = ret;
uvc_control_intf.bInterfaceNumber = ret;
uvc->control_intf = ret;
+ opts->control_interface = ret;
if ((ret = usb_interface_id(c, f)) < 0)
goto error;
uvc_streaming_intf_alt0.bInterfaceNumber = ret;
uvc_streaming_intf_alt1.bInterfaceNumber = ret;
uvc->streaming_intf = ret;
+ opts->streaming_interface = ret;
/* Copy descriptors */
f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 2ed292e94fbc..5242d489e20a 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -25,6 +25,9 @@ struct f_uvc_opts {
unsigned int streaming_maxpacket;
unsigned int streaming_maxburst;
+ unsigned int control_interface;
+ unsigned int streaming_interface;
+
/*
* Control descriptors array pointers for full-/high-speed and
* super-speed. They point by default to the uvc_fs_control_cls and
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index e019ea317c7a..801117686108 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -687,8 +687,39 @@ static const struct uvcg_config_group_type uvcg_control_class_grp_type = {
* control
*/
+static ssize_t uvcg_default_control_b_interface_number_show(
+ struct config_item *item, char *page)
+{
+ struct config_group *group = to_config_group(item);
+ struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+ struct config_item *opts_item;
+ struct f_uvc_opts *opts;
+ int result = 0;
+
+ mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+ opts_item = item->ci_parent;
+ opts = to_f_uvc_opts(opts_item);
+
+ mutex_lock(&opts->lock);
+ result += sprintf(page, "%u\n", opts->control_interface);
+ mutex_unlock(&opts->lock);
+
+ mutex_unlock(su_mutex);
+
+ return result;
+}
+
+UVC_ATTR_RO(uvcg_default_control_, b_interface_number, bInterfaceNumber);
+
+static struct configfs_attribute *uvcg_default_control_attrs[] = {
+ &uvcg_default_control_attr_b_interface_number,
+ NULL,
+};
+
static const struct uvcg_config_group_type uvcg_control_grp_type = {
.type = {
+ .ct_attrs = uvcg_default_control_attrs,
.ct_owner = THIS_MODULE,
},
.name = "control",
@@ -2246,8 +2277,39 @@ static const struct uvcg_config_group_type uvcg_streaming_class_grp_type = {
* streaming
*/
+static ssize_t uvcg_default_streaming_b_interface_number_show(
+ struct config_item *item, char *page)
+{
+ struct config_group *group = to_config_group(item);
+ struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+ struct config_item *opts_item;
+ struct f_uvc_opts *opts;
+ int result = 0;
+
+ mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+ opts_item = item->ci_parent;
+ opts = to_f_uvc_opts(opts_item);
+
+ mutex_lock(&opts->lock);
+ result += sprintf(page, "%u\n", opts->streaming_interface);
+ mutex_unlock(&opts->lock);
+
+ mutex_unlock(su_mutex);
+
+ return result;
+}
+
+UVC_ATTR_RO(uvcg_default_streaming_, b_interface_number, bInterfaceNumber);
+
+static struct configfs_attribute *uvcg_default_streaming_attrs[] = {
+ &uvcg_default_streaming_attr_b_interface_number,
+ NULL,
+};
+
static const struct uvcg_config_group_type uvcg_streaming_grp_type = {
.type = {
+ .ct_attrs = uvcg_default_streaming_attrs,
.ct_owner = THIS_MODULE,
},
.name = "streaming",
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-01 21:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-01 21:42 [4/8] usb: gadget: uvc: configfs: Add interface number attributes Laurent Pinchart
-- strict thread matches above, loose matches on Subject: below --
2018-08-01 10:07 Kieran Bingham
2018-08-01 0:29 Laurent Pinchart
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).