On 28/05/2020 14:24, Hans Verkuil wrote:
> On 27/05/2020 16:07, Linus Walleij wrote:
>> We provided the right semantics on open drain lines being
>> by definition output but incidentally the irq set up function
>> would only allow IRQs on lines that were "not output".
>>
>> Fix the semantics to allow output open drain lines to be used
>> for IRQs.
>>
>> Reported-by: Hans Verkuil <hverkuil@xxxxxxxxx>
>
> Tested-by: Hans Verkuil <hverkuil@xxxxxxxxx>
>
> Whether this is the right/best fix or not, I cannot tell, but it certainly
> fixes the cec-gpio driver!
Close, but no cigar. I didn't check the kernel log and I now see this warning:
[ 4.157000] ------------[ cut here ]------------
[ 4.161699] WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:4260 gpiochip_enable_irq+0x6c/0x78
[ 4.170522] Modules linked in:
[ 4.173626] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc2-rpi3 #179
[ 4.180510] Hardware name: Raspberry Pi 3 Model B (DT)
[ 4.185724] pstate: 80000085 (Nzcv daIf -PAN -UAO)
[ 4.190585] pc : gpiochip_enable_irq+0x6c/0x78
[ 4.195094] lr : gpiochip_irq_enable+0x20/0x58
[ 4.199597] sp : ffff8000100239e0
[ 4.202956] x29: ffff8000100239e0 x28: 0000000000000000
[ 4.208346] x27: ffff0000042b9808 x26: ffff00000425eca4
[ 4.213735] x25: 0000000000000000 x24: ffff00000425ed58
[ 4.219124] x23: 0000000000000000 x22: 0000000000000000
[ 4.224512] x21: 0000000000000001 x20: ffff0000041d1b00
[ 4.229902] x19: ffff00000425ec00 x18: 0000000000000000
[ 4.235291] x17: ffff8000107ddce0 x16: ffff8000107dd1b8
[ 4.240680] x15: ffff000037870450 x14: ffffffffffffffff
[ 4.246070] x13: ffff0000041d1a87 x12: ffff0000041d1a85
[ 4.251459] x11: 0000000000000000 x10: 0000000000000040
[ 4.256848] x9 : 0000000000000000 x8 : 0000000000000080
[ 4.262237] x7 : 0000000000000000 x6 : 000000000000003f
[ 4.267626] x5 : 0000000000000000 x4 : ffff0000372d3990
[ 4.273014] x3 : ffff00000425ec28 x2 : 0000000000000036
[ 4.278403] x1 : ffff0000372d40e0 x0 : 0000000000000683
[ 4.283793] Call trace:
[ 4.286274] gpiochip_enable_irq+0x6c/0x78
[ 4.290432] irq_enable+0x3c/0x80
[ 4.293795] __irq_startup+0x7c/0xa8
[ 4.297419] irq_startup+0x5c/0x138
[ 4.300957] __setup_irq+0x82c/0x8f8
[ 4.304583] request_threaded_irq+0xd8/0x198
[ 4.308913] devm_request_threaded_irq+0x78/0xf8
[ 4.313599] cec_gpio_probe+0x118/0x288
[ 4.317490] platform_drv_probe+0x54/0xb0
[ 4.321558] really_probe+0xd8/0x330
[ 4.325184] driver_probe_device+0x58/0xf8
[ 4.329339] device_driver_attach+0x74/0x80
[ 4.333582] __driver_attach+0x58/0xe0
[ 4.337383] bus_for_each_dev+0x70/0xc0
[ 4.341273] driver_attach+0x24/0x30
[ 4.344899] bus_add_driver+0x150/0x1e0
[ 4.348789] driver_register+0x64/0x128
[ 4.352678] __platform_driver_register+0x48/0x58
[ 4.357452] cec_gpio_pdrv_init+0x1c/0x28
[ 4.361519] do_one_initcall+0x54/0x1b8
[ 4.365412] kernel_init_freeable+0x1cc/0x244
[ 4.369833] kernel_init+0x14/0x108
[ 4.373371] ret_from_fork+0x10/0x1c
[ 4.377000] ---[ end trace 49bf91ba04222a1a ]---
[ 4.382253] Registered IR keymap rc-cec
[ 4.386344] rc rc0: cec-gpio@7 as /devices/platform/cec-gpio@7/rc/rc0
[ 4.393135] input: cec-gpio@7 as /devices/platform/cec-gpio@7/rc/rc0/input0
The fix seems to be to just add this change to the patch:
Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
---
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 40f2d7f69be2..5a8214d5e927 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4255,7 +4257,8 @@ void gpiochip_enable_irq(struct gpio_chip *gc, unsigned int offset)
if (!IS_ERR(desc) &&
!WARN_ON(!test_bit(FLAG_USED_AS_IRQ, &desc->flags))) {
- WARN_ON(test_bit(FLAG_IS_OUT, &desc->flags));
+ WARN_ON(test_bit(FLAG_IS_OUT, &desc->flags) &&
+ !test_bit(FLAG_OPEN_DRAIN, &desc->flags));
set_bit(FLAG_IRQ_IS_ENABLED, &desc->flags);
}
}
I wish I could test this on the BeagleBone Black, which is the other use-case,
but it's in another country and the earliest I can test is probably August or
September (depending on how long the travel restrictions stay in place).
If someone else has a BBB handy, then it would be nice if this patch can be
tested.
Regards,
Hans
>
> Regards,
>
> Hans
>
>> Cc: Russell King <linux@xxxxxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 256efaea1fdc ("gpiolib: fix up emulated open drain outputs")
>> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> ---
>> drivers/gpio/gpiolib.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index b4b5792fe2ff..edd74ff31cea 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -4220,7 +4220,9 @@ int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset)
>> }
>> }
>>
>> - if (test_bit(FLAG_IS_OUT, &desc->flags)) {
>> + /* To be valid for IRQ the line needs to be input or open drain */
>> + if (test_bit(FLAG_IS_OUT, &desc->flags) &&
>> + !test_bit(FLAG_OPEN_DRAIN, &desc->flags)) {
>> chip_err(gc,
>> "%s: tried to flag a GPIO set as output for IRQ\n",
>> __func__);
>>
>
