linux-kernel - Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Sep 2013 12:11:25 +0200
From:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
To:	Alexander Holler <holler@...oftware.de>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Grant Likely <grant.likely@...aro.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Linux-OMAP <linux-omap@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Enric Balletbo i Serra <eballetbo@...il.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Kevin Hilman <khilman@...aro.org>,
	Balaji T K <balajitk@...com>,
	Tony Lindgren <tony@...mide.com>,
	Jon Hunter <jgchunter@...il.com>
Subject: Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs

On 09/12/2013 10:55 AM, Alexander Holler wrote:
> Am 11.09.2013 19:42, schrieb Alexander Holler:
>> Am 11.09.2013 18:14, schrieb Javier Martinez Canillas:
> 
>>> So for example in an OMAP board DT you can define something like this:
>>>
>>> ethernet@5,0 {
>>>          compatible = "smsc,lan9221", "smsc,lan9115";
>>>          interrupt-parent = <&gpio6>;
>>>          interrupts = <16 8>;
>>> };
>>>
>>> Since each OMAP GPIO bank has 32 GPIO pins, then what you are defining
>>> is that
>>> the GPIO 176 (5 * 32 + 16) will be mapped as the IRQ line for the
>>> ethernet
>>> controller.
> 
> By the way, how do you define two GPIOs/IRQs from different
> gpio-banks/irq-controllers wuth that scheme?
> 

That is indeed a very good question and I don't have a definite answer.

> Would that be like below?
> 
>  ethernet@5,0 {
>           compatible = "smsc,lan9221", "smsc,lan9115";
>           interrupt-parent = <&gpio6>;
>           interrupts = <16 8>;
>           interrupt-parent = <&gpio7>;
>           interrupts = <1 IRQF_TRIGGER_FALLING>; /* GPIO7_1 */
>  };
>

I just looked at
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt and it
doesn't mention that use-case (same device using two different interrupts from
two different interrupt-controller).

So I went and look at the source in drivers/of/irq.c and noticed that the
"interrupts" property and its "interrupt-parent" is parsed by the
of_irq_map_one() function.

/**
 * of_irq_map_one - Resolve an interrupt for a device
 * @device: the device whose interrupt is to be resolved
 * @index: index of the interrupt to resolve
 * @out_irq: structure of_irq filled by this function
 *
 * This function resolves an interrupt, walking the tree, for a given
 * device-tree node. It's the high level pendant to of_irq_map_raw().
 */
int of_irq_map_one(struct device_node *device, int index, struct of_irq *out_irq)
{
        struct device_node *p;
        ...
        /* Get the interrupts property */
        intspec = of_get_property(device, "interrupts", &intlen);
        ...
        /* Look for the interrupt parent. */
        p = of_irq_find_parent(device);
        ...
}

/**
 * of_irq_find_parent - Given a device node, find its interrupt parent node
 * @child: pointer to device node
 *
 * Returns a pointer to the interrupt parent node, or NULL if the interrupt
 * parent could not be determined.
 */
struct device_node *of_irq_find_parent(struct device_node *child)
{
	struct device_node *p;
	const __be32 *parp;

	if (!of_node_get(child))
		return NULL;

	do {
		parp = of_get_property(child, "interrupt-parent", NULL);
		if (parp == NULL)
			p = of_get_parent(child);
		else {
			if (of_irq_workarounds & OF_IMAP_NO_PHANDLE)
				p = of_node_get(of_irq_dflt_pic);
			else
				p = of_find_node_by_phandle(be32_to_cpup(parp));
		}
		of_node_put(child);
		child = p;
	} while (p && of_get_property(p, "#interrupt-cells", NULL) == NULL);

	return p;
}

So, if I understood the code correctly the DT IRQ core doesn't expect a device
node to have more than one "interrupt-parent" property.

It *should* work though if you have multiple "interrupts" properties defined and
all of them have the same "interrupt-parent":

       interrupt-parent = <&gpio6>;
       interrupts = <1 IRQF_TRIGGER_HIGH>; /* GPIO6_1 */
       interrupts = <2 IRQF_TRIGGER_LOW>; /* GPIO6_2 */

since of_irq_map_one() will be called for each "interrupts" and the correct
"interrupt-parent" will get obtained by of_irq_find_parent().

> So multiple definitions of interrupt-parent are allowed and the order
> does matter? And such does work? Sorry for asking, but I'm relatively
> new to DT. ;)
>

No worries, I'm very new to DT too so let's wait for Grant, Stephen or Linus to
give us a definite answer :)

> Regards,
> 
> Alexander Holler
> 

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/