Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1348422 > unrolled thread

[PATCH 0/5] MIDI USB Gadget improvements

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] MIDI USB Gadget improvements "Felipe F. Tonello" <eu@felipetonello.com> - 2016-03-02 20:40 +0100
    [PATCH 5/5] usb: gadget: f_midi: updated copyright "Felipe F. Tonello" <eu@felipetonello.com> - 2016-03-02 20:40 +0100
      Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright Felipe Balbi <balbi@kernel.org> - 2016-03-04 08:20 +0100
        Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-04 19:50 +0100
          Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright Felipe Balbi <balbi@kernel.org> - 2016-03-07 08:40 +0100
            Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-07 10:30 +0100
    [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes "Felipe F. Tonello" <eu@felipetonello.com> - 2016-03-02 20:40 +0100
      Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Balbi <balbi@kernel.org> - 2016-03-04 08:20 +0100
      Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Michal Nazarewicz <mina86@mina86.com> - 2016-03-04 20:20 +0100
        Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-04 21:20 +0100
          Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Michal Nazarewicz <mina86@mina86.com> - 2016-03-05 17:30 +0100
            Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Greg KH <greg@kroah.com> - 2016-03-05 20:40 +0100
              Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-06 01:00 +0100
                Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Greg KH <greg@kroah.com> - 2016-03-06 04:10 +0100
            Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-06 01:00 +0100
              Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Balbi <balbi@kernel.org> - 2016-03-07 08:40 +0100
                Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-07 10:40 +0100
                  Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe Balbi <balbi@kernel.org> - 2016-03-08 08:50 +0100
    [PATCH 1/5] usb: gadget: f_midi: refactor state machine "Felipe F. Tonello" <eu@felipetonello.com> - 2016-03-02 20:50 +0100
      Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Clemens Ladisch <clemens@ladisch.de> - 2016-03-02 22:20 +0100
        Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-03 10:00 +0100
          Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Clemens Ladisch <clemens@ladisch.de> - 2016-03-03 12:40 +0100
            Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-03 17:30 +0100
              Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Clemens Ladisch <clemens@ladisch.de> - 2016-03-04 10:40 +0100
                Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-04 19:50 +0100
                  Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine Clemens Ladisch <clemens@ladisch.de> - 2016-03-04 19:50 +0100
    Re: [PATCH 0/5] MIDI USB Gadget improvements Felipe Balbi <balbi@kernel.org> - 2016-03-04 08:20 +0100
      Re: [PATCH 0/5] MIDI USB Gadget improvements Felipe Ferreri Tonello <eu@felipetonello.com> - 2016-03-04 19:50 +0100

Page 1 of 2  [1] 2  Next page →

#1348422 — [PATCH 0/5] MIDI USB Gadget improvements

Patches are pretty much self-described.

Patch 1 is revised from comments.
Patch 2 is a bug fix.

Felipe F. Tonello (5):
  usb: gadget: f_midi: refactor state machine
  usb: gadget: f_midi: added spinlock on transmit function
  usb: gadget: gmidi: remove bus powered requirement on bmAttributes
  usb: gadget: f_midi: cleanups and typos fixes
  usb: gadget: f_midi: updated copyright

 drivers/usb/gadget/function/f_midi.c | 293 ++++++++++++++++++++++-------------
 drivers/usb/gadget/legacy/gmidi.c    |  14 +-
 2 files changed, 193 insertions(+), 114 deletions(-)

-- 
2.7.2

[toc] | [next] | [standalone]

#1348425 — [PATCH 5/5] usb: gadget: f_midi: updated copyright

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 drivers/usb/gadget/function/f_midi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 9a9e6112e224..5c7f5c780fda 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -5,6 +5,9 @@
  * Developed for Thumtronics by Grey Innovation
  * Ben Williamson <ben.williamson@greyinnovation.com>
  *
+ * Copyright (C) 2015,2016 ROLI Ltd.
+ * Felipe F. Tonello <felipe.tonello@roli.com>
+ *
  * Rewritten for the composite framework
  *   Copyright (C) 2011 Daniel Mack <zonque@gmail.com>
  *
-- 
2.7.2

[toc] | [prev] | [next] | [standalone]

#1349908 — Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright

[Multipart message — attachments visible in raw view] — view raw

"Felipe F. Tonello" <eu@felipetonello.com> writes:
> [ text/plain ]
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>

no commit log == no commit

> ---
>  drivers/usb/gadget/function/f_midi.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 9a9e6112e224..5c7f5c780fda 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -5,6 +5,9 @@
>   * Developed for Thumtronics by Grey Innovation
>   * Ben Williamson <ben.williamson@greyinnovation.com>
>   *
> + * Copyright (C) 2015,2016 ROLI Ltd.
> + * Felipe F. Tonello <felipe.tonello@roli.com>

Did you check with your company's lawyer that your changes are enough to
justify a copyright ?

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1350515 — Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright

Hi Balbi, 

On March 4, 2016 7:13:05 AM GMT+00:00, Felipe Balbi <balbi@kernel.org> wrote:
>"Felipe F. Tonello" <eu@felipetonello.com> writes:
>> [ text/plain ]
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>
>no commit log == no commit

Got it. 

>
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c
>b/drivers/usb/gadget/function/f_midi.c
>> index 9a9e6112e224..5c7f5c780fda 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -5,6 +5,9 @@
>>   * Developed for Thumtronics by Grey Innovation
>>   * Ben Williamson <ben.williamson@greyinnovation.com>
>>   *
>> + * Copyright (C) 2015,2016 ROLI Ltd.
>> + * Felipe F. Tonello <felipe.tonello@roli.com>
>
>Did you check with your company's lawyer that your changes are enough
>to
>justify a copyright ?

Yes. Specially if that state machine refractor gets approved. TBH I can't see it won't. 

Thanks, 

Felipe 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1351346 — Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright

[Multipart message — attachments visible in raw view] — view raw

Hi,

Felipe Ferreri Tonello <eu@felipetonello.com> writes:
> [ text/plain ]
> Hi Balbi, 
>
> On March 4, 2016 7:13:05 AM GMT+00:00, Felipe Balbi <balbi@kernel.org> wrote:
>>"Felipe F. Tonello" <eu@felipetonello.com> writes:
>>> [ text/plain ]
>>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>>
>>no commit log == no commit
>
> Got it. 
>
>>
>>> ---
>>>  drivers/usb/gadget/function/f_midi.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/function/f_midi.c
>>b/drivers/usb/gadget/function/f_midi.c
>>> index 9a9e6112e224..5c7f5c780fda 100644
>>> --- a/drivers/usb/gadget/function/f_midi.c
>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>> @@ -5,6 +5,9 @@
>>>   * Developed for Thumtronics by Grey Innovation
>>>   * Ben Williamson <ben.williamson@greyinnovation.com>
>>>   *
>>> + * Copyright (C) 2015,2016 ROLI Ltd.
>>> + * Felipe F. Tonello <felipe.tonello@roli.com>
>>
>>Did you check with your company's lawyer that your changes are enough
>>to
>>justify a copyright ?
>
> Yes. Specially if that state machine refractor gets approved. TBH I
> can't see it won't.

okay, so did that same lawyer tell you to change the driver's license ?

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1351425 — Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright

[Multipart message — attachments visible in raw view] — view raw

Hi Balbi,

On 07/03/16 07:36, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello <eu@felipetonello.com> writes:
>> [ text/plain ]
>> Hi Balbi, 
>>
>> On March 4, 2016 7:13:05 AM GMT+00:00, Felipe Balbi <balbi@kernel.org> wrote:
>>> "Felipe F. Tonello" <eu@felipetonello.com> writes:
>>>> [ text/plain ]
>>>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>>>
>>> no commit log == no commit
>>
>> Got it. 
>>
>>>
>>>> ---
>>>>  drivers/usb/gadget/function/f_midi.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_midi.c
>>> b/drivers/usb/gadget/function/f_midi.c
>>>> index 9a9e6112e224..5c7f5c780fda 100644
>>>> --- a/drivers/usb/gadget/function/f_midi.c
>>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>>> @@ -5,6 +5,9 @@
>>>>   * Developed for Thumtronics by Grey Innovation
>>>>   * Ben Williamson <ben.williamson@greyinnovation.com>
>>>>   *
>>>> + * Copyright (C) 2015,2016 ROLI Ltd.
>>>> + * Felipe F. Tonello <felipe.tonello@roli.com>
>>>
>>> Did you check with your company's lawyer that your changes are enough
>>> to
>>> justify a copyright ?
>>
>> Yes. Specially if that state machine refractor gets approved. TBH I
>> can't see it won't.
> 
> okay, so did that same lawyer tell you to change the driver's license ?
> 

No. That was my bad call. TBH I really don't care about this copyright.
You can just ignore this patch and patch 4.

Thanks

Felipe

[toc] | [prev] | [next] | [standalone]

#1348426 — [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 drivers/usb/gadget/function/f_midi.c | 77 +++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 8475e3dc82d4..9a9e6112e224 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1,5 +1,5 @@
 /*
- * f_midi.c -- USB MIDI class function driver
+ * f_midi.c -- USB-MIDI class function driver
  *
  * Copyright (C) 2006 Thumtronics Pty Ltd.
  * Developed for Thumtronics by Grey Innovation
@@ -16,7 +16,7 @@
  *   Copyright (C) 2006 Thumtronics Pty Ltd.
  *   Ben Williamson <ben.williamson@greyinnovation.com>
  *
- * Licensed under the GPL-2 or later.
+ * Licensed under the GPLv2.
  */
 
 #include <linux/kernel.h>
@@ -41,8 +41,8 @@
 MODULE_AUTHOR("Ben Williamson");
 MODULE_LICENSE("GPL v2");
 
-static const char f_midi_shortname[] = "f_midi";
-static const char f_midi_longname[] = "MIDI Gadget";
+static const char f_midi_shortname[] =	"f_midi";
+static const char f_midi_longname[] =	"MIDI Gadget";
 
 /*
  * We can only handle 16 cables on one single endpoint, as cable numbers are
@@ -78,28 +78,31 @@ struct gmidi_in_port {
 };
 
 struct f_midi {
-	struct usb_function	func;
-	struct usb_gadget	*gadget;
-	struct usb_ep		*in_ep, *out_ep;
-	struct snd_card		*card;
-	struct snd_rawmidi	*rmidi;
-	u8			ms_id;
-
-	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
-
-	unsigned long		out_triggered;
-	struct tasklet_struct	tasklet;
+	struct usb_function func;
+	struct usb_gadget *gadget;
+	struct usb_ep *in_ep, *out_ep;
+	u8 ms_id;
+	unsigned long out_triggered;
 	unsigned int in_ports;
 	unsigned int out_ports;
-	int index;
-	char *id;
-	unsigned int buflen, qlen;
+	unsigned int buflen;
+	unsigned int qlen;
+	unsigned int len;
+
 	/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
 	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
 	spinlock_t transmit_lock;
+
+	/* ALSA stuff */
+	struct snd_card *card;
+	struct snd_rawmidi *rmidi;
+	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
+	struct tasklet_struct tasklet;
 	unsigned int in_last_port;
+	int index;
+	char *id;
 
-	struct gmidi_in_port	in_ports_array[/* in_ports */];
+	struct gmidi_in_port in_ports_array[/* in_ports */];
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -191,7 +194,7 @@ static struct usb_ms_endpoint_descriptor_16 ms_in_desc = {
 
 /* string IDs are assigned dynamically */
 
-#define STRING_FUNC_IDX			0
+#define STRING_FUNC_IDX 0
 
 static struct usb_string midi_string_defs[] = {
 	[STRING_FUNC_IDX].s = "MIDI function",
@@ -199,7 +202,7 @@ static struct usb_string midi_string_defs[] = {
 };
 
 static struct usb_gadget_strings midi_stringtab = {
-	.language	= 0x0409,	/* en-us */
+	.language	= 0x0409, /* en-us */
 	.strings	= midi_string_defs,
 };
 
@@ -409,7 +412,7 @@ static int f_midi_snd_free(struct snd_device *device)
 }
 
 /*
- * Converts MIDI commands to USB MIDI packets.
+ * Converts MIDI commands to USB-MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 				 struct gmidi_in_port *port, uint8_t b)
@@ -956,15 +959,15 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
 		in_emb->iJack			= 0;
 		midi_function[i++] = (struct usb_descriptor_header *) in_emb;
 
-		out_ext->bLength =		USB_DT_MIDI_OUT_SIZE(1);
-		out_ext->bDescriptorType =	USB_DT_CS_INTERFACE;
-		out_ext->bDescriptorSubtype =	USB_MS_MIDI_OUT_JACK;
-		out_ext->bJackType =		USB_MS_EXTERNAL;
-		out_ext->bJackID =		jack++;
-		out_ext->bNrInputPins =		1;
-		out_ext->iJack =		0;
-		out_ext->pins[0].baSourceID =	in_emb->bJackID;
-		out_ext->pins[0].baSourcePin =	1;
+		out_ext->bLength		= USB_DT_MIDI_OUT_SIZE(1);
+		out_ext->bDescriptorType	= USB_DT_CS_INTERFACE;
+		out_ext->bDescriptorSubtype	= USB_MS_MIDI_OUT_JACK;
+		out_ext->bJackType		= USB_MS_EXTERNAL;
+		out_ext->bJackID		= jack++;
+		out_ext->bNrInputPins		= 1;
+		out_ext->iJack			= 0;
+		out_ext->pins[0].baSourceID	= in_emb->bJackID;
+		out_ext->pins[0].baSourcePin	= 1;
 		midi_function[i++] = (struct usb_descriptor_header *) out_ext;
 
 		/* link it to the endpoint */
@@ -1251,12 +1254,12 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
 		status = -ENOMEM;
 		goto setup_fail;
 	}
-	midi->in_ports = opts->in_ports;
-	midi->out_ports = opts->out_ports;
-	midi->index = opts->index;
-	midi->buflen = opts->buflen;
-	midi->qlen = opts->qlen;
-	midi->in_last_port = 0;
+	midi->in_ports		= opts->in_ports;
+	midi->out_ports		= opts->out_ports;
+	midi->index		= opts->index;
+	midi->buflen		= opts->buflen;
+	midi->qlen		= opts->qlen;
+	midi->in_last_port	= 0;
 
 	status = kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL);
 	if (status)
-- 
2.7.2

[toc] | [prev] | [next] | [standalone]

#1349906 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Multipart message — attachments visible in raw view] — view raw

"Felipe F. Tonello" <eu@felipetonello.com> writes:
> [ text/plain ]
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>

no commit log == no commit

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1350531 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

On Wed, Mar 02 2016, Felipe F. Tonello wrote:
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
>  drivers/usb/gadget/function/f_midi.c | 77 +++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 8475e3dc82d4..9a9e6112e224 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1,5 +1,5 @@
>  /*
> - * f_midi.c -- USB MIDI class function driver
> + * f_midi.c -- USB-MIDI class function driver
>   *
>   * Copyright (C) 2006 Thumtronics Pty Ltd.
>   * Developed for Thumtronics by Grey Innovation
> @@ -16,7 +16,7 @@
>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>   *
> - * Licensed under the GPL-2 or later.
> + * Licensed under the GPLv2.

Any particular reason to do that?

>   */
>  
>  #include <linux/kernel.h>
> @@ -41,8 +41,8 @@
>  MODULE_AUTHOR("Ben Williamson");
>  MODULE_LICENSE("GPL v2");
>  
> -static const char f_midi_shortname[] = "f_midi";
> -static const char f_midi_longname[] = "MIDI Gadget";
> +static const char f_midi_shortname[] =	"f_midi";
> +static const char f_midi_longname[] =	"MIDI Gadget";
>  
>  /*
>   * We can only handle 16 cables on one single endpoint, as cable numbers are
> @@ -78,28 +78,31 @@ struct gmidi_in_port {
>  };
>  
>  struct f_midi {
> -	struct usb_function	func;
> -	struct usb_gadget	*gadget;
> -	struct usb_ep		*in_ep, *out_ep;
> -	struct snd_card		*card;
> -	struct snd_rawmidi	*rmidi;
> -	u8			ms_id;
> -
> -	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
> -
> -	unsigned long		out_triggered;
> -	struct tasklet_struct	tasklet;
> +	struct usb_function func;
> +	struct usb_gadget *gadget;
> +	struct usb_ep *in_ep, *out_ep;
> +	u8 ms_id;
> +	unsigned long out_triggered;
>  	unsigned int in_ports;
>  	unsigned int out_ports;
> -	int index;
> -	char *id;
> -	unsigned int buflen, qlen;
> +	unsigned int buflen;
> +	unsigned int qlen;
> +	unsigned int len;
> +
>  	/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
>  	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
>  	spinlock_t transmit_lock;
> +
> +	/* ALSA stuff */
> +	struct snd_card *card;
> +	struct snd_rawmidi *rmidi;
> +	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
> +	struct tasklet_struct tasklet;
>  	unsigned int in_last_port;
> +	int index;
> +	char *id;
>  
> -	struct gmidi_in_port	in_ports_array[/* in_ports */];
> +	struct gmidi_in_port in_ports_array[/* in_ports */];
>  };
>  
>  static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -191,7 +194,7 @@ static struct usb_ms_endpoint_descriptor_16 ms_in_desc = {
>  
>  /* string IDs are assigned dynamically */
>  
> -#define STRING_FUNC_IDX			0
> +#define STRING_FUNC_IDX 0
>  
>  static struct usb_string midi_string_defs[] = {
>  	[STRING_FUNC_IDX].s = "MIDI function",
> @@ -199,7 +202,7 @@ static struct usb_string midi_string_defs[] = {
>  };
>  
>  static struct usb_gadget_strings midi_stringtab = {
> -	.language	= 0x0409,	/* en-us */
> +	.language	= 0x0409, /* en-us */
>  	.strings	= midi_string_defs,
>  };
>  
> @@ -409,7 +412,7 @@ static int f_midi_snd_free(struct snd_device *device)
>  }
>  
>  /*
> - * Converts MIDI commands to USB MIDI packets.
> + * Converts MIDI commands to USB-MIDI packets.
>   */
>  static void f_midi_transmit_byte(struct usb_request *req,
>  				 struct gmidi_in_port *port, uint8_t b)
> @@ -956,15 +959,15 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
>  		in_emb->iJack			= 0;
>  		midi_function[i++] = (struct usb_descriptor_header *) in_emb;
>  
> -		out_ext->bLength =		USB_DT_MIDI_OUT_SIZE(1);
> -		out_ext->bDescriptorType =	USB_DT_CS_INTERFACE;
> -		out_ext->bDescriptorSubtype =	USB_MS_MIDI_OUT_JACK;
> -		out_ext->bJackType =		USB_MS_EXTERNAL;
> -		out_ext->bJackID =		jack++;
> -		out_ext->bNrInputPins =		1;
> -		out_ext->iJack =		0;
> -		out_ext->pins[0].baSourceID =	in_emb->bJackID;
> -		out_ext->pins[0].baSourcePin =	1;
> +		out_ext->bLength		= USB_DT_MIDI_OUT_SIZE(1);
> +		out_ext->bDescriptorType	= USB_DT_CS_INTERFACE;
> +		out_ext->bDescriptorSubtype	= USB_MS_MIDI_OUT_JACK;
> +		out_ext->bJackType		= USB_MS_EXTERNAL;
> +		out_ext->bJackID		= jack++;
> +		out_ext->bNrInputPins		= 1;
> +		out_ext->iJack			= 0;
> +		out_ext->pins[0].baSourceID	= in_emb->bJackID;
> +		out_ext->pins[0].baSourcePin	= 1;
>  		midi_function[i++] = (struct usb_descriptor_header *) out_ext;
>  
>  		/* link it to the endpoint */
> @@ -1251,12 +1254,12 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
>  		status = -ENOMEM;
>  		goto setup_fail;
>  	}
> -	midi->in_ports = opts->in_ports;
> -	midi->out_ports = opts->out_ports;
> -	midi->index = opts->index;
> -	midi->buflen = opts->buflen;
> -	midi->qlen = opts->qlen;
> -	midi->in_last_port = 0;
> +	midi->in_ports		= opts->in_ports;
> +	midi->out_ports		= opts->out_ports;
> +	midi->index		= opts->index;
> +	midi->buflen		= opts->buflen;
> +	midi->qlen		= opts->qlen;
> +	midi->in_last_port	= 0;

I don’t understand this patch.  You seem to be opposed to lining up
field names in a structure, but you are explicitly adding lining up to
assignment?  What is going on here?  IS this patch really improving
things?

>  
>  	status = kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL);
>  	if (status)
> -- 
> 2.7.2
>

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

[toc] | [prev] | [next] | [standalone]

#1350558 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

Hi Michal, 

On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 77
>+++++++++++++++++++-----------------
>>  1 file changed, 40 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c
>b/drivers/usb/gadget/function/f_midi.c
>> index 8475e3dc82d4..9a9e6112e224 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * f_midi.c -- USB MIDI class function driver
>> + * f_midi.c -- USB-MIDI class function driver
>>   *
>>   * Copyright (C) 2006 Thumtronics Pty Ltd.
>>   * Developed for Thumtronics by Grey Innovation
>> @@ -16,7 +16,7 @@
>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>   *
>> - * Licensed under the GPL-2 or later.
>> + * Licensed under the GPLv2.
>
>Any particular reason to do that?

Because the kernel is v2 only and not later. 

>
>>   */
>>  
>>  #include <linux/kernel.h>
>> @@ -41,8 +41,8 @@
>>  MODULE_AUTHOR("Ben Williamson");
>>  MODULE_LICENSE("GPL v2");
>>  
>> -static const char f_midi_shortname[] = "f_midi";
>> -static const char f_midi_longname[] = "MIDI Gadget";
>> +static const char f_midi_shortname[] =	"f_midi";
>> +static const char f_midi_longname[] =	"MIDI Gadget";
>>  
>>  /*
>>   * We can only handle 16 cables on one single endpoint, as cable
>numbers are
>> @@ -78,28 +78,31 @@ struct gmidi_in_port {
>>  };
>>  
>>  struct f_midi {
>> -	struct usb_function	func;
>> -	struct usb_gadget	*gadget;
>> -	struct usb_ep		*in_ep, *out_ep;
>> -	struct snd_card		*card;
>> -	struct snd_rawmidi	*rmidi;
>> -	u8			ms_id;
>> -
>> -	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
>> -
>> -	unsigned long		out_triggered;
>> -	struct tasklet_struct	tasklet;
>> +	struct usb_function func;
>> +	struct usb_gadget *gadget;
>> +	struct usb_ep *in_ep, *out_ep;
>> +	u8 ms_id;
>> +	unsigned long out_triggered;
>>  	unsigned int in_ports;
>>  	unsigned int out_ports;
>> -	int index;
>> -	char *id;
>> -	unsigned int buflen, qlen;
>> +	unsigned int buflen;
>> +	unsigned int qlen;
>> +	unsigned int len;
>> +
>>  	/* This fifo is used as a buffer ring for pre-allocated IN
>usb_requests */
>>  	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
>>  	spinlock_t transmit_lock;
>> +
>> +	/* ALSA stuff */
>> +	struct snd_card *card;
>> +	struct snd_rawmidi *rmidi;
>> +	struct snd_rawmidi_substream *out_substream[MAX_PORTS];
>> +	struct tasklet_struct tasklet;
>>  	unsigned int in_last_port;
>> +	int index;
>> +	char *id;
>>  
>> -	struct gmidi_in_port	in_ports_array[/* in_ports */];
>> +	struct gmidi_in_port in_ports_array[/* in_ports */];
>>  };
>>  
>>  static inline struct f_midi *func_to_midi(struct usb_function *f)
>> @@ -191,7 +194,7 @@ static struct usb_ms_endpoint_descriptor_16
>ms_in_desc = {
>>  
>>  /* string IDs are assigned dynamically */
>>  
>> -#define STRING_FUNC_IDX			0
>> +#define STRING_FUNC_IDX 0
>>  
>>  static struct usb_string midi_string_defs[] = {
>>  	[STRING_FUNC_IDX].s = "MIDI function",
>> @@ -199,7 +202,7 @@ static struct usb_string midi_string_defs[] = {
>>  };
>>  
>>  static struct usb_gadget_strings midi_stringtab = {
>> -	.language	= 0x0409,	/* en-us */
>> +	.language	= 0x0409, /* en-us */
>>  	.strings	= midi_string_defs,
>>  };
>>  
>> @@ -409,7 +412,7 @@ static int f_midi_snd_free(struct snd_device
>*device)
>>  }
>>  
>>  /*
>> - * Converts MIDI commands to USB MIDI packets.
>> + * Converts MIDI commands to USB-MIDI packets.
>>   */
>>  static void f_midi_transmit_byte(struct usb_request *req,
>>  				 struct gmidi_in_port *port, uint8_t b)
>> @@ -956,15 +959,15 @@ static int f_midi_bind(struct usb_configuration
>*c, struct usb_function *f)
>>  		in_emb->iJack			= 0;
>>  		midi_function[i++] = (struct usb_descriptor_header *) in_emb;
>>  
>> -		out_ext->bLength =		USB_DT_MIDI_OUT_SIZE(1);
>> -		out_ext->bDescriptorType =	USB_DT_CS_INTERFACE;
>> -		out_ext->bDescriptorSubtype =	USB_MS_MIDI_OUT_JACK;
>> -		out_ext->bJackType =		USB_MS_EXTERNAL;
>> -		out_ext->bJackID =		jack++;
>> -		out_ext->bNrInputPins =		1;
>> -		out_ext->iJack =		0;
>> -		out_ext->pins[0].baSourceID =	in_emb->bJackID;
>> -		out_ext->pins[0].baSourcePin =	1;
>> +		out_ext->bLength		= USB_DT_MIDI_OUT_SIZE(1);
>> +		out_ext->bDescriptorType	= USB_DT_CS_INTERFACE;
>> +		out_ext->bDescriptorSubtype	= USB_MS_MIDI_OUT_JACK;
>> +		out_ext->bJackType		= USB_MS_EXTERNAL;
>> +		out_ext->bJackID		= jack++;
>> +		out_ext->bNrInputPins		= 1;
>> +		out_ext->iJack			= 0;
>> +		out_ext->pins[0].baSourceID	= in_emb->bJackID;
>> +		out_ext->pins[0].baSourcePin	= 1;
>>  		midi_function[i++] = (struct usb_descriptor_header *) out_ext;
>>  
>>  		/* link it to the endpoint */
>> @@ -1251,12 +1254,12 @@ static struct usb_function
>*f_midi_alloc(struct usb_function_instance *fi)
>>  		status = -ENOMEM;
>>  		goto setup_fail;
>>  	}
>> -	midi->in_ports = opts->in_ports;
>> -	midi->out_ports = opts->out_ports;
>> -	midi->index = opts->index;
>> -	midi->buflen = opts->buflen;
>> -	midi->qlen = opts->qlen;
>> -	midi->in_last_port = 0;
>> +	midi->in_ports		= opts->in_ports;
>> +	midi->out_ports		= opts->out_ports;
>> +	midi->index		= opts->index;
>> +	midi->buflen		= opts->buflen;
>> +	midi->qlen		= opts->qlen;
>> +	midi->in_last_port	= 0;
>
>I don’t understand this patch.  You seem to be opposed to lining up
>field names in a structure, but you are explicitly adding lining up to
>assignment?  What is going on here?  IS this patch really improving
>things?

