Re: RT and omap-gpio irqchip with DeviceTree
On Tue, Jul 2, 2013 at 4:31 PM, Torben Hohn wrote:
> On Tue, Jul 02, 2013 at 02:15:46PM +0200, Javier Martinez Canillas wrote:
>> + Grant, Linus W and Jean-Christophe.
>>
>> On Tue, Jul 2, 2013 at 1:44 PM, Torben Hohn wrote:
>> >
>> > Hi..
>> >
>> > I am running into Problems with a network adapter IRQ connected to an
>> > omap-gpio pin.
>> >
>> > omap-gpio expects gpio-request() to be called before i can use the pin.
>> > But this is abstracted via the DeviceTree bindings.
>> >
>> > I see 8d4c277e185c31359cf70573d8b0351fb7dd0dfe in mainline.
>> > This one just puts a warning into the exact place, i am dealing with.
>> > But i need to make this work, instead of bailing out.
>> >
>>
>> This has been discussed extensively on the linux-omap mailing list and
>> the agreement is that it has to be handled by the IRQ core. So when a
>> GPIO line is mapped in the IRQ domain with irq_create_of_mapping(),
>> the core has to take care to request the GPIO and configure it as
>> input.
>
> Can you give me a pointer to this discussion please ?
> Because i fail to understand why you just dont save the irq_type in some
> private data structure, and then use chip->irq_enable() or something.
>
>
Yes, it starts here [1] but it the thread is 90+ mails long.
>>
>> But until we have this general solution we have to do it on a per irq
>> chip driver basis and the less hack-ish solution is to have a custom
>> .map function handler that request the GPIO used as IRQ.
>>
>> There is already a patch [1] queued in Linus Walleij linux-gpio tree
>> [2] for-next branch that implements this for OMAP GPIO. It would be
>> great if you can test it and give feedback.
>
> Thanks for the pointer.
> The kernel i am working on is based on 3.8.x (this is before the
> irq_domain change to linear)
>
> First i only backported: [1] and [2]
>
> With these i see big streams like these:
> [1.554809] omap_gpio gpio.9: Could not request GPIO-256
> [1.560394] omap_gpio gpio.9: Could not request GPIO-255
> [1.566009] omap_gpio gpio.9: Could not request GPIO-254
> [1.571594] omap_gpio gpio.9: Could not request GPIO-253
> [1.577178] omap_gpio gpio.9: Could not request GPIO-252
> [1.582794] omap_gpio gpio.9: Could not request GPIO-251
>
> This is basically coming from irq_domain_add_legacy() which calls
> ops->map(domain, irq, hwirq) before the gpio chip is actually added.
>
> I fixed it by also taking [3].
>
Yes, sorry for not mentioning that it has as a dependency the legacy
to linear domain mapping change. I didn't mentioned since I thought
that you were basing your work on mainline.
> However, OMAP1 is still calling irq_domain_add_legacy(). This would
> probably break with DeviceTree.
Indeed, OMAP1 doesn't support DT. In fact if you are interested in
OMAP1, then you need to cherry-pick another patch from mainline since
linear domain mapping does not work with OMAP1 because OMAP1 does not
support SPARSE_IRQ. That's why the change from legacy to linear domain
mapping was partially reverted for OMAP1 in mainline commit 397eada9
("gpio/omap: don't use linear domain mapping for OMAP1")
> But since omap1 doesnt use DeviceTree, this is probably a non-issue.
>
Yes, the GPIO is only requested on the .map function handler if
bank->chip.of_node is not NULL so it doesn't have a side effect on
platforms that don't support DT.
After all, this issue has only been detected once we migrated to DT
since for legacy booting is the platform code the responsible to do
this setup (e.g: configuring the OMAP pin mux, request the GPIO,
configure it as input, etc).
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=570c4bb53366157fa076922d0fc7e7adfd81cf42
> [2]
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=50fc1d067d9f4b6c99717b91c1618075f859
> [3]
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=ede4d7a5b9835510fd1f724367f68d2fa4128453
>
>
> --
> Mit freundlichen Grüßen
> Torben Hohn
>
> Linutronix GmbH
>
> Standort: Bremen
>
> Phone: +49 421 5650 2310 ; Fax.: +49 7556 919 886
> mailto: torb...@linutronix.de
> Firmensitz / Registered Office: D-88690 Uhldingen, Auf dem Berg 3
> Registergericht / Local District Court: Freiburg i. Br., HRB Nr. / Trade
> register no.: 700 806;
>
> Geschäftsführer / Managing Directors: Heinz Egger, Thomas Gleixner
Best regards,
Javier
[1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85544.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RT and omap-gpio irqchip with DeviceTree
On Tue, Jul 02, 2013 at 02:15:46PM +0200, Javier Martinez Canillas wrote: > + Grant, Linus W and Jean-Christophe. > > On Tue, Jul 2, 2013 at 1:44 PM, Torben Hohn wrote: > > > > Hi.. > > > > I am running into Problems with a network adapter IRQ connected to an > > omap-gpio pin. > > > > omap-gpio expects gpio-request() to be called before i can use the pin. > > But this is abstracted via the DeviceTree bindings. > > > > I see 8d4c277e185c31359cf70573d8b0351fb7dd0dfe in mainline. > > This one just puts a warning into the exact place, i am dealing with. > > But i need to make this work, instead of bailing out. > > > > This has been discussed extensively on the linux-omap mailing list and > the agreement is that it has to be handled by the IRQ core. So when a > GPIO line is mapped in the IRQ domain with irq_create_of_mapping(), > the core has to take care to request the GPIO and configure it as > input. Can you give me a pointer to this discussion please ? Because i fail to understand why you just dont save the irq_type in some private data structure, and then use chip->irq_enable() or something. > > But until we have this general solution we have to do it on a per irq > chip driver basis and the less hack-ish solution is to have a custom > .map function handler that request the GPIO used as IRQ. > > There is already a patch [1] queued in Linus Walleij linux-gpio tree > [2] for-next branch that implements this for OMAP GPIO. It would be > great if you can test it and give feedback. Thanks for the pointer. The kernel i am working on is based on 3.8.x (this is before the irq_domain change to linear) First i only backported: [1] and [2] With these i see big streams like these: [1.554809] omap_gpio gpio.9: Could not request GPIO-256 [1.560394] omap_gpio gpio.9: Could not request GPIO-255 [1.566009] omap_gpio gpio.9: Could not request GPIO-254 [1.571594] omap_gpio gpio.9: Could not request GPIO-253 [1.577178] omap_gpio gpio.9: Could not request GPIO-252 [1.582794] omap_gpio gpio.9: Could not request GPIO-251 This is basically coming from irq_domain_add_legacy() which calls ops->map(domain, irq, hwirq) before the gpio chip is actually added. I fixed it by also taking [3]. However, OMAP1 is still calling irq_domain_add_legacy(). This would probably break with DeviceTree. But since omap1 doesnt use DeviceTree, this is probably a non-issue. [1] https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=570c4bb53366157fa076922d0fc7e7adfd81cf42 [2] https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=50fc1d067d9f4b6c99717b91c1618075f859 [3] https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=ede4d7a5b9835510fd1f724367f68d2fa4128453 -- Mit freundlichen Grüßen Torben Hohn Linutronix GmbH Standort: Bremen Phone: +49 421 5650 2310 ; Fax.: +49 7556 919 886 mailto: torb...@linutronix.de Firmensitz / Registered Office: D-88690 Uhldingen, Auf dem Berg 3 Registergericht / Local District Court: Freiburg i. Br., HRB Nr. / Trade register no.: 700 806; Geschäftsführer / Managing Directors: Heinz Egger, Thomas Gleixner signature.asc Description: Digital signature
Re: RT and omap-gpio irqchip with DeviceTree
+ Grant, Linus W and Jean-Christophe.
On Tue, Jul 2, 2013 at 1:44 PM, Torben Hohn wrote:
>
> Hi..
>
> I am running into Problems with a network adapter IRQ connected to an
> omap-gpio pin.
>
> omap-gpio expects gpio-request() to be called before i can use the pin.
> But this is abstracted via the DeviceTree bindings.
>
> I see 8d4c277e185c31359cf70573d8b0351fb7dd0dfe in mainline.
> This one just puts a warning into the exact place, i am dealing with.
> But i need to make this work, instead of bailing out.
>
This has been discussed extensively on the linux-omap mailing list and
the agreement is that it has to be handled by the IRQ core. So when a
GPIO line is mapped in the IRQ domain with irq_create_of_mapping(),
the core has to take care to request the GPIO and configure it as
input.
But until we have this general solution we have to do it on a per irq
chip driver basis and the less hack-ish solution is to have a custom
.map function handler that request the GPIO used as IRQ.
There is already a patch [1] queued in Linus Walleij linux-gpio tree
[2] for-next branch that implements this for OMAP GPIO. It would be
great if you can test it and give feedback.
> I have this fix in place, instead:
> ---
> commit 4ca17be8e7e24863cb98f440992fd89034aa34f5
> Author: Torben Hohn
> Date: Fri Jun 21 17:34:24 2013 +0200
>
> gpio omap: Fix for DT calling into gpio_irq_type without request_gpio()
>
> gpio_irq_type() requires that the GPIO is requested, do that.
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index f1fbedb2..57d956c 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -87,6 +87,8 @@ struct gpio_bank {
> #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
> #define GPIO_MOD_CTRL_BIT BIT(0)
>
> +static int omap_gpio_request(struct gpio_chip *chip, unsigned offset);
> +
> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
> {
> return gpio_irq - bank->irq_base + bank->chip.base;
> @@ -429,6 +431,9 @@ static int gpio_irq_type(struct irq_data *d, unsigned
> type)
> if (!gpio)
> gpio = irq_to_gpio(bank, d->irq);
>
> + if (!bank->mod_usage)
> + omap_gpio_request( &bank->chip, gpio );
> +
> if (type & ~IRQ_TYPE_SENSE_MASK)
> return -EINVAL;
>
>
> -
>
>
> But now with RT i am running into a Problem, because gpio_irq_type() is called
> under a raw_spinlock() But its calling pm_runtime_suspend() which carries a
> normal spinlock.
>
>
> -
> /**
> * irq_set_type - set the irq trigger type for an irq
> * @irq: irq number
> * @type: IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
> */
> int irq_set_irq_type(unsigned int irq, unsigned int type)
> {
> unsigned long flags;
> struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
> IRQ_GET_DESC_CHECK_GLOBAL);
> int ret = 0;
>
> if (!desc)
> return -EINVAL;
>
> type &= IRQ_TYPE_SENSE_MASK;
> ret = __irq_set_trigger(desc, irq, type);
> irq_put_desc_busunlock(desc, flags);
> return ret;
> }
> -
>
> I think, i need a normal hook in the irq_chip, which "prepares" the irq, but
> i dont see one.
>
>
> --
> Mit freundlichen Grüßen
> Torben Hohn
>
> Linutronix GmbH
>
> Standort: Bremen
>
> Phone: +49 421 5650 2310 ; Fax.: +49 7556 919 886
> mailto: torb...@linutronix.de
> Firmensitz / Registered Office: D-88690 Uhldingen, Auf dem Berg 3
> Registergericht / Local District Court: Freiburg i. Br., HRB Nr. / Trade
> register no.: 700 806;
>
> Geschäftsführer / Managing Directors: Heinz Egger, Thomas Gleixner
Thanks a lot and best regards,
Javier
[1]:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=570c4bb53366157fa076922d0fc7e7adfd81cf42
[2]: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
