[PATCH v2] gpio: rcar: Request GPIO while enabling interrupt

2018-11-06 Thread Fabrizio Castro
There are cases when the bootloader configures a pin to work
as a function rather than GPIO, and other cases when the pin
is configured as a function at POR.
This commit makes sure the pin is configured as a GPIO the
moment we need it to work as an interrupt.

Signed-off-by: Fabrizio Castro 

---
v1->v2:
* Moved gpio_rcar_request call from gpio_rcar_irq_set_type to
  rcar_gpio_irq_request_resources
* Added rcar_gpio_irq_release_resources for calling gpio_rcar_free

Comments very welcome!

 drivers/gpio/gpio-rcar.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 3c82bb3..c16598b 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -344,6 +344,29 @@ static int gpio_rcar_direction_output(struct gpio_chip 
*chip, unsigned offset,
return 0;
 }
 
+static int rcar_gpio_irq_request_resources(struct irq_data *d)
+{
+   struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+   struct gpio_rcar_priv *p = gpiochip_get_data(gc);
+   unsigned int hwirq = irqd_to_hwirq(d);
+   int err;
+
+   gpio_rcar_direction_input(gc, hwirq);
+   err = gpio_rcar_request(gc, hwirq);
+   if (err)
+   dev_err(&p->pdev->dev, "Can't request GPIO %u from %s\n", hwirq,
+   gc->label);
+   return err;
+}
+
+static void rcar_gpio_irq_release_resources(struct irq_data *d)
+{
+   struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+   unsigned int hwirq = irqd_to_hwirq(d);
+
+   gpio_rcar_free(gc, hwirq);
+}
+
 struct gpio_rcar_info {
bool has_both_edge_trigger;
 };
@@ -491,6 +514,8 @@ static int gpio_rcar_probe(struct platform_device *pdev)
irq_chip->irq_set_type = gpio_rcar_irq_set_type;
irq_chip->irq_set_wake = gpio_rcar_irq_set_wake;
irq_chip->flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_MASK_ON_SUSPEND;
+   irq_chip->irq_request_resources = rcar_gpio_irq_request_resources;
+   irq_chip->irq_release_resources = rcar_gpio_irq_release_resources;
 
ret = gpiochip_add_data(gpio_chip, p);
if (ret) {
-- 
2.7.4



Re: [PATCH v2] gpio: rcar: Request GPIO while enabling interrupt

2018-11-14 Thread Geert Uytterhoeven
Hi Fabrizio,

On Tue, Nov 6, 2018 at 8:19 PM Fabrizio Castro
 wrote:
> There are cases when the bootloader configures a pin to work
> as a function rather than GPIO, and other cases when the pin
> is configured as a function at POR.
> This commit makes sure the pin is configured as a GPIO the
> moment we need it to work as an interrupt.
>
> Signed-off-by: Fabrizio Castro 
>
> ---
> v1->v2:
> * Moved gpio_rcar_request call from gpio_rcar_irq_set_type to
>   rcar_gpio_irq_request_resources
> * Added rcar_gpio_irq_release_resources for calling gpio_rcar_free

Thanks for your patch!

While I could see no obvious deficiencies at first glance, I gave your
patch a try on Koelsch and Salvator-XS.

Koelsch:

  - ADV7511 HDMI encoder WARN_ON(!test_bit(FLAG_USED_AS_IRQ, &desc->flags)
in gpiochip_enable_irq():

WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:3513
gpiochip_irq_enable+0x18/0x34
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.20.0-rc2-koelsch-00873-gf433e294a90792da-dirty #179
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x7c/0x9c)
[] (dump_stack) from [] (__warn+0xd0/0xec)
[] (__warn) from [] (warn_slowpath_null+0x38/0x44)
[] (warn_slowpath_null) from []
(gpiochip_irq_enable+0x18/0x34)
[] (gpiochip_irq_enable) from [] (irq_enable+0x3c/0x50)
[] (irq_enable) from [] (__irq_startup+0x94/0xa4)
[] (__irq_startup) from [] (irq_startup+0x4c/0x11c)
[] (irq_startup) from [] (__setup_irq+0x4d0/0x6a4)
[] (__setup_irq) from []
(request_threaded_irq+0x9c/0x134)
[] (request_threaded_irq) from []
(devm_request_threaded_irq+0x68/0xa4)
[] (devm_request_threaded_irq) from []
(adv7511_probe+0x748/0x860)
[] (adv7511_probe) from []
(i2c_device_probe+0x210/0x228)
[] (i2c_device_probe) from [] (really_probe+0x1f0/0x2c0)
[] (really_probe) from []
(driver_probe_device+0x140/0x15c)
[] (driver_probe_device) from []
(bus_for_each_drv+0xa0/0xb4)
[] (bus_for_each_drv) from []
(__device_attach+0xb0/0x124)
[] (__device_attach) from []
(bus_probe_device+0x28/0x80)
[] (bus_probe_device) from [] (device_add+0x438/0x570)
[] (device_add) from [] (i2c_new_device+0x238/0x278)
[] (i2c_new_device) from []
(of_i2c_register_device+0x40/0x80)
[] (of_i2c_register_device) from []
(of_i2c_register_devices+0x80/0xc0)
[] (of_i2c_register_devices) from []
(i2c_register_adapter+0x1ec/0x390)
[] (i2c_register_adapter) from []
(i2c_demux_activate_master+0xd4/0x158)
[] (i2c_demux_activate_master) from []
(i2c_demux_pinctrl_probe+0x190/0x1f0)
[] (i2c_demux_pinctrl_probe) from []
(platform_drv_probe+0x48/0x94)
[] (platform_drv_probe) from []
(really_probe+0x1f0/0x2c0)
[] (really_probe) from []
(driver_probe_device+0x140/0x15c)
[] (driver_probe_device) from []
(__driver_attach+0x8c/0xc8)
[] (__driver_attach) from []
(bus_for_each_dev+0x64/0xa0)
[] (bus_for_each_dev) from []
(bus_add_driver+0x16c/0x1d4)
[] (bus_add_driver) from [] (driver_register+0xac/0xf0)
[] (driver_register) from []
(do_one_initcall+0x70/0x170)
[] (do_one_initcall) from []
(kernel_init_freeable+0x194/0x1d8)
[] (kernel_init_freeable) from []
(kernel_init+0x8/0x110)
[] (kernel_init) from [] (ret_from_fork+0x14/0x3c)
Exception stack(0xeb44dfb0 to 0xeb44dff8)
dfa0:  
 
dfc0:      
 
dfe0:     0013 

  - SDHI CD pin failure (first channel shown only):

sh-pfc e606.pin-controller: pin GP_6_6 already requested by
e6055400.gpio:812; cannot claim for e6055400.gpio:812
sh-pfc e606.pin-controller: pin-198 (e6055400.gpio:812) status -22
gpio_rcar e6055400.gpio: Can't request GPIO 6 from e6055400.gpio
genirq: Failed to request resources for ee10.sd cd (irq 186)
on irqchip e6055400.gpio

  - gpio-keys failure:

sh-pfc e606.pin-controller: pin GP_5_0 already requested by
e6055000.gpio:838; cannot claim for e6055000.gpio:838
sh-pfc e606.pin-controller: pin-160 (e6055000.gpio:838) status -22
gpio_rcar e6055000.gpio: Can't request GPIO 0 from e6055000.gpio
genirq: Failed to request resources for SW2-1 (irq 189) on irqchip
e6055000.gpio
gpio-keys keyboard: Unable to claim irq 189; error -22
gpio-keys: probe of keyboard failed with error -22

  - /sys/kernel/debug/pinctrl/e606.pin-controller-sh-pfc/pinmux-pins
changes:

ADV7511 IRQ is now claimed as a GPIO, which was the intention of your
patch:

-pin 125 (GP_3_29): (MUX UNCLAIMED) (GPIO UNCLAIMED)
+pin 125 (GP_3_29): (MUX UNCLAIMED) e6053000.gpio:931

With #define DEBUG in drivers/pinctrl/sh-pfc/core.c:

sh-pfc e606.pin-controller: write_reg addr = e6060010,
value = 0x0, field = 2, r_width = 

RE: [PATCH v2] gpio: rcar: Request GPIO while enabling interrupt

2018-11-15 Thread Fabrizio Castro
Hello Geert,

Thank you for your feedback!

> From: Geert Uytterhoeven 
> Sent: 14 November 2018 12:52
> Subject: Re: [PATCH v2] gpio: rcar: Request GPIO while enabling interrupt
>
> Hi Fabrizio,
>
> On Tue, Nov 6, 2018 at 8:19 PM Fabrizio Castro
>  wrote:
> > There are cases when the bootloader configures a pin to work
> > as a function rather than GPIO, and other cases when the pin
> > is configured as a function at POR.
> > This commit makes sure the pin is configured as a GPIO the
> > moment we need it to work as an interrupt.
> >
> > Signed-off-by: Fabrizio Castro 
> >
> > ---
> > v1->v2:
> > * Moved gpio_rcar_request call from gpio_rcar_irq_set_type to
> >   rcar_gpio_irq_request_resources
> > * Added rcar_gpio_irq_release_resources for calling gpio_rcar_free
>
> Thanks for your patch!
>
> While I could see no obvious deficiencies at first glance, I gave your
> patch a try on Koelsch and Salvator-XS.

Thank you for testing the patch!

These issues seem to be related to a few patches that were merged only recently,
I have rebased my work on top of the next-20181115 now, and I get the same 
thing.
I need to look into this from scratch again, I'll be in touch as soon as I have 
found
an alternative way to fix the issue.

Thanks,
Fab

>
> Koelsch:
>
>   - ADV7511 HDMI encoder WARN_ON(!test_bit(FLAG_USED_AS_IRQ, &desc->flags)
> in gpiochip_enable_irq():
>
> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:3513
> gpiochip_irq_enable+0x18/0x34
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.20.0-rc2-koelsch-00873-gf433e294a90792da-dirty #179
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0x7c/0x9c)
> [] (dump_stack) from [] (__warn+0xd0/0xec)
> [] (__warn) from [] (warn_slowpath_null+0x38/0x44)
> [] (warn_slowpath_null) from []
> (gpiochip_irq_enable+0x18/0x34)
> [] (gpiochip_irq_enable) from [] 
> (irq_enable+0x3c/0x50)
> [] (irq_enable) from [] (__irq_startup+0x94/0xa4)
> [] (__irq_startup) from [] (irq_startup+0x4c/0x11c)
> [] (irq_startup) from [] (__setup_irq+0x4d0/0x6a4)
> [] (__setup_irq) from []
> (request_threaded_irq+0x9c/0x134)
> [] (request_threaded_irq) from []
> (devm_request_threaded_irq+0x68/0xa4)
> [] (devm_request_threaded_irq) from []
> (adv7511_probe+0x748/0x860)
> [] (adv7511_probe) from []
> (i2c_device_probe+0x210/0x228)
> [] (i2c_device_probe) from [] 
> (really_probe+0x1f0/0x2c0)
> [] (really_probe) from []
> (driver_probe_device+0x140/0x15c)
> [] (driver_probe_device) from []
> (bus_for_each_drv+0xa0/0xb4)
> [] (bus_for_each_drv) from []
> (__device_attach+0xb0/0x124)
> [] (__device_attach) from []
> (bus_probe_device+0x28/0x80)
> [] (bus_probe_device) from [] (device_add+0x438/0x570)
> [] (device_add) from [] (i2c_new_device+0x238/0x278)
> [] (i2c_new_device) from []
> (of_i2c_register_device+0x40/0x80)
> [] (of_i2c_register_device) from []
> (of_i2c_register_devices+0x80/0xc0)
> [] (of_i2c_register_devices) from []
> (i2c_register_adapter+0x1ec/0x390)
> [] (i2c_register_adapter) from []
> (i2c_demux_activate_master+0xd4/0x158)
> [] (i2c_demux_activate_master) from []
> (i2c_demux_pinctrl_probe+0x190/0x1f0)
> [] (i2c_demux_pinctrl_probe) from []
> (platform_drv_probe+0x48/0x94)
> [] (platform_drv_probe) from []
> (really_probe+0x1f0/0x2c0)
> [] (really_probe) from []
> (driver_probe_device+0x140/0x15c)
> [] (driver_probe_device) from []
> (__driver_attach+0x8c/0xc8)
> [] (__driver_attach) from []
> (bus_for_each_dev+0x64/0xa0)
> [] (bus_for_each_dev) from []
> (bus_add_driver+0x16c/0x1d4)
> [] (bus_add_driver) from [] 
> (driver_register+0xac/0xf0)
> [] (driver_register) from []
> (do_one_initcall+0x70/0x170)
> [] (do_one_initcall) from []
> (kernel_init_freeable+0x194/0x1d8)
> [] (kernel_init_freeable) from []
> (kernel_init+0x8/0x110)
> [] (kernel_init) from [] (ret_from_fork+0x14/0x3c)
> Exception stack(0xeb44dfb0 to 0xeb44dff8)
> dfa0:  
>  
> dfc0:      
>  
> dfe0:     0013 
>
>   - SDHI CD pin failure (first channel shown only):
>
> sh-pfc e606.pin-controller: pin GP_6_6 already requested by
> e6055400.gpio:812; cannot claim for e6055400.gpio:812
> sh-pfc e606.pin-controller: pin-198 (e6055400.