I just tried to make this driver more consistent with the coding style used across the kernel. That's it. 

>
>>  
>>  	status = kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL);
>>  	if (status)
>> -- 
>> 2.7.2
>>

Felipe 
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1350930 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>> @@ -16,7 +16,7 @@
>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>>   *
>>> - * Licensed under the GPL-2 or later.
>>> + * Licensed under the GPLv2.

> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>> Any particular reason to do that?

On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
> Because the kernel is v2 only and not later. 

Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
copyright noticed clear unless you explicitly want your contribution be
GPLv2 only which brings the whole file GPLv2 only.

> I just tried to make this driver more consistent with the coding style
> used across the kernel. That's it.

Column alignment of field names or RHS of assignment operators is quite
inconsistent already within drivers/usb/gadget/ which is why I’m
concerned whether this is really helping.

Anyway, I actually don’t care much, just adding my two rappen.

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

[toc] | [prev] | [next] | [standalone]

#1350962 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

On Sat, Mar 05, 2016 at 11:28:45AM -0500, Michal Nazarewicz wrote:
> >> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
> >>> @@ -16,7 +16,7 @@
> >>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
> >>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
> >>>   *
> >>> - * Licensed under the GPL-2 or later.
> >>> + * Licensed under the GPLv2.
> 
> > On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
> >> Any particular reason to do that?
> 
> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
> > Because the kernel is v2 only and not later. 
> 
> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
> copyright noticed clear unless you explicitly want your contribution be
> GPLv2 only which brings the whole file GPLv2 only.

