svn commit: r184605 - head/sys/dev/usb

[Christoph Mallon]

Hi Nick.

Nick Hibma wrote:
> Author: n_hibma
> Date: Mon Nov  3 22:09:27 2008
> New Revision: 184605
> URL: http://svn.freebsd.org/changeset/base/184605
> 
> Log:
>   Bugfix: Cut&paste error from the NetBSD code.
>   
>   Also: Change the initialisation of the command string to a static
>   initialiser. Verify it against the output of umass.c when being sent a
>   command using 'camcontrol eject da0' to a Bulk-Only device.
>   
>   This should make those devices work that need a SCSI eject command to
>   switch to modem mode (Novatel 950D and others).
> 
> Modified:
>   head/sys/dev/usb/u3g.c
> 
> Modified: head/sys/dev/usb/u3g.c
> ==============================================================================
> --- head/sys/dev/usb/u3g.c	Mon Nov  3 22:05:44 2008	(r184604)
> +++ head/sys/dev/usb/u3g.c	Mon Nov  3 22:09:27 2008	(r184605)
> @@ -458,34 +458,38 @@ u3gstub_huawei_init(struct u3gstub_softc
>  static int
>  u3gstub_scsi_eject(struct u3gstub_softc *sc, struct usb_attach_arg *uaa)
>  {
> -	unsigned char cmd[31];
> +	/* See definition of umass_bbb_cbw_t in sys/dev/usb/umass.c and struct
> +	 * scsi_start_stop_unit in sys/cam/scsi/scsi_all.h .
> +         */
> +	unsigned char cmd[31] = {
> +	    0x55, 0x53, 0x42, 0x43,	/* 0..3: Command Block Wrapper (CBW) signature */
> +	    0x01, 0x00, 0x00, 0x00,	/* 4..7: CBW Tag, unique 32-bit number */
> +	    0x00, 0x00, 0x00, 0x00,	/* 8..11: CBW Transfer Length, no data here */
> +	    0x00,			/* 12: CBW Flag: output, so 0 */
> +	    0x00,			/* 13: CBW Lun */
> +	    0x06,			/* 14: CBW Length */
> +
> +	    0x1b,			/* 15+0: opcode: SCSI START/STOP */
> +	    0x00,			/* 15+1: byte2: Not immediate */
> +	    0x00, 0x00,			/* 15+2..3: reserved */
> +	    0x02,			/* 15+4: Load/Eject command */
> +	    0x00,			/* 15+5: control */
> +	    0x00, 0x00, 0x00, 0x00,	/* 15+6..15: unused */
> +	    0x00, 0x00, 0x00, 0x00,
> +	    0x00, 0x00
> +	};
> +
>  	usb_interface_descriptor_t *id;
>  	usb_endpoint_descriptor_t *ed = NULL;
>  	int i;
>  
> -	memset(cmd, 0, sizeof(cmd));
> -	cmd[0] = 0x55;		/* Byte 0..3: Command Block Wrapper (CBW) signature */
> -	cmd[1] = 0x53;
> -	cmd[2] = 0x42;
> -	cmd[3] = 0x43;
> -	cmd[4] = 0x01;		/* 4..7: CBW Tag, has to unique, but only a single transfer used. */
> -				/* 8..11: CBW Transfer Length, no data here */
> -				/* 12: CBW Flag: output, so 0 */
> -				/* 13: CBW Lun: 0 */
> -				/* 14: CBW Length */
> -	cmd[14] = 0x06;
> -	cmd[15] = 0x1b;		/* 0: SCSI START/STOP opcode */
> -				/* 1..3 unused */
> -	cmd[15+4] = 0x02;	/* 4 Load/Eject command */
> -				/* 5: unused */
> -
>  
>  	/* Find the bulk-out endpoints */
>  	id = usbd_get_interface_descriptor(uaa->iface);

You may want to make cmd[] static and/or const, so the compiler really 
just puts the bytes in the data section instead of generating code, 
which pretty much is the same as the assignments you just removed. GCC 
creates 31 consecutive movb instructions for the static initialiser, 
which is most probably worse than the code before

Regards
	Christoph