Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-27 Thread Felipe Ferreri Tonello 
Hi Robert,

On 27/10/15 06:47, Robert Baldyga wrote:
> On 10/26/2015 11:53 PM, Felipe Tonello wrote:
>> Hi Robert,
>>
>> On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
>>  wrote:
>>> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
 This avoids duplication of USB requests for OUT endpoint and
 re-enabling endpoints.

 Signed-off-by: Felipe F. Tonello 
 ---
  drivers/usb/gadget/function/f_midi.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/usb/gadget/function/f_midi.c 
 b/drivers/usb/gadget/function/f_midi.c
 index 0e9cdeb..a617df3 100644
 --- a/drivers/usb/gadget/function/f_midi.c
 +++ b/drivers/usb/gadget/function/f_midi.c
 @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
 unsigned intf, unsigned alt)
   unsigned i;
   int err;

 + /* We don't care if it is not MIDIStreaming interface */
 + if (intf != ms_interface_desc.bInterfaceNumber)
 + return 0;
 +
>>>
>>> These global descriptors are overwritten in bind() of each instance of
>>> f_midi, so you have no guarantee that your bInterfaceNumber is correct
>>> for your current instance. Instead you should store value obtained from
>>> usb_interface_id() during bind().
>>
>> Ok.
>>
>> But then this interface descriptors shouldn't be global static,
>> because they will always reflect the latest bind() only. Right?
> 
> They are copied for each instance of USB function, so they are actually
> template to fill and copy in bind(). I'm currently working on some
> patches changing a bit behavior of set_alt() to make it clearer, but for
> now the only way to distinguish between altsettings properly is to store
> bInterfaceNumber value for each function instance in its bind().

Ok. Thanks.

-- 
Felipe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-27 Thread Robert Baldyga 
On 10/26/2015 11:53 PM, Felipe Tonello wrote:
> Hi Robert,
> 
> On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
>  wrote:
>> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
>>> This avoids duplication of USB requests for OUT endpoint and
>>> re-enabling endpoints.
>>>
>>> Signed-off-by: Felipe F. Tonello 
>>> ---
>>>  drivers/usb/gadget/function/f_midi.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>>> b/drivers/usb/gadget/function/f_midi.c
>>> index 0e9cdeb..a617df3 100644
>>> --- a/drivers/usb/gadget/function/f_midi.c
>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
>>> unsigned intf, unsigned alt)
>>>   unsigned i;
>>>   int err;
>>>
>>> + /* We don't care if it is not MIDIStreaming interface */
>>> + if (intf != ms_interface_desc.bInterfaceNumber)
>>> + return 0;
>>> +
>>
>> These global descriptors are overwritten in bind() of each instance of
>> f_midi, so you have no guarantee that your bInterfaceNumber is correct
>> for your current instance. Instead you should store value obtained from
>> usb_interface_id() during bind().
> 
> Ok.
> 
> But then this interface descriptors shouldn't be global static,
> because they will always reflect the latest bind() only. Right?

They are copied for each instance of USB function, so they are actually
template to fill and copy in bind(). I'm currently working on some
patches changing a bit behavior of set_alt() to make it clearer, but for
now the only way to distinguish between altsettings properly is to store
bInterfaceNumber value for each function instance in its bind().

Best regards,
Robert

> 
>>
>>>   err = f_midi_start_ep(midi, f, midi->in_ep);
>>>   if (err)
>>>   return err;
>>>
> 
> Felipe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-27 Thread Robert Baldyga 
On 10/26/2015 11:53 PM, Felipe Tonello wrote:
> Hi Robert,
> 
> On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
>  wrote:
>> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
>>> This avoids duplication of USB requests for OUT endpoint and
>>> re-enabling endpoints.
>>>
>>> Signed-off-by: Felipe F. Tonello 
>>> ---
>>>  drivers/usb/gadget/function/f_midi.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>>> b/drivers/usb/gadget/function/f_midi.c
>>> index 0e9cdeb..a617df3 100644
>>> --- a/drivers/usb/gadget/function/f_midi.c
>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
>>> unsigned intf, unsigned alt)
>>>   unsigned i;
>>>   int err;
>>>
>>> + /* We don't care if it is not MIDIStreaming interface */
>>> + if (intf != ms_interface_desc.bInterfaceNumber)
>>> + return 0;
>>> +
>>
>> These global descriptors are overwritten in bind() of each instance of
>> f_midi, so you have no guarantee that your bInterfaceNumber is correct
>> for your current instance. Instead you should store value obtained from
>> usb_interface_id() during bind().
> 
> Ok.
> 
> But then this interface descriptors shouldn't be global static,
> because they will always reflect the latest bind() only. Right?

They are copied for each instance of USB function, so they are actually
template to fill and copy in bind(). I'm currently working on some
patches changing a bit behavior of set_alt() to make it clearer, but for
now the only way to distinguish between altsettings properly is to store
bInterfaceNumber value for each function instance in its bind().

Best regards,
Robert

> 
>>
>>>   err = f_midi_start_ep(midi, f, midi->in_ep);
>>>   if (err)
>>>   return err;
>>>
> 
> Felipe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-27 Thread Felipe Ferreri Tonello 
Hi Robert,

On 27/10/15 06:47, Robert Baldyga wrote:
> On 10/26/2015 11:53 PM, Felipe Tonello wrote:
>> Hi Robert,
>>
>> On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
>>  wrote:
>>> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
 This avoids duplication of USB requests for OUT endpoint and
 re-enabling endpoints.

 Signed-off-by: Felipe F. Tonello 
 ---
  drivers/usb/gadget/function/f_midi.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/usb/gadget/function/f_midi.c 
 b/drivers/usb/gadget/function/f_midi.c
 index 0e9cdeb..a617df3 100644
 --- a/drivers/usb/gadget/function/f_midi.c
 +++ b/drivers/usb/gadget/function/f_midi.c
 @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
 unsigned intf, unsigned alt)
   unsigned i;
   int err;

 + /* We don't care if it is not MIDIStreaming interface */
 + if (intf != ms_interface_desc.bInterfaceNumber)
 + return 0;
 +
>>>
>>> These global descriptors are overwritten in bind() of each instance of
>>> f_midi, so you have no guarantee that your bInterfaceNumber is correct
>>> for your current instance. Instead you should store value obtained from
>>> usb_interface_id() during bind().
>>
>> Ok.
>>
>> But then this interface descriptors shouldn't be global static,
>> because they will always reflect the latest bind() only. Right?
> 
> They are copied for each instance of USB function, so they are actually
> template to fill and copy in bind(). I'm currently working on some
> patches changing a bit behavior of set_alt() to make it clearer, but for
> now the only way to distinguish between altsettings properly is to store
> bInterfaceNumber value for each function instance in its bind().

Ok. Thanks.

-- 
Felipe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-26 Thread Felipe Tonello 
Hi Robert,

On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
 wrote:
> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
>> This avoids duplication of USB requests for OUT endpoint and
>> re-enabling endpoints.
>>
>> Signed-off-by: Felipe F. Tonello 
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>> b/drivers/usb/gadget/function/f_midi.c
>> index 0e9cdeb..a617df3 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>>   unsigned i;
>>   int err;
>>
>> + /* We don't care if it is not MIDIStreaming interface */
>> + if (intf != ms_interface_desc.bInterfaceNumber)
>> + return 0;
>> +
>
> These global descriptors are overwritten in bind() of each instance of
> f_midi, so you have no guarantee that your bInterfaceNumber is correct
> for your current instance. Instead you should store value obtained from
> usb_interface_id() during bind().

Ok.

But then this interface descriptors shouldn't be global static,
because they will always reflect the latest bind() only. Right?

>
>>   err = f_midi_start_ep(midi, f, midi->in_ep);
>>   if (err)
>>   return err;
>>

Felipe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-26 Thread Robert Baldyga 
On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
> This avoids duplication of USB requests for OUT endpoint and
> re-enabling endpoints.
> 
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/function/f_midi.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 0e9cdeb..a617df3 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
>   unsigned i;
>   int err;
>  
> + /* We don't care if it is not MIDIStreaming interface */
> + if (intf != ms_interface_desc.bInterfaceNumber)
> + return 0;
> +

These global descriptors are overwritten in bind() of each instance of
f_midi, so you have no guarantee that your bInterfaceNumber is correct
for your current instance. Instead you should store value obtained from
usb_interface_id() during bind().

>   err = f_midi_start_ep(midi, f, midi->in_ep);
>   if (err)
>   return err;
> 

Best regards,
Robert

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-26 Thread Felipe Tonello 
Hi Robert,

On Mon, Oct 26, 2015 at 10:30 PM, Robert Baldyga
 wrote:
> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
>> This avoids duplication of USB requests for OUT endpoint and
>> re-enabling endpoints.
>>
>> Signed-off-by: Felipe F. Tonello 
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>> b/drivers/usb/gadget/function/f_midi.c
>> index 0e9cdeb..a617df3 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>>   unsigned i;
>>   int err;
>>
>> + /* We don't care if it is not MIDIStreaming interface */
>> + if (intf != ms_interface_desc.bInterfaceNumber)
>> + return 0;
>> +
>
> These global descriptors are overwritten in bind() of each instance of
> f_midi, so you have no guarantee that your bInterfaceNumber is correct
> for your current instance. Instead you should store value obtained from
> usb_interface_id() during bind().

Ok.

But then this interface descriptors shouldn't be global static,
because they will always reflect the latest bind() only. Right?

>
>>   err = f_midi_start_ep(midi, f, midi->in_ep);
>>   if (err)
>>   return err;
>>

Felipe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 5/7] usb: gadget: f_midi: set altsettings only for MIDIStreaming interface

2015-10-26 Thread Robert Baldyga 
On 10/26/2015 05:55 PM, Felipe F. Tonello wrote:
> This avoids duplication of USB requests for OUT endpoint and
> re-enabling endpoints.
> 
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/function/f_midi.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 0e9cdeb..a617df3 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -323,6 +323,10 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
>   unsigned i;
>   int err;
>  
> + /* We don't care if it is not MIDIStreaming interface */
> + if (intf != ms_interface_desc.bInterfaceNumber)
> + return 0;
> +

These global descriptors are overwritten in bind() of each instance of
f_midi, so you have no guarantee that your bInterfaceNumber is correct
for your current instance. Instead you should store value obtained from
usb_interface_id() during bind().

>   err = f_midi_start_ep(midi, f, midi->in_ep);
>   if (err)
>   return err;
> 

Best regards,
Robert

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/