lkml.org 
[lkml]   [2013]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
    On 09/10/2013 09:00 AM, Joel Fernandes wrote:
    > On 07/31/2013 03:35 AM, Javier Martinez Canillas wrote:
    >> On 07/31/2013 01:44 AM, Linus Walleij wrote:
    >>> On Tue, Jul 30, 2013 at 6:30 AM, Grant Likely <grant.likely@linaro.org> wrote:
    >>>> On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
    >>>>> To solve this dilemma, perform an interrupt consistency check
    >>>>> when adding a GPIO chip: if the chip is both gpio-controller and
    >>>>> interrupt-controller, walk all children of the device tree,
    >>>>> check if these in turn reference the interrupt-controller, and
    >>>>> if they do, loop over the interrupts used by that child and
    >>>>> perform gpio_reques() and gpio_direction_input() on these,
    >>>>> making them unreachable from the GPIO side.
    >>>>
    >>>> Ugh, that's pretty awful, and it doesn't actually solve the root
    >>>> problem of the GPIO and IRQ subsystems not cooperating. It's also a
    >>>> very DT-centric solution even though we're going to see the exact same
    >>>> issue on ACPI machines.
    >>>
    >>> The problem is that the patches for OMAP that I applied
    >>> and now have had to revert solves it in an even uglier way,
    >>> leading to breaking boards, as was noticed.
    >>>
    >>> The approach in this patch has the potential to actually
    >>> work without regressing a bunch of boards...
    >>>
    >>> Whether this is a problem in ACPI or not remains to be seen,
    >>> but I'm not sure about that. Device trees allows for a GPIO line
    >>> to be used as an interrupt source and GPIO line orthogonally,
    >>> and that is the root of this problem. Does ACPI have the same
    >>> problem, or does it impose natural restrictions on such use
    >>> cases?
    >>>
    >>
    >> I agree with Linus here. The problem is that GPIO controllers that can work as
    >> IRQ sources are treated in the kernel as if there where two separate controlers
    >> that are rather orthogonal: an irq_chip and a gpio_chip.
    >> But DT allows to use a GPIO line as an IRQ just by using an omap-gpio phandle as
    >> "interrupt-parent".
    >>
    >> So, there should be a place where both irq_chip and gpio_chip has to be related
    >> somehow to properly configure a GPIO (request it and setting it as input) when
    >> used as an IRQ by DT.
    >>
    >> My patch for OMAP used an irq_domain_ops .map function handler to configure the
    >> GPIO when a IRQ was mapped since that seemed to me as the best place to do it.
    >> This worked well in OMAP2+ platforms but unfortunately broke OMAP1 platforms
    >> since they are still using legacy domain mapping thus not call .map.
    >
    > Just wondering- why .map not called for omap1? irq_create_mapping does seem to
    > call -> irq_domain_associate which calls map function. For omap case, GPIO
    > driver does call irq_create_mapping, just like omap2+ no?
    >

    That is what I understood too when writing the patch but I remember someone
    mentioning legacy domain mapping not calling the .map function handler as a
    possible cause for the OMAP1 regression and since Linus decided to revert the
    patches in favor of a more general solution I didn't care to check if that was
    true or not. Now looking at irq_create_mapping() I see that my assumption was
    correct so I don't know what was the bug that caused the OMAP1 regression.

    > Further, if for any reason the .map is not called. Can you not call gpio_request
    > yourself direct in omap_gpio_chip_init function?
    >

    No, since you can't request a GPIO for all GPIO pins in the bank. Users have to
    do it explicitly (or implicitly in the case of GPIO mapped as IRQ in DT).

    > Does it really matter if you call gpio_request from .map or from the chip_init
    > function?
    >

    Yes it does, because in DT the core calls irq_create_of_mapping() ->
    irq_create_mapping() -> .map(). That way only are requested the GPIO pins that
    are mapped as IRQ and not all of them.

    > Also on a different note.. this would call gpio_request for *every* gpio line,
    > but isn't that what your original patch that got reverted was doing in
    > omap_gpio_chip_init:
    >
    > + if (!bank->chip.of_node)
    > + for (j = 0; j < bank->width; j++)
    > + irq_create_mapping(bank->domain, j);
    >

    No it won't. This is only needed for the legacy (non-DT) boot since no one calls
    irq_create_mapping() so it has to be called explicitly.

    And in that case .map will be called but gpio_request() won't since the call is
    made only when bank->chip.of_node is not NULL.

    > Just trying to understand your initial patch better.
    >
    > Regards,
    >
    > -Joel
    >

    Best regards,
    Javier



    \
     
     \ /
      Last update: 2013-09-10 15:41    [W:4.820 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site