From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756682AbbIXQtV (ORCPT ); Thu, 24 Sep 2015 12:49:21 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:38844 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755779AbbIXQtS (ORCPT ); Thu, 24 Sep 2015 12:49:18 -0400 Date: Thu, 24 Sep 2015 11:49:13 -0500 From: Felipe Balbi To: Robert Baldyga CC: , , , , , , Subject: Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling Message-ID: <20150924164913.GE14151@saruman.tx.rr.com> Reply-To: References: <1443108189-3464-1-git-send-email-r.baldyga@samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="XuV1QlJbYrcVoo+x" Content-Disposition: inline In-Reply-To: <1443108189-3464-1-git-send-email-r.baldyga@samsung.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --XuV1QlJbYrcVoo+x Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Cc: # 3.10+ > Signed-off-by: Robert Baldyga 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(-) >=20 > diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gad= get/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; > }; > =20 > 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 u= sb_function *f) > return container_of(f, struct f_sourcesink, function); > } > =20 > -static unsigned pattern; > -static unsigned isoc_interval; > -static unsigned isoc_maxpacket; > -static unsigned isoc_mult; > -static unsigned isoc_maxburst; > -static unsigned buflen; > - > /*----------------------------------------------------------------------= ---*/ > =20 > static struct usb_interface_descriptor source_sink_intf_alt0 =3D { > @@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[= ] =3D { > =20 > 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 =3D ep->driver_data; > + > + return alloc_ep_req(ep, len, ss->buflen); > } > =20 > void free_ep_req(struct usb_ep *ep, struct usb_request *req) > @@ -357,22 +359,22 @@ autoconf_fail: > goto autoconf_fail; > =20 > /* sanity check the isoc module parameters */ > - if (isoc_interval < 1) > - isoc_interval =3D 1; > - if (isoc_interval > 16) > - isoc_interval =3D 16; > - if (isoc_mult > 2) > - isoc_mult =3D 2; > - if (isoc_maxburst > 15) > - isoc_maxburst =3D 15; > + if (ss->isoc_interval < 1) > + ss->isoc_interval =3D 1; > + if (ss->isoc_interval > 16) > + ss->isoc_interval =3D 16; > + if (ss->isoc_mult > 2) > + ss->isoc_mult =3D 2; > + if (ss->isoc_maxburst > 15) > + ss->isoc_maxburst =3D 15; > =20 > /* fill in the FS isoc descriptors from the module parameters */ > - fs_iso_source_desc.wMaxPacketSize =3D isoc_maxpacket > 1023 ? > - 1023 : isoc_maxpacket; > - fs_iso_source_desc.bInterval =3D isoc_interval; > - fs_iso_sink_desc.wMaxPacketSize =3D isoc_maxpacket > 1023 ? > - 1023 : isoc_maxpacket; > - fs_iso_sink_desc.bInterval =3D isoc_interval; > + fs_iso_source_desc.wMaxPacketSize =3D ss->isoc_maxpacket > 1023 ? > + 1023 : ss->isoc_maxpacket; > + fs_iso_source_desc.bInterval =3D ss->isoc_interval; > + fs_iso_sink_desc.wMaxPacketSize =3D ss->isoc_maxpacket > 1023 ? > + 1023 : ss->isoc_maxpacket; > + fs_iso_sink_desc.bInterval =3D ss->isoc_interval; > =20 > /* allocate iso endpoints */ > ss->iso_in_ep =3D usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc); > @@ -394,8 +396,8 @@ no_iso: > ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] =3D NULL; > } > =20 > - if (isoc_maxpacket > 1024) > - isoc_maxpacket =3D 1024; > + if (ss->isoc_maxpacket > 1024) > + ss->isoc_maxpacket =3D 1024; > =20 > /* support high speed hardware */ > hs_source_desc.bEndpointAddress =3D 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 =3D isoc_maxpacket; > - hs_iso_source_desc.wMaxPacketSize |=3D isoc_mult << 11; > - hs_iso_source_desc.bInterval =3D isoc_interval; > + hs_iso_source_desc.wMaxPacketSize =3D ss->isoc_maxpacket; > + hs_iso_source_desc.wMaxPacketSize |=3D ss->isoc_mult << 11; > + hs_iso_source_desc.bInterval =3D ss->isoc_interval; > hs_iso_source_desc.bEndpointAddress =3D > fs_iso_source_desc.bEndpointAddress; > =20 > - hs_iso_sink_desc.wMaxPacketSize =3D isoc_maxpacket; > - hs_iso_sink_desc.wMaxPacketSize |=3D isoc_mult << 11; > - hs_iso_sink_desc.bInterval =3D isoc_interval; > + hs_iso_sink_desc.wMaxPacketSize =3D ss->isoc_maxpacket; > + hs_iso_sink_desc.wMaxPacketSize |=3D ss->isoc_mult << 11; > + hs_iso_sink_desc.bInterval =3D ss->isoc_interval; > hs_iso_sink_desc.bEndpointAddress =3D fs_iso_sink_desc.bEndpointAddress; > =20 > /* 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 =3D isoc_maxpacket; > - ss_iso_source_desc.bInterval =3D isoc_interval; > - ss_iso_source_comp_desc.bmAttributes =3D isoc_mult; > - ss_iso_source_comp_desc.bMaxBurst =3D isoc_maxburst; > - ss_iso_source_comp_desc.wBytesPerInterval =3D > - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1); > + ss_iso_source_desc.wMaxPacketSize =3D ss->isoc_maxpacket; > + ss_iso_source_desc.bInterval =3D ss->isoc_interval; > + ss_iso_source_comp_desc.bmAttributes =3D ss->isoc_mult; > + ss_iso_source_comp_desc.bMaxBurst =3D ss->isoc_maxburst; > + ss_iso_source_comp_desc.wBytesPerInterval =3D ss->isoc_maxpacket * > + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1); > ss_iso_source_desc.bEndpointAddress =3D > fs_iso_source_desc.bEndpointAddress; > =20 > - ss_iso_sink_desc.wMaxPacketSize =3D isoc_maxpacket; > - ss_iso_sink_desc.bInterval =3D isoc_interval; > - ss_iso_sink_comp_desc.bmAttributes =3D isoc_mult; > - ss_iso_sink_comp_desc.bMaxBurst =3D isoc_maxburst; > - ss_iso_sink_comp_desc.wBytesPerInterval =3D > - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1); > + ss_iso_sink_desc.wMaxPacketSize =3D ss->isoc_maxpacket; > + ss_iso_sink_desc.bInterval =3D ss->isoc_interval; > + ss_iso_sink_comp_desc.bmAttributes =3D ss->isoc_mult; > + ss_iso_sink_comp_desc.bMaxBurst =3D ss->isoc_maxburst; > + ss_iso_sink_comp_desc.wBytesPerInterval =3D ss->isoc_maxpacket * > + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1); > ss_iso_sink_desc.bEndpointAddress =3D fs_iso_sink_desc.bEndpointAddress; > =20 > ret =3D 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 =3D ss->function.config->cdev; > int max_packet_size =3D le16_to_cpu(ss->out_ep->desc->wMaxPacketSize); > =20 > - if (pattern =3D=3D 2) > + if (ss->pattern =3D=3D 2) > return 0; > =20 > for (i =3D 0; i < req->actual; i++, buf++) { > - switch (pattern) { > + switch (ss->pattern) { > =20 > /* all-zeroes has no synchronization issues */ > case 0: > @@ -518,8 +520,9 @@ static void reinit_write_data(struct usb_ep *ep, stru= ct usb_request *req) > unsigned i; > u8 *buf =3D req->buf; > int max_packet_size =3D le16_to_cpu(ep->desc->wMaxPacketSize); > + struct f_sourcesink *ss =3D ep->driver_data; > =20 > - 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, s= truct usb_request *req) > case 0: /* normal completion? */ > if (ep =3D=3D ss->out_ep) { > check_read_data(ss, req); > - if (pattern !=3D 2) > + if (ss->pattern !=3D 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 =3D isoc_maxpacket * (isoc_mult + 1) * > - (isoc_maxburst + 1); > + size =3D ss->isoc_maxpacket * > + (ss->isoc_mult + 1) * > + (ss->isoc_maxburst + 1); > break; > case USB_SPEED_HIGH: > - size =3D isoc_maxpacket * (isoc_mult + 1); > + size =3D ss->isoc_maxpacket * (ss->isoc_mult + 1); > break; > default: > - size =3D isoc_maxpacket > 1023 ? > - 1023 : isoc_maxpacket; > + size =3D ss->isoc_maxpacket > 1023 ? > + 1023 : ss->isoc_maxpacket; > break; > } > ep =3D 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 =3D source_sink_complete; > if (is_in) > reinit_write_data(ep, req); > - else if (pattern !=3D 2) > + else if (ss->pattern !=3D 2) > memset(req->buf, 0x55, req->length); > =20 > status =3D 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); > =20 > - pattern =3D ss_opts->pattern; > - isoc_interval =3D ss_opts->isoc_interval; > - isoc_maxpacket =3D ss_opts->isoc_maxpacket; > - isoc_mult =3D ss_opts->isoc_mult; > - isoc_maxburst =3D ss_opts->isoc_maxburst; > - buflen =3D ss_opts->bulk_buflen; > + ss->pattern =3D ss_opts->pattern; > + ss->isoc_interval =3D ss_opts->isoc_interval; > + ss->isoc_maxpacket =3D ss_opts->isoc_maxpacket; > + ss->isoc_mult =3D ss_opts->isoc_mult; > + ss->isoc_maxburst =3D ss_opts->isoc_maxburst; > + ss->buflen =3D ss_opts->bulk_buflen; > =20 > ss->function.name =3D "source/sink"; > ss->function.bind =3D sourcesink_bind; > --=20 > 1.9.1 >=20 --=20 balbi --XuV1QlJbYrcVoo+x Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWBCmJAAoJEIaOsuA1yqREg7oP+gJEM23JIdYndzQBSC6kmphQ rzuYYqbJ7nOsYT1Vbb0kSSwD6JT23fyiNBKQSwPjpzeWSpUJCQdATsDOaCww0bXj p0Cj/DP3i7gMEIYUedyIYZ35Qkb+w59EuRtpK/qZPR2sQuCMMH9kkQfLPSn7ibFD QRiEj6VyVhczBPDDeyYWFxumfYysYLMkwnSxU83sF16eb+nOH17fIOlY55g8yuZb jMJvFTgQdPC65g79ysOKqMK+xOmTk2AIkQ6rCJRSHL1i43zlkNaTypGyhGOnOyXF tfBGVVWlzH9XtIBKY/QOSWR3shAQgz3KgvXl3iI1N9ixym1/cKYhPZgV8eu5RIxG 8lhNvBlZLHtTAOTQ1oVJVwuZJV5V13nPwOCWptGb+P7FNJTIej1D2ZNvsX9JADV0 8+eRdzVKR1SIe02LOYhmwQhvBk7PqPaRFl5jkVp2cDDSzwVQjvpOD9sRdHwkBpZe /TW2eM5POOIc5tNJhoAIc3a5kdyYlVVjak6tPaMLCBeelR2N9Wh+LxnrOeutnaYN gcES+EbocAbDrB05Hdfd8Qzw+bnrF1okEFesMd1KgVXdESD7wMqySQkT2T9GbQQp 4byw0/ssC/hpu64h9tN6ivkwLgvvOZ+facyqi5JcTqksLT+hn15qepbs2qoz41G2 YuCXco5SHCOVMsg3tiVn =aGaL -----END PGP SIGNATURE----- --XuV1QlJbYrcVoo+x--