Re: [patch 34/75] arm: ep93xx: Kill another instance of broken irq_descfiddling

From: Ryan Mallon
Date: Fri Feb 11 2011 - 15:17:10 EST


On 12/02/11 00:42, Thomas Gleixner wrote:
On Fri, 11 Feb 2011, Ryan Mallon wrote:

On 02/11/2011 12:37 PM, Thomas Gleixner wrote:
1. This is a copy of the borked code in gpiolib
2. If you need information about irq state which is not exposed, then talk
to the maintainer of that code instead of adding totaly horrible open
coded access.
This code got added simply because it is sometimes helpful to be able to
see how various gpio/irq pins are configured. I'm happy to drop the
functionality (see below), but is there a better way to get this
information? Is it already available somewhere else (proc, sys)?
No, but we can add that if it's required in a sane form.

I don't have objections to expose debug informations in general, but
I have objections that random code does this especially, when it's
duplicated random code :)


Ok. Does it make sense to expose the interrupt configuration for all interrupts via proc, sys, debug, etc? Is this information useful to enough people to warrant it? I would rather have a global debug rather than individual platforms and drivers implementing it.

For the time being can we fix up the ep93xx gpio code with the amended patch below. It keeps the information that the pin is also configured as an interrupt and cleans the code up a bit.

Signed-off-by: Ryan Mallon <ryan@xxxxxxxxxxxxxxxx>
---

diff --git a/arch/arm/mach-ep93xx/gpio.c b/arch/arm/mach-ep93xx/gpio.c
index bec34b8..d0bcce4 100644
--- a/arch/arm/mach-ep93xx/gpio.c
+++ b/arch/arm/mach-ep93xx/gpio.c
@@ -347,52 +347,14 @@ static void ep93xx_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
gpio = ep93xx_chip->chip.base;
for (i = 0; i< chip->ngpio; i++, gpio++) {
int is_out = data_dir_reg& (1<< i);
+ int irq = gpio_to_irq(gpio);

- seq_printf(s, " %s%d gpio-%-3d (%-12s) %s %s",
+ seq_printf(s, " %s%d gpio-%-3d (%-12s) %s %s %s\n",
chip->label, i, gpio,
gpiochip_is_requested(chip, i) ? : "",
is_out ? "out" : "in ",
- (data_reg& (1<< i)) ? "hi" : "lo");
-
- if (!is_out) {
- int irq = gpio_to_irq(gpio);
- struct irq_desc *desc = irq_desc + irq;
-
- if (irq>= 0&& desc->action) {
- char *trigger;
-
- switch (desc->status& IRQ_TYPE_SENSE_MASK) {
- case IRQ_TYPE_NONE:
- trigger = "(default)";
- break;
- case IRQ_TYPE_EDGE_FALLING:
- trigger = "edge-falling";
- break;
- case IRQ_TYPE_EDGE_RISING:
- trigger = "edge-rising";
- break;
- case IRQ_TYPE_EDGE_BOTH:
- trigger = "edge-both";
- break;
- case IRQ_TYPE_LEVEL_HIGH:
- trigger = "level-high";
- break;
- case IRQ_TYPE_LEVEL_LOW:
- trigger = "level-low";
- break;
- default:
- trigger = "?trigger?";
- break;
- }
-
- seq_printf(s, " irq-%d %s%s",
- irq, trigger,
- (desc->status& IRQ_WAKEUP)
- ? " wakeup" : "");
- }
- }
-
- seq_printf(s, "\n");
+ (data_reg& (1<< i)) ? "hi" : "lo",
+ (!is_out&& irq>= 0) ? "(interrupt)" : "");
}
}


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