Re: [PATCH v9 01/25] gpio/omap: remove dependency on gpio_bank_count
From: Cousson, Benoit
Date: Thu Feb 02 2012 - 15:48:20 EST
On 2/2/2012 8:45 PM, Felipe Balbi wrote:
On Thu, Feb 02, 2012 at 12:16:30PM -0700, Grant Likely wrote:
On Thu, Feb 02, 2012 at 08:41:07PM +0200, Felipe Balbi wrote:
Hi,
On Thu, Feb 02, 2012 at 11:00:27PM +0530, Tarun Kanti DebBarma wrote:
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0b05629..6ea7390 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -28,7 +28,10 @@
#include<asm/gpio.h>
#include<asm/mach/irq.h>
+static LIST_HEAD(omap_gpio_list);
I guess it's now too late because patch is acked and everything, but I
think if you make the driver handle one bank alone and just instantiate
it multiple times (omap_gpio.0, omap_gpio.1, omap_gpio.3, etc) driver
would be faaaaaar simpler.
Is there any shared state between the banks? On my very cursory glance it
looked like banks still have some interaction between them. If not, then
yes I agree that multiple instances would be better.
A quick glance at the TRM shows that banks have separate address spaces
and IRQ lines. I think it's done this way because we can handoff one (or
more) bank to other cores on the SoC, so they need to be pretty
independent.
I could be missing something though.
In fact the driver already handled the 6 GPIOS banks as individual devices:
[ 0.185638] gpiochip_add: registered GPIOs 0 to 31 on device: gpio
[ 0.185882] OMAP GPIO hardware version 0.1
[ 0.186767] gpiochip_add: registered GPIOs 32 to 63 on device: gpio
[ 0.187744] gpiochip_add: registered GPIOs 64 to 95 on device: gpio
[ 0.188751] gpiochip_add: registered GPIOs 96 to 127 on device: gpio
[ 0.189819] gpiochip_add: registered GPIOs 128 to 159 on device: gpio
[ 0.190917] gpiochip_add: registered GPIOs 160 to 191 on device: gpio
That list is only used to iterate over all the instances during CPU idle:
void omap2_gpio_prepare_for_idle(int pwr_mode)
{
struct gpio_bank *bank;
list_for_each_entry(bank, &omap_gpio_list, node) {
if (!bank->mod_usage || !bank->loses_context)
continue;
bank->power_mode = pwr_mode;
pm_runtime_put_sync_suspend(bank->dev);
}
}
void omap2_gpio_resume_after_idle(void)
{
struct gpio_bank *bank;
list_for_each_entry(bank, &omap_gpio_list, node) {
if (!bank->mod_usage || !bank->loses_context)
continue;
pm_runtime_get_sync(bank->dev);
}
}
I don't know if there is some reason to not use driver_for_each_device.
Kevin,
Do we have any constraint inside omap_sram_idle to not use the device
iterator?
Regards,
Benoit
--
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/