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

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

[Stephen Warren]

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@nvidia.com>
---
v3: Also add the field to i2c_board_info, and copy the field from
    i2c_board_info to i2c_client upon instantiation

 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;
 };
 
 /**
-- 
1.7.0.4

[PATCH V3 2/5] spi: Add irq_gpio field to struct spi_device, spi_board_info.

[Stephen Warren]

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@nvidia.com>
---
v3: New patch for v3; apply the same change to spi as for i2c per Mark
    Brown.

 drivers/spi/spi.c       |    1 +
 include/linux/spi/spi.h |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 77eae99..9932572 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -433,6 +433,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
 	proxy->max_speed_hz = chip->max_speed_hz;
 	proxy->mode = chip->mode;
 	proxy->irq = chip->irq;
+	proxy->irq_gpio = chip->irq_gpio;
 	strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
 	proxy->dev.platform_data = (void *) chip->platform_data;
 	proxy->controller_data = chip->controller_data;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bb4f5fb..086b591 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -50,6 +50,12 @@ extern struct bus_type spi_bus_type;
  *	The spi_transfer.bits_per_word can override this for each transfer.
  * @irq: Negative, or the number passed to request_irq() to receive
  *	interrupts from this device.
+ * @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.
  * @controller_state: Controller's runtime state
  * @controller_data: Board-specific definitions for controller, such as
  *	FIFO initialization parameters; from board_info.controller_data
@@ -86,6 +92,7 @@ struct spi_device {
 #define	SPI_READY	0x80			/* slave pulls low to pause */
 	u8			bits_per_word;
 	int			irq;
+	int			irq_gpio;
 	void			*controller_state;
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
@@ -692,6 +699,8 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd)
  * @controller_data: Initializes spi_device.controller_data; some
  *	controllers need hints about hardware setup, e.g. for DMA.
  * @irq: Initializes spi_device.irq; depends on how the board is wired.
+ * @irq_gpio: Initializes spi_device.irq_gpio; depends on how the board
+ *	is wired.
  * @max_speed_hz: Initializes spi_device.max_speed_hz; based on limits
  *	from the chip datasheet and board-specific signal quality issues.
  * @bus_num: Identifies which spi_master parents the spi_device; unused
@@ -727,6 +736,7 @@ struct spi_board_info {
 	const void	*platform_data;
 	void		*controller_data;
 	int		irq;
+	int		irq_gpio;
 
 	/* slower signaling on noisy or low voltage boards */
 	u32		max_speed_hz;
-- 
1.7.0.4

[PATCH V3 3/5] staging:iio:magnetometer:ak8975 Don't use irq_to_gpio()

[Stephen Warren]

Tegra doesn't have irq_to_gpio() any more, and ak8975 is included in
tegra_defconfig. This causes a build failure.

Instead, obtain the GPIO ID corresponding to the chip's IRQ from the new
i2c_client field irq_gpio.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/staging/iio/magnetometer/ak8975.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index a17fa9f..14076da 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -477,7 +477,7 @@ static int ak8975_probe(struct i2c_client *client,
 	int err;
 
 	/* Grab and set up the supplied GPIO. */
-	eoc_gpio = irq_to_gpio(client->irq);
+	eoc_gpio = client->irq_gpio;
 
 	/* We may not have a GPIO based IRQ to scan, that is fine, we will
 	   poll if so */
-- 
1.7.0.4

[PATCH V3 4/5] staging:iio:magnetometer:ak8975: Don't assume 0 is an invalid GPIO

[Stephen Warren]

gpio_is_valid() is the defined mechanism to determine whether a GPIO is
valid. Use this instead of assuming that 0 is an invalid GPIO.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/staging/iio/magnetometer/ak8975.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index 14076da..0dfdf50 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -373,7 +373,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	}
 
 	/* Wait for the conversion to complete. */
-	if (data->eoc_gpio)
+	if (gpio_is_valid(data->eoc_gpio))
 		ret = wait_conversion_complete_gpio(data);
 	else
 		ret = wait_conversion_complete_polled(data);
@@ -481,7 +481,7 @@ static int ak8975_probe(struct i2c_client *client,
 
 	/* We may not have a GPIO based IRQ to scan, that is fine, we will
 	   poll if so */
-	if (eoc_gpio > 0) {
+	if (gpio_is_valid(eoc_gpio)) {
 		err = gpio_request(eoc_gpio, "ak_8975");
 		if (err < 0) {
 			dev_err(&client->dev,
@@ -497,8 +497,7 @@ static int ak8975_probe(struct i2c_client *client,
 						eoc_gpio, err);
 			goto exit_gpio;
 		}
-	} else
-		eoc_gpio = 0;	/* No GPIO available */
+	}
 
 	/* Register with IIO */
 	indio_dev = iio_allocate_device(sizeof(*data));
@@ -534,7 +533,7 @@ static int ak8975_probe(struct i2c_client *client,
 exit_free_iio:
 	iio_free_device(indio_dev);
 exit_gpio:
-	if (eoc_gpio)
+	if (gpio_is_valid(eoc_gpio))
 		gpio_free(eoc_gpio);
 exit:
 	return err;
@@ -549,7 +548,7 @@ static int ak8975_remove(struct i2c_client *client)
 	iio_device_unregister(indio_dev);
 	iio_free_device(indio_dev);
 
-	if (eoc_gpio)
+	if (gpio_is_valid(eoc_gpio))
 		gpio_free(eoc_gpio);
 
 	return 0;
-- 
1.7.0.4

[PATCH V3 5/5] staging:iio:magnetometer:ak8975: Fix probe() error-handling

[Stephen Warren]

Fix ak8975_probe() to jump to the appropriate exit labels when an error
occurs. With the previous code, some cleanup actions were being skipped
for some error conditions.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/staging/iio/magnetometer/ak8975.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index 0dfdf50..cc20e8d 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -510,7 +510,7 @@ static int ak8975_probe(struct i2c_client *client,
 	err = ak8975_setup(client);
 	if (err < 0) {
 		dev_err(&client->dev, "AK8975 initialization fails\n");
-		goto exit_gpio;
+		goto exit_free_iio;
 	}
 
 	i2c_set_clientdata(client, indio_dev);
-- 
1.7.0.4

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

[Stephen Warren]

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@nvidia.com>
---
v3: Also add the field to i2c_board_info, and copy the field from
    i2c_board_info to i2c_client upon instantiation

 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;
 };
 
 /**
-- 
1.7.0.4

[PATCH V3 2/5] spi: Add irq_gpio field to struct spi_device, spi_board_info.

[Stephen Warren]

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@nvidia.com>
---
v3: New patch for v3; apply the same change to spi as for i2c per Mark
    Brown.

 drivers/spi/spi.c       |    1 +
 include/linux/spi/spi.h |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 77eae99..9932572 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -433,6 +433,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
 	proxy->max_speed_hz = chip->max_speed_hz;
 	proxy->mode = chip->mode;
 	proxy->irq = chip->irq;
+	proxy->irq_gpio = chip->irq_gpio;
 	strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
 	proxy->dev.platform_data = (void *) chip->platform_data;
 	proxy->controller_data = chip->controller_data;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bb4f5fb..086b591 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -50,6 +50,12 @@ extern struct bus_type spi_bus_type;
  *	The spi_transfer.bits_per_word can override this for each transfer.
  * @irq: Negative, or the number passed to request_irq() to receive
  *	interrupts from this device.
+ * @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.
  * @controller_state: Controller's runtime state
  * @controller_data: Board-specific definitions for controller, such as
  *	FIFO initialization parameters; from board_info.controller_data
@@ -86,6 +92,7 @@ struct spi_device {
 #define	SPI_READY	0x80			/* slave pulls low to pause */
 	u8			bits_per_word;
 	int			irq;
+	int			irq_gpio;
 	void			*controller_state;
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
@@ -692,6 +699,8 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd)
  * @controller_data: Initializes spi_device.controller_data; some
  *	controllers need hints about hardware setup, e.g. for DMA.
  * @irq: Initializes spi_device.irq; depends on how the board is wired.
+ * @irq_gpio: Initializes spi_device.irq_gpio; depends on how the board
+ *	is wired.
  * @max_speed_hz: Initializes spi_device.max_speed_hz; based on limits
  *	from the chip datasheet and board-specific signal quality issues.
  * @bus_num: Identifies which spi_master parents the spi_device; unused
@@ -727,6 +736,7 @@ struct spi_board_info {
 	const void	*platform_data;
 	void		*controller_data;
 	int		irq;
+	int		irq_gpio;
 
 	/* slower signaling on noisy or low voltage boards */
 	u32		max_speed_hz;
-- 
1.7.0.4

[PATCH V3 3/5] staging:iio:magnetometer:ak8975 Don't use irq_to_gpio()

[Stephen Warren]

Tegra doesn't have irq_to_gpio() any more, and ak8975 is included in
tegra_defconfig. This causes a build failure.

Instead, obtain the GPIO ID corresponding to the chip's IRQ from the new
i2c_client field irq_gpio.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/staging/iio/magnetometer/ak8975.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index a17fa9f..14076da 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -477,7 +477,7 @@ static int ak8975_probe(struct i2c_client *client,
 	int err;
 
 	/* Grab and set up the supplied GPIO. */
-	eoc_gpio = irq_to_gpio(client->irq);
+	eoc_gpio = client->irq_gpio;
 
 	/* We may not have a GPIO based IRQ to scan, that is fine, we will
 	   poll if so */
-- 
1.7.0.4

[PATCH V3 4/5] staging:iio:magnetometer:ak8975: Don't assume 0 is an invalid GPIO

[Stephen Warren]

gpio_is_valid() is the defined mechanism to determine whether a GPIO is
valid. Use this instead of assuming that 0 is an invalid GPIO.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/staging/iio/magnetometer/ak8975.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index 14076da..0dfdf50 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -373,7 +373,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	}
 
 	/* Wait for the conversion to complete. */
-	if (data->eoc_gpio)
+	if (gpio_is_valid(data->eoc_gpio))
 		ret = wait_conversion_complete_gpio(data);
 	else
 		ret = wait_conversion_complete_polled(data);
@@ -481,7 +481,7 @@ static int ak8975_probe(struct i2c_client *client,
 
 	/* We may not have a GPIO based IRQ to scan, that is fine, we will
 	   poll if so */
-	if (eoc_gpio > 0) {
+	if (gpio_is_valid(eoc_gpio)) {
 		err = gpio_request(eoc_gpio, "ak_8975");
 		if (err < 0) {
 			dev_err(&client->dev,
@@ -497,8 +497,7 @@ static int ak8975_probe(struct i2c_client *client,
 						eoc_gpio, err);
 			goto exit_gpio;
 		}
-	} else
-		eoc_gpio = 0;	/* No GPIO available */
+	}
 
 	/* Register with IIO */
 	indio_dev = iio_allocate_device(sizeof(*data));
@@ -534,7 +533,7 @@ static int ak8975_probe(struct i2c_client *client,
 exit_free_iio:
 	iio_free_device(indio_dev);
 exit_gpio:
-	if (eoc_gpio)
+	if (gpio_is_valid(eoc_gpio))
 		gpio_free(eoc_gpio);
 exit:
 	return err;
@@ -549,7 +548,7 @@ static int ak8975_remove(struct i2c_client *client)
 	iio_device_unregister(indio_dev);
 	iio_free_device(indio_dev);
 
-	if (eoc_gpio)
+	if (gpio_is_valid(eoc_gpio))
 		gpio_free(eoc_gpio);
 
 	return 0;
-- 
1.7.0.4

[PATCH V3 5/5] staging:iio:magnetometer:ak8975: Fix probe() error-handling

[Stephen Warren]

Fix ak8975_probe() to jump to the appropriate exit labels when an error
occurs. With the previous code, some cleanup actions were being skipped
for some error conditions.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/staging/iio/magnetometer/ak8975.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index 0dfdf50..cc20e8d 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -510,7 +510,7 @@ static int ak8975_probe(struct i2c_client *client,
 	err = ak8975_setup(client);
 	if (err < 0) {
 		dev_err(&client->dev, "AK8975 initialization fails\n");
-		goto exit_gpio;
+		goto exit_free_iio;
 	}
 
 	i2c_set_clientdata(client, indio_dev);
-- 
1.7.0.4

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

[Jean Delvare]

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

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

[Jonathan Cameron]

On 09/02/11 07:56, Jean Delvare wrote:
> 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?
Why not make it platform data for now and 'if' it becomes way more common
(probably won't) we can always propose adding as a general field at a later
date.

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

[Jean Delvare]

Hi Jonathan,

On Fri, 02 Sep 2011 10:19:24 +0100, Jonathan Cameron wrote:
> On 09/02/11 07:56, Jean Delvare wrote:
> > 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?
>
> Why not make it platform data for now and 'if' it becomes way more common
> (probably won't) we can always propose adding as a general field at a later
> date.

Yes, this sounds like a much more reasonable approach.

-- 
Jean Delvare

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

[Stephen Warren]

Jean Delvare wrote at Friday, September 02, 2011 3:25 AM:
> Hi Jonathan,
> 
> On Fri, 02 Sep 2011 10:19:24 +0100, Jonathan Cameron wrote:
> > On 09/02/11 07:56, Jean Delvare wrote:
> > > 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?
> >
> > Why not make it platform data for now and 'if' it becomes way more common
> > (probably won't) we can always propose adding as a general field at a later
> > date.
> 
> Yes, this sounds like a much more reasonable approach.

BTW, if that's the direction that's decided, just take the first version
of the patchset I posted, plus Jonathan Cameron's subsequent patch to
modify ak8975 to accept GPIO ID through platform data.

-- 
nvpublic

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

[Greg KH]

On Fri, Sep 02, 2011 at 11:24:04AM -0700, Stephen Warren wrote:
> Jean Delvare wrote at Friday, September 02, 2011 3:25 AM:
> > Hi Jonathan,
> > 
> > On Fri, 02 Sep 2011 10:19:24 +0100, Jonathan Cameron wrote:
> > > On 09/02/11 07:56, Jean Delvare wrote:
> > > > 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?
> > >
> > > Why not make it platform data for now and 'if' it becomes way more common
> > > (probably won't) we can always propose adding as a general field at a later
> > > date.
> > 
> > Yes, this sounds like a much more reasonable approach.
> 
> BTW, if that's the direction that's decided, just take the first version
> of the patchset I posted, plus Jonathan Cameron's subsequent patch to
> modify ak8975 to accept GPIO ID through platform data.

I don't know which patchset that would be, can you please just resend
what you want applied so that I know I get the correct one?

thanks,

greg k-h

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

[Stephen Warren]

Greg KH wrote at Tuesday, September 06, 2011 4:57 PM:
> On Fri, Sep 02, 2011 at 11:24:04AM -0700, Stephen Warren wrote:
> > Jean Delvare wrote at Friday, September 02, 2011 3:25 AM:
...
> > > > Why not make it platform data for now and 'if' it becomes way more common
> > > > (probably won't) we can always propose adding as a general field at a later
> > > > date.
> > >
> > > Yes, this sounds like a much more reasonable approach.
> >
> > BTW, if that's the direction that's decided, just take the first version
> > of the patchset I posted, plus Jonathan Cameron's subsequent patch to
> > modify ak8975 to accept GPIO ID through platform data.
> 
> I don't know which patchset that would be, can you please just resend
> what you want applied so that I know I get the correct one?

Sorry for the slow response; I was on vacation for the last couple weeks.
I've now posted an updated version of this patchset, without any of the
changes for I2C/SPI core/types/...

-- 
nvpublic

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

[Grant Likely]

On Fri, Sep 02, 2011 at 08:56:20AM +0200, Jean Delvare wrote:
> 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?

I have to second the concern; but for a different reason.  This
shouldn't even remotely be necessary.  If the pin is used as an
interrupt, then interrupt controller driver (which I would assume is
also the gpio controller driver) should be responsible for setting up
the pin so that it can be used correctly as a irq line.  Why does the
gpio number need to be explicitly passed?

g.

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

[Jonathan Cameron]

On 09/20/11 05:16, Grant Likely wrote:
> On Fri, Sep 02, 2011 at 08:56:20AM +0200, Jean Delvare wrote:
>> 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?
> 
> I have to second the concern; but for a different reason.  This
> shouldn't even remotely be necessary.  If the pin is used as an
> interrupt, then interrupt controller driver (which I would assume is
> also the gpio controller driver) should be responsible for setting up
> the pin so that it can be used correctly as a irq line.  Why does the
> gpio number need to be explicitly passed?
The particular driver covered here is somewhat of a false positive.
It really ought to be rewritten to do everything 'properly' with
interrupts.  Right now no one with the inclination has the hardware
to fix it up.

The nasty case we are trying to cover is peripherals that use level
interrupts talking to gpio chips that only do edge triggered ones.
We use the pin as an irq but have to query it as a gpio to discover
if the sneaky chip raised it again without it actually going low.

There are sometimes work arounds involving polling registers on
the device to check if the interrupt is active, but give the bus is
slow, pinging the gpio to find out it's state is often much faster.

So it's a dirty hack for dirty hardware.  Having said that I agree
that it's a niche case and certainly shouldn't be part of any core
code, unless it is done right at the core in the generic interrupt
code and previous discussions suggest that is tricky to say the least!