[PATCH] I2C: add the source of the IRQ in struct i2c_client

From: Benjamin Tissoires
Date: Wed Jan 04 2017 - 12:26:54 EST


With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
not alert"), the IRQ provided in struct i2c_client might be assigned while
it has not been explicitly declared by either the platform information
or OF or ACPI.
Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
not to trigger a different behavior (exposing /dev/freefall in this case).

Provide a way for others to know who set the IRQ and so they can behave
accordingly.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
---
drivers/i2c/i2c-core.c | 7 +++++++
include/linux/i2c.h | 11 +++++++++++
2 files changed, 18 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396..226c75d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
irq = of_irq_get_byname(dev->of_node, "irq");
if (irq == -EINVAL || irq == -ENODATA)
irq = of_irq_get(dev->of_node, 0);
+ if (irq > 0)
+ client->irq_source = I2C_IRQ_SOURCE_OF;
} else if (ACPI_COMPANION(dev)) {
irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
+ if (irq > 0)
+ client->irq_source = I2C_IRQ_SOURCE_ACPI;
}
if (irq == -EPROBE_DEFER)
return irq;
@@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
if (irq < 0) {
dev_dbg(dev, "Using Host Notify IRQ\n");
irq = i2c_smbus_host_notify_to_irq(client);
+ if (irq > 0)
+ client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
}
if (irq < 0)
irq = 0;
@@ -1317,6 +1323,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_source = I2C_IRQ_SOURCE_PLATFORM;

strlcpy(client->name, info->type, sizeof(client->name));

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c5..7d0368d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -213,6 +213,13 @@ struct i2c_driver {
};
#define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)

+enum i2c_irq_source {
+ I2C_IRQ_SOURCE_PLATFORM,
+ I2C_IRQ_SOURCE_OF,
+ I2C_IRQ_SOURCE_ACPI,
+ I2C_IRQ_SOURCE_HOST_NOTIFY,
+};
+
/**
* struct i2c_client - represent an I2C slave device
* @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
@@ -227,6 +234,9 @@ struct i2c_driver {
* userspace_devices list
* @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
* calls it to pass on slave events to the slave driver.
+ * @irq_source: Enum which provides the source of the IRQ. Useful to know
+ * if the IRQ was issued from Host Notify or if it was provided by an other
+ * component.
*
* An i2c_client identifies a single device (i.e. chip) connected to an
* i2c bus. The behaviour exposed to Linux is defined by the driver
@@ -245,6 +255,7 @@ struct i2c_client {
#if IS_ENABLED(CONFIG_I2C_SLAVE)
i2c_slave_cb_t slave_cb; /* callback for slave mode */
#endif
+ enum i2c_irq_source irq_source; /* which component assigned the irq */
};
#define to_i2c_client(d) container_of(d, struct i2c_client, dev)

--
2.9.3
---

Dmitry, Wolfram, Jean, would this be acceptable for you?

>
> I would rather introduce new IRQ property in both DTS and lis3lv02d
> platform data (e.g. irq-freefall) which will unambiguously identify
> freefall IRQ.

Sigh. You asked above to not break existing DT bindings, so I don't
understand why you suddenly want to change every bindings by introducing
a change which breaks backward compatibility.

>
> But I do not think it should be part of my Dell patch. It is
> independent fix for lis3lv02d driver for bug which was introduced by
> commit 4d5538f5882a.

It's not part of the dell patch, but it can be part of the series you
are about to send. Please remember that the current state is that there
is no breakage (only a spurious /dev/freefall which you are not happy
about). The only breakage is with your patch, so it doesn't hurt to have
both sent together so you can control all the requirements at once.
So please incorporate my patch in your series if Wolfram, Jean and Dmitry
agree, and also add the fix in lis3lv02d_i2c to make use of the IRQ source.

Cheers,
Benjamin

>
> >
> > Cheers,
> > Benjamin
> >
> > > >
> > > > > But given that they are DT and not
> > > > > ACPI, this won't conflict with dell-smo8800, so there won't be any
> > > > > errors, just a dangling unused device node.
> > > >
> > > > Yes, for upcoming Dell in this patch (with IRQ -1) it is not a problem.
> > > >
> > > > > This approach is IMO the best if you want to have this in the kernel.
> > > > >
> > > > > Cheers,
> > > > > Benjamin
> > > >
> > >
> > > --
> > > Pali Rohár
> > > pali.rohar@xxxxxxxxx
>
> --
> Pali Rohár
> pali.rohar@xxxxxxxxx