Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling

From: Felipe Balbi
Date: Thu Sep 24 2015 - 12:49:23 EST

Next message: Andrea Arcangeli: "Re: [RFC] futex: prevent endless loop on s390x with emulated hugepages"
Previous message: Borislav Petkov: "Re: [PATCH v2 2/2] EDAC: Fix sysfs dimm_label store operation"
In reply to: Robert Baldyga: "[PATCH v2] usb: gadget: f_sourcesink: fix function params handling"
Next in thread: Krzysztof Opasiak: "Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:
> Move function parameters to struct f_sourcesink to make them per instance
> instead of having them as global variables. Since we can have multiple
> instances of USB function we also want to have separate set of parameters
> for each instance.
>
> Cc: <stable@xxxxxxxxxxxxxxx> # 3.10+
> Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>

why do you think this is stable material ? Looks like it's not
fixing anything to me; just implementing a brand new requirement.

> ---
> drivers/usb/gadget/function/f_sourcesink.c | 120 +++++++++++++++--------------
> 1 file changed, 62 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
> index 1353465..128abfb 100644
> --- a/drivers/usb/gadget/function/f_sourcesink.c
> +++ b/drivers/usb/gadget/function/f_sourcesink.c
> @@ -50,6 +50,13 @@ struct f_sourcesink {
> struct usb_ep *iso_in_ep;
> struct usb_ep *iso_out_ep;
> int cur_alt;
> +
> + unsigned pattern;
> + unsigned isoc_interval;
> + unsigned isoc_maxpacket;
> + unsigned isoc_mult;
> + unsigned isoc_maxburst;
> + unsigned buflen;
> };
>
> static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
> @@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
> return container_of(f, struct f_sourcesink, function);
> }
>
> -static unsigned pattern;
> -static unsigned isoc_interval;
> -static unsigned isoc_maxpacket;
> -static unsigned isoc_mult;
> -static unsigned isoc_maxburst;
> -static unsigned buflen;
> -
> /*-------------------------------------------------------------------------*/
>
> static struct usb_interface_descriptor source_sink_intf_alt0 = {
> @@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
>
> static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
> {
> - return alloc_ep_req(ep, len, buflen);
> + struct f_sourcesink *ss = ep->driver_data;
> +
> + return alloc_ep_req(ep, len, ss->buflen);
> }
>
> void free_ep_req(struct usb_ep *ep, struct usb_request *req)
> @@ -357,22 +359,22 @@ autoconf_fail:
> goto autoconf_fail;
>
> /* sanity check the isoc module parameters */
> - if (isoc_interval < 1)
> - isoc_interval = 1;
> - if (isoc_interval > 16)
> - isoc_interval = 16;
> - if (isoc_mult > 2)
> - isoc_mult = 2;
> - if (isoc_maxburst > 15)
> - isoc_maxburst = 15;
> + if (ss->isoc_interval < 1)
> + ss->isoc_interval = 1;
> + if (ss->isoc_interval > 16)
> + ss->isoc_interval = 16;
> + if (ss->isoc_mult > 2)
> + ss->isoc_mult = 2;
> + if (ss->isoc_maxburst > 15)
> + ss->isoc_maxburst = 15;
>
> /* fill in the FS isoc descriptors from the module parameters */
> - fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
> - 1023 : isoc_maxpacket;
> - fs_iso_source_desc.bInterval = isoc_interval;
> - fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
> - 1023 : isoc_maxpacket;
> - fs_iso_sink_desc.bInterval = isoc_interval;
> + fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> + fs_iso_source_desc.bInterval = ss->isoc_interval;
> + fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> + fs_iso_sink_desc.bInterval = ss->isoc_interval;
>
> /* allocate iso endpoints */
> ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
> @@ -394,8 +396,8 @@ no_iso:
> ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
> }
>
> - if (isoc_maxpacket > 1024)
> - isoc_maxpacket = 1024;
> + if (ss->isoc_maxpacket > 1024)
> + ss->isoc_maxpacket = 1024;
>
> /* support high speed hardware */
> hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
> @@ -406,15 +408,15 @@ no_iso:
> * We assume that the user knows what they are doing and won't
> * give parameters that their UDC doesn't support.
> */
> - hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
> - hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
> - hs_iso_source_desc.bInterval = isoc_interval;
> + hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
> + hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
> + hs_iso_source_desc.bInterval = ss->isoc_interval;
> hs_iso_source_desc.bEndpointAddress =
> fs_iso_source_desc.bEndpointAddress;
>
> - hs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
> - hs_iso_sink_desc.wMaxPacketSize |= isoc_mult << 11;
> - hs_iso_sink_desc.bInterval = isoc_interval;
> + hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
> + hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
> + hs_iso_sink_desc.bInterval = ss->isoc_interval;
> hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>
> /* support super speed hardware */
> @@ -428,21 +430,21 @@ no_iso:
> * We assume that the user knows what they are doing and won't
> * give parameters that their UDC doesn't support.
> */
> - ss_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
> - ss_iso_source_desc.bInterval = isoc_interval;
> - ss_iso_source_comp_desc.bmAttributes = isoc_mult;
> - ss_iso_source_comp_desc.bMaxBurst = isoc_maxburst;
> - ss_iso_source_comp_desc.wBytesPerInterval =
> - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1);
> + ss_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
> + ss_iso_source_desc.bInterval = ss->isoc_interval;
> + ss_iso_source_comp_desc.bmAttributes = ss->isoc_mult;
> + ss_iso_source_comp_desc.bMaxBurst = ss->isoc_maxburst;
> + ss_iso_source_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
> + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
> ss_iso_source_desc.bEndpointAddress =
> fs_iso_source_desc.bEndpointAddress;
>
> - ss_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
> - ss_iso_sink_desc.bInterval = isoc_interval;
> - ss_iso_sink_comp_desc.bmAttributes = isoc_mult;
> - ss_iso_sink_comp_desc.bMaxBurst = isoc_maxburst;
> - ss_iso_sink_comp_desc.wBytesPerInterval =
> - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1);
> + ss_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
> + ss_iso_sink_desc.bInterval = ss->isoc_interval;
> + ss_iso_sink_comp_desc.bmAttributes = ss->isoc_mult;
> + ss_iso_sink_comp_desc.bMaxBurst = ss->isoc_maxburst;
> + ss_iso_sink_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
> + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
> ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>
> ret = usb_assign_descriptors(f, fs_source_sink_descs,
> @@ -482,11 +484,11 @@ static int check_read_data(struct f_sourcesink *ss, struct usb_request *req)
> struct usb_composite_dev *cdev = ss->function.config->cdev;
> int max_packet_size = le16_to_cpu(ss->out_ep->desc->wMaxPacketSize);
>
> - if (pattern == 2)
> + if (ss->pattern == 2)
> return 0;
>
> for (i = 0; i < req->actual; i++, buf++) {
> - switch (pattern) {
> + switch (ss->pattern) {
>
> /* all-zeroes has no synchronization issues */
> case 0:
> @@ -518,8 +520,9 @@ static void reinit_write_data(struct usb_ep *ep, struct usb_request *req)
> unsigned i;
> u8 *buf = req->buf;
> int max_packet_size = le16_to_cpu(ep->desc->wMaxPacketSize);
> + struct f_sourcesink *ss = ep->driver_data;
>
> - switch (pattern) {
> + switch (ss->pattern) {
> case 0:
> memset(req->buf, 0, req->length);
> break;
> @@ -549,7 +552,7 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
> case 0: /* normal completion? */
> if (ep == ss->out_ep) {
> check_read_data(ss, req);
> - if (pattern != 2)
> + if (ss->pattern != 2)
> memset(req->buf, 0x55, req->length);
> }
> break;
> @@ -598,15 +601,16 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
> if (is_iso) {
> switch (speed) {
> case USB_SPEED_SUPER:
> - size = isoc_maxpacket * (isoc_mult + 1) *
> - (isoc_maxburst + 1);
> + size = ss->isoc_maxpacket *
> + (ss->isoc_mult + 1) *
> + (ss->isoc_maxburst + 1);
> break;
> case USB_SPEED_HIGH:
> - size = isoc_maxpacket * (isoc_mult + 1);
> + size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
> break;
> default:
> - size = isoc_maxpacket > 1023 ?
> - 1023 : isoc_maxpacket;
> + size = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> break;
> }
> ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
> @@ -622,7 +626,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
> req->complete = source_sink_complete;
> if (is_in)
> reinit_write_data(ep, req);
> - else if (pattern != 2)
> + else if (ss->pattern != 2)
> memset(req->buf, 0x55, req->length);
>
> status = usb_ep_queue(ep, req, GFP_ATOMIC);
> @@ -859,12 +863,12 @@ static struct usb_function *source_sink_alloc_func(
> ss_opts->refcnt++;
> mutex_unlock(&ss_opts->lock);
>
> - pattern = ss_opts->pattern;
> - isoc_interval = ss_opts->isoc_interval;
> - isoc_maxpacket = ss_opts->isoc_maxpacket;
> - isoc_mult = ss_opts->isoc_mult;
> - isoc_maxburst = ss_opts->isoc_maxburst;
> - buflen = ss_opts->bulk_buflen;
> + ss->pattern = ss_opts->pattern;
> + ss->isoc_interval = ss_opts->isoc_interval;
> + ss->isoc_maxpacket = ss_opts->isoc_maxpacket;
> + ss->isoc_mult = ss_opts->isoc_mult;
> + ss->isoc_maxburst = ss_opts->isoc_maxburst;
> + ss->buflen = ss_opts->bulk_buflen;
>
> ss->function.name = "source/sink";
> ss->function.bind = sourcesink_bind;
> --
> 1.9.1
>

--
balbi

Attachment: signature.asc
Description: PGP signature

Next message: Andrea Arcangeli: "Re: [RFC] futex: prevent endless loop on s390x with emulated hugepages"
Previous message: Borislav Petkov: "Re: [PATCH v2 2/2] EDAC: Fix sysfs dimm_label store operation"
In reply to: Robert Baldyga: "[PATCH v2] usb: gadget: f_sourcesink: fix function params handling"
Next in thread: Krzysztof Opasiak: "Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]