Re: [PATCH] gspca main: reorganize loop

["Németh Márton" <nm127@freemail.hu>]

Jean-Francois Moine wrote:
> On Sun, 29 Nov 2009 11:24:31 +0100
> Németh Márton <nm127@freemail.hu> wrote:
> 
>> From: Márton Németh <nm127@freemail.hu>
>>
>> Eliminate redundant code by reorganizing the loop.
>>
>> Signed-off-by: Márton Németh <nm127@freemail.hu>
>> ---
>> diff -r 064a82aa2daa linux/drivers/media/video/gspca/gspca.c
>> --- a/linux/drivers/media/video/gspca/gspca.c	Thu Nov 26
>> 19:36:40 2009 +0100 +++
>> b/linux/drivers/media/video/gspca/gspca.c	Sun Nov 29 11:09:33
>> 2009 +0100 @@ -623,12 +623,12 @@ if (ret < 0)
>>  			goto out;
>>  	}
>> -	ep = get_ep(gspca_dev);
>> -	if (ep == NULL) {
>> -		ret = -EIO;
>> -		goto out;
>> -	}
>>  	for (;;) {
>> +		ep = get_ep(gspca_dev);
>> +		if (ep == NULL) {
>> +			ret = -EIO;
>> +			goto out;
>> +		}
>>  		PDEBUG(D_STREAM, "init transfer alt %d",
>> gspca_dev->alt); ret = create_urbs(gspca_dev, ep);
>>  		if (ret < 0)
>> @@ -677,12 +677,6 @@
>>  			ret =
>> gspca_dev->sd_desc->isoc_nego(gspca_dev); if (ret < 0)
>>  				goto out;
>> -		} else {
>> -			ep = get_ep(gspca_dev);
>> -			if (ep == NULL) {
>> -				ret = -EIO;
>> -				goto out;
>> -			}
>>  		}
>>  	}
>>  out:
> 
> Hello Márton,
> 
> As you may see, in the loop, get_ep() is called only when isoc_nego()
> is not called. So, your patch does not work.

You are right, I overseen that.

Is there any subdriver where the isoc_nego() is implemented? I couldn't find
one. What would be the task of the isoc_nego() function? Should it set
the interface by calling usb_set_interface() as the get_ep() does? Should
it create URBs for the endpoint?

Although I found the patch where the isoc_nego() was introduced
( http://linuxtv.org/hg/v4l-dvb/rev/5a5b23605bdb56aec86c9a89de8ca8b8ae9cb925 )
it is not clear how the "ep" pointer is updated when not the isoc_nego() is
called instead of get_ep() in the current implementation.

Regards,

	Márton Németh