From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>,
Chen-Yu Tsai <wens@csie.org>, Jonathan Cameron <jic23@kernel.org>,
linux-iio@vger.kernel.org,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>,
devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCH 1/2] iio: sun4i-lradc: Add binding documentation
Date: Sat, 2 Jul 2016 15:32:43 +0200 [thread overview]
Message-ID: <20160702133243.GB20045@piout.net> (raw)
In-Reply-To: <9b02394e-b38b-c804-780b-e9707c292486@redhat.com>
On 02/07/2016 at 13:45:18 +0200, Hans de Goede wrote :
> Hi,
>
> On 02-07-16 13:02, Maxime Ripard wrote:
> > On Sat, Jul 02, 2016 at 11:32:07AM +0200, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 02-07-16 11:12, Chen-Yu Tsai wrote:
> > > > Hi,
> > > >
> > > > On Sat, Jul 2, 2016 at 5:00 AM, Alexandre Belloni
> > > > <alexandre.belloni@free-electrons.com> wrote:
> > > > > Document the bindings for the Allwinner LRADC.
> > > >
> > > > We already have Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> > > > and I'm pretty sure Hans (CC-ed) argued that this is not a generic ADC
> > > > block.
> > >
> > > Right, this block is used on many tablets and some dev boards to
> > > provide buttons (as in the hid type) and the block is designed for
> > > this purpose, giving an irq when the adc level crosses a certain
> > > threshold.
> > >
> > > Sure it can be used in a more generic way, but that is not its
> > > primary goal.
> >
> > We've always had a different view on this, but it's a detail :)
> >
> > > So any generic purpose adc driver must not break the current
> > > use-case (which is already used in mainline kernel dts files
> > > in plenty of cases).
> >
> > Yep.
> >
> > > I believe that the best way to deal with this is to add an
> > > "allwinner,general-purpose-mode" flag to the existing binding
> > > (as well as document general purpose mode in the existing
> > > binding rather then in a new binding doc).
> > >
> > > That seems to be the right thing to do purely looking at this
> > > from a dt binding pov.
> >
> > There's a way simpler solution: if there's no child nodes, it's meant
> > to be used as an ADC, otherwise, as input.
> >
> > The logic will have to be a bit more complex than that, since there's
> > two channels, and you could only require one for the buttons, leaving
> > the other one available as an ADC.
> >
> > But that doesn't require any new property.
>
> True, if there are no button nodes, then go general-purpose will
> work too.
>
Well, I'd still argue that we need two different compatibles because
encoding the usage of an IP in its compatible string is less than ideal.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: alexandre.belloni@free-electrons.com (Alexandre Belloni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] iio: sun4i-lradc: Add binding documentation
Date: Sat, 2 Jul 2016 15:32:43 +0200 [thread overview]
Message-ID: <20160702133243.GB20045@piout.net> (raw)
In-Reply-To: <9b02394e-b38b-c804-780b-e9707c292486@redhat.com>
On 02/07/2016 at 13:45:18 +0200, Hans de Goede wrote :
> Hi,
>
> On 02-07-16 13:02, Maxime Ripard wrote:
> > On Sat, Jul 02, 2016 at 11:32:07AM +0200, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 02-07-16 11:12, Chen-Yu Tsai wrote:
> > > > Hi,
> > > >
> > > > On Sat, Jul 2, 2016 at 5:00 AM, Alexandre Belloni
> > > > <alexandre.belloni@free-electrons.com> wrote:
> > > > > Document the bindings for the Allwinner LRADC.
> > > >
> > > > We already have Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> > > > and I'm pretty sure Hans (CC-ed) argued that this is not a generic ADC
> > > > block.
> > >
> > > Right, this block is used on many tablets and some dev boards to
> > > provide buttons (as in the hid type) and the block is designed for
> > > this purpose, giving an irq when the adc level crosses a certain
> > > threshold.
> > >
> > > Sure it can be used in a more generic way, but that is not its
> > > primary goal.
> >
> > We've always had a different view on this, but it's a detail :)
> >
> > > So any generic purpose adc driver must not break the current
> > > use-case (which is already used in mainline kernel dts files
> > > in plenty of cases).
> >
> > Yep.
> >
> > > I believe that the best way to deal with this is to add an
> > > "allwinner,general-purpose-mode" flag to the existing binding
> > > (as well as document general purpose mode in the existing
> > > binding rather then in a new binding doc).
> > >
> > > That seems to be the right thing to do purely looking at this
> > > from a dt binding pov.
> >
> > There's a way simpler solution: if there's no child nodes, it's meant
> > to be used as an ADC, otherwise, as input.
> >
> > The logic will have to be a bit more complex than that, since there's
> > two channels, and you could only require one for the buttons, leaving
> > the other one available as an ADC.
> >
> > But that doesn't require any new property.
>
> True, if there are no button nodes, then go general-purpose will
> work too.
>
Well, I'd still argue that we need two different compatibles because
encoding the usage of an IP in its compatible string is less than ideal.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>,
Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
linux-kernel
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 1/2] iio: sun4i-lradc: Add binding documentation
Date: Sat, 2 Jul 2016 15:32:43 +0200 [thread overview]
Message-ID: <20160702133243.GB20045@piout.net> (raw)
In-Reply-To: <9b02394e-b38b-c804-780b-e9707c292486-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 02/07/2016 at 13:45:18 +0200, Hans de Goede wrote :
> Hi,
>
> On 02-07-16 13:02, Maxime Ripard wrote:
> > On Sat, Jul 02, 2016 at 11:32:07AM +0200, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 02-07-16 11:12, Chen-Yu Tsai wrote:
> > > > Hi,
> > > >
> > > > On Sat, Jul 2, 2016 at 5:00 AM, Alexandre Belloni
> > > > <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > > > Document the bindings for the Allwinner LRADC.
> > > >
> > > > We already have Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> > > > and I'm pretty sure Hans (CC-ed) argued that this is not a generic ADC
> > > > block.
> > >
> > > Right, this block is used on many tablets and some dev boards to
> > > provide buttons (as in the hid type) and the block is designed for
> > > this purpose, giving an irq when the adc level crosses a certain
> > > threshold.
> > >
> > > Sure it can be used in a more generic way, but that is not its
> > > primary goal.
> >
> > We've always had a different view on this, but it's a detail :)
> >
> > > So any generic purpose adc driver must not break the current
> > > use-case (which is already used in mainline kernel dts files
> > > in plenty of cases).
> >
> > Yep.
> >
> > > I believe that the best way to deal with this is to add an
> > > "allwinner,general-purpose-mode" flag to the existing binding
> > > (as well as document general purpose mode in the existing
> > > binding rather then in a new binding doc).
> > >
> > > That seems to be the right thing to do purely looking at this
> > > from a dt binding pov.
> >
> > There's a way simpler solution: if there's no child nodes, it's meant
> > to be used as an ADC, otherwise, as input.
> >
> > The logic will have to be a bit more complex than that, since there's
> > two channels, and you could only require one for the buttons, leaving
> > the other one available as an ADC.
> >
> > But that doesn't require any new property.
>
> True, if there are no button nodes, then go general-purpose will
> work too.
>
Well, I'd still argue that we need two different compatibles because
encoding the usage of an IP in its compatible string is less than ideal.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
next prev parent reply other threads:[~2016-07-02 13:32 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-01 21:00 [PATCH 1/2] iio: sun4i-lradc: Add binding documentation Alexandre Belloni
2016-07-01 21:00 ` Alexandre Belloni
2016-07-01 21:00 ` Alexandre Belloni
2016-07-01 21:00 ` [PATCH 2/2] iio: adc: sun4i_lradc: new driver Alexandre Belloni
2016-07-01 21:00 ` Alexandre Belloni
2016-07-03 12:11 ` Jonathan Cameron
2016-07-03 12:11 ` Jonathan Cameron
2016-07-04 16:27 ` Alexandre Belloni
2016-07-04 16:27 ` Alexandre Belloni
2016-07-02 9:12 ` [PATCH 1/2] iio: sun4i-lradc: Add binding documentation Chen-Yu Tsai
2016-07-02 9:12 ` Chen-Yu Tsai
2016-07-02 9:12 ` Chen-Yu Tsai
2016-07-02 9:32 ` Hans de Goede
2016-07-02 9:32 ` Hans de Goede
2016-07-02 11:02 ` Maxime Ripard
2016-07-02 11:02 ` Maxime Ripard
2016-07-02 11:02 ` Maxime Ripard
2016-07-02 11:45 ` Hans de Goede
2016-07-02 11:45 ` Hans de Goede
2016-07-02 11:45 ` Hans de Goede
2016-07-02 13:32 ` Alexandre Belloni [this message]
2016-07-02 13:32 ` Alexandre Belloni
2016-07-02 13:32 ` Alexandre Belloni
2016-07-02 13:46 ` Maxime Ripard
2016-07-02 13:46 ` Maxime Ripard
2016-07-02 13:46 ` Maxime Ripard
2016-07-02 13:35 ` Alexandre Belloni
2016-07-02 13:35 ` Alexandre Belloni
2016-07-02 13:35 ` Alexandre Belloni
2016-07-02 19:46 ` Hans de Goede
2016-07-02 19:46 ` Hans de Goede
2016-07-02 19:46 ` Hans de Goede
2016-07-02 20:43 ` Alexandre Belloni
2016-07-02 20:43 ` Alexandre Belloni
2016-07-02 20:43 ` Alexandre Belloni
2016-07-03 12:01 ` Jonathan Cameron
2016-07-03 12:01 ` 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=20160702133243.GB20045@piout.net \
--to=alexandre.belloni@free-electrons.com \
--cc=devicetree@vger.kernel.org \
--cc=hdegoede@redhat.com \
--cc=jic23@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.ripard@free-electrons.com \
--cc=robh+dt@kernel.org \
--cc=wens@csie.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.