| Message ID | 20221121092517.225242-3-dan.scally@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add XU support to UVC Gadget | expand |
Hi Dan, Thank you for the patch. On Mon, Nov 21, 2022 at 09:25:10AM +0000, Daniel Scally wrote: > the __uvcg_*frm_intrv() helper functions can be helpful when adding s/the/The/ > support for similar attributes. Generalise the function names and > move them higher in the file for better coverage. We also need copies > of the functions for different sized targets, so refactor them to > a macro with configurable bit size. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > Changes in v2: > > - none > > drivers/usb/gadget/function/uvc_configfs.c | 109 +++++++++++---------- > 1 file changed, 56 insertions(+), 53 deletions(-) > > diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c > index af4258120f9a..d542dd251633 100644 > --- a/drivers/usb/gadget/function/uvc_configfs.c > +++ b/drivers/usb/gadget/function/uvc_configfs.c > @@ -594,6 +594,60 @@ static const struct uvcg_config_group_type uvcg_terminal_grp_type = { > }, > }; > > +static inline int __uvcg_count_item_entries(char *buf, void *priv) > +{ > + ++*((int *)priv); > + return 0; > +} > + > +#define UVCG_ITEM_ENTRY_FUNCS(bits) \ > +static inline int __uvcg_fill_item_entries_u##bits(char *buf, void *priv)\ > +{ \ > + u##bits num, **interv; \ I'd rename interv to values as the function is now generic. > + int ret; \ > + \ > + ret = kstrtou##bits(buf, 0, &num); \ > + if (ret) \ > + return ret; \ > + \ > + interv = priv; \ > + **interv = num; \ > + ++*interv; \ > + \ > + return 0; \ > +} \ > + \ > +static int __uvcg_iter_item_entries_u##bits(const char *page, size_t len,\ > + int (*fun)(char *, void *), void *priv)\ > +{ \ > + /* sign, base 2 representation, newline, terminator */ \ > + char buf[1 + sizeof(u##bits) * 8 + 1 + 1]; \ As the only dependence on bits is a size, how about passing the size to the function as a parameter instead of duplicating the whole implementation ? > + const char *pg = page; \ > + int i, ret; \ > + \ > + if (!fun) \ > + return -EINVAL; \ > + \ > + while (pg - page < len) { \ > + i = 0; \ > + while (i < sizeof(buf) && (pg - page < len) && \ > + *pg != '\0' && *pg != '\n') \ > + buf[i++] = *pg++; \ > + if (i == sizeof(buf)) \ > + return -EINVAL; \ > + while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))\ > + ++pg; \ > + buf[i] = '\0'; \ > + ret = fun(buf, priv); \ > + if (ret) \ > + return ret; \ > + } \ > + \ > + return 0; \ > +} > + > +UVCG_ITEM_ENTRY_FUNCS(32) > + > /* ----------------------------------------------------------------------------- > * control/class/{fs|ss} > */ > @@ -1186,57 +1240,6 @@ static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item, > return result; > } > > -static inline int __uvcg_count_frm_intrv(char *buf, void *priv) > -{ > - ++*((int *)priv); > - return 0; > -} > - > -static inline int __uvcg_fill_frm_intrv(char *buf, void *priv) > -{ > - u32 num, **interv; > - int ret; > - > - ret = kstrtou32(buf, 0, &num); > - if (ret) > - return ret; > - > - interv = priv; > - **interv = num; > - ++*interv; > - > - return 0; > -} > - > -static int __uvcg_iter_frm_intrv(const char *page, size_t len, > - int (*fun)(char *, void *), void *priv) > -{ > - /* sign, base 2 representation, newline, terminator */ > - char buf[1 + sizeof(u32) * 8 + 1 + 1]; > - const char *pg = page; > - int i, ret; > - > - if (!fun) > - return -EINVAL; > - > - while (pg - page < len) { > - i = 0; > - while (i < sizeof(buf) && (pg - page < len) && > - *pg != '\0' && *pg != '\n') > - buf[i++] = *pg++; > - if (i == sizeof(buf)) > - return -EINVAL; > - while ((pg - page < len) && (*pg == '\0' || *pg == '\n')) > - ++pg; > - buf[i] = '\0'; > - ret = fun(buf, priv); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, > const char *page, size_t len) > { > @@ -1260,7 +1263,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, > goto end; > } > > - ret = __uvcg_iter_frm_intrv(page, len, __uvcg_count_frm_intrv, &n); > + ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_count_item_entries, &n); A wrapper macro could be nice, to be written ret = uvcg_count_item_entries(u32, page, len, &n); > if (ret) > goto end; > > @@ -1270,7 +1273,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, > goto end; > } > > - ret = __uvcg_iter_frm_intrv(page, len, __uvcg_fill_frm_intrv, &tmp); > + ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_fill_item_entries_u32, &tmp); And ret = uvcg_fill_item_entries(u32, page, len, &tmp); This in particular would make sure that the __uvcg_fill_item_entries_*() variant always matches the size passed to __uvcg_iter_item_entries_u32(). This is probably a candidate for a separate patch on top of this one. > if (ret) { > kfree(frm_intrv); > goto end;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index af4258120f9a..d542dd251633 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -594,6 +594,60 @@ static const struct uvcg_config_group_type uvcg_terminal_grp_type = { }, }; +static inline int __uvcg_count_item_entries(char *buf, void *priv) +{ + ++*((int *)priv); + return 0; +} + +#define UVCG_ITEM_ENTRY_FUNCS(bits) \ +static inline int __uvcg_fill_item_entries_u##bits(char *buf, void *priv)\ +{ \ + u##bits num, **interv; \ + int ret; \ + \ + ret = kstrtou##bits(buf, 0, &num); \ + if (ret) \ + return ret; \ + \ + interv = priv; \ + **interv = num; \ + ++*interv; \ + \ + return 0; \ +} \ + \ +static int __uvcg_iter_item_entries_u##bits(const char *page, size_t len,\ + int (*fun)(char *, void *), void *priv)\ +{ \ + /* sign, base 2 representation, newline, terminator */ \ + char buf[1 + sizeof(u##bits) * 8 + 1 + 1]; \ + const char *pg = page; \ + int i, ret; \ + \ + if (!fun) \ + return -EINVAL; \ + \ + while (pg - page < len) { \ + i = 0; \ + while (i < sizeof(buf) && (pg - page < len) && \ + *pg != '\0' && *pg != '\n') \ + buf[i++] = *pg++; \ + if (i == sizeof(buf)) \ + return -EINVAL; \ + while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))\ + ++pg; \ + buf[i] = '\0'; \ + ret = fun(buf, priv); \ + if (ret) \ + return ret; \ + } \ + \ + return 0; \ +} + +UVCG_ITEM_ENTRY_FUNCS(32) + /* ----------------------------------------------------------------------------- * control/class/{fs|ss} */ @@ -1186,57 +1240,6 @@ static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item, return result; } -static inline int __uvcg_count_frm_intrv(char *buf, void *priv) -{ - ++*((int *)priv); - return 0; -} - -static inline int __uvcg_fill_frm_intrv(char *buf, void *priv) -{ - u32 num, **interv; - int ret; - - ret = kstrtou32(buf, 0, &num); - if (ret) - return ret; - - interv = priv; - **interv = num; - ++*interv; - - return 0; -} - -static int __uvcg_iter_frm_intrv(const char *page, size_t len, - int (*fun)(char *, void *), void *priv) -{ - /* sign, base 2 representation, newline, terminator */ - char buf[1 + sizeof(u32) * 8 + 1 + 1]; - const char *pg = page; - int i, ret; - - if (!fun) - return -EINVAL; - - while (pg - page < len) { - i = 0; - while (i < sizeof(buf) && (pg - page < len) && - *pg != '\0' && *pg != '\n') - buf[i++] = *pg++; - if (i == sizeof(buf)) - return -EINVAL; - while ((pg - page < len) && (*pg == '\0' || *pg == '\n')) - ++pg; - buf[i] = '\0'; - ret = fun(buf, priv); - if (ret) - return ret; - } - - return 0; -} - static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, const char *page, size_t len) { @@ -1260,7 +1263,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, goto end; } - ret = __uvcg_iter_frm_intrv(page, len, __uvcg_count_frm_intrv, &n); + ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_count_item_entries, &n); if (ret) goto end; @@ -1270,7 +1273,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item, goto end; } - ret = __uvcg_iter_frm_intrv(page, len, __uvcg_fill_frm_intrv, &tmp); + ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_fill_item_entries_u32, &tmp); if (ret) { kfree(frm_intrv); goto end;
the __uvcg_*frm_intrv() helper functions can be helpful when adding support for similar attributes. Generalise the function names and move them higher in the file for better coverage. We also need copies of the functions for different sized targets, so refactor them to a macro with configurable bit size. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- Changes in v2: - none drivers/usb/gadget/function/uvc_configfs.c | 109 +++++++++++---------- 1 file changed, 56 insertions(+), 53 deletions(-)