Re: [PATCH v2] [media] vp702x: fix buffer handling
On Fri, 26 Aug 2011 00:11:15 +0200 Florian Mickler wrote: > In my previous change to this driver, I was not aware, that > dvb_usb_device_init > calls the frontend_attach routine which needs a transfer > buffer. So we can not setup anything private in the probe routine beforehand > but > have to allocate when needed. This means also that we cannot use a private > buffer mutex to serialize that buffer but instead need to use the > dvb_usb_device's usb_mutex. > > Fixes: https://bugzilla.novell.com/show_bug.cgi?id=709440 > > Tested-by: Markus Stephan > Signed-off-by: Florian Mickler > --- > > So, someone who could test that driver found me after all. > > I renamed the functions to get rid of that ugly and pointless _unlocked > suffix I > deliriously added earlier. Markus tested this patch modulo function renaming. > I am > so shure that this version will still work for him, that I already added his > Tested-by. *fingerscrossed* Hi Mauro! I just checked patchwork and in case you hold off on this because of my *fingerscrossed* remark above: Markus reported off-list that this version still works for him. Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: vp702x_fe_set_frontend+0x156/0x1a0 [dvb_usb_vp702x]
On Fri, 26 Aug 2011 23:17:51 +0200
Markus Stephan wrote:
> Hi Florian,
>
> Jiri made an other test kernel for me with your final + debug patch applied.
>
> The box works fine and the message:
> vp702x: usb in operation failed. (-110)
>
> seams to be triggered by:
> vp702x_fe_set_frontend+0x156/0x1a0 [dvb_usb_vp702x]
>
> Full dmesg is attached.
>
> Thank you,
> Markus Stephan
Hi!
Here is a patch to check if that failing op is even necessary.
But even if it works I don't feel comfortable of suggesting this to be
applied, since I don't have any overview about the hardware supported by this
driver.
Maybe Patrick has any opinion on this, he wrote this driver after all :)
Regards,
Flo
>8--->8>8->8>8-------
commit fdcb46dd3627683fc82d14488af10d3072e923f5
Author: Florian Mickler
Date: Sat Aug 27 12:01:34 2011 +0200
Let's check if that failing in-op is even necessary.
Even if this turns out to be unnecessary, the right course of action is
probably
to still leave it there and demote the error message to debug information.
diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c
b/drivers/media/dvb/dvb-usb/vp702x-fe.c
index ad16455..cb6c230 100644
--- a/drivers/media/dvb/dvb-usb/vp702x-fe.c
+++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c
@@ -198,7 +198,7 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
st->status_check_interval = 250;
st->next_status_check = jiffies;
- vp702x_usb_inout_op(st->d, cmd, 8, cmd, 10, 100);
+ vp702x_usb_out_op(st->d, REQUEST_OUT, 0, 0, cmd, 10);
if (cmd[2] == 0 && cmd[3] == 0)
deb_fe("tuning failed.\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] vp702x: fix buffer handling
In my previous change to this driver, I was not aware, that dvb_usb_device_init
calls the frontend_attach routine which needs a transfer
buffer. So we can not setup anything private in the probe routine beforehand but
have to allocate when needed. This means also that we cannot use a private
buffer mutex to serialize that buffer but instead need to use the
dvb_usb_device's usb_mutex.
Fixes: https://bugzilla.novell.com/show_bug.cgi?id=709440
Tested-by: Markus Stephan
Signed-off-by: Florian Mickler
---
So, someone who could test that driver found me after all.
I renamed the functions to get rid of that ugly and pointless _unlocked suffix I
deliriously added earlier. Markus tested this patch modulo function renaming. I
am
so shure that this version will still work for him, that I already added his
Tested-by. *fingerscrossed*
drivers/media/dvb/dvb-usb/vp702x-fe.c | 92 +++
drivers/media/dvb/dvb-usb/vp702x.c| 211 ++---
drivers/media/dvb/dvb-usb/vp702x.h| 13 ++-
3 files changed, 193 insertions(+), 123 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c
b/drivers/media/dvb/dvb-usb/vp702x-fe.c
index 2bb8d4c..ad16455 100644
--- a/drivers/media/dvb/dvb-usb/vp702x-fe.c
+++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c
@@ -43,21 +43,30 @@ static int vp702x_fe_refresh_state(struct vp702x_fe_state
*st)
{
struct vp702x_device_state *dst = st->d->priv;
u8 *buf;
+ int ret;
if (time_after(jiffies, st->next_status_check)) {
- mutex_lock(&dst->buf_mutex);
- buf = dst->buf;
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
+
+ ret = vp702x_buffer_setup(dst, &buf, 10);
+ if (ret)
+ goto unlock;
vp702x_usb_in_op(st->d, READ_STATUS, 0, 0, buf, 10);
st->lock = buf[4];
- vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0, buf, 1);
+ vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0,
+ buf, 1);
st->snr = buf[0];
- vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, buf, 1);
+ vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0,
+ buf, 1);
st->sig = buf[0];
- mutex_unlock(&dst->buf_mutex);
+unlock:
+ mutex_unlock(&st->d->usb_mutex);
st->next_status_check = jiffies +
(st->status_check_interval*HZ)/1000;
}
return 0;
@@ -145,11 +154,15 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
/* u16 frequencyRef[16] = { 2, 4, 8, 16, 32, 64, 128, 256, 24, 5, 10, 20,
40, 80, 160, 320 }; */
u64 sr;
u8 *cmd;
+ int ret;
- mutex_lock(&dst->buf_mutex);
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
- cmd = dst->buf;
- memset(cmd, 0, 10);
+ ret = vp702x_buffer_setup(dst, &cmd, 10);
+ if (ret)
+ goto unlock;
cmd[0] = (freq >> 8) & 0x7f;
cmd[1] = freq & 0xff;
@@ -191,17 +204,25 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
deb_fe("tuning failed.\n");
else
deb_fe("tuning succeeded.\n");
+ ret = 0;
- mutex_unlock(&dst->buf_mutex);
-
- return 0;
+unlock:
+ mutex_unlock(&st->d->usb_mutex);
+ return ret;
}
static int vp702x_fe_init(struct dvb_frontend *fe)
{
struct vp702x_fe_state *st = fe->demodulator_priv;
+ int ret;
+
deb_fe("%s\n",__func__);
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
vp702x_usb_in_op(st->d, RESET_TUNER, 0, 0, NULL, 0);
+
+ mutex_unlock(&st->d->usb_mutex);
return 0;
}
@@ -224,15 +245,21 @@ static int vp702x_fe_send_diseqc_msg (struct
dvb_frontend* fe,
u8 *cmd;
struct vp702x_fe_state *st = fe->demodulator_priv;
struct vp702x_device_state *dst = st->d->priv;
+ int ret;
deb_fe("%s\n",__func__);
if (m->msg_len > 4)
return -EINVAL;
- mutex_lock(&dst->buf_mutex);
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
+
+ ret = vp702x_buffer_setup(dst, &cmd, 10);
+ if (ret)
+ goto unlock;
- cmd = dst->buf;
cmd[1] = SET_DISEQC_CMD;
cmd[2] = m->msg_len;
memcpy(&cmd[3], m->msg, m->msg_len);
@@ -244,10 +271,12 @@
Re: [PATCH] [media] vp7045: fix buffer setup
On Wed, 10 Aug 2011 12:05:20 +0200 Florian Mickler wrote: > dvb_usb_device_init calls the frontend_attach method of this driver which > uses vp7045_usb_ob. In order to have a buffer ready in vp7045_usb_op, it has > to > be allocated before that happens. > > Luckily we can use the whole private data as the buffer as it gets separately > allocated on the heap via kzalloc in dvb_usb_device_init and is thus apt for > use via usb_control_msg. > > This fixes a > BUG: unable to handle kernel paging request at 1e78 > > reported by Tino Keitel and diagnosed by Dan Carpenter. > > References: https://bugzilla.kernel.org/show_bug.cgi?id=40062 > Cc: v3.0.y > Tested-by: Tino Keitel > Signed-off-by: Florian Mickler ...ping... -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] vp7045: fix buffer setup
dvb_usb_device_init calls the frontend_attach method of this driver which
uses vp7045_usb_ob. In order to have a buffer ready in vp7045_usb_op, it has to
be allocated before that happens.
Luckily we can use the whole private data as the buffer as it gets separately
allocated on the heap via kzalloc in dvb_usb_device_init and is thus apt for
use via usb_control_msg.
This fixes a
BUG: unable to handle kernel paging request at 1e78
reported by Tino Keitel and diagnosed by Dan Carpenter.
References: https://bugzilla.kernel.org/show_bug.cgi?id=40062
Cc: v3.0.y
Tested-by: Tino Keitel
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp7045.c | 26 --
1 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp7045.c
b/drivers/media/dvb/dvb-usb/vp7045.c
index 3db89e3..536c16c 100644
--- a/drivers/media/dvb/dvb-usb/vp7045.c
+++ b/drivers/media/dvb/dvb-usb/vp7045.c
@@ -224,26 +224,8 @@ static struct dvb_usb_device_properties vp7045_properties;
static int vp7045_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
- struct dvb_usb_device *d;
- int ret = dvb_usb_device_init(intf, &vp7045_properties,
- THIS_MODULE, &d, adapter_nr);
- if (ret)
- return ret;
-
- d->priv = kmalloc(20, GFP_KERNEL);
- if (!d->priv) {
- dvb_usb_device_exit(intf);
- return -ENOMEM;
- }
-
- return ret;
-}
-
-static void vp7045_usb_disconnect(struct usb_interface *intf)
-{
- struct dvb_usb_device *d = usb_get_intfdata(intf);
- kfree(d->priv);
- dvb_usb_device_exit(intf);
+ return dvb_usb_device_init(intf, &vp7045_properties,
+ THIS_MODULE, NULL, adapter_nr);
}
static struct usb_device_id vp7045_usb_table [] = {
@@ -258,7 +240,7 @@ MODULE_DEVICE_TABLE(usb, vp7045_usb_table);
static struct dvb_usb_device_properties vp7045_properties = {
.usb_ctrl = CYPRESS_FX2,
.firmware = "dvb-usb-vp7045-01.fw",
- .size_of_priv = sizeof(u8 *),
+ .size_of_priv = 20,
.num_adapters = 1,
.adapter = {
@@ -305,7 +287,7 @@ static struct dvb_usb_device_properties vp7045_properties =
{
static struct usb_driver vp7045_usb_driver = {
.name = "dvb_usb_vp7045",
.probe = vp7045_usb_probe,
- .disconnect = vp7045_usb_disconnect,
+ .disconnect = dvb_usb_device_exit,
.id_table = vp7045_usb_table,
};
--
1.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Betr: [linux-dvb] dib0700 hangs when usb receiver is unplugged while watching TV
To the best of my knowledge, you did. I cc'd Patrick and Olivier who are involved with dib ... On Sun, 10 Jul 2011 09:22:05 +0200 cedric.dew...@telfort.nl wrote: > > >-- Oorspronkelijk bericht -- > >Date: Fri, 24 Jun 2011 11:01:37 +0200 > >From: cedric.dew...@telfort.nl > >To: linux-...@linuxtv.org > >Subject: [linux-dvb] dib0700 hangs when usb receiver is unplugged while > > watching TV > >Reply-To: linux-media@vger.kernel.org > > > > > >Hi All, > > > >I have the PCTV nanostick solo. This works perfectly, but when I pull out > >the stick while i'm watching TV, the driver crashes. When I replug the stick, > >there's no reaction in dmesg. > > > >To reproduce: > >1)plugin the stick > >1a)scan channels with scan, see also > >https://wiki.archlinux.org/index.php/Digitenne#Configure_Sasc-ng > >2)use tzap, cat and mplayer to watch TV > >3)unplug the stick > >4)watch the fireworks in /var/log/everything.log (dmesg) > > Hi All, > > Did I post the above message in the correct mailing list? > > Best regards, > Cedric -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with Hauppauge Nova-TD (dib0070/dib7000PC)
On Mon, 11 Jul 2011 12:23:28 +0100 David Waring wrote: > I'm currently using 3 of these USB sticks on a PC with the videolan.org > dvblast program to multicast the UK Freeview DVB-T muxes on our local > network. I'm also using a PCTV nanostick 290e to multicast the DVB-T2 > mux too. > > I'm having a problem with the Nova-TD sticks (52009) using recent builds > from the media_build git repository (to get the 290e drivers) on Debian > squeeze using 2.6.38-bpo.2-686. The problem is that only one half of > each Nova-TD stick will tune and give data. Which half seems to be > random and changes with each reboot. Occasionally I'll get a whole stick > working or one of the sticks will not work at all. If I try to use a > non-working half of a stick it will knock out the working half until I > stop using trying to use the non-working half. So I'm seeing > interference of one logical dvb adapter from another that are both on > the same physical hardware. > > Also after a few days the sticks stop working completely and need to be > powered down before they work again, but this may be a different issue. > > I'm getting a few "dib0700: tx buffer length is larger than 4. Not > supported." in dmesg during first tune. Maybe coincidence, but I've > noticed that on the last reboot 4 tuners (out of the 6 total Nova-TD > tuners) are not working and I have 4 of the above message in dmesg, so > there could be a link. > > I've tried turning on the debugging for both the dvb_usb_dib0700 and > dvb_usb modules but there was no indication of the problem. > > Any suggestions for what I could try next to find the cause and fix this? > > David Hi, maybe the fixes Patrick posted[1] fix your issue ? http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/36393 Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: vp702x
On Thu, 4 Aug 2011 13:29:42 +0300 Dan Carpenter wrote: > On Thu, Aug 04, 2011 at 12:21:29PM +0200, Florian Mickler wrote: > > Mauro, what to do? > > Apply the fix which Tino tested, perhaps? :P (obviously). > > The bug is present in 3.0 so it should be tagged for stable as well. > > regards, > dan carpenter Two different drivers ;) I fear the fix Tino tested does not apply to vp702x.. (it's for vp7045) Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: vp702x
Hi, On Wed, 3 Aug 2011 09:39:46 +0200 (CEST) Patrick Boettcher wrote: > Hi Florian, > > > I'm not sure whether I still have exactly this box. There were two > versions and I got rid of at least one of them. > > I moved recently into a new house and right now a lot of things are hidden > in some boxes somewhere... I'll try to find some time to check it. Don't > ask me when that will be :). > ok, thanks for the heads up nonetheless. Mauro, what to do? Currently it will bug on driver probe like the vp7045 (which Tino confirmed works now again with my recent patch). Before the patchseries it would use on-stack dma buffers (don't know about the actual harm of these, at least a WARN from libdma, but rumours are that they may format your disk if the moon aligns with the sun and a special unspecified planet). Regards, Flo > regards, > -- > > Patrick Boettcher - Kernel Labs > http://www.kernellabs.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] vp702x: fix buffer handling
In my previous change to this driver, I was not aware that dvb_usb_device_init
calls the frontend_attach routine which needs a transfer
buffer. So we can not setup anything private in the probe routine beforehand but
have to allocate when needed. This means also that we cannot use a private
buffer mutex to serialize that buffer but instead need to use the
dvb_usb_device's usb_mutex.
Note: Compile tested only!
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x-fe.c | 30 -
drivers/media/dvb/dvb-usb/vp702x.c| 220 ++---
drivers/media/dvb/dvb-usb/vp702x.h|8 +-
3 files changed, 150 insertions(+), 108 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c
b/drivers/media/dvb/dvb-usb/vp702x-fe.c
index 2bb8d4c..d9eff02 100644
--- a/drivers/media/dvb/dvb-usb/vp702x-fe.c
+++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c
@@ -43,21 +43,30 @@ static int vp702x_fe_refresh_state(struct vp702x_fe_state
*st)
{
struct vp702x_device_state *dst = st->d->priv;
u8 *buf;
+ int ret;
if (time_after(jiffies, st->next_status_check)) {
- mutex_lock(&dst->buf_mutex);
- buf = dst->buf;
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
- vp702x_usb_in_op(st->d, READ_STATUS, 0, 0, buf, 10);
+ ret = vp702x_buffer_setup(dst, &buf, 10);
+ if (ret)
+ goto unlock;
+
+ vp702x_usb_in_op_unlocked(st->d, READ_STATUS, 0, 0, buf, 10);
st->lock = buf[4];
- vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0, buf, 1);
+ vp702x_usb_in_op_unlocked(st->d, READ_TUNER_REG_REQ, 0x11, 0,
+ buf, 1);
st->snr = buf[0];
- vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, buf, 1);
+ vp702x_usb_in_op_unlocked(st->d, READ_TUNER_REG_REQ, 0x15, 0,
+ buf, 1);
st->sig = buf[0];
- mutex_unlock(&dst->buf_mutex);
+unlock:
+ mutex_unlock(&st->d->usb_mutex);
st->next_status_check = jiffies +
(st->status_check_interval*HZ)/1000;
}
return 0;
@@ -200,8 +209,15 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
static int vp702x_fe_init(struct dvb_frontend *fe)
{
struct vp702x_fe_state *st = fe->demodulator_priv;
+ int ret;
+
deb_fe("%s\n",__func__);
- vp702x_usb_in_op(st->d, RESET_TUNER, 0, 0, NULL, 0);
+ ret = mutex_lock_interruptible(&st->d->usb_mutex);
+ if (ret < 0)
+ return ret;
+ vp702x_usb_in_op_unlocked(st->d, RESET_TUNER, 0, 0, NULL, 0);
+
+ mutex_unlock(&st->d->usb_mutex);
return 0;
}
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 54355f8..a34938e 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -30,7 +30,8 @@ struct vp702x_adapter_state {
u8 pid_filter_state;
};
-static int vp702x_usb_in_op_unlocked(struct dvb_usb_device *d, u8 req,
+/* usb_mutex has to be held around this */
+int vp702x_usb_in_op_unlocked(struct dvb_usb_device *d, u8 req,
u16 value, u16 index, u8 *b, int blen)
{
int ret;
@@ -55,20 +56,9 @@ static int vp702x_usb_in_op_unlocked(struct dvb_usb_device
*d, u8 req,
return ret;
}
-int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value,
- u16 index, u8 *b, int blen)
-{
- int ret;
-
- mutex_lock(&d->usb_mutex);
- ret = vp702x_usb_in_op_unlocked(d, req, value, index, b, blen);
- mutex_unlock(&d->usb_mutex);
-
- return ret;
-}
-
-int vp702x_usb_out_op_unlocked(struct dvb_usb_device *d, u8 req, u16 value,
- u16 index, u8 *b, int blen)
+/* usb_mutex has to be held around this */
+static int vp702x_usb_out_op_unlocked(struct dvb_usb_device *d, u8 req,
+ u16 value, u16 index, u8 *b, int blen)
{
int ret;
deb_xfer("out: req. %02x, val: %04x, ind: %04x, buffer:
",req,value,index);
@@ -86,33 +76,50 @@ int vp702x_usb_out_op_unlocked(struct dvb_usb_device *d, u8
req, u16 value,
return 0;
}
-int vp702x_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,
-u16 index, u8 *b, int blen)
+static int vp702x_usb_out_op(struct dvb_usb_device *d, u8 req, u16 val, u16
idx)
{
int ret;
+ ret = mutex_lock_interruptible(&d->usb_mutex);
+ if (ret < 0)
+ return ret;
- mutex_lock(&d->usb_mutex);
- ret = vp702
vp702x
Hi Mauro! Hi Patrick! I realized this morning, that I broke vp702x (if it was working before) with my last patchseries. Sorry. :( I'm gonna follow up on this mail with a patch to hopefully fix it, but if nobody can test it, I'd say to rather revert my patchseries for v3.1 . It will then still use on-stack dma buffers and will produce a WARN() in the dmesg if it does so, but hopefully nothing bad happens. Patrick, do you still have the hardware to test this? I'm semi confident that I did not make any silly mistakes. :| (it compiles) Regards, Flo p.s.: or I could wipe up a dead simple patch that just kmalloc's a new buffer everytime we need one... -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] get rid of on-stack dma buffers (part1)
On Sat, 30 Apr 2011 19:30:52 -0300 Mauro Carvalho Chehab wrote: > Hi Florian, > > Em 30-04-2011 15:54, Florian Mickler escreveu: > > Hi Mauro! > > > > I just saw that you picked up some patches of mine. What about these? > > These are actually tested... > > I'm still in process of applying the pending patches. Due to > patchwork.kernel.org > troubles (including the loss of about 270 patches from its SQL database only > recovered yesterday[1]), I have a long backlog. So, I'm gradually applying > the remaing > stuff. It will take some time though, and it will depend on patchwork mood, > but I intend > to spend some time during this weekend to minimize the backlog. > > > Cheers, > Mauro > > [1] The recover lost the email's body/SOB, so I've wrote a script to use my > email > queue to get the data, using patchwork just to mark what patches were already > processed. This increses the time I have to spend on each patch, as I need to > run > a script to match the patchwork patch with the patch ID inside my email queue. > Ah ok, no time pressure over here.. just wanted to make sure that these don't get lost. Regards and thanks, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] get rid of on-stack dma buffers (part1)
Hi Mauro! I just saw that you picked up some patches of mine. What about these? These are actually tested... Regards, Flo On Sun, 20 Mar 2011 22:50:47 +0100 Florian Mickler wrote: > Hi Mauro! > > These are the patches which got tested already and > should be good to go. [first batch of patches] > > I have another batch with updated patches (dib0700, gp8psk, vp702x) > where I did some more extensive changes to use preallocated memory. > And a small update to the vp7045 patch. > > Third batch are the patches to opera1, m920x, dw2102, friio, > a800 which I left as is, for the time beeing. > Regards, > Flo > > Florian Mickler (5): > [media] ec168: get rid of on-stack dma buffers > [media] ce6230: get rid of on-stack dma buffer > [media] au6610: get rid of on-stack dma buffer > [media] lmedm04: correct indentation > [media] lmedm04: get rid of on-stack dma buffers > > drivers/media/dvb/dvb-usb/au6610.c | 22 -- > drivers/media/dvb/dvb-usb/ce6230.c | 11 +-- > drivers/media/dvb/dvb-usb/ec168.c | 18 +++--- > drivers/media/dvb/dvb-usb/lmedm04.c | 35 > +++ > 4 files changed, 63 insertions(+), 23 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] dib0700: get rid of on-stack dma buffers
On Fri, 29 Apr 2011 18:32:34 -0300 Mauro Carvalho Chehab wrote: > As it is a trivial fix, I'll be picking it directly. Zdenek reported in the bug that it doesn't fix all instances of the warning. Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] dib0700: get rid of on-stack dma buffers
On Mon, 4 Apr 2011 09:42:04 +0200 (CEST) Patrick Boettcher wrote: > Hi Florian, > > For this one we implemented an alternative. See here: > > http://git.linuxtv.org/pb/media_tree.git?a=commit;h=16b54de2d8b46e48c5c8bdf9b350eac04e8f6b46 > > which I pushed, but obviously forgot to send the pull-request. > OK, I just looked over it. What about dib0700_rc_query_old_firmware, that would also need to be fixed. I don't have an overview over the media framework, so I wonder what arbitrates concurrent access to the buffer? Functions which are only called from the initialization and probe routines are probably properly arbitrated by the driver core. But I would expect (perhaps that is me being naive) stuff like dib0700_change_protocol to need some sort of mutex ? It seems to be called from some /sys/class/*/ file while for example legacy_dvb_usb_read_remote_control, which calls dib0700_rc_query_old_firmware, is described as being a polling function, i.e. periodically executed... or the streaming_ctrl function, that looks like it is executed at times... Thanks, Flo p.s.: can you add yourself to the MAINTAINERS file please? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] dib0700: get rid of on-stack dma buffers
On Mon, 4 Apr 2011 09:42:04 +0200 (CEST) Patrick Boettcher wrote: > For this one we implemented an alternative. See here: > > http://git.linuxtv.org/pb/media_tree.git?a=commit;h=16b54de2d8b46e48c5c8bdf9b350eac04e8f6b46 > > which I pushed, but obviously forgot to send the pull-request. > > This is done now. Thanks for the information. I see there is a CC: Florian Mickler in there, but I didn't get any email... maybe something wrong on your side? It helps a lot with closing bug reports in the bugzilla, if people add a reference to the bugreport - if there is one . I.e. a line: "This should address bug X. " Or even a link (preferred). Regards, Flo > > For the second patch I will incorperate it as soon as I find the time. no probs. > > best regards, > -- > > Patrick -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] dib0700: remove unused variable
This variable is never used.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/dib0700_core.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index ca80520..7f6f023 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -625,7 +625,6 @@ struct dib0700_rc_response {
static void dib0700_rc_urb_completion(struct urb *purb)
{
struct dvb_usb_device *d = purb->context;
- struct dib0700_state *st;
struct dib0700_rc_response *poll_reply;
u32 uninitialized_var(keycode);
u8 toggle;
@@ -640,7 +639,6 @@ static void dib0700_rc_urb_completion(struct urb *purb)
return;
}
- st = d->priv;
poll_reply = purb->transfer_buffer;
if (purb->status < 0) {
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] [media] dib0700: get rid of on-stack dma buffers
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.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
Reported-by: Zdenek Kabelac
Signed-off-by: Florian Mickler
---
[v2: use preallocated buffer; fix sizeof in one case]
[v3: use seperate kmalloc mapping for the preallocation,
dont ignore errors in probe codepaths ]
[v4: minor style nit: functions opening brace goes onto it's own line ]
[v5: use preallocated buffer whereever we have a dvb_usb_device
available even if it means acquiring i2c_mutex where we did previously
not + found some more on-stack buffers that escaped my first
review somehow...]
drivers/media/dvb/dvb-usb/dib0700.h |5 +-
drivers/media/dvb/dvb-usb/dib0700_core.c| 142 +-
drivers/media/dvb/dvb-usb/dib0700_devices.c |9 ++-
3 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700.h
b/drivers/media/dvb/dvb-usb/dib0700.h
index b2a87f2..368fbcf 100644
--- a/drivers/media/dvb/dvb-usb/dib0700.h
+++ b/drivers/media/dvb/dvb-usb/dib0700.h
@@ -46,8 +46,9 @@ struct dib0700_state {
u8 is_dib7000pc;
u8 fw_use_new_i2c_api;
u8 disable_streaming_master_mode;
-u32 fw_version;
-u32 nb_packet_buffer_size;
+ u32 fw_version;
+ u32 nb_packet_buffer_size;
+ u8 *buf; /* protected by dvb_usb_devices i2c_mutex */
};
extern int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index b79af68..ca80520 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -27,11 +27,16 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
u32 *romversion, u32 *ramversion, u32 *fwtype)
{
- u8 b[16];
- int ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+ struct dib0700_state *st = d->priv;
+ int ret;
+ u8 *b = st->buf;
+
+ mutex_lock(&d->i2c_mutex);
+
+ ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
REQUEST_GET_VERSION,
USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- b, sizeof(b), USB_CTRL_GET_TIMEOUT);
+ b, 16, USB_CTRL_GET_TIMEOUT);
if (hwversion != NULL)
*hwversion = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) |
b[3];
if (romversion != NULL)
@@ -40,6 +45,9 @@ int dib0700_get_version(struct dvb_usb_device *d, u32
*hwversion,
*ramversion = (b[8] << 24) | (b[9] << 16) | (b[10] << 8) |
b[11];
if (fwtype != NULL)
*fwtype = (b[12] << 24) | (b[13] << 16) | (b[14] << 8) |
b[15];
+
+ mutex_unlock(&d->i2c_mutex);
+
return ret;
}
@@ -101,16 +109,30 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx, u8
txlen, u8 *rx, u8 rxlen
int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
gpio_dir, u8 gpio_val)
{
- u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
((gpio_val & 0x01) << 6) };
- return dib0700_ctrl_wr(d, buf, sizeof(buf));
+ s16 ret;
+ struct dib0700_state *st = d->priv;
+ u8 *buf = st->buf;
+
+ mutex_lock(&d->i2c_mutex);
+
+ buf[0] = REQUEST_SET_GPIO;
+ buf[1] = gpio;
+ buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
+ ret = dib0700_ctrl_wr(d, buf, 3);
+
+ mutex_unlock(&d->i2c_mutex);
+
+ return ret;
}
static int dib0700_set_usb_xfer_len(struct dvb_usb_device *d, u16
nb_ts_packets)
{
struct dib0700_state *st = d->priv;
- u8 b[3];
+ u8 *b = st->buf;
int ret;
+ mutex_lock(&d->i2c_mutex);
+
if (st->fw_version >= 0x10201) {
b[0] = REQUEST_SET_USB_XFER_LEN;
b[1] = (nb_ts_packets >> 8) & 0xff;
@@ -118,12 +140,14 @@ static int dib0700_set_usb_xfer_len(struct dvb_usb_device
*d, u16 nb_ts_packets)
deb_info("set the USB xfer len to %i Ts packet\n",
nb_ts_packets);
- ret = dib0700_ctrl_wr(d, b, sizeof(b));
+ ret = dib0700_ctrl_wr(d, b, 3);
} else {
deb_info("this firmware does not allow to change the USB xfer
len\n");
ret = -EIO;
}
+ mutex_un
[media] dib0700: get rid of on-stack dma buffers
Hi, since I got no reaction[1] on the vp702x driver, I proceed with the dib0700. There are multiple drivers in drivers/media/dvb/dvb-usb/ which use usb_control_msg to perform dma to stack-allocated buffers. This is a bad idea because of cache-coherency issues and on some platforms the stack is mapped virtually and also lib/dma-debug.c warn's about it at runtime. Patches to ec168, ce6230, au6610 and lmedm04 were already tested and reviewed and submitted for inclusion [2]. Patches to a800, vp7045, friio, dw2102, m920x and opera1 are still waiting for for review and testing [3]. This patch to dib0700 is a fix for a warning seen and reported by Zdenek Kabalec in Bug #15977 [4]. Florian Mickler (2): [media] dib0700: get rid of on-stack dma buffers [media] dib0700: remove unused variable Regards, Flo References: [1]: http://www.spinics.net/lists/linux-media/msg30448.html [2]: http://comments.gmane.org/gmane.linux.kernel/1115404 [3]: http://groups.google.com/group/fa.linux.kernel/browse_frm/thread/e169edc121b91181/f1498cd026a59fe2 [4]: https://bugzilla.kernel.org/show_bug.cgi?id=15977 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] get rid of on-stack dma buffers
2011/3/22 Roedel, Joerg : > On Mon, Mar 21, 2011 at 05:03:15PM -0400, Florian Mickler wrote: >> I guess (not verified), that the dma api takes sufficient precautions >> to abort the dma transfer if a timeout happens. So freeing _should_ >> not be an issue. (At least, I would expect big fat warnings everywhere >> if that were the case) > > Freeing is very well an issue. All you can expect from the DMA-API is to > give you a valid DMA handle for your device. But it can not prevent that > a device uses this handle after you returned it. You need to make sure > yourself that any pending DMA is canceled before calling kfree(). sorry, I meant usb_control_msg above when I said 'dma api'... as that is the function these drivers use to do the dma. Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] get rid of on-stack dma buffers
2011/3/22 James Bottomley : > On Mon, 2011-03-21 at 22:03 +0100, Florian Mickler wrote: >> On Mon, 21 Mar 2011 15:26:43 -0400 >> Andy Walls wrote: >> >> > Florian Mickler wrote: >> >> To be blunt, I'm not shure I fully understand the requirements myself. >> But as far as I grasped it, the main problem is that we need memory >> which the processor can see as soon as the device has scribbled upon >> it. (think caches and the like) >> >> Somewhere down the line, the buffer to usb_control_msg get's to be >> a parameter to dma_map_single which is described as part of >> the DMA API in Documentation/DMA-API.txt >> >> The main point I filter out from that is that the memory has to begin >> exactly at a cache line boundary... > > The API will round up so that the correct region covers the API. > However, if you have other structures packed into the space (as very > often happens on stack), you get cache line interference in the CPU if > they get accessed: The act of accessing an adjacent object pulls in > cache above your object and destroys DMA coherence. This is the > principle reason why DMA to stack is a bad idea. Thanks, this was the missing piece of information to make sense of why it's bad for stack memory to be part of this. > >> I guess (not verified), that the dma api takes sufficient precautions >> to abort the dma transfer if a timeout happens. So freeing _should_ >> not be an issue. (At least, I would expect big fat warnings everywhere >> if that were the case) I did mean s/dma api/usb_control_msg/ in the above paragraph. As that is the ''dma api'' these drivers are using... sorry for the confusion there... > > No, it doesn't take any precautions like this. the DMA API is just > mapping (possibly via an IOMMU). If the transfer times out, that's done > in the DMA engine of the card, and must be cleaned up by the driver and > unmapped. ok. > The general rule though is never DMA to stack. On some processors, the > way stack is allocated can actually make this not work. > > James thanks, Flo p.s.: hope this message get's through to the list... I am on the road at the moment, so I'm not shure that there won't be any html in it again :( -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] get rid of on-stack dma buffers
On Mon, 21 Mar 2011 15:26:43 -0400 Andy Walls wrote: > Florian Mickler wrote: > > >Hi all! > > > >These patches get rid of on-stack dma buffers for some of the dvb-usb > >drivers. > >I do not own the hardware, so these are only compile tested. I would > >appreciate testing and review. > >They were previously sent to the list, but some error on my side > >prevented (some of?) them from beeing delivered to all parties (the > >lists). > > > >These changes are motivated by > >https://bugzilla.kernel.org/show_bug.cgi?id=15977 . > > > >The patches which got tested already were submitted to Mauro (and > >lkml/linux-media) yesterday seperately. Those fix this same issue for > >ec168, > >ce6230, au6610 and lmedm04. > > > >A fix for vp702x has been submitted seperately for review on the list. > >I have > >similiar fixes like the vp702x-fix for dib0700 (overlooked some > >on-stack > >buffers in there in my original submission as well) and gp8psk, but I > >am > >holding them back 'till I got time to recheck those and getting some > >feedback > >on vp702x. > > > >Please review and test. > > > >Regards, > >Flo > > > >Florian Mickler (6): > > [media] a800: get rid of on-stack dma buffers > > [media v2] vp7045: get rid of on-stack dma buffers > > [media] friio: get rid of on-stack dma buffers > > [media] dw2102: get rid of on-stack dma buffer > > [media] m920x: get rid of on-stack dma buffers > > [media] opera1: get rid of on-stack dma buffer > > > > drivers/media/dvb/dvb-usb/a800.c | 17 ++--- > > drivers/media/dvb/dvb-usb/dw2102.c | 10 ++- > > drivers/media/dvb/dvb-usb/friio.c | 23 ++--- > > drivers/media/dvb/dvb-usb/m920x.c | 33 > > drivers/media/dvb/dvb-usb/opera1.c | 31 +++ > >drivers/media/dvb/dvb-usb/vp7045.c | 47 > >++-- > > 6 files changed, 116 insertions(+), 45 deletions(-) > > > >-- > >1.7.4.1 > > > >-- > >To unsubscribe from this list: send the line "unsubscribe linux-media" > >in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html > > Florian, > > For all of these, what happens when the USB call times out and you kfree() > the buffer? Can the USB DMA actually complete after this kfree(), possibly > corrupting space that has been reallocated off the heap, since the kfree()? > > This is the scenario for which I assume allocating off the stack is bad. > > Do these changes simply make corruption less noticable since heap gets > corrupted vs stack? > > Regards, > Andy To be blunt, I'm not shure I fully understand the requirements myself. But as far as I grasped it, the main problem is that we need memory which the processor can see as soon as the device has scribbled upon it. (think caches and the like) Somewhere down the line, the buffer to usb_control_msg get's to be a parameter to dma_map_single which is described as part of the DMA API in Documentation/DMA-API.txt The main point I filter out from that is that the memory has to begin exactly at a cache line boundary... I guess (not verified), that the dma api takes sufficient precautions to abort the dma transfer if a timeout happens. So freeing _should_ not be an issue. (At least, I would expect big fat warnings everywhere if that were the case) I cc'd some people that hopefully will correct me if I'm wrong... regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6 v2] [media] vp7045: get rid of on-stack dma buffers
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
---
[v2: pulled buffer access under the mutex]
drivers/media/dvb/dvb-usb/vp7045.c | 47 ++--
1 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp7045.c
b/drivers/media/dvb/dvb-usb/vp7045.c
index ab0ab3c..3db89e3 100644
--- a/drivers/media/dvb/dvb-usb/vp7045.c
+++ b/drivers/media/dvb/dvb-usb/vp7045.c
@@ -28,9 +28,9 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8 *out, int outlen, u8
*in, int inlen, int msec)
{
int ret = 0;
- u8 inbuf[12] = { 0 }, outbuf[20] = { 0 };
+ u8 *buf = d->priv;
- outbuf[0] = cmd;
+ buf[0] = cmd;
if (outlen > 19)
outlen = 19;
@@ -38,19 +38,21 @@ int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8
*out, int outlen, u8 *in,
if (inlen > 11)
inlen = 11;
+ ret = mutex_lock_interruptible(&d->usb_mutex);
+ if (ret)
+ return ret;
+
if (out != NULL && outlen > 0)
- memcpy(&outbuf[1], out, outlen);
+ memcpy(&buf[1], out, outlen);
deb_xfer("out buffer: ");
- debug_dump(outbuf,outlen+1,deb_xfer);
+ debug_dump(buf, outlen+1, deb_xfer);
- if ((ret = mutex_lock_interruptible(&d->usb_mutex)))
- return ret;
if (usb_control_msg(d->udev,
usb_sndctrlpipe(d->udev,0),
TH_COMMAND_OUT, USB_TYPE_VENDOR | USB_DIR_OUT, 0, 0,
- outbuf, 20, 2000) != 20) {
+ buf, 20, 2000) != 20) {
err("USB control message 'out' went wrong.");
ret = -EIO;
goto unlock;
@@ -61,17 +63,17 @@ int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8
*out, int outlen, u8 *in,
if (usb_control_msg(d->udev,
usb_rcvctrlpipe(d->udev,0),
TH_COMMAND_IN, USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- inbuf, 12, 2000) != 12) {
+ buf, 12, 2000) != 12) {
err("USB control message 'in' went wrong.");
ret = -EIO;
goto unlock;
}
deb_xfer("in buffer: ");
- debug_dump(inbuf,12,deb_xfer);
+ debug_dump(buf, 12, deb_xfer);
if (in != NULL && inlen > 0)
- memcpy(in,&inbuf[1],inlen);
+ memcpy(in, &buf[1], inlen);
unlock:
mutex_unlock(&d->usb_mutex);
@@ -222,8 +224,26 @@ static struct dvb_usb_device_properties vp7045_properties;
static int vp7045_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
- return dvb_usb_device_init(intf, &vp7045_properties,
- THIS_MODULE, NULL, adapter_nr);
+ struct dvb_usb_device *d;
+ int ret = dvb_usb_device_init(intf, &vp7045_properties,
+ THIS_MODULE, &d, adapter_nr);
+ if (ret)
+ return ret;
+
+ d->priv = kmalloc(20, GFP_KERNEL);
+ if (!d->priv) {
+ dvb_usb_device_exit(intf);
+ return -ENOMEM;
+ }
+
+ return ret;
+}
+
+static void vp7045_usb_disconnect(struct usb_interface *intf)
+{
+ struct dvb_usb_device *d = usb_get_intfdata(intf);
+ kfree(d->priv);
+ dvb_usb_device_exit(intf);
}
static struct usb_device_id vp7045_usb_table [] = {
@@ -238,6 +258,7 @@ MODULE_DEVICE_TABLE(usb, vp7045_usb_table);
static struct dvb_usb_device_properties vp7045_properties = {
.usb_ctrl = CYPRESS_FX2,
.firmware = "dvb-usb-vp7045-01.fw",
+ .size_of_priv = sizeof(u8 *),
.num_adapters = 1,
.adapter = {
@@ -284,7 +305,7 @@ static struct dvb_usb_device_properties vp7045_properties =
{
static struct usb_driver vp7045_usb_driver = {
.name = "dvb_usb_vp7045",
.probe = vp7045_usb_probe,
- .disconnect = dvb_usb_device_exit,
+ .disconnect = vp7045_usb_disconnect,
.id_table = vp7045_usb_table,
};
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] [media] m920x: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/m920x.c | 33 ++---
1 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/m920x.c
b/drivers/media/dvb/dvb-usb/m920x.c
index da9dc91..f66eaa3 100644
--- a/drivers/media/dvb/dvb-usb/m920x.c
+++ b/drivers/media/dvb/dvb-usb/m920x.c
@@ -134,13 +134,17 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
{
struct m920x_state *m = d->priv;
int i, ret = 0;
- u8 rc_state[2];
+ u8 *rc_state;
+
+ rc_state = kmalloc(2, GFP_KERNEL);
+ if (!rc_state)
+ return -ENOMEM;
if ((ret = m920x_read(d->udev, M9206_CORE, 0x0, M9206_RC_STATE,
rc_state, 1)) != 0)
- goto unlock;
+ goto out;
if ((ret = m920x_read(d->udev, M9206_CORE, 0x0, M9206_RC_KEY, rc_state
+ 1, 1)) != 0)
- goto unlock;
+ goto out;
for (i = 0; i < d->props.rc.legacy.rc_map_size; i++)
if (rc5_data(&d->props.rc.legacy.rc_map_table[i]) ==
rc_state[1]) {
@@ -149,7 +153,7 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
switch(rc_state[0]) {
case 0x80:
*state = REMOTE_NO_KEY_PRESSED;
- goto unlock;
+ goto out;
case 0x88: /* framing error or "invalid code" */
case 0x99:
@@ -157,7 +161,7 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
case 0xd8:
*state = REMOTE_NO_KEY_PRESSED;
m->rep_count = 0;
- goto unlock;
+ goto out;
case 0x93:
case 0x92:
@@ -165,7 +169,7 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
case 0x82:
m->rep_count = 0;
*state = REMOTE_KEY_PRESSED;
- goto unlock;
+ goto out;
case 0x91:
case 0x81: /* pinnacle PCTV310e */
@@ -174,12 +178,12 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
*state = REMOTE_KEY_REPEAT;
else
*state = REMOTE_NO_KEY_PRESSED;
- goto unlock;
+ goto out;
default:
deb("Unexpected rc state %02x\n", rc_state[0]);
*state = REMOTE_NO_KEY_PRESSED;
- goto unlock;
+ goto out;
}
}
@@ -188,8 +192,8 @@ static int m920x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
*state = REMOTE_NO_KEY_PRESSED;
- unlock:
-
+ out:
+ kfree(rc_state);
return ret;
}
@@ -339,13 +343,19 @@ static int m920x_pid_filter(struct dvb_usb_adapter *adap,
int index, u16 pid, in
static int m920x_firmware_download(struct usb_device *udev, const struct
firmware *fw)
{
u16 value, index, size;
- u8 read[4], *buff;
+ u8 *read, *buff;
int i, pass, ret = 0;
buff = kmalloc(65536, GFP_KERNEL);
if (buff == NULL)
return -ENOMEM;
+ read = kmalloc(4, GFP_KERNEL);
+ if (!read) {
+ kfree(buff);
+ return -ENOMEM;
+ }
+
if ((ret = m920x_read(udev, M9206_FILTER, 0x0, 0x8000, read, 4)) != 0)
goto done;
deb("%x %x %x %x\n", read[0], read[1], read[2], read[3]);
@@ -396,6 +406,7 @@ static int m920x_firmware_download(struct usb_device *udev,
const struct firmwar
deb("firmware uploaded!\n");
done:
+ kfree(read);
kfree(buff);
return ret;
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] [media] opera1: get rid of on-stack dma buffer
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
---
drivers/media/dvb/dvb-usb/opera1.c | 31 ---
1 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/opera1.c
b/drivers/media/dvb/dvb-usb/opera1.c
index 1f1b7d6..2ca6e87 100644
--- a/drivers/media/dvb/dvb-usb/opera1.c
+++ b/drivers/media/dvb/dvb-usb/opera1.c
@@ -53,27 +53,36 @@ static int opera1_xilinx_rw(struct usb_device *dev, u8
request, u16 value,
u8 * data, u16 len, int flags)
{
int ret;
- u8 r;
- u8 u8buf[len];
-
+ u8 tmp;
+ u8 *buf;
unsigned int pipe = (flags == OPERA_READ_MSG) ?
usb_rcvctrlpipe(dev,0) : usb_sndctrlpipe(dev, 0);
u8 request_type = (flags == OPERA_READ_MSG) ? USB_DIR_IN : USB_DIR_OUT;
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
if (flags == OPERA_WRITE_MSG)
- memcpy(u8buf, data, len);
- ret =
- usb_control_msg(dev, pipe, request, request_type |
USB_TYPE_VENDOR,
- value, 0x0, u8buf, len, 2000);
+ memcpy(buf, data, len);
+ ret = usb_control_msg(dev, pipe, request,
+ request_type | USB_TYPE_VENDOR, value, 0x0,
+ buf, len, 2000);
if (request == OPERA_TUNER_REQ) {
+ tmp = buf[0];
if (usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
- OPERA_TUNER_REQ, USB_DIR_IN | USB_TYPE_VENDOR,
- 0x01, 0x0, &r, 1, 2000)<1 || r!=0x08)
- return 0;
+ OPERA_TUNER_REQ, USB_DIR_IN | USB_TYPE_VENDOR,
+ 0x01, 0x0, buf, 1, 2000) < 1 || buf[0] != 0x08) {
+ ret = 0;
+ goto out;
+ }
+ buf[0] = tmp;
}
if (flags == OPERA_READ_MSG)
- memcpy(data, u8buf, len);
+ memcpy(data, buf, len);
+out:
+ kfree(buf);
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] [media] friio: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/friio.c | 23 +++
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/friio.c
b/drivers/media/dvb/dvb-usb/friio.c
index 14a65b4..76159ae 100644
--- a/drivers/media/dvb/dvb-usb/friio.c
+++ b/drivers/media/dvb/dvb-usb/friio.c
@@ -142,17 +142,20 @@ static u32 gl861_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_I2C;
}
-
static int friio_ext_ctl(struct dvb_usb_adapter *adap,
u32 sat_color, int lnb_on)
{
int i;
int ret;
struct i2c_msg msg;
- u8 buf[2];
+ u8 *buf;
u32 mask;
u8 lnb = (lnb_on) ? FRIIO_CTL_LNB : 0;
+ buf = kmalloc(2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
msg.addr = 0x00;
msg.flags = 0;
msg.len = 2;
@@ -189,6 +192,7 @@ static int friio_ext_ctl(struct dvb_usb_adapter *adap,
buf[1] |= FRIIO_CTL_CLK;
ret += gl861_i2c_xfer(&adap->dev->i2c_adap, &msg, 1);
+ kfree(buf);
return (ret == 70);
}
@@ -219,11 +223,20 @@ static int friio_initialize(struct dvb_usb_device *d)
int ret;
int i;
int retry = 0;
- u8 rbuf[2];
- u8 wbuf[3];
+ u8 *rbuf, *wbuf;
deb_info("%s called.\n", __func__);
+ wbuf = kmalloc(3, GFP_KERNEL);
+ if (!wbuf)
+ return -ENOMEM;
+
+ rbuf = kmalloc(2, GFP_KERNEL);
+ if (!rbuf) {
+ kfree(wbuf);
+ return -ENOMEM;
+ }
+
/* use gl861_i2c_msg instead of gl861_i2c_xfer(), */
/* because the i2c device is not set up yet. */
wbuf[0] = 0x11;
@@ -358,6 +371,8 @@ restart:
return 0;
error:
+ kfree(wbuf);
+ kfree(rbuf);
deb_info("%s:ret == %d\n", __func__, ret);
return -EIO;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] [media] dw2102: get rid of on-stack dma buffer
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
---
drivers/media/dvb/dvb-usb/dw2102.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dw2102.c
b/drivers/media/dvb/dvb-usb/dw2102.c
index 2c307ba..4c8ff39 100644
--- a/drivers/media/dvb/dvb-usb/dw2102.c
+++ b/drivers/media/dvb/dvb-usb/dw2102.c
@@ -101,12 +101,16 @@ static int dw210x_op_rw(struct usb_device *dev, u8
request, u16 value,
u16 index, u8 * data, u16 len, int flags)
{
int ret;
- u8 u8buf[len];
-
+ u8 *u8buf;
unsigned int pipe = (flags == DW210X_READ_MSG) ?
usb_rcvctrlpipe(dev, 0) : usb_sndctrlpipe(dev,
0);
u8 request_type = (flags == DW210X_READ_MSG) ? USB_DIR_IN : USB_DIR_OUT;
+ u8buf = kmalloc(len, GFP_KERNEL);
+ if (!u8buf)
+ return -ENOMEM;
+
+
if (flags == DW210X_WRITE_MSG)
memcpy(u8buf, data, len);
ret = usb_control_msg(dev, pipe, request, request_type |
USB_TYPE_VENDOR,
@@ -114,6 +118,8 @@ static int dw210x_op_rw(struct usb_device *dev, u8 request,
u16 value,
if (flags == DW210X_READ_MSG)
memcpy(data, u8buf, len);
+
+ kfree(u8buf);
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] [media] a800: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/a800.c | 17 +
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/a800.c b/drivers/media/dvb/dvb-usb/a800.c
index 53b93a4..fe2366b 100644
--- a/drivers/media/dvb/dvb-usb/a800.c
+++ b/drivers/media/dvb/dvb-usb/a800.c
@@ -78,17 +78,26 @@ static struct rc_map_table rc_map_a800_table[] = {
static int a800_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
{
- u8 key[5];
+ int ret;
+ u8 *key = kmalloc(5, GFP_KERNEL);
+ if (!key)
+ return -ENOMEM;
+
if (usb_control_msg(d->udev,usb_rcvctrlpipe(d->udev,0),
0x04, USB_TYPE_VENDOR | USB_DIR_IN, 0, 0, key,
5,
- 2000) != 5)
- return -ENODEV;
+ 2000) != 5) {
+ ret = -ENODEV;
+ goto out;
+ }
/* call the universal NEC remote processor, to find out the key's state
and event */
dvb_usb_nec_rc_key_to_event(d,key,event,state);
if (key[0] != 0)
deb_rc("key: %x %x %x %x
%x\n",key[0],key[1],key[2],key[3],key[4]);
- return 0;
+ ret = 0;
+out:
+ kfree(key);
+ return ret;
}
/* USB Driver stuff */
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] get rid of on-stack dma buffers
Hi all! These patches get rid of on-stack dma buffers for some of the dvb-usb drivers. I do not own the hardware, so these are only compile tested. I would appreciate testing and review. They were previously sent to the list, but some error on my side prevented (some of?) them from beeing delivered to all parties (the lists). These changes are motivated by https://bugzilla.kernel.org/show_bug.cgi?id=15977 . The patches which got tested already were submitted to Mauro (and lkml/linux-media) yesterday seperately. Those fix this same issue for ec168, ce6230, au6610 and lmedm04. A fix for vp702x has been submitted seperately for review on the list. I have similiar fixes like the vp702x-fix for dib0700 (overlooked some on-stack buffers in there in my original submission as well) and gp8psk, but I am holding them back 'till I got time to recheck those and getting some feedback on vp702x. Please review and test. Regards, Flo Florian Mickler (6): [media] a800: get rid of on-stack dma buffers [media v2] vp7045: get rid of on-stack dma buffers [media] friio: get rid of on-stack dma buffers [media] dw2102: get rid of on-stack dma buffer [media] m920x: get rid of on-stack dma buffers [media] opera1: get rid of on-stack dma buffer drivers/media/dvb/dvb-usb/a800.c | 17 ++--- drivers/media/dvb/dvb-usb/dw2102.c | 10 ++- drivers/media/dvb/dvb-usb/friio.c | 23 ++--- drivers/media/dvb/dvb-usb/m920x.c | 33 drivers/media/dvb/dvb-usb/opera1.c | 31 +++ drivers/media/dvb/dvb-usb/vp7045.c | 47 ++-- 6 files changed, 116 insertions(+), 45 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] [media] vp702x: remove unused variable
struct vp702x_device_state.power_state is nowhere referenced.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.h |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.h
b/drivers/media/dvb/dvb-usb/vp702x.h
index 86960c6..20b9005 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.h
+++ b/drivers/media/dvb/dvb-usb/vp702x.h
@@ -99,7 +99,6 @@ extern int dvb_usb_vp702x_debug;
/* IN i: 0, v: 0, no extra buffer */
struct vp702x_device_state {
- u8 power_state;
struct mutex buf_mutex;
int buf_len;
u8 *buf;
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] [media] vp702x: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/vp702x-fe.c | 89 +++--
drivers/media/dvb/dvb-usb/vp702x.c| 78 ++--
2 files changed, 124 insertions(+), 43 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c
b/drivers/media/dvb/dvb-usb/vp702x-fe.c
index ccc7e44..7468a38 100644
--- a/drivers/media/dvb/dvb-usb/vp702x-fe.c
+++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c
@@ -41,15 +41,26 @@ struct vp702x_fe_state {
static int vp702x_fe_refresh_state(struct vp702x_fe_state *st)
{
- u8 buf[10];
- if (time_after(jiffies,st->next_status_check)) {
- vp702x_usb_in_op(st->d,READ_STATUS,0,0,buf,10);
-
+ u8 *buf;
+
+ if (time_after(jiffies, st->next_status_check)) {
+ buf = kmalloc(10, GFP_KERNEL);
+ if (!buf) {
+ deb_fe("%s: buffer alloc failed\n", __func__);
+ return -ENOMEM;
+ }
+ vp702x_usb_in_op(st->d, READ_STATUS, 0, 0, buf, 10);
st->lock = buf[4];
- vp702x_usb_in_op(st->d,READ_TUNER_REG_REQ,0x11,0,&st->snr,1);
- vp702x_usb_in_op(st->d,READ_TUNER_REG_REQ,0x15,0,&st->sig,1);
+
+ vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0, buf, 1);
+ st->snr = buf[0];
+
+ vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, buf, 1);
+ st->sig = buf[0];
+
st->next_status_check = jiffies +
(st->status_check_interval*HZ)/1000;
+ kfree(buf);
}
return 0;
}
@@ -134,7 +145,11 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
/*CalFrequency*/
/* u16 frequencyRef[16] = { 2, 4, 8, 16, 32, 64, 128, 256, 24, 5, 10, 20,
40, 80, 160, 320 }; */
u64 sr;
- u8 cmd[8] = { 0 },ibuf[10];
+ u8 *cmd;
+
+ cmd = kzalloc(10, GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
cmd[0] = (freq >> 8) & 0x7f;
cmd[1] = freq & 0xff;
@@ -170,13 +185,14 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
st->status_check_interval = 250;
st->next_status_check = jiffies;
- vp702x_usb_inout_op(st->d,cmd,8,ibuf,10,100);
+ vp702x_usb_inout_op(st->d, cmd, 8, cmd, 10, 100);
- if (ibuf[2] == 0 && ibuf[3] == 0)
+ if (cmd[2] == 0 && cmd[3] == 0)
deb_fe("tuning failed.\n");
else
deb_fe("tuning succeeded.\n");
+ kfree(cmd);
return 0;
}
@@ -204,28 +220,36 @@ static int vp702x_fe_get_frontend(struct dvb_frontend* fe,
static int vp702x_fe_send_diseqc_msg (struct dvb_frontend* fe,
struct dvb_diseqc_master_cmd *m)
{
+ int ret;
+ u8 *cmd;
struct vp702x_fe_state *st = fe->demodulator_priv;
- u8 cmd[8],ibuf[10];
- memset(cmd,0,8);
+
+ cmd = kzalloc(10, GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
deb_fe("%s\n",__func__);
- if (m->msg_len > 4)
- return -EINVAL;
+ if (m->msg_len > 4) {
+ ret = -EINVAL;
+ goto out;
+ }
cmd[1] = SET_DISEQC_CMD;
cmd[2] = m->msg_len;
memcpy(&cmd[3], m->msg, m->msg_len);
- cmd[7] = vp702x_chksum(cmd,0,7);
+ cmd[7] = vp702x_chksum(cmd, 0, 7);
- vp702x_usb_inout_op(st->d,cmd,8,ibuf,10,100);
+ vp702x_usb_inout_op(st->d, cmd, 8, cmd, 10, 100);
- if (ibuf[2] == 0 && ibuf[3] == 0)
+ if (cmd[2] == 0 && cmd[3] == 0)
deb_fe("diseqc cmd failed.\n");
else
deb_fe("diseqc cmd succeeded.\n");
-
- return 0;
+ ret = 0;
+out:
+ kfree(cmd);
+ return ret;
}
static int vp702x_fe_send_diseqc_burst (struct dvb_frontend* fe,
fe_sec_mini_cmd_t burst)
@@ -237,9 +261,14 @@ static int vp702x_fe_send_diseqc_burst (struct
dvb_frontend* fe, fe_sec_mini_cmd
static int vp702x_fe_set_tone(struct dvb_frontend* fe, fe_sec_tone_mode_t tone)
{
struct vp702x_fe_state *st = fe->demodulator_priv;
- u8 ibuf[10];
+ u8 *buf;
+
deb_fe("%s\n",__func__);
+ buf = kmalloc(10, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
st->tone_mode = tone;
if (tone == SEC_TONE_ON)
@@ -247,1
[PATCH 8/9] [media] vp702x: use preallocated buffer in vp702x_usb_inout_cmd
If we need a bigger buffer, we reallocte a new buffer and free the old
one.
Note: This change is tested to compile only as I don't have the
hardware.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c | 23 +++
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 6dd50bc..54355f8 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -116,13 +116,28 @@ int vp702x_usb_inout_op(struct dvb_usb_device *d, u8 *o,
int olen, u8 *i, int il
static int vp702x_usb_inout_cmd(struct dvb_usb_device *d, u8 cmd, u8 *o,
int olen, u8 *i, int ilen, int msec)
{
+ struct vp702x_device_state *st = d->priv;
int ret = 0;
u8 *buf;
int buflen = max(olen + 2, ilen + 1);
- buf = kmalloc(buflen, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ ret = mutex_lock_interruptible(&st->buf_mutex);
+ if (ret < 0)
+ return ret;
+
+ if (buflen > st->buf_len) {
+ buf = kmalloc(buflen, GFP_KERNEL);
+ if (!buf) {
+ mutex_unlock(&st->buf_mutex);
+ return -ENOMEM;
+ }
+ info("successfully reallocated a bigger buffer");
+ kfree(st->buf);
+ st->buf = buf;
+ st->buf_len = buflen;
+ } else {
+ buf = st->buf;
+ }
buf[0] = 0x00;
buf[1] = cmd;
@@ -132,8 +147,8 @@ static int vp702x_usb_inout_cmd(struct dvb_usb_device *d,
u8 cmd, u8 *o,
if (ret == 0)
memcpy(i, &buf[1], ilen);
+ mutex_unlock(&st->buf_mutex);
- kfree(buf);
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] [media] vp702x: use preallocated buffer
Note: This change is tested to compile only as I don't have the
hardware.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c | 59 +--
1 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index c82cb6b..6dd50bc 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -140,38 +140,44 @@ static int vp702x_usb_inout_cmd(struct dvb_usb_device *d,
u8 cmd, u8 *o,
static int vp702x_set_pld_mode(struct dvb_usb_adapter *adap, u8 bypass)
{
int ret;
- u8 *buf = kzalloc(16, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ struct vp702x_device_state *st = adap->dev->priv;
+ u8 *buf;
+
+ mutex_lock(&st->buf_mutex);
+
+ buf = st->buf;
+ memset(buf, 0, 16);
ret = vp702x_usb_in_op(adap->dev, 0xe0, (bypass << 8) | 0x0e,
0, buf, 16);
- kfree(buf);
+ mutex_unlock(&st->buf_mutex);
return ret;
}
static int vp702x_set_pld_state(struct dvb_usb_adapter *adap, u8 state)
{
int ret;
- u8 *buf = kzalloc(16, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ struct vp702x_device_state *st = adap->dev->priv;
+ u8 *buf;
+
+ mutex_lock(&st->buf_mutex);
+ buf = st->buf;
+ memset(buf, 0, 16);
ret = vp702x_usb_in_op(adap->dev, 0xe0, (state << 8) | 0x0f,
0, buf, 16);
- kfree(buf);
+
+ mutex_unlock(&st->buf_mutex);
+
return ret;
}
static int vp702x_set_pid(struct dvb_usb_adapter *adap, u16 pid, u8 id, int
onoff)
{
struct vp702x_adapter_state *st = adap->priv;
+ struct vp702x_device_state *dst = adap->dev->priv;
u8 *buf;
- buf = kzalloc(16, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
if (onoff)
st->pid_filter_state |= (1 << id);
else {
@@ -182,10 +188,16 @@ static int vp702x_set_pid(struct dvb_usb_adapter *adap,
u16 pid, u8 id, int onof
id = 0x10 + id*2;
vp702x_set_pld_state(adap, st->pid_filter_state);
+
+ mutex_lock(&dst->buf_mutex);
+
+ buf = dst->buf;
+ memset(buf, 0, 16);
vp702x_usb_in_op(adap->dev, 0xe0, (((pid >> 8) & 0xff) << 8) | (id), 0,
buf, 16);
vp702x_usb_in_op(adap->dev, 0xe0, (((pid ) & 0xff) << 8) | (id+1),
0, buf, 16);
- kfree(buf);
+ mutex_unlock(&dst->buf_mutex);
+
return 0;
}
@@ -193,13 +205,10 @@ static int vp702x_set_pid(struct dvb_usb_adapter *adap,
u16 pid, u8 id, int onof
static int vp702x_init_pid_filter(struct dvb_usb_adapter *adap)
{
struct vp702x_adapter_state *st = adap->priv;
+ struct vp702x_device_state *dst = adap->dev->priv;
int i;
u8 *b;
- b = kzalloc(10, GFP_KERNEL);
- if (!b)
- return -ENOMEM;
-
st->pid_filter_count = 8;
st->pid_filter_can_bypass = 1;
st->pid_filter_state = 0x00;
@@ -209,12 +218,15 @@ static int vp702x_init_pid_filter(struct dvb_usb_adapter
*adap)
for (i = 0; i < st->pid_filter_count; i++)
vp702x_set_pid(adap, 0x, i, 1);
+ mutex_lock(&dst->buf_mutex);
+ b = dst->buf;
+ memset(b, 0, 10);
vp702x_usb_in_op(adap->dev, 0xb5, 3, 0, b, 10);
vp702x_usb_in_op(adap->dev, 0xb5, 0, 0, b, 10);
vp702x_usb_in_op(adap->dev, 0xb5, 1, 0, b, 10);
+ mutex_unlock(&dst->buf_mutex);
+ /*vp702x_set_pld_mode(d, 0); // filter */
- //vp702x_set_pld_mode(d, 0); // filter
- kfree(b);
return 0;
}
@@ -266,16 +278,15 @@ static int vp702x_rc_query(struct dvb_usb_device *d, u32
*event, int *state)
static int vp702x_read_mac_addr(struct dvb_usb_device *d,u8 mac[6])
{
u8 i, *buf;
+ struct vp702x_device_state *st = d->priv;
- buf = kmalloc(6, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
+ mutex_lock(&st->buf_mutex);
+ buf = st->buf;
for (i = 6; i < 12; i++)
vp702x_usb_in_op(d, READ_EEPROM_REQ, i, 1, &buf[i - 6], 1);
memcpy(mac, buf, 6);
- kfree(buf);
+ mutex_unlock(&st->buf_mutex);
return 0;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] [media] vp702x: use preallocated buffer in the frontend
Note: This change is tested to compile only as I don't have the
hardware.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x-fe.c | 69 +
1 files changed, 35 insertions(+), 34 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c
b/drivers/media/dvb/dvb-usb/vp702x-fe.c
index 7468a38..2bb8d4c 100644
--- a/drivers/media/dvb/dvb-usb/vp702x-fe.c
+++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c
@@ -41,14 +41,13 @@ struct vp702x_fe_state {
static int vp702x_fe_refresh_state(struct vp702x_fe_state *st)
{
+ struct vp702x_device_state *dst = st->d->priv;
u8 *buf;
if (time_after(jiffies, st->next_status_check)) {
- buf = kmalloc(10, GFP_KERNEL);
- if (!buf) {
- deb_fe("%s: buffer alloc failed\n", __func__);
- return -ENOMEM;
- }
+ mutex_lock(&dst->buf_mutex);
+ buf = dst->buf;
+
vp702x_usb_in_op(st->d, READ_STATUS, 0, 0, buf, 10);
st->lock = buf[4];
@@ -58,9 +57,8 @@ static int vp702x_fe_refresh_state(struct vp702x_fe_state *st)
vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, buf, 1);
st->sig = buf[0];
-
+ mutex_unlock(&dst->buf_mutex);
st->next_status_check = jiffies +
(st->status_check_interval*HZ)/1000;
- kfree(buf);
}
return 0;
}
@@ -141,15 +139,17 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
struct dvb_frontend_parameters *fep)
{
struct vp702x_fe_state *st = fe->demodulator_priv;
+ struct vp702x_device_state *dst = st->d->priv;
u32 freq = fep->frequency/1000;
/*CalFrequency*/
/* u16 frequencyRef[16] = { 2, 4, 8, 16, 32, 64, 128, 256, 24, 5, 10, 20,
40, 80, 160, 320 }; */
u64 sr;
u8 *cmd;
- cmd = kzalloc(10, GFP_KERNEL);
- if (!cmd)
- return -ENOMEM;
+ mutex_lock(&dst->buf_mutex);
+
+ cmd = dst->buf;
+ memset(cmd, 0, 10);
cmd[0] = (freq >> 8) & 0x7f;
cmd[1] = freq & 0xff;
@@ -192,7 +192,8 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe,
else
deb_fe("tuning succeeded.\n");
- kfree(cmd);
+ mutex_unlock(&dst->buf_mutex);
+
return 0;
}
@@ -220,21 +221,18 @@ static int vp702x_fe_get_frontend(struct dvb_frontend* fe,
static int vp702x_fe_send_diseqc_msg (struct dvb_frontend* fe,
struct dvb_diseqc_master_cmd *m)
{
- int ret;
u8 *cmd;
struct vp702x_fe_state *st = fe->demodulator_priv;
-
- cmd = kzalloc(10, GFP_KERNEL);
- if (!cmd)
- return -ENOMEM;
+ struct vp702x_device_state *dst = st->d->priv;
deb_fe("%s\n",__func__);
- if (m->msg_len > 4) {
- ret = -EINVAL;
- goto out;
- }
+ if (m->msg_len > 4)
+ return -EINVAL;
+
+ mutex_lock(&dst->buf_mutex);
+ cmd = dst->buf;
cmd[1] = SET_DISEQC_CMD;
cmd[2] = m->msg_len;
memcpy(&cmd[3], m->msg, m->msg_len);
@@ -246,10 +244,10 @@ static int vp702x_fe_send_diseqc_msg (struct
dvb_frontend* fe,
deb_fe("diseqc cmd failed.\n");
else
deb_fe("diseqc cmd succeeded.\n");
- ret = 0;
-out:
- kfree(cmd);
- return ret;
+
+ mutex_unlock(&dst->buf_mutex);
+
+ return 0;
}
static int vp702x_fe_send_diseqc_burst (struct dvb_frontend* fe,
fe_sec_mini_cmd_t burst)
@@ -261,14 +259,11 @@ static int vp702x_fe_send_diseqc_burst (struct
dvb_frontend* fe, fe_sec_mini_cmd
static int vp702x_fe_set_tone(struct dvb_frontend* fe, fe_sec_tone_mode_t tone)
{
struct vp702x_fe_state *st = fe->demodulator_priv;
+ struct vp702x_device_state *dst = st->d->priv;
u8 *buf;
deb_fe("%s\n",__func__);
- buf = kmalloc(10, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
st->tone_mode = tone;
if (tone == SEC_TONE_ON)
@@ -277,6 +272,10 @@ static int vp702x_fe_set_tone(struct dvb_frontend* fe,
fe_sec_tone_mode_t tone)
st->lnb_buf[2] = 0x00;
st->lnb_buf[7] = vp702x_chksum(st->lnb_buf, 0, 7);
+
+ mutex_lock(&dst->buf_mutex);
+
+ buf = dst->buf;
memcpy(buf, st->lnb_buf, 8);
vp702x_usb_inout_op(st->d, buf, 8, buf, 10, 100);
@@ -285,7 +284,8 @@ static int vp702x_fe_set_tone(struct dvb_frontend* fe,
fe_sec_tone_mode_t tone)
else
deb_fe("set_tone cmd succeeded.\n");
- kfree(buf)
[PATCH 6/9] [media] vp702x: fix locking of usb operations
Otherwise it is not obvious that vp702x_usb_in_op or vp702x_usb_out_op
will not interfere with any vp702x_usb_inout_op.
Note: This change is tested to compile only, as I don't have the
hardware.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c | 37 +--
1 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 35fe778..c82cb6b 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -30,8 +30,8 @@ struct vp702x_adapter_state {
u8 pid_filter_state;
};
-/* check for mutex FIXME */
-int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index,
u8 *b, int blen)
+static int vp702x_usb_in_op_unlocked(struct dvb_usb_device *d, u8 req,
+u16 value, u16 index, u8 *b, int blen)
{
int ret;
@@ -55,8 +55,20 @@ int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16
value, u16 index, u8
return ret;
}
-static int vp702x_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,
-u16 index, u8 *b, int blen)
+int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value,
+ u16 index, u8 *b, int blen)
+{
+ int ret;
+
+ mutex_lock(&d->usb_mutex);
+ ret = vp702x_usb_in_op_unlocked(d, req, value, index, b, blen);
+ mutex_unlock(&d->usb_mutex);
+
+ return ret;
+}
+
+int vp702x_usb_out_op_unlocked(struct dvb_usb_device *d, u8 req, u16 value,
+ u16 index, u8 *b, int blen)
{
int ret;
deb_xfer("out: req. %02x, val: %04x, ind: %04x, buffer:
",req,value,index);
@@ -74,6 +86,18 @@ static int vp702x_usb_out_op(struct dvb_usb_device *d, u8
req, u16 value,
return 0;
}
+int vp702x_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,
+u16 index, u8 *b, int blen)
+{
+ int ret;
+
+ mutex_lock(&d->usb_mutex);
+ ret = vp702x_usb_out_op_unlocked(d, req, value, index, b, blen);
+ mutex_unlock(&d->usb_mutex);
+
+ return ret;
+}
+
int vp702x_usb_inout_op(struct dvb_usb_device *d, u8 *o, int olen, u8 *i, int
ilen, int msec)
{
int ret;
@@ -81,12 +105,11 @@ int vp702x_usb_inout_op(struct dvb_usb_device *d, u8 *o,
int olen, u8 *i, int il
if ((ret = mutex_lock_interruptible(&d->usb_mutex)))
return ret;
- ret = vp702x_usb_out_op(d,REQUEST_OUT,0,0,o,olen);
+ ret = vp702x_usb_out_op_unlocked(d, REQUEST_OUT, 0, 0, o, olen);
msleep(msec);
- ret = vp702x_usb_in_op(d,REQUEST_IN,0,0,i,ilen);
+ ret = vp702x_usb_in_op_unlocked(d, REQUEST_IN, 0, 0, i, ilen);
mutex_unlock(&d->usb_mutex);
-
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] [media] vp702x: rename struct vp702x_state -> vp702x_adapter_state
We need a state struct for the dvb_usb_device.
In order to reduce confusion we rename the vp702x_state struct.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 4c9939f..25536f9 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -23,7 +23,7 @@ MODULE_PARM_DESC(debug, "set debugging level
(1=info,xfer=2,rc=4 (or-able))." DV
DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
-struct vp702x_state {
+struct vp702x_adapter_state {
int pid_filter_count;
int pid_filter_can_bypass;
u8 pid_filter_state;
@@ -126,7 +126,7 @@ static int vp702x_set_pld_state(struct dvb_usb_adapter
*adap, u8 state)
static int vp702x_set_pid(struct dvb_usb_adapter *adap, u16 pid, u8 id, int
onoff)
{
- struct vp702x_state *st = adap->priv;
+ struct vp702x_adapter_state *st = adap->priv;
u8 buf[16] = { 0 };
if (onoff)
@@ -147,7 +147,7 @@ static int vp702x_set_pid(struct dvb_usb_adapter *adap, u16
pid, u8 id, int onof
static int vp702x_init_pid_filter(struct dvb_usb_adapter *adap)
{
- struct vp702x_state *st = adap->priv;
+ struct vp702x_adapter_state *st = adap->priv;
int i;
u8 b[10] = { 0 };
@@ -279,7 +279,7 @@ static struct dvb_usb_device_properties vp702x_properties =
{
}
}
},
- .size_of_priv = sizeof(struct vp702x_state),
+ .size_of_priv = sizeof(struct vp702x_adapter_state),
}
},
.read_mac_address = vp702x_read_mac_addr,
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] [media] vp702x: cleanup: whitespace and indentation
Some whitespace, one linebreak and one unneded variable
initialization...
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c | 23 ---
1 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 7890e75..4c9939f 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -36,14 +36,14 @@ struct vp702x_device_state {
/* check for mutex FIXME */
int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index,
u8 *b, int blen)
{
- int ret = -1;
+ int ret;
- ret = usb_control_msg(d->udev,
- usb_rcvctrlpipe(d->udev,0),
- req,
- USB_TYPE_VENDOR | USB_DIR_IN,
- value,index,b,blen,
- 2000);
+ ret = usb_control_msg(d->udev,
+ usb_rcvctrlpipe(d->udev, 0),
+ req,
+ USB_TYPE_VENDOR | USB_DIR_IN,
+ value, index, b, blen,
+ 2000);
if (ret < 0) {
warn("usb in operation failed. (%d)", ret);
@@ -221,7 +221,8 @@ static int vp702x_frontend_attach(struct dvb_usb_adapter
*adap)
vp702x_usb_out_op(adap->dev, SET_TUNER_POWER_REQ, 0, 7, NULL, 0);
- if (vp702x_usb_inout_cmd(adap->dev, GET_SYSTEM_STRING, NULL, 0, buf,
10, 10))
+ if (vp702x_usb_inout_cmd(adap->dev, GET_SYSTEM_STRING, NULL, 0,
+ buf, 10, 10))
return -EIO;
buf[9] = '\0';
@@ -307,9 +308,9 @@ static struct dvb_usb_device_properties vp702x_properties =
{
/* usb specific object needed to register this driver with the usb subsystem */
static struct usb_driver vp702x_usb_driver = {
.name = "dvb_usb_vp702x",
- .probe = vp702x_usb_probe,
- .disconnect = dvb_usb_device_exit,
- .id_table = vp702x_usb_table,
+ .probe = vp702x_usb_probe,
+ .disconnect = dvb_usb_device_exit,
+ .id_table = vp702x_usb_table,
};
/* module stuff */
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] [media] vp702x: preallocate memory on device probe
This sets up a buffer and a mutex protecting that buffer in
the struct vp702x_device_state.
The definition of struct vp702x_device_state is moved into the header
in order to use the buffer also in the frontend.
Signed-off-by: Florian Mickler
---
drivers/media/dvb/dvb-usb/vp702x.c | 41 +--
drivers/media/dvb/dvb-usb/vp702x.h |8 +++
2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp702x.c
b/drivers/media/dvb/dvb-usb/vp702x.c
index 25536f9..569c93f 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -15,6 +15,7 @@
* see Documentation/dvb/README.dvb-usb for more information
*/
#include "vp702x.h"
+#include
/* debug */
int dvb_usb_vp702x_debug;
@@ -29,10 +30,6 @@ struct vp702x_adapter_state {
u8 pid_filter_state;
};
-struct vp702x_device_state {
- u8 power_state;
-};
-
/* check for mutex FIXME */
int vp702x_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index,
u8 *b, int blen)
{
@@ -241,8 +238,38 @@ static struct dvb_usb_device_properties vp702x_properties;
static int vp702x_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
- return dvb_usb_device_init(intf, &vp702x_properties,
- THIS_MODULE, NULL, adapter_nr);
+ struct dvb_usb_device *d;
+ struct vp702x_device_state *st;
+ int ret;
+
+ ret = dvb_usb_device_init(intf, &vp702x_properties,
+ THIS_MODULE, &d, adapter_nr);
+ if (ret)
+ goto out;
+
+ st = d->priv;
+ st->buf_len = 16;
+ st->buf = kmalloc(st->buf_len, GFP_KERNEL);
+ if (!st->buf) {
+ ret = -ENOMEM;
+ dvb_usb_device_exit(intf);
+ goto out;
+ }
+ mutex_init(&st->buf_mutex);
+
+out:
+ return ret;
+
+}
+
+static void vp702x_usb_disconnect(struct usb_interface *intf)
+{
+ struct dvb_usb_device *d = usb_get_intfdata(intf);
+ struct vp702x_device_state *st = d->priv;
+ mutex_lock(&st->buf_mutex);
+ kfree(st->buf);
+ mutex_unlock(&st->buf_mutex);
+ dvb_usb_device_exit(intf);
}
static struct usb_device_id vp702x_usb_table [] = {
@@ -309,7 +336,7 @@ static struct dvb_usb_device_properties vp702x_properties =
{
static struct usb_driver vp702x_usb_driver = {
.name = "dvb_usb_vp702x",
.probe = vp702x_usb_probe,
- .disconnect = dvb_usb_device_exit,
+ .disconnect = vp702x_usb_disconnect,
.id_table = vp702x_usb_table,
};
diff --git a/drivers/media/dvb/dvb-usb/vp702x.h
b/drivers/media/dvb/dvb-usb/vp702x.h
index c2f97f9..86960c6 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.h
+++ b/drivers/media/dvb/dvb-usb/vp702x.h
@@ -98,6 +98,14 @@ extern int dvb_usb_vp702x_debug;
#define RESET_TUNER0xBE
/* IN i: 0, v: 0, no extra buffer */
+struct vp702x_device_state {
+ u8 power_state;
+ struct mutex buf_mutex;
+ int buf_len;
+ u8 *buf;
+};
+
+
extern struct dvb_frontend * vp702x_fe_attach(struct dvb_usb_device *d);
extern int vp702x_usb_inout_op(struct dvb_usb_device *d, u8 *o, int olen, u8
*i, int ilen, int msec);
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] vp702x: get rid of on-stack dma buffers (part2 1/2)
Hi! This is a patchset modifying the vp702x to get rid of on-stack dma buffers and additionally preallocating the used buffers on device probe. I can not test these patches, as I don't have the hardware. They compile though... I made it a bit more finegrained for easier review. If someone could test these, that would be appreciated. I have a few more patches to dib0700 and gp8psk to also use a preallocated buffer, but I'm not shure the added complexity is worth it. So I'm waiting on feedback to the vp702x to proceed. I have another batch of patches to opera1, m920x, dw2102, friio, a800 which I did not modify since my first patch submission. Regards, Flo Florian Mickler (9): [media] vp702x: cleanup: whitespace and indentation [media] vp702x: rename struct vp702x_state -> vp702x_adapter_state [media] vp702x: preallocate memory on device probe [media] vp702x: remove unused variable [media] vp702x: get rid of on-stack dma buffers [media] vp702x: fix locking of usb operations [media] vp702x: use preallocated buffer [media] vp702x: use preallocated buffer in vp702x_usb_inout_cmd [media] vp702x: use preallocated buffer in the frontend drivers/media/dvb/dvb-usb/vp702x-fe.c | 80 + drivers/media/dvb/dvb-usb/vp702x.c| 213 ++--- drivers/media/dvb/dvb-usb/vp702x.h|7 + 3 files changed, 233 insertions(+), 67 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] [media] au6610: get rid of on-stack dma buffer
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
Signed-off-by: Florian Mickler
Acked-by: Antti Palosaari
Reviewed-by: Antti Palosaari
Tested-by: Antti Palosaari
---
drivers/media/dvb/dvb-usb/au6610.c | 22 --
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/au6610.c
b/drivers/media/dvb/dvb-usb/au6610.c
index eb34cc3..2351077 100644
--- a/drivers/media/dvb/dvb-usb/au6610.c
+++ b/drivers/media/dvb/dvb-usb/au6610.c
@@ -33,8 +33,16 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8
operation, u8 addr,
{
int ret;
u16 index;
- u8 usb_buf[6]; /* enough for all known requests,
- read returns 5 and write 6 bytes */
+ u8 *usb_buf;
+
+ /*
+* allocate enough for all known requests,
+* read returns 5 and write 6 bytes
+*/
+ usb_buf = kmalloc(6, GFP_KERNEL);
+ if (!usb_buf)
+ return -ENOMEM;
+
switch (wlen) {
case 1:
index = wbuf[0] << 8;
@@ -45,14 +53,15 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8
operation, u8 addr,
break;
default:
warn("wlen = %x, aborting.", wlen);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0), operation,
USB_TYPE_VENDOR|USB_DIR_IN, addr << 1, index,
- usb_buf, sizeof(usb_buf), AU6610_USB_TIMEOUT);
+ usb_buf, 6, AU6610_USB_TIMEOUT);
if (ret < 0)
- return ret;
+ goto error;
switch (operation) {
case AU6610_REQ_I2C_READ:
@@ -60,7 +69,8 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8
operation, u8 addr,
/* requested value is always 5th byte in buffer */
rbuf[0] = usb_buf[4];
}
-
+error:
+ kfree(usb_buf);
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] [media] ec168: get rid of on-stack dma buffers
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
Signed-off-by: Florian Mickler
Acked-by: Antti Palosaari
Reviewed-by: Antti Palosaari
Tested-by: Antti Palosaari
---
drivers/media/dvb/dvb-usb/ec168.c | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/ec168.c
b/drivers/media/dvb/dvb-usb/ec168.c
index 52f5d4f..1ba3e5d 100644
--- a/drivers/media/dvb/dvb-usb/ec168.c
+++ b/drivers/media/dvb/dvb-usb/ec168.c
@@ -36,7 +36,9 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
int ret;
unsigned int pipe;
u8 request, requesttype;
- u8 buf[req->size];
+ u8 *buf;
+
+
switch (req->cmd) {
case DOWNLOAD_FIRMWARE:
@@ -72,6 +74,12 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
goto error;
}
+ buf = kmalloc(req->size, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
/* write */
memcpy(buf, req->data, req->size);
@@ -84,13 +92,13 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
msleep(1); /* avoid I2C errors */
ret = usb_control_msg(udev, pipe, request, requesttype, req->value,
- req->index, buf, sizeof(buf), EC168_USB_TIMEOUT);
+ req->index, buf, req->size, EC168_USB_TIMEOUT);
ec168_debug_dump(request, requesttype, req->value, req->index, buf,
req->size, deb_xfer);
if (ret < 0)
- goto error;
+ goto err_dealloc;
else
ret = 0;
@@ -98,7 +106,11 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
memcpy(req->data, buf, req->size);
+ kfree(buf);
return ret;
+
+err_dealloc:
+ kfree(buf);
error:
deb_info("%s: failed:%d\n", __func__, ret);
return ret;
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] get rid of on-stack dma buffers (part1)
Hi Mauro! These are the patches which got tested already and should be good to go. [first batch of patches] I have another batch with updated patches (dib0700, gp8psk, vp702x) where I did some more extensive changes to use preallocated memory. And a small update to the vp7045 patch. Third batch are the patches to opera1, m920x, dw2102, friio, a800 which I left as is, for the time beeing. Regards, Flo Florian Mickler (5): [media] ec168: get rid of on-stack dma buffers [media] ce6230: get rid of on-stack dma buffer [media] au6610: get rid of on-stack dma buffer [media] lmedm04: correct indentation [media] lmedm04: get rid of on-stack dma buffers drivers/media/dvb/dvb-usb/au6610.c | 22 -- drivers/media/dvb/dvb-usb/ce6230.c | 11 +-- drivers/media/dvb/dvb-usb/ec168.c | 18 +++--- drivers/media/dvb/dvb-usb/lmedm04.c | 35 +++ 4 files changed, 63 insertions(+), 23 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5 v2] [media] lmedm04: get rid of on-stack dma buffers
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
Tested-By: Malcolm Priestley
Signed-off-by: Florian Mickler
---
[v2: fix use after free as noted by Malcom]
drivers/media/dvb/dvb-usb/lmedm04.c | 19 +++
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c
b/drivers/media/dvb/dvb-usb/lmedm04.c
index 0a3e88f..8a79354 100644
--- a/drivers/media/dvb/dvb-usb/lmedm04.c
+++ b/drivers/media/dvb/dvb-usb/lmedm04.c
@@ -314,13 +314,19 @@ 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]);
- return (ret < 0) ? -ENODEV : data[2];
+ ret = (ret < 0) ? -ENODEV : data[2];
+ kfree(data);
+ return ret;
}
static int lme2510_msg(struct dvb_usb_device *d,
@@ -603,7 +609,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 +617,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)");
+ return -ENOMEM;
+ }
info("FRM Starting Firmware Download");
@@ -654,7 +665,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;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] [media] ce6230: get rid of on-stack dma buffer
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
Signed-off-by: Florian Mickler
Acked-by: Antti Palosaari
Reviewed-by: Antti Palosaari
Tested-by: Antti Palosaari
---
drivers/media/dvb/dvb-usb/ce6230.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/ce6230.c
b/drivers/media/dvb/dvb-usb/ce6230.c
index 3df2045..6d1a304 100644
--- a/drivers/media/dvb/dvb-usb/ce6230.c
+++ b/drivers/media/dvb/dvb-usb/ce6230.c
@@ -39,7 +39,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
u8 requesttype;
u16 value;
u16 index;
- u8 buf[req->data_len];
+ u8 *buf;
request = req->cmd;
value = req->value;
@@ -62,6 +62,12 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
goto error;
}
+ buf = kmalloc(req->data_len, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
/* write */
memcpy(buf, req->data, req->data_len);
@@ -74,7 +80,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
msleep(1); /* avoid I2C errors */
ret = usb_control_msg(udev, pipe, request, requesttype, value, index,
- buf, sizeof(buf), CE6230_USB_TIMEOUT);
+ buf, req->data_len, CE6230_USB_TIMEOUT);
ce6230_debug_dump(request, requesttype, value, index, buf,
req->data_len, deb_xfer);
@@ -88,6 +94,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
memcpy(req->data, buf, req->data_len);
+ kfree(buf);
error:
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] [media] lmedm04: correct indentation
This should not change anything except whitespace. Signed-off-by: Florian Mickler --- drivers/media/dvb/dvb-usb/lmedm04.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c index 9eea418..0a3e88f 100644 --- a/drivers/media/dvb/dvb-usb/lmedm04.c +++ b/drivers/media/dvb/dvb-usb/lmedm04.c @@ -626,15 +626,15 @@ static int lme2510_download_firmware(struct usb_device *dev, data[0] = i | 0x80; dlen = (u8)(end - j)-1; } - data[1] = dlen; - memcpy(&data[2], fw_data, dlen+1); - wlen = (u8) dlen + 4; - data[wlen-1] = check_sum(fw_data, dlen+1); - deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3], + data[1] = dlen; + memcpy(&data[2], fw_data, dlen+1); + wlen = (u8) dlen + 4; + data[wlen-1] = check_sum(fw_data, dlen+1); + deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3], data[dlen+2], data[dlen+3]); - ret |= lme2510_bulk_write(dev, data, wlen, 1); - ret |= lme2510_bulk_read(dev, data, len_in , 1); - ret |= (data[0] == 0x88) ? 0 : -1; + ret |= lme2510_bulk_write(dev, data, wlen, 1); + ret |= lme2510_bulk_read(dev, data, len_in , 1); + ret |= (data[0] == 0x88) ? 0 : -1; } } -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/16] [media] ec168: get rid of on stack dma buffers
On Fri, 18 Mar 2011 18:36:53 +0200 Antti Palosaari wrote: > On 03/15/2011 10:43 AM, 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 > > Acked-by: Antti Palosaari > Reviewed-by: Antti Palosaari > Tested-by: Antti Palosaari > > t. Antti > Thanks for the feedback! Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] [media] ce6230: get rid of on-stack dma buffer
On Fri, 18 Mar 2011 18:36:11 +0200 Antti Palosaari wrote: > On 03/15/2011 10:43 AM, 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 > > Acked-by: Antti Palosaari > Reviewed-by: Antti Palosaari > Tested-by: Antti Palosaari > > > t. Antti > Thanks for your feedback! Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/16] [media] au6610: get rid of on-stack dma buffer
On Fri, 18 Mar 2011 18:34:58 +0200 Antti Palosaari wrote: > On 03/15/2011 10:43 AM, 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 > > > This patch did not found from patchwork! Probably skipped due to broken > Cc at my contact. Please resend. > > Anyhow, I tested and reviewed it. > > Acked-by: Antti Palosaari > Reviewed-by: Antti Palosaari > Tested-by: Antti Palosaari > > [1] https://patchwork.kernel.org/project/linux-media/list/ > > Antti > Yes, there was some broken adressing on my side. Sorry. Thanks for review && test! I will resend (hopefully this weekend) the series when I reviewed some of the other patches if it is feasible/better to use prealocated memory as suggested by Mauro. How often does au6610_usb_msg get called in normal operation? Should it use preallocated memory? Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/16] [media] lmedm04: get rid of on-stack dma buffers
On Tue, 15 Mar 2011 20:54:43 +
Malcolm Priestley wrote:
> 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
> > ---
> > 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;
Yes. Thanks. I updated the patch locally.
>
> > @@ -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,
I choose to ignore this warning to keep the string on one line (for
grep and co).
> > + 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
>
Thanks. I added a
Tested-by: Malcolm Priestley ,
if that is ok?
Do you know how often/when is the .identify_state callback called during
normal operation? I.e. is this memory allocation/deallocation
in lme2510_return_status happening often and should use a preallocated
buffer?
Regards,
Flo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/16] [media] dib0700: get rid of on-stack dma buffers
On Tue, 15 Mar 2011 09:02:00 -0300 Mauro Carvalho Chehab wrote: > > You're allocating a buffer for URB control messages. IMO, this is a good > idea, as > this way, allocating/freeing on each urb call is avoided. However, on most > places, > you're not using it. The better would be to just use this buffer on all > the above places. > > You should just take care of protecting such buffer with a mutex, to avoid > concurrency. > Such kind of protection is generally ok, as dvb applications generally > serialize > the calls anyway. > I didn't do so already, because I had/have no overview over the big picture operation of the dvb framework and thus feared to introduce deadlocks or massive serializations where concurrency is needed. But if you suggest it, I guess it should be benign. I'm wondering about the purpose of the usb_mutex and the i2c_mutex ... Should I introduce new driver specific mutexes to protect the buffer or is it possible to reuse one of the 2? vp702x_usb_inout_op takes the usb_mutex, but vp702x_usb_out_op and vp702x_usb_in_op get called without that mutex hold. That makes me wonder what that mutex purpose is in that driver? Other drivers like the az6027 introduce a driver specific mutex and also use the usb_mutex. That make conceptually (to my inexperienced mind) more sense. (each layer does it's own locking) but the idea of operation is not yet clear in my mind... Can you perhaps shed some light on what the purpose of those locks is and if it is sufficient to use the usb_mutex to serialize all usb_control_msg calls (which would probably protect the buffer sufficiently but may be too much if the dvb-usb framework uses that mutex for different purposes). In the meantime I will respin this series using preallocated buffers and will hopefully work out stuff that is unclear to me yet ... Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/16] [media] lmedm04: get rid of on-stack dma buffers
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
---
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];
}
@@ -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)");
+ 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;
}
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/16] [media] vp7045: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/vp7045.c | 41 ++-
1 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/vp7045.c
b/drivers/media/dvb/dvb-usb/vp7045.c
index ab0ab3c..17478ec 100644
--- a/drivers/media/dvb/dvb-usb/vp7045.c
+++ b/drivers/media/dvb/dvb-usb/vp7045.c
@@ -28,9 +28,9 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8 *out, int outlen, u8
*in, int inlen, int msec)
{
int ret = 0;
- u8 inbuf[12] = { 0 }, outbuf[20] = { 0 };
+ u8 *buf = d->priv;
- outbuf[0] = cmd;
+ buf[0] = cmd;
if (outlen > 19)
outlen = 19;
@@ -39,10 +39,10 @@ int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8
*out, int outlen, u8 *in,
inlen = 11;
if (out != NULL && outlen > 0)
- memcpy(&outbuf[1], out, outlen);
+ memcpy(&buf[1], out, outlen);
deb_xfer("out buffer: ");
- debug_dump(outbuf,outlen+1,deb_xfer);
+ debug_dump(buf, outlen+1, deb_xfer);
if ((ret = mutex_lock_interruptible(&d->usb_mutex)))
return ret;
@@ -50,7 +50,7 @@ int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8 *out,
int outlen, u8 *in,
if (usb_control_msg(d->udev,
usb_sndctrlpipe(d->udev,0),
TH_COMMAND_OUT, USB_TYPE_VENDOR | USB_DIR_OUT, 0, 0,
- outbuf, 20, 2000) != 20) {
+ buf, 20, 2000) != 20) {
err("USB control message 'out' went wrong.");
ret = -EIO;
goto unlock;
@@ -61,17 +61,17 @@ int vp7045_usb_op(struct dvb_usb_device *d, u8 cmd, u8
*out, int outlen, u8 *in,
if (usb_control_msg(d->udev,
usb_rcvctrlpipe(d->udev,0),
TH_COMMAND_IN, USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- inbuf, 12, 2000) != 12) {
+ buf, 12, 2000) != 12) {
err("USB control message 'in' went wrong.");
ret = -EIO;
goto unlock;
}
deb_xfer("in buffer: ");
- debug_dump(inbuf,12,deb_xfer);
+ debug_dump(buf, 12, deb_xfer);
if (in != NULL && inlen > 0)
- memcpy(in,&inbuf[1],inlen);
+ memcpy(in, &buf[1], inlen);
unlock:
mutex_unlock(&d->usb_mutex);
@@ -222,8 +222,26 @@ static struct dvb_usb_device_properties vp7045_properties;
static int vp7045_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
- return dvb_usb_device_init(intf, &vp7045_properties,
- THIS_MODULE, NULL, adapter_nr);
+ struct dvb_usb_device *d;
+ int ret = dvb_usb_device_init(intf, &vp7045_properties,
+ THIS_MODULE, &d, adapter_nr);
+ if (ret)
+ return ret;
+
+ d->priv = kmalloc(20, GFP_KERNEL);
+ if (!d->priv) {
+ dvb_usb_device_exit(intf);
+ return -ENOMEM;
+ }
+
+ return ret;
+}
+
+static void vp7045_usb_disconnect(struct usb_interface *intf)
+{
+ struct dvb_usb_device *d = usb_get_intfdata(intf);
+ kfree(d->priv);
+ dvb_usb_device_exit(intf);
}
static struct usb_device_id vp7045_usb_table [] = {
@@ -238,6 +256,7 @@ MODULE_DEVICE_TABLE(usb, vp7045_usb_table);
static struct dvb_usb_device_properties vp7045_properties = {
.usb_ctrl = CYPRESS_FX2,
.firmware = "dvb-usb-vp7045-01.fw",
+ .size_of_priv = sizeof(u8 *),
.num_adapters = 1,
.adapter = {
@@ -284,7 +303,7 @@ static struct dvb_usb_device_properties vp7045_properties =
{
static struct usb_driver vp7045_usb_driver = {
.name = "dvb_usb_vp7045",
.probe = vp7045_usb_probe,
- .disconnect = dvb_usb_device_exit,
+ .disconnect = vp7045_usb_disconnect,
.id_table = vp7045_usb_table,
};
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/16] [media] dib0700: get rid of on-stack dma buffers
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.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
Reported-by: Zdenek Kabelac
Signed-off-by: Florian Mickler
[v2: use preallocated buffer; fix sizeof in one case]
[v3: use seperate kmalloc mapping for the preallocation,
dont ignore errors in probe codepaths ]
[v4: minor style nit: functions opening brace goes onto it's own line ]
---
drivers/media/dvb/dvb-usb/dib0700.h |5 +-
drivers/media/dvb/dvb-usb/dib0700_core.c | 120 --
2 files changed, 99 insertions(+), 26 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700.h
b/drivers/media/dvb/dvb-usb/dib0700.h
index 3537d65..99a1485 100644
--- a/drivers/media/dvb/dvb-usb/dib0700.h
+++ b/drivers/media/dvb/dvb-usb/dib0700.h
@@ -45,8 +45,9 @@ struct dib0700_state {
u8 is_dib7000pc;
u8 fw_use_new_i2c_api;
u8 disable_streaming_master_mode;
-u32 fw_version;
-u32 nb_packet_buffer_size;
+ u32 fw_version;
+ u32 nb_packet_buffer_size;
+ u8 *buf;
};
extern int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 98ffb40..1c19b73 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -27,11 +27,17 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
u32 *romversion, u32 *ramversion, u32 *fwtype)
{
- u8 b[16];
- int ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+ int ret;
+ u8 *b;
+
+ b = kmalloc(16, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
REQUEST_GET_VERSION,
USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- b, sizeof(b), USB_CTRL_GET_TIMEOUT);
+ b, 16, USB_CTRL_GET_TIMEOUT);
if (hwversion != NULL)
*hwversion = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) |
b[3];
if (romversion != NULL)
@@ -40,6 +46,8 @@ int dib0700_get_version(struct dvb_usb_device *d, u32
*hwversion,
*ramversion = (b[8] << 24) | (b[9] << 16) | (b[10] << 8) |
b[11];
if (fwtype != NULL)
*fwtype = (b[12] << 24) | (b[13] << 16) | (b[14] << 8) |
b[15];
+
+ kfree(b);
return ret;
}
@@ -101,8 +109,19 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx, u8
txlen, u8 *rx, u8 rxlen
int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
gpio_dir, u8 gpio_val)
{
- u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
((gpio_val & 0x01) << 6) };
- return dib0700_ctrl_wr(d, buf, sizeof(buf));
+ s16 ret;
+ u8 *buf = kmalloc(3, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = REQUEST_SET_GPIO;
+ buf[1] = gpio;
+ buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
+
+ ret = dib0700_ctrl_wr(d, buf, 3);
+
+ kfree(buf);
+ return ret;
}
static int dib0700_set_usb_xfer_len(struct dvb_usb_device *d, u16
nb_ts_packets)
@@ -137,11 +156,12 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
properly support i2c read calls not preceded by a write */
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
uint8_t bus_mode = 1; /* 0=eeprom bus, 1=frontend bus */
uint8_t gen_mode = 0; /* 0=master i2c, 1=gpio i2c */
uint8_t en_start = 0;
uint8_t en_stop = 0;
- uint8_t buf[255]; /* TBV: malloc ? */
+ uint8_t *buf = st->buf;
int result, i;
/* Ensure nobody else hits the i2c bus while we're sending our
@@ -221,6 +241,7 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
}
}
mutex_unlock(&d->i2c_mutex);
+
return i;
}
@@ -231,8 +252,9 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,
struct i2c_msg *msg, int num)
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
int i,len;
- u8 buf[255];
+ u8 *buf = st->buf;
if (mutex_lock_inter
[PATCH 03/16] [media] a800: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/a800.c | 17 +
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/a800.c b/drivers/media/dvb/dvb-usb/a800.c
index 53b93a4..fe2366b 100644
--- a/drivers/media/dvb/dvb-usb/a800.c
+++ b/drivers/media/dvb/dvb-usb/a800.c
@@ -78,17 +78,26 @@ static struct rc_map_table rc_map_a800_table[] = {
static int a800_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
{
- u8 key[5];
+ int ret;
+ u8 *key = kmalloc(5, GFP_KERNEL);
+ if (!key)
+ return -ENOMEM;
+
if (usb_control_msg(d->udev,usb_rcvctrlpipe(d->udev,0),
0x04, USB_TYPE_VENDOR | USB_DIR_IN, 0, 0, key,
5,
- 2000) != 5)
- return -ENODEV;
+ 2000) != 5) {
+ ret = -ENODEV;
+ goto out;
+ }
/* call the universal NEC remote processor, to find out the key's state
and event */
dvb_usb_nec_rc_key_to_event(d,key,event,state);
if (key[0] != 0)
deb_rc("key: %x %x %x %x
%x\n",key[0],key[1],key[2],key[3],key[4]);
- return 0;
+ ret = 0;
+out:
+ kfree(key);
+ return ret;
}
/* USB Driver stuff */
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/16] [media] lmedm04: correct indentation
This should not change anything except whitespace. Signed-off-by: Florian Mickler --- drivers/media/dvb/dvb-usb/lmedm04.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c index 9eea418..0a3e88f 100644 --- a/drivers/media/dvb/dvb-usb/lmedm04.c +++ b/drivers/media/dvb/dvb-usb/lmedm04.c @@ -626,15 +626,15 @@ static int lme2510_download_firmware(struct usb_device *dev, data[0] = i | 0x80; dlen = (u8)(end - j)-1; } - data[1] = dlen; - memcpy(&data[2], fw_data, dlen+1); - wlen = (u8) dlen + 4; - data[wlen-1] = check_sum(fw_data, dlen+1); - deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3], + data[1] = dlen; + memcpy(&data[2], fw_data, dlen+1); + wlen = (u8) dlen + 4; + data[wlen-1] = check_sum(fw_data, dlen+1); + deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3], data[dlen+2], data[dlen+3]); - ret |= lme2510_bulk_write(dev, data, wlen, 1); - ret |= lme2510_bulk_read(dev, data, len_in , 1); - ret |= (data[0] == 0x88) ? 0 : -1; + ret |= lme2510_bulk_write(dev, data, wlen, 1); + ret |= lme2510_bulk_read(dev, data, len_in , 1); + ret |= (data[0] == 0x88) ? 0 : -1; } } -- 1.7.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/16] [media] dib0700: remove unused variable
This variable is never used.
Signed-off-by: Florian Mickler
--
drivers/media/dvb/dvb-usb/dib0700_core.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
---
drivers/media/dvb/dvb-usb/dib0700_core.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 1c19b73..c705ea4 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -576,7 +576,6 @@ struct dib0700_rc_response {
static void dib0700_rc_urb_completion(struct urb *purb)
{
struct dvb_usb_device *d = purb->context;
- struct dib0700_state *st;
struct dib0700_rc_response *poll_reply;
u32 uninitialized_var(keycode);
u8 toggle;
@@ -591,7 +590,6 @@ static void dib0700_rc_urb_completion(struct urb *purb)
return;
}
- st = d->priv;
poll_reply = purb->transfer_buffer;
if (purb->status < 0) {
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/16] [media] ec168: get rid of on stack dma buffers
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
---
drivers/media/dvb/dvb-usb/ec168.c | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/ec168.c
b/drivers/media/dvb/dvb-usb/ec168.c
index 52f5d4f..1ba3e5d 100644
--- a/drivers/media/dvb/dvb-usb/ec168.c
+++ b/drivers/media/dvb/dvb-usb/ec168.c
@@ -36,7 +36,9 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
int ret;
unsigned int pipe;
u8 request, requesttype;
- u8 buf[req->size];
+ u8 *buf;
+
+
switch (req->cmd) {
case DOWNLOAD_FIRMWARE:
@@ -72,6 +74,12 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
goto error;
}
+ buf = kmalloc(req->size, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
/* write */
memcpy(buf, req->data, req->size);
@@ -84,13 +92,13 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
msleep(1); /* avoid I2C errors */
ret = usb_control_msg(udev, pipe, request, requesttype, req->value,
- req->index, buf, sizeof(buf), EC168_USB_TIMEOUT);
+ req->index, buf, req->size, EC168_USB_TIMEOUT);
ec168_debug_dump(request, requesttype, req->value, req->index, buf,
req->size, deb_xfer);
if (ret < 0)
- goto error;
+ goto err_dealloc;
else
ret = 0;
@@ -98,7 +106,11 @@ static int ec168_rw_udev(struct usb_device *udev, struct
ec168_req *req)
if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
memcpy(req->data, buf, req->size);
+ kfree(buf);
return ret;
+
+err_dealloc:
+ kfree(buf);
error:
deb_info("%s: failed:%d\n", __func__, ret);
return ret;
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/16] [media] ce6230: get rid of on-stack dma buffer
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
---
drivers/media/dvb/dvb-usb/ce6230.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/ce6230.c
b/drivers/media/dvb/dvb-usb/ce6230.c
index 3df2045..6d1a304 100644
--- a/drivers/media/dvb/dvb-usb/ce6230.c
+++ b/drivers/media/dvb/dvb-usb/ce6230.c
@@ -39,7 +39,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
u8 requesttype;
u16 value;
u16 index;
- u8 buf[req->data_len];
+ u8 *buf;
request = req->cmd;
value = req->value;
@@ -62,6 +62,12 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
goto error;
}
+ buf = kmalloc(req->data_len, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
/* write */
memcpy(buf, req->data, req->data_len);
@@ -74,7 +80,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
msleep(1); /* avoid I2C errors */
ret = usb_control_msg(udev, pipe, request, requesttype, value, index,
- buf, sizeof(buf), CE6230_USB_TIMEOUT);
+ buf, req->data_len, CE6230_USB_TIMEOUT);
ce6230_debug_dump(request, requesttype, value, index, buf,
req->data_len, deb_xfer);
@@ -88,6 +94,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct
req_t *req)
if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
memcpy(req->data, buf, req->data_len);
+ kfree(buf);
error:
return ret;
}
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/16] [media] friio: get rid of on-stack dma buffers
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
---
drivers/media/dvb/dvb-usb/friio.c | 23 +++
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/friio.c
b/drivers/media/dvb/dvb-usb/friio.c
index 14a65b4..76159ae 100644
--- a/drivers/media/dvb/dvb-usb/friio.c
+++ b/drivers/media/dvb/dvb-usb/friio.c
@@ -142,17 +142,20 @@ static u32 gl861_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_I2C;
}
-
static int friio_ext_ctl(struct dvb_usb_adapter *adap,
u32 sat_color, int lnb_on)
{
int i;
int ret;
struct i2c_msg msg;
- u8 buf[2];
+ u8 *buf;
u32 mask;
u8 lnb = (lnb_on) ? FRIIO_CTL_LNB : 0;
+ buf = kmalloc(2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
msg.addr = 0x00;
msg.flags = 0;
msg.len = 2;
@@ -189,6 +192,7 @@ static int friio_ext_ctl(struct dvb_usb_adapter *adap,
buf[1] |= FRIIO_CTL_CLK;
ret += gl861_i2c_xfer(&adap->dev->i2c_adap, &msg, 1);
+ kfree(buf);
return (ret == 70);
}
@@ -219,11 +223,20 @@ static int friio_initialize(struct dvb_usb_device *d)
int ret;
int i;
int retry = 0;
- u8 rbuf[2];
- u8 wbuf[3];
+ u8 *rbuf, *wbuf;
deb_info("%s called.\n", __func__);
+ wbuf = kmalloc(3, GFP_KERNEL);
+ if (!wbuf)
+ return -ENOMEM;
+
+ rbuf = kmalloc(2, GFP_KERNEL);
+ if (!rbuf) {
+ kfree(wbuf);
+ return -ENOMEM;
+ }
+
/* use gl861_i2c_msg instead of gl861_i2c_xfer(), */
/* because the i2c device is not set up yet. */
wbuf[0] = 0x11;
@@ -358,6 +371,8 @@ restart:
return 0;
error:
+ kfree(wbuf);
+ kfree(rbuf);
deb_info("%s:ret == %d\n", __func__, ret);
return -EIO;
}
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers
On Sun, 6 Mar 2011 18:57:13 +0100
Florian Mickler wrote:
> On Sun, 6 Mar 2011 18:47:56 +0100
> Florian Mickler wrote:
>
>
> > +static void dib0700_disconnect(struct usb_interface *intf) {
>
>
> That { should go on its own line... sorry ;-)
>
> If that patch is acceptable, I can resend with that fixed.
>
> Regards,
> Flo
Hi Mauro,
what about this patch? I have similar patches for the same problem in
the other dvb-usb drivers in need of beeing fixed. Are you
maintaining these drivers or should I find someone else to pick these
up?
Regards,
Flo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers
On Sun, 6 Mar 2011 18:47:56 +0100
Florian Mickler wrote:
> +static void dib0700_disconnect(struct usb_interface *intf) {
That { should go on its own line... sorry ;-)
If that patch is acceptable, I can resend with that fixed.
Regards,
Flo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] dib0700: remove unused variable
Signed-off-by: Florian Mickler
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
CC: Oliver Neukum
---
drivers/media/dvb/dvb-usb/dib0700_core.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 0b04cb6..5770265 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -576,7 +576,6 @@ struct dib0700_rc_response {
static void dib0700_rc_urb_completion(struct urb *purb)
{
struct dvb_usb_device *d = purb->context;
- struct dib0700_state *st;
struct dib0700_rc_response *poll_reply;
u32 uninitialized_var(keycode);
u8 toggle;
@@ -591,7 +590,6 @@ static void dib0700_rc_urb_completion(struct urb *purb)
return;
}
- st = d->priv;
poll_reply = purb->transfer_buffer;
if (purb->status < 0) {
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 v3] [media] dib0700: get rid of on-stack dma buffers
This should fix warnings seen by some:
WARNING: at lib/dma-debug.c:866 check_for_stack
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
Reported-by: Zdenek Kabelac
Signed-off-by: Florian Mickler
CC: Mauro Carvalho Chehab
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
CC: Oliver Neukum
CC: Jack Stone
---
[v2: use preallocated buffer; fix sizeof in one case]
[v3: use seperate kmalloc mapping for the preallocation,
dont ignore errors in probe codepaths ]
drivers/media/dvb/dvb-usb/dib0700.h |5 +-
drivers/media/dvb/dvb-usb/dib0700_core.c | 119 --
2 files changed, 98 insertions(+), 26 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700.h
b/drivers/media/dvb/dvb-usb/dib0700.h
index 3537d65..99a1485 100644
--- a/drivers/media/dvb/dvb-usb/dib0700.h
+++ b/drivers/media/dvb/dvb-usb/dib0700.h
@@ -45,8 +45,9 @@ struct dib0700_state {
u8 is_dib7000pc;
u8 fw_use_new_i2c_api;
u8 disable_streaming_master_mode;
-u32 fw_version;
-u32 nb_packet_buffer_size;
+ u32 fw_version;
+ u32 nb_packet_buffer_size;
+ u8 *buf;
};
extern int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 98ffb40..0b04cb6 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -27,11 +27,17 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
u32 *romversion, u32 *ramversion, u32 *fwtype)
{
- u8 b[16];
- int ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+ int ret;
+ u8 *b;
+
+ b = kmalloc(16, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
REQUEST_GET_VERSION,
USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- b, sizeof(b), USB_CTRL_GET_TIMEOUT);
+ b, 16, USB_CTRL_GET_TIMEOUT);
if (hwversion != NULL)
*hwversion = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) |
b[3];
if (romversion != NULL)
@@ -40,6 +46,8 @@ int dib0700_get_version(struct dvb_usb_device *d, u32
*hwversion,
*ramversion = (b[8] << 24) | (b[9] << 16) | (b[10] << 8) |
b[11];
if (fwtype != NULL)
*fwtype = (b[12] << 24) | (b[13] << 16) | (b[14] << 8) |
b[15];
+
+ kfree(b);
return ret;
}
@@ -101,8 +109,19 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx, u8
txlen, u8 *rx, u8 rxlen
int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
gpio_dir, u8 gpio_val)
{
- u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
((gpio_val & 0x01) << 6) };
- return dib0700_ctrl_wr(d, buf, sizeof(buf));
+ s16 ret;
+ u8 *buf = kmalloc(3, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = REQUEST_SET_GPIO;
+ buf[1] = gpio;
+ buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
+
+ ret = dib0700_ctrl_wr(d, buf, 3);
+
+ kfree(buf);
+ return ret;
}
static int dib0700_set_usb_xfer_len(struct dvb_usb_device *d, u16
nb_ts_packets)
@@ -137,11 +156,12 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
properly support i2c read calls not preceded by a write */
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
uint8_t bus_mode = 1; /* 0=eeprom bus, 1=frontend bus */
uint8_t gen_mode = 0; /* 0=master i2c, 1=gpio i2c */
uint8_t en_start = 0;
uint8_t en_stop = 0;
- uint8_t buf[255]; /* TBV: malloc ? */
+ uint8_t *buf = st->buf;
int result, i;
/* Ensure nobody else hits the i2c bus while we're sending our
@@ -221,6 +241,7 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
}
}
mutex_unlock(&d->i2c_mutex);
+
return i;
}
@@ -231,8 +252,9 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,
struct i2c_msg *msg, int num)
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
int i,len;
- u8 buf[255];
+ u8 *buf = st->buf;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
@@ -264,8 +286,8 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,
Re: [PATCH] [media] dib0700: get rid of on-stack dma buffers
On Sun, 6 Mar 2011 16:06:38 +0100 Oliver Neukum wrote: > Am Sonntag, 6. März 2011, 15:38:05 schrieb Florian Mickler: > > On Sun, 6 Mar 2011 13:06:09 +0100 > > Oliver Neukum wrote: > > > > > Am Sonntag, 6. März 2011, 12:16:52 schrieb Florian Mickler: > > > > > Please take a look at it, as I do not do that much kernel hacking > > > > and don't wanna brake anybodys computer... :) > > > > > > > > From my point of view this should _not_ go to stable even though it > > > > would > > > > be applicable. But if someone feels strongly about that and can > > > > take responsibility for that change... > > > > > > The patch looks good and is needed in stable. > > > It could be improved by using a buffer allocated once in the places > > > you hold a mutex anyway. > > > > > > Regards > > > Oliver > > > > Ok, I now put a buffer member in the priv dib0700_state which gets > > allocated on the heap. > > This however is wrong. Just like DMA on the stack this breaks > coherency rules. You may do DMA to the heap in the sense that > you can do DMA to buffers allocated on the heap, but you cannot > do DMA to a part of another structure allocated on the heap. > You need a separate kmalloc for each buffer. > You can reuse the buffer with proper locking, but you must allocate > it seperately once. > > Regards > Oliver Hm.. allocating the buffer in the probe routine and deallocating it in the usb_driver disconnect callback should work? How come that it must be a seperate kmalloc buffer? Is it some aligning that kmalloc garantees? Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] [media] dib0700: don't ignore errors in driver probe
Signed-off-by: Florian Mickler
CC: Mauro Carvalho Chehab
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
CC: Oliver Neukum
CC: Jack Stone
---
drivers/media/dvb/dvb-usb/dib0700_core.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 77a3060..3ad18ce 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -698,6 +698,7 @@ static int dib0700_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
int i;
+ int ret;
struct dvb_usb_device *dev;
for (i = 0; i < dib0700_device_count; i++)
@@ -706,8 +707,10 @@ static int dib0700_probe(struct usb_interface *intf,
struct dib0700_state *st = dev->priv;
u32 hwversion, romversion, fw_version, fwtype;
- dib0700_get_version(dev, &hwversion, &romversion,
+ ret = dib0700_get_version(dev, &hwversion, &romversion,
&fw_version, &fwtype);
+ if (ret < 0)
+ goto out;
deb_info("Firmware version: %x, %d, 0x%x, %d\n",
hwversion, romversion, fw_version, fwtype);
@@ -721,11 +724,15 @@ static int dib0700_probe(struct usb_interface *intf,
else
dev->props.rc.core.bulk_mode = false;
- dib0700_rc_setup(dev);
+ ret = dib0700_rc_setup(dev);
+ if (ret)
+ goto out;
return 0;
+out:
+ dvb_usb_device_exit();
+ return ret;
}
-
return -ENODEV;
}
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] [media] dib0700: remove unused variable
Signed-off-by: Florian Mickler
CC: Mauro Carvalho Chehab
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
CC: Oliver Neukum
CC: Jack Stone
---
drivers/media/dvb/dvb-usb/dib0700_core.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 028ed87..77a3060 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -576,7 +576,6 @@ struct dib0700_rc_response {
static void dib0700_rc_urb_completion(struct urb *purb)
{
struct dvb_usb_device *d = purb->context;
- struct dib0700_state *st;
struct dib0700_rc_response *poll_reply;
u32 uninitialized_var(keycode);
u8 toggle;
@@ -591,7 +590,6 @@ static void dib0700_rc_urb_completion(struct urb *purb)
return;
}
- st = d->priv;
poll_reply = purb->transfer_buffer;
if (purb->status < 0) {
--
1.7.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3 v2] [media] dib0700: get rid of on-stack dma buffers
This should fix warnings seen by some:
WARNING: at lib/dma-debug.c:866 check_for_stack
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
Reported-by: Zdenek Kabelac
Signed-off-by: Florian Mickler
CC: Mauro Carvalho Chehab
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
CC: Oliver Neukum
CC: Jack Stone
[v2: use preallocated buffer where the mutex is held; fix sizeof in one case]
---
drivers/media/dvb/dvb-usb/dib0700.h |5 +-
drivers/media/dvb/dvb-usb/dib0700_core.c | 92 +++---
2 files changed, 74 insertions(+), 23 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700.h
b/drivers/media/dvb/dvb-usb/dib0700.h
index 3537d65..1401e7d 100644
--- a/drivers/media/dvb/dvb-usb/dib0700.h
+++ b/drivers/media/dvb/dvb-usb/dib0700.h
@@ -45,8 +45,9 @@ struct dib0700_state {
u8 is_dib7000pc;
u8 fw_use_new_i2c_api;
u8 disable_streaming_master_mode;
-u32 fw_version;
-u32 nb_packet_buffer_size;
+ u32 fw_version;
+ u32 nb_packet_buffer_size;
+ u8 buf[255];
};
extern int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 98ffb40..028ed87 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -27,11 +27,17 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
u32 *romversion, u32 *ramversion, u32 *fwtype)
{
- u8 b[16];
- int ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+ int ret;
+ u8 *b;
+
+ b = kmalloc(16, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
REQUEST_GET_VERSION,
USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- b, sizeof(b), USB_CTRL_GET_TIMEOUT);
+ b, 16, USB_CTRL_GET_TIMEOUT);
if (hwversion != NULL)
*hwversion = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) |
b[3];
if (romversion != NULL)
@@ -40,6 +46,8 @@ int dib0700_get_version(struct dvb_usb_device *d, u32
*hwversion,
*ramversion = (b[8] << 24) | (b[9] << 16) | (b[10] << 8) |
b[11];
if (fwtype != NULL)
*fwtype = (b[12] << 24) | (b[13] << 16) | (b[14] << 8) |
b[15];
+
+ kfree(b);
return ret;
}
@@ -101,8 +109,19 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx, u8
txlen, u8 *rx, u8 rxlen
int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
gpio_dir, u8 gpio_val)
{
- u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
((gpio_val & 0x01) << 6) };
- return dib0700_ctrl_wr(d, buf, sizeof(buf));
+ s16 ret;
+ u8 *buf = kmalloc(3, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = REQUEST_SET_GPIO;
+ buf[1] = gpio;
+ buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
+
+ ret = dib0700_ctrl_wr(d, buf, 3);
+
+ kfree(buf);
+ return ret;
}
static int dib0700_set_usb_xfer_len(struct dvb_usb_device *d, u16
nb_ts_packets)
@@ -137,11 +156,12 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
properly support i2c read calls not preceded by a write */
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
uint8_t bus_mode = 1; /* 0=eeprom bus, 1=frontend bus */
uint8_t gen_mode = 0; /* 0=master i2c, 1=gpio i2c */
uint8_t en_start = 0;
uint8_t en_stop = 0;
- uint8_t buf[255]; /* TBV: malloc ? */
+ uint8_t *buf = st->buf;
int result, i;
/* Ensure nobody else hits the i2c bus while we're sending our
@@ -221,6 +241,7 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
}
}
mutex_unlock(&d->i2c_mutex);
+
return i;
}
@@ -231,8 +252,9 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,
struct i2c_msg *msg, int num)
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
+ struct dib0700_state *st = d->priv;
int i,len;
- u8 buf[255];
+ u8 *buf = st->buf;
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
return -EAGAIN;
@@ -264,8 +286,8 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,
break;
}
}
-
mutex_unlock
Re: [PATCH] [media] dib0700: get rid of on-stack dma buffers
On Sun, 6 Mar 2011 13:06:09 +0100 Oliver Neukum wrote: > Am Sonntag, 6. März 2011, 12:16:52 schrieb Florian Mickler: > > This should fix warnings seen by some: > > WARNING: at lib/dma-debug.c:866 check_for_stack > > > > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=15977. > > Reported-by: Zdenek Kabelac > > Signed-off-by: Florian Mickler > > CC: Mauro Carvalho Chehab > > CC: linux-media@vger.kernel.org > > CC: linux-ker...@vger.kernel.org > > CC: Greg Kroah-Hartman > > CC: Rafael J. Wysocki > > CC: Maciej Rutecki > > --- > > > > Please take a look at it, as I do not do that much kernel hacking > > and don't wanna brake anybodys computer... :) > > > > From my point of view this should _not_ go to stable even though it would > > be applicable. But if someone feels strongly about that and can > > take responsibility for that change... > > The patch looks good and is needed in stable. > It could be improved by using a buffer allocated once in the places > you hold a mutex anyway. > > Regards > Oliver Ok, I now put a buffer member in the priv dib0700_state which gets allocated on the heap. My patch introduces a new error condition in the dib0700_identify_state callback which gets not checked for in dvb_usb_find_device... Should we worry? Same for dib0700_get_version in the probe callback... But there, there was already the possibility of usb_control_msg returning an error... Regards, Flo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] dib0700: get rid of on-stack dma buffers
On Sun, 06 Mar 2011 13:49:56 +
Jack Stone wrote:
> On 06/03/2011 11:16, Florian Mickler wrote:
> > This should fix warnings seen by some:
> > WARNING: at lib/dma-debug.c:866 check_for_stack
> >
> > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
> > Reported-by: Zdenek Kabelac
> > Signed-off-by: Florian Mickler
> > CC: Mauro Carvalho Chehab
> > CC: linux-media@vger.kernel.org
> > CC: linux-ker...@vger.kernel.org
> > CC: Greg Kroah-Hartman
> > CC: Rafael J. Wysocki
> > CC: Maciej Rutecki
> > ---
> > @@ -101,8 +109,19 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx,
> > u8 txlen, u8 *rx, u8 rxlen
> >
> > int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
> > gpio_dir, u8 gpio_val)
> > {
> > - u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
> > ((gpio_val & 0x01) << 6) };
> > - return dib0700_ctrl_wr(d, buf, sizeof(buf));
> > + s16 ret;
> > + u8 *buf = kmalloc(3, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + buf[0] = REQUEST_SET_GPIO;
> > + buf[1] = gpio;
> > + buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
> > +
> > + ret = dib0700_ctrl_wr(d, buf, sizeof(buf));
>
> Shouldn't this sizeof be changed as well?
>
> Thanks,
>
> Jack
argh... indeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] dib0700: get rid of on-stack dma buffers
This should fix warnings seen by some:
WARNING: at lib/dma-debug.c:866 check_for_stack
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=15977.
Reported-by: Zdenek Kabelac
Signed-off-by: Florian Mickler
CC: Mauro Carvalho Chehab
CC: linux-media@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: Greg Kroah-Hartman
CC: Rafael J. Wysocki
CC: Maciej Rutecki
---
Please take a look at it, as I do not do that much kernel hacking
and don't wanna brake anybodys computer... :)
>From my point of view this should _not_ go to stable even though it would
be applicable. But if someone feels strongly about that and can
take responsibility for that change...
drivers/media/dvb/dvb-usb/dib0700_core.c | 121 +++---
1 files changed, 94 insertions(+), 27 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dib0700_core.c
b/drivers/media/dvb/dvb-usb/dib0700_core.c
index 98ffb40..1a12182 100644
--- a/drivers/media/dvb/dvb-usb/dib0700_core.c
+++ b/drivers/media/dvb/dvb-usb/dib0700_core.c
@@ -27,11 +27,17 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
int dib0700_get_version(struct dvb_usb_device *d, u32 *hwversion,
u32 *romversion, u32 *ramversion, u32 *fwtype)
{
- u8 b[16];
- int ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+ int ret;
+ u8 *b;
+
+ b = kmalloc(16, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
REQUEST_GET_VERSION,
USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
- b, sizeof(b), USB_CTRL_GET_TIMEOUT);
+ b, 16, USB_CTRL_GET_TIMEOUT);
if (hwversion != NULL)
*hwversion = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) |
b[3];
if (romversion != NULL)
@@ -40,6 +46,8 @@ int dib0700_get_version(struct dvb_usb_device *d, u32
*hwversion,
*ramversion = (b[8] << 24) | (b[9] << 16) | (b[10] << 8) |
b[11];
if (fwtype != NULL)
*fwtype = (b[12] << 24) | (b[13] << 16) | (b[14] << 8) |
b[15];
+
+ kfree(b);
return ret;
}
@@ -101,8 +109,19 @@ int dib0700_ctrl_rd(struct dvb_usb_device *d, u8 *tx, u8
txlen, u8 *rx, u8 rxlen
int dib0700_set_gpio(struct dvb_usb_device *d, enum dib07x0_gpios gpio, u8
gpio_dir, u8 gpio_val)
{
- u8 buf[3] = { REQUEST_SET_GPIO, gpio, ((gpio_dir & 0x01) << 7) |
((gpio_val & 0x01) << 6) };
- return dib0700_ctrl_wr(d, buf, sizeof(buf));
+ s16 ret;
+ u8 *buf = kmalloc(3, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = REQUEST_SET_GPIO;
+ buf[1] = gpio;
+ buf[2] = ((gpio_dir & 0x01) << 7) | ((gpio_val & 0x01) << 6);
+
+ ret = dib0700_ctrl_wr(d, buf, sizeof(buf));
+
+ kfree(buf);
+ return ret;
}
static int dib0700_set_usb_xfer_len(struct dvb_usb_device *d, u16
nb_ts_packets)
@@ -141,13 +160,20 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
uint8_t gen_mode = 0; /* 0=master i2c, 1=gpio i2c */
uint8_t en_start = 0;
uint8_t en_stop = 0;
- uint8_t buf[255]; /* TBV: malloc ? */
+ uint8_t *buf;
int result, i;
+ buf = kmalloc(255, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
/* Ensure nobody else hits the i2c bus while we're sending our
sequence of messages, (such as the remote control thread) */
- if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
- return -EAGAIN;
+ if (mutex_lock_interruptible(&d->i2c_mutex) < 0) {
+ result = -EAGAIN;
+ goto out;
+ }
+
for (i = 0; i < num; i++) {
if (i == 0) {
@@ -220,8 +246,12 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap,
struct i2c_msg *msg,
}
}
}
+ result = i;
mutex_unlock(&d->i2c_mutex);
- return i;
+
+out:
+ kfree(buf);
+ return result;
}
/*
@@ -232,10 +262,17 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter
*adap,
{
struct dvb_usb_device *d = i2c_get_adapdata(adap);
int i,len;
- u8 buf[255];
+ s16 ret;
+ u8 *buf;
- if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
- return -EAGAIN;
+ buf = kmalloc(255, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ if (mutex_lock_interruptible(&d->i2c_mutex) < 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
for (i = 0; i < num; i++) {
/* fill in the address */
@@ -264,9 +301,11 @@ static int dib0700_i2c_xfer_legacy(
