Hi Dan,

Thank you for the patch.

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 <dan.carpen...@oracle.com>

Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

> ---
> 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.
> 
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c
> b/drivers/usb/gadget/function/uvc_configfs.c index a0443a2..87beb8c 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -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

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to