Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1193158 > unrolled thread

[PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings Martin Kepplinger <martink@posteo.de> - 2015-07-27 16:20 +0200
    Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all  pin wirings Mark Rutland <mark.rutland@arm.com> - 2015-07-27 16:30 +0200
      Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all  pin wirings Martin Kepplinger <martink@posteo.de> - 2015-07-27 16:40 +0200
        Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all  pin wirings Martin Kepplinger <martin.kepplinger@theobroma-systems.com> - 2015-07-28 11:20 +0200
          Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all  pin wirings Mark Rutland <mark.rutland@arm.com> - 2015-07-28 11:30 +0200
            Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all  pin wirings Jonathan Cameron <jic23@kernel.org> - 2015-08-02 18:30 +0200
      [PATCHv2 8/8] iio: mma8452: add devicetree property to allow all pin wirings Martin Kepplinger <martink@posteo.de> - 2015-07-27 17:00 +0200

#1193158 — [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a bitmask DT property for users to configure interrupt sources
for INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired. At the moment, only 0xff for using INT2 for all
available interrupt sources is supported. See the devicetree documentation
file for more details.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
 drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 8d98e05..738a430 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -10,6 +10,9 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use_int2: bitmask to choose interrupt sources assumed to be wired to
+    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
+    source) is supported at the moment.
 
 Example:
 
@@ -18,4 +21,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use_int2 = /bits/ 8 <0xff>;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 918ab59..a03836b1 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
 	const struct of_device_id *match;
+	int ret;
+	u8 int2;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
@@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
+		if (int2 == 0xff) {
+			dev_dbg(&client->dev, "use interrupt line INT2\n");
+		} else {
+			dev_dbg(&client->dev, "use interrupt line INT1\n");
+			ret = i2c_smbus_write_byte_data(client,
+							MMA8452_CTRL_REG5,
+							supported_interrupts);
+			if (ret < 0)
+				return ret;
+		}
 
 		ret = i2c_smbus_write_byte_data(client,
 						MMA8452_CTRL_REG4,
-- 
2.1.4

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

[toc] | [next] | [standalone]

#1193189 — Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
> 
> This adds a bitmask DT property for users to configure interrupt sources
> for INT2, if that is the wired interrupt pin for them.

This sounds like configureation rather than a HW property. Why does this
need to be in the DT?

> This is important for everyone to be able to use this driver, no matter
> how their chip is wired. At the moment, only 0xff for using INT2 for all
> available interrupt sources is supported. See the devicetree documentation
> file for more details.
> 
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..738a430 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,9 @@ Optional properties:
>  
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: interrupt mapping for GPIO IRQ
> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> +    source) is supported at the moment.

s/_/-/ in property names, please.

We generally avoid bitmasks in properties, and we also usually exepct a
full cell even if data is smaller. The fact that you expect /bits/ 8 
must be documented here if that's truly necessary.

Thanks,
Mark

