On Thu, Dec 29, 2022 at 02:30:04AM +0200, Laurent Pinchart wrote: > Hi Dan, > > Thank you for the patch. > > On Mon, Nov 21, 2022 at 09:25:09AM +0000, Daniel Scally wrote: > > At the moment, the UVC function graph is hardcoded IT -> PU -> OT. > > To add XU support we need the ability to insert the XU descriptors > > into the chain. To facilitate that, make the output terminal's > > bSourceID attribute writeable so that we can configure its source. > > > > Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> > > --- > > Changes in v2: > > > > - Updated the ABI Documentation to reflect the change. > > > > .../ABI/testing/configfs-usb-gadget-uvc | 2 +- > > drivers/usb/gadget/function/uvc_configfs.c | 57 ++++++++++++++++++- > > 2 files changed, 57 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc > > index 611b23e6488d..feb3f2cc0c16 100644 > > --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc > > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc > > @@ -52,7 +52,7 @@ Date: Dec 2014 > > KernelVersion: 4.0 > > Description: Default output terminal descriptors > > > > - All attributes read only: > > + All attributes read only except bSourceID: > > > > ============== ============================================= > > iTerminal index of string descriptor > > diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c > > index 4303a3283ba0..af4258120f9a 100644 > > --- a/drivers/usb/gadget/function/uvc_configfs.c > > +++ b/drivers/usb/gadget/function/uvc_configfs.c > > @@ -483,11 +483,66 @@ UVC_ATTR_RO(uvcg_default_output_, cname, aname) > > UVCG_DEFAULT_OUTPUT_ATTR(b_terminal_id, bTerminalID, 8); > > UVCG_DEFAULT_OUTPUT_ATTR(w_terminal_type, wTerminalType, 16); > > UVCG_DEFAULT_OUTPUT_ATTR(b_assoc_terminal, bAssocTerminal, 8); > > -UVCG_DEFAULT_OUTPUT_ATTR(b_source_id, bSourceID, 8); > > UVCG_DEFAULT_OUTPUT_ATTR(i_terminal, iTerminal, 8); > > > > #undef UVCG_DEFAULT_OUTPUT_ATTR > > > > +static ssize_t uvcg_default_output_b_source_id_show(struct config_item *item, > > + char *page) > > +{ > > + struct config_group *group = to_config_group(item); > > + struct f_uvc_opts *opts; > > + struct config_item *opts_item; > > + struct mutex *su_mutex = &group->cg_subsys->su_mutex; > > + struct uvc_output_terminal_descriptor *cd; > > + int result; > > + > > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */ > > + > > + opts_item = group->cg_item.ci_parent->ci_parent->ci_parent->ci_parent; > > + opts = to_f_uvc_opts(opts_item); > > + cd = &opts->uvc_output_terminal; > > + > > + mutex_lock(&opts->lock); > > + result = sprintf(page, "%u\n", le8_to_cpu(cd->bSourceID)); > > + mutex_unlock(&opts->lock); > > + > > + mutex_unlock(su_mutex); > > + > > + return result; > > +} > > + > > +static ssize_t uvcg_default_output_b_source_id_store(struct config_item *item, > > + const char *page, size_t len) > > +{ > > + struct config_group *group = to_config_group(item); > > + struct f_uvc_opts *opts; > > + struct config_item *opts_item; > > + struct mutex *su_mutex = &group->cg_subsys->su_mutex; > > + struct uvc_output_terminal_descriptor *cd; > > + int result; > > + u8 num; > > + > > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */ > > + > > + opts_item = group->cg_item.ci_parent->ci_parent->ci_parent->ci_parent; > > + opts = to_f_uvc_opts(opts_item); > > + cd = &opts->uvc_output_terminal; > > + > > + result = kstrtou8(page, 0, &num); > > + if (result) > > + return result; > > + > > + mutex_lock(&opts->lock); > > + cd->bSourceID = num; > > + mutex_unlock(&opts->lock); > > + > > + mutex_unlock(su_mutex); > > + > > + return len; > > +} > > +UVC_ATTR(uvcg_default_output_, b_source_id, bSourceID); > > Feel free to shoot this idea down if it's a bad one: given that the > bSourceID attributes serve to create a pipeline by linking entities, > would it make sense to model these links with symlinks ? I forgot to mention that this would handle the bSourceID attribute automatically, avoiding mistakes. But maybe we're over-engineering all this... > > + > > static struct configfs_attribute *uvcg_default_output_attrs[] = { > > &uvcg_default_output_attr_b_terminal_id, > > &uvcg_default_output_attr_w_terminal_type, -- Regards, Laurent Pinchart