Re: [PATCH V3 1/5] i2c: Add irq_gpio field to struct i2c_client, i2c_board_info.

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Stephen,

Can you please fix your e-mail client / system / whatever so that your
patch series are no longer sent duplicated?

On Thu,  1 Sep 2011 16:04:27 -0600, Stephen Warren wrote:
> Some devices use a single pin as both an IRQ and a GPIO. In that case,
> irq_gpio is the GPIO ID for that pin. Not all drivers use this feature.
> Where they do, and the use of this feature is optional, and the system
> wishes to disable this feature, this field must be explicitly set to a
> defined invalid GPIO ID, such as -1.
> 
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v3: Also add the field to i2c_board_info, and copy the field from
>     i2c_board_info to i2c_client upon instantiation

I don't get the idea. The i2c core doesn't make any use of the field,
and that field will only be used by a few drivers amongst the 420+
i2c drivers in the tree. This looks like a waste of memory. What's wrong
with including the new field in the private platform or arch data
structure for drivers which need it?

> 
>  drivers/i2c/i2c-core.c |    1 +
>  include/linux/i2c.h    |    9 +++++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 131079a..da12540 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -518,6 +518,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
>  	client->flags = info->flags;
>  	client->addr = info->addr;
>  	client->irq = info->irq;
> +	client->irq_gpio = info->irq_gpio;
>  
>  	strlcpy(client->name, info->type, sizeof(client->name));
>  
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 3fad485..49e2e36 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -192,6 +192,12 @@ struct i2c_driver {
>   * @driver: device's driver, hence pointer to access routines
>   * @dev: Driver model device node for the slave.
>   * @irq: indicates the IRQ generated by this device (if any)
> + * @irq_gpio: some devices use a single pin as both an IRQ and a GPIO. In
> + *	that case, irq_gpio is the GPIO ID for that pin. Not all drivers
> + *	use this feature. Where they do, and the use of this feature is
> + *	optional, and the system wishes to disable this feature, this
> + *	field must be explicitly set to a defined invalid GPIO ID, such
> + *	as -1.
>   * @detected: member of an i2c_driver.clients list or i2c-core's
>   *	userspace_devices list
>   *
> @@ -209,6 +215,7 @@ struct i2c_client {
>  	struct i2c_driver *driver;	/* and our access routines	*/
>  	struct device dev;		/* the device structure		*/
>  	int irq;			/* irq issued by device		*/
> +	int irq_gpio;			/* gpio corresponding to irq	*/
>  	struct list_head detected;
>  };
>  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> @@ -240,6 +247,7 @@ static inline void i2c_set_clientdata(struct i2c_client *dev, void *data)
>   * @archdata: copied into i2c_client.dev.archdata
>   * @of_node: pointer to OpenFirmware device node
>   * @irq: stored in i2c_client.irq
> + * @irq_gpio: stored in i2c_client.irq_gpio
>   *
>   * I2C doesn't actually support hardware probing, although controllers and
>   * devices may be able to use I2C_SMBUS_QUICK to tell whether or not there's
> @@ -260,6 +268,7 @@ struct i2c_board_info {
>  	struct dev_archdata	*archdata;
>  	struct device_node *of_node;
>  	int		irq;
> +	int		irq_gpio;
>  };
>  
>  /**


-- 
Jean Delvare