Don't use stack for DMA transers on media usb drivers
Subsystems (2)
v2 cover Don't use stack for DMA transers on media usb drivers
Sending URB control messages from stack was never supported. Yet, on x86, the stack was usually at a memory region that allows DMA transfer.
So, several drivers got it wrong. On Kernel 4.9, if VMAP_STACK=y, none of those drivers will work, as the stack won't be on a DMA-able area anymore.
So, fix the media drivers that requre it. It should noticed that the vast majority are at dvb-usb.
Please notice that those patches are compile-tested only. So, I really appreciate if people with devices using those drivers could test and report if they don't break anything. I intend to send this
PS.: I didn't verify any media driver inside staging.
Version 2:
- Added other USB drivers that are out of dvb-usb. After this series, all drivers will use kmalloc'ed buffers, although I won't doubt that some implementations may still have issues.
- Be sure that, on all places where it is using a buffer allocated at the dvb-usb state structs, they'll be protected by some mutex. On some such cases, there was already a mutex at the DVB core. So, I didn't need to add a new one. Thanks to Antti that reminded me about such need.
Mauro Carvalho Chehab (31):
af9005: don't do DMA on stack
cinergyT2-core: don't do DMA on stack
cinergyT2-core: handle error code on RC query
cinergyT2-fe: cache stats at cinergyt2_fe_read_status()
cinergyT2-fe: don't do DMA on stack
cxusb: don't do DMA on stack
dib0700: be sure that dib0700_ctrl_rd() users can do DMA
dib0700_core: don't use stack on I2C reads
dibusb: don't do DMA on stack
dibusb: handle error code on RC query
digitv: don't do DMA on stack
dtt200u-fe: don't keep waiting for lock at set_frontend()
dtt200u-fe: don't do DMA on stack
dtt200u-fe: handle errors on USB control messages
dtt200u: don't do DMA on stack
dtt200u: handle USB control message errors
dtv5100: don't do DMA on stack
gp8psk: don't do DMA on stack
gp8psk: don't go past the buffer size
nova-t-usb2: don't do DMA on stack
pctv452e: don't do DMA on stack
pctv452e: don't call BUG_ON() on non-fatal error
technisat-usb2: use DMA buffers for I2C transfers
dvb-usb: warn if return value for USB read/write routines is not
checked
nova-t-usb2: handle error code on RC query
dw2102: return error if su3000_power_ctrl() fails
digitv: handle error code on RC query
cpia2_usb: don't use stack for DMA
s2255drv: don't use stack for DMA
stk-webcam: don't use stack for DMA
flexcop-usb: don't use stack for DMA
drivers/media/usb/b2c2/flexcop-usb.c | 105 +++++---- drivers/media/usb/b2c2/flexcop-usb.h | 4 + drivers/media/usb/cpia2/cpia2_usb.c | 32 ++- drivers/media/usb/dvb-usb/af9005.c | 317 ++++++++++++++++------------ drivers/media/usb/dvb-usb/cinergyT2-core.c | 90 +++++--- drivers/media/usb/dvb-usb/cinergyT2-fe.c | 100 ++++----- drivers/media/usb/dvb-usb/cxusb.c | 62 +++--- drivers/media/usb/dvb-usb/cxusb.h | 6 + drivers/media/usb/dvb-usb/dib0700_core.c | 31 ++- drivers/media/usb/dvb-usb/dib0700_devices.c | 25 +-- drivers/media/usb/dvb-usb/dibusb-common.c | 113 +++++++--- drivers/media/usb/dvb-usb/dibusb.h | 3 + drivers/media/usb/dvb-usb/digitv.c | 26 ++- drivers/media/usb/dvb-usb/digitv.h | 3 + drivers/media/usb/dvb-usb/dtt200u-fe.c | 128 +++++++---- drivers/media/usb/dvb-usb/dtt200u.c | 120 ++++++++--- drivers/media/usb/dvb-usb/dtv5100.c | 10 +- drivers/media/usb/dvb-usb/dvb-usb.h | 6 +- drivers/media/usb/dvb-usb/dw2102.c | 2 +- drivers/media/usb/dvb-usb/gp8psk.c | 25 ++- drivers/media/usb/dvb-usb/nova-t-usb2.c | 25 ++- drivers/media/usb/dvb-usb/pctv452e.c | 136 +++++++----- drivers/media/usb/dvb-usb/technisat-usb2.c | 16 +- drivers/media/usb/s2255/s2255drv.c | 15 +- drivers/media/usb/stkwebcam/stk-webcam.c | 16 +- 25 files changed, 918 insertions(+), 498 deletions(-)
Signature · 1 line
2.7.4
PATCH v2 1/31 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.
--- drivers/media/usb/dvb-usb/af9005.c | 317 +++++++++++++++++++++---------------- 1 file changed, 180 insertions(+), 137 deletions(-)
drivers/media/usb/dvb-usb/af9005.c +180 -137
@@ -52,17 +52,16 @@ u8 regmask[8] = { 0x01, 0x03, 0x07, 0x0f, 0x1f, 0x3f, 0x7f, 0xff };struct af9005_device_state {u8 sequence;int led_state; +unsigned char data[256 ]; +struct mutex data_mutex; };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 +72,17 @@ 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 */ +mutex_lock (&st->data_mutex); + 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,51 +96,52 @@ 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; + goto 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 ; + ret = -EIO ; + goto ret; } -if (ibuf[3 ] !=0x0d ) { +if (st->data[3 ] !=0x0d ) {err ("generic read/write, wrong length in reply." ); -return -EIO ; + ret = -EIO ; + goto ret; } -if (ibuf[4 ] != obuf[4 ]) { +if (st->data[4 ] != seq) {err ("generic read/write, wrong sequence in reply." ); -return -EIO ; + ret = -EIO ; + goto ret; }/* -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 ; + ret = -EIO ; + goto ret; } +if (readwrite ==AF9005_CMD_READ )for (i =0 ; i < len; i++) - values[i] = ibuf[8 + i]; + values[i] = st->data[8 + i]; -return 0 ; +ret: +mutex_unlock (&st->data_mutex); +return ret; }
@@ -464,8 +465,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,94 +480,97 @@ 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 ); - buf[2 ] =0x26 ;/* packet type */ - buf[3 ] = wlen +3 ; - buf[4 ] = st->sequence++; - buf[5 ] = command; - buf[6 ] = wlen; +mutex_lock (&st->data_mutex); + + st->data[0 ] = (u8) (packet_len &0xff ); + st->data[1 ] = (u8) ((packet_len &0xff00 ) >>8 ); + + 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 ); -if (ret) -return ret; -if (ibuf[2 ] !=0x27 ) { + st->data[7 + i] = wbuf[i]; + ret =dvb_usb_generic_rw (d, st->data, wlen +7 , st->data, rlen +7 ,0 ); +if (st->data[2 ] !=0x27 ) {err ("send command, wrong reply code." ); -return -EIO ; - } -if (ibuf[4 ] != buf[4 ]) { + ret = -EIO ; + }else if (st->data[4 ] != seq) {err ("send command, wrong sequence in reply." ); -return -EIO ; - } -if (ibuf[5 ] !=0x01 ) { + ret = -EIO ; + }else if (st->data[5 ] !=0x01 ) {err ("send command, wrong status code in reply." ); -return -EIO ; - } -if (ibuf[6 ] != rlen) { + ret = -EIO ; + }else if (st->data[6 ] != rlen) {err ("send command, invalid data length in reply." ); -return -EIO ; + ret = -EIO ; } -for (i =0 ; i < rlen; i++) - rbuf[i] = ibuf[i +7 ]; -return 0 ; +if (!ret) { +for (i =0 ; i < rlen; i++) + rbuf[i] = st->data[i +7 ]; + } + +mutex_unlock (&st->data_mutex); +return ret; }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)); +mutex_lock (&st->data_mutex); - obuf[0 ] =14 ;/* length of rest of packet low */ - obuf[1 ] =0 ;/* length of rest of packer high */ +memset (st->data,0 ,sizeof (st->data)); - obuf[2 ] =0x2a ;/* read/write eeprom */ + st->data[0 ] =14 ;/* length of rest of packet low */ + st->data[1 ] =0 ;/* length of rest of packer high */ - obuf[3 ] =12 ;/* size */ + st->data[2 ] =0x2a ;/* read/write eeprom */ - obuf[4 ] = st->sequence++; + st->data[3 ] =12 ;/* size */ - obuf[5 ] =0 ;/* read */ + st->data[4 ] = seq = st->sequence++; - obuf[6 ] = len; - obuf[7 ] = address; - ret =dvb_usb_generic_rw (d, obuf,16 , ibuf,14 ,0 ); -if (ret) -return ret; -if (ibuf[2 ] !=0x2b ) { + st->data[5 ] =0 ;/* read */ + + st->data[6 ] = len; + st->data[7 ] = address; + ret =dvb_usb_generic_rw (d, st->data,16 , st->data,14 ,0 ); +if (st->data[2 ] !=0x2b ) {err ("Read eeprom, invalid reply code" ); -return -EIO ; - } -if (ibuf[3 ] !=10 ) { + ret = -EIO ; + }else if (st->data[3 ] !=10 ) {err ("Read eeprom, invalid reply length" ); -return -EIO ; - } -if (ibuf[4 ] != obuf[4 ]) { + ret = -EIO ; + }else if (st->data[4 ] != seq) {err ("Read eeprom, wrong sequence in reply " ); -return -EIO ; - } -if (ibuf[5 ] !=1 ) { + ret = -EIO ; + }else if (st->data[5 ] !=1 ) {err ("Read eeprom, wrong status in reply " ); -return -EIO ; + ret = -EIO ; } -for (i =0 ; i < len; i++) { - values[i] = ibuf[6 + i]; + +if (!ret) { +for (i =0 ; i < len; i++) + values[i] = st->data[6 + i]; } -return 0 ; +mutex_unlock (&st->data_mutex); + +return ret; } -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 +723,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 +752,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,53 +839,59 @@ 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 ) {/* it shouldn't never come here */ return 0 ; } + +mutex_lock (&st->data_mutex); +/* 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; + goto ret; } -if (ibuf[2 ] !=0x41 ) { +if (st->data[2 ] !=0x41 ) {err ("rc query bad header." ); -return -EIO ; - } -if (ibuf[4 ] != obuf[4 ]) { + ret = -EIO ; + goto ret; + }else if (st->data[4 ] != seq) {err ("rc query bad sequence." ); -return -EIO ; + ret = -EIO ; + goto ret; } - len = ibuf[5 ]; + len = st->data[5 ];if (len >246 ) {err ("rc query invalid length" ); -return -EIO ; + ret = -EIO ; + goto ret; }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; + goto ret; }else {deb_rc ("rc_decode state %x event %x\n" , *state, *event);if (*state ==REMOTE_KEY_REPEAT ) *event = d->last_event; } } -return 0 ; + +ret: +mutex_unlock (&st->data_mutex); +return ret; }static int af9005_power_ctrl (struct dvb_usb_device *d,int onoff)
@@ -953,10 +975,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 +993,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;
@@ -973,8 +1004,20 @@ static struct dvb_usb_device_properties af9005_properties;static int af9005_usb_probe (struct usb_interface *intf,const struct usb_device_id *id) { -return dvb_usb_device_init (intf, &af9005_properties, -THIS_MODULE ,NULL , adapter_nr); +struct dvb_usb_device *d; +struct af9005_device_state *st; +int ret; + + ret =dvb_usb_device_init (intf, &af9005_properties, +THIS_MODULE , &d, adapter_nr); + +if (ret <0 ) +return ret; + + st = d->priv; +mutex_init (&st->data_mutex); + +return 0 ; }enum af9005_usb_table_entry {
Signature · 1 line
2.7.4
PATCH v2 2/31 cinergyT2-core: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/cinergyT2-core.c | 84 ++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 23 deletions(-)
drivers/media/usb/dvb-usb/cinergyT2-core.c +61 -23
@@ -41,6 +41,8 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);struct cinergyt2_state {u8 rc_counter; +unsigned char data[64 ]; +struct mutex data_mutex; };/* We are missing a release hook with usb_device data */
@@ -50,33 +52,52 @@ 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; +int ret; + +mutex_lock (&st->data_mutex); + st->data[0 ] =CINERGYT2_EP1_CONTROL_STREAM_TRANSFER ; + st->data[1 ] = enable ?1 :0 ; + + ret =dvb_usb_generic_rw (d, st->data,2 , st->data,64 ,0 ); +mutex_unlock (&st->data_mutex); + +return ret; }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; +int ret; + +mutex_lock (&st->data_mutex); + st->data[0 ] =CINERGYT2_EP1_SLEEP_MODE ; + st->data[1 ] = enable ?0 :1 ; + + ret =dvb_usb_generic_rw (d, st->data,2 , st->data,3 ,0 ); +mutex_unlock (&st->data_mutex); + +return ret; }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 ); +mutex_lock (&st->data_mutex); + 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" ); } +mutex_unlock (&st->data_mutex);/* Copy this pointer as we are gonna need it in the release phase */ cinergyt2_usb_device = adap->dev;
@@ -141,13 +162,16 @@ 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 ) { +mutex_lock (&st->data_mutex); + 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 ) {
@@ -157,31 +181,45 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) *event = d->last_event;deb_rc ("repeat key, event %x\n" , *event); -return 0 ; + goto ret; } }deb_rc ("repeated key (non repeatable)\n" ); } -return 0 ; + goto ret; }/* 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 ; + +ret: +mutex_unlock (&st->data_mutex); +return ret; }static int cinergyt2_usb_probe (struct usb_interface *intf,const struct usb_device_id *id) { -return dvb_usb_device_init (intf, &cinergyt2_properties, -THIS_MODULE ,NULL , adapter_nr); +struct dvb_usb_device *d; +struct cinergyt2_state *st; +int ret; + + ret =dvb_usb_device_init (intf, &cinergyt2_properties, +THIS_MODULE , &d, adapter_nr); +if (ret <0 ) +return ret; + + st = d->priv; +mutex_init (&st->data_mutex); + +return 0 ; }
Signature · 1 line
2.7.4
Re: [PATCH v2 02/31] cinergyT2-core: don't do DMA on stack
drivers/media/usb/dvb-usb/cinergyT2-core.c DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); · line 46
Quoted 9 lines · Mauro Carvalho Chehab
drivers/media/usb/dvb-usb/cinergyT2-core.c +2
-0@@ -41,6 +41,8 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);struct cinergyt2_state {u8 rc_counter; +unsigned char data[64 ]; +struct mutex data_mutex; };
Sometimes my thinking is slow but it just occured to me that this creates a potential issue with cache line sharing. On an architecture which manages cache coherence in software (ARM, MIPS etc.) a write to e.g. rc_counter in this example would dirty the cache line, and a later writeback from the cache could overwrite parts of data[] which was received via DMA. In contrast, if the DMA buffer is allocated seperately via kmalloc it is guaranteed to be safe wrt cache line sharing. (see bottom of Documentation/DMA-API-HOWTO.txt).
But of course DMA on stack also had the same issue and no one ever noticed so it's apparently not critical...
Johannes
Re: [PATCH v2 02/31] cinergyT2-core: don't do DMA on stack
Em Sat, 15 Oct 2016 22:54:49 +0200
Johannes Stezenbach <[email protected]> escreveu:
Quoted 5 lines · Mauro Carvalho Chehab
Quoted 4 lines
drivers/media/usb/dvb-usb/cinergyT2-core.c +0
-0@@ -41,6 +41,8 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
Quoted 16 lines
drivers/media/usb/dvb-usb/cinergyT2-core.c DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); · line 46
Quoted 5 lines
drivers/media/usb/dvb-usb/cinergyT2-core.c +2
-0struct cinergyt2_state {u8 rc_counter; +unsigned char data[64 ]; +struct mutex data_mutex; };Sometimes my thinking is slow but it just occured to me that this creates a potential issue with cache line sharing. On an architecture which manages cache coherence in software (ARM, MIPS etc.) a write to e.g.
rc_counterin this example would dirty the cache line, and a later writeback from the cache could overwrite parts of data[] which was received via DMA. In contrast, if the DMA buffer is allocated seperately via kmalloc it is guaranteed to be safe wrt cache line sharing. (see bottom ofDocumentation/DMA-API-HOWTO.txt).
Interesting point. I'm not sure well this would work with non-fully coherent cache lines. I guess that will depend on how the USB driver will be handling it.
Quoted 2 lines
But of course DMA on stack also had the same issue and no one ever noticed so it's apparently not critical...
Yes, this shouldn't do it any worse than what we currently have. In the past, I tested some drivers that uses a shared buffed for control URB transfers in the past, on arm32 and arm64. I don't remember seeing anything weird there that could be related to cache coherency, although I remember several problems with USB on OMAP and RPi version 1, leading troubles after several minutes of ISOC transfers on analog TV, but they seemed to be unrelated to URB control traffic.
I'd say that we should keep our eyes on those drivers, after applying this patch series and see if people will notice bad behavior on non-x86 archs.
Thanks,
Mauro
PATCH v2 3/31 cinergyT2-core: handle error code on RC query
In reply to Mauro Carvalho ChehabThere's no sense on decoding and generating a RC key code if there was an error on the URB control message.
--- drivers/media/usb/dvb-usb/cinergyT2-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/media/usb/dvb-usb/cinergyT2-core.c +5 -3
@@ -102,7 +102,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap)/* Copy this pointer as we are gonna need it in the release phase */ cinergyt2_usb_device = adap->dev; -return 0 ; +return ret; }static struct rc_map_table rc_map_cinergyt2_table[] = {
@@ -162,14 +162,16 @@ static int repeatable_keys[] = {static int cinergyt2_rc_query (struct dvb_usb_device *d,u32 *event,int *state) {struct cinergyt2_state *st = d->priv; -int i; +int i, ret; *state =REMOTE_NO_KEY_PRESSED ;mutex_lock (&st->data_mutex); st->data[0 ] =CINERGYT2_EP1_GET_RC_EVENTS ; -dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); + ret =dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); +if (ret <0 ) +return ret;if (st->data[4 ] ==0xff ) {/* key repeat */
Signature · 1 line
2.7.4
PATCH v2 3/31 cinergyT2-core: 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.
drivers/media/usb/dvb-usb/cinergyT2-core.c +5 -3
@@ -102,7 +102,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap)/* Copy this pointer as we are gonna need it in the release phase */ cinergyt2_usb_device = adap->dev; -return 0 ; +return ret; }static struct rc_map_table rc_map_cinergyt2_table[] = {
@@ -162,14 +162,16 @@ static int repeatable_keys[] = {static int cinergyt2_rc_query (struct dvb_usb_device *d,u32 *event,int *state) {struct cinergyt2_state *st = d->priv; -int i; +int i, ret; *state =REMOTE_NO_KEY_PRESSED ;mutex_lock (&st->data_mutex); st->data[0 ] =CINERGYT2_EP1_GET_RC_EVENTS ; -dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); + ret =dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); +if (ret <0 ) + goto ret;if (st->data[4 ] ==0xff ) {/* key repeat */
PATCH v2 4/31 cinergyT2-fe: cache stats at cinergyt2_fe_read_status()
In reply to Mauro Carvalho ChehabInstead of sending USB commands for every stats call, collect them once, when status is updated. As the frontend kthread will call it on every few seconds, the stats will still be collected.
Besides reducing the amount of USB/I2C transfers, this also warrants that all stats will be collected at the same time, and makes easier to convert it to DVBv5 stats in the future.
--- drivers/media/usb/dvb-usb/cinergyT2-fe.c | 48 +++++--------------------------- 1 file changed, 7 insertions(+), 41 deletions(-)
drivers/media/usb/dvb-usb/cinergyT2-fe.c +7 -41
@@ -139,6 +139,7 @@ static uint16_t compute_tps(struct dtv_frontend_properties *op)struct cinergyt2_fe_state {struct dvb_frontend fe;struct dvb_usb_device *d; +struct dvbt_get_status_msg status; }; static int cinergyt2_fe_read_status(struct dvb_frontend *fe,
@@ -154,6 +155,8 @@ static int cinergyt2_fe_read_status(struct dvb_frontend *fe,if (ret <0 )return ret; + state->status = result; + *status =0 ;if (0xffff -le16_to_cpu (result.gain) >30 )
@@ -177,34 +180,16 @@ static int cinergyt2_fe_read_status(struct dvb_frontend *fe,static int cinergyt2_fe_read_ber (struct dvb_frontend *fe,u32 *ber) {struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_get_status_msg status; -char cmd[] = {CINERGYT2_EP1_GET_TUNER_STATUS }; -int ret; - ret =dvb_usb_generic_rw (state->d, cmd,sizeof (cmd), (char *)&status, -sizeof (status),0 ); -if (ret <0 ) -return ret; - - *ber =le32_to_cpu (status.viterbi_error_rate); + *ber =le32_to_cpu (state->status.viterbi_error_rate);return 0 ; }static int cinergyt2_fe_read_unc_blocks (struct dvb_frontend *fe,u32 *unc) {struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_get_status_msg status; -u8 cmd[] = {CINERGYT2_EP1_GET_TUNER_STATUS }; -int ret; - ret =dvb_usb_generic_rw (state->d, cmd,sizeof (cmd), (u8 *)&status, -sizeof (status),0 ); -if (ret <0 ) { -err ("cinergyt2_fe_read_unc_blocks() Failed! (Error=%d)\n" , - ret); -return ret; - } - *unc =le32_to_cpu (status.uncorrected_block_count); + *unc =le32_to_cpu (state->status.uncorrected_block_count);return 0 ; }
@@ -212,35 +197,16 @@ static int cinergyt2_fe_read_signal_strength(struct dvb_frontend *fe, u16 *strength) {struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_get_status_msg status; -char cmd[] = {CINERGYT2_EP1_GET_TUNER_STATUS }; -int ret; - ret =dvb_usb_generic_rw (state->d, cmd,sizeof (cmd), (char *)&status, -sizeof (status),0 ); -if (ret <0 ) { -err ("cinergyt2_fe_read_signal_strength() Failed!" -" (Error=%d)\n" , ret); -return ret; - } - *strength = (0xffff -le16_to_cpu (status.gain)); + *strength = (0xffff -le16_to_cpu (state->status.gain));return 0 ; }static int cinergyt2_fe_read_snr (struct dvb_frontend *fe,u16 *snr) {struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_get_status_msg status; -char cmd[] = {CINERGYT2_EP1_GET_TUNER_STATUS }; -int ret; - ret =dvb_usb_generic_rw (state->d, cmd,sizeof (cmd), (char *)&status, -sizeof (status),0 ); -if (ret <0 ) { -err ("cinergyt2_fe_read_snr() Failed! (Error=%d)\n" , ret); -return ret; - } - *snr = (status.snr <<8 ) | status.snr; + *snr = (state->status.snr <<8 ) | state->status.snr;return 0 ; }
Signature · 1 line
2.7.4
PATCH v2 5/31 cinergyT2-fe: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/cinergyT2-fe.c | 56 +++++++++++++++++++------------- 1 file changed, 33 insertions(+), 23 deletions(-)
drivers/media/usb/dvb-usb/cinergyT2-fe.c +33 -23
@@ -139,6 +139,10 @@ static uint16_t compute_tps(struct dtv_frontend_properties *op)struct cinergyt2_fe_state {struct dvb_frontend fe;struct dvb_usb_device *d; + +unsigned char data[64 ]; +struct mutex data_mutex; +struct dvbt_get_status_msg status; };
@@ -146,28 +150,31 @@ static int cinergyt2_fe_read_status(struct dvb_frontend *fe, enum fe_status *status) {struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_get_status_msg result; -u8 cmd[] = {CINERGYT2_EP1_GET_TUNER_STATUS };int ret; - ret =dvb_usb_generic_rw (state->d, cmd,sizeof (cmd), (u8 *)&result, -sizeof (result),0 ); +mutex_lock (&state->data_mutex); + state->data[0 ] =CINERGYT2_EP1_GET_TUNER_STATUS ; + + ret =dvb_usb_generic_rw (state->d, state->data,1 , + state->data,sizeof (state->status),0 ); +if (!ret) +memcpy (&state->status, state->data,sizeof (state->status)); +mutex_unlock (&state->data_mutex); +if (ret <0 )return ret; - state->status = result; - *status =0 ; -if (0xffff -le16_to_cpu (result.gain) >30 ) +if (0xffff -le16_to_cpu (state->status.gain) >30 ) *status |=FE_HAS_SIGNAL ; -if (result.lock_bits & (1 <<6 )) +if (state->status.lock_bits & (1 <<6 )) *status |=FE_HAS_LOCK ; -if (result.lock_bits & (1 <<5 )) +if (state->status.lock_bits & (1 <<5 )) *status |=FE_HAS_SYNC ; -if (result.lock_bits & (1 <<4 )) +if (state->status.lock_bits & (1 <<4 )) *status |=FE_HAS_CARRIER ; -if (result.lock_bits & (1 <<1 )) +if (state->status.lock_bits & (1 <<1 )) *status |=FE_HAS_VITERBI ; if ((*status & (FE_HAS_CARRIER | FE_HAS_VITERBI | FE_HAS_SYNC)) !=
@@ -232,34 +239,36 @@ static int cinergyt2_fe_set_frontend(struct dvb_frontend *fe) {struct dtv_frontend_properties *fep = &fe->dtv_property_cache;struct cinergyt2_fe_state *state = fe->demodulator_priv; -struct dvbt_set_parameters_msg param; -char result[2 ]; +struct dvbt_set_parameters_msg *param;int err; - param.cmd =CINERGYT2_EP1_SET_TUNER_PARAMETERS ; - param.tps =cpu_to_le16 (compute_tps (fep)); - param.freq =cpu_to_le32 (fep->frequency /1000 ); - param.flags =0 ; +mutex_lock (&state->data_mutex); + + param = (void *)state->data; + param->cmd =CINERGYT2_EP1_SET_TUNER_PARAMETERS ; + param->tps =cpu_to_le16 (compute_tps (fep)); + param->freq =cpu_to_le32 (fep->frequency /1000 ); + param->flags =0 ;switch (fep->bandwidth_hz) {default :case 8000000 : - param.bandwidth =8 ; + param->bandwidth =8 ;break ;case 7000000 : - param.bandwidth =7 ; + param->bandwidth =7 ;break ;case 6000000 : - param.bandwidth =6 ; + param->bandwidth =6 ;break ; } - err =dvb_usb_generic_rw (state->d, - (char *)¶m,sizeof (param), - result,sizeof (result),0 ); + err =dvb_usb_generic_rw (state->d, state->data,sizeof (*param), + state->data,2 ,0 );if (err <0 )err ("cinergyt2_fe_set_frontend() Failed! err=%d\n" , err); +mutex_unlock (&state->data_mutex);return (err <0 ) ? err :0 ; }
@@ -281,6 +290,7 @@ struct dvb_frontend *cinergyt2_fe_attach(struct dvb_usb_device *d) s->d = d;memcpy (&s->fe.ops, &cinergyt2_fe_ops,sizeof (struct dvb_frontend_ops )); s->fe.demodulator_priv = s; +mutex_init (&s->data_mutex);return &s->fe; }
Signature · 1 line
2.7.4
PATCH v2 6/31 cxusb: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/cxusb.c | 62 ++++++++++++++++++++++----------------- drivers/media/usb/dvb-usb/cxusb.h | 6 ++++ 2 files changed, 41 insertions(+), 27 deletions(-)
drivers/media/usb/dvb-usb/cxusb.c +35 -27
@@ -45,9 +45,6 @@#include "si2168.h" #include "si2157.h" -/* Max transfer size done by I2C transfer functions */ -#define MAX_XFER_SIZE 80 -/* debug */ static int dvb_usb_cxusb_debug;module_param_named (debug, dvb_usb_cxusb_debug, int,0644 );
@@ -61,23 +58,27 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);static int cxusb_ctrl_msg (struct dvb_usb_device *d,u8 cmd,u8 *wbuf,int wlen,u8 *rbuf,int rlen) { -int wo = (rbuf ==NULL || rlen ==0 );/* write-only */ -u8 sndbuf[MAX_XFER_SIZE ]; +struct cxusb_state *st = d->priv; +int ret, wo; -if (1 + wlen >sizeof (sndbuf)) { -warn ("i2c wr: len=%d is too big!\n" , - wlen); +if (1 + wlen >MAX_XFER_SIZE ) { +warn ("i2c wr: len=%d is too big!\n" , wlen);return -EOPNOTSUPP ; } -memset (sndbuf,0 ,1 +wlen); + wo = (rbuf ==NULL || rlen ==0 );/* write-only */ - sndbuf[0 ] = cmd; -memcpy (&sndbuf[1 ], wbuf, wlen); +mutex_lock (&st->data_mutex); + st->data[0 ] = cmd; +memcpy (&st->data[1 ], wbuf, wlen);if (wo) -return dvb_usb_generic_write (d, sndbuf,1 +wlen); + ret =dvb_usb_generic_write (d, st->data,1 + wlen);else -return dvb_usb_generic_rw (d, sndbuf,1 +wlen, rbuf, rlen,0 ); + ret =dvb_usb_generic_rw (d, st->data,1 + wlen, + rbuf, rlen,0 ); + +mutex_unlock (&st->data_mutex); +return ret; }/* GPIO */
@@ -1460,36 +1461,43 @@ static struct dvb_usb_device_properties cxusb_mygica_t230_properties;static int cxusb_probe (struct usb_interface *intf,const struct usb_device_id *id) { +struct dvb_usb_device *d; +struct cxusb_state *st; +if (0 ==dvb_usb_device_init (intf, &cxusb_medion_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_lgh064f_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_dee1601_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_lgz201_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_dtt7579_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_dualdig4_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_nano2_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_nano2_needsfirmware_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_aver_a868r_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_bluebird_dualdig4_rev2_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_d680_dmb_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_mygica_d689_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &cxusb_mygica_t230_properties, -THIS_MODULE ,NULL , adapter_nr) || -0 ) +THIS_MODULE , &d, adapter_nr) || +0 ) { + st = d->priv; +mutex_init (&st->data_mutex); +return 0 ; + }return -EINVAL ; }
drivers/media/usb/dvb-usb/cxusb.h +6 -0
@@ -28,10 +28,16 @@#define CMD_ANALOG 0x50#define CMD_DIGITAL 0x51 +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 80 +struct cxusb_state {u8 gpio_write_state[3 ];struct i2c_client *i2c_client_demod;struct i2c_client *i2c_client_tuner; + +unsigned char data[MAX_XFER_SIZE ]; +struct mutex data_mutex; };#endif
Signature · 1 line
2.7.4
PATCH v2 7/31 dib0700: be sure that dib0700_ctrl_rd() users can do DMA
In reply to Mauro Carvalho Chehabdib0700_ctrl_rd() takes a RX and a TX pointer. Be sure that both will point to a memory allocated via kmalloc().
--- drivers/media/usb/dvb-usb/dib0700_core.c | 4 +++- drivers/media/usb/dvb-usb/dib0700_devices.c | 25 +++++++++++++------------ 2 files changed, 16 insertions(+), 13 deletions(-)
drivers/media/usb/dvb-usb/dib0700_core.c +3 -1
@@ -292,13 +292,15 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,/* special thing in the current firmware: when length is zero the read-failed */ len =dib0700_ctrl_rd (d, st->buf, msg[i].len +2 , - msg[i+1 ].buf, msg[i+1 ].len); + st->buf, msg[i +1 ].len);if (len <=0 ) {deb_info ("I2C read failed on address 0x%02x\n" , msg[i].addr);break ; } +memcpy (msg[i +1 ].buf, st->buf, msg[i +1 ].len); + msg[i+1 ].len = len; i++;
drivers/media/usb/dvb-usb/dib0700_devices.c +13 -12
@@ -508,8 +508,6 @@ static int stk7700ph_tuner_attach(struct dvb_usb_adapter *adap)#define DEFAULT_RC_INTERVAL 50 -static u8 rc_request[] = {REQUEST_POLL_RC ,0 }; -/* * This function is used only when firmware is < 1.20 version. Newer * firmwares use bulk mode, with functions implemented at dib0700_core,
@@ -517,7 +515,6 @@ static u8 rc_request[] = { REQUEST_POLL_RC, 0 };*/ static int dib0700_rc_query_old_firmware (struct dvb_usb_device *d) { -u8 key[4 ];enum rc_type protocol;u32 scancode;u8 toggle;
@@ -532,39 +529,43 @@ static int dib0700_rc_query_old_firmware(struct dvb_usb_device *d)return 0 ; } - i =dib0700_ctrl_rd (d, rc_request,2 , key,4 ); + st->buf[0 ] =REQUEST_POLL_RC ; + st->buf[1 ] =0 ; + + i =dib0700_ctrl_rd (d, st->buf,2 , st->buf,4 );if (i <=0 ) {err ("RC Query Failed" ); -return -1 ; +return -EIO ; }/* losing half of KEY_0 events from Philipps rc5 remotes.. */ -if (key[0 ] ==0 && key[1 ] ==0 && key[2 ] ==0 && key[3 ] ==0 ) +if (st->buf[0 ] ==0 && st->buf[1 ] ==0 + && st->buf[2 ] ==0 && st->buf[3 ] ==0 )return 0 ; -/* info("%d: %2X %2X %2X %2X",dvb_usb_dib0700_ir_proto,(int)key[3-2],(int)key[3-3],(int)key[3-1],(int)key[3]); */ +/* info("%d: %2X %2X %2X %2X",dvb_usb_dib0700_ir_proto,(int)st->buf[3 - 2],(int)st->buf[3 - 3],(int)st->buf[3 - 1],(int)st->buf[3]); */ dib0700_rc_setup (d,NULL );/* reset ir sensor data to prevent false events */ switch (d->props.rc.core.protocol) {case RC_BIT_NEC :/* NEC protocol sends repeat code as 0 0 0 FF */ -if ((key[3 -2 ] ==0x00 ) && (key[3 -3 ] ==0x00 ) && - (key[3 ] ==0xff )) { +if ((st->buf[3 -2 ] ==0x00 ) && (st->buf[3 -3 ] ==0x00 ) && + (st->buf[3 ] ==0xff )) {rc_repeat (d->rc_dev);return 0 ; } protocol =RC_TYPE_NEC ; - scancode =RC_SCANCODE_NEC (key[3 -2 ], key[3 -3 ]); + scancode =RC_SCANCODE_NEC (st->buf[3 -2 ], st->buf[3 -3 ]); toggle =0 ;break ;default :/* RC-5 protocol changes toggle bit on new keypress */ protocol =RC_TYPE_RC5 ; - scancode =RC_SCANCODE_RC5 (key[3 -2 ], key[3 -3 ]); - toggle = key[3 -1 ]; + scancode =RC_SCANCODE_RC5 (st->buf[3 -2 ], st->buf[3 -3 ]); + toggle = st->buf[3 -1 ];break ; }
Signature · 1 line
2.7.4
PATCH v2 8/31 dib0700_core: don't use stack on I2C reads
In reply to Mauro Carvalho ChehabBe sure that I2C reads won't use stack by passing a pointer to the state buffer, that we know it was allocated via kmalloc, instead of relying on the buffer allocated by an I2C client.
--- drivers/media/usb/dvb-usb/dib0700_core.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
drivers/media/usb/dvb-usb/dib0700_core.c +26 -1
@@ -213,7 +213,7 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap, struct i2c_msg *msg,usb_rcvctrlpipe (d->udev,0 ),REQUEST_NEW_I2C_READ ,USB_TYPE_VENDOR |USB_DIR_IN , - value, index, msg[i].buf, + value, index, st->buf, msg[i].len, USB_CTRL_GET_TIMEOUT);if (result <0 ) {
@@ -221,6 +221,14 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap, struct i2c_msg *msg,break ; } +if (msg[i].len >sizeof (st->buf)) { +deb_info ("buffer too small to fit %d bytes\n" , + msg[i].len); +return -EIO ; + } + +memcpy (msg[i].buf, st->buf, msg[i].len); +deb_data ("<<< " );debug_dump (msg[i].buf, msg[i].len, deb_data);
@@ -238,6 +246,13 @@ static int dib0700_i2c_xfer_new(struct i2c_adapter *adap, struct i2c_msg *msg,/* I2C ctrl + FE bus; */ st->buf[3 ] = ((gen_mode <<6 ) &0xC0 ) | ((bus_mode <<4 ) &0x30 ); + +if (msg[i].len >sizeof (st->buf) -4 ) { +deb_info ("i2c message to big: %d\n" , + msg[i].len); +return -EIO ; + } +/* The Actual i2c payload */ memcpy (&st->buf[4 ], msg[i].buf, msg[i].len);
@@ -283,6 +298,11 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,/* fill in the address */ st->buf[1 ] = msg[i].addr <<1 ;/* fill the buffer */ +if (msg[i].len >sizeof (st->buf) -2 ) { +deb_info ("i2c xfer to big: %d\n" , + msg[i].len); +return -EIO ; + }memcpy (&st->buf[2 ], msg[i].buf, msg[i].len);/* write/read request */
@@ -299,6 +319,11 @@ static int dib0700_i2c_xfer_legacy(struct i2c_adapter *adap,break ; } +if (msg[i +1 ].len >sizeof (st->buf)) { +deb_info ("i2c xfer buffer to small for %d\n" , + msg[i].len); +return -EIO ; + }memcpy (msg[i +1 ].buf, st->buf, msg[i +1 ].len); msg[i+1 ].len = len;
Signature · 1 line
2.7.4
PATCH v2 9/31 dibusb: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/dibusb-common.c | 107 +++++++++++++++++++++++------- drivers/media/usb/dvb-usb/dibusb.h | 3 + 2 files changed, 85 insertions(+), 25 deletions(-)
drivers/media/usb/dvb-usb/dibusb-common.c +82 -25
@@ -63,72 +63,117 @@ 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 };int ret; +u8 *b; + + b =kmalloc (3 ,GFP_KERNEL ); +if (!b) +return -ENOMEM ;if ((ret =dibusb_streaming_ctrl (adap,onoff)) <0 ) -return ret; + goto ret;if (onoff) { b[0 ] =DIBUSB_REQ_SET_STREAMING_MODE ; b[1 ] =0x00 ; -if ((ret =dvb_usb_generic_write (adap->dev,b,2 )) <0 ) -return ret; + ret =dvb_usb_generic_write (adap->dev, b,2 ); +if (ret <0 ) + goto 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 ); + ret =dvb_usb_generic_write (adap->dev, b,3 ); + +ret: +kfree (b); +return ret; }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 +365,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);
drivers/media/usb/dvb-usb/dibusb.h +3 -0
@@ -96,6 +96,9 @@#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;
Signature · 1 line
2.7.4
PATCH v2 10/31 dibusb: handle error code on RC query
In reply to Mauro Carvalho ChehabThere's no sense on decoding and generating a RC key code if there was an error on the URB control message.
--- drivers/media/usb/dvb-usb/dibusb-common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/media/usb/dvb-usb/dibusb-common.c +6 -2
@@ -366,6 +366,7 @@ EXPORT_SYMBOL(rc_map_dibusb_table);int dibusb_rc_query (struct dvb_usb_device *d,u32 *event,int *state) {u8 *buf; +int ret; buf =kmalloc (5 ,GFP_KERNEL );if (!buf)
@@ -373,7 +374,9 @@ int dibusb_rc_query(struct dvb_usb_device *d, u32 *event, int *state) buf[0 ] =DIBUSB_REQ_POLL_REMOTE ; -dvb_usb_generic_rw (d, buf,1 , buf,5 ,0 ); + ret =dvb_usb_generic_rw (d, buf,1 , buf,5 ,0 ); +if (ret <0 ) + goto ret;dvb_usb_nec_rc_key_to_event (d, buf, event, state);
@@ -382,6 +385,7 @@ int dibusb_rc_query(struct dvb_usb_device *d, u32 *event, int *state)kfree (buf); -return 0 ; +ret: +return ret; }EXPORT_SYMBOL (dibusb_rc_query);
Signature · 1 line
2.7.4
PATCH v2 11/31 digitv: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/digitv.c | 20 +++++++++++--------- drivers/media/usb/dvb-usb/digitv.h | 3 +++ 2 files changed, 14 insertions(+), 9 deletions(-)
drivers/media/usb/dvb-usb/digitv.c +11 -9
@@ -28,20 +28,22 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);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 */ -u8 sndbuf[7 ],rcvbuf[7 ]; -memset (sndbuf,0 ,7 );memset (rcvbuf,0 ,7 ); - sndbuf[0 ] = cmd; - sndbuf[1 ] = vv; - sndbuf[2 ] = wo ? wlen : rlen; +memset (st->sndbuf,0 ,7 ); +memset (st->rcvbuf,0 ,7 ); + + st->sndbuf[0 ] = cmd; + st->sndbuf[1 ] = vv; + st->sndbuf[2 ] = wo ? wlen : rlen;if (wo) { -memcpy (&sndbuf[3 ],wbuf,wlen); -dvb_usb_generic_write (d,sndbuf,7 ); +memcpy (&st->sndbuf[3 ], wbuf, wlen); +dvb_usb_generic_write (d, st->sndbuf,7 ); }else { -dvb_usb_generic_rw (d,sndbuf,7 ,rcvbuf,7 ,10 ); -memcpy (rbuf,&rcvbuf[3 ],rlen); +dvb_usb_generic_rw (d, st->sndbuf,7 , st->rcvbuf,7 ,10 ); +memcpy (rbuf, &st->rcvbuf[3 ], rlen); }return 0 ; }
drivers/media/usb/dvb-usb/digitv.h +3 -0
@@ -6,6 +6,9 @@struct digitv_state {int is_nxt6000; + +unsigned char sndbuf[7 ]; +unsigned char rcvbuf[7 ]; };/* protocol (from usblogging and the SDK:
Signature · 1 line
2.7.4
PATCH v2 12/31 dtt200u-fe: don't keep waiting for lock at set_frontend()
In reply to Mauro Carvalho ChehabIt is up to the frontend kthread to wait for lock.
--- drivers/media/usb/dvb-usb/dtt200u-fe.c | 9 --------- 1 file changed, 9 deletions(-)
drivers/media/usb/dvb-usb/dtt200u-fe.c +0 -9
@@ -105,8 +105,6 @@ static int dtt200u_fe_set_frontend(struct dvb_frontend *fe) {struct dtv_frontend_properties *fep = &fe->dtv_property_cache;struct dtt200u_fe_state *state = fe->demodulator_priv; -int i; -enum fe_status st;u16 freq = fep->frequency /250000 ;u8 bwbuf[2 ] = {SET_BANDWIDTH ,0 },freqbuf[3 ] = {SET_RF_FREQ ,0 ,0 };
@@ -130,13 +128,6 @@ static int dtt200u_fe_set_frontend(struct dvb_frontend *fe) freqbuf[2 ] = (freq >>8 ) &0xff ;dvb_usb_generic_write (state->d,freqbuf,3 ); -for (i =0 ; i <30 ; i++) { -msleep (20 ); -dtt200u_fe_read_status (fe, &st); -if (st &FE_TIMEDOUT ) -continue ; - } -return 0 ; }
Signature · 1 line
2.7.4
PATCH v2 13/31 dtt200u-fe: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/dtt200u-fe.c | 95 +++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 30 deletions(-)
drivers/media/usb/dvb-usb/dtt200u-fe.c +65 -30
@@ -18,17 +18,22 @@ struct dtt200u_fe_state {struct dtv_frontend_properties fep;struct dvb_frontend frontend; + +unsigned char data[80 ]; +struct mutex data_mutex; };static int dtt200u_fe_read_status (struct dvb_frontend *fe,enum fe_status *stat) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 st =GET_TUNE_STATUS , b[3 ]; -dvb_usb_generic_rw (state->d,&st,1 ,b,3 ,0 ); +mutex_lock (&state->data_mutex); + state->data[0 ] =GET_TUNE_STATUS ; -switch (b[0 ]) { +dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); + +switch (state->data[0 ]) {case 0x01 : *stat =FE_HAS_SIGNAL |FE_HAS_CARRIER |FE_HAS_VITERBI |FE_HAS_SYNC |FE_HAS_LOCK ;
@@ -41,51 +46,75 @@ static int dtt200u_fe_read_status(struct dvb_frontend *fe, *stat =0 ;break ; } +mutex_unlock (&state->data_mutex);return 0 ; }static int dtt200u_fe_read_ber (struct dvb_frontend * fe,u32 *ber) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 bw =GET_VIT_ERR_CNT ,b[3 ]; -dvb_usb_generic_rw (state->d,&bw,1 ,b,3 ,0 ); - *ber = (b[0 ] <<16 ) | (b[1 ] <<8 ) | b[2 ]; + +mutex_lock (&state->data_mutex); + state->data[0 ] =GET_VIT_ERR_CNT ; + +dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); + *ber = (state->data[0 ] <<16 ) | (state->data[1 ] <<8 ) | state->data[2 ]; + +mutex_unlock (&state->data_mutex);return 0 ; }static int dtt200u_fe_read_unc_blocks (struct dvb_frontend * fe,u32 *unc) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 bw =GET_RS_UNCOR_BLK_CNT ,b[2 ]; -dvb_usb_generic_rw (state->d,&bw,1 ,b,2 ,0 ); - *unc = (b[0 ] <<8 ) | b[1 ]; -return 0 ; +mutex_lock (&state->data_mutex); + state->data[0 ] =GET_RS_UNCOR_BLK_CNT ; + +dvb_usb_generic_rw (state->d, state->data,1 , state->data,2 ,0 ); + +mutex_unlock (&state->data_mutex); +return ret; }static int dtt200u_fe_read_signal_strength (struct dvb_frontend * fe,u16 *strength) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 bw =GET_AGC , b; -dvb_usb_generic_rw (state->d,&bw,1 ,&b,1 ,0 ); - *strength = (b <<8 ) | b; -return 0 ; + +mutex_lock (&state->data_mutex); + state->data[0 ] =GET_AGC ; + +dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); + +mutex_unlock (&state->data_mutex); +return ret; }static int dtt200u_fe_read_snr (struct dvb_frontend * fe,u16 *snr) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 bw =GET_SNR ,br; -dvb_usb_generic_rw (state->d,&bw,1 ,&br,1 ,0 ); - *snr = ~((br <<8 ) | br); -return 0 ; + +mutex_lock (&state->data_mutex); + state->data[0 ] =GET_SNR ; + +dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); + +mutex_unlock (&state->data_mutex); +return ret; }static int dtt200u_fe_init (struct dvb_frontend * fe) {struct dtt200u_fe_state *state = fe->demodulator_priv; -u8 b =SET_INIT ; -return dvb_usb_generic_write (state->d,&b,1 ); +int ret; + +mutex_lock (&state->data_mutex); + state->data[0 ] =SET_INIT ; + + ret =dvb_usb_generic_write (state->d, state->data,1 ); +mutex_unlock (&state->data_mutex); + +return ret; }static int dtt200u_fe_sleep (struct dvb_frontend * fe)
@@ -106,29 +135,34 @@ static int dtt200u_fe_set_frontend(struct dvb_frontend *fe)struct dtv_frontend_properties *fep = &fe->dtv_property_cache;struct dtt200u_fe_state *state = fe->demodulator_priv;u16 freq = fep->frequency /250000 ; -u8 bwbuf[2 ] = {SET_BANDWIDTH ,0 },freqbuf[3 ] = {SET_RF_FREQ ,0 ,0 }; +mutex_lock (&state->data_mutex); + state->data[0 ] =SET_BANDWIDTH ;switch (fep->bandwidth_hz) {case 8000000 : - bwbuf[1 ] =8 ; + state->data[1 ] =8 ;break ;case 7000000 : - bwbuf[1 ] =7 ; + state->data[1 ] =7 ;break ;case 6000000 : - bwbuf[1 ] =6 ; + state->data[1 ] =6 ;break ;default : -return -EINVAL ; + ret = -EINVAL ; + goto ret; } -dvb_usb_generic_write (state->d,bwbuf,2 ); +dvb_usb_generic_write (state->d, state->data,2 ); - freqbuf[1 ] = freq &0xff ; - freqbuf[2 ] = (freq >>8 ) &0xff ; -dvb_usb_generic_write (state->d,freqbuf,3 ); + state->data[0 ] =SET_RF_FREQ ; + state->data[1 ] = freq &0xff ; + state->data[2 ] = (freq >>8 ) &0xff ; +dvb_usb_generic_write (state->d, state->data,3 ); -return 0 ; +ret: +mutex_unlock (&state->data_mutex); +return ret; } static int dtt200u_fe_get_frontend(struct dvb_frontend* fe,
@@ -160,6 +194,7 @@ struct dvb_frontend* dtt200u_fe_attach(struct dvb_usb_device *d)deb_info ("attaching frontend dtt200u\n" ); state->d = d; +mutex_init (&state->data_mutex);memcpy (&state->frontend.ops,&dtt200u_fe_ops,sizeof (struct dvb_frontend_ops )); state->frontend.demodulator_priv = state;
Signature · 1 line
2.7.4
PATCH v2 14/31 dtt200u-fe: handle errors on USB control messages
In reply to Mauro Carvalho ChehabIf something goes wrong, return an error code, instead of assuming that everything went fine.
--- drivers/media/usb/dvb-usb/dtt200u-fe.c | 40 ++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 9 deletions(-)
drivers/media/usb/dvb-usb/dtt200u-fe.c +31 -9
@@ -27,11 +27,17 @@ static int dtt200u_fe_read_status(struct dvb_frontend *fe, enum fe_status *stat) {struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;mutex_lock (&state->data_mutex); state->data[0 ] =GET_TUNE_STATUS ; -dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); + ret =dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); +if (ret <0 ) { + *stat =0 ; +mutex_unlock (&state->data_mutex); +return ret; + }switch (state->data[0 ]) {case 0x01 :
@@ -53,25 +59,30 @@ static int dtt200u_fe_read_status(struct dvb_frontend *fe,static int dtt200u_fe_read_ber (struct dvb_frontend * fe,u32 *ber) {struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;mutex_lock (&state->data_mutex); state->data[0 ] =GET_VIT_ERR_CNT ; -dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); - *ber = (state->data[0 ] <<16 ) | (state->data[1 ] <<8 ) | state->data[2 ]; + ret =dvb_usb_generic_rw (state->d, state->data,1 , state->data,3 ,0 ); +if (ret >=0 ) + *ber = (state->data[0 ] <<16 ) | (state->data[1 ] <<8 ) | state->data[2 ];mutex_unlock (&state->data_mutex); -return 0 ; +return ret; }static int dtt200u_fe_read_unc_blocks (struct dvb_frontend * fe,u32 *unc) {struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;mutex_lock (&state->data_mutex); state->data[0 ] =GET_RS_UNCOR_BLK_CNT ; -dvb_usb_generic_rw (state->d, state->data,1 , state->data,2 ,0 ); + ret =dvb_usb_generic_rw (state->d, state->data,1 , state->data,2 ,0 ); +if (ret >=0 ) + *unc = (state->data[0 ] <<8 ) | state->data[1 ];mutex_unlock (&state->data_mutex);return ret;
@@ -80,11 +91,14 @@ static int dtt200u_fe_read_unc_blocks(struct dvb_frontend* fe, u32 *unc)static int dtt200u_fe_read_signal_strength (struct dvb_frontend * fe,u16 *strength) {struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;mutex_lock (&state->data_mutex); state->data[0 ] =GET_AGC ; -dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); + ret =dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); +if (ret >=0 ) + *strength = (state->data[0 ] <<8 ) | state->data[0 ];mutex_unlock (&state->data_mutex);return ret;
@@ -93,11 +107,14 @@ static int dtt200u_fe_read_signal_strength(struct dvb_frontend* fe, u16 *strengtstatic int dtt200u_fe_read_snr (struct dvb_frontend * fe,u16 *snr) {struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;mutex_lock (&state->data_mutex); state->data[0 ] =GET_SNR ; -dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); + ret =dvb_usb_generic_rw (state->d, state->data,1 , state->data,1 ,0 ); +if (ret >=0 ) + *snr = ~((state->data[0 ] <<8 ) | state->data[0 ]);mutex_unlock (&state->data_mutex);return ret;
@@ -134,6 +151,7 @@ static int dtt200u_fe_set_frontend(struct dvb_frontend *fe) {struct dtv_frontend_properties *fep = &fe->dtv_property_cache;struct dtt200u_fe_state *state = fe->demodulator_priv; +int ret;u16 freq = fep->frequency /250000 ;mutex_lock (&state->data_mutex);
@@ -153,12 +171,16 @@ static int dtt200u_fe_set_frontend(struct dvb_frontend *fe) goto ret; } -dvb_usb_generic_write (state->d, state->data,2 ); + ret =dvb_usb_generic_write (state->d, state->data,2 ); +if (ret <0 ) + goto ret; state->data[0 ] =SET_RF_FREQ ; state->data[1 ] = freq &0xff ; state->data[2 ] = (freq >>8 ) &0xff ; -dvb_usb_generic_write (state->d, state->data,3 ); + ret =dvb_usb_generic_write (state->d, state->data,3 ); +if (ret <0 ) + goto ret; ret:mutex_unlock (&state->data_mutex);
Signature · 1 line
2.7.4
PATCH v2 15/31 dtt200u: don't do DMA on stack
In reply to Mauro Carvalho ChehabFrom: Mauro Carvalho Chehab <[email protected]>
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.
--- drivers/media/usb/dvb-usb/dtt200u.c | 104 ++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 29 deletions(-)
drivers/media/usb/dvb-usb/dtt200u.c +75 -29
@@ -20,74 +20,103 @@ MODULE_PARM_DESC(debug, "set debugging level (1=info,xfer=2 (or-able))." DVB_USBDVB_DEFINE_MOD_OPT_ADAPTER_NR (adapter_nr); +struct dtt200u_state { +unsigned char data[80 ]; +struct mutex data_mutex; +}; +static int dtt200u_power_ctrl (struct dvb_usb_device *d,int onoff) { -u8 b =SET_INIT ; +struct dtt200u_state *st = d->priv; + +mutex_lock (&st->data_mutex); + + st->data[0 ] =SET_INIT ;if (onoff) -dvb_usb_generic_write (d,&b,2 ); +dvb_usb_generic_write (d, st->data,2 ); +mutex_unlock (&st->data_mutex);return 0 ; }static int dtt200u_streaming_ctrl (struct dvb_usb_adapter *adap,int onoff) { -u8 b_streaming[2 ] = {SET_STREAMING , onoff }; -u8 b_rst_pid =RESET_PID_FILTER ; +struct dtt200u_state *st = adap->dev->priv; -dvb_usb_generic_write (adap->dev, b_streaming,2 ); +mutex_lock (&st->data_mutex); + st->data[0 ] =SET_STREAMING ; + st->data[1 ] = onoff; + +dvb_usb_generic_write (adap->dev, st->data,2 ); + +if (onoff) +return 0 ; + + st->data[0 ] =RESET_PID_FILTER ; +dvb_usb_generic_write (adap->dev, st->data,1 ); + +mutex_unlock (&st->data_mutex); -if (onoff ==0 ) -dvb_usb_generic_write (adap->dev, &b_rst_pid,1 );return 0 ; }static int dtt200u_pid_filter (struct dvb_usb_adapter *adap,int index,u16 pid,int onoff) { -u8 b_pid[4 ]; +struct dtt200u_state *st = adap->dev->priv; +int ret; + pid = onoff ? pid :0 ; - b_pid[0 ] =SET_PID_FILTER ; - b_pid[1 ] = index; - b_pid[2 ] = pid &0xff ; - b_pid[3 ] = (pid >>8 ) &0x1f ; +mutex_lock (&st->data_mutex); + st->data[0 ] =SET_PID_FILTER ; + st->data[1 ] = index; + st->data[2 ] = pid &0xff ; + st->data[3 ] = (pid >>8 ) &0x1f ; -return dvb_usb_generic_write (adap->dev, b_pid,4 ); + ret =dvb_usb_generic_write (adap->dev, st->data,4 ); +mutex_unlock (&st->data_mutex); + +return ret; }static int dtt200u_rc_query (struct dvb_usb_device *d) { -u8 key[5 ],cmd =GET_RC_CODE ; +struct dtt200u_state *st = d->priv;u32 scancode; -dvb_usb_generic_rw (d,&cmd,1 ,key,5 ,0 ); -if (key[0 ] ==1 ) { +mutex_lock (&st->data_mutex); + st->data[0 ] =GET_RC_CODE ; + +dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); +if (st->data[0 ] ==1 ) {enum rc_type proto =RC_TYPE_NEC ; - scancode = key[1 ]; -if ((u8 ) ~key[1 ] != key[2 ]) { + scancode = st->data[1 ]; +if ((u8 ) ~st->data[1 ] != st->data[2 ]) {/* Extended NEC */ scancode = scancode <<8 ; - scancode |= key[2 ]; + scancode |= st->data[2 ]; proto =RC_TYPE_NECX ; } scancode = scancode <<8 ; - scancode |= key[3 ]; + scancode |= st->data[3 ];/* Check command checksum is ok */ -if ((u8 ) ~key[3 ] == key[4 ]) +if ((u8 ) ~st->data[3 ] == st->data[4 ])rc_keydown (d->rc_dev, proto, scancode,0 );else rc_keyup (d->rc_dev); - }else if (key[0 ] ==2 ) { + }else if (st->data[0 ] ==2 ) {rc_repeat (d->rc_dev); }else {rc_keyup (d->rc_dev); } -if (key[0 ] !=0 ) -deb_info ("key: %*ph\n" ,5 , key); +if (st->data[0 ] !=0 ) +deb_info ("st->data: %*ph\n" ,5 , st->data); +mutex_unlock (&st->data_mutex);return 0 ; }
@@ -106,17 +135,24 @@ static struct dvb_usb_device_properties wt220u_miglia_properties;static int dtt200u_usb_probe (struct usb_interface *intf,const struct usb_device_id *id) { +struct dvb_usb_device *d; +struct dtt200u_state *st; +if (0 ==dvb_usb_device_init (intf, &dtt200u_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &wt220u_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &wt220u_fc_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &wt220u_zl0353_properties, -THIS_MODULE ,NULL , adapter_nr) || +THIS_MODULE , &d, adapter_nr) ||0 ==dvb_usb_device_init (intf, &wt220u_miglia_properties, -THIS_MODULE ,NULL , adapter_nr)) +THIS_MODULE , &d, adapter_nr)) { + st = d->priv; +mutex_init (&st->data_mutex); +return 0 ; + }return -ENODEV ; }
@@ -140,6 +176,8 @@ static struct dvb_usb_device_properties dtt200u_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-dtt200u-01.fw", + .size_of_priv = sizeof(struct dtt200u_state), + .num_adapters = 1, .adapter = { {
@@ -190,6 +228,8 @@ static struct dvb_usb_device_properties wt220u_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-wt220u-02.fw", + .size_of_priv = sizeof(struct dtt200u_state), + .num_adapters = 1, .adapter = { {
@@ -240,6 +280,8 @@ static struct dvb_usb_device_properties wt220u_fc_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-wt220u-fc03.fw", + .size_of_priv = sizeof(struct dtt200u_state), + .num_adapters = 1, .adapter = { {
@@ -290,6 +332,8 @@ static struct dvb_usb_device_properties wt220u_zl0353_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-wt220u-zl0353-01.fw", + .size_of_priv = sizeof(struct dtt200u_state), + .num_adapters = 1, .adapter = { {
@@ -340,6 +384,8 @@ static struct dvb_usb_device_properties wt220u_miglia_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-wt220u-miglia-01.fw", + .size_of_priv = sizeof(struct dtt200u_state), + .num_adapters = 1, .generic_bulk_ctrl_endpoint = 0x01,
Signature · 1 line
2.7.4
PATCH v2 16/31 dtt200u: handle USB control message errors
In reply to Mauro Carvalho ChehabIf something bad happens while an USB control message is transfered, return an error code.
--- drivers/media/usb/dvb-usb/dtt200u.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
drivers/media/usb/dvb-usb/dtt200u.c +18 -8
@@ -28,37 +28,42 @@ struct dtt200u_state {static int dtt200u_power_ctrl (struct dvb_usb_device *d,int onoff) {struct dtt200u_state *st = d->priv; +int ret =0 ;mutex_lock (&st->data_mutex); st->data[0 ] =SET_INIT ;if (onoff) -dvb_usb_generic_write (d, st->data,2 ); + ret =dvb_usb_generic_write (d, st->data,2 );mutex_unlock (&st->data_mutex); -return 0 ; +return ret; }static int dtt200u_streaming_ctrl (struct dvb_usb_adapter *adap,int onoff) {struct dtt200u_state *st = adap->dev->priv; +int ret;mutex_lock (&st->data_mutex); st->data[0 ] =SET_STREAMING ; st->data[1 ] = onoff; -dvb_usb_generic_write (adap->dev, st->data,2 ); + ret =dvb_usb_generic_write (adap->dev, st->data,2 ); +if (ret <0 ) + goto ret;if (onoff) -return 0 ; + goto ret; st->data[0 ] =RESET_PID_FILTER ; -dvb_usb_generic_write (adap->dev, st->data,1 ); + ret =dvb_usb_generic_write (adap->dev, st->data,1 ); +ret:mutex_unlock (&st->data_mutex); -return 0 ; +return ret; }static int dtt200u_pid_filter (struct dvb_usb_adapter *adap,int index,u16 pid,int onoff)
@@ -84,11 +89,15 @@ static int dtt200u_rc_query(struct dvb_usb_device *d) {struct dtt200u_state *st = d->priv;u32 scancode; +int ret;mutex_lock (&st->data_mutex); st->data[0 ] =GET_RC_CODE ; -dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); + ret =dvb_usb_generic_rw (d, st->data,1 , st->data,5 ,0 ); +if (ret <0 ) + goto ret; +if (st->data[0 ] ==1 ) {enum rc_type proto =RC_TYPE_NEC ;
@@ -116,8 +125,9 @@ static int dtt200u_rc_query(struct dvb_usb_device *d)if (st->data[0 ] !=0 )deb_info ("st->data: %*ph\n" ,5 , st->data); +ret:mutex_unlock (&st->data_mutex); -return 0 ; +return ret; }static int dtt200u_frontend_attach (struct dvb_usb_adapter *adap)
Signature · 1 line
2.7.4
PATCH v2 17/31 dtv5100: don't do DMA on stack
In reply to Mauro Carvalho ChehabFrom: Mauro Carvalho Chehab <[email protected]>
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.
--- drivers/media/usb/dvb-usb/dtv5100.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/media/usb/dvb-usb/dtv5100.c +8 -2
@@ -31,9 +31,14 @@ module_param_named(debug, dvb_usb_dtv5100_debug, int, 0644);MODULE_PARM_DESC (debug,"set debugging level" DVB_USB_DEBUG_STATUS );DVB_DEFINE_MOD_OPT_ADAPTER_NR (adapter_nr); +struct dtv5100_state { +unsigned char data[80 ]; +}; +static int dtv5100_i2c_msg (struct dvb_usb_device *d,u8 addr,u8 *wbuf,u16 wlen,u8 *rbuf,u16 rlen) { +struct dtv5100_state *st = d->priv;u8 request;u8 type;u16 value;
@@ -60,9 +65,10 @@ static int dtv5100_i2c_msg(struct dvb_usb_device *d, u8 addr, } index = (addr <<8 ) + wbuf[0 ]; +memcpy (st->data, rbuf, rlen);msleep (1 );/* avoid I2C errors */ return usb_control_msg (d->udev,usb_rcvctrlpipe (d->udev,0 ), request, - type, value, index, rbuf, rlen, + type, value, index, st->data, rlen,DTV5100_USB_TIMEOUT ); }
@@ -176,7 +182,7 @@ static struct dvb_usb_device_properties dtv5100_properties = { .caps = DVB_USB_IS_AN_I2C_ADAPTER, .usb_ctrl = DEVICE_SPECIFIC, - .size_of_priv = 0, + .size_of_priv = sizeof(struct dtv5100_state), .num_adapters = 1, .adapter = {{
Signature · 1 line
2.7.4
PATCH v2 18/31 gp8psk: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/gp8psk.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/media/usb/dvb-usb/gp8psk.c +12 -2
@@ -24,6 +24,10 @@ MODULE_PARM_DESC(debug, "set debugging level (1=info,xfer=2,rc=4 (or-able))." DVDVB_DEFINE_MOD_OPT_ADAPTER_NR (adapter_nr); +struct gp8psk_state { +unsigned char data[80 ]; +}; +static int gp8psk_get_fw_version (struct dvb_usb_device *d,u8 *fw_vers) {return (gp8psk_usb_in_op (d,GET_FW_VERS ,0 ,0 , fw_vers,6 ));
@@ -53,17 +57,19 @@ static void gp8psk_info(struct dvb_usb_device *d)int gp8psk_usb_in_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret =0 ,try =0 ;if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;while (ret >=0 && ret != blen && try <3 ) { +memcpy (st->data, b, blen); ret =usb_control_msg (d->udev,usb_rcvctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_IN , - value,index,b,blen, + value, index, st->data, blen,2000 );deb_info ("reading number %d (ret: %d)\n" ,try,ret); try++;
@@ -86,6 +92,7 @@ int gp8psk_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index, u8int gp8psk_usb_out_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret;deb_xfer ("out: req. %x, val: %x, ind: %x, buffer: " ,req,value,index);
@@ -94,11 +101,12 @@ int gp8psk_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret; +memcpy (st->data, b, blen);if (usb_control_msg (d->udev,usb_sndctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_OUT , - value,index,b,blen, + value,index, st->data, blen,2000 ) != blen) {warn ("usb out operation failed." ); ret = -EIO ;
@@ -265,6 +273,8 @@ static struct dvb_usb_device_properties gp8psk_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-gp8psk-01.fw", + .size_of_priv = sizeof(struct gp8psk_state), + .num_adapters = 1, .adapter = { {
Signature · 1 line
2.7.4
Re: [PATCH v2 18/31] gp8psk: don't do DMA on stack
I applied this patch to the 4.8.4 kernel driver (that I'm currently running) and it caused nothing but "frontend 0/0 timed out while tuning". Is there another patch that should be used in conjunction with this? If not, this patch breaks the gp8psk driver.
Thanks.
Quoted 84 lines · Mauro Carvalho Chehab
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/gp8psk.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/media/usb/dvb-usb/gp8psk.c +12
-3@@ -24,6 +24,10 @@ MODULE_PARM_DESC(debug, "set debugging level (1=info,xfer=2,rc=4 (or-able))." DVDVB_DEFINE_MOD_OPT_ADAPTER_NR (adapter_nr); +struct gp8psk_state { +unsigned char data[80 ]; +}; +static int gp8psk_get_fw_version (struct dvb_usb_device *d,u8 *fw_vers) {return (gp8psk_usb_in_op (d,GET_FW_VERS ,0 ,0 , fw_vers,6 ));@@ -53,17 +57,19 @@ static void gp8psk_info(struct dvb_usb_device *d)int gp8psk_usb_in_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret =0 ,try =0 ;if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;while (ret >=0 && ret != blen && try <3 ) { +memcpy (st->data, b, blen); ret =usb_control_msg (d->udev,usb_rcvctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_IN , - value,index,b,blen, + value, index, st->data, blen,2000 );deb_info ("reading number %d (ret: %d)\n" ,try,ret); try++;@@ -86,6 +92,7 @@ int gp8psk_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index, u8int gp8psk_usb_out_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret;deb_xfer ("out: req. %x, val: %x, ind: %x, buffer: " ,req,value,index);@@ -94,11 +101,12 @@ int gp8psk_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret; +memcpy (st->data, b, blen);if (usb_control_msg (d->udev,usb_sndctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_OUT , - value,index,b,blen, + value,index, st->data, blen,2000 ) != blen) {warn ("usb out operation failed." ); ret = -EIO ;@@ -265,6 +273,8 @@ static struct dvb_usb_device_properties gp8psk_properties = { .usb_ctrl = CYPRESS_FX2, .firmware = "dvb-usb-gp8psk-01.fw", + .size_of_priv = sizeof(struct gp8psk_state), + .num_adapters = 1, .adapter = { { --2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 18/31] gp8psk: don't do DMA on stack
Quoted 6 lines · VDR User
I applied this patch to the 4.8.4 kernel driver (that I'm currently running) and it caused nothing but "frontend 0/0 timed out while tuning". Is there another patch that should be used in conjunction with this? If not, this patch breaks the gp8psk driver.
Thanks.
Thanks for testing. "If it's not tested it's broken"...
Quoted 15 lines
Quoted 12 lines · Mauro Carvalho Chehab
index
5d0384dd45b5..fa215ad37f7b100644drivers/media/usb/dvb-usb/gp8psk.c +1
-0int gp8psk_usb_in_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret =0 ,try =0 ;if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;
Quoted 9 lines
drivers/media/usb/dvb-usb/gp8psk.c static void gp8psk_info(struct dvb_usb_device *d) · line 73
Quoted 9 lines
while (ret >= 0 && ret != blen && try < 3) {drivers/media/usb/dvb-usb/gp8psk.c +2
-1+memcpy (st->data, b, blen); ret =usb_control_msg (d->udev,usb_rcvctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_IN , - value,index,b,blen, + value, index, st->data, blen,2000 );
I guess for usb_in the memcpy should be after the usb_control_msg and from st->data to b.
Johannes
Re: [PATCH v2 18/31] gp8psk: don't do DMA on stack
Em Mon, 7 Nov 2016 12:29:47 +0100
Johannes Stezenbach <[email protected]> escreveu:
Quoted 35 lines · VDR User
Quoted 6 lines
I applied this patch to the 4.8.4 kernel driver (that I'm currently running) and it caused nothing but "frontend 0/0 timed out while tuning". Is there another patch that should be used in conjunction with this? If not, this patch breaks the gp8psk driver.
Thanks.
Thanks for testing. "If it's not tested it's broken"...
Quoted 15 lines
Quoted 12 lines · Mauro Carvalho Chehab
index
5d0384dd45b5..fa215ad37f7b100644drivers/media/usb/dvb-usb/gp8psk.c +1
-0int gp8psk_usb_in_op (struct dvb_usb_device *d,u8 req,u16 value,u16 index,u8 *b,int blen) { +struct gp8psk_state *st = d->priv;int ret =0 ,try =0 ;if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;Quoted 9 lines
drivers/media/usb/dvb-usb/gp8psk.c static void gp8psk_info(struct dvb_usb_device *d) · line 73
Quoted 9 lines
while (ret >= 0 && ret != blen && try < 3) {drivers/media/usb/dvb-usb/gp8psk.c +2
-1+memcpy (st->data, b, blen); ret =usb_control_msg (d->udev,usb_rcvctrlpipe (d->udev,0 ), req,USB_TYPE_VENDOR |USB_DIR_IN , - value,index,b,blen, + value, index, st->data, blen,2000 );
Quoted 2 lines
I guess for
usb_inthe memcpy should be after theusb_control_msgand fromst->datato b.
Yes, this should fix the issue. Just sent a patch with Johannes suggestion. Please test.
Regards,
Mauro
PATCH v2 19/31 gp8psk: don't go past the buffer size
In reply to Mauro Carvalho ChehabAdd checks to avoid going out of the buffer.
--- drivers/media/usb/dvb-usb/gp8psk.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/media/usb/dvb-usb/gp8psk.c +11 -0
@@ -60,6 +60,9 @@ int gp8psk_usb_in_op(struct dvb_usb_device *d, u8 req, u16 value, u16 index, u8struct gp8psk_state *st = d->priv;int ret =0 ,try =0 ; +if (blen >sizeof (st->data)) +return -EIO ; +if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;
@@ -98,6 +101,9 @@ int gp8psk_usb_out_op(struct dvb_usb_device *d, u8 req, u16 value,deb_xfer ("out: req. %x, val: %x, ind: %x, buffer: " ,req,value,index);debug_dump (b,blen,deb_xfer); +if (blen >sizeof (st->data)) +return -EIO ; +if ((ret =mutex_lock_interruptible (&d->usb_mutex)))return ret;
@@ -151,6 +157,11 @@ static int gp8psk_load_bcm4500fw(struct dvb_usb_device *d)err ("failed to load bcm4500 firmware." ); goto out_free; } +if (buflen >64 ) { +err ("firmare chunk size bigger than 64 bytes." ); + goto out_free; + } +memcpy (buf, ptr, buflen);if (dvb_usb_generic_write (d, buf, buflen)) {err ("failed to load bcm4500 firmware." );
Signature · 1 line
2.7.4
PATCH v2 20/31 nova-t-usb2: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/nova-t-usb2.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
drivers/media/usb/dvb-usb/nova-t-usb2.c +13 -5
@@ -74,22 +74,29 @@ static struct rc_map_table rc_map_haupp_table[] = {*/ static int nova_t_rc_query (struct dvb_usb_device *d,u32 *event,int *state) { -u8 key[5 ],cmd[2 ] = {DIBUSB_REQ_POLL_REMOTE ,0x35 }, data,toggle,custom; +u8 *buf, data, toggle, custom;u16 raw;int i;struct dibusb_device_state *st = d->priv; -dvb_usb_generic_rw (d,cmd,2 ,key,5 ,0 ); + buf =kmalloc (5 ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + buf[0 ] =DIBUSB_REQ_POLL_REMOTE ; + buf[1 ] =0x35 ; +dvb_usb_generic_rw (d, buf,2 , buf,5 ,0 ); *state =REMOTE_NO_KEY_PRESSED ; -switch (key[0 ]) { +switch (buf[0 ]) {case DIBUSB_RC_HAUPPAUGE_KEY_PRESSED : - raw = ((key[1 ] <<8 ) | key[2 ]) >>3 ; + raw = ((buf[1 ] <<8 ) | buf[2 ]) >>3 ; toggle = !!(raw &0x800 ); data = raw &0x3f ; custom = (raw >>6 ) &0x1f ; -deb_rc ("raw key code 0x%02x, 0x%02x, 0x%02x to c: %02x d: %02x toggle: %d\n" ,key[1 ],key[2 ],key[3 ],custom,data,toggle); +deb_rc ("raw key code 0x%02x, 0x%02x, 0x%02x to c: %02x d: %02x toggle: %d\n" , + buf[1 ], buf[2 ], buf[3 ], custom, data, toggle);for (i =0 ; i <ARRAY_SIZE (rc_map_haupp_table); i++) { if (rc5_data(&rc_map_haupp_table[i]) == data &&
@@ -117,6 +124,7 @@ static int nova_t_rc_query(struct dvb_usb_device *d, u32 *event, int *state)break ; } +kfree (buf);return 0 ; }
Signature · 1 line
2.7.4
PATCH v2 21/31 pctv452e: don't do DMA on stack
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/pctv452e.c | 129 ++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 55 deletions(-)
drivers/media/usb/dvb-usb/pctv452e.c +74 -55
@@ -97,13 +97,14 @@ struct pctv452e_state {u8 c;/* transaction counter, wraps around... */ u8 initialized;/* set to 1 if 0x15 has been sent */ u16 last_rc_key; + +unsigned char data[80 ]; };static int tt3650_ci_msg (struct dvb_usb_device *d,u8 cmd,u8 *data,unsigned int write_len,unsigned int read_len) {struct pctv452e_state *state = (struct pctv452e_state *)d->priv; -u8 buf[64 ];u8 id;unsigned int rlen;int ret;
@@ -112,33 +113,36 @@ static int tt3650_ci_msg(struct dvb_usb_device *d, u8 cmd, u8 *data,BUG_ON (write_len >64 -4 );BUG_ON (read_len >64 -4 ); +mutex_lock (&state->ca_mutex); id = state->c++; - buf[0 ] =SYNC_BYTE_OUT ; - buf[1 ] = id; - buf[2 ] = cmd; - buf[3 ] = write_len; + state->data[0 ] =SYNC_BYTE_OUT ; + state->data[1 ] = id; + state->data[2 ] = cmd; + state->data[3 ] = write_len; -memcpy (buf +4 , data, write_len); +memcpy (state->data +4 , data, write_len); rlen = (read_len >0 ) ?64 :0 ; - ret =dvb_usb_generic_rw (d, buf,4 + write_len, - buf, rlen,/* delay_ms */ 0 ); + ret =dvb_usb_generic_rw (d, state->data,4 + write_len, + state->data, rlen,/* delay_ms */ 0 );if (0 != ret) goto failed; ret = -EIO ; -if (SYNC_BYTE_IN != buf[0 ] || id != buf[1 ]) +if (SYNC_BYTE_IN != state->data[0 ] || id != state->data[1 ]) goto failed; -memcpy (data, buf +4 , read_len); +memcpy (data, state->data +4 , read_len); +mutex_unlock (&state->ca_mutex);return 0 ; failed:err ("CI error %d; %02X %02X %02X -> %*ph." , - ret,SYNC_BYTE_OUT , id, cmd,3 , buf); + ret,SYNC_BYTE_OUT , id, cmd,3 , state->data); +mutex_unlock (&state->ca_mutex);return ret; }
@@ -405,52 +409,53 @@ static int pctv452e_i2c_msg(struct dvb_usb_device *d, u8 addr, u8 *rcv_buf, u8 rcv_len) {struct pctv452e_state *state = (struct pctv452e_state *)d->priv; -u8 buf[64 ];u8 id;int ret; +mutex_lock (&state->ca_mutex); id = state->c++; ret = -EINVAL ;if (snd_len >64 -7 || rcv_len >64 -7 ) goto failed; - buf[0 ] =SYNC_BYTE_OUT ; - buf[1 ] = id; - buf[2 ] =PCTV_CMD_I2C ; - buf[3 ] = snd_len +3 ; - buf[4 ] = addr <<1 ; - buf[5 ] = snd_len; - buf[6 ] = rcv_len; + state->data[0 ] =SYNC_BYTE_OUT ; + state->data[1 ] = id; + state->data[2 ] =PCTV_CMD_I2C ; + state->data[3 ] = snd_len +3 ; + state->data[4 ] = addr <<1 ; + state->data[5 ] = snd_len; + state->data[6 ] = rcv_len; -memcpy (buf +7 , snd_buf, snd_len); +memcpy (state->data +7 , snd_buf, snd_len); - ret =dvb_usb_generic_rw (d, buf,7 + snd_len, - buf,/* rcv_len */ 64 , + ret =dvb_usb_generic_rw (d, state->data,7 + snd_len, + state->data,/* rcv_len */ 64 ,/* delay_ms */ 0 );if (ret <0 ) goto failed;/* TT USB protocol error. */ ret = -EIO ; -if (SYNC_BYTE_IN != buf[0 ] || id != buf[1 ]) +if (SYNC_BYTE_IN != state->data[0 ] || id != state->data[1 ]) goto failed;/* I2C device didn't respond as expected. */ ret = -EREMOTEIO ; -if (buf[5 ] < snd_len || buf[6 ] < rcv_len) +if (state->data[5 ] < snd_len || state->data[6 ] < rcv_len) goto failed; -memcpy (rcv_buf, buf +7 , rcv_len); +memcpy (rcv_buf, state->data +7 , rcv_len); +mutex_unlock (&state->ca_mutex);return rcv_len; failed: -err ("I2C error %d; %02X %02X %02X %02X %02X -> " -"%02X %02X %02X %02X %02X." , +err ("I2C error %d; %02X %02X %02X %02X %02X -> %*ph" , ret,SYNC_BYTE_OUT , id, addr <<1 , snd_len, rcv_len, - buf[0 ], buf[1 ], buf[4 ], buf[5 ], buf[6 ]); +7 , state->data); +mutex_unlock (&state->ca_mutex);return ret; }
@@ -499,8 +504,7 @@ static u32 pctv452e_i2c_func(struct i2c_adapter *adapter)static int pctv452e_power_ctrl (struct dvb_usb_device *d,int i) {struct pctv452e_state *state = (struct pctv452e_state *)d->priv; -u8 b0[] = {0xaa ,0 ,PCTV_CMD_RESET ,1 ,0 }; -u8 rx[PCTV_ANSWER_LEN ]; +u8 *rx;int ret;info ("%s: %d\n" , __func__, i);
@@ -511,6 +515,11 @@ static int pctv452e_power_ctrl(struct dvb_usb_device *d, int i)if (state->initialized)return 0 ; + rx =kmalloc (PCTV_ANSWER_LEN ,GFP_KERNEL ); +if (!rx) +return -ENOMEM ; + +mutex_lock (&state->ca_mutex);/* hmm where shoud this should go? */ ret =usb_set_interface (d->udev,0 ,ISOC_INTERFACE_ALTERNATIVE );if (ret !=0 )
@@ -518,65 +527,75 @@ static int pctv452e_power_ctrl(struct dvb_usb_device *d, int i) __func__, ret);/* this is a one-time initialization, dont know where to put */ - b0[1 ] = state->c++; + state->data[0 ] =0xaa ; + state->data[1 ] = state->c++; + state->data[2 ] =PCTV_CMD_RESET ; + state->data[3 ] =1 ; + state->data[4 ] =0 ;/* reset board */ - ret =dvb_usb_generic_rw (d, b0,sizeof (b0), rx,PCTV_ANSWER_LEN ,0 ); + ret =dvb_usb_generic_rw (d, state->data,5 , rx,PCTV_ANSWER_LEN ,0 );if (ret) -return ret; + goto ret; - b0[1 ] = state->c++; - b0[4 ] =1 ; + state->data[1 ] = state->c++; + state->data[4 ] =1 ;/* reset board (again?) */ - ret =dvb_usb_generic_rw (d, b0,sizeof (b0), rx,PCTV_ANSWER_LEN ,0 ); + ret =dvb_usb_generic_rw (d, state->data,5 , rx,PCTV_ANSWER_LEN ,0 );if (ret) -return ret; + goto ret; state->initialized =1 ; -return 0 ; +ret: +mutex_unlock (&state->ca_mutex); +kfree (rx); +return ret; }static int pctv452e_rc_query (struct dvb_usb_device *d) {struct pctv452e_state *state = (struct pctv452e_state *)d->priv; -u8 b[CMD_BUFFER_SIZE ]; -u8 rx[PCTV_ANSWER_LEN ];int ret, i; -u8 id = state->c++; +u8 id; + +mutex_lock (&state->ca_mutex); + id = state->c++;/* prepare command header */ - b[0 ] =SYNC_BYTE_OUT ; - b[1 ] = id; - b[2 ] =PCTV_CMD_IR ; - b[3 ] =0 ; + state->data[0 ] =SYNC_BYTE_OUT ; + state->data[1 ] = id; + state->data[2 ] =PCTV_CMD_IR ; + state->data[3 ] =0 ;/* send ir request */ - ret =dvb_usb_generic_rw (d, b,4 , rx,PCTV_ANSWER_LEN ,0 ); + ret =dvb_usb_generic_rw (d, state->data,4 , + state->data,PCTV_ANSWER_LEN ,0 );if (ret !=0 ) -return ret; + goto ret;if (debug >3 ) { -info ("%s: read: %2d: %*ph: " , __func__, ret,3 , rx); -for (i =0 ; (i < rx[3 ]) && ((i+3 ) <PCTV_ANSWER_LEN ); i++) -info (" %02x" , rx[i+3 ]); +info ("%s: read: %2d: %*ph: " , __func__, ret,3 , state->data); +for (i =0 ; (i < state->data[3 ]) && ((i +3 ) <PCTV_ANSWER_LEN ); i++) +info (" %02x" , state->data[i +3 ]);info ("\n" ); } -if ((rx[3 ] ==9 ) && (rx[12 ] &0x01 )) { +if ((state->data[3 ] ==9 ) && (state->data[12 ] &0x01 )) {/* got a "press" event */ - state->last_rc_key =RC_SCANCODE_RC5 (rx[7 ], rx[6 ]); + state->last_rc_key =RC_SCANCODE_RC5 (state->data[7 ], state->data[6 ]);if (debug >2 )info ("%s: cmd=0x%02x sys=0x%02x\n" , - __func__, rx[6 ], rx[7 ]); + __func__, state->data[6 ], state->data[7 ]);rc_keydown (d->rc_dev,RC_TYPE_RC5 , state->last_rc_key,0 ); }else if (state->last_rc_key) {rc_keyup (d->rc_dev); state->last_rc_key =0 ; } - -return 0 ; +ret: +mutex_unlock (&state->ca_mutex); +return ret; }static int pctv452e_read_mac_address (struct dvb_usb_device *d,u8 mac[6 ])
Signature · 1 line
2.7.4
PATCH v2 22/31 pctv452e: don't call BUG_ON() on non-fatal error
In reply to Mauro Carvalho ChehabThere are some conditions on this driver that are tested with BUG_ON() with are not serious enough to hang a machine.
So, just return an error if this happens.
--- drivers/media/usb/dvb-usb/pctv452e.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/media/usb/dvb-usb/pctv452e.c +4 -3
@@ -109,9 +109,10 @@ static int tt3650_ci_msg(struct dvb_usb_device *d, u8 cmd, u8 *data,unsigned int rlen;int ret; -BUG_ON (NULL == data &&0 != (write_len | read_len)); -BUG_ON (write_len >64 -4 ); -BUG_ON (read_len >64 -4 ); +if (!data || (write_len >64 -4 ) || (read_len >64 -4 )) { +err ("%s: transfer data invalid" , __func__); +return -EIO ; + };mutex_lock (&state->ca_mutex); id = state->c++;
Signature · 1 line
2.7.4
PATCH v2 23/31 technisat-usb2: use DMA buffers for I2C transfers
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/usb/dvb-usb/technisat-usb2.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/media/usb/dvb-usb/technisat-usb2.c +11 -5
@@ -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,
Signature · 1 line
2.7.4
PATCH v2 24/31 dvb-usb: warn if return value for USB read/write routines is not checked
In reply to Mauro Carvalho Chehabthe return values for dvb_usb_generic_rw() and dvb_usb_generic_write() should be checked, as otherwise the drivers won't be doing the right thing in the case of errors.
So, add __must_check to both declarations.
--- drivers/media/usb/dvb-usb/dvb-usb.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/media/usb/dvb-usb/dvb-usb.h +4 -2
@@ -462,8 +462,10 @@ extern int dvb_usb_device_init(struct usb_interface *,extern void dvb_usb_device_exit (struct usb_interface *);/* the generic read/write method for device control */ -extern int dvb_usb_generic_rw (struct dvb_usb_device *,u8 *,u16 ,u8 *,u16 ,int ); -extern int dvb_usb_generic_write (struct dvb_usb_device *,u8 *,u16 ); +extern int __must_check +dvb_usb_generic_rw (struct dvb_usb_device *,u8 *,u16 ,u8 *,u16 ,int ); +extern int __must_check +dvb_usb_generic_write (struct dvb_usb_device *,u8 *,u16 );/* commonly used remote control parsing */ extern int dvb_usb_nec_rc_key_to_event (struct dvb_usb_device *,u8 [],u32 *,int *);
Signature · 1 line
2.7.4
PATCH v2 25/31 nova-t-usb2: handle error code on RC query
In reply to Mauro Carvalho ChehabThere's no sense on decoding and generating a RC key code if there was an error on the URB control message.
--- drivers/media/usb/dvb-usb/nova-t-usb2.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/media/usb/dvb-usb/nova-t-usb2.c +6 -3
@@ -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 ])
Signature · 1 line
2.7.4
PATCH v2 26/31 dw2102: return error if su3000_power_ctrl() fails
In reply to Mauro Carvalho ChehabInstead of silently ignoring the error, return it.
--- drivers/media/usb/dvb-usb/dw2102.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/usb/dvb-usb/dw2102.c +1 -1
@@ -852,7 +852,7 @@ static int su3000_power_ctrl(struct dvb_usb_device *d, int i)if (i && !state->initialized) { state->initialized =1 ;/* reset board */ -dvb_usb_generic_rw (d, obuf,2 ,NULL ,0 ,0 ); +return dvb_usb_generic_rw (d, obuf,2 ,NULL ,0 ,0 ); }return 0 ;
Signature · 1 line
2.7.4
PATCH v2 27/31 digitv: handle error code on RC query
In reply to Mauro Carvalho ChehabThere's no sense on decoding and generating a RC key code if there was an error on the URB control message.
--- drivers/media/usb/dvb-usb/digitv.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/media/usb/dvb-usb/digitv.c +6 -4
@@ -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 */
Signature · 1 line
2.7.4
PATCH v2 28/31 cpia2_usb: don't use stack for DMA
In reply to Mauro Carvalho ChehabFrom: Mauro Carvalho Chehab <[email protected]>
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.
--- drivers/media/usb/cpia2/cpia2_usb.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)
drivers/media/usb/cpia2/cpia2_usb.c +29 -3
@@ -545,10 +545,19 @@ static void free_sbufs(struct camera_data *cam)static int write_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; - return usb_control_msg(udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + +memcpy (buf, registers, size); + + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
@@ -557,6 +566,9 @@ static int write_packet(struct usb_device *udev, registers,/* buffer */ size, HZ); + +kfree (buf); +return ret; }/****************************************************************************
@@ -567,18 +579,32 @@ static int write_packet(struct usb_device *udev,static int read_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; -return usb_control_msg (udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret =usb_control_msg (udev,usb_rcvctrlpipe (udev,0 ), request,USB_DIR_IN |USB_TYPE_VENDOR |USB_RECIP_DEVICE , start,/* value */ 0 ,/* index */ - registers,/* buffer */ + buf,/* buffer */ size,HZ ); + +if (ret >=0 ) +memcpy (registers, buf, size); + +kfree (buf); + +return ret; }/******************************************************************************
Signature · 1 line
2.7.4
Re: [PATCH v2 28/31] cpia2_usb: don't use stack for DMA
Hi,
Quoted 36 lines
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]>Signed-off-by Mauro Carvalho Chehab <[email protected]>--- drivers/media/usb/cpia2/cpia2_usb.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)drivers/media/usb/cpia2/cpia2_usb.c +10
-1@@ -545,10 +545,19 @@ static void free_sbufs(struct camera_data *cam)static int write_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; - return usb_control_msg(udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + +memcpy (buf, registers, size); + + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE,
drivers/media/usb/cpia2/cpia2_usb.c static int write_packet(struct usb_device *udev, · line 566
Quoted 1 line
drivers/media/usb/cpia2/cpia2_usb.c +0
-0@@ -557,6 +566,9 @@ static int write_packet(struct usb_device *udev, registers, /* buffer */
=========
I think you also want to change the argument to usb_control_msg() from "registers" to "buf" in write_packet().
drivers/media/usb/cpia2/cpia2_usb.c static int write_packet(struct usb_device *udev, · line 572
Quoted 6 lines
drivers/media/usb/cpia2/cpia2_usb.c +3
-0size, HZ); + +kfree (buf); +return ret; }
..
---
Kosuke TATSUKAWA | 1st Platform Software Division
| NEC Solution Innovators
| [email protected]PATCH v2 28/31 cpia2_usb: don't use stack for DMA
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.
Signature · 2 lines
v2.1: As pointed by Kosuke Tatsukawa, I forgot to replace "registers" var to "buf" at one of the usb_control_msg() calls.
drivers/media/usb/cpia2/cpia2_usb.c +30 -4
@@ -545,18 +545,30 @@ static void free_sbufs(struct camera_data *cam)static int write_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; -return usb_control_msg (udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + +memcpy (buf, registers, size); + + ret =usb_control_msg (udev,usb_sndctrlpipe (udev,0 ), request,USB_TYPE_VENDOR |USB_RECIP_DEVICE , start,/* value */ 0 ,/* index */ - registers,/* buffer */ + buf,/* buffer */ size,HZ ); + +kfree (buf); +return ret; }/****************************************************************************
@@ -567,18 +579,32 @@ static int write_packet(struct usb_device *udev,static int read_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; -return usb_control_msg (udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret =usb_control_msg (udev,usb_rcvctrlpipe (udev,0 ), request,USB_DIR_IN |USB_TYPE_VENDOR |USB_RECIP_DEVICE , start,/* value */ 0 ,/* index */ - registers,/* buffer */ + buf,/* buffer */ size,HZ ); + +if (ret >=0 ) +memcpy (registers, buf, size); + +kfree (buf); + +return ret; }/******************************************************************************
Re: [PATCH v2 28/31] cpia2_usb: don't use stack for DMA
In reply to Kosuke TatsukawaEm Tue, 11 Oct 2016 22:56:06 +0000
Kosuke Tatsukawa <[email protected]> escreveu:
Quoted 41 lines
Hi,
Quoted 36 lines
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]>Signed-off-by Mauro Carvalho Chehab <[email protected]>--- drivers/media/usb/cpia2/cpia2_usb.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)drivers/media/usb/cpia2/cpia2_usb.c +10
-1@@ -545,10 +545,19 @@ static void free_sbufs(struct camera_data *cam)static int write_packet (struct usb_device *udev,u8 request,u8 * registers,u16 start,size_t size) { +unsigned char *buf; +int ret; +if (!registers || size <=0 )return -EINVAL ; - return usb_control_msg(udev, + buf =kmalloc (size,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + +memcpy (buf, registers, size); + + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE,drivers/media/usb/cpia2/cpia2_usb.c static int write_packet(struct usb_device *udev, · line 566
Quoted 1 line
drivers/media/usb/cpia2/cpia2_usb.c +0
-0@@ -557,6 +566,9 @@ static int write_packet(struct usb_device *udev, registers, /* buffer */=========
Quoted 2 lines
I think you also want to change the argument to
usb_control_msg()from "registers" to "buf" inwrite_packet().
True!
Thanks for pointing it! Just sent a version of the patch with this fixed.
Regards,
Mauro
PATCH v2 29/31 s2255drv: don't use stack for DMA
In reply to Mauro Carvalho ChehabFrom: Mauro Carvalho Chehab <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/media/usb/s2255/s2255drv.c +13 -2
@@ -1901,19 +1901,30 @@ static long s2255_vendor_req(struct s2255_dev *dev, unsigned char Request, s32 TransferBufferLength, int bOut) { int r; +unsigned char *buf; + + buf =kmalloc (TransferBufferLength,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; +if (!bOut) { r =usb_control_msg (dev->udev,usb_rcvctrlpipe (dev->udev,0 ), Request,USB_TYPE_VENDOR |USB_RECIP_DEVICE |USB_DIR_IN , - Value, Index, TransferBuffer, + Value, Index, buf, TransferBufferLength,HZ *5 ); + +if (r >=0 ) +memcpy (TransferBuffer, buf, TransferBufferLength); }else { +memcpy (buf, TransferBuffer, TransferBufferLength); r =usb_control_msg (dev->udev,usb_sndctrlpipe (dev->udev,0 ), Request,USB_TYPE_VENDOR |USB_RECIP_DEVICE , - Value, Index, TransferBuffer, + Value, Index, buf, TransferBufferLength,HZ *5 ); } +kfree (buf);return r; }
Signature · 1 line
2.7.4
PATCH v2 30/31 stk-webcam: don't use stack for DMA
In reply to Mauro Carvalho ChehabFrom: Mauro Carvalho Chehab <[email protected]>
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.
--- drivers/media/usb/stkwebcam/stk-webcam.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/media/usb/stkwebcam/stk-webcam.c +11 -5
@@ -147,20 +147,26 @@ int stk_camera_write_reg(struct stk_camera *dev, u16 index, u8 value)int stk_camera_read_reg (struct stk_camera *dev,u16 index,int *value) {struct usb_device *udev = dev->udev; +unsigned char *buf;int ret; + buf =kmalloc (sizeof (u8),GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + ret =usb_control_msg (udev,usb_rcvctrlpipe (udev,0 ),0x00 ,USB_DIR_IN |USB_TYPE_VENDOR |USB_RECIP_DEVICE ,0x00 , index, - (u8 *) value, + buf,sizeof (u8),500 ); -if (ret <0 ) -return ret; -else -return 0 ; +if (ret >=0 ) +memcpy (value, buf,sizeof (u8)); + +kfree (buf); +return ret; }static int stk_start_stream (struct stk_camera *dev)
Signature · 1 line
2.7.4
PATCH v2 31/31 flexcop-usb: don't use stack for DMA
In reply to Mauro Carvalho ChehabThe 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.
While here, remove a dead function calling usb_control_msg().
--- drivers/media/usb/b2c2/flexcop-usb.c | 105 +++++++++++++++++++++++------------ drivers/media/usb/b2c2/flexcop-usb.h | 4 ++ 2 files changed, 72 insertions(+), 37 deletions(-)
drivers/media/usb/b2c2/flexcop-usb.c +68 -37
@@ -73,23 +73,34 @@ static int flexcop_usb_readwrite_dw(struct flexcop_device *fc, u16 wRegOffsPCI,u8 request_type = (read ?USB_DIR_IN :USB_DIR_OUT ) |USB_TYPE_VENDOR ;u8 wAddress =B2C2_FLEX_PCIOFFSET_TO_INTERNALADDR (wRegOffsPCI) | (read ?0x80 :0 ); +int ret; -int len =usb_control_msg (fc_usb->udev, +mutex_lock (&fc_usb->data_mutex); +if (!read) +memcpy (fc_usb->data, val,sizeof (*val)); + + ret =usb_control_msg (fc_usb->udev, read ?B2C2_USB_CTRL_PIPE_IN :B2C2_USB_CTRL_PIPE_OUT , request, request_type,/* 0xc0 read or 0x40 write */ wAddress,0 , - val, + fc_usb->data,sizeof (u32),B2C2_WAIT_FOR_OPERATION_RDW *HZ ); -if (len !=sizeof (u32)) { +if (ret !=sizeof (u32)) {err ("error while %s dword from %d (%d)." , read ?"reading" :"writing" , wAddress, wRegOffsPCI); -return -EIO ; +if (ret >=0 ) + ret = -EIO ; } -return 0 ; + +if (read && ret >=0 ) +memcpy (val, fc_usb->data,sizeof (*val)); +mutex_unlock (&fc_usb->data_mutex); + +return ret; }/* * DKT 010817 - add support for V8 memory read/write and flash update
@@ -100,9 +111,14 @@ static int flexcop_usb_v8_memory_req(struct flexcop_usb *fc_usb, {u8 request_type =USB_TYPE_VENDOR ;u16 wIndex; -int nWaitTime, pipe, len; +int nWaitTime, pipe, ret; wIndex = page <<8 ; +if (buflen >sizeof (fc_usb->data)) { +err ("Buffer size bigger than max URB control message\n" ); +return -EIO ; + } +switch (req) {case B2C2_USB_READ_V8_MEM : nWaitTime =B2C2_WAIT_FOR_OPERATION_V8READ ;
@@ -127,17 +143,32 @@ static int flexcop_usb_v8_memory_req(struct flexcop_usb *fc_usb,deb_v8 ("v8mem: %02x %02x %04x %04x, len: %d\n" , request_type, req, wAddress, wIndex, buflen); - len =usb_control_msg (fc_usb->udev, pipe, +mutex_lock (&fc_usb->data_mutex); + +if ((request_type &USB_ENDPOINT_DIR_MASK ) ==USB_DIR_OUT ) +memcpy (fc_usb->data, pbBuffer, buflen); + + ret =usb_control_msg (fc_usb->udev, pipe, req, request_type, wAddress, wIndex, - pbBuffer, + fc_usb->data, buflen, nWaitTime *HZ ); +if (ret != buflen) + ret = -EIO ; -debug_dump (pbBuffer, len, deb_v8); -return len == buflen ?0 : -EIO ; +if (ret >=0 ) { + ret =0 ; +if ((request_type &USB_ENDPOINT_DIR_MASK ) ==USB_DIR_IN ) +memcpy (pbBuffer, fc_usb->data, buflen); + } + +mutex_unlock (&fc_usb->data_mutex); + +debug_dump (pbBuffer, ret, deb_v8); +return ret; }#define bytes_left_to_read_on_page (paddr,buflen) \
@@ -196,29 +227,6 @@ static int flexcop_usb_get_mac_addr(struct flexcop_device *fc, int extended) fc->dvb_adapter.proposed_mac, 6); } -#if 0 -static int flexcop_usb_utility_req (struct flexcop_usb *fc_usb,int set, -flexcop_usb_utility_function_t func,u8 extra,u16 wIndex, -u16 buflen,u8 *pvBuffer) -{ -u16 wValue; -u8 request_type = (set ?USB_DIR_OUT :USB_DIR_IN ) |USB_TYPE_VENDOR ; -int nWaitTime =2 , - pipe = set ?B2C2_USB_CTRL_PIPE_OUT :B2C2_USB_CTRL_PIPE_IN , len; - wValue = (func <<8 ) | extra; - - len =usb_control_msg (fc_usb->udev,pipe, -B2C2_USB_UTILITY , - request_type, - wValue, - wIndex, - pvBuffer, - buflen, - nWaitTime *HZ ); -return len == buflen ?0 : -EIO ; -} -#endif -/* usb i2c stuff */ static int flexcop_usb_i2c_req(struct flexcop_i2c_adapter *i2c, flexcop_usb_request_t req, flexcop_usb_i2c_function_t func,
@@ -226,9 +234,14 @@ static int flexcop_usb_i2c_req(struct flexcop_i2c_adapter *i2c, {struct flexcop_usb *fc_usb = i2c->fc->bus_specific;u16 wValue, wIndex; -int nWaitTime,pipe,len; +int nWaitTime, pipe, ret;u8 request_type =USB_TYPE_VENDOR ; +if (buflen >sizeof (fc_usb->data)) { +err ("Buffer size bigger than max URB control message\n" ); +return -EIO ; + } +switch (func) {case USB_FUNC_I2C_WRITE :case USB_FUNC_I2C_MULTIWRITE :
@@ -257,15 +270,32 @@ static int flexcop_usb_i2c_req(struct flexcop_i2c_adapter *i2c, wValue &0xff , wValue >>8 , wIndex & 0xff, wIndex >> 8); - len =usb_control_msg (fc_usb->udev,pipe, +mutex_lock (&fc_usb->data_mutex); + +if ((request_type &USB_ENDPOINT_DIR_MASK ) ==USB_DIR_OUT ) +memcpy (fc_usb->data, buf, buflen); + + ret =usb_control_msg (fc_usb->udev,pipe, req, request_type, wValue, wIndex, - buf, + fc_usb->data, buflen, nWaitTime *HZ ); -return len == buflen ?0 : -EREMOTEIO ; + +if (ret != buflen) + ret = -EIO ; + +if (ret >=0 ) { + ret =0 ; +if ((request_type &USB_ENDPOINT_DIR_MASK ) ==USB_DIR_IN ) +memcpy (buf, fc_usb->data, buflen); + } + +mutex_unlock (&fc_usb->data_mutex); + +return 0 ; }/* actual bus specific access functions,
@@ -516,6 +546,7 @@ static int flexcop_usb_probe(struct usb_interface *intf,/* general flexcop init */ fc_usb = fc->bus_specific; fc_usb->fc_dev = fc; +mutex_init (&fc_usb->data_mutex); fc->read_ibi_reg = flexcop_usb_read_ibi_reg; fc->write_ibi_reg = flexcop_usb_write_ibi_reg;
drivers/media/usb/b2c2/flexcop-usb.h +4 -0
@@ -29,6 +29,10 @@ struct flexcop_usb {u8 tmp_buffer[1023 +190 ];int tmp_buffer_length; + +/* for URB control messages */ +u8 data[80 ]; +struct mutex data_mutex; };#if 0
Signature · 1 line
2.7.4