But you can't change the license of someone else's code, which is what
is happening here.  Felipe T, you can't do that at all unless you want
to get into big trouble, please consult a lawyer for all of the gory
details.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1351012 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

Hi Greg, 

On March 5, 2016 7:39:13 PM GMT+00:00, Greg KH <greg@kroah.com> wrote:
>On Sat, Mar 05, 2016 at 11:28:45AM -0500, Michal Nazarewicz wrote:
>> >> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>> >>> @@ -16,7 +16,7 @@
>> >>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>> >>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>> >>>   *
>> >>> - * Licensed under the GPL-2 or later.
>> >>> + * Licensed under the GPLv2.
>> 
>> > On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
><mina86@mina86.com> wrote:
>> >> Any particular reason to do that?
>> 
>> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
>> > Because the kernel is v2 only and not later. 
>> 
>> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean
>that
>> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>> copyright noticed clear unless you explicitly want your contribution
>be
>> GPLv2 only which brings the whole file GPLv2 only.
>
>But you can't change the license of someone else's code, which is what
>is happening here.  Felipe T, you can't do that at all unless you want
>to get into big trouble, please consult a lawyer for all of the gory
>details.

Thanks for letting me know. TBH, I had no idea about it. 

Felipe 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1351035 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

On Sat, Mar 05, 2016 at 11:53:35PM +0000, Felipe Ferreri Tonello wrote:
> Hi Greg, 
> 
> On March 5, 2016 7:39:13 PM GMT+00:00, Greg KH <greg@kroah.com> wrote:
> >On Sat, Mar 05, 2016 at 11:28:45AM -0500, Michal Nazarewicz wrote:
> >> >> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
> >> >>> @@ -16,7 +16,7 @@
> >> >>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
> >> >>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
> >> >>>   *
> >> >>> - * Licensed under the GPL-2 or later.
> >> >>> + * Licensed under the GPLv2.
> >> 
> >> > On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
> ><mina86@mina86.com> wrote:
> >> >> Any particular reason to do that?
> >> 
> >> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
> >> > Because the kernel is v2 only and not later. 
> >> 
> >> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean
> >that
> >> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
> >> copyright noticed clear unless you explicitly want your contribution
> >be
> >> GPLv2 only which brings the whole file GPLv2 only.
> >
> >But you can't change the license of someone else's code, which is what
> >is happening here.  Felipe T, you can't do that at all unless you want
> >to get into big trouble, please consult a lawyer for all of the gory
> >details.
> 
> Thanks for letting me know. TBH, I had no idea about it. 

Never change a copyright or a license if you don't know exactly what you
are doing, or why you are doing it, and have consulted with a lawyer
beforehand.  The issues here are real, don't take them lightly.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1351009 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

Hi Michal, 

On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>>> @@ -16,7 +16,7 @@
>>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>>>   *
>>>> - * Licensed under the GPL-2 or later.
>>>> + * Licensed under the GPLv2.
>
>> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
><mina86@mina86.com> wrote:
>>> Any particular reason to do that?
>
>On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
>> Because the kernel is v2 only and not later. 
>
>Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
>parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>copyright noticed clear unless you explicitly want your contribution be
>GPLv2 only which brings the whole file GPLv2 only.
>
>> I just tried to make this driver more consistent with the coding
>style
>> used across the kernel. That's it.
>
>Column alignment of field names or RHS of assignment operators is quite
>inconsistent already within drivers/usb/gadget/ which is why I’m
>concerned whether this is really helping.
>
>Anyway, I actually don’t care much, just adding my two rappen.

Right, I am ok with Balbi completely ignoring this patch. But I prefer to have at least this driver consistent than nothing. Of course I'll remove the license change I made. 

Felipe 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1351345 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Multipart message — attachments visible in raw view] — view raw

Hi,

Felipe Ferreri Tonello <eu@felipetonello.com> writes:
> [ text/plain ]
> Hi Michal, 
>
> On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>>>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>>>> @@ -16,7 +16,7 @@
>>>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>>>>   *
>>>>> - * Licensed under the GPL-2 or later.
>>>>> + * Licensed under the GPLv2.
>>
>>> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
>><mina86@mina86.com> wrote:
>>>> Any particular reason to do that?
>>
>>On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
>>> Because the kernel is v2 only and not later. 
>>
>>Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
>>parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>>copyright noticed clear unless you explicitly want your contribution be
>>GPLv2 only which brings the whole file GPLv2 only.
>>
>>> I just tried to make this driver more consistent with the coding
>>style
>>> used across the kernel. That's it.
>>
>>Column alignment of field names or RHS of assignment operators is quite
>>inconsistent already within drivers/usb/gadget/ which is why I’m
>>concerned whether this is really helping.
>>
>>Anyway, I actually don’t care much, just adding my two rappen.
>
> Right, I am ok with Balbi completely ignoring this patch. But I prefer
> to have at least this driver consistent than nothing. Of course I'll
> remove the license change I made.

consistent in what way ?

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1351430 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Multipart message — attachments visible in raw view] — view raw

Hi Balbi,

On 07/03/16 07:35, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello <eu@felipetonello.com> writes:
>> [ text/plain ]
>> Hi Michal, 
>>
>> On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>>>>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>>>>> @@ -16,7 +16,7 @@
>>>>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>>>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>>>>>   *
>>>>>> - * Licensed under the GPL-2 or later.
>>>>>> + * Licensed under the GPLv2.
>>>
>>>> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
>>> <mina86@mina86.com> wrote:
>>>>> Any particular reason to do that?
>>>
>>> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
>>>> Because the kernel is v2 only and not later. 
>>>
>>> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
>>> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>>> copyright noticed clear unless you explicitly want your contribution be
>>> GPLv2 only which brings the whole file GPLv2 only.
>>>
>>>> I just tried to make this driver more consistent with the coding
>>> style
>>>> used across the kernel. That's it.
>>>
>>> Column alignment of field names or RHS of assignment operators is quite
>>> inconsistent already within drivers/usb/gadget/ which is why I’m
>>> concerned whether this is really helping.
>>>
>>> Anyway, I actually don’t care much, just adding my two rappen.
>>
>> Right, I am ok with Balbi completely ignoring this patch. But I prefer
>> to have at least this driver consistent than nothing. Of course I'll
>> remove the license change I made.
> 
> consistent in what way ?

Source-code.

The goal of this patch is to update this driver coding style to promote
consistency, readability, and maintainability based on the Linux coding
style.

If this patch does not achieving that or if that is not necessary, than
just ignore this patch.

Thanks,
Felipe

[toc] | [prev] | [next] | [standalone]

#1352690 — Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Multipart message — attachments visible in raw view] — view raw

Hi,

Felipe Ferreri Tonello <eu@felipetonello.com> writes:
>>> On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz <mina86@mina86.com> wrote:
>>>>>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>>>>>> @@ -16,7 +16,7 @@
>>>>>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>>>>>   *   Ben Williamson <ben.williamson@greyinnovation.com>
>>>>>>>   *
>>>>>>> - * Licensed under the GPL-2 or later.
>>>>>>> + * Licensed under the GPLv2.
>>>>
>>>>> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
>>>> <mina86@mina86.com> wrote:
>>>>>> Any particular reason to do that?
>>>>
>>>> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
>>>>> Because the kernel is v2 only and not later. 
>>>>
>>>> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
>>>> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>>>> copyright noticed clear unless you explicitly want your contribution be
>>>> GPLv2 only which brings the whole file GPLv2 only.
>>>>
>>>>> I just tried to make this driver more consistent with the coding
>>>> style
>>>>> used across the kernel. That's it.
>>>>
>>>> Column alignment of field names or RHS of assignment operators is quite
>>>> inconsistent already within drivers/usb/gadget/ which is why I’m
>>>> concerned whether this is really helping.
>>>>
>>>> Anyway, I actually don’t care much, just adding my two rappen.
>>>
>>> Right, I am ok with Balbi completely ignoring this patch. But I prefer
>>> to have at least this driver consistent than nothing. Of course I'll
>>> remove the license change I made.
>> 
>> consistent in what way ?
>
> Source-code.
>
> The goal of this patch is to update this driver coding style to promote
> consistency, readability, and maintainability based on the Linux coding
> style.
>
> If this patch does not achieving that or if that is not necessary, than
> just ignore this patch.

yeah, I don't think that's what you're doing here.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1348431 — [PATCH 1/5] usb: gadget: f_midi: refactor state machine

This refactor results in a cleaner state machine code and as a result fixed a
bug when packaging a USB-MIDI packet right after a non-conformant MIDI byte stream.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 drivers/usb/gadget/function/f_midi.c | 204 ++++++++++++++++++++++-------------
 1 file changed, 129 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 84c0ee5ebd1e..3cdb0741f3f8 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -50,6 +50,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
  */
 #define MAX_PORTS 16
 
+/* MIDI message states */
+enum {
+	STATE_INITIAL = 0,	/* pseudo state */
+	STATE_1PARAM,
+	STATE_2PARAM_1,
+	STATE_2PARAM_2,
+	STATE_SYSEX_0,
+	STATE_SYSEX_1,
+	STATE_SYSEX_2,
+	STATE_REAL_TIME,
+	STATE_FINISHED,		/* pseudo state */
+};
+
 /*
  * This is a gadget, and the IN/OUT naming is from the host's perspective.
  * USB -> OUT endpoint -> rawmidi
@@ -60,13 +73,6 @@ struct gmidi_in_port {
 	int active;
 	uint8_t cable;
 	uint8_t state;
-#define STATE_UNKNOWN	0
-#define STATE_1PARAM	1
-#define STATE_2PARAM_1	2
-#define STATE_2PARAM_2	3
-#define STATE_SYSEX_0	4
-#define STATE_SYSEX_1	5
-#define STATE_SYSEX_2	6
 	uint8_t data[2];
 };
 
@@ -400,118 +406,166 @@ static int f_midi_snd_free(struct snd_device *device)
 	return 0;
 }
 
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
-					uint8_t p1, uint8_t p2, uint8_t p3)
-{
-	unsigned length = req->length;
-	u8 *buf = (u8 *)req->buf + length;
-
-	buf[0] = p0;
-	buf[1] = p1;
-	buf[2] = p2;
-	buf[3] = p3;
-	req->length = length + 4;
-}
-
 /*
  * Converts MIDI commands to USB MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 				 struct gmidi_in_port *port, uint8_t b)
 {
-	uint8_t p0 = port->cable << 4;
+	uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+	uint8_t next_state = STATE_INITIAL;
+
+	switch (b) {
+	case 0xf8 ... 0xff:
+		/* System Real-Time Messages */
+		p[0] |= 0x0f;
+		p[1] = b;
+		next_state = port->state;
+		port->state = STATE_REAL_TIME;
+		break;
+
+	case 0xf7:
+		/* End of SysEx */
+		switch (port->state) {
+		case STATE_SYSEX_0:
+			p[0] |= 0x05;
+			p[1] = 0xf7;
+			next_state = STATE_FINISHED;
+			break;
+		case STATE_SYSEX_1:
+			p[0] |= 0x06;
+			p[1] = port->data[0];
+			p[2] = 0xf7;
+			next_state = STATE_FINISHED;
+			break;
+		case STATE_SYSEX_2:
+			p[0] |= 0x07;
+			p[1] = port->data[0];
+			p[2] = port->data[1];
+			p[3] = 0xf7;
+			next_state = STATE_FINISHED;
+			break;
+		default:
+			/* Ignore byte */
+			next_state = port->state;
+			port->state = STATE_INITIAL;
+		}
+		break;
 
