Re: [PATCH v5 12/22] gpio/omap: cleanup set_gpio_triggering function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 04 August 2011 04:34 PM, Tarun Kanti DebBarma wrote:
Getting rid of ifdefs within the function by adding register offset intctrl
and associating OMAPXXXX_GPIO_INT_CONTROL in respective SoC specific files.
Also, use wkup_status register consistently instead of referring to wakeup
clear and wakeup set register offsets.

Signed-off-by: Charulatha V<charu@xxxxxx>
Signed-off-by: Tarun Kanti DebBarma<tarun.kanti@xxxxxx>
---
Good. Some comments.

  arch/arm/mach-omap1/gpio15xx.c         |    2 +
  arch/arm/mach-omap1/gpio16xx.c         |    3 +
  arch/arm/mach-omap1/gpio7xx.c          |    2 +
  arch/arm/plat-omap/include/plat/gpio.h |    3 +
  drivers/gpio/gpio-omap.c               |  159 ++++++++-----------------------
  5 files changed, 51 insertions(+), 118 deletions(-)

diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
index f8c15ea..2adfece 100644
--- a/arch/arm/mach-omap1/gpio15xx.c
+++ b/arch/arm/mach-omap1/gpio15xx.c
@@ -42,6 +42,7 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
  };

  static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = {
@@ -83,6 +84,7 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
  	.irqstatus	= OMAP1510_GPIO_INT_STATUS,
  	.irqenable	= OMAP1510_GPIO_INT_MASK,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP1510_GPIO_INT_CONTROL,
  };

  static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = {
diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
index ed9f285..f619805 100644
--- a/arch/arm/mach-omap1/gpio16xx.c
+++ b/arch/arm/mach-omap1/gpio16xx.c
@@ -45,6 +45,7 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
  };

  static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = {
@@ -90,6 +91,8 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
  	.set_irqenable	= OMAP1610_GPIO_SET_IRQENABLE1,
  	.clr_irqenable	= OMAP1610_GPIO_CLEAR_IRQENABLE1,
  	.wkup_status	= OMAP1610_GPIO_WAKEUPENABLE,
+	.edgectrl1	= OMAP1610_GPIO_EDGE_CTRL1,
+	.edgectrl2	= OMAP1610_GPIO_EDGE_CTRL2,
  };

  static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = {
diff --git a/arch/arm/mach-omap1/gpio7xx.c b/arch/arm/mach-omap1/gpio7xx.c
index 923eaa1..cb083c5 100644
--- a/arch/arm/mach-omap1/gpio7xx.c
+++ b/arch/arm/mach-omap1/gpio7xx.c
@@ -47,6 +47,7 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT / 2,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT / 2,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE / 2,

Shift operator would have been better here.

[...]

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e7c9fe5..21cb0d4 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c

[..]

+			trigger != 0);
+
+	/* This part needs to be executed always for OMAP34xx */
+	if (cpu_is_omap34xx() || (bank->non_wakeup_gpios&  gpio_bit)) {
Why ? You might want handle this special case with some flag instead.

  		/*
  		 * Log the edge gpio and manually trigger the IRQ
  		 * after resume if the input level changes
@@ -261,7 +236,6 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
  		__raw_readl(bank->base + bank->regs->leveldetect0) |
  		__raw_readl(bank->base + bank->regs->leveldetect1);
  }
-#endif

  #ifdef CONFIG_ARCH_OMAP1
  /*

[...]

-#endif
-	default:
-		goto bad;
+
+		_gpio_rmw(base, bank->regs->wkup_status, 1<<  gpio, trigger);
+
Avoid this extra line.

Rest of the changes looks good to me.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux