From: Martin Kepplinger <martink@posteo.de> To: Mark Rutland <mark.rutland@arm.com> Cc: "jic23@kernel.org" <jic23@kernel.org>, "knaack.h@gmx.de" <knaack.h@gmx.de>, "lars@metafoo.de" <lars@metafoo.de>, "pmeerw@pmeerw.net" <pmeerw@pmeerw.net>, "mfuzzey@parkeon.com" <mfuzzey@parkeon.com>, "roberta.dobrescu@gmail.com" <roberta.dobrescu@gmail.com>, "robh+dt@kernel.org" <robh+dt@kernel.org>, Pawel Moll <Pawel.Moll@arm.com>, "ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>, "galak@codeaurora.org" <galak@codeaurora.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "christoph.muellner@theobroma-systems.com" <christoph.muellner@theobroma-systems.com>, Martin Kepplinger <martin.kepplinger@theobroma-systems.com> Subject: Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings Date: Mon, 27 Jul 2015 16:37:48 +0200 [thread overview] Message-ID: <55B6423C.7030301@posteo.de> (raw) In-Reply-To: <20150727142336.GB10821@leverpostej> 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 >>
WARNING: multiple messages have this Message-ID (diff)
From: Martin Kepplinger <martink-1KBjaw7Xf1+zQB+pC5nmwQ@public.gmane.org> To: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> Cc: "jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, "knaack.h-Mmb7MZpHnFY@public.gmane.org" <knaack.h-Mmb7MZpHnFY@public.gmane.org>, "lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org" <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>, "pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org" <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>, "mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org" <mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org>, "roberta.dobrescu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <roberta.dobrescu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>, "ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org" <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, "galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org" <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>, Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> Subject: Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings Date: Mon, 27 Jul 2015 16:37:48 +0200 [thread overview] Message-ID: <55B6423C.7030301@posteo.de> (raw) In-Reply-To: <20150727142336.GB10821@leverpostej> 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-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> >> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> >> --- >> .../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 >>
next prev parent reply other threads:[~2015-07-27 14:39 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-07-27 14:08 [PATCH v5 0/8] iio: mma8452: improve driver and support more chips Martin Kepplinger 2015-07-27 14:08 ` Martin Kepplinger 2015-07-27 14:08 ` [PATCH 1/8] iio: mma8452: refactor for seperating chip specific data Martin Kepplinger 2015-07-27 14:08 ` [PATCH 2/8] iio: mma8452: add support for MMA8453Q accelerometer chip Martin Kepplinger 2015-07-27 14:08 ` [PATCH 3/8] iio: mma8452: add freefall / motion interrupt source Martin Kepplinger 2015-07-27 14:08 ` Martin Kepplinger 2015-07-27 14:08 ` [PATCH 4/8] iio: mma8452: add support for MMA8652FC and MMA8653FC accelerometers Martin Kepplinger 2015-07-27 14:08 ` Martin Kepplinger 2015-07-27 14:08 ` [PATCH 5/8] iio: mma8452: add devicetree binding document Martin Kepplinger 2015-07-27 14:20 ` Mark Rutland 2015-07-27 14:20 ` Mark Rutland 2015-07-27 14:59 ` [PATCHv2 " Martin Kepplinger 2015-07-27 14:59 ` Martin Kepplinger 2015-07-27 14:08 ` [PATCH 6/8] iio: mma8452: add copyright notice comment Martin Kepplinger 2015-07-27 14:08 ` [PATCH 7/8] iio: mma8452: leave sysfs namings to the iio core Martin Kepplinger 2015-07-27 14:08 ` [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings Martin Kepplinger 2015-07-27 14:23 ` Mark Rutland 2015-07-27 14:23 ` Mark Rutland 2015-07-27 14:37 ` Martin Kepplinger [this message] 2015-07-27 14:37 ` Martin Kepplinger 2015-07-27 14:37 ` Martin Kepplinger 2015-07-27 17:33 ` Mark Rutland 2015-07-27 17:33 ` Mark Rutland 2015-07-27 17:33 ` Mark Rutland 2015-07-28 9:11 ` Martin Kepplinger 2015-07-28 9:11 ` Martin Kepplinger 2015-07-28 9:11 ` Martin Kepplinger 2015-07-28 9:28 ` Mark Rutland 2015-07-28 9:28 ` Mark Rutland 2015-07-28 23:12 ` Martin Kepplinger 2015-07-28 23:12 ` Martin Kepplinger 2015-07-28 23:12 ` Martin Kepplinger 2015-08-02 16:24 ` Jonathan Cameron 2015-08-02 16:24 ` Jonathan Cameron 2015-08-02 16:24 ` Jonathan Cameron 2015-07-27 14:56 ` [PATCHv2 " Martin Kepplinger 2015-07-27 14:56 ` Martin Kepplinger 2015-08-08 16:42 ` [PATCH v5 0/8] iio: mma8452: improve driver and support more chips Jonathan Cameron 2015-08-08 16:42 ` Jonathan Cameron -- strict thread matches above, loose matches on Subject: below -- 2015-07-06 12:34 [PATCH v4 0/8] iio:mma8452: " Martin Kepplinger 2015-07-06 12:34 ` [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings Martin Kepplinger 2015-07-06 12:34 ` Martin Kepplinger 2015-07-19 13:47 ` Jonathan Cameron 2015-07-19 13:47 ` Jonathan Cameron 2015-07-19 18:42 ` Martin Kepplinger 2015-07-19 18:42 ` Martin Kepplinger 2015-07-20 8:38 ` Martin Fuzzey 2015-07-20 8:38 ` Martin Fuzzey 2015-07-20 10:01 ` Martin Kepplinger 2015-07-20 10:01 ` Martin Kepplinger
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=55B6423C.7030301@posteo.de \ --to=martink@posteo.de \ --cc=Pawel.Moll@arm.com \ --cc=christoph.muellner@theobroma-systems.com \ --cc=devicetree@vger.kernel.org \ --cc=galak@codeaurora.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=jic23@kernel.org \ --cc=knaack.h@gmx.de \ --cc=lars@metafoo.de \ --cc=linux-iio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=martin.kepplinger@theobroma-systems.com \ --cc=mfuzzey@parkeon.com \ --cc=pmeerw@pmeerw.net \ --cc=roberta.dobrescu@gmail.com \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.