> On 04/04/16 13:11, Michal Nazarewicz wrote:
>> Or maybe even this which gets away with gotos all together:
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c
>> b/drivers/usb/gadget/function/f_midi.c
>> index 56e2dde..91cae60 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -598,27 +598,20 @@ done:
>> static void f_midi_transmit(struct f_midi *midi)
>> {
>> struct usb_ep *ep = midi->in_ep;
>> - int ret;
>> unsigned long flags;
>> + int ret = -1;
>>
>> /* We only care about USB requests if IN endpoint is enabled */
>> - if (!ep || !ep->enabled)
>> - goto drop_out;
>> -
>> - spin_lock_irqsave(&midi->transmit_lock, flags);
>> -
>> - do {
>> - ret = f_midi_do_transmit(midi, ep);
>> - if (ret < 0)
>> - goto drop_out;
>> - } while (ret);
>> -
>> - spin_unlock_irqrestore(&midi->transmit_lock, flags);
>> -
>> - return;
>> + if (ep && ep->enabled) {
>> + spin_lock_irqsave(&midi->transmit_lock, flags);
>> + while ((ret = f_midi_do_transmit(midi, ep)) > 0) {
>> + /* nop */
>> + }
>> + spin_unlock_irqrestore(&midi->transmit_lock, flags);
>> + }
>>
>> -drop_out:
>> - f_midi_drop_out_substreams(midi);
>> + if (ret < 0)
>> + f_midi_drop_out_substreams(midi);
>> }
>>
>> static void f_midi_in_tasklet(unsigned long data)
On Mon, Apr 04 2016, Felipe Ferreri Tonello wrote:
> I am fine with these options, probably the second, but I don't think
> they are any clearer than before, so I don't see any real benefits with
> the changes.
The spin_lock_irqsave is paired with exactly one spin_unlock_irqrestore
which is cleaner in my book. I don’t have strong feelings about this
though.
> In fact, I think f_midi_do_transmit should be documented, since there
> are 3 possible return condition:
> <0 breaks the loop and drop out substreams
> 0 breaks the loop
> >0 continues the loop
That’s of course separate issue.
--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
