linux-kernel - Re: [PATCH v2] media: cx18: Remove unused cx18_reset_ir

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b580805b-8f00-40d1-bc1c-3ac19a19ac45@xs4all.nl>
Date: Tue, 29 Oct 2024 11:57:16 +0100
From: Hans Verkuil <hverkuil@...all.nl>
To: Sean Young <sean@...s.org>, linux@...blig.org
Cc: awalls@...metrocast.net, mchehab@...nel.org, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] media: cx18: Remove unused cx18_reset_ir_gpio

On 10/28/24 20:58, Sean Young wrote:
> On Sun, Oct 13, 2024 at 12:39:32AM +0100, linux@...blig.org wrote:
>> From: "Dr. David Alan Gilbert" <linux@...blig.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@...blig.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@...ix.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@...ix.net>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@...radead.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
>>
>