-	if (b >= 0xf8) {
-		f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
-	} else if (b >= 0xf0) {
+	case 0xf0 ... 0xf6:
+		/* System Common Messages */
+		port->data[0] = port->data[1] = 0;
+		port->state = STATE_INITIAL;
 		switch (b) {
 		case 0xf0:
 			port->data[0] = b;
-			port->state = STATE_SYSEX_1;
+			port->data[1] = 0;
+			next_state = STATE_SYSEX_1;
 			break;
 		case 0xf1:
 		case 0xf3:
 			port->data[0] = b;
-			port->state = STATE_1PARAM;
+			next_state = STATE_1PARAM;
 			break;
 		case 0xf2:
 			port->data[0] = b;
-			port->state = STATE_2PARAM_1;
+			next_state = STATE_2PARAM_1;
 			break;
 		case 0xf4:
 		case 0xf5:
-			port->state = STATE_UNKNOWN;
+			next_state = STATE_INITIAL;
 			break;
 		case 0xf6:
-			f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
-			port->state = STATE_UNKNOWN;
-			break;
-		case 0xf7:
-			switch (port->state) {
-			case STATE_SYSEX_0:
-				f_midi_transmit_packet(req,
-					p0 | 0x05, 0xf7, 0, 0);
-				break;
-			case STATE_SYSEX_1:
-				f_midi_transmit_packet(req,
-					p0 | 0x06, port->data[0], 0xf7, 0);
-				break;
-			case STATE_SYSEX_2:
-				f_midi_transmit_packet(req,
-					p0 | 0x07, port->data[0],
-					port->data[1], 0xf7);
-				break;
-			}
-			port->state = STATE_UNKNOWN;
+			p[0] |= 0x05;
+			p[1] = 0xf6;
+			next_state = STATE_FINISHED;
 			break;
 		}
-	} else if (b >= 0x80) {
+		break;
+
+	case 0x80 ... 0xef:
+		/*
+		 * Channel Voice Messages, Channel Mode Messages
+		 * and Control Change Messages.
+		 */
 		port->data[0] = b;
+		port->data[1] = 0;
+		port->state = STATE_INITIAL;
 		if (b >= 0xc0 && b <= 0xdf)
-			port->state = STATE_1PARAM;
+			next_state = STATE_1PARAM;
 		else
-			port->state = STATE_2PARAM_1;
-	} else { /* b < 0x80 */
+			next_state = STATE_2PARAM_1;
+		break;
+
+	case 0x00 ... 0x7f:
+		/* Message parameters */
 		switch (port->state) {
 		case STATE_1PARAM:
-			if (port->data[0] < 0xf0) {
-				p0 |= port->data[0] >> 4;
-			} else {
-				p0 |= 0x02;
-				port->state = STATE_UNKNOWN;
-			}
-			f_midi_transmit_packet(req, p0, port->data[0], b, 0);
+			if (port->data[0] < 0xf0)
+				p[0] |= port->data[0] >> 4;
+			else
+				p[0] |= 0x02;
+
+			p[1] = port->data[0];
+			p[2] = b;
+			/* This is to allow Running State Messages */
+			next_state = STATE_1PARAM;
 			break;
 		case STATE_2PARAM_1:
 			port->data[1] = b;
-			port->state = STATE_2PARAM_2;
+			next_state = STATE_2PARAM_2;
 			break;
 		case STATE_2PARAM_2:
-			if (port->data[0] < 0xf0) {
-				p0 |= port->data[0] >> 4;
-				port->state = STATE_2PARAM_1;
-			} else {
-				p0 |= 0x03;
-				port->state = STATE_UNKNOWN;
-			}
-			f_midi_transmit_packet(req,
-				p0, port->data[0], port->data[1], b);
+			if (port->data[0] < 0xf0)
+				p[0] |= port->data[0] >> 4;
+			else
+				p[0] |= 0x03;
+
+			p[1] = port->data[0];
+			p[2] = port->data[1];
+			p[3] = b;
+			/* This is to allow Running State Messages */
+			next_state = STATE_2PARAM_1;
 			break;
 		case STATE_SYSEX_0:
 			port->data[0] = b;
-			port->state = STATE_SYSEX_1;
+			next_state = STATE_SYSEX_1;
 			break;
 		case STATE_SYSEX_1:
 			port->data[1] = b;
-			port->state = STATE_SYSEX_2;
+			next_state = STATE_SYSEX_2;
 			break;
 		case STATE_SYSEX_2:
-			f_midi_transmit_packet(req,
-				p0 | 0x04, port->data[0], port->data[1], b);
-			port->state = STATE_SYSEX_0;
+			p[0] |= 0x04;
+			p[1] = port->data[0];
+			p[2] = port->data[1];
+			p[3] = b;
+			next_state = STATE_SYSEX_0;
 			break;
 		}
+		break;
+	}
+
+	/* States where we have to write into the USB request */
+	if (next_state == STATE_FINISHED ||
+	    port->state == STATE_SYSEX_2 ||
+	    port->state == STATE_1PARAM ||
+	    port->state == STATE_2PARAM_2 ||
+	    port->state == STATE_REAL_TIME) {
+
+		unsigned length = req->length;
+		u8 *buf = (u8 *)req->buf + length;
+
+		memcpy(buf, p, sizeof(p));
+		req->length = length + sizeof(p);
+
+		if (next_state == STATE_FINISHED) {
+			next_state = STATE_INITIAL;
+			port->data[0] = port->data[1] = 0;
+		}
 	}
+
+	port->state = next_state;
 }
 
 static void f_midi_drop_out_substreams(struct f_midi *midi)
@@ -631,7 +685,7 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
 	VDBG(midi, "%s()\n", __func__);
 	port = midi->in_ports_array + substream->number;
 	port->substream = substream;
-	port->state = STATE_UNKNOWN;
+	port->state = STATE_INITIAL;
 	return 0;
 }
 
-- 
2.7.2

[toc] | [prev] | [next] | [standalone]

#1348477 — Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine

Felipe F. Tonello wrote:
> This refactor results in a cleaner state machine code

It increases the number of states, and now juggles two state variables.
I cannot agree to it being cleaner.

> and as a result fixed a bug when packaging a USB-MIDI packet right after
> a non-conformant MIDI byte stream.

I have been unable to determine where exactly the new code behaves
differently.  Can you show an example?


Regards,
Clemens

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web