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

Felipe Ferreri Tonello
2016-03-04T20:17:19Z

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.