Hi Linus,
On 11/18/2013 03:11 PM, Linus Walleij wrote:
On Fri, Nov 8, 2013 at 11:11 AM, Prabhakar Lad
<prabhakar.csengg@xxxxxxxxx> wrote:
From: "Lad, Prabhakar" <prabhakar.csengg@xxxxxxxxx>
This patch converts the davinci gpio driver to use irqdomain
support.
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx>
(...)
@@ -282,8 +283,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
desc->irq_data.chip->irq_ack(&desc->irq_data);
while (1) {
u32 status;
- int n;
- int res;
+ int bit;
Why is this an int? I think u8 would suffice.
/* now demux them to the right lowlevel handler */
- n = d->irq_base;
- if (irq & 1) {
- n += 16;
- status >>= 16;
- }
-
while (status) {
- res = ffs(status);
- n += res;
- generic_handle_irq(n - 1);
- status >>= res;
+ bit = __ffs(status);
+ status &= ~(1 << bit);
+ generic_handle_irq(irq_find_mapping(d->irq_domain,
+ bit));
This is a nice hunk of the patch.
I would use <linux/bitops.h> and do:
status &= ~BIT(bit);
@@ -313,10 +307,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- if (d->irq_base >= 0)
- return d->irq_base + offset;
- else
- return -ENODEV;
+ return irq_create_mapping(d->irq_domain, offset);
}
I think we recently established that map creating cannot be done
in gpio_to_irq* functions as that will not work properly if you request
an IRQ from device tree without first obtaining the IRQ from the GPIO
number with this function.
Why? Could you point on corresponding thread, pls?
Current call tree is:
irq_create_of_mapping()
|-hwirq = omain->ops->xlate()
|-irq_create_mapping(domain, hwirq)
Instead call irq_create_mapping() on *all* used IRQ lines in the
probe function and use irq_find_mapping() here too.
+ for (gpio = 0, bank = 0; gpio < ngpio; bank++, bank_irq++, gpio += 16) {
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
writel(~0, &g->clr_falling);
@@ -467,14 +483,6 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
*/
irq_set_handler_data(bank_irq, &chips[gpio / 32]);
So for example here you could call iurq_create_mapping();
Also: please write a patch that marks the IRQ lines.
Call gpio_lock_as_irq(*gpio_chip, offset); in the
irqchip startup/shutdown functions. (Can be a separate
patch.)
It seems, some misunderstanding is here. So I'd like to clarify few
points and would be very appreciate for your comments:
1) This patch itself will work unless we switch to DT (as in the
following patch)
2) After this patch the following object structure will be created
during Davinci GPIO driver initialization (DA850 has 144 IRQ lines):
- gpio_chip0(0..31)
|- irq_domain1
- gpio_chip1(32..63)
|- irq_domain2
- gpio_chip2(64..95)
|- irq_domain3
- gpio_chip3(96..127)
|- irq_domain4
- gpio_chip4(128..143)
|- irq_domain5
3) But in case of DT only one GPIO controller node will be created
Example:
gpio: gpio@1e26000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
reg = <0x226000 0x1000>;
interrupt-parent = <&intc>;
interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
50 IRQ_TYPE_EDGE_BOTH>;
interrupt-controller;
#interrupt-cells = <2>;
ti,ngpio = <144>;
ti,davinci-gpio-unbanked = <0>;
}
4) As result, to make GPIO bindings/mappings work - we'll need to
implement .of_xlate() callback per GPIO chip, which will provide us with
valid valid gpio_chip and offset of gpio inside chip
(It was discussed before and supposed to be done as next step).
For example, gpio_chip3 and offset=15 should be selected:
devA {
gpios = <&gpio 111 GPIO_ACTIVE_HIGH>;
}
5) What should be done to make GPIO IRQ bindings/mappings work?
Example:
devB {
interrupt-parent = <&gpio>;
interrupts = <111 IRQ_TYPE_EDGE_BOTH>;
}
Should we implement one IRQ domain per all GPIO chips (per GPIO controller)?
Thanks.
Regards,
-Grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
