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

From: Jean Delvare
Date: Fri Sep 02 2011 - 02:56:59 EST


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@xxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/