2011-03-21 10:19:29

by Florian Mickler

[permalink] [raw]
Subject: [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


2011-03-21 10:19:37

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2011-03-21 10:19:32

by Florian Mickler

[permalink] [raw]
Subject: [PATCH 1/9] [media] vp702x: cleanup: whitespace and indentation

Some whitespace, one linebreak and one unneded variable
initialization...

Signed-off-by: Florian Mickler <[email protected]>
---
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

2011-03-21 10:19:43

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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);
+ mutex_unlock(&dst->buf_mutex);
+
return 0;
}

@@ -293,13 +293,10 @@ static int vp702x_fe_set_voltage (struct dvb_frontend* fe, fe_sec_voltage_t
voltage)
{
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->voltage = voltage;

if (voltage != SEC_VOLTAGE_OFF)
@@ -308,6 +305,10 @@ static int vp702x_fe_set_voltage (struct dvb_frontend* fe, fe_sec_voltage_t
st->lnb_buf[4] = 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);
@@ -316,7 +317,7 @@ static int vp702x_fe_set_voltage (struct dvb_frontend* fe, fe_sec_voltage_t
else
deb_fe("set_voltage cmd succeeded.\n");

- kfree(buf);
+ mutex_unlock(&dst->buf_mutex);
return 0;
}

--
1.7.4.1

2011-03-21 10:19:39

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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 <linux/mutex.h>

/* 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_TUNER 0xBE
/* 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

2011-03-21 10:19:41

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2011-03-21 10:20:16

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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, 0xffff, 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

2011-03-21 10:20:32

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2011-03-21 10:20:56

by Florian Mickler

[permalink] [raw]
Subject: [PATCH 4/9] [media] vp702x: remove unused variable

struct vp702x_device_state.power_state is nowhere referenced.

Signed-off-by: Florian Mickler <[email protected]>
---
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

2011-03-21 10:20:54

by Florian Mickler

[permalink] [raw]
Subject: [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 <[email protected]>
---
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,14 +276,16 @@ static int vp702x_fe_set_tone(struct dvb_frontend* fe, fe_sec_tone_mode_t tone)
else
st->lnb_buf[2] = 0x00;

- st->lnb_buf[7] = vp702x_chksum(st->lnb_buf,0,7);
+ st->lnb_buf[7] = vp702x_chksum(st->lnb_buf, 0, 7);
+ memcpy(buf, st->lnb_buf, 8);

- vp702x_usb_inout_op(st->d,st->lnb_buf,8,ibuf,10,100);
- if (ibuf[2] == 0 && ibuf[3] == 0)
+ vp702x_usb_inout_op(st->d, buf, 8, buf, 10, 100);
+ if (buf[2] == 0 && buf[3] == 0)
deb_fe("set_tone cmd failed.\n");
else
deb_fe("set_tone cmd succeeded.\n");

+ kfree(buf);
return 0;
}

@@ -262,9 +293,13 @@ static int vp702x_fe_set_voltage (struct dvb_frontend* fe, fe_sec_voltage_t
voltage)
{
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->voltage = voltage;

if (voltage != SEC_VOLTAGE_OFF)
@@ -272,14 +307,16 @@ static int vp702x_fe_set_voltage (struct dvb_frontend* fe, fe_sec_voltage_t
else
st->lnb_buf[4] = 0x00;

- st->lnb_buf[7] = vp702x_chksum(st->lnb_buf,0,7);
+ st->lnb_buf[7] = vp702x_chksum(st->lnb_buf, 0, 7);
+ memcpy(buf, st->lnb_buf, 8);

- vp702x_usb_inout_op(st->d,st->lnb_buf,8,ibuf,10,100);
- if (ibuf[2] == 0 && ibuf[3] == 0)
+ vp702x_usb_inout_op(st->d, buf, 8, buf, 10, 100);
+ if (buf[2] == 0 && buf[3] == 0)
deb_fe("set_voltage cmd failed.\n");
else
deb_fe("set_voltage cmd succeeded.\n");

+ kfree(buf);
return 0;
}

diff --git a/drivers/media/dvb/dvb-usb/vp702x.c b/drivers/media/dvb/dvb-usb/vp702x.c
index 569c93f..35fe778 100644
--- a/drivers/media/dvb/dvb-usb/vp702x.c
+++ b/drivers/media/dvb/dvb-usb/vp702x.c
@@ -93,38 +93,61 @@ 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)
{
- u8 bout[olen+2];
- u8 bin[ilen+1];
int ret = 0;
+ u8 *buf;
+ int buflen = max(olen + 2, ilen + 1);

- bout[0] = 0x00;
- bout[1] = cmd;
- memcpy(&bout[2],o,olen);
+ buf = kmalloc(buflen, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;

- ret = vp702x_usb_inout_op(d, bout, olen+2, bin, ilen+1,msec);
+ buf[0] = 0x00;
+ buf[1] = cmd;
+ memcpy(&buf[2], o, olen);
+
+ ret = vp702x_usb_inout_op(d, buf, olen+2, buf, ilen+1, msec);

if (ret == 0)
- memcpy(i,&bin[1],ilen);
+ memcpy(i, &buf[1], ilen);

+ kfree(buf);
return ret;
}

static int vp702x_set_pld_mode(struct dvb_usb_adapter *adap, u8 bypass)
{
- u8 buf[16] = { 0 };
- return vp702x_usb_in_op(adap->dev, 0xe0, (bypass << 8) | 0x0e, 0, buf, 16);
+ int ret;
+ u8 *buf = kzalloc(16, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = vp702x_usb_in_op(adap->dev, 0xe0, (bypass << 8) | 0x0e,
+ 0, buf, 16);
+ kfree(buf);
+ return ret;
}

static int vp702x_set_pld_state(struct dvb_usb_adapter *adap, u8 state)
{
- u8 buf[16] = { 0 };
- return vp702x_usb_in_op(adap->dev, 0xe0, (state << 8) | 0x0f, 0, buf, 16);
+ int ret;
+ u8 *buf = kzalloc(16, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = vp702x_usb_in_op(adap->dev, 0xe0, (state << 8) | 0x0f,
+ 0, buf, 16);
+ kfree(buf);
+ 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;
- u8 buf[16] = { 0 };
+ u8 *buf;
+
+ buf = kzalloc(16, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;

if (onoff)
st->pid_filter_state |= (1 << id);
@@ -138,6 +161,8 @@ static int vp702x_set_pid(struct dvb_usb_adapter *adap, u16 pid, u8 id, int onof
vp702x_set_pld_state(adap, st->pid_filter_state);
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);
return 0;
}

@@ -146,13 +171,17 @@ static int vp702x_init_pid_filter(struct dvb_usb_adapter *adap)
{
struct vp702x_adapter_state *st = adap->priv;
int i;
- u8 b[10] = { 0 };
+ 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;

- vp702x_set_pld_mode(adap, 1); // bypass
+ vp702x_set_pld_mode(adap, 1); /* bypass */

for (i = 0; i < st->pid_filter_count; i++)
vp702x_set_pid(adap, 0xffff, i, 1);
@@ -162,6 +191,7 @@ static int vp702x_init_pid_filter(struct dvb_usb_adapter *adap)
vp702x_usb_in_op(adap->dev, 0xb5, 1, 0, b, 10);

//vp702x_set_pld_mode(d, 0); // filter
+ kfree(b);
return 0;
}

@@ -179,18 +209,23 @@ static struct rc_map_table rc_map_vp702x_table[] = {
/* remote control stuff (does not work with my box) */
static int vp702x_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
{
- u8 key[10];
+ u8 *key;
int i;

/* remove the following return to enabled remote querying */
return 0;

+ key = kmalloc(10, GFP_KERNEL);
+ if (!key)
+ return -ENOMEM;
+
vp702x_usb_in_op(d,READ_REMOTE_REQ,0,0,key,10);

deb_rc("remote query key: %x %d\n",key[1],key[1]);

if (key[1] == 0x44) {
*state = REMOTE_NO_KEY_PRESSED;
+ kfree(key);
return 0;
}

@@ -200,15 +235,24 @@ static int vp702x_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
*event = rc_map_vp702x_table[i].keycode;
break;
}
+ kfree(key);
return 0;
}


static int vp702x_read_mac_addr(struct dvb_usb_device *d,u8 mac[6])
{
- u8 i;
+ u8 i, *buf;
+
+ buf = kmalloc(6, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
for (i = 6; i < 12; i++)
- vp702x_usb_in_op(d, READ_EEPROM_REQ, i, 1, &mac[i - 6], 1);
+ vp702x_usb_in_op(d, READ_EEPROM_REQ, i, 1, &buf[i - 6], 1);
+
+ memcpy(mac, buf, 6);
+ kfree(buf);
return 0;
}

--
1.7.4.1