Re: [PATCH v2] media: cx18: Remove unused cx18_reset_ir_gpio

Hans Verkuil
2024-10-29T10:57:19Z

On 10/28/24 20:58, Sean Young wrote:
> On Sun, Oct 13, 2024 at 12:39:32AM +0100, linux@treblig.org wrote:
>> From: "Dr. David Alan Gilbert" <linux@treblig.org>
>>
>> cx18_reset_ir_gpio() has been unused in tree since 2009
>> commit eefe1010a465 ("V4L/DVB (10759): cx18: Convert GPIO connected
>> functions to act as v4l2_subdevices")
>>
>> It has a comment saying it's exported for use by 'lirc_pvr150' but I don't
>> see any sign of it in the lirc git, and I see it removed support
>> for lirc_i2c.c 'Flavors of the Hauppage PVR-150...' in 2014.
>>
>> Remove it.
>
> Interesting, I can't find any call site either. The ir-i2c-kbd driver could
> potentially use this, but it would to know the correct v4l2_dev for the
> device; also there are devices other than the cx18 which use the same IR
> module, so they would not have a way to force a reset or need a different
> mechanism (e.g. ivtv driver).
>
> So I don't understand how this could be wired up or how it was ever wired
> up.
>
> This could be great because if done correctly, we could remove the
> VIDIOC_INT_RESET ioctl completely. Then again, I don't know how often the
> device hangs. With the current driver the IR module I don't know of any
> hangs -- maybe the ioctl could just go anyway.
>
>
> Sean
>
>>
>> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
>> ---
>> drivers/media/pci/cx18/cx18-gpio.c | 15 ---------------
>> drivers/media/pci/cx18/cx18-gpio.h | 1 -
>> 2 files changed, 16 deletions(-)
>>
>> diff --git a/drivers/media/pci/cx18/cx18-gpio.c b/drivers/media/pci/cx18/cx18-gpio.c
>> index c85eb8d25837..485a6cbeb15a 100644
>> --- a/drivers/media/pci/cx18/cx18-gpio.c
>> +++ b/drivers/media/pci/cx18/cx18-gpio.c
>> @@ -305,21 +305,6 @@ int cx18_gpio_register(struct cx18 *cx, u32 hw)
>> return v4l2_device_register_subdev(&cx->v4l2_dev, sd);
>> }
>>
>> -void cx18_reset_ir_gpio(void *data)
>> -{
>> - struct cx18 *cx = to_cx18(data);
>> -
>> - if (cx->card->gpio_i2c_slave_reset.ir_reset_mask == 0)
>> - return;
>> -
>> - CX18_DEBUG_INFO("Resetting IR microcontroller\n");
>> -
>> - v4l2_subdev_call(&cx->sd_resetctrl,
>> - core, reset, CX18_GPIO_RESET_Z8F0811);

Ah, this calls core.reset. But VIDIOC_INT_RESET in cx18_default() does the
same (actually, it calls this for all subdevs). So dropping this code should
be fine since you can still do the same thing with cx18-ctl --reset.

This function was probably used a long time ago by lirc_pvr150, but I can't
even find the source for that anymore. I assume it has a modern replacement.

This is the original commit adding this function:

commit 02fa272fcb6edda9059d6dbaab20dfe919f4f4d2
Author: Andy Walls <awalls@radix.net>
Date: Sun Jul 13 19:30:15 2008 -0300

V4L/DVB (8332): cx18: Suport external reset of the Z8F0811 IR controller on HVR-1600 for lirc

cx18: added in cx18_ir_reset_gpio function for lirc_pvr150 like module. Also
added the ability to reset the IR chip via ioctl like ivtv. This needs the
mutex to protect gpio_dir and gpio_val in struct cx18 as gpio changes can
come from a few different asynchronous sources now.

Signed-off-by: Andy Walls <awalls@radix.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

Perhaps Andy remembers how frequent the lock ups were.

Regards,

Hans

>> -}
>> -EXPORT_SYMBOL(cx18_reset_ir_gpio);
>> -/* This symbol is exported for use by lirc_pvr150 for the IR-blaster */
>> -
>> /* Xceive tuner reset function */
>> int cx18_reset_tuner_gpio(void *dev, int component, int cmd, int value)
>> {
>> diff --git a/drivers/media/pci/cx18/cx18-gpio.h b/drivers/media/pci/cx18/cx18-gpio.h
>> index 0fa4c7ad2286..8d5797dea7f5 100644
>> --- a/drivers/media/pci/cx18/cx18-gpio.h
>> +++ b/drivers/media/pci/cx18/cx18-gpio.h
>> @@ -17,5 +17,4 @@ enum cx18_gpio_reset_type {
>> CX18_GPIO_RESET_XC2028 = 2,
>> };
>>
>> -void cx18_reset_ir_gpio(void *data);
>> int cx18_reset_tuner_gpio(void *dev, int component, int cmd, int value);
>> --
>> 2.47.0
>>
>