All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Fuzzey <mfuzzey@parkeon.com>
To: Jonathan Cameron <jic23@kernel.org>,
	Martin Kepplinger <martink@posteo.de>,
	knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
	roberta.dobrescu@gmail.com, robh+dt@kernel.org,
	pawel.moll@arm.com, mark.rutland@arm.com,
	ijc+devicetree@hellion.org.uk, galak@codeaurora.org
Cc: devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	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, 20 Jul 2015 10:38:43 +0200	[thread overview]
Message-ID: <55ACB393.8000603@parkeon.com> (raw)
In-Reply-To: <55ABAA8B.3080706@kernel.org>

On 19/07/15 15:47, Jonathan Cameron wrote:
> On 06/07/15 13:34, 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 simple boolean DT property, for users to configure it 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.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
A remark about this.

The hardware allows each of the interrupt sources (threshold, freefall, 
data available, wakeup, ...) to be individiually mapped to one of the 
two available interrupt pins.
This patch only allows one pin or the other one to be selected for all 
the sources.

Granted this will certainly cover 99% of use cases (even if both are 
wired that means we can use either).
However there is one possible case not covered which is when one of the 
interrupt signals is connected to some other circuitry (alarm, 
power,...) in addition to being used as an interrupt to the CPU.

Normally for code I'd just say let's cross that bridge when we come to 
it given that it's pretty unlikely.
However since this affects the DT binding and they can be awkward to 
change while keeping compatibility maybe it would be better to use a 
bitmask DT property rather than a boolean to actually describe the full 
hardware capabilities and keep that door open?

Of course, actually supporting arbitary mappings would be a bit more 
work in the driver (both irqs would need to requested...).
However it would be possible to use a bitmask in the DT but, for the 
moment, only accept all zeros or all ones, leaving the rest for a future 
driver update if and when it is needed.

I don't have a strong objection to doing it the current way, it's 
certainly the simplest way and it's likely to be all we ever need just 
pointing out a possible shortcoming.

I think It would be good to have the DT maintainers view on this.

>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
I'll try to give it a spin this week. Can't promise though


Regards,

Martin

WARNING: multiple messages have this Message-ID (diff)
From: Martin Fuzzey <mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org>
To: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Martin Kepplinger
	<martink-1KBjaw7Xf1+zQB+pC5nmwQ@public.gmane.org>,
	knaack.h-Mmb7MZpHnFY@public.gmane.org,
	lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org,
	pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org,
	roberta.dobrescu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@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, 20 Jul 2015 10:38:43 +0200	[thread overview]
Message-ID: <55ACB393.8000603@parkeon.com> (raw)
In-Reply-To: <55ABAA8B.3080706-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On 19/07/15 15:47, Jonathan Cameron wrote:
> On 06/07/15 13:34, 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 simple boolean DT property, for users to configure it 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.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
A remark about this.

The hardware allows each of the interrupt sources (threshold, freefall, 
data available, wakeup, ...) to be individiually mapped to one of the 
two available interrupt pins.
This patch only allows one pin or the other one to be selected for all 
the sources.

Granted this will certainly cover 99% of use cases (even if both are 
wired that means we can use either).
However there is one possible case not covered which is when one of the 
interrupt signals is connected to some other circuitry (alarm, 
power,...) in addition to being used as an interrupt to the CPU.

Normally for code I'd just say let's cross that bridge when we come to 
it given that it's pretty unlikely.
However since this affects the DT binding and they can be awkward to 
change while keeping compatibility maybe it would be better to use a 
bitmask DT property rather than a boolean to actually describe the full 
hardware capabilities and keep that door open?

Of course, actually supporting arbitary mappings would be a bit more 
work in the driver (both irqs would need to requested...).
However it would be possible to use a bitmask in the DT but, for the 
moment, only accept all zeros or all ones, leaving the rest for a future 
driver update if and when it is needed.

I don't have a strong objection to doing it the current way, it's 
certainly the simplest way and it's likely to be all we ever need just 
pointing out a possible shortcoming.

I think It would be good to have the DT maintainers view on this.

>> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
I'll try to give it a spin this week. Can't promise though


Regards,

Martin

  parent reply	other threads:[~2015-07-20  8:38 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-06 12:34 [PATCH v4 0/8] iio:mma8452: improve driver and support more chips Martin Kepplinger
2015-07-06 12:34 ` [PATCH 1/8] iio: mma8452: refactor for seperating chip specific data Martin Kepplinger
2015-07-19 13:42   ` Jonathan Cameron
2015-07-19 13:42     ` Jonathan Cameron
2015-07-06 12:34 ` [PATCH 2/8] iio: mma8452: add support for MMA8453Q accelerometer chip Martin Kepplinger
2015-07-06 12:34   ` Martin Kepplinger
2015-07-19 13:43   ` Jonathan Cameron
2015-07-19 13:43     ` Jonathan Cameron
2015-07-06 12:34 ` [PATCH 3/8] iio: mma8452: add freefall / motion interrupt source Martin Kepplinger
2015-07-06 12:34   ` Martin Kepplinger
2015-07-06 12:47   ` Martin Kepplinger
2015-07-06 12:47     ` Martin Kepplinger
2015-07-19 13:45   ` Jonathan Cameron
2015-07-06 12:34 ` [PATCH 4/8] iio: mma8452: add support for MMA8652FC and MMA8653FC accelerometers Martin Kepplinger
2015-07-06 12:34 ` [PATCH 5/8] iio: mma8452: add devicetree binding document Martin Kepplinger
2015-07-06 12:34 ` [PATCH 6/8] iio: mma8452: add copyright notice comment Martin Kepplinger
2015-07-06 12:34   ` Martin Kepplinger
2015-07-06 12:34 ` [PATCH 7/8] iio: mma8452: leave sysfs namings to the iio core 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 [this message]
2015-07-20  8:38       ` Martin Fuzzey
2015-07-20 10:01       ` Martin Kepplinger
2015-07-20 10:01         ` Martin Kepplinger
2015-07-09  8:51 ` [PATCH v4 0/8] iio:mma8452: improve driver and support more chips Martin Kepplinger
2015-07-09  8:51   ` Martin Kepplinger
2015-07-13  8:07   ` Martin Kepplinger
2015-07-13  9:22     ` Jonathan Cameron
  -- strict thread matches above, loose matches on Subject: below --
2015-07-27 14:08 [PATCH v5 0/8] iio: mma8452: " 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: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-28  9:11         ` Martin Kepplinger
2015-07-28  9:11           ` Martin Kepplinger
2015-07-28  9:28           ` Mark Rutland
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

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=55ACB393.8000603@parkeon.com \
    --to=mfuzzey@parkeon.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=martink@posteo.de \
    --cc=pawel.moll@arm.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: link
Be 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.