On Tue, Oct 29, 2024 at 11:57:16AM +0100, Hans Verkuil wrote:
> On 10/28/24 20:58, Sean Young wrote:
> > On Sun, Oct 13, 2024 at 12:39:32AM +0100, linux@xxxxxxxxxxx wrote:
> >> From: "Dr. David Alan Gilbert" <linux@xxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxx>
> >> ---
> >> 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.
That's true.
> 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.
ir-kbd-i2c.c handles this now, both receiving and sending. I've done some work
on this driver and done a fair amount of testing. It certainly is possible
to make the firmware hang - by sending it the wrong commands.
With the current driver I've never seen any hangs or heard of reports of hangs.
> This is the original commit adding this function:
>
> commit 02fa272fcb6edda9059d6dbaab20dfe919f4f4d2
> Author: Andy Walls <awalls@xxxxxxxxx>
> 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@xxxxxxxxx>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx>
>
> Perhaps Andy remembers how frequent the lock ups were.
>
> Regards,
>
> Hans
>
> >> -}
> >> -EXPORT_SYMBOL(cx18_reset_ir_gpio);
So I've been looking everywhere including the lirc cvs repo, and I can't where
this function is used. Looking at the history with cvs is awful of course,
so I may have missed something.
I think since we don't know of any lockups and we don't know of any users,
we can safely remove the function, i.e. let's apply this patch.
The reason I was looking for users of this function is that I thought it
would be nice if ir-kbd-i2c could use this function directly, and therefore
the cx18-ctl --reset is no longer needed (and no need for the ioctl either).
Something for another day.
Sean
