All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tm6000: Add parameter to keep urb bufs allocated.
@ 2012-10-04 14:04 Julian Scheel
  2012-10-04 17:35 ` Ezequiel Garcia
  0 siblings, 1 reply; 4+ messages in thread
From: Julian Scheel @ 2012-10-04 14:04 UTC (permalink / raw)
  To: linux-media; +Cc: Julian Scheel

On systems where it cannot be assured that enough continous memory is available
all the time it can be very useful to only allocate the memory once when it is
needed the first time. Afterwards the initially allocated memory will be
reused, so it is ensured that the memory will stay available until the driver
is unloaded.

Signed-off-by: Julian Scheel <julian@jusst.de>
---
 drivers/media/video/tm6000/tm6000-video.c | 111 +++++++++++++++++++++++++-----
 drivers/media/video/tm6000/tm6000.h       |   5 ++
 2 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/drivers/media/video/tm6000/tm6000-video.c b/drivers/media/video/tm6000/tm6000-video.c
index 03de3d8..1b8db35 100644
--- a/drivers/media/video/tm6000/tm6000-video.c
+++ b/drivers/media/video/tm6000/tm6000-video.c
@@ -49,12 +49,15 @@
 #define TM6000_MIN_BUF 4
 #define TM6000_DEF_BUF 8
 
+#define TM6000_NUM_URB_BUF 8
+
 #define TM6000_MAX_ISO_PACKETS	46	/* Max number of ISO packets */
 
 /* Declare static vars that will be used as parameters */
 static unsigned int vid_limit = 16;	/* Video memory limit, in Mb */
 static int video_nr = -1;		/* /dev/videoN, -1 for autodetect */
 static int radio_nr = -1;		/* /dev/radioN, -1 for autodetect */
+static int keep_urb = 0;		/* keep urb buffers allocated */
 
 /* Debug level */
 int tm6000_debug;
@@ -570,6 +573,70 @@ static void tm6000_irq_callback(struct urb *urb)
 }
 
 /*
+ * Allocate URB buffers
+ */
+static int tm6000_alloc_urb_buffers(struct tm6000_core *dev)
+{
+	int num_bufs = TM6000_NUM_URB_BUF;
+	int i;
+
+	if (dev->urb_buffer != NULL)
+		return 0;
+
+	dev->urb_buffer = kmalloc(sizeof(void *)*num_bufs, GFP_KERNEL);
+	if (!dev->urb_buffer) {
+		tm6000_err("cannot allocate memory for urb buffers\n");
+		return -ENOMEM;
+	}
+
+	dev->urb_dma = kmalloc(sizeof(dma_addr_t *)*num_bufs, GFP_KERNEL);
+	if (!dev->urb_dma) {
+		tm6000_err("cannot allocate memory for urb dma pointers\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_bufs; i++) {
+		dev->urb_buffer[i] = usb_alloc_coherent(dev->udev, dev->urb_size,
+					GFP_KERNEL, &dev->urb_dma[i]);
+		if (!dev->urb_buffer[i]) {
+			tm6000_err("unable to allocate %i bytes for transfer"
+					" buffer %i\n", dev->urb_size, i);
+			return -ENOMEM;
+		}
+		memset(dev->urb_buffer[i], 0, dev->urb_size);
+	}
+
+	return 0;
+}
+
+/*
+ * Free URB buffers
+ */
+static int tm6000_free_urb_buffers(struct tm6000_core *dev)
+{
+	int i;
+
+	if (dev->urb_buffer == NULL)
+		return 0;
+
+	for (i = 0; i < TM6000_NUM_URB_BUF; i++) {
+		if (dev->urb_buffer[i]) {
+			usb_free_coherent(dev->udev,
+					dev->urb_size,
+					dev->urb_buffer[i],
+					dev->urb_dma[i]);
+			dev->urb_buffer[i] = NULL;
+		}
+	}
+	kfree (dev->urb_buffer);
+	kfree (dev->urb_dma);
+	dev->urb_buffer = NULL;
+	dev->urb_dma = NULL;
+
+	return 0;
+}
+
+/*
  * Stop and Deallocate URBs
  */
 static void tm6000_uninit_isoc(struct tm6000_core *dev)
@@ -585,18 +652,15 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
 		if (urb) {
 			usb_kill_urb(urb);
 			usb_unlink_urb(urb);
-			if (dev->isoc_ctl.transfer_buffer[i]) {
-				usb_free_coherent(dev->udev,
-						urb->transfer_buffer_length,
-						dev->isoc_ctl.transfer_buffer[i],
-						urb->transfer_dma);
-			}
 			usb_free_urb(urb);
 			dev->isoc_ctl.urb[i] = NULL;
 		}
 		dev->isoc_ctl.transfer_buffer[i] = NULL;
 	}
 
+	if (!keep_urb)
+		tm6000_free_urb_buffers(dev);
+
 	kfree(dev->isoc_ctl.urb);
 	kfree(dev->isoc_ctl.transfer_buffer);
 
@@ -606,12 +670,12 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
 }
 
 /*
- * Allocate URBs and start IRQ
+ * Assign URBs and start IRQ
  */
 static int tm6000_prepare_isoc(struct tm6000_core *dev)
 {
 	struct tm6000_dmaqueue *dma_q = &dev->vidq;
-	int i, j, sb_size, pipe, size, max_packets, num_bufs = 8;
+	int i, j, sb_size, pipe, size, max_packets, num_bufs = TM6000_NUM_URB_BUF;
 	struct urb *urb;
 
 	/* De-allocates all pending stuff */
@@ -634,6 +698,7 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
 
 	max_packets = TM6000_MAX_ISO_PACKETS;
 	sb_size = max_packets * size;
+	dev->urb_size = sb_size;
 
 	dev->isoc_ctl.num_bufs = num_bufs;
 
@@ -656,6 +721,17 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
 		    max_packets, num_bufs, sb_size,
 		    dev->isoc_in.maxsize, size);
 
+
+	if (!dev->urb_buffer && tm6000_alloc_urb_buffers(dev) < 0) {
+		tm6000_err("cannot allocate memory for urb buffers\n");
+
+		/* call free, as some buffers might have been allocated */
+		tm6000_free_urb_buffers(dev);
+		kfree(dev->isoc_ctl.urb);
+		kfree(dev->isoc_ctl.transfer_buffer);
+		return -ENOMEM;
+	}
+
 	/* allocate urbs and transfer buffers */
 	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
 		urb = usb_alloc_urb(max_packets, GFP_KERNEL);
@@ -667,17 +743,8 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
 		}
 		dev->isoc_ctl.urb[i] = urb;
 
-		dev->isoc_ctl.transfer_buffer[i] = usb_alloc_coherent(dev->udev,
-			sb_size, GFP_KERNEL, &urb->transfer_dma);
-		if (!dev->isoc_ctl.transfer_buffer[i]) {
-			tm6000_err("unable to allocate %i bytes for transfer"
-					" buffer %i%s\n",
-					sb_size, i,
-					in_interrupt() ? " while in int" : "");
-			tm6000_uninit_isoc(dev);
-			return -ENOMEM;
-		}
-		memset(dev->isoc_ctl.transfer_buffer[i], 0, sb_size);
+		urb->transfer_dma = dev->urb_dma[i];
+		dev->isoc_ctl.transfer_buffer[i] = dev->urb_buffer[i];
 
 		usb_fill_bulk_urb(urb, dev->udev, pipe,
 				  dev->isoc_ctl.transfer_buffer[i], sb_size,
@@ -1833,6 +1900,9 @@ int tm6000_v4l2_unregister(struct tm6000_core *dev)
 {
 	video_unregister_device(dev->vfd);
 
+	/* if URB buffers are still allocated free them now */
+	tm6000_free_urb_buffers(dev);
+
 	if (dev->radio_dev) {
 		if (video_is_registered(dev->radio_dev))
 			video_unregister_device(dev->radio_dev);
@@ -1858,3 +1928,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
 module_param(vid_limit, int, 0644);
 MODULE_PARM_DESC(vid_limit, "capture memory limit in megabytes");
 
+module_param(keep_urb, bool, 0);
+MODULE_PARM_DESC(keep_urb, "Keep urb buffers allocated even when the device "
+				"is closed by the user");
diff --git a/drivers/media/video/tm6000/tm6000.h b/drivers/media/video/tm6000/tm6000.h
index 6531d16..4bd3a0d 100644
--- a/drivers/media/video/tm6000/tm6000.h
+++ b/drivers/media/video/tm6000/tm6000.h
@@ -267,6 +267,11 @@ struct tm6000_core {
 
 	spinlock_t                   slock;
 
+	/* urb dma buffers */
+	char				**urb_buffer;
+	dma_addr_t			*urb_dma;
+	unsigned int			urb_size;
+
 	unsigned long quirks;
 };
 
-- 
1.7.12.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tm6000: Add parameter to keep urb bufs allocated.
  2012-10-04 14:04 [PATCH] tm6000: Add parameter to keep urb bufs allocated Julian Scheel
@ 2012-10-04 17:35 ` Ezequiel Garcia
  2012-10-08  8:49   ` Julian Scheel
  0 siblings, 1 reply; 4+ messages in thread
From: Ezequiel Garcia @ 2012-10-04 17:35 UTC (permalink / raw)
  To: Julian Scheel; +Cc: linux-media

Hi Julian,

Nice work! Just one pico-tiny nitpick:

On Thu, Oct 4, 2012 at 11:04 AM, Julian Scheel <julian@jusst.de> wrote:
> On systems where it cannot be assured that enough continous memory is available
> all the time it can be very useful to only allocate the memory once when it is
> needed the first time. Afterwards the initially allocated memory will be
> reused, so it is ensured that the memory will stay available until the driver
> is unloaded.
>
> Signed-off-by: Julian Scheel <julian@jusst.de>
> ---
>  drivers/media/video/tm6000/tm6000-video.c | 111 +++++++++++++++++++++++++-----
>  drivers/media/video/tm6000/tm6000.h       |   5 ++
>  2 files changed, 97 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/video/tm6000/tm6000-video.c b/drivers/media/video/tm6000/tm6000-video.c
> index 03de3d8..1b8db35 100644
> --- a/drivers/media/video/tm6000/tm6000-video.c
> +++ b/drivers/media/video/tm6000/tm6000-video.c
> @@ -49,12 +49,15 @@
>  #define TM6000_MIN_BUF 4
>  #define TM6000_DEF_BUF 8
>
> +#define TM6000_NUM_URB_BUF 8
> +
>  #define TM6000_MAX_ISO_PACKETS 46      /* Max number of ISO packets */
>
>  /* Declare static vars that will be used as parameters */
>  static unsigned int vid_limit = 16;    /* Video memory limit, in Mb */
>  static int video_nr = -1;              /* /dev/videoN, -1 for autodetect */
>  static int radio_nr = -1;              /* /dev/radioN, -1 for autodetect */
> +static int keep_urb = 0;               /* keep urb buffers allocated */
>

There's no need to initialize this one to zero.

>  /* Debug level */
>  int tm6000_debug;
> @@ -570,6 +573,70 @@ static void tm6000_irq_callback(struct urb *urb)
>  }
>
>  /*
> + * Allocate URB buffers
> + */
> +static int tm6000_alloc_urb_buffers(struct tm6000_core *dev)
> +{
> +       int num_bufs = TM6000_NUM_URB_BUF;
> +       int i;
> +
> +       if (dev->urb_buffer != NULL)
> +               return 0;
> +
> +       dev->urb_buffer = kmalloc(sizeof(void *)*num_bufs, GFP_KERNEL);
> +       if (!dev->urb_buffer) {
> +               tm6000_err("cannot allocate memory for urb buffers\n");
> +               return -ENOMEM;
> +       }
> +
> +       dev->urb_dma = kmalloc(sizeof(dma_addr_t *)*num_bufs, GFP_KERNEL);
> +       if (!dev->urb_dma) {
> +               tm6000_err("cannot allocate memory for urb dma pointers\n");
> +               return -ENOMEM;
> +       }
> +
> +       for (i = 0; i < num_bufs; i++) {
> +               dev->urb_buffer[i] = usb_alloc_coherent(dev->udev, dev->urb_size,
> +                                       GFP_KERNEL, &dev->urb_dma[i]);
> +               if (!dev->urb_buffer[i]) {
> +                       tm6000_err("unable to allocate %i bytes for transfer"
> +                                       " buffer %i\n", dev->urb_size, i);
> +                       return -ENOMEM;
> +               }
> +               memset(dev->urb_buffer[i], 0, dev->urb_size);
> +       }
> +
> +       return 0;
> +}
> +
> +/*
> + * Free URB buffers
> + */
> +static int tm6000_free_urb_buffers(struct tm6000_core *dev)
> +{
> +       int i;
> +
> +       if (dev->urb_buffer == NULL)
> +               return 0;
> +
> +       for (i = 0; i < TM6000_NUM_URB_BUF; i++) {
> +               if (dev->urb_buffer[i]) {
> +                       usb_free_coherent(dev->udev,
> +                                       dev->urb_size,
> +                                       dev->urb_buffer[i],
> +                                       dev->urb_dma[i]);
> +                       dev->urb_buffer[i] = NULL;
> +               }
> +       }
> +       kfree (dev->urb_buffer);
> +       kfree (dev->urb_dma);
> +       dev->urb_buffer = NULL;
> +       dev->urb_dma = NULL;
> +
> +       return 0;
> +}
> +
> +/*
>   * Stop and Deallocate URBs
>   */
>  static void tm6000_uninit_isoc(struct tm6000_core *dev)
> @@ -585,18 +652,15 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
>                 if (urb) {
>                         usb_kill_urb(urb);
>                         usb_unlink_urb(urb);
> -                       if (dev->isoc_ctl.transfer_buffer[i]) {
> -                               usb_free_coherent(dev->udev,
> -                                               urb->transfer_buffer_length,
> -                                               dev->isoc_ctl.transfer_buffer[i],
> -                                               urb->transfer_dma);
> -                       }
>                         usb_free_urb(urb);
>                         dev->isoc_ctl.urb[i] = NULL;
>                 }
>                 dev->isoc_ctl.transfer_buffer[i] = NULL;
>         }
>
> +       if (!keep_urb)
> +               tm6000_free_urb_buffers(dev);
> +
>         kfree(dev->isoc_ctl.urb);
>         kfree(dev->isoc_ctl.transfer_buffer);
>
> @@ -606,12 +670,12 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
>  }
>
>  /*
> - * Allocate URBs and start IRQ
> + * Assign URBs and start IRQ
>   */
>  static int tm6000_prepare_isoc(struct tm6000_core *dev)
>  {
>         struct tm6000_dmaqueue *dma_q = &dev->vidq;
> -       int i, j, sb_size, pipe, size, max_packets, num_bufs = 8;
> +       int i, j, sb_size, pipe, size, max_packets, num_bufs = TM6000_NUM_URB_BUF;
>         struct urb *urb;
>
>         /* De-allocates all pending stuff */
> @@ -634,6 +698,7 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
>
>         max_packets = TM6000_MAX_ISO_PACKETS;
>         sb_size = max_packets * size;
> +       dev->urb_size = sb_size;
>
>         dev->isoc_ctl.num_bufs = num_bufs;
>
> @@ -656,6 +721,17 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
>                     max_packets, num_bufs, sb_size,
>                     dev->isoc_in.maxsize, size);
>
> +
> +       if (!dev->urb_buffer && tm6000_alloc_urb_buffers(dev) < 0) {
> +               tm6000_err("cannot allocate memory for urb buffers\n");
> +
> +               /* call free, as some buffers might have been allocated */
> +               tm6000_free_urb_buffers(dev);
> +               kfree(dev->isoc_ctl.urb);
> +               kfree(dev->isoc_ctl.transfer_buffer);
> +               return -ENOMEM;
> +       }
> +
>         /* allocate urbs and transfer buffers */
>         for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
>                 urb = usb_alloc_urb(max_packets, GFP_KERNEL);
> @@ -667,17 +743,8 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
>                 }
>                 dev->isoc_ctl.urb[i] = urb;
>
> -               dev->isoc_ctl.transfer_buffer[i] = usb_alloc_coherent(dev->udev,
> -                       sb_size, GFP_KERNEL, &urb->transfer_dma);
> -               if (!dev->isoc_ctl.transfer_buffer[i]) {
> -                       tm6000_err("unable to allocate %i bytes for transfer"
> -                                       " buffer %i%s\n",
> -                                       sb_size, i,
> -                                       in_interrupt() ? " while in int" : "");
> -                       tm6000_uninit_isoc(dev);
> -                       return -ENOMEM;
> -               }
> -               memset(dev->isoc_ctl.transfer_buffer[i], 0, sb_size);
> +               urb->transfer_dma = dev->urb_dma[i];
> +               dev->isoc_ctl.transfer_buffer[i] = dev->urb_buffer[i];
>
>                 usb_fill_bulk_urb(urb, dev->udev, pipe,
>                                   dev->isoc_ctl.transfer_buffer[i], sb_size,
> @@ -1833,6 +1900,9 @@ int tm6000_v4l2_unregister(struct tm6000_core *dev)
>  {
>         video_unregister_device(dev->vfd);
>
> +       /* if URB buffers are still allocated free them now */
> +       tm6000_free_urb_buffers(dev);
> +
>         if (dev->radio_dev) {
>                 if (video_is_registered(dev->radio_dev))
>                         video_unregister_device(dev->radio_dev);
> @@ -1858,3 +1928,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
>  module_param(vid_limit, int, 0644);
>  MODULE_PARM_DESC(vid_limit, "capture memory limit in megabytes");
>
> +module_param(keep_urb, bool, 0);
> +MODULE_PARM_DESC(keep_urb, "Keep urb buffers allocated even when the device "
> +                               "is closed by the user");
> diff --git a/drivers/media/video/tm6000/tm6000.h b/drivers/media/video/tm6000/tm6000.h
> index 6531d16..4bd3a0d 100644
> --- a/drivers/media/video/tm6000/tm6000.h
> +++ b/drivers/media/video/tm6000/tm6000.h
> @@ -267,6 +267,11 @@ struct tm6000_core {
>
>         spinlock_t                   slock;
>
> +       /* urb dma buffers */
> +       char                            **urb_buffer;
> +       dma_addr_t                      *urb_dma;
> +       unsigned int                    urb_size;
> +
>         unsigned long quirks;
>  };
>
> --
> 1.7.12.2
>

Thanks,
Ezequiel.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tm6000: Add parameter to keep urb bufs allocated.
  2012-10-04 17:35 ` Ezequiel Garcia
@ 2012-10-08  8:49   ` Julian Scheel
  2012-10-08 17:36     ` Ezequiel Garcia
  0 siblings, 1 reply; 4+ messages in thread
From: Julian Scheel @ 2012-10-08  8:49 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-media

Hi Ezequiel,

Am Donnerstag, den 04.10.2012, 14:35 -0300 schrieb Ezequiel Garcia:
> Nice work! Just one pico-tiny nitpick:

Should I update the patch to reflect this? Or is it ok if the maintainer
integrated your proposal when comitting it?

> On Thu, Oct 4, 2012 at 11:04 AM, Julian Scheel <julian@jusst.de> wrote:
> > On systems where it cannot be assured that enough continous memory is available
> > all the time it can be very useful to only allocate the memory once when it is
> > needed the first time. Afterwards the initially allocated memory will be
> > reused, so it is ensured that the memory will stay available until the driver
> > is unloaded.
> >
> > Signed-off-by: Julian Scheel <julian@jusst.de>
> > ---
> >  drivers/media/video/tm6000/tm6000-video.c | 111 +++++++++++++++++++++++++-----
> >  drivers/media/video/tm6000/tm6000.h       |   5 ++
> >  2 files changed, 97 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/media/video/tm6000/tm6000-video.c b/drivers/media/video/tm6000/tm6000-video.c
> > index 03de3d8..1b8db35 100644
> > --- a/drivers/media/video/tm6000/tm6000-video.c
> > +++ b/drivers/media/video/tm6000/tm6000-video.c
> > @@ -49,12 +49,15 @@
> >  #define TM6000_MIN_BUF 4
> >  #define TM6000_DEF_BUF 8
> >
> > +#define TM6000_NUM_URB_BUF 8
> > +
> >  #define TM6000_MAX_ISO_PACKETS 46      /* Max number of ISO packets */
> >
> >  /* Declare static vars that will be used as parameters */
> >  static unsigned int vid_limit = 16;    /* Video memory limit, in Mb */
> >  static int video_nr = -1;              /* /dev/videoN, -1 for autodetect */
> >  static int radio_nr = -1;              /* /dev/radioN, -1 for autodetect */
> > +static int keep_urb = 0;               /* keep urb buffers allocated */
> >
> 
> There's no need to initialize this one to zero.
> 
> >  /* Debug level */
> >  int tm6000_debug;
> > @@ -570,6 +573,70 @@ static void tm6000_irq_callback(struct urb *urb)
> >  }
> >
> >  /*
> > + * Allocate URB buffers
> > + */
> > +static int tm6000_alloc_urb_buffers(struct tm6000_core *dev)
> > +{
> > +       int num_bufs = TM6000_NUM_URB_BUF;
> > +       int i;
> > +
> > +       if (dev->urb_buffer != NULL)
> > +               return 0;
> > +
> > +       dev->urb_buffer = kmalloc(sizeof(void *)*num_bufs, GFP_KERNEL);
> > +       if (!dev->urb_buffer) {
> > +               tm6000_err("cannot allocate memory for urb buffers\n");
> > +               return -ENOMEM;
> > +       }
> > +
> > +       dev->urb_dma = kmalloc(sizeof(dma_addr_t *)*num_bufs, GFP_KERNEL);
> > +       if (!dev->urb_dma) {
> > +               tm6000_err("cannot allocate memory for urb dma pointers\n");
> > +               return -ENOMEM;
> > +       }
> > +
> > +       for (i = 0; i < num_bufs; i++) {
> > +               dev->urb_buffer[i] = usb_alloc_coherent(dev->udev, dev->urb_size,
> > +                                       GFP_KERNEL, &dev->urb_dma[i]);
> > +               if (!dev->urb_buffer[i]) {
> > +                       tm6000_err("unable to allocate %i bytes for transfer"
> > +                                       " buffer %i\n", dev->urb_size, i);
> > +                       return -ENOMEM;
> > +               }
> > +               memset(dev->urb_buffer[i], 0, dev->urb_size);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/*
> > + * Free URB buffers
> > + */
> > +static int tm6000_free_urb_buffers(struct tm6000_core *dev)
> > +{
> > +       int i;
> > +
> > +       if (dev->urb_buffer == NULL)
> > +               return 0;
> > +
> > +       for (i = 0; i < TM6000_NUM_URB_BUF; i++) {
> > +               if (dev->urb_buffer[i]) {
> > +                       usb_free_coherent(dev->udev,
> > +                                       dev->urb_size,
> > +                                       dev->urb_buffer[i],
> > +                                       dev->urb_dma[i]);
> > +                       dev->urb_buffer[i] = NULL;
> > +               }
> > +       }
> > +       kfree (dev->urb_buffer);
> > +       kfree (dev->urb_dma);
> > +       dev->urb_buffer = NULL;
> > +       dev->urb_dma = NULL;
> > +
> > +       return 0;
> > +}
> > +
> > +/*
> >   * Stop and Deallocate URBs
> >   */
> >  static void tm6000_uninit_isoc(struct tm6000_core *dev)
> > @@ -585,18 +652,15 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
> >                 if (urb) {
> >                         usb_kill_urb(urb);
> >                         usb_unlink_urb(urb);
> > -                       if (dev->isoc_ctl.transfer_buffer[i]) {
> > -                               usb_free_coherent(dev->udev,
> > -                                               urb->transfer_buffer_length,
> > -                                               dev->isoc_ctl.transfer_buffer[i],
> > -                                               urb->transfer_dma);
> > -                       }
> >                         usb_free_urb(urb);
> >                         dev->isoc_ctl.urb[i] = NULL;
> >                 }
> >                 dev->isoc_ctl.transfer_buffer[i] = NULL;
> >         }
> >
> > +       if (!keep_urb)
> > +               tm6000_free_urb_buffers(dev);
> > +
> >         kfree(dev->isoc_ctl.urb);
> >         kfree(dev->isoc_ctl.transfer_buffer);
> >
> > @@ -606,12 +670,12 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
> >  }
> >
> >  /*
> > - * Allocate URBs and start IRQ
> > + * Assign URBs and start IRQ
> >   */
> >  static int tm6000_prepare_isoc(struct tm6000_core *dev)
> >  {
> >         struct tm6000_dmaqueue *dma_q = &dev->vidq;
> > -       int i, j, sb_size, pipe, size, max_packets, num_bufs = 8;
> > +       int i, j, sb_size, pipe, size, max_packets, num_bufs = TM6000_NUM_URB_BUF;
> >         struct urb *urb;
> >
> >         /* De-allocates all pending stuff */
> > @@ -634,6 +698,7 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
> >
> >         max_packets = TM6000_MAX_ISO_PACKETS;
> >         sb_size = max_packets * size;
> > +       dev->urb_size = sb_size;
> >
> >         dev->isoc_ctl.num_bufs = num_bufs;
> >
> > @@ -656,6 +721,17 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
> >                     max_packets, num_bufs, sb_size,
> >                     dev->isoc_in.maxsize, size);
> >
> > +
> > +       if (!dev->urb_buffer && tm6000_alloc_urb_buffers(dev) < 0) {
> > +               tm6000_err("cannot allocate memory for urb buffers\n");
> > +
> > +               /* call free, as some buffers might have been allocated */
> > +               tm6000_free_urb_buffers(dev);
> > +               kfree(dev->isoc_ctl.urb);
> > +               kfree(dev->isoc_ctl.transfer_buffer);
> > +               return -ENOMEM;
> > +       }
> > +
> >         /* allocate urbs and transfer buffers */
> >         for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
> >                 urb = usb_alloc_urb(max_packets, GFP_KERNEL);
> > @@ -667,17 +743,8 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
> >                 }
> >                 dev->isoc_ctl.urb[i] = urb;
> >
> > -               dev->isoc_ctl.transfer_buffer[i] = usb_alloc_coherent(dev->udev,
> > -                       sb_size, GFP_KERNEL, &urb->transfer_dma);
> > -               if (!dev->isoc_ctl.transfer_buffer[i]) {
> > -                       tm6000_err("unable to allocate %i bytes for transfer"
> > -                                       " buffer %i%s\n",
> > -                                       sb_size, i,
> > -                                       in_interrupt() ? " while in int" : "");
> > -                       tm6000_uninit_isoc(dev);
> > -                       return -ENOMEM;
> > -               }
> > -               memset(dev->isoc_ctl.transfer_buffer[i], 0, sb_size);
> > +               urb->transfer_dma = dev->urb_dma[i];
> > +               dev->isoc_ctl.transfer_buffer[i] = dev->urb_buffer[i];
> >
> >                 usb_fill_bulk_urb(urb, dev->udev, pipe,
> >                                   dev->isoc_ctl.transfer_buffer[i], sb_size,
> > @@ -1833,6 +1900,9 @@ int tm6000_v4l2_unregister(struct tm6000_core *dev)
> >  {
> >         video_unregister_device(dev->vfd);
> >
> > +       /* if URB buffers are still allocated free them now */
> > +       tm6000_free_urb_buffers(dev);
> > +
> >         if (dev->radio_dev) {
> >                 if (video_is_registered(dev->radio_dev))
> >                         video_unregister_device(dev->radio_dev);
> > @@ -1858,3 +1928,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
> >  module_param(vid_limit, int, 0644);
> >  MODULE_PARM_DESC(vid_limit, "capture memory limit in megabytes");
> >
> > +module_param(keep_urb, bool, 0);
> > +MODULE_PARM_DESC(keep_urb, "Keep urb buffers allocated even when the device "
> > +                               "is closed by the user");
> > diff --git a/drivers/media/video/tm6000/tm6000.h b/drivers/media/video/tm6000/tm6000.h
> > index 6531d16..4bd3a0d 100644
> > --- a/drivers/media/video/tm6000/tm6000.h
> > +++ b/drivers/media/video/tm6000/tm6000.h
> > @@ -267,6 +267,11 @@ struct tm6000_core {
> >
> >         spinlock_t                   slock;
> >
> > +       /* urb dma buffers */
> > +       char                            **urb_buffer;
> > +       dma_addr_t                      *urb_dma;
> > +       unsigned int                    urb_size;
> > +
> >         unsigned long quirks;
> >  };
> >
> > --
> > 1.7.12.2
> >



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tm6000: Add parameter to keep urb bufs allocated.
  2012-10-08  8:49   ` Julian Scheel
@ 2012-10-08 17:36     ` Ezequiel Garcia
  0 siblings, 0 replies; 4+ messages in thread
From: Ezequiel Garcia @ 2012-10-08 17:36 UTC (permalink / raw)
  To: Julian Scheel; +Cc: linux-media

On Mon, Oct 8, 2012 at 5:49 AM, Julian Scheel <julian@jusst.de> wrote:
> Hi Ezequiel,
>
> Am Donnerstag, den 04.10.2012, 14:35 -0300 schrieb Ezequiel Garcia:
>> Nice work! Just one pico-tiny nitpick:
>
> Should I update the patch to reflect this? Or is it ok if the maintainer
> integrated your proposal when comitting it?
>

You can re-send a new patch with this subject:

[PATCH v2] tm6000: Add parameter to keep urb bufs allocated

Like here:
https://lkml.org/lkml/2012/9/23/128

Notice you can place comments (like a patch changelog)
after the:

Signed-off
---

The maintainer will pick the latest version of each patch.

Thanks!
Ezequiel.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-10-08 17:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-04 14:04 [PATCH] tm6000: Add parameter to keep urb bufs allocated Julian Scheel
2012-10-04 17:35 ` Ezequiel Garcia
2012-10-08  8:49   ` Julian Scheel
2012-10-08 17:36     ` Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.