>  
>  Example:
>  
> @@ -18,4 +21,5 @@ Example:
>  		reg = <0x1d>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <5 0>;
> +		use_int2 = /bits/ 8 <0xff>;
>  	};
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 918ab59..a03836b1 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>  {
>  	struct mma8452_data *data;
>  	struct iio_dev *indio_dev;
> -	int ret;
>  	const struct of_device_id *match;
> +	int ret;
> +	u8 int2;
>  
>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>  	if (!match) {
> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>  		int enabled_interrupts = MMA8452_INT_TRANS |
>  					 MMA8452_INT_FF_MT;
>  
> -		/* Assume wired to INT1 pin */
> -		ret = i2c_smbus_write_byte_data(client,
> -						MMA8452_CTRL_REG5,
> -						supported_interrupts);
> -		if (ret < 0)
> -			return ret;
> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
> +		if (int2 == 0xff) {
> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
> +		} else {
> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
> +			ret = i2c_smbus_write_byte_data(client,
> +							MMA8452_CTRL_REG5,
> +							supported_interrupts);
> +			if (ret < 0)
> +				return ret;
> +		}
>  
>  		ret = i2c_smbus_write_byte_data(client,
>  						MMA8452_CTRL_REG4,
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193199 — Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a bitmask DT property for users to configure interrupt sources
>> for INT2, if that is the wired interrupt pin for them.
> 
> This sounds like configureation rather than a HW property. Why does this
> need to be in the DT?

It's a hardware property of the board that uses the device. There might
be boards that connect just one of them at random, which is the reason
for this DT property. There also might be exotic users who will want
to use both pins to route different interrupt sources to (not yet
supported, but no problem with such a bitmask).

> 
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>> available interrupt sources is supported. See the devicetree documentation
>> file for more details.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> ---
>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..738a430 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,9 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>> +    source) is supported at the moment.
> 
> s/_/-/ in property names, please.

ok. If I don't do a version 6 really soon, I'll reply with this patch
corrected here.

> 
> We generally avoid bitmasks in properties, and we also usually exepct a
> full cell even if data is smaller. The fact that you expect /bits/ 8 
> must be documented here if that's truly necessary.

It's not truly necessary. It's just a nice fit. There is one 8 bit
(device memory) register that basically could (in the future) be
exposed through this DT property.

For now it's just 0xff or nothing. We only don't want to create an
interface that could restrict us from implementing more in the future
without breaking anything.

> 
> Thanks,
> Mark
> 
>>  
>>  Example:
>>  
>> @@ -18,4 +21,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2 = /bits/ 8 <0xff>;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 918ab59..a03836b1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>>  {
>>  	struct mma8452_data *data;
>>  	struct iio_dev *indio_dev;
>> -	int ret;
>>  	const struct of_device_id *match;
>> +	int ret;
>> +	u8 int2;
>>  
>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>>  	if (!match) {
>> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
>> +		if (int2 == 0xff) {
>> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
>> +		} else {
>> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
>> +			ret = i2c_smbus_write_byte_data(client,
>> +							MMA8452_CTRL_REG5,
>> +							supported_interrupts);
>> +			if (ret < 0)
>> +				return ret;
>> +		}
>>  
>>  		ret = i2c_smbus_write_byte_data(client,
>>  						MMA8452_CTRL_REG4,
>> -- 
>> 2.1.4
>>

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

[toc] | [prev] | [next] | [standalone]

#1193920 — Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

On 2015-07-27 19:33, Mark Rutland wrote:
> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>
>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>> for INT2, if that is the wired interrupt pin for them.
>>>
>>> This sounds like configureation rather than a HW property. Why does this
>>> need to be in the DT?
>>
>> It's a hardware property of the board that uses the device. There might
>> be boards that connect just one of them at random, which is the reason
>> for this DT property. There also might be exotic users who will want
>> to use both pins to route different interrupt sources to (not yet
>> supported, but no problem with such a bitmask).
> 
> Ok, so I'm somewhat confused as to what the hardware looks like and what
> this means.
> 
> Could you elaborate on how INT1 and INT2 are used? It looks like they're
> used as output pins, and so interrupt-names would seem appropriate for
> describing the combination which is wired up.

They are just the chip's two possible interrupt lines for us to get
notified about event.

You build a board, you use one of these 4 chips, wiring up just one of
the 2 interrupt pins. By far most people won't ever need both pins.

DT describes your hardware, right? So you describe how you built your
board (wired the accelerometer chip) with this DT property.

> 
> w.r.t. configuring the choice of output(s), that sounds like a runtime
> decision rather than something which needs to be configured statically.

This won't be useful during runtime. (De)activating events is what you
do in iio sysfs.

Even in the rare case (maybe supported in the future) when you want one
interrupt source on one pin and another source on the other pin, that
describes your hardware. You wire, say, data-ready to Linux and
motion-detection to some strange alarm system. When you change your
hardware (say, use Linux for both pins), I think it would justify
changing a DT property.

Btw, we are talking about very theoretical stuff here. For now (and even
possibly forever) we just don't ever want to break a DT propery we
introduce here, thus the bitmask.

> 
>>>> This is important for everyone to be able to use this driver, no matter
>>>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>>>> available interrupt sources is supported. See the devicetree documentation
>>>> file for more details.
>>>>
>>>> Since this doesn't change the default behaviour, it doesn't break anything
>>>> for existing users.
>>>>
>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>> ---
>>>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> index 8d98e05..738a430 100644
>>>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> @@ -10,6 +10,9 @@ Optional properties:
>>>>  
>>>>    - interrupt-parent: should be the phandle for the interrupt controller
>>>>    - interrupts: interrupt mapping for GPIO IRQ
>>>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>>>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>>>> +    source) is supported at the moment.
>>>
>>> s/_/-/ in property names, please.
>>
>> ok. If I don't do a version 6 really soon, I'll reply with this patch
>> corrected here.
>>
>>>
>>> We generally avoid bitmasks in properties, and we also usually exepct a
>>> full cell even if data is smaller. The fact that you expect /bits/ 8 
>>> must be documented here if that's truly necessary.
>>
>> It's not truly necessary. It's just a nice fit. There is one 8 bit
>> (device memory) register that basically could (in the future) be
>> exposed through this DT property.
>>
>> For now it's just 0xff or nothing. We only don't want to create an
>> interface that could restrict us from implementing more in the future
>> without breaking anything.
> 
> It sounds like you wouldn't need this (at least for now) if you were to
> use interrupt-names to describe whether INT1 and/or INT2 were wired up.
> 
> Thanks,
> Mark.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193928 — Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
> 
> 
> On 2015-07-27 19:33, Mark Rutland wrote:
> > On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> >> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> >>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >>>> For the devices supported by the mma8452 driver, two interrupt pins are
> >>>> available to route the interrupt signals to. By default INT1 is assumed.
> >>>>
> >>>> This adds a bitmask DT property for users to configure interrupt sources
> >>>> for INT2, if that is the wired interrupt pin for them.
> >>>
> >>> This sounds like configureation rather than a HW property. Why does this
> >>> need to be in the DT?
> >>
> >> It's a hardware property of the board that uses the device. There might
> >> be boards that connect just one of them at random, which is the reason
> >> for this DT property. There also might be exotic users who will want
> >> to use both pins to route different interrupt sources to (not yet
> >> supported, but no problem with such a bitmask).
> > 
> > Ok, so I'm somewhat confused as to what the hardware looks like and what
> > this means.
> > 
> > Could you elaborate on how INT1 and INT2 are used? It looks like they're
> > used as output pins, and so interrupt-names would seem appropriate for
> > describing the combination which is wired up.
> 
> They are just the chip's two possible interrupt lines for us to get
> notified about event.

Ok. So that matches my understanding.

> You build a board, you use one of these 4 chips, wiring up just one of
> the 2 interrupt pins. By far most people won't ever need both pins.
> 
> DT describes your hardware, right? So you describe how you built your
> board (wired the accelerometer chip) with this DT property.

Ok.

> > w.r.t. configuring the choice of output(s), that sounds like a runtime
> > decision rather than something which needs to be configured statically.
> 
> This won't be useful during runtime. (De)activating events is what you
> do in iio sysfs.
> 
> Even in the rare case (maybe supported in the future) when you want one
> interrupt source on one pin and another source on the other pin, that
> describes your hardware. You wire, say, data-ready to Linux and
> motion-detection to some strange alarm system. When you change your
> hardware (say, use Linux for both pins), I think it would justify
> changing a DT property.

In that case you would need additional properties anyway.

> Btw, we are talking about very theoretical stuff here. For now (and even
> possibly forever) we just don't ever want to break a DT propery we
> introduce here, thus the bitmask.

I don't think you need the bitmask.

I think all you need is interrupt-names, e.g.

dev1 {
	/* both wired up */
	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
	interrupt-names = "INT1", "INT2";
}

dev2 {
	/* only INT2 wired up */
	interrupts = <&some_ic 3 96>;
	interrupt-names = "INT2";
}

You can figure out which interrupts are wired up by trying to acquire
them by name, then falling back to acquiting an anonymouos interrupt
(assuming it's INT1) to keep compatible with existing DTBs. You can
choose which to use arbitrarily, try to load balance, or whatever you'd
like.

If it's later necessary to route some interrupts to another device,
additional properties can be added to accomodate that. We already know
that the bitmask alone is not sufficient for that case.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198342 — Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

On 29/07/15 00:12, Martin Kepplinger wrote:
> Am 2015-07-28 um 11:28 schrieb Mark Rutland:
>> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>>
>>>
>>> On 2015-07-27 19:33, Mark Rutland wrote:
>>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>>
>>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>>
>>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>>> need to be in the DT?
>>>>>
>>>>> It's a hardware property of the board that uses the device. There might
>>>>> be boards that connect just one of them at random, which is the reason
>>>>> for this DT property. There also might be exotic users who will want
>>>>> to use both pins to route different interrupt sources to (not yet
>>>>> supported, but no problem with such a bitmask).
>>>>
>>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>>> this means.
>>>>
>>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>>> used as output pins, and so interrupt-names would seem appropriate for
>>>> describing the combination which is wired up.
>>>
>>> They are just the chip's two possible interrupt lines for us to get
>>> notified about event.
>>
>> Ok. So that matches my understanding.
>>
>>> You build a board, you use one of these 4 chips, wiring up just one of
>>> the 2 interrupt pins. By far most people won't ever need both pins.
>>>
>>> DT describes your hardware, right? So you describe how you built your
>>> board (wired the accelerometer chip) with this DT property.
>>
>> Ok.
>>
>>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>>> decision rather than something which needs to be configured statically.
>>>
>>> This won't be useful during runtime. (De)activating events is what you
>>> do in iio sysfs.
>>>
>>> Even in the rare case (maybe supported in the future) when you want one
>>> interrupt source on one pin and another source on the other pin, that
>>> describes your hardware. You wire, say, data-ready to Linux and
>>> motion-detection to some strange alarm system. When you change your
>>> hardware (say, use Linux for both pins), I think it would justify
>>> changing a DT property.
>>
>> In that case you would need additional properties anyway.
>>
>>> Btw, we are talking about very theoretical stuff here. For now (and even
>>> possibly forever) we just don't ever want to break a DT propery we
>>> introduce here, thus the bitmask.
>>
>> I don't think you need the bitmask.
>>
>> I think all you need is interrupt-names, e.g.
>>
>> dev1 {
>> 	/* both wired up */
>> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
>> 	interrupt-names = "INT1", "INT2";
>> }
>>
>> dev2 {
>> 	/* only INT2 wired up */
>> 	interrupts = <&some_ic 3 96>;
>> 	interrupt-names = "INT2";
>> }
>>
>> You can figure out which interrupts are wired up by trying to acquire
>> them by name, then falling back to acquiting an anonymouos interrupt
>> (assuming it's INT1) to keep compatible with existing DTBs. You can
>> choose which to use arbitrarily, try to load balance, or whatever you'd
>> like.
>>
>> If it's later necessary to route some interrupts to another device,
>> additional properties can be added to accomodate that. We already know
>> that the bitmask alone is not sufficient for that case.
>>
> 
> Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
> rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
> be happy to write and test this properly in one month from now, if not
> done by somebody until then.
> 
> Until then, since patches 1-7 only introduce a bindings document, they
> shouldn't be problematic for devicetree people.
> 
> 
> So if Jonathan and IIO people find anybody for review, feel free to take
> patches 1-7. In any case, there is direct register access via debugfs to
> at least somehow make the driver work for everybody ;)
> 
> so long, thanks.
>                     martin
Cool, will kick this one into the long grass until you get back most
likely!

Have a good holiday when you get to it!

Jonathan

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

[toc] | [prev] | [next] | [standalone]

#1193215 — [PATCHv2 8/8] iio: mma8452: add devicetree property to allow all pin wirings

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a bitmask DT property for users to configure interrupt sources
for INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired. At the moment, only 0xff for using INT2 for all
available interrupt sources is supported. See the devicetree documentation
file for more details.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
PATCH v2 of the series' 5th version. DT cleanup and a little clearer
documentation.


 .../devicetree/bindings/iio/accel/mma8452.txt        |  3 +++
 drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 57feb16..32f137e 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -13,6 +13,8 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use-int2: To use interrupt pin INT2 instead of INT1 (default), use
+    "/bits/ 8 <0xff>" here. More options might be available in the future.
 
 Example:
 
@@ -21,4 +23,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use-int2 = /bits/ 8 <0xff>;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 918ab59..ab40fa9 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
 	const struct of_device_id *match;
+	int ret;
+	u8 int2;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
@@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		of_property_read_u8(client->dev.of_node, "use-int2", &int2);
+		if (int2 == 0xff) {
+			dev_dbg(&client->dev, "use interrupt line INT2\n");
+		} else {
+			dev_dbg(&client->dev, "use interrupt line INT1\n");
+			ret = i2c_smbus_write_byte_data(client,
+							MMA8452_CTRL_REG5,
+							supported_interrupts);
+			if (ret < 0)
+				return ret;
+		}
 
 		ret = i2c_smbus_write_byte_data(client,
 						MMA8452_CTRL_REG4,
-- 
2.1.4

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web