Messages in this thread | | | Date | Tue, 18 Sep 2012 14:00:45 -0500 | From | Rob Herring <> | Subject | Re: [PATCH] gpio: Describe interrupt-controller binding |
| |
On 09/18/2012 01:45 PM, Thierry Reding wrote: > On Tue, Sep 18, 2012 at 12:15:02PM -0600, Stephen Warren wrote: >> On 09/18/2012 12:06 PM, Thierry Reding wrote: >>> On Tue, Sep 18, 2012 at 08:55:40AM -0600, Stephen Warren wrote: >>>> On 09/18/2012 07:28 AM, Rob Herring wrote: >>>>> On 09/18/2012 03:51 AM, Thierry Reding wrote: >>>>>> In order to use GPIO controllers as interrupt controllers, >>>>>> they need to be marked with the DT interrupt-controller >>>>>> property. This commit adds some documentation about this to >>>>>> the general GPIO binding document. >>>>>> >>>>>> Cc: Linus Walleij <linus.walleij@stericsson.com> Cc: Grant >>>>>> Likely <grant.likely@secretlab.ca> Cc: Rob Herring >>>>>> <rob.herring@calxeda.com> Cc: >>>>>> devicetree-discuss@lists.ozlabs.org Cc: >>>>>> linux-kernel@vger.kernel.org Signed-off-by: Thierry Reding >>>>>> <thierry.reding@avionic-design.de> >>>>> >>>>> Applied for 3.7. >>>>> >>>>> Rob >>>>> >>>>>> --- Documentation/devicetree/bindings/gpio/gpio.txt | 33 >>>>>> +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt >>>>>> b/Documentation/devicetree/bindings/gpio/gpio.txt index >>>>>> 4e16ba4..8d125b0 100644 --- >>>>>> a/Documentation/devicetree/bindings/gpio/gpio.txt +++ >>>>>> b/Documentation/devicetree/bindings/gpio/gpio.txt @@ -75,4 >>>>>> +75,37 @@ Example of two SOC GPIO banks defined as >>>>>> gpio-controller nodes: gpio-controller; }; >>>>>> >>>>>> +If the GPIO controller supports the generation of >>>>>> interrupts, it should +also contain an empty >>>>>> "interrupt-controller" property as well as an >>>>>> +"#interrupt-cells" property. This is required in order for >>>>>> other nodes +to use the GPIO controller as their interrupt >>>>>> parent. >>>> >>>> Surely this is generic information for any interrupt controller, >>>> and hence doesn't belong in the GPIO binding? >>> >>> LinusW requested this in order to avoid having to list these >>> properties in every GPIO controller. >> >> There's no need to list this property in an individual GPIO >> controller's binding either; it's a standard property for any >> interrupt controller of any type. >> >>> I suppose that having it in an extra binding for interrupt >>> controllers might make sense as well, but in that case we should >>> probably provide a reference because the GPIO binding is where >>> people are most likely to look for this information. >> >> Yes, we probably should have a centralized .txt for the base interrupt >> controller properties. I guess we don't today because it's probably so >> old everyone assumes it. > > Since each driver binding still needs to document it and in order to > avoid needless duplication I think having a central location for this > makes a lot of sense. > >> For some other patches I sent, I created >> Documentation/devicetree/bindings/interrupt-controller/ - a file in >> that directory would make sense (bike-shedding: irq.txt, interrupts.txt?) >> >> Yes, each individual GPIO binding (that actually is an interrupt >> controller; some may not be) should probably mention this and refer to >> whatever documents the interrupt controller properties. > > Okay. So how about I add a file interrupts.txt in that directory and put > something like what this patch contains into it? Then I can just add a > reference to the driver binding that the controller can be used as an > interrupt-controller and that a description can be find in this new > document. > >>> There is Documentation/devicetree/bindings/open-pic.txt, which >>> already lists most of this information, so maybe a reference to >>> that document will do just as well? >> >> I think that's just one random interrupt controller. The common/core >> properties probably should be separated out. >> >>>>>> +If #interrupt-cells is 1, the single cell is used to specify >>>>>> the number +of the GPIO that is to be used as an interrupt. >>>>>> + +If #interrupt-cells is 2, the first cell is used to >>>>>> specify the number +of the GPIO that is to be used as an >>>>>> interrupt, whereas the second cell +is used to specify any of >>>>>> the following flags: + - bits[3:0] trigger type and level >>>>>> flags + 1 = low-to-high edge triggered + 2 = >>>>>> high-to-low edge triggered + 4 = active high >>>>>> level-sensitive + 8 = active low level-sensitive >>>> >>>> That certainly shouldn't be in the generic GPIO binding; the >>>> format of the interrupt specifier is determined by the binding >>>> for the individual device that is the interrupt controller. Just >>>> because a device is also a GPIO controller doesn't mean that it >>>> has to conform to a specific format for the interrupt specifier. >>> >>> I think it does make sense to provide a description of the most >>> commonly used variants. The above certainly is what the majority is >>> using and many of those that do not use one of the predefined >>> irq_domain_xlate_*() functions reimplement them with some >>> additional checks or conversions. >> >> OK, but the document can't say "this is how the IRQ specifier is >> formatted", when it clearly isn't generally true. >> >> The document should say that the format of the IRQ specifier is >> entirely determined by the individual binding, but that bindings may >> often choose to re-use the following common format. Each individual >> binding would then need to document whether it did choose to use that >> common format, or whether it instead chose something custom. > > Yes, that makes sense. > > Rob, given the above discussion I think it'd be better if I followed up > with a patch that moves this description into a more generic location > and we removed this patch.
Yes, agreed.
Rob
| |