[permalink] [raw]

Subject: [PATCH 11/26] digitv: don't do DMA on stack

2016-10-07 17:25:09

by Mauro Carvalho Chehab

[permalink] [raw]

Subject: [PATCH 26/26] digitv: handle error code on RC query

There's no sense on decoding and generating a RC key code if
there was an error on the URB control message.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb/digitv.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/digitv.c b/drivers/media/usb/dvb-usb/digitv.c
index 09f8c28bd4db..4284f6984dc1 100644
--- a/drivers/media/usb/dvb-usb/digitv.c
+++ b/drivers/media/usb/dvb-usb/digitv.c
@@ -29,7 +29,9 @@ static int digitv_ctrl_msg(struct dvb_usb_device *d,
u8 cmd, u8 vv, u8 *wbuf, int wlen, u8 *rbuf, int rlen)
{
struct digitv_state *st = d->priv;
- int wo = (rbuf == NULL || rlen == 0); /* write-only */
+ int ret, wo;
+
+ wo = (rbuf == NULL || rlen == 0); /* write-only */

memset(st->sndbuf, 0, 7);
memset(st->rcvbuf, 0, 7);
@@ -40,12 +42,12 @@ static int digitv_ctrl_msg(struct dvb_usb_device *d,

if (wo) {
memcpy(&st->sndbuf[3], wbuf, wlen);
- dvb_usb_generic_write(d, st->sndbuf, 7);
+ ret = dvb_usb_generic_write(d, st->sndbuf, 7);
} else {
- dvb_usb_generic_rw(d, st->sndbuf, 7, st->rcvbuf, 7, 10);
+ ret = dvb_usb_generic_rw(d, st->sndbuf, 7, st->rcvbuf, 7, 10);
memcpy(rbuf, &st->rcvbuf[3], rlen);
}
- return 0;
+ return ret;
}

/* I2C */
--
2.7.4

2016-10-07 17:25:16

by Mauro Carvalho Chehab

[permalink] [raw]

2016-10-07 17:26:55

by Mauro Carvalho Chehab

[permalink] [raw]

Subject: [PATCH 22/26] technisat-usb2: use DMA buffers for I2C transfers

The USB control messages require DMA to work. We cannot pass
a stack-allocated buffer, as it is not warranted that the
stack would be into a DMA enabled area.

On this driver, most of the transfers are OK, but the I2C
one was using stack.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb/technisat-usb2.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c
index d9f3262bf071..4706628a3ed5 100644
--- a/drivers/media/usb/dvb-usb/technisat-usb2.c
+++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
@@ -89,9 +89,13 @@ struct technisat_usb2_state {
static int technisat_usb2_i2c_access(struct usb_device *udev,
u8 device_addr, u8 *tx, u8 txlen, u8 *rx, u8 rxlen)
{
- u8 b[64];
+ u8 *b;
int ret, actual_length;

+ b = kmalloc(64, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
deb_i2c("i2c-access: %02x, tx: ", device_addr);
debug_dump(tx, txlen, deb_i2c);
deb_i2c(" ");
@@ -123,7 +127,7 @@ static int technisat_usb2_i2c_access(struct usb_device *udev,

if (ret < 0) {
err("i2c-error: out failed %02x = %d", device_addr, ret);
- return -ENODEV;
+ goto err;
}

ret = usb_bulk_msg(udev,
@@ -131,7 +135,7 @@ static int technisat_usb2_i2c_access(struct usb_device *udev,
b, 64, &actual_length, 1000);
if (ret < 0) {
err("i2c-error: in failed %02x = %d", device_addr, ret);
- return -ENODEV;
+ goto err;
}

if (b[0] != I2C_STATUS_OK) {
@@ -140,7 +144,7 @@ static int technisat_usb2_i2c_access(struct usb_device *udev,
if (!(b[0] == I2C_STATUS_NAK &&
device_addr == 0x60
/* && device_is_technisat_usb2 */))
- return -ENODEV;
+ goto err;
}

deb_i2c("status: %d, ", b[0]);
@@ -154,7 +158,9 @@ static int technisat_usb2_i2c_access(struct usb_device *udev,

deb_i2c("\n");

- return 0;
+err:
+ kfree(b);
+ return ret;
}

static int technisat_usb2_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
--
2.7.4

2016-10-07 17:27:05

by Mauro Carvalho Chehab

[permalink] [raw]

2016-10-07 17:27:38

by Mauro Carvalho Chehab

[permalink] [raw]

Subject: [PATCH 01/26] af9005: don't do DMA on stack

The USB control messages require DMA to work. We cannot pass
a stack-allocated buffer, as it is not warranted that the
stack would be into a DMA enabled area.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb/af9005.c | 211 +++++++++++++++++++------------------
1 file changed, 111 insertions(+), 100 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/af9005.c b/drivers/media/usb/dvb-usb/af9005.c
index efa782ed6e2d..cc5815de1cfb 100644
--- a/drivers/media/usb/dvb-usb/af9005.c
+++ b/drivers/media/usb/dvb-usb/af9005.c
@@ -52,17 +52,15 @@ u8 regmask[8] = { 0x01, 0x03, 0x07, 0x0f, 0x1f, 0x3f, 0x7f, 0xff };
struct af9005_device_state {
u8 sequence;
int led_state;
+ unsigned char data[256];
};

static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg,
int readwrite, int type, u8 * values, int len)
{
struct af9005_device_state *st = d->priv;
- u8 obuf[16] = { 0 };
- u8 ibuf[17] = { 0 };
- u8 command;
- int i;
- int ret;
+ u8 command, seq;
+ int i, ret;

if (len < 1) {
err("generic read/write, less than 1 byte. Makes no sense.");
@@ -73,16 +71,16 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg,
return -EINVAL;
}

- obuf[0] = 14; /* rest of buffer length low */
- obuf[1] = 0; /* rest of buffer length high */
+ st->data[0] = 14; /* rest of buffer length low */
+ st->data[1] = 0; /* rest of buffer length high */

- obuf[2] = AF9005_REGISTER_RW; /* register operation */
- obuf[3] = 12; /* rest of buffer length */
+ st->data[2] = AF9005_REGISTER_RW; /* register operation */
+ st->data[3] = 12; /* rest of buffer length */

- obuf[4] = st->sequence++; /* sequence number */
+ st->data[4] = seq = st->sequence++; /* sequence number */

- obuf[5] = (u8) (reg >> 8); /* register address */
- obuf[6] = (u8) (reg & 0xff);
+ st->data[5] = (u8) (reg >> 8); /* register address */
+ st->data[6] = (u8) (reg & 0xff);

if (type == AF9005_OFDM_REG) {
command = AF9005_CMD_OFDM_REG;
@@ -96,49 +94,43 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg,
command |= readwrite;
if (readwrite == AF9005_CMD_WRITE)
for (i = 0; i < len; i++)
- obuf[8 + i] = values[i];
+ st->data[8 + i] = values[i];
else if (type == AF9005_TUNER_REG)
/* read command for tuner, the first byte contains the i2c address */
- obuf[8] = values[0];
- obuf[7] = command;
+ st->data[8] = values[0];
+ st->data[7] = command;

- ret = dvb_usb_generic_rw(d, obuf, 16, ibuf, 17, 0);
+ ret = dvb_usb_generic_rw(d, st->data, 16, st->data, 17, 0);
if (ret)
return ret;

/* sanity check */
- if (ibuf[2] != AF9005_REGISTER_RW_ACK) {
+ if (st->data[2] != AF9005_REGISTER_RW_ACK) {
err("generic read/write, wrong reply code.");
return -EIO;
}
- if (ibuf[3] != 0x0d) {
+ if (st->data[3] != 0x0d) {
err("generic read/write, wrong length in reply.");
return -EIO;
}
- if (ibuf[4] != obuf[4]) {
+ if (st->data[4] != seq) {
err("generic read/write, wrong sequence in reply.");
return -EIO;
}
/*
- Windows driver doesn't check these fields, in fact sometimes
- the register in the reply is different that what has been sent
-
- if (ibuf[5] != obuf[5] || ibuf[6] != obuf[6]) {
- err("generic read/write, wrong register in reply.");
- return -EIO;
- }
- if (ibuf[7] != command) {
- err("generic read/write wrong command in reply.");
- return -EIO;
- }
+ * In thesis, both input and output buffers should have
+ * identical values for st->data[5] to st->data[8].
+ * However, windows driver doesn't check these fields, in fact
+ * sometimes the register in the reply is different that what
+ * has been sent
*/
- if (ibuf[16] != 0x01) {
+ if (st->data[16] != 0x01) {
err("generic read/write wrong status code in reply.");
return -EIO;
}
if (readwrite == AF9005_CMD_READ)
for (i = 0; i < len; i++)
- values[i] = ibuf[8 + i];
+ values[i] = st->data[8 + i];

return 0;

@@ -464,8 +456,7 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf,
struct af9005_device_state *st = d->priv;

int ret, i, packet_len;
- u8 buf[64];
- u8 ibuf[64];
+ u8 seq;

if (wlen < 0) {
err("send command, wlen less than 0 bytes. Makes no sense.");
@@ -480,37 +471,37 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf,
return -EINVAL;
}
packet_len = wlen + 5;
- buf[0] = (u8) (packet_len & 0xff);
- buf[1] = (u8) ((packet_len & 0xff00) >> 8);
+ st->data[0] = (u8) (packet_len & 0xff);
+ st->data[1] = (u8) ((packet_len & 0xff00) >> 8);

- buf[2] = 0x26; /* packet type */
- buf[3] = wlen + 3;
- buf[4] = st->sequence++;
- buf[5] = command;
- buf[6] = wlen;
+ st->data[2] = 0x26; /* packet type */
+ st->data[3] = wlen + 3;
+ st->data[4] = seq = st->sequence++;
+ st->data[5] = command;
+ st->data[6] = wlen;
for (i = 0; i < wlen; i++)
- buf[7 + i] = wbuf[i];
- ret = dvb_usb_generic_rw(d, buf, wlen + 7, ibuf, rlen + 7, 0);
+ st->data[7 + i] = wbuf[i];
+ ret = dvb_usb_generic_rw(d, st->data, wlen + 7, st->data, rlen + 7, 0);
if (ret)
return ret;
- if (ibuf[2] != 0x27) {
+ if (st->data[2] != 0x27) {
err("send command, wrong reply code.");
return -EIO;
}
- if (ibuf[4] != buf[4]) {
+ if (st->data[4] != seq) {
err("send command, wrong sequence in reply.");
return -EIO;
}
- if (ibuf[5] != 0x01) {
+ if (st->data[5] != 0x01) {
err("send command, wrong status code in reply.");
return -EIO;
}
- if (ibuf[6] != rlen) {
+ if (st->data[6] != rlen) {
err("send command, invalid data length in reply.");
return -EIO;
}
for (i = 0; i < rlen; i++)
- rbuf[i] = ibuf[i + 7];
+ rbuf[i] = st->data[i + 7];
return 0;
}

@@ -518,56 +509,56 @@ int af9005_read_eeprom(struct dvb_usb_device *d, u8 address, u8 * values,
int len)
{
struct af9005_device_state *st = d->priv;
- u8 obuf[16], ibuf[14];
+ u8 seq;
int ret, i;

- memset(obuf, 0, sizeof(obuf));
- memset(ibuf, 0, sizeof(ibuf));
+ memset(st->data, 0, sizeof(st->data));

- obuf[0] = 14; /* length of rest of packet low */
- obuf[1] = 0; /* length of rest of packer high */
+ st->data[0] = 14; /* length of rest of packet low */
+ st->data[1] = 0; /* length of rest of packer high */

- obuf[2] = 0x2a; /* read/write eeprom */
+ st->data[2] = 0x2a; /* read/write eeprom */

- obuf[3] = 12; /* size */
+ st->data[3] = 12; /* size */

- obuf[4] = st->sequence++;
+ st->data[4] = seq = st->sequence++;

- obuf[5] = 0; /* read */
+ st->data[5] = 0; /* read */

- obuf[6] = len;
- obuf[7] = address;
- ret = dvb_usb_generic_rw(d, obuf, 16, ibuf, 14, 0);
+ st->data[6] = len;
+ st->data[7] = address;
+ ret = dvb_usb_generic_rw(d, st->data, 16, st->data, 14, 0);
if (ret)
return ret;
- if (ibuf[2] != 0x2b) {
+ if (st->data[2] != 0x2b) {
err("Read eeprom, invalid reply code");
return -EIO;
}
- if (ibuf[3] != 10) {
+ if (st->data[3] != 10) {
err("Read eeprom, invalid reply length");
return -EIO;
}
- if (ibuf[4] != obuf[4]) {
+ if (st->data[4] != seq) {
err("Read eeprom, wrong sequence in reply ");
return -EIO;
}
- if (ibuf[5] != 1) {
+ if (st->data[5] != 1) {
err("Read eeprom, wrong status in reply ");
return -EIO;
}
for (i = 0; i < len; i++) {
- values[i] = ibuf[6 + i];
+ values[i] = st->data[6 + i];
}
return 0;
}

-static int af9005_boot_packet(struct usb_device *udev, int type, u8 * reply)
+static int af9005_boot_packet(struct usb_device *udev, int type, u8 *reply,
+ u8 *buf, int size)
{
- u8 buf[FW_BULKOUT_SIZE + 2];
u16 checksum;
int act_len, i, ret;
- memset(buf, 0, sizeof(buf));
+
+ memset(buf, 0, size);
buf[0] = (u8) (FW_BULKOUT_SIZE & 0xff);
buf[1] = (u8) ((FW_BULKOUT_SIZE >> 8) & 0xff);
switch (type) {
@@ -720,15 +711,21 @@ static int af9005_download_firmware(struct usb_device *udev, const struct firmwa
{
int i, packets, ret, act_len;

- u8 buf[FW_BULKOUT_SIZE + 2];
+ u8 *buf;
u8 reply;

- ret = af9005_boot_packet(udev, FW_CONFIG, &reply);
+ buf = kmalloc(FW_BULKOUT_SIZE + 2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = af9005_boot_packet(udev, FW_CONFIG, &reply, buf,
+ FW_BULKOUT_SIZE + 2);
if (ret)
- return ret;
+ goto err;
if (reply != 0x01) {
err("before downloading firmware, FW_CONFIG expected 0x01, received 0x%x", reply);
- return -EIO;
+ ret = -EIO;
+ goto err;
}
packets = fw->size / FW_BULKOUT_SIZE;
buf[0] = (u8) (FW_BULKOUT_SIZE & 0xff);
@@ -743,28 +740,35 @@ static int af9005_download_firmware(struct usb_device *udev, const struct firmwa
buf, FW_BULKOUT_SIZE + 2, &act_len, 1000);
if (ret) {
err("firmware download failed at packet %d with code %d", i, ret);
- return ret;
+ goto err;
}
}
- ret = af9005_boot_packet(udev, FW_CONFIRM, &reply);
+ ret = af9005_boot_packet(udev, FW_CONFIRM, &reply,
+ buf, FW_BULKOUT_SIZE + 2);
if (ret)
- return ret;
+ goto err;
if (reply != (u8) (packets & 0xff)) {
err("after downloading firmware, FW_CONFIRM expected 0x%x, received 0x%x", packets & 0xff, reply);
- return -EIO;
+ ret = -EIO;
+ goto err;
}
- ret = af9005_boot_packet(udev, FW_BOOT, &reply);
+ ret = af9005_boot_packet(udev, FW_BOOT, &reply, buf,
+ FW_BULKOUT_SIZE + 2);
if (ret)
- return ret;
- ret = af9005_boot_packet(udev, FW_CONFIG, &reply);
+ goto err;
+ ret = af9005_boot_packet(udev, FW_CONFIG, &reply, buf,
+ FW_BULKOUT_SIZE + 2);
if (ret)
- return ret;
+ goto err;
if (reply != 0x02) {
err("after downloading firmware, FW_CONFIG expected 0x02, received 0x%x", reply);
- return -EIO;
+ ret = -EIO;
+ goto err;
}

- return 0;
+err:
+ kfree(buf);
+ return ret;

}

@@ -823,9 +827,7 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state)
{
struct af9005_device_state *st = d->priv;
int ret, len;
-
- u8 obuf[5];
- u8 ibuf[256];
+ u8 seq;

*state = REMOTE_NO_KEY_PRESSED;
if (rc_decode == NULL) {
@@ -833,33 +835,33 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state)
return 0;
}
/* deb_info("rc_query\n"); */
- obuf[0] = 3; /* rest of packet length low */
- obuf[1] = 0; /* rest of packet lentgh high */
- obuf[2] = 0x40; /* read remote */
- obuf[3] = 1; /* rest of packet length */
- obuf[4] = st->sequence++; /* sequence number */
- ret = dvb_usb_generic_rw(d, obuf, 5, ibuf, 256, 0);
+ st->data[0] = 3; /* rest of packet length low */
+ st->data[1] = 0; /* rest of packet lentgh high */
+ st->data[2] = 0x40; /* read remote */
+ st->data[3] = 1; /* rest of packet length */
+ st->data[4] = seq = st->sequence++; /* sequence number */
+ ret = dvb_usb_generic_rw(d, st->data, 5, st->data, 256, 0);
if (ret) {
err("rc query failed");
return ret;
}
- if (ibuf[2] != 0x41) {
+ if (st->data[2] != 0x41) {
err("rc query bad header.");
return -EIO;
}
- if (ibuf[4] != obuf[4]) {
+ if (st->data[4] != seq) {
err("rc query bad sequence.");
return -EIO;
}
- len = ibuf[5];
+ len = st->data[5];
if (len > 246) {
err("rc query invalid length");
return -EIO;
}
if (len > 0) {
deb_rc("rc data (%d) ", len);
- debug_dump((ibuf + 6), len, deb_rc);
- ret = rc_decode(d, &ibuf[6], len, event, state);
+ debug_dump((st->data + 6), len, deb_rc);
+ ret = rc_decode(d, &st->data[6], len, event, state);
if (ret) {
err("rc_decode failed");
return ret;
@@ -953,10 +955,16 @@ static int af9005_identify_state(struct usb_device *udev,
int *cold)
{
int ret;
- u8 reply;
- ret = af9005_boot_packet(udev, FW_CONFIG, &reply);
+ u8 reply, *buf;
+
+ buf = kmalloc(FW_BULKOUT_SIZE + 2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = af9005_boot_packet(udev, FW_CONFIG, &reply,
+ buf, FW_BULKOUT_SIZE + 2);
if (ret)
- return ret;
+ goto err;
deb_info("result of FW_CONFIG in identify state %d\n", reply);
if (reply == 0x01)
*cold = 1;
@@ -965,7 +973,10 @@ static int af9005_identify_state(struct usb_device *udev,
else
return -EIO;
deb_info("Identify state cold = %d\n", *cold);
- return 0;
+
+err:
+ kfree(buf);
+ return ret;
}

static struct dvb_usb_device_properties af9005_properties;
--
2.7.4

2016-10-07 17:27:47

by Mauro Carvalho Chehab

[permalink] [raw]

Subject: [PATCH 24/26] nova-t-usb2: handle error code on RC query

There's no sense on decoding and generating a RC key code if
there was an error on the URB control message.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb/nova-t-usb2.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/nova-t-usb2.c b/drivers/media/usb/dvb-usb/nova-t-usb2.c
index 26d7188a1163..1babd3341910 100644
--- a/drivers/media/usb/dvb-usb/nova-t-usb2.c
+++ b/drivers/media/usb/dvb-usb/nova-t-usb2.c
@@ -76,7 +76,7 @@ static int nova_t_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
{
u8 *buf, data, toggle, custom;
u16 raw;
- int i;
+ int i, ret;
struct dibusb_device_state *st = d->priv;

buf = kmalloc(5, GFP_KERNEL);
@@ -85,7 +85,9 @@ static int nova_t_rc_query(struct dvb_usb_device *d, u32 *event, int *state)

buf[0] = DIBUSB_REQ_POLL_REMOTE;
buf[1] = 0x35;
- dvb_usb_generic_rw(d, buf, 2, buf, 5, 0);
+ ret = dvb_usb_generic_rw(d, buf, 2, buf, 5, 0);
+ if (ret < 0)
+ goto ret;

*state = REMOTE_NO_KEY_PRESSED;
switch (buf[0]) {
@@ -124,8 +126,9 @@ static int nova_t_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
break;
}

+ret:
kfree(buf);
- return 0;
+ return ret;
}

static int nova_t_read_mac_address (struct dvb_usb_device *d, u8 mac[6])
--
2.7.4

2016-10-07 17:27:56

by Mauro Carvalho Chehab

[permalink] [raw]

2016-10-07 17:28:34

by Mauro Carvalho Chehab

[permalink] [raw]

On Fri, 7 Oct 2016 14:24:12 -0300
Mauro Carvalho Chehab <[email protected]> wrote:

> The USB control messages require DMA to work. We cannot pass
> a stack-allocated buffer, as it is not warranted that the
> stack would be into a DMA enabled area.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/usb/dvb-usb/cinergyT2-core.c | 45
> ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 18
> deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c
> b/drivers/media/usb/dvb-usb/cinergyT2-core.c index
> 9fd1527494eb..91640c927776 100644 ---
> a/drivers/media/usb/dvb-usb/cinergyT2-core.c +++
> b/drivers/media/usb/dvb-usb/cinergyT2-core.c @@ -41,6 +41,7 @@
> DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
> struct cinergyt2_state {
> u8 rc_counter;
> + unsigned char data[64];
> };
>
> /* We are missing a release hook with usb_device data */
> @@ -50,29 +51,36 @@ static struct dvb_usb_device_properties
> cinergyt2_properties;
> static int cinergyt2_streaming_ctrl(struct dvb_usb_adapter *adap,
> int enable) {
> - char buf[] = { CINERGYT2_EP1_CONTROL_STREAM_TRANSFER,
> enable ? 1 : 0 };
> - char result[64];
> - return dvb_usb_generic_rw(adap->dev, buf, sizeof(buf),
> result,
> - sizeof(result), 0);
> + struct dvb_usb_device *d = adap->dev;
> + struct cinergyt2_state *st = d->priv;
> +
> + st->data[0] = CINERGYT2_EP1_CONTROL_STREAM_TRANSFER;
> + st->data[1] = enable ? 1 : 0;
> +
> + return dvb_usb_generic_rw(d, st->data, 2, st->data, 64, 0);
> }
>
> static int cinergyt2_power_ctrl(struct dvb_usb_device *d, int enable)
> {
> - char buf[] = { CINERGYT2_EP1_SLEEP_MODE, enable ? 0 : 1 };
> - char state[3];
> - return dvb_usb_generic_rw(d, buf, sizeof(buf), state,
> sizeof(state), 0);
> + struct cinergyt2_state *st = d->priv;
> +
> + st->data[0] = CINERGYT2_EP1_SLEEP_MODE;
> + st->data[1] = enable ? 0 : 1;
> +
> + return dvb_usb_generic_rw(d, st->data, 2, st->data, 3, 0);
> }
>
> static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap)
> {
> - char query[] = { CINERGYT2_EP1_GET_FIRMWARE_VERSION };
> - char state[3];
> + struct dvb_usb_device *d = adap->dev;
> + struct cinergyt2_state *st = d->priv;
> int ret;
>
> adap->fe_adap[0].fe = cinergyt2_fe_attach(adap->dev);
>
> - ret = dvb_usb_generic_rw(adap->dev, query, sizeof(query),
> state,
> - sizeof(state), 0);
> + st->data[0] = CINERGYT2_EP1_GET_FIRMWARE_VERSION;
> +
> + ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 3, 0);
> if (ret < 0) {
> deb_rc("cinergyt2_power_ctrl() Failed to retrieve
> sleep " "state info\n");
> @@ -141,13 +149,14 @@ static int repeatable_keys[] = {
> static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event,
> int *state) {
> struct cinergyt2_state *st = d->priv;
> - u8 key[5] = {0, 0, 0, 0, 0}, cmd =
> CINERGYT2_EP1_GET_RC_EVENTS; int i;
>
> *state = REMOTE_NO_KEY_PRESSED;
>
> - dvb_usb_generic_rw(d, &cmd, 1, key, sizeof(key), 0);
> - if (key[4] == 0xff) {
> + st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS;
> +
> + dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0);
> + if (st->data[4] == 0xff) {
> /* key repeat */
> st->rc_counter++;
> if (st->rc_counter > RC_REPEAT_DELAY) {
> @@ -166,13 +175,13 @@ static int cinergyt2_rc_query(struct
> dvb_usb_device *d, u32 *event, int *state) }
>
> /* hack to pass checksum on the custom field */
> - key[2] = ~key[1];
> - dvb_usb_nec_rc_key_to_event(d, key, event, state);
> - if (key[0] != 0) {
> + st->data[2] = ~st->data[1];
> + dvb_usb_nec_rc_key_to_event(d, st->data, event, state);
> + if (st->data[0] != 0) {
> if (*event != d->last_event)
> st->rc_counter = 0;
>
> - deb_rc("key: %*ph\n", 5, key);
> + deb_rc("key: %*ph\n", 5, st->data);
> }
> return 0;
> }

Reviewed-By: Patrick Boettcher <[email protected]>

2016-10-10 06:40:12

by Patrick Boettcher

[permalink] [raw]

Subject: Re: [PATCH 09/26] dibusb: don't do DMA on stack

On Fri, 7 Oct 2016 14:24:19 -0300
Mauro Carvalho Chehab <[email protected]> wrote:

> The USB control messages require DMA to work. We cannot pass
> a stack-allocated buffer, as it is not warranted that the
> stack would be into a DMA enabled area.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/usb/dvb-usb/dibusb-common.c | 106
> +++++++++++++++++++++---------
> drivers/media/usb/dvb-usb/dibusb.h | 5 ++ 2 files changed,
> 81 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c
> b/drivers/media/usb/dvb-usb/dibusb-common.c index
> 4b08c2a47ae2..76b26dc7339c 100644 ---
> a/drivers/media/usb/dvb-usb/dibusb-common.c +++
> b/drivers/media/usb/dvb-usb/dibusb-common.c @@ -12,9 +12,6 @@
> #include <linux/kconfig.h>
> #include "dibusb.h"
>
> -/* Max transfer size done by I2C transfer functions */
> -#define MAX_XFER_SIZE 64
> -
> static int debug;
> module_param(debug, int, 0644);
> MODULE_PARM_DESC(debug, "set debugging level (1=info (|-able))."
> DVB_USB_DEBUG_STATUS); @@ -63,72 +60,109 @@
> EXPORT_SYMBOL(dibusb_pid_filter_ctrl);
> int dibusb_power_ctrl(struct dvb_usb_device *d, int onoff)
> {
> - u8 b[3];
> + u8 *b;
> int ret;
> +
> + b = kmalloc(3, GFP_KERNEL);
> + if (!b)
> + return -ENOMEM;
> +
> b[0] = DIBUSB_REQ_SET_IOCTL;
> b[1] = DIBUSB_IOCTL_CMD_POWER_MODE;
> b[2] = onoff ? DIBUSB_IOCTL_POWER_WAKEUP :
> DIBUSB_IOCTL_POWER_SLEEP;
> - ret = dvb_usb_generic_write(d,b,3);
> +
> + ret = dvb_usb_generic_write(d, b, 3);
> +
> + kfree(b);
> +
> msleep(10);
> +
> return ret;
> }
> EXPORT_SYMBOL(dibusb_power_ctrl);
>
> int dibusb2_0_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff)
> {
> - u8 b[3] = { 0 };
> + struct dibusb_state *st = adap->priv;
> int ret;
>
> if ((ret = dibusb_streaming_ctrl(adap,onoff)) < 0)
> return ret;
>
> if (onoff) {
> - b[0] = DIBUSB_REQ_SET_STREAMING_MODE;
> - b[1] = 0x00;
> - if ((ret = dvb_usb_generic_write(adap->dev,b,2)) < 0)
> + st->data[0] = DIBUSB_REQ_SET_STREAMING_MODE;
> + st->data[1] = 0x00;
> + ret = dvb_usb_generic_write(adap->dev, st->data, 2);
> + if (ret < 0)
> return ret;
> }
>
> - b[0] = DIBUSB_REQ_SET_IOCTL;
> - b[1] = onoff ? DIBUSB_IOCTL_CMD_ENABLE_STREAM :
> DIBUSB_IOCTL_CMD_DISABLE_STREAM;
> - return dvb_usb_generic_write(adap->dev,b,3);
> + st->data[0] = DIBUSB_REQ_SET_IOCTL;
> + st->data[1] = onoff ? DIBUSB_IOCTL_CMD_ENABLE_STREAM :
> DIBUSB_IOCTL_CMD_DISABLE_STREAM;
> + return dvb_usb_generic_write(adap->dev, st->data, 3);
> }
> EXPORT_SYMBOL(dibusb2_0_streaming_ctrl);
>
> int dibusb2_0_power_ctrl(struct dvb_usb_device *d, int onoff)
> {
> - if (onoff) {
> - u8 b[3] = { DIBUSB_REQ_SET_IOCTL,
> DIBUSB_IOCTL_CMD_POWER_MODE, DIBUSB_IOCTL_POWER_WAKEUP };
> - return dvb_usb_generic_write(d,b,3);
> - } else
> + u8 *b;
> + int ret;
> +
> + if (!onoff)
> return 0;
> +
> + b = kmalloc(3, GFP_KERNEL);
> + if (!b)
> + return -ENOMEM;
> +
> + b[0] = DIBUSB_REQ_SET_IOCTL;
> + b[1] = DIBUSB_IOCTL_CMD_POWER_MODE;
> + b[2] = DIBUSB_IOCTL_POWER_WAKEUP;
> +
> + ret = dvb_usb_generic_write(d, b, 3);
> +
> + kfree(b);
> +
> + return ret;
> }
> EXPORT_SYMBOL(dibusb2_0_power_ctrl);
>
> static int dibusb_i2c_msg(struct dvb_usb_device *d, u8 addr,
> u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen)
> {
> - u8 sndbuf[MAX_XFER_SIZE]; /* lead(1) devaddr,direction(1)
> addr(2) data(wlen) (len(2) (when reading)) */
> + u8 *sndbuf;
> + int ret, wo, len;
> +
> /* write only ? */
> - int wo = (rbuf == NULL || rlen == 0),
> - len = 2 + wlen + (wo ? 0 : 2);
> + wo = (rbuf == NULL || rlen == 0);
>
> - if (4 + wlen > sizeof(sndbuf)) {
> + len = 2 + wlen + (wo ? 0 : 2);
> +
> + sndbuf = kmalloc(MAX_XFER_SIZE, GFP_KERNEL);
> + if (!sndbuf)
> + return -ENOMEM;
> +
> + if (4 + wlen > MAX_XFER_SIZE) {
> warn("i2c wr: len=%d is too big!\n", wlen);
> - return -EOPNOTSUPP;
> + ret = -EOPNOTSUPP;
> + goto ret;
> }
>
> sndbuf[0] = wo ? DIBUSB_REQ_I2C_WRITE : DIBUSB_REQ_I2C_READ;
> sndbuf[1] = (addr << 1) | (wo ? 0 : 1);
>
> - memcpy(&sndbuf[2],wbuf,wlen);
> + memcpy(&sndbuf[2], wbuf, wlen);
>
> if (!wo) {
> - sndbuf[wlen+2] = (rlen >> 8) & 0xff;
> - sndbuf[wlen+3] = rlen & 0xff;
> + sndbuf[wlen + 2] = (rlen >> 8) & 0xff;
> + sndbuf[wlen + 3] = rlen & 0xff;
> }
>
> - return dvb_usb_generic_rw(d,sndbuf,len,rbuf,rlen,0);
> + ret = dvb_usb_generic_rw(d, sndbuf, len, rbuf, rlen, 0);
> +
> +ret:
> + kfree(sndbuf);
> + return ret;
> }
>
> /*
> @@ -320,11 +354,23 @@ EXPORT_SYMBOL(rc_map_dibusb_table);
>
> int dibusb_rc_query(struct dvb_usb_device *d, u32 *event, int *state)
> {
> - u8 key[5],cmd = DIBUSB_REQ_POLL_REMOTE;
> - dvb_usb_generic_rw(d,&cmd,1,key,5,0);
> - dvb_usb_nec_rc_key_to_event(d,key,event,state);
> - if (key[0] != 0)
> - deb_info("key: %*ph\n", 5, key);
> + u8 *buf;
> +
> + buf = kmalloc(5, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + buf[0] = DIBUSB_REQ_POLL_REMOTE;
> +
> + dvb_usb_generic_rw(d, buf, 1, buf, 5, 0);
> +
> + dvb_usb_nec_rc_key_to_event(d, buf, event, state);
> +
> + if (buf[0] != 0)
> + deb_info("key: %*ph\n", 5, buf);
> +
> + kfree(buf);
> +
> return 0;
> }
> EXPORT_SYMBOL(dibusb_rc_query);
> diff --git a/drivers/media/usb/dvb-usb/dibusb.h
> b/drivers/media/usb/dvb-usb/dibusb.h index 3f82163d8ab8..42e9750393e5
> 100644 --- a/drivers/media/usb/dvb-usb/dibusb.h
> +++ b/drivers/media/usb/dvb-usb/dibusb.h
> @@ -96,10 +96,15 @@
> #define DIBUSB_IOCTL_CMD_ENABLE_STREAM 0x01
> #define DIBUSB_IOCTL_CMD_DISABLE_STREAM 0x02
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> struct dibusb_state {
> struct dib_fe_xfer_ops ops;
> int mt2060_present;
> u8 tuner_addr;
> +
> + unsigned char data[MAX_XFER_SIZE];
> };
>
> struct dibusb_device_state {

Reviewed-By: Patrick Boettcher <[email protected]>