public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Malcolm Priestley <tvboxspy@gmail.com>
To: Florian Mickler <florian@mickler.org>
Cc: mchehab@infradead.org, oliver@neukum.org, jwjstone@fastmail.fm,
	linux-kernel@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@redhat.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 12/16] [media] lmedm04: get rid of on-stack dma buffers
Date: Tue, 15 Mar 2011 20:54:43 +0000	[thread overview]
Message-ID: <1300222483.1910.12.camel@localhost> (raw)
In-Reply-To: <1300178655-24832-12-git-send-email-florian@mickler.org>

The patch failed for the following reason.

On Tue, 2011-03-15 at 09:43 +0100, Florian Mickler wrote:
> usb_control_msg initiates (and waits for completion of) a dma transfer using
> the supplied buffer. That buffer thus has to be seperately allocated on
> the heap.
> 
> In lib/dma_debug.c the function check_for_stack even warns about it:
> 	WARNING: at lib/dma-debug.c:866 check_for_stack
> 
> Note: This change is tested to compile only, as I don't have the hardware.
> 
> Signed-off-by: Florian Mickler <florian@mickler.org>
> ---
>  drivers/media/dvb/dvb-usb/lmedm04.c |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c
> index 0a3e88f..bec5439 100644
> --- a/drivers/media/dvb/dvb-usb/lmedm04.c
> +++ b/drivers/media/dvb/dvb-usb/lmedm04.c
> @@ -314,12 +314,17 @@ static int lme2510_int_read(struct dvb_usb_adapter *adap)
>  static int lme2510_return_status(struct usb_device *dev)
>  {
>  	int ret = 0;
> -	u8 data[10] = {0};
> +	u8 *data;
> +
> +	data = kzalloc(10, GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
>  
>  	ret |= usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
>  			0x06, 0x80, 0x0302, 0x00, data, 0x0006, 200);
>  	info("Firmware Status: %x (%x)", ret , data[2]);
>  
> +	kfree(data);
>  	return (ret < 0) ? -ENODEV : data[2];

data has been killed off when we need the buffer contents.
changing to the following fixed.

	ret = (ret < 0) ? -ENODEV : data[2];

	kfree(data);

	return ret;


> @@ -603,7 +608,7 @@ static int lme2510_download_firmware(struct usb_device *dev,
>  					const struct firmware *fw)
>  {
>  	int ret = 0;
> -	u8 data[512] = {0};
> +	u8 *data;
>  	u16 j, wlen, len_in, start, end;
>  	u8 packet_size, dlen, i;
>  	u8 *fw_data;
> @@ -611,6 +616,11 @@ static int lme2510_download_firmware(struct usb_device *dev,
>  	packet_size = 0x31;
>  	len_in = 1;
>  
> +	data = kzalloc(512, GFP_KERNEL);
> +	if (!data) {
> +		info("FRM Could not start Firmware Download (Buffer allocation failed)");

Longer than 80 characters, 

> +		return -ENOMEM;
> +	}
>  
>  	info("FRM Starting Firmware Download");
>  
> @@ -654,7 +664,7 @@ static int lme2510_download_firmware(struct usb_device *dev,
>  	else
>  		info("FRM Firmware Download Completed - Resetting Device");
>  
> -
> +	kfree(data);
>  	return (ret < 0) ? -ENODEV : 0;
>  }
>  

Otherwise the patch as corrected has been put on test. No initial
problems have been encountered.

Regards 

Malcolm




  reply	other threads:[~2011-03-15 21:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-06 11:16 [PATCH] [media] dib0700: get rid of on-stack dma buffers Florian Mickler
2011-03-06 12:06 ` Oliver Neukum
2011-03-06 14:38   ` Florian Mickler
2011-03-06 14:45     ` [PATCH 1/3 v2] " Florian Mickler
2011-03-06 14:45       ` [PATCH 2/3] [media] dib0700: remove unused variable Florian Mickler
2011-03-06 14:45       ` [PATCH 3/3] [media] dib0700: don't ignore errors in driver probe Florian Mickler
2011-03-06 15:06     ` [PATCH] [media] dib0700: get rid of on-stack dma buffers Oliver Neukum
2011-03-06 15:45       ` Florian Mickler
2011-03-06 16:44         ` Oliver Neukum
2011-03-06 17:47           ` [PATCH 1/2 v3] " Florian Mickler
2011-03-06 17:47             ` [PATCH 2/2] [media] dib0700: remove unused variable Florian Mickler
2011-03-06 17:57             ` [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:36               ` Florian Mickler
2011-03-15  8:43                 ` [PATCH 01/16] " Florian Mickler
2011-03-15  8:43                   ` [PATCH 02/16] [media] dib0700: remove unused variable Florian Mickler
2011-03-15  8:43                   ` [PATCH 03/16] [media] a800: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:43                   ` [PATCH 04/16] [media] vp7045: " Florian Mickler
2011-03-15  8:43                   ` [PATCH 05/16] [media] ec168: get rid of on stack " Florian Mickler
2011-03-18 16:36                     ` Antti Palosaari
2011-03-18 21:33                       ` Florian Mickler
2011-03-15  8:43                   ` [PATCH 06/16] [media] ce6230: get rid of on-stack dma buffer Florian Mickler
2011-03-18 16:36                     ` Antti Palosaari
2011-03-18 21:28                       ` Florian Mickler
2011-03-15  8:43                   ` [PATCH 07/16] [media] friio: get rid of on-stack dma buffers Florian Mickler
2011-03-15  8:43                   ` [PATCH 11/16] [media] lmedm04: correct indentation Florian Mickler
2011-03-15  8:43                   ` [PATCH 12/16] [media] lmedm04: get rid of on-stack dma buffers Florian Mickler
2011-03-15 20:54                     ` Malcolm Priestley [this message]
2011-03-15 21:46                       ` Florian Mickler
2011-03-15 12:02                   ` [PATCH 01/16] [media] dib0700: " Mauro Carvalho Chehab
2011-03-15 21:14                     ` Florian Mickler
     [not found]                   ` <1300178655-24832-9-git-send-email-florian@mickler.org>
2011-03-18 16:34                     ` [PATCH 09/16] [media] au6610: get rid of on-stack dma buffer Antti Palosaari
2011-03-18 21:27                       ` Florian Mickler
2011-03-18 21:40                         ` Antti Palosaari
2011-03-15 11:37                 ` [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers Mauro Carvalho Chehab
2011-03-06 13:49 ` [PATCH] " Jack Stone
2011-03-06 14:00   ` Florian Mickler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1300222483.1910.12.camel@localhost \
    --to=tvboxspy@gmail.com \
    --cc=florian@mickler.org \
    --cc=jwjstone@fastmail.fm \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=mchehab@redhat.com \
    --cc=oliver@neukum.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox