Hi Ezequiel,

I just test built the kernel with my patch included and things run fine. To guide me in my assignment, could you please clarify what Hans Verkuil had been mentioning would be a good thing to code, he said in one of our previous emails:

What tw686x_video_free() does really should be done in the release function
of the video_device: vdev->release is currently set to video_device_release,
but that should be a custom function that calls dev->dma_ops->free.

But in order to provide a thorough explanation that makes sense, as you mentioned and that I'd like to do, does the custom function and edits I made provide the functionality Hans mentioned, as per how it is seen in the code below?:

(Please note I didn't provide a description this time as the purpose of this email is to get a better idea of how to make one)

From 6d38673ca7d2206b21deb7d28971f52ee8453346 Mon Sep 17 00:00:00 2001
From: Mark Balantzyan <mbala...@gmail.com>
Date: Wed, 24 Jul 2019 14:01:30 -0700
Subject: [PATCH] media input infrastructure:tw686x: Added custom function to provide dev->dma_ops->free for vdev->release in tw686x_video_init() in tw686x driver

---
 drivers/media/pci/tw686x/tw686x-video.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index 3a06c000..1b875a7c 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -1151,18 +1151,32 @@ void tw686x_video_irq(struct tw686x_dev *dev, unsigned long requests,
     }
 }

+void tw686x_video_device_release(struct tw686x_video_channel *vc) {
+
+    struct tw686x_dev *dev = vc->dev;
+
+    unsigned int ch;
+
+    for (ch = 0; ch < max_channels(dev); ch++) {
+        struct tw686x_video_channel *vc = &dev->video_channels[ch];
+
+    }
+
+    dev->dma_ops->free;
+
+    video_device_release((struct video_device*)dev);
+
+}
+
 void tw686x_video_free(struct tw686x_dev *dev)
 {
-    unsigned int ch, pb;
+    unsigned int ch;

     for (ch = 0; ch < max_channels(dev); ch++) {
         struct tw686x_video_channel *vc = &dev->video_channels[ch];

         video_unregister_device(vc->device);

-        if (dev->dma_ops->free)
-            for (pb = 0; pb < 2; pb++)
-                dev->dma_ops->free(vc, pb);
     }
 }

@@ -1277,7 +1291,7 @@ int tw686x_video_init(struct tw686x_dev *dev)
         snprintf(vdev->name, sizeof(vdev->name), "%s video", dev->name);
         vdev->fops = &tw686x_video_fops;
         vdev->ioctl_ops = &tw686x_video_ioctl_ops;
-        vdev->release = video_device_release;
+        vdev->release = tw686x_video_device_release;
         vdev->v4l2_dev = &dev->v4l2_dev;
         vdev->queue = &vc->vidq;
         vdev->tvnorms = V4L2_STD_525_60 | V4L2_STD_625_50;
--
2.17.1

Reply via email to