diff mbox

usb: gadget: uvc: configfs: Add bFrameIndex attributes

Message ID 20180608195335.12947-1-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Pinchart June 8, 2018, 7:53 p.m. UTC 
From: Joel Pepper <joel.pepper@rwth-aachen.de>

- 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.

Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
[Merge Joel's documentation and Paul's code]
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/ABI/testing/configfs-usb-gadget-uvc |  8 ++++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 10 ++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

This is the mix of Paul's and Joel's patches. I've also simplified the
ABI documentation by omitting the changelog date and kernel version as
seems to be the common practice when adding new attributes to existing
directories.

Comments

kernel test robot June 9, 2018, 1:05 p.m. UTC | #1 
Hi Joel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/usb-gadget-uvc-configfs-Add-bFrameIndex-attributes/20180609-194618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-s0-06091928 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb//gadget/function/uvc_configfs.c:994:20: error: expected ')' before numeric constant
      noop_conversion, 8);
                       ^
>> drivers/usb//gadget/function/uvc_configfs.c:1141:3: error: 'uvcg_frame_attr_b_frame_index' undeclared here (not in a function)
     &uvcg_frame_attr_b_frame_index,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/uvcg_frame_attr_b_frame_index +1141 drivers/usb//gadget/function/uvc_configfs.c

   992	
   993	UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
 > 994			noop_conversion, 8);
   995	UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
   996			noop_conversion, 8);
   997	UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
   998	UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
   999	UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32, 32);
  1000	UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, cpu_to_le32, 32);
  1001	UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize,
  1002			le32_to_cpu, cpu_to_le32, 32);
  1003	UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval,
  1004			le32_to_cpu, cpu_to_le32, 32);
  1005	
  1006	#undef noop_conversion
  1007	
  1008	#undef UVCG_FRAME_ATTR
  1009	
  1010	static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item,
  1011							 char *page)
  1012	{
  1013		struct uvcg_frame *frm = to_uvcg_frame(item);
  1014		struct f_uvc_opts *opts;
  1015		struct config_item *opts_item;
  1016		struct mutex *su_mutex = &frm->item.ci_group->cg_subsys->su_mutex;
  1017		int result, i;
  1018		char *pg = page;
  1019	
  1020		mutex_lock(su_mutex); /* for navigating configfs hierarchy */
  1021	
  1022		opts_item = frm->item.ci_parent->ci_parent->ci_parent->ci_parent;
  1023		opts = to_f_uvc_opts(opts_item);
  1024	
  1025		mutex_lock(&opts->lock);
  1026		for (result = 0, i = 0; i < frm->frame.b_frame_interval_type; ++i) {
  1027			result += sprintf(pg, "%d\n",
  1028					  le32_to_cpu(frm->dw_frame_interval[i]));
  1029			pg = page + result;
  1030		}
  1031		mutex_unlock(&opts->lock);
  1032	
  1033		mutex_unlock(su_mutex);
  1034		return result;
  1035	}
  1036	
  1037	static inline int __uvcg_count_frm_intrv(char *buf, void *priv)
  1038	{
  1039		++*((int *)priv);
  1040		return 0;
  1041	}
  1042	
  1043	static inline int __uvcg_fill_frm_intrv(char *buf, void *priv)
  1044	{
  1045		u32 num, **interv;
  1046		int ret;
  1047	
  1048		ret = kstrtou32(buf, 0, &num);
  1049		if (ret)
  1050			return ret;
  1051	
  1052		interv = priv;
  1053		**interv = cpu_to_le32(num);
  1054		++*interv;
  1055	
  1056		return 0;
  1057	}
  1058	
  1059	static int __uvcg_iter_frm_intrv(const char *page, size_t len,
  1060					 int (*fun)(char *, void *), void *priv)
  1061	{
  1062		/* sign, base 2 representation, newline, terminator */
  1063		char buf[1 + sizeof(u32) * 8 + 1 + 1];
  1064		const char *pg = page;
  1065		int i, ret;
  1066	
  1067		if (!fun)
  1068			return -EINVAL;
  1069	
  1070		while (pg - page < len) {
  1071			i = 0;
  1072			while (i < sizeof(buf) && (pg - page < len) &&
  1073					*pg != '\0' && *pg != '\n')
  1074				buf[i++] = *pg++;
  1075			if (i == sizeof(buf))
  1076				return -EINVAL;
  1077			while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))
  1078				++pg;
  1079			buf[i] = '\0';
  1080			ret = fun(buf, priv);
  1081			if (ret)
  1082				return ret;
  1083		}
  1084	
  1085		return 0;
  1086	}
  1087	
  1088	static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
  1089							  const char *page, size_t len)
  1090	{
  1091		struct uvcg_frame *ch = to_uvcg_frame(item);
  1092		struct f_uvc_opts *opts;
  1093		struct config_item *opts_item;
  1094		struct uvcg_format *fmt;
  1095		struct mutex *su_mutex = &ch->item.ci_group->cg_subsys->su_mutex;
  1096		int ret = 0, n = 0;
  1097		u32 *frm_intrv, *tmp;
  1098	
  1099		mutex_lock(su_mutex); /* for navigating configfs hierarchy */
  1100	
  1101		opts_item = ch->item.ci_parent->ci_parent->ci_parent->ci_parent;
  1102		opts = to_f_uvc_opts(opts_item);
  1103		fmt = to_uvcg_format(ch->item.ci_parent);
  1104	
  1105		mutex_lock(&opts->lock);
  1106		if (fmt->linked || opts->refcnt) {
  1107			ret = -EBUSY;
  1108			goto end;
  1109		}
  1110	
  1111		ret = __uvcg_iter_frm_intrv(page, len, __uvcg_count_frm_intrv, &n);
  1112		if (ret)
  1113			goto end;
  1114	
  1115		tmp = frm_intrv = kcalloc(n, sizeof(u32), GFP_KERNEL);
  1116		if (!frm_intrv) {
  1117			ret = -ENOMEM;
  1118			goto end;
  1119		}
  1120	
  1121		ret = __uvcg_iter_frm_intrv(page, len, __uvcg_fill_frm_intrv, &tmp);
  1122		if (ret) {
  1123			kfree(frm_intrv);
  1124			goto end;
  1125		}
  1126	
  1127		kfree(ch->dw_frame_interval);
  1128		ch->dw_frame_interval = frm_intrv;
  1129		ch->frame.b_frame_interval_type = n;
  1130		ret = len;
  1131	
  1132	end:
  1133		mutex_unlock(&opts->lock);
  1134		mutex_unlock(su_mutex);
  1135		return ret;
  1136	}
  1137	
  1138	UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
  1139	
  1140	static struct configfs_attribute *uvcg_frame_attrs[] = {
> 1141		&uvcg_frame_attr_b_frame_index,
  1142		&uvcg_frame_attr_bm_capabilities,
  1143		&uvcg_frame_attr_w_width,
  1144		&uvcg_frame_attr_w_height,
  1145		&uvcg_frame_attr_dw_min_bit_rate,
  1146		&uvcg_frame_attr_dw_max_bit_rate,
  1147		&uvcg_frame_attr_dw_max_video_frame_buffer_size,
  1148		&uvcg_frame_attr_dw_default_frame_interval,
  1149		&uvcg_frame_attr_dw_frame_interval,
  1150		NULL,
  1151	};
  1152	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 9, 2018, 1:22 p.m. UTC | #2 
Hi Joel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/usb-gadget-uvc-configfs-Add-bFrameIndex-attributes/20180609-194618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x011-201822 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/function/uvc_configfs.c:994:20: error: expected ')' before numeric constant
      noop_conversion, 8);
                       ^
>> drivers/usb/gadget/function/uvc_configfs.c:1141:3: error: 'uvcg_frame_attr_b_frame_index' undeclared here (not in a function); did you mean 'uvcg_frame_attr_dw_frame_interval'?
     &uvcg_frame_attr_b_frame_index,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      uvcg_frame_attr_dw_frame_interval

vim +994 drivers/usb/gadget/function/uvc_configfs.c

   992	
   993	UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
 > 994			noop_conversion, 8);
   995	UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
   996			noop_conversion, 8);
   997	UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
   998	UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
   999	UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32, 32);
  1000	UVCG_FRAME_ATTR(dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, cpu_to_le32, 32);
  1001	UVCG_FRAME_ATTR(dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize,
  1002			le32_to_cpu, cpu_to_le32, 32);
  1003	UVCG_FRAME_ATTR(dw_default_frame_interval, dwDefaultFrameInterval,
  1004			le32_to_cpu, cpu_to_le32, 32);
  1005	
  1006	#undef noop_conversion
  1007	
  1008	#undef UVCG_FRAME_ATTR
  1009	
  1010	static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item,
  1011							 char *page)
  1012	{
  1013		struct uvcg_frame *frm = to_uvcg_frame(item);
  1014		struct f_uvc_opts *opts;
  1015		struct config_item *opts_item;
  1016		struct mutex *su_mutex = &frm->item.ci_group->cg_subsys->su_mutex;
  1017		int result, i;
  1018		char *pg = page;
  1019	
  1020		mutex_lock(su_mutex); /* for navigating configfs hierarchy */
  1021	
  1022		opts_item = frm->item.ci_parent->ci_parent->ci_parent->ci_parent;
  1023		opts = to_f_uvc_opts(opts_item);
  1024	
  1025		mutex_lock(&opts->lock);
  1026		for (result = 0, i = 0; i < frm->frame.b_frame_interval_type; ++i) {
  1027			result += sprintf(pg, "%d\n",
  1028					  le32_to_cpu(frm->dw_frame_interval[i]));
  1029			pg = page + result;
  1030		}
  1031		mutex_unlock(&opts->lock);
  1032	
  1033		mutex_unlock(su_mutex);
  1034		return result;
  1035	}
  1036	
  1037	static inline int __uvcg_count_frm_intrv(char *buf, void *priv)
  1038	{
  1039		++*((int *)priv);
  1040		return 0;
  1041	}
  1042	
  1043	static inline int __uvcg_fill_frm_intrv(char *buf, void *priv)
  1044	{
  1045		u32 num, **interv;
  1046		int ret;
  1047	
  1048		ret = kstrtou32(buf, 0, &num);
  1049		if (ret)
  1050			return ret;
  1051	
  1052		interv = priv;
  1053		**interv = cpu_to_le32(num);
  1054		++*interv;
  1055	
  1056		return 0;
  1057	}
  1058	
  1059	static int __uvcg_iter_frm_intrv(const char *page, size_t len,
  1060					 int (*fun)(char *, void *), void *priv)
  1061	{
  1062		/* sign, base 2 representation, newline, terminator */
  1063		char buf[1 + sizeof(u32) * 8 + 1 + 1];
  1064		const char *pg = page;
  1065		int i, ret;
  1066	
  1067		if (!fun)
  1068			return -EINVAL;
  1069	
  1070		while (pg - page < len) {
  1071			i = 0;
  1072			while (i < sizeof(buf) && (pg - page < len) &&
  1073					*pg != '\0' && *pg != '\n')
  1074				buf[i++] = *pg++;
  1075			if (i == sizeof(buf))
  1076				return -EINVAL;
  1077			while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))
  1078				++pg;
  1079			buf[i] = '\0';
  1080			ret = fun(buf, priv);
  1081			if (ret)
  1082				return ret;
  1083		}
  1084	
  1085		return 0;
  1086	}
  1087	
  1088	static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
  1089							  const char *page, size_t len)
  1090	{
  1091		struct uvcg_frame *ch = to_uvcg_frame(item);
  1092		struct f_uvc_opts *opts;
  1093		struct config_item *opts_item;
  1094		struct uvcg_format *fmt;
  1095		struct mutex *su_mutex = &ch->item.ci_group->cg_subsys->su_mutex;
  1096		int ret = 0, n = 0;
  1097		u32 *frm_intrv, *tmp;
  1098	
  1099		mutex_lock(su_mutex); /* for navigating configfs hierarchy */
  1100	
  1101		opts_item = ch->item.ci_parent->ci_parent->ci_parent->ci_parent;
  1102		opts = to_f_uvc_opts(opts_item);
  1103		fmt = to_uvcg_format(ch->item.ci_parent);
  1104	
  1105		mutex_lock(&opts->lock);
  1106		if (fmt->linked || opts->refcnt) {
  1107			ret = -EBUSY;
  1108			goto end;
  1109		}
  1110	
  1111		ret = __uvcg_iter_frm_intrv(page, len, __uvcg_count_frm_intrv, &n);
  1112		if (ret)
  1113			goto end;
  1114	
  1115		tmp = frm_intrv = kcalloc(n, sizeof(u32), GFP_KERNEL);
  1116		if (!frm_intrv) {
  1117			ret = -ENOMEM;
  1118			goto end;
  1119		}
  1120	
  1121		ret = __uvcg_iter_frm_intrv(page, len, __uvcg_fill_frm_intrv, &tmp);
  1122		if (ret) {
  1123			kfree(frm_intrv);
  1124			goto end;
  1125		}
  1126	
  1127		kfree(ch->dw_frame_interval);
  1128		ch->dw_frame_interval = frm_intrv;
  1129		ch->frame.b_frame_interval_type = n;
  1130		ret = len;
  1131	
  1132	end:
  1133		mutex_unlock(&opts->lock);
  1134		mutex_unlock(su_mutex);
  1135		return ret;
  1136	}
  1137	
  1138	UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
  1139	
  1140	static struct configfs_attribute *uvcg_frame_attrs[] = {
> 1141		&uvcg_frame_attr_b_frame_index,
  1142		&uvcg_frame_attr_bm_capabilities,
  1143		&uvcg_frame_attr_w_width,
  1144		&uvcg_frame_attr_w_height,
  1145		&uvcg_frame_attr_dw_min_bit_rate,
  1146		&uvcg_frame_attr_dw_max_bit_rate,
  1147		&uvcg_frame_attr_dw_max_video_frame_buffer_size,
  1148		&uvcg_frame_attr_dw_default_frame_interval,
  1149		&uvcg_frame_attr_dw_frame_interval,
  1150		NULL,
  1151	};
  1152	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 9281e2aa38df..bee6ab72e6b1 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -177,6 +177,10 @@  Date:		Dec 2014
 KernelVersion:	4.0
 Description:	Specific MJPEG frame descriptors
 
+		bFrameIndex		- unique id for this framedescriptor;
+					only defined after parent format is
+					linked into the streaming header;
+					read-only
 		dwFrameInterval		- indicates how frame interval can be
 					programmed; a number of values
 					separated by newline can be specified
@@ -224,6 +228,10 @@  Date:		Dec 2014
 KernelVersion:	4.0
 Description:	Specific uncompressed frame descriptors
 
+		bFrameIndex		- unique id for this framedescriptor;
+					only defined after parent format is
+					linked into the streaming header;
+					read-only
 		dwFrameInterval		- indicates how frame interval can be
 					programmed; a number of values
 					separated by newline can be specified
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 1122659ccfe6..1645be415933 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1155,6 +1155,8 @@  UVC_ATTR(uvcg_frame_, cname, aname);
 
 #define noop_conversion(x) (x)
 
+UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
+		noop_conversion, 8);
 UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
 		noop_conversion, 8);
 UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
@@ -1301,6 +1303,7 @@  static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
 UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);
 
 static struct configfs_attribute *uvcg_frame_attrs[] = {
+	&uvcg_frame_attr_b_frame_index,
 	&uvcg_frame_attr_bm_capabilities,
 	&uvcg_frame_attr_w_width,
 	&uvcg_frame_attr_w_height,
@@ -2133,12 +2136,15 @@  static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
 			sizeof(*frm->dw_frame_interval);
 		memcpy(*dest, frm->dw_frame_interval, sz);
 		*dest += sz;
-		if (frm->fmt_type == UVCG_UNCOMPRESSED)
+		if (frm->fmt_type == UVCG_UNCOMPRESSED) {
 			h->bLength = UVC_DT_FRAME_UNCOMPRESSED_SIZE(
 				frm->frame.b_frame_interval_type);
-		else if (frm->fmt_type == UVCG_MJPEG)
+			frm->frame.b_frame_index = n + 1;
+		} else if (frm->fmt_type == UVCG_MJPEG) {
 			h->bLength = UVC_DT_FRAME_MJPEG_SIZE(
 				frm->frame.b_frame_interval_type);
+			frm->frame.b_frame_index = n + 1;
+		}
 	}
 	break;
 	}