Re: [PATCH v4 1/2] gpio/omap: don't create an IRQ mapping for every GPIO on DT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alexander,

On Mon, Jul 29, 2013 at 8:41 AM, Alexander Holler <holler@xxxxxxxxxxxxx> wrote:
> Am 28.07.2013 21:06, schrieb Javier Martinez Canillas:
>
>> On Sun, Jul 28, 2013 at 8:22 PM, Linus Walleij <linus.walleij@xxxxxxxxxx>
>> wrote:
>>>
>>> On Sun, Jul 28, 2013 at 7:33 PM, Javier Martinez Canillas
>>> <martinez.javier@xxxxxxxxx> wrote:
>>>
>>>> According to Documentation/devicetree/bindings/mmc/mmc.txt:
>>>>
>>>> cd-gpios: Specify GPIOs for card detection, see gpio binding
>>>>
>>>> So it just says that it is a GPIO for card detection and not an IRQ so
>>>> this assumption comes from either the omap_hsmmc driver or Alexander'
>>>> DTS is missing something like:
>>>>
>>>>                  interrupt-parent = <&gpio6>;
>>>>                  interrupts = <16 8>;
>
>
> What do the values 16 and 8 mean here? GPIO numbers?
> And where do I have to place that?
>

If you look at Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
for the two cells interrupt controllers definition:

  b) two cells
  ------------
  The #interrupt-cells property is set to 2 and the first cell defines the
  index of the interrupt within the controller, while 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

So, the first cell in this example means the 16th GPIO in the omap
gpio6 controller using the edge/level flag IRQ_TYPE_LEVEL_LOW.

> I've now tried the following:
>
> --
> &mmc1 {
>         vmmc-supply = <&vmmcsd_fixed>;
>         status = "okay";
>         pinctrl-names = "default";
>         pinctrl-0 = <&mmc1_pins>; /* pinmux for GPIO0__6 */
>         interrupt-parent = <&gpio0>;
>         interrupts = <6>;
>
>         cd-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
>         cd-inverted;
> };
> --
> (the gpio which should be used for the IRQ is GPIO0_6)
>
> The result was that the gpio_request() failed with -EBUSY.
> Then I've commented out that, because it isn't necessary anymore as the
> gpio should be set as input automatically (as I've understood the commit
> msg).
>

Indeed, drivers shouldn't explicitly call gpio_request() when booting
with DT. This only made sense with legacy boot. So maybe the
omap_hsmmc driver has not been completely converted to be used with DT
booting when using the "cd-gpios" property?

> The result was
>
> [    1.397100] genirq: Flags mismatch irq 144. 00002003 (mmc0) vs.
> 00000000 (mmc0)
>
> and request_threaded_irq() returned with -EBUSY.

By looking at kernel/irq/manage.c, this error comes from __setup_irq()
because the same interrupt line is tried to be shared by two users
with different trigger (edge/level) type flags. Don't know what is
triggering that with your DTS though.

>
>
> To stop that discussion about some "non-standard" dts I'm using (I wonder
> where the standard is), I try to formulate a clear question:
>
> If a driver uses
> --
> irq = gpio_to_irq(some_gpio_number);
> /*
> gpio_request();
> gpio_direction_input();
> */
> request_threaded_irq(irq);
> --
>
> How should the dts or the driver be changed that this works with 3.11-rc2?
>

Drivers shouldn't do that IMHO, they should just use IRQ (virtual)
numbers and request them. Drivers shouldn't care whether it is a GPIO
line acting as an IRQ number or a real IRQ from an interrupt
controller.

My understanding is that defining:

interrupt-parent = <&phandle>;
interrupts = <index type_flags>;

*should* be enough to make the sequence you are mentioning to work.
Now, you said that this breaks your DTS when using the "cd-gpios"
property with the omap hsmmc driver. Unfortunately I'm not familiar
with neither the omap hsmmc driver nor the mmc "cd-gpios" property so
I can't be of too much help here. I guess Balaji can take a step
forward here and shed some light on this.

My guess is that it was working on your setup because the omap hsmmc
driver is handling GPIO-IRQ in the same way that board files do and it
was not even described in your DTS because support for GPIO-IRQ was
not really working for DT in OMAP (an explicit call to gpio_request
had to be made anyways).

So, I would like to first check if this really is a regression (bug)
introduced by these changes or is an issue with the omap hsmmc driver,
how it handles the "cd-gpios" binding or your DTS definition and
reverting the patch-set will just hide the real problem.

As I said, I don't mind if these patches are reverted although many
people (Jon, Santosh, Grant and myself) think that this is the right
approach and reverting them will break DTS for OMAP boards that are
already in mainline.

Yes, it is a PITA that things get broken but is the price of progress
I think, I'll prefer fixing them than just blindly reverting patches
because they show hidden issues. If I'm proven to be wrong I'll be
happy if these patches get reverted though.

> Regards,
>
> Alexander Holler

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux