ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

PATCH New Eunki Kim 3 participants 6 messages Sep 27, 2012 – Oct 23, 2012

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm Samsung Exynos

lore

Share & tools

lore

Eunki Kim

author

2012-09-27 03:55 UTC #0 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.

This patch inserts the bitmap_gpio_int member in struct samsung_ gpio_chip in order to represent use of GPIO interrupt for each pin and sets the related bit when s5p_register_gpio_interrupt function is called.

Signed-off-by Eunki Kim <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
 arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
 arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/plat-samsung/include/plat/gpio-core.h
index 1fe6917..dfd8b7a 100644

arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0

@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
  * @config: special function and pull-resistor control information.
  * @lock: Lock for exclusive access to this gpio bank.
  * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
  *
  * This wrapper provides the necessary information for the Samsung
  * specific gpios being registered with gpiolib.

@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
 #ifdef CONFIG_PM
 	u32			pm_save[4];
 #endif
+	u32			bitmap_gpio_int;
 };
 
 static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip *gpc)

arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2

index f9431fe..d981c61 100644
@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
 	/* check if the group has been already registered */
 	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
 	/* register gpio group */
 	ret = s5p_gpioint_add(my_chip);

@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
 		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
 		printk(KERN_INFO "Registered interrupt support for gpio group %d.\n",
 		       group);
-		return my_chip->irq_base + offset;
+		goto success;
 	}
 	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
 }
 
 int __init s5p_register_gpioint_bank(int chain_irq, int start, int nr_groups)

Signature · 1 line

1.7.1

Joonyoung Shim

2012-09-27 04:50 UTC #1 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

Quoted 6 lines · Eunki Kim

When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.

If for this, already the patch was posted.

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074387.html

Thanks.

Quoted 62 lines

Signed-off-by Eunki Kim <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
  arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
  arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
  2 files changed, 8 insertions(+), 2 deletions(-)

arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0

index 1fe6917..dfd8b7a 100644
@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
   * @config: special function and pull-resistor control information.
   * @lock: Lock for exclusive access to this gpio bank.
   * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
   *
   * This wrapper provides the necessary information for the Samsung
   * specific gpios being registered with gpiolib.

@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
  #ifdef CONFIG_PM
  	u32			pm_save[4];
  #endif
+	u32			bitmap_gpio_int;
  };
 
  static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip *gpc)

arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2

index f9431fe..d981c61 100644
@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
  	/* check if the group has been already registered */
  	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
  	/* register gpio group */
  	ret = s5p_gpioint_add(my_chip);

@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
  		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
  		printk(KERN_INFO "Registered interrupt support for gpio group %d.\n",
  		       group);
-		return my_chip->irq_base + offset;
+		goto success;
  	}
  	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
  }
 
  int __init s5p_register_gpioint_bank(int chain_irq, int start, int nr_groups)

Eunki Kim

author

2012-09-27 05:55 UTC #2 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

Quoted 16 lines

-----Original Message-----
From: Joonyoung Shim [mailto:jy0922.shim at samsung.com]
Sent: Thursday, September 27, 2012 1:50 PM
To: Eunki Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org; grant.likely at secretlab.ca; linus.walleij at linaro.org; kgene.kim at samsung.com
Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
Quoted 6 lines · Eunki Kim
When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.

Quoted 6 lines

If for this, already the patch was posted.
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074387.html
Thanks.

The old posted patch and my new patches solve the same problem.

However, the old patch fixes s5p_gpioint_set_type function which is irq_set_type function of GPIO int. The objective of irq_set_type function is setting IRQ type as rising, falling, level and etc. It's not setting GPIO pin configuration.

The new patches solve the problem root. It monitors use of GPIO int for each pin. If then, it sets INTERRUPT mode instead of INPUT mode when gpio_direction_input function is called. As you know, the objective of gpio_direction_input function is setting the GPIO configuration.

Quoted 64 lines

Signed-off-by Eunki Kim <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
  arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
  arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
  2 files changed, 8 insertions(+), 2 deletions(-)

arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0

index 1fe6917..dfd8b7a 100644
@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
   * @config: special function and pull-resistor control information.
   * @lock: Lock for exclusive access to this gpio bank.
   * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
   *
   * This wrapper provides the necessary information for the Samsung
   * specific gpios being registered with gpiolib.

@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
  #ifdef CONFIG_PM
  	u32			pm_save[4];
  #endif
+	u32			bitmap_gpio_int;
  };
 
  static inline struct samsung_gpio_chip *to_samsung_gpio(struct

gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c
index f9431fe..d981c61 100644

arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2

@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
  	/* check if the group has been already registered */
  	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
  	/* register gpio group */
  	ret = s5p_gpioint_add(my_chip);

@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
  		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
  		printk(KERN_INFO "Registered interrupt support for gpio group %d.\n",
  		       group);
-		return my_chip->irq_base + offset;
+		goto success;
  	}
  	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
  }
 
  int __init s5p_register_gpioint_bank(int chain_irq, int start, int

nr_groups)

Joonyoung Shim

2012-09-27 07:41 UTC #3 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

Quoted 19 lines · Eunki Kim

Quoted 19 lines
-----Original Message-----
From: Joonyoung Shim [mailto:jy0922.shim at samsung.com]
Sent: Thursday, September 27, 2012 1:50 PM
To: Eunki Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org; grant.likely at secretlab.ca; linus.walleij at linaro.org; kgene.kim at samsung.com
Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
Quoted 6 lines · Eunki Kim
When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.
If for this, already the patch was posted.
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074387.html

Quoted 7 lines

Quoted 2 lines
Thanks.
The old posted patch and my new patches solve the same problem.
However, the old patch fixes s5p_gpioint_set_type function which is irq_set_type function of GPIO int. The objective of irq_set_type function is setting IRQ type as rising, falling, level and etc. It's not setting GPIO pin configuration.

Ok, it's reasonable but s5p_irq_eint_set_type() does it. So, do we need to remove it in s5p_irq_eint_set_type() later?

Quoted 4 lines

The new patches solve the problem root. It monitors use of GPIO int for each pin. If then, it sets INTERRUPT mode instead of INPUT mode when gpio_direction_input function is called. As you know, the objective of gpio_direction_input function is setting the GPIO configuration.

Ok. It is minor concern, we will have to take care of the function call order dependency because s5p_register_gpio_interrupt() should be called prior to gpio_direction_input().

Thanks.

Quoted 66 lines

Quoted 65 lines

Signed-off-by Eunki Kim <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
   arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
   arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
   2 files changed, 8 insertions(+), 2 deletions(-)

arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0

index 1fe6917..dfd8b7a 100644
@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
    * @config: special function and pull-resistor control information.
    * @lock: Lock for exclusive access to this gpio bank.
    * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
    *
    * This wrapper provides the necessary information for the Samsung
    * specific gpios being registered with gpiolib.

@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
   #ifdef CONFIG_PM
   	u32			pm_save[4];
   #endif
+	u32			bitmap_gpio_int;
   };
 
   static inline struct samsung_gpio_chip *to_samsung_gpio(struct

gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c
index f9431fe..d981c61 100644

arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2

@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
   	/* check if the group has been already registered */
   	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
   	/* register gpio group */
   	ret = s5p_gpioint_add(my_chip);

@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
   		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
   		printk(KERN_INFO "Registered interrupt support for gpio group %d.\n",
   		       group);
-		return my_chip->irq_base + offset;
+		goto success;
   	}
   	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
   }
 
   int __init s5p_register_gpioint_bank(int chain_irq, int start, int

nr_groups)

Eunki Kim

author

2012-09-28 02:41 UTC #4 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

Quoted 36 lines

-----Original Message-----
From: Joonyoung Shim [mailto:jy0922.shim at samsung.com]
Sent: Thursday, September 27, 2012 4:42 PM
To: Eunki Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org; grant.likely at secretlab.ca; linus.walleij at linaro.org; kgene.kim at samsung.com
Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
Quoted 27 lines · Eunki Kim
Quoted 18 lines
-----Original Message-----
From: Joonyoung Shim [mailto:jy0922.shim at samsung.com]
Sent: Thursday, September 27, 2012 1:50 PM
To: Eunki Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org;
grant.likely at secretlab.ca; linus.walleij at linaro.org; kgene.kim at samsung.com
Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
Quoted 6 lines · Eunki Kim
When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.
Quoted 7 lines
If for this, already the patch was posted.
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/0
74387.html
Thanks.
The old posted patch and my new patches solve the same problem.

Quoted 8 lines

Quoted 2 lines
However, the old patch fixes s5p_gpioint_set_type function which is
irq_set_type function of GPIO int. The objective of irq_set_type function is setting IRQ type as
rising, falling, level and etc.
Quoted 1 line
It's not setting GPIO pin configuration.
Ok, it's reasonable but s5p_irq_eint_set_type() does it. So, do we need to remove it in s5p_irq_eint_set_type() later?

What do you mean? The old patch was not merged to mainline. I have checked 3.6-rc7 kernel source, but there isn't.

