> -----Original Message----- > From: Joonyoung Shim [mailto:jy0922.shim@xxxxxxxxxxx] > Sent: Thursday, September 27, 2012 4:42 PM > To: Eunki Kim > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; > grant.likely@xxxxxxxxxxxx; linus.walleij@xxxxxxxxxx; kgene.kim@xxxxxxxxxxx > Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip > > On 09/27/2012 02:55 PM, Eunki Kim wrote: > >> -----Original Message----- > >> From: Joonyoung Shim [mailto:jy0922.shim@xxxxxxxxxxx] > >> Sent: Thursday, September 27, 2012 1:50 PM > >> To: Eunki Kim > >> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > >> linux-samsung-soc@xxxxxxxxxxxxxxx; > >> grant.likely@xxxxxxxxxxxx; linus.walleij@xxxxxxxxxx; > >> kgene.kim@xxxxxxxxxxx > >> Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member > >> in samsung_gpio_chip > >> > >> On 09/27/2012 12:55 PM, Eunki Kim wrote: > >>> 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/0 > >> 74387.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. > > 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. > > > > 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. > > > >>> 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 <eunki_kim@xxxxxxxxxxx> > >>> Cc: Grant Likely <grant.likely@xxxxxxxxxxxx> > >>> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx> > >>> --- > >>> 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(-) > >>> > >>> diff --git a/arch/arm/plat-samsung/include/plat/gpio-core.h > >>> b/arch/arm/plat-samsung/include/plat/gpio-core.h > >>> index 1fe6917..dfd8b7a 100644 > >>> --- a/arch/arm/plat-samsung/include/plat/gpio-core.h > >>> +++ b/arch/arm/plat-samsung/include/plat/gpio-core.h > >>> @@ -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 > >>> --- a/arch/arm/plat-samsung/s5p-irq-gpioint.c > >>> +++ b/arch/arm/plat-samsung/s5p-irq-gpioint.c > >>> @@ -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) > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html