speedtch usbatm.c,1.26,1.27

Roman Kagan rkagan at mail.ru
Tue May 17 16:48:13 EDT 2005


On Sun, May 15, 2005 at 10:37:39PM +0200, matthieu castet wrote:
> Roman Kagan wrote:
> >I can only humbly repeat my suggestion to move channel's enpoints
> >initialization back to before .bind(), to allow the subdriver to
> >override it, but apparently Duncan doesn't like this approach...
> >
> If he don't like it, why don't use something like that :
> it could be statiquely initialized, changed in usb_probe and in
> usbatm_bind .
> 
> Matthieu
> 

> Index: usbatm.c
> ===================================================================
> RCS file: /home/cvs/usbatm/usbatm.c,v
> retrieving revision 1.44
> diff -u -r1.44 usbatm.c
> --- usbatm.c	11 May 2005 14:59:39 -0000	1.44
> +++ usbatm.c	15 May 2005 19:44:49 -0000
> @@ -1019,7 +1019,10 @@
>  	usbatm_init_channel(&instance->tx_channel);
>  	tasklet_init(&instance->rx_channel.tasklet, usbatm_rx_process, (unsigned long)instance);
>  	tasklet_init(&instance->tx_channel.tasklet, usbatm_tx_process, (unsigned long)instance);
> -	instance->rx_channel.endpoint = usb_rcvbulkpipe(usb_dev, driver->in);
> +	if (driver->use_iso)
> +		instance->rx_channel.endpoint = usb_rcvisocpipe(usb_dev, driver->in);
> +	else
> +		instance->rx_channel.endpoint = usb_rcvbulkpipe(usb_dev, driver->in);
>  	instance->tx_channel.endpoint = usb_sndbulkpipe(usb_dev, driver->out);
>  	instance->rx_channel.stride = ATM_CELL_SIZE + driver->rx_padding;
>  	instance->tx_channel.stride = ATM_CELL_SIZE + driver->tx_padding;


IMHO this is making simple thing complex...  Simply setting
instance->rx_channel.endpoint to the right pipe in .bind() is easier and
more concise.

BTW how are you going to change it in driver's .bind() in your patch?
It seems to exclude the possibility for the driver to decide at run time
which transfer type it likes better, and this sounds like a bad idea.

Cheers,
  Roman.



More information about the Usbatm mailing list