From: Felipe Balbi <balbi@kernel.org>
To: "Felipe F. Tonello" <eu@felipetonello.com>, linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Michal Nazarewicz <mina86@mina86.com>,
        Clemens Ladisch <clemens@ladisch.de>
Subject: Re: [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes
In-Reply-To: <1456947640-20673-4-git-send-email-eu@felipetonello.com>
References: <1456947640-20673-1-git-send-email-eu@felipetonello.com> <1456947640-20673-4-git-send-email-eu@felipetonello.com>
User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/25.0.50.2 (x86_64-pc-linux-gnu)
Date: Fri, 04 Mar 2016 09:16:42 +0200
Message-ID: <87wppi67c5.fsf@ti.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2951
Lines: 83

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable


Hi,

"Felipe F. Tonello" <eu@felipetonello.com> writes:
> [ text/plain ]
> This gadget uses a bmAttributes and MaxPower that requires the USB bus to=
 be
> powered from the host, which is not correct because this configuration is=
 device
> specific, not a USB-MIDI requirement.
>
> This patch adds two modules parameters, bmAttributes and MaxPower, that a=
llows
> the user to set those flags. The default values are what the gadget used =
to use
> for backward compatibility.
>
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
>  drivers/usb/gadget/legacy/gmidi.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/gmidi.c b/drivers/usb/gadget/legac=
y/gmidi.c
> index fc2ac150f5ff..0553435cc360 100644
> --- a/drivers/usb/gadget/legacy/gmidi.c
> +++ b/drivers/usb/gadget/legacy/gmidi.c
> @@ -63,6 +63,14 @@ static unsigned int out_ports =3D 1;
>  module_param(out_ports, uint, S_IRUGO);
>  MODULE_PARM_DESC(out_ports, "Number of MIDI output ports");
>=20=20
> +static unsigned int bmAttributes =3D USB_CONFIG_ATT_ONE;
> +module_param(bmAttributes, uint, S_IRUGO);
> +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's bmAttributes =
parameter");
> +
> +static unsigned int MaxPower =3D CONFIG_USB_GADGET_VBUS_DRAW;
> +module_param(MaxPower, uint, S_IRUGO);
> +MODULE_PARM_DESC(MaxPower, "Used to calculate Configuration Descriptor's=
 bMaxPower parameter");

you didn't run checkpatch, did you ? Also, are you sure you will need to
change this by simply reloading the module ? I'm not convinced.

> @@ -119,8 +127,8 @@ static struct usb_configuration midi_config =3D {
>  	.label		=3D "MIDI Gadget",
>  	.bConfigurationValue =3D 1,
>  	/* .iConfiguration =3D DYNAMIC */
> -	.bmAttributes	=3D USB_CONFIG_ATT_ONE,

nack, nackety nack nack nack :-)

USB_CONFIG_ATT_ONE *must* always be set. With your module parameter you
give users the oportunity to violate USB spec.

=2D-=20
balbi

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJW2TZbAAoJEIaOsuA1yqRE1FMQAJJRHFp8jbWHIjtKSdUSX9Zc
nPkrzyVWADFcaHVf2/pK8HV27LG1iDIHnQE2pohZJdLiUfEUZDTCJV6KvwwzTk/M
+5briJ+VVvfaEC/Tb0jqxVTRgqIRtfYkVNeltE2Oc4UzYZUglgSiGEi2kKroosQa
C80eMBDlVAx7EaxoJqoxlEdoBXCpgpY2M6suDR3g3C5myXAoVLQG2ew2QgKc9gen
yOBFMydrMR7o3ooa7Wi7U+aq7xGktPrd3rEhnxYUmwWzEkliOmpWGNqfLzzuq06K
UIsTRC3Wv6JHDHhMYgeuC5Iv/IcTNCfEfZfOk6Cs2X9rA48wHVgrUDKn0eIvwTxX
Rbt20QQ6AfskVP9U4H4WCBuYYc6Avc0fIsrgsKLtnF27U3OYitXMulSUhLGhJvMA
VMl/efOaMi69n3iTZWznHcuohZ+bovNuuyCkzeLKKCNNHD/nwRYKxQPMk25pkVSf
ZMr1zn5DoEbmNO5879PJjumTz3Q/rSI8NJYAVfp7qeU4XAragYHsJ5GpTozc1dSV
o3l5sfpK18m17sLEKeT9rycBgRWWNGmq0vz5V5Pn3t2Gu5Tslzlw5wNAc7vzCd4D
FViH1qN+H7AaLAs5+aAGMu/OI3q7PkL6MaXwo9aB6w1r5928g8NIn7w39MajVUFS
EI5xm9/ojQVtwIK0+rKE
=q4sN
-----END PGP SIGNATURE-----
--=-=-=--