Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759607AbcCDSq0 (ORCPT ); Fri, 4 Mar 2016 13:46:26 -0500 Received: from mail-wm0-f46.google.com ([74.125.82.46]:35632 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759066AbcCDSqZ (ORCPT ); Fri, 4 Mar 2016 13:46:25 -0500 User-Agent: K-9 Mail for Android In-Reply-To: <87wppi67c5.fsf@ti.com> References: <1456947640-20673-1-git-send-email-eu@felipetonello.com> <1456947640-20673-4-git-send-email-eu@felipetonello.com> <87wppi67c5.fsf@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes From: Felipe Ferreri Tonello Date: Fri, 04 Mar 2016 18:46:18 +0000 To: Felipe Balbi , linux-usb@vger.kernel.org CC: linux-kernel@vger.kernel.org, Michal Nazarewicz , Clemens Ladisch Message-ID: <270D9ECD-1810-48BC-BBE9-9C9DD5E44D4F@felipetonello.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2309 Lines: 70 Hi Balbi, On March 4, 2016 7:16:42 AM GMT+00:00, Felipe Balbi wrote: > >Hi, > >"Felipe F. Tonello" 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 allows >> 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 >> --- >> 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/legacy/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 = 1; >> module_param(out_ports, uint, S_IRUGO); >> MODULE_PARM_DESC(out_ports, "Number of MIDI output ports"); >> >> +static unsigned int bmAttributes = USB_CONFIG_ATT_ONE; >> +module_param(bmAttributes, uint, S_IRUGO); >> +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's >bmAttributes parameter"); >> + >> +static unsigned int MaxPower = 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. Yes I always run checkpatch :) What do you mean by reloading the module? > >> @@ -119,8 +127,8 @@ static struct usb_configuration midi_config = { >> .label = "MIDI Gadget", >> .bConfigurationValue = 1, >> /* .iConfiguration = DYNAMIC */ >> - .bmAttributes = 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. You are right. It needs to check the value before it assigns to bmAttributes. Felipe -- Sent from my Android device with K-9 Mail. Please excuse my brevity.