Re: [patch 02/05] ti_usb, changing firmware: adopting userspace helper

On Fri, Feb 23, 2007 at 06:34:10AM +0100, Oleg Verych wrote:
> pp-by: Oleg Verych
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 148 +++++++++++++++++++++-------------
>  drivers/usb/serial/ti_usb_3410_5052.h |  27 +++---
>  2 files changed, 109 insertions(+), 66 deletions(-)
> 
> Index: linux-2.6.21-rc1/drivers/usb/serial/ti_usb_3410_5052.h
> ===================================================================
> --- linux-2.6.21-rc1.orig/drivers/usb/serial/ti_usb_3410_5052.h	2007-02-23 02:19:57.950025000 +0100
> +++ linux-2.6.21-rc1/drivers/usb/serial/ti_usb_3410_5052.h	2007-02-23 04:02:08.977190250 +0100
> @@ -21,8 +21,8 @@
>  #define _TI_3410_5052_H_
>  
>  /* Configuration ids */
> -#define TI_BOOT_CONFIG			1
> -#define TI_ACTIVE_CONFIG		2
> +#define TI_BOOT_CONFIG			1 /* boot config to get firmware  */
> +#define TI_ACTIVE_CONFIG		2 /* actual working device config */
>  
>  /* Vendor and product ids */
>  #define TI_VENDOR_ID			0x0451
> @@ -206,19 +206,24 @@
>  #define TI_CODE_DATA_ERROR		0x03
>  #define TI_CODE_MODEM_STATUS		0x04
>  
> -/* Download firmware max packet size */
> -#define TI_DOWNLOAD_MAX_PACKET_SIZE	64
> -
> -/* Firmware image header */
> -struct ti_firmware_header {
> -	__le16	wLength;
> -	__u8	bCheckSum;
> -} __attribute__((packed));
> -
>  /* UART addresses */
>  #define TI_UART1_BASE_ADDR		0xFFA0	/* UART 1 base address */
>  #define TI_UART2_BASE_ADDR		0xFFB0	/* UART 2 base address */
>  #define TI_UART_OFFSET_LCR		0x0002	/* UART MCR register offset */
>  #define TI_UART_OFFSET_MCR		0x0004	/* UART MCR register offset */
>  
> +/* Firmware */
> +#define TI_FW_PACKET_SIZE		64
> +#define TI_MAX_FIRMWARE_SIZE		16284
> +
> +#define ti_fw_file_3410			"umpf3410.i51"
> +#define ti_fw_file_5052			"umpf5052.i51"
> +
> +typedef union {
> +	__le32 a; /* all */
> +	struct {
> +		__le32 sz : 16, cs : 8;
> +	} d;
> +} ti_firmware_header_t;

No, do not add typedefs to the kernel, it is not needed.  What was wrong
with the original structure name?

Also, please provide better variable names, like the original structure
had.  "d", "a", "cs", and "sz" are not sufficient.

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel