[i2c] [patch 2.6.21-rc2-git +i2c-patches] shrink i2c_client

[Jean Delvare]

Hi David,

On Mon, 5 Mar 2007 22:44:18 -0800, David Brownell wrote:
> This shrinks the size of "struct i2c_client" by over 32 bytes:
> 
>  - Substantially shrinks the string used to identify the chip type
>  - The "flags" don't need to be so big
>  - Removes some internal padding
> 
> It also adds kerneldoc for that struct, explaining how "name" is really a
> chip type identifier; it's otherwise potentially confusing.
> 
> Because the I2C_NAME_SIZE symbol was abused for both i2c_client.name
> and for i2c_adapter.name, this needed to affect i2c_adapter too.  The
> adapters which used that symbol now use the more-obviously-correct
> idiom of taking the size of that field.

Applied, thanks. Two questions:

> Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
> ---
> As previously discussed.  Goes on top of various patches now in Jean's
> I2C queue (#4-6 of a group posted recently) plus the cleanup patch
> that I just posted.
> 
>  drivers/i2c/busses/i2c-ali1535.c             |    2 +-
>  drivers/i2c/busses/i2c-ali15x3.c             |    2 +-
>  drivers/i2c/busses/i2c-amd8111.c             |    2 +-
>  drivers/i2c/busses/i2c-i801.c                |    2 +-
>  drivers/i2c/busses/i2c-ixp2000.c             |    2 +-
>  drivers/i2c/busses/i2c-ixp4xx.c              |    2 +-
>  drivers/i2c/busses/i2c-mv64xxx.c             |    2 +-
>  drivers/i2c/busses/i2c-nforce2.c             |    2 +-
>  drivers/i2c/busses/i2c-pasemi.c              |    2 +-
>  drivers/i2c/busses/i2c-piix4.c               |    2 +-
>  drivers/i2c/busses/i2c-sis96x.c              |    2 +-
>  drivers/i2c/busses/i2c-viapro.c              |    2 +-
>  drivers/i2c/busses/scx200_acb.c              |    2 +-
>  drivers/media/dvb/b2c2/flexcop-i2c.c         |    2 +-
>  drivers/media/dvb/dvb-usb/dvb-usb-i2c.c      |    2 +-
>  drivers/media/dvb/frontends/dibx000_common.c |    4 ++--
>  drivers/video/intelfb/intelfb_i2c.c          |    2 +-
>  drivers/video/matrox/i2c-matroxfb.c          |    2 +-
>  include/linux/i2c.h                          |   21 +++++++++++++--------
>  19 files changed, 32 insertions(+), 27 deletions(-)
> 
> Index: at91/include/linux/i2c.h
> ===================================================================
> --- at91.orig/include/linux/i2c.h	2007-03-05 12:45:47.000000000 -0800
> +++ at91/include/linux/i2c.h	2007-03-05 12:45:52.000000000 -0800
> @@ -139,25 +139,30 @@ struct i2c_driver {
>  };
>  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
>  
> -#define I2C_NAME_SIZE	50
> +#define I2C_NAME_SIZE	20
>  
> -/*
> - * i2c_client identifies a single device (i.e. chip) that is connected to an
> - * i2c bus. The behaviour is defined by the routines of the driver. This
> - * function is mainly used for lookup & other admin. functions.
> +/**
> + * struct i2c_client - represent an I2C slave device
> + * @addr: Address used on the I2C bus connected to the parent adapter.
> + * @name: Indicates the type of the device, usually a chip name that's
> + *	generic enough to hide second-sourcing and compatible revisions.
> + * @dev: Driver model device node for the slave.
> + *
> + * An i2c_client identifies a single device (i.e. chip) connected to an
> + * i2c bus. The behaviour is defined by the routines of the driver.
>   */
>  struct i2c_client {
> -	unsigned int flags;		/* div., see below		*/
> +	unsigned short flags;		/* div., see below		*/
>  	unsigned short addr;		/* chip address - NOTE: 7bit	*/
>  					/* addresses are stored in the	*/
>  					/* _LOWER_ 7 bits		*/
> +	char name[I2C_NAME_SIZE];
>  	struct i2c_adapter *adapter;	/* the adapter we sit on	*/
>  	struct i2c_driver *driver;	/* and our access routines	*/
>  	int usage_count;		/* How many accesses currently  */
>  					/* to the client		*/
>  	struct device dev;		/* the device structure		*/
>  	struct list_head list;
> -	char name[I2C_NAME_SIZE];

Do we actually spare padding by moving name? If not I'd rather leave it
where it was.

>  	struct completion released;
>  };
>  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> @@ -230,7 +235,7 @@ struct i2c_adapter {
>  	int nr;
>  	struct list_head clients;
>  	struct list_head list;
> -	char name[I2C_NAME_SIZE];
> +	char name[50];
>  	struct completion dev_released;
>  };

Don't we want to make it 48 chars instead of 50 while we're here? A
multiple of 8 would avoid wasting space in padding, and 48 chars should
still be enough for everyone.

Thanks,
-- 
Jean Delvare