From: Guenter Roeck <linux@roeck-us.net>
To: NeilBrown <neilb@suse.de>, Mark Rutland <mark.rutland@arm.com>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
"grant.likely@linaro.org" <grant.likely@linaro.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] extcon-gpio: add devicetree support.
Date: Sat, 02 Nov 2013 23:38:49 -0700 [thread overview]
Message-ID: <5275EF79.2000805@roeck-us.net> (raw)
In-Reply-To: <20131102103323.17e208cf@notabene.brown>
On 11/01/2013 04:33 PM, NeilBrown wrote:
> On Fri, 1 Nov 2013 10:16:44 -0700 Mark Rutland <mark.rutland@arm.com> wrote:
>
>> Hi Neil,
>>
>> While I'm not fundamentally opposed to this binding, I have some issues with
>> its current form and would not want to see this version hit mainline.
>>
>
> Thanks for the review.
>
>> On Fri, Nov 01, 2013 at 09:50:05AM +0000, NeilBrown wrote:
>>>
>>> As this device is not vendor specific, I haven't included any "vendor,"
>>> prefixes. For my model I used "regulator-gpio" which takes a similar
>>> approach.
>>>
>>> Signed-off-by: NeilBrown <neilb@suse.de>
>>>
>>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>>> new file mode 100644
>>> index 000000000000..2346b61cc620
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>>> @@ -0,0 +1,26 @@
>>> +* EXTCON detector using GPIO
>>
>> EXTCON is _extremely_ Linux-specific. The binding document needs a description
>> of the class of device it's inteded to describe that does not just refer to
>> Linux internals.
>>
>> I would prefer if we could have a better name for this that was not tied to the
>> Linux driver name. Perhaps "gpio-presence-detector"?
>
> Maybe "cable-presence-detector" as in this case the GPIO is just an
> implementation detail. Which isn't much different from "external-connector"
> which is where "extcon" comes from...
>
> I propose "external-connector" if you don't like "extcon".
>
>
>>
>>> +
>>> +Required Properties:
>>> + - compatible: "extcon-gpio"
>>> + - gpios: gpio line that detects connector
>>> + - interrupts: interrupt generated by that gpio
>>
>> We don't need this. If we need the interrupt a gpio generates, we should ask
>> the gpio controller driver to map the gpio to an interrupt.
>>
>> We have gpiod_to_irq for this in Linux.
>
> The reason I did this was that the pre-existing platform_data wants
> 'irq_flags'. I could have an 'irq-flags' property, but it seems to make more
> sense to use "interrupts" as that already provides a way to pass irq-flags to
> a device.
>
> On reflection though, I cannot imagine why any extcon-gpio would use anything
> other than IRQ_TYPE_EDGE_BOTH. Maybe MyungJoo Ham can explain that???
>
> If there is no need for specifying irq-flags per-platform, the "interrupts"
> property can definitely go.
>
When I tried to add DT support to extcon-pio, I used IRQ_TYPE_EDGE_BOTH.
See [1]. A useful fallback for 'label' - if not specified - may be
np->name, to match LED functionality.
A patch to add 'active low' to the platform data for extcon-gpio is pending
in linux-next. It might make sense to extract this flag from the gpio flags
and add it as well. Again see [1] for an example how this could be implemented.
Guenter
[1] https://lkml.org/lkml/2013/8/30/26
next prev parent reply other threads:[~2013-11-03 6:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-01 9:50 [PATCH] extcon-gpio: add devicetree support NeilBrown
2013-11-01 17:16 ` Mark Rutland
2013-11-01 23:33 ` NeilBrown
2013-11-02 20:43 ` NeilBrown
2013-11-03 6:38 ` Guenter Roeck [this message]
2013-11-04 18:39 ` Mark Brown
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=5275EF79.2000805@roeck-us.net \
--to=linux@roeck-us.net \
--cc=cw00.choi@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=myungjoo.ham@samsung.com \
--cc=neilb@suse.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).