usb: gadget: uvc: cleanup UVCG_FRAME_ATTR macro
Subsystems (3)
PATCH 6/6 usb: gadget: uvc: cleanup UVCG_FRAME_ATTR macro
1) Change "conv" an "vnoc" to "to_cpu_endian" to "to_little_endian".
2) No need to check the "limit" because that is already handled in kstrtoXX so delete that parameter along with the check.
3) By using a "bits" parameter, we can combine the "uxx" parameter and the "str2u" parameters.
4) The kstrtou##bits() conversion does not need to be done under the mutex so move it to the start of the function.
5) Change the name of "identity_conv" to "noop_conversion".
---
This file has a couple pages of Sparse endian warnings.
http://lwn.net/Articles/205624/
It's hard to review the static checker warnings when there are so many.
drivers/usb/gadget/function/uvc_configfs.c +18 -26
@@ -1032,7 +1032,7 @@ static struct configfs_item_operations uvcg_frame_item_ops = { .store_attribute = uvcg_frame_attr_store, }; -#define UVCG_FRAME_ATTR (cname, aname, conv, str2u, uxx, vnoc, limit) \ +#define UVCG_FRAME_ATTR (cname, aname, to_cpu_endian, to_little_endian, bits) \ static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\ { \ struct f_uvc_opts *opts; \
@@ -1046,7 +1046,7 @@ static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\ opts =to_f_uvc_opts (opts_item); \ \mutex_lock (&opts->lock); \ - result =sprintf (page,"%d\n" ,conv (f->frame.cname)); \ + result =sprintf (page,"%d\n" ,to_cpu_endian (f->frame.cname)); \mutex_unlock (&opts->lock); \ \mutex_unlock (su_mutex); \
@@ -1061,7 +1061,11 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \struct uvcg_format *fmt; \struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\int ret; \ -uxx num; \ + u##bits num; \ + \ + ret = kstrtou##bits(page, 0, &num); \ + if (ret) \ + return ret; \ \ mutex_lock(su_mutex);/* for navigating configfs hierarchy */ \ \
@@ -1075,15 +1079,7 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \ goto end; \ } \ \ - ret =str2u (page,0 , &num); \ -if (ret) \ - goto end; \ - \ -if (num > limit) { \ - ret = -EINVAL ; \ - goto end; \ - } \ - f->frame.cname =vnoc (num); \ + f->frame.cname =to_little_endian (num); \ ret = len; \ end: \mutex_unlock (&opts->lock); \
@@ -1097,24 +1093,20 @@ static struct uvcg_frame_attribute \ uvcg_frame_##cname##_show, \ uvcg_frame_##cname##_store) -#define identity_conv (x) (x) +#define noop_conversion (x) (x) -UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, identity_conv, kstrtou8, u8, - identity_conv,0xFF ); -UVCG_FRAME_ATTR (w_width, wWidth, le16_to_cpu, kstrtou16, u16, cpu_to_le16, -0xFFFF ); -UVCG_FRAME_ATTR (w_height, wHeight, le16_to_cpu, kstrtou16, u16, cpu_to_le16, -0xFFFF ); -UVCG_FRAME_ATTR (dw_min_bit_rate, dwMinBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); -UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); +UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, 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 ); +UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, cpu_to_le32,32 );UVCG_FRAME_ATTR (dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32,32 );UVCG_FRAME_ATTR (dw_default_frame_interval, dwDefaultFrameInterval, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32,32 ); -#undef identity_conv +#undef noop_conversion#undef UVCG_FRAME_ATTR
W dniu 14.01.2015 o 22:06, Dan Carpenter pisze:
Quoted 11 lines
1) Change "conv" an "vnoc" to "
to_cpu_endian" to "to_little_endian".
2) No need to check the "limit" because that is already handled in kstrtoXX so delete that parameter along with the check.
3) By using a "bits" parameter, we can combine the "uxx" parameter and the "str2u" parameters.
4) The kstrtou##bits() conversion does not need to be done under the mutex so move it to the start of the function.
5) Change the name of "identity_conv" to "noop_conversion".Signed-off-by Dan Carpenter <[email protected]>
---
Quoted 96 lines
This file has a couple pages of Sparse endian warnings.
http://lwn.net/Articles/205624/
It's hard to review the static checker warnings when there are so many.drivers/usb/gadget/function/uvc_configfs.c +18
-27@@ -1032,7 +1032,7 @@ static struct configfs_item_operations uvcg_frame_item_ops = { .store_attribute = uvcg_frame_attr_store, }; -#define UVCG_FRAME_ATTR (cname, aname, conv, str2u, uxx, vnoc, limit) \ +#define UVCG_FRAME_ATTR (cname, aname, to_cpu_endian, to_little_endian, bits) \ static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\ { \ struct f_uvc_opts *opts; \@@ -1046,7 +1046,7 @@ static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\ opts =to_f_uvc_opts (opts_item); \ \mutex_lock (&opts->lock); \ - result =sprintf (page,"%d\n" ,conv (f->frame.cname)); \ + result =sprintf (page,"%d\n" ,to_cpu_endian (f->frame.cname)); \mutex_unlock (&opts->lock); \ \mutex_unlock (su_mutex); \@@ -1061,7 +1061,11 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \struct uvcg_format *fmt; \struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\int ret; \ -uxx num; \ + u##bits num; \ + \ + ret = kstrtou##bits(page, 0, &num); \ + if (ret) \ + return ret; \ \ mutex_lock(su_mutex);/* for navigating configfs hierarchy */ \ \@@ -1075,15 +1079,7 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \ goto end; \ } \ \ - ret =str2u (page,0 , &num); \ -if (ret) \ - goto end; \ - \ -if (num > limit) { \ - ret = -EINVAL ; \ - goto end; \ - } \ - f->frame.cname =vnoc (num); \ + f->frame.cname =to_little_endian (num); \ ret = len; \ end: \mutex_unlock (&opts->lock); \@@ -1097,24 +1093,20 @@ static struct uvcg_frame_attribute \ uvcg_frame_##cname##_show, \ uvcg_frame_##cname##_store) -#define identity_conv (x) (x) +#define noop_conversion (x) (x) -UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, identity_conv, kstrtou8, u8, - identity_conv,0xFF ); -UVCG_FRAME_ATTR (w_width, wWidth, le16_to_cpu, kstrtou16, u16, cpu_to_le16, -0xFFFF ); -UVCG_FRAME_ATTR (w_height, wHeight, le16_to_cpu, kstrtou16, u16, cpu_to_le16, -0xFFFF ); -UVCG_FRAME_ATTR (dw_min_bit_rate, dwMinBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); -UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); +UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, 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 ); +UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, cpu_to_le32,32 );UVCG_FRAME_ATTR (dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32,32 );UVCG_FRAME_ATTR (dw_default_frame_interval, dwDefaultFrameInterval, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32,32 ); -#undef identity_conv +#undef noop_conversion#undef UVCG_FRAME_ATTR --To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dan,
Thank you for the patch.
Quoted 10 lines · On Thursday 15 January 2015 00:06:35 Dan Carpenter wrote:
1) Change "conv" an "vnoc" to "
to_cpu_endian" to "to_little_endian".
2) No need to check the "limit" because that is already handled in kstrtoXX so delete that parameter along with the check.
3) By using a "bits" parameter, we can combine the "uxx" parameter and the "str2u" parameters.
4) The kstrtou##bits() conversion does not need to be done under the mutex so move it to the start of the function.
5) Change the name of "identity_conv" to "noop_conversion".Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/gadget/function/uvc_configfs.c static struct uvcg_frame_attribute
Quoted 58 lines
---
This file has a couple pages of Sparse endian warnings.
http://lwn.net/Articles/205624/
It's hard to review the static checker warnings when there are so many.(quoted diff) +0
-0
b/drivers/usb/gadget/function/uvc_configfs.cindex a0443a2..87beb8c 100644drivers/usb/gadget/function/uvc_configfs.c +1
-1@@ -1032,7 +1032,7 @@ static struct configfs_item_operations uvcg_frame_item_ops = { .store_attribute = uvcg_frame_attr_store, }; -#define UVCG_FRAME_ATTR (cname, aname, conv, str2u, uxx, vnoc, limit) \ +#define UVCG_FRAME_ATTR(cname, aname, to_cpu_endian, to_little_endian,bits) \ static
ssize_tuvcg_frame_##cname##_show(structuvcg_frame*f, char
*page)\ { \drivers/usb/gadget/function/uvc_configfs.c +7
-11struct f_uvc_opts *opts; \@@ -1046,7 +1046,7 @@ static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\ opts = to_f_uvc_opts(opts_item); \ \mutex_lock (&opts->lock); \ - result =sprintf (page,"%d\n" ,conv (f->frame.cname)); \ + result =sprintf (page,"%d\n" ,to_cpu_endian (f->frame.cname)); \mutex_unlock (&opts->lock); \ \mutex_unlock (su_mutex); \@@ -1061,7 +1061,11 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \ struct uvcg_format *fmt; \struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\int ret; \ -uxx num; \ + u##bits num; \ + \ + ret = kstrtou##bits(page, 0, &num); \ + if (ret) \ + return ret; \ \ mutex_lock(su_mutex);/* for navigating configfs hierarchy */ \ \@@ -1075,15 +1079,7 @@ static ssize_t uvcg_frame_##cname##_store(struct uvcg_frame *f, \ goto end; \ } \ \ - ret =str2u (page,0 , &num); \ -if (ret) \ - goto end; \ - \ -if (num > limit) { \ - ret = -EINVAL ; \ - goto end; \ - } \ - f->frame.cname =vnoc (num); \ + f->frame.cname =to_little_endian (num); \ ret = len; \ end: \mutex_unlock (&opts->lock); \@@ -1097,24 +1093,20 @@ static struct uvcg_frame_attribute
\
Quoted 34 lines
drivers/usb/gadget/function/uvc_configfs.c +9
-11uvcg_frame_##cname##_show, \ uvcg_frame_##cname##_store) -#define identity_conv (x) (x) +#define noop_conversion (x) (x) -UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, identity_conv, kstrtou8, u8, - identity_conv, 0xFF); -UVCG_FRAME_ATTR (w_width, wWidth, le16_to_cpu, kstrtou16, u16, cpu_to_le16, -0xFFFF ); -UVCG_FRAME_ATTR (w_height, wHeight, le16_to_cpu, kstrtou16, u16, cpu_to_le16, - 0xFFFF); -UVCG_FRAME_ATTR (dw_min_bit_rate, dwMinBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); -UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, kstrtou32, u32, - cpu_to_le32,0xFFFFFFFF ); +UVCG_FRAME_ATTR (bm_capabilities, bmCapabilities, 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 ); +UVCG_FRAME_ATTR (dw_max_bit_rate, dwMaxBitRate, le32_to_cpu, cpu_to_le32,32 );UVCG_FRAME_ATTR (dw_max_video_frame_buffer_size, dwMaxVideoFrameBufferSize, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32, 32);UVCG_FRAME_ATTR (dw_default_frame_interval, dwDefaultFrameInterval, - le32_to_cpu, kstrtou32, u32, cpu_to_le32,0xFFFFFFFF ); + le32_to_cpu, cpu_to_le32,32 ); -#undef identity_conv +#undef noop_conversion#undef UVCG_FRAME_ATTR
Signature · 3 lines
Regards, Laurent Pinchart