* [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-14 13:15 Martin Kepplinger
2015-10-14 15:12 ` Lars-Peter Clausen
0 siblings, 1 reply; 9+ messages in thread
From: Martin Kepplinger @ 2015-10-14 13:15 UTC (permalink / raw)
To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, jic23,
knaack.h, lars, pmeerw, mfuzzey
Cc: devicetree, linux-kernel, linux-iio, Martin Kepplinger,
Martin Kepplinger
This change is important in order for everyone to be easily able to use the
driver for one of the supported accelerometer chips!
Until now, the driver blindly assumed that the INT1 interrupt line is wired
on a user's board. But these devices have 2 interrupt lines and can route
their different interrupt sources to one of them. Since only their motion
detection interrupt source is implemented as IIO events, users just use
either one of the pins.
Of course, this also falls back to assuming INT1, so for existing users
nothing will break. The new functionality is described in the bindings doc.
Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
---
changelog:
v3: correctly assign irq if both pins are described in DT. thanks again
v2: don't warn but normally handle if both pins are described in dts
thanks Mark Rutland
v1: initial post
.../devicetree/bindings/iio/accel/mma8452.txt | 6 +++++
drivers/iio/accel/mma8452.c | 26 +++++++++++++++++-----
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index e3c3746..3c10e85 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -7,13 +7,18 @@ Required properties:
* "fsl,mma8453"
* "fsl,mma8652"
* "fsl,mma8653"
+
- reg: the I2C address of the chip
Optional properties:
- interrupt-parent: should be the phandle for the interrupt controller
+
- interrupts: interrupt mapping for GPIO IRQ
+ - interrupt-names: should contain "INT1" and/or "INT2", the accelerometer's
+ interrupt line in use.
+
Example:
mma8453fc@1d {
@@ -21,4 +26,5 @@ Example:
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <5 0>;
+ interrupt-names = "INT2";
};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1eccc2d..6b9b360 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -29,6 +29,7 @@
#include <linux/iio/events.h>
#include <linux/delay.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#define MMA8452_STATUS 0x00
#define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
@@ -1130,13 +1131,26 @@ static int mma8452_probe(struct i2c_client *client,
MMA8452_INT_FF_MT;
int enabled_interrupts = MMA8452_INT_TRANS |
MMA8452_INT_FF_MT;
+ int irq1, irq2;
- /* Assume wired to INT1 pin */
- ret = i2c_smbus_write_byte_data(client,
- MMA8452_CTRL_REG5,
- supported_interrupts);
- if (ret < 0)
- return ret;
+ irq1 = of_irq_get_byname(client->dev.of_node, "INT1");
+ irq2 = of_irq_get_byname(client->dev.of_node, "INT2");
+
+ /* if INT2 is found, use it. Otherwise INT1 */
+ if (!(irq2 > 0 && irq1 < 0)) {
+ ret = i2c_smbus_write_byte_data(client,
+ MMA8452_CTRL_REG5,
+ supported_interrupts);
+ if (ret < 0)
+ return ret;
+
+ if (irq1 > 0)
+ client->irq = irq1;
+ dev_info(&client->dev, "using interrupt line INT1\n");
+ } else {
+ client->irq = irq2;
+ dev_info(&client->dev, "using interrupt line INT2\n");
+ }
ret = i2c_smbus_write_byte_data(client,
MMA8452_CTRL_REG4,
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-14 15:12 ` Lars-Peter Clausen
0 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2015-10-14 15:12 UTC (permalink / raw)
To: Martin Kepplinger, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, jic23, knaack.h, pmeerw, mfuzzey
Cc: devicetree, linux-kernel, linux-iio, Martin Kepplinger
On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
[...]
> + if (irq1 > 0)
> + client->irq = irq1;
You must not overwrite client->irq, that field is manged by the I2C core and
is supposed to be read only for device drivers.
> + dev_info(&client->dev, "using interrupt line INT1\n");
This should probably be a dev_dbg()
> + } else {
> + client->irq = irq2;
> + dev_info(&client->dev, "using interrupt line INT2\n");
Same here
> + }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-14 15:12 ` Lars-Peter Clausen
0 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2015-10-14 15:12 UTC (permalink / raw)
To: Martin Kepplinger, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Martin Kepplinger
On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
[...]
> + if (irq1 > 0)
> + client->irq = irq1;
You must not overwrite client->irq, that field is manged by the I2C core and
is supposed to be read only for device drivers.
> + dev_info(&client->dev, "using interrupt line INT1\n");
This should probably be a dev_dbg()
> + } else {
> + client->irq = irq2;
> + dev_info(&client->dev, "using interrupt line INT2\n");
Same here
> + }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 9:32 ` Martin Kepplinger
0 siblings, 0 replies; 9+ messages in thread
From: Martin Kepplinger @ 2015-10-15 9:32 UTC (permalink / raw)
To: Lars-Peter Clausen, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, jic23, knaack.h, pmeerw, mfuzzey
Cc: devicetree, linux-kernel, linux-iio, Martin Kepplinger
Am 2015-10-14 um 17:12 schrieb Lars-Peter Clausen:
> On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> [...]
>> + if (irq1 > 0)
>> + client->irq = irq1;
>
> You must not overwrite client->irq, that field is manged by the I2C core and
> is supposed to be read only for device drivers.
>
I thought about it again and before I implement it, let me show you:
since interrupt-names would be "invented" anyways ("INT1", "INT2"),
here's an idea for the bindings doc that would be a more long-term
solution to implement:
- interrupts: interrupt mapping for GPIO IRQ
These devices have two interrupt pins called INT1 and INT2 they
can route their different interrupt sources to:
IRQ Name Interrupt Source Wired Pin
-------- ---------------- ---------
DATA_READY_1 data ready INT1
DATA_READY_2 INT2
MOTION_1 motion events INT1
MOTION_2 INT2
INT1 all INT1
INT2 INT2
- interrupt-names: should contain IRQ Names in the order in which they
were supplied in the interrupts property.
Depending on how your chip is wired and what
interrupt sources should be handled by the driver,
choose one IRQ Name per Interrupt source, or
INT1/INT2 for all sources to one pin here.
Example 1:
mma8453fc@1d {
compatible = "fsl,mma8453";
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <5 0>;
interrupt-names = "INT2"; // interrupt pin wired to INT2
};
Example 2:
mma8453fc@1d {
compatible = "fsl,mma8453";
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <4 0>, <5 0>;
/* data ready comes from INT2, motion events from INT1 */
interrupt-names = "DATA_READY_2", "MOTION_1";
};
What do you think?
thanks
martin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 9:32 ` Martin Kepplinger
0 siblings, 0 replies; 9+ messages in thread
From: Martin Kepplinger @ 2015-10-15 9:32 UTC (permalink / raw)
To: Lars-Peter Clausen, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Martin Kepplinger
Am 2015-10-14 um 17:12 schrieb Lars-Peter Clausen:
> On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> [...]
>> + if (irq1 > 0)
>> + client->irq = irq1;
>
> You must not overwrite client->irq, that field is manged by the I2C core and
> is supposed to be read only for device drivers.
>
I thought about it again and before I implement it, let me show you:
since interrupt-names would be "invented" anyways ("INT1", "INT2"),
here's an idea for the bindings doc that would be a more long-term
solution to implement:
- interrupts: interrupt mapping for GPIO IRQ
These devices have two interrupt pins called INT1 and INT2 they
can route their different interrupt sources to:
IRQ Name Interrupt Source Wired Pin
-------- ---------------- ---------
DATA_READY_1 data ready INT1
DATA_READY_2 INT2
MOTION_1 motion events INT1
MOTION_2 INT2
INT1 all INT1
INT2 INT2
- interrupt-names: should contain IRQ Names in the order in which they
were supplied in the interrupts property.
Depending on how your chip is wired and what
interrupt sources should be handled by the driver,
choose one IRQ Name per Interrupt source, or
INT1/INT2 for all sources to one pin here.
Example 1:
mma8453fc@1d {
compatible = "fsl,mma8453";
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <5 0>;
interrupt-names = "INT2"; // interrupt pin wired to INT2
};
Example 2:
mma8453fc@1d {
compatible = "fsl,mma8453";
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <4 0>, <5 0>;
/* data ready comes from INT2, motion events from INT1 */
interrupt-names = "DATA_READY_2", "MOTION_1";
};
What do you think?
thanks
martin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 10:14 ` Mark Rutland
0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2015-10-15 10:14 UTC (permalink / raw)
To: Martin Kepplinger
Cc: Lars-Peter Clausen, robh+dt, pawel.moll, ijc+devicetree, galak,
jic23, knaack.h, pmeerw, mfuzzey, devicetree, linux-kernel,
linux-iio, Martin Kepplinger
On Thu, Oct 15, 2015 at 11:32:59AM +0200, Martin Kepplinger wrote:
> Am 2015-10-14 um 17:12 schrieb Lars-Peter Clausen:
> > On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> > [...]
> >> + if (irq1 > 0)
> >> + client->irq = irq1;
> >
> > You must not overwrite client->irq, that field is manged by the I2C core and
> > is supposed to be read only for device drivers.
> >
>
> I thought about it again and before I implement it, let me show you:
>
> since interrupt-names would be "invented" anyways ("INT1", "INT2"),
> here's an idea for the bindings doc that would be a more long-term
> solution to implement:
>
> - interrupts: interrupt mapping for GPIO IRQ
>
> These devices have two interrupt pins called INT1 and INT2 they
> can route their different interrupt sources to:
This being the case, the binding should only talk about INT1 and INT2.
The names might be "invented", but they describe the physical pins on
the device, and thus describe a physical property of the device.
> IRQ Name Interrupt Source Wired Pin
> -------- ---------------- ---------
> DATA_READY_1 data ready INT1
> DATA_READY_2 INT2
> MOTION_1 motion events INT1
> MOTION_2 INT2
> INT1 all INT1
> INT2 INT2
>
> - interrupt-names: should contain IRQ Names in the order in which they
> were supplied in the interrupts property.
>
> Depending on how your chip is wired and what
> interrupt sources should be handled by the driver,
> choose one IRQ Name per Interrupt source, or
> INT1/INT2 for all sources to one pin here.
The configuration of logical interrupts to those physical pins is a
choice that can be made at runtime, and should not live in the DT. It is
not a property of the hardware.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 10:14 ` Mark Rutland
0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2015-10-15 10:14 UTC (permalink / raw)
To: Martin Kepplinger
Cc: Lars-Peter Clausen, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Martin Kepplinger
On Thu, Oct 15, 2015 at 11:32:59AM +0200, Martin Kepplinger wrote:
> Am 2015-10-14 um 17:12 schrieb Lars-Peter Clausen:
> > On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> > [...]
> >> + if (irq1 > 0)
> >> + client->irq = irq1;
> >
> > You must not overwrite client->irq, that field is manged by the I2C core and
> > is supposed to be read only for device drivers.
> >
>
> I thought about it again and before I implement it, let me show you:
>
> since interrupt-names would be "invented" anyways ("INT1", "INT2"),
> here's an idea for the bindings doc that would be a more long-term
> solution to implement:
>
> - interrupts: interrupt mapping for GPIO IRQ
>
> These devices have two interrupt pins called INT1 and INT2 they
> can route their different interrupt sources to:
This being the case, the binding should only talk about INT1 and INT2.
The names might be "invented", but they describe the physical pins on
the device, and thus describe a physical property of the device.
> IRQ Name Interrupt Source Wired Pin
> -------- ---------------- ---------
> DATA_READY_1 data ready INT1
> DATA_READY_2 INT2
> MOTION_1 motion events INT1
> MOTION_2 INT2
> INT1 all INT1
> INT2 INT2
>
> - interrupt-names: should contain IRQ Names in the order in which they
> were supplied in the interrupts property.
>
> Depending on how your chip is wired and what
> interrupt sources should be handled by the driver,
> choose one IRQ Name per Interrupt source, or
> INT1/INT2 for all sources to one pin here.
The configuration of logical interrupts to those physical pins is a
choice that can be made at runtime, and should not live in the DT. It is
not a property of the hardware.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 10:20 ` Mark Rutland
0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2015-10-15 10:20 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Martin Kepplinger, robh+dt, pawel.moll, ijc+devicetree, galak,
jic23, knaack.h, pmeerw, mfuzzey, devicetree, linux-kernel,
linux-iio, Martin Kepplinger
On Wed, Oct 14, 2015 at 05:12:32PM +0200, Lars-Peter Clausen wrote:
> On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> [...]
> > + if (irq1 > 0)
> > + client->irq = irq1;
>
> You must not overwrite client->irq, that field is manged by the I2C core and
> is supposed to be read only for device drivers.
Ok.
I think for the moment the simlpest thing to do would be to check which
of INT1 and INT2 match client->irq, and use that.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] iio: mma8452: support either of the available interrupt pins
@ 2015-10-15 10:20 ` Mark Rutland
0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2015-10-15 10:20 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Martin Kepplinger, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Martin Kepplinger
On Wed, Oct 14, 2015 at 05:12:32PM +0200, Lars-Peter Clausen wrote:
> On 10/14/2015 03:15 PM, Martin Kepplinger wrote:
> [...]
> > + if (irq1 > 0)
> > + client->irq = irq1;
>
> You must not overwrite client->irq, that field is manged by the I2C core and
> is supposed to be read only for device drivers.
Ok.
I think for the moment the simlpest thing to do would be to check which
of INT1 and INT2 match client->irq, and use that.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-15 10:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-14 13:15 [PATCH v3] iio: mma8452: support either of the available interrupt pins Martin Kepplinger
2015-10-14 15:12 ` Lars-Peter Clausen
2015-10-14 15:12 ` Lars-Peter Clausen
2015-10-15 9:32 ` Martin Kepplinger
2015-10-15 9:32 ` Martin Kepplinger
2015-10-15 10:14 ` Mark Rutland
2015-10-15 10:14 ` Mark Rutland
2015-10-15 10:20 ` Mark Rutland
2015-10-15 10:20 ` Mark Rutland
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.