Quoted 11 lines

Quoted 4 lines
The new patches solve the problem root. It monitors use of GPIO int
for each pin. If then, it sets INTERRUPT mode instead of INPUT mode
when gpio_direction_input function is called. As you know, the objective of gpio_direction_input
function is setting the GPIO configuration.
Ok. It is minor concern, we will have to take care of the function call order dependency because s5p_register_gpio_interrupt() should be called prior to gpio_direction_input().
Thanks.

The s5p_register_gpio_interrupt function is called from the board file like mach-nuri.c mach-universal_c210.c. mach-goni.c generally. And also, setting GPIO pin to INTERRUPT mode codes (ex: s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));) follow the s5p_register_gpio_interrupt function. It's better to remain setting INTERRUPT mode. This patch just prevents from changing to INPUT mode by gpio_direction_input in any driver's probe function.

Thanks.

Quoted 67 lines

Quoted 66 lines

Signed-off-by Eunki Kim <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
   arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
   arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
   2 files changed, 8 insertions(+), 2 deletions(-)

arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0

index 1fe6917..dfd8b7a 100644
@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
    * @config: special function and pull-resistor control information.
    * @lock: Lock for exclusive access to this gpio bank.
    * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
    *
    * This wrapper provides the necessary information for the Samsung
    * specific gpios being registered with gpiolib.

@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
   #ifdef CONFIG_PM
   	u32			pm_save[4];
   #endif
+	u32			bitmap_gpio_int;
   };
 
   static inline struct samsung_gpio_chip *to_samsung_gpio(struct

gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c
index f9431fe..d981c61 100644

arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2

@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
   	/* check if the group has been already registered */
   	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
   	/* register gpio group */
   	ret = s5p_gpioint_add(my_chip);

@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
   		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
   		printk(KERN_INFO "Registered interrupt support for gpio group %d.\n",
   		       group);
-		return my_chip->irq_base + offset;
+		goto success;
   	}
   	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
   }
 
   int __init s5p_register_gpioint_bank(int chain_irq, int start,

int
nr_groups)

Kukjin Kim

2012-10-23 14:17 UTC #5 lore raw

PATCH 1/2 ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip

In reply to Eunki Kim

Quoted 72 lines · Eunki Kim

When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt.
This patch inserts the bitmap_gpio_int member in struct samsung_ gpio_chip in order to represent use of GPIO interrupt for each pin and sets the related bit when s5p_register_gpio_interrupt function is called.
Signed-off-by Eunki Kim <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linus Walleij <[email protected]>
---
 arch/arm/plat-samsung/include/plat/gpio-core.h |    2 ++
 arch/arm/plat-samsung/s5p-irq-gpioint.c        |    8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/plat-samsung/include/plat/gpio-core.h +2 -0
index 1fe6917..dfd8b7a 100644
@@ -48,6 +48,7 @@ struct samsung_gpio_cfg;
  * @config: special function and pull-resistor control information.
  * @lock: Lock for exclusive access to this gpio bank.
  * @pm_save: Save information for suspend/resume support.
+ * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not.
  *
  * This wrapper provides the necessary information for the Samsung
  * specific gpios being registered with gpiolib.
@@ -71,6 +72,7 @@ struct samsung_gpio_chip {
 #ifdef CONFIG_PM
 	u32			pm_save[4];
 #endif
+	u32			bitmap_gpio_int;
 };
 
 static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip
*gpc)
arch/arm/plat- renamed from arch/arm/plat-samsung/s5p-irq-gpioint.c +0 -0
samsung/s5p-irq-gpioint.c
index f9431fe..d981c61 100644
arch/arm/plat-samsung/s5p-irq-gpioint.c +6 -2
@@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin)
 
 	/* check if the group has been already registered */
 	if (my_chip->irq_base)
-		return my_chip->irq_base + offset;
+		goto success;
 
 	/* register gpio group */
 	ret = s5p_gpioint_add(my_chip);
@@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin)
 		my_chip->chip.to_irq = samsung_gpiolib_to_irq;
 		printk(KERN_INFO "Registered interrupt support for gpio
 group %d.\n",
 		       group);
-		return my_chip->irq_base + offset;
+		goto success;
 	}
 	return ret;
+success:
+	my_chip->bitmap_gpio_int |= BIT(offset);
+
+	return my_chip->irq_base + offset;
 }
 

Quoted 4 lines

int __init s5p_register_gpioint_bank(int chain_irq, int start, int
nr_groups)
--
1.7.1

OK, applied.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.