From 5ae8f9a3757e4010c7ea9c07c047088fb812335e Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Thu, 26 May 2011 15:51:11 -0300 Subject: [media] mceusb: add and use mce_dbg printk macro Using dev_dbg is more complexity than many users are able to deal with. Make it easier to get debug spew feedback from them by adding an mce_dbg printk macro that spews using dev_info when debug=1 is set for the mceusb module. Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index ad927fcaa020..10ec54b93d26 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -108,6 +108,12 @@ static int debug = 1; static int debug; #endif +#define mce_dbg(dev, fmt, ...) \ + do { \ + if (debug) \ + dev_info(dev, fmt, ## __VA_ARGS__); \ + } while (0) + /* general constants */ #define SEND_FLAG_IN_PROGRESS 1 #define SEND_FLAG_COMPLETE 2 @@ -606,7 +612,7 @@ static void mce_async_callback(struct urb *urb, struct pt_regs *regs) if (ir) { len = urb->actual_length; - dev_dbg(ir->dev, "callback called (status=%d len=%d)\n", + mce_dbg(ir->dev, "callback called (status=%d len=%d)\n", urb->status, len); mceusb_dev_printdata(ir, urb->transfer_buffer, 0, len, true); @@ -655,17 +661,17 @@ static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data, return; } - dev_dbg(dev, "receive request called (size=%#x)\n", size); + mce_dbg(dev, "receive request called (size=%#x)\n", size); async_urb->transfer_buffer_length = size; async_urb->dev = ir->usbdev; res = usb_submit_urb(async_urb, GFP_ATOMIC); if (res) { - dev_dbg(dev, "receive request FAILED! (res=%d)\n", res); + mce_dbg(dev, "receive request FAILED! (res=%d)\n", res); return; } - dev_dbg(dev, "receive request complete (res=%d)\n", res); + mce_dbg(dev, "receive request complete (res=%d)\n", res); } static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size) @@ -794,7 +800,7 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32 carrier) ir->carrier = carrier; cmdbuf[2] = MCE_CMD_SIG_END; cmdbuf[3] = MCE_IRDATA_TRAILER; - dev_dbg(ir->dev, "%s: disabling carrier " + mce_dbg(ir->dev, "%s: disabling carrier " "modulation\n", __func__); mce_async_out(ir, cmdbuf, sizeof(cmdbuf)); return carrier; @@ -806,7 +812,7 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32 carrier) ir->carrier = carrier; cmdbuf[2] = prescaler; cmdbuf[3] = divisor; - dev_dbg(ir->dev, "%s: requesting %u HZ " + mce_dbg(ir->dev, "%s: requesting %u HZ " "carrier\n", __func__, carrier); /* Transmit new carrier to mce device */ @@ -879,7 +885,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len) rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK) * US_TO_NS(MCE_TIME_UNIT); - dev_dbg(ir->dev, "Storing %s with duration %d\n", + mce_dbg(ir->dev, "Storing %s with duration %d\n", rawir.pulse ? "pulse" : "space", rawir.duration); @@ -911,7 +917,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len) if (ir->parser_state != CMD_HEADER && !ir->rem) ir->parser_state = CMD_HEADER; } - dev_dbg(ir->dev, "processed IR data, calling ir_raw_event_handle\n"); + mce_dbg(ir->dev, "processed IR data, calling ir_raw_event_handle\n"); ir_raw_event_handle(ir->rc); } @@ -933,7 +939,7 @@ static void mceusb_dev_recv(struct urb *urb, struct pt_regs *regs) if (ir->send_flags == RECV_FLAG_IN_PROGRESS) { ir->send_flags = SEND_FLAG_COMPLETE; - dev_dbg(ir->dev, "setup answer received %d bytes\n", + mce_dbg(ir->dev, "setup answer received %d bytes\n", buf_len); } @@ -951,7 +957,7 @@ static void mceusb_dev_recv(struct urb *urb, struct pt_regs *regs) case -EPIPE: default: - dev_dbg(ir->dev, "Error: urb status = %d\n", urb->status); + mce_dbg(ir->dev, "Error: urb status = %d\n", urb->status); break; } @@ -978,8 +984,8 @@ static void mceusb_gen1_init(struct mceusb_dev *ir) ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0), USB_REQ_SET_ADDRESS, USB_TYPE_VENDOR, 0, 0, data, USB_CTRL_MSG_SZ, HZ * 3); - dev_dbg(dev, "%s - ret = %d\n", __func__, ret); - dev_dbg(dev, "%s - data[0] = %d, data[1] = %d\n", + mce_dbg(dev, "%s - ret = %d\n", __func__, ret); + mce_dbg(dev, "%s - data[0] = %d, data[1] = %d\n", __func__, data[0], data[1]); /* set feature: bit rate 38400 bps */ @@ -987,19 +993,19 @@ static void mceusb_gen1_init(struct mceusb_dev *ir) USB_REQ_SET_FEATURE, USB_TYPE_VENDOR, 0xc04e, 0x0000, NULL, 0, HZ * 3); - dev_dbg(dev, "%s - ret = %d\n", __func__, ret); + mce_dbg(dev, "%s - ret = %d\n", __func__, ret); /* bRequest 4: set char length to 8 bits */ ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0), 4, USB_TYPE_VENDOR, 0x0808, 0x0000, NULL, 0, HZ * 3); - dev_dbg(dev, "%s - retB = %d\n", __func__, ret); + mce_dbg(dev, "%s - retB = %d\n", __func__, ret); /* bRequest 2: set handshaking to use DTR/DSR */ ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0), 2, USB_TYPE_VENDOR, 0x0000, 0x0100, NULL, 0, HZ * 3); - dev_dbg(dev, "%s - retC = %d\n", __func__, ret); + mce_dbg(dev, "%s - retC = %d\n", __func__, ret); /* device reset */ mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET)); @@ -1122,7 +1128,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, bool tx_mask_normal; int ir_intfnum; - dev_dbg(&intf->dev, "%s called\n", __func__); + mce_dbg(&intf->dev, "%s called\n", __func__); idesc = intf->cur_altsetting; @@ -1150,7 +1156,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, ep_in = ep; ep_in->bmAttributes = USB_ENDPOINT_XFER_INT; ep_in->bInterval = 1; - dev_dbg(&intf->dev, "acceptable inbound endpoint " + mce_dbg(&intf->dev, "acceptable inbound endpoint " "found\n"); } @@ -1165,12 +1171,12 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, ep_out = ep; ep_out->bmAttributes = USB_ENDPOINT_XFER_INT; ep_out->bInterval = 1; - dev_dbg(&intf->dev, "acceptable outbound endpoint " + mce_dbg(&intf->dev, "acceptable outbound endpoint " "found\n"); } } if (ep_in == NULL) { - dev_dbg(&intf->dev, "inbound and/or endpoint not found\n"); + mce_dbg(&intf->dev, "inbound and/or endpoint not found\n"); return -ENODEV; } -- cgit v1.2.3 From b825fe1b1bb5927402c3d3084641355946ef05f8 Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Thu, 26 May 2011 16:03:17 -0300 Subject: [media] mceusb: support I-O Data GV-MC7/RCKIT There's an SMK-device-id remote kit from I-O Data avaiable primarily in Japan, which appears to have no tx hardware, but has rx functionality that works with the mceusb driver by simply adding its device ID. Reported-by: Jeremy Kwok Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 10ec54b93d26..650b2f479519 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -252,6 +252,9 @@ static struct usb_device_id mceusb_dev_table[] = { .driver_info = MCE_GEN2_TX_INV }, /* SMK eHome Infrared Transceiver */ { USB_DEVICE(VENDOR_SMK, 0x0338) }, + /* SMK/I-O Data GV-MC7/RCKIT Receiver */ + { USB_DEVICE(VENDOR_SMK, 0x0353), + .driver_info = MCE_GEN2_NO_TX }, /* Tatung eHome Infrared Transceiver */ { USB_DEVICE(VENDOR_TATUNG, 0x9150) }, /* Shuttle eHome Infrared Transceiver */ -- cgit v1.2.3 From 3a918aa69daf001910640cc910ea4053ba840a6e Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Thu, 26 May 2011 14:23:18 -0300 Subject: [media] mceusb: mce_sync_in is brain-dead Aside from the initial "hey, lets make sure we've flushed any pre-existing data on the device" call to mce_sync_in, every other one of the calls was entirely superfluous. Ergo, remove them all, and rename the one and only (questionably) useful one to reflect what it really does. Verified on both gen2 and gen3 hardware to make zero difference. Well, except that you no longer get a bunch of urb submit failures from the unneeded mce_sync_in calls. Oh. And move that flush to a point *after* we've wired up the inbound urb, or it won't do squat. I have half a mind to just remove it entirely, but someone thought it was necessary at some point, and it doesn't seem to hurt, so lets leave it for the time being. This excercise took place due to insightful questions asked by Hans Petter Selasky, about the possible reuse of the inbound urb before it was actually availble by mce_sync_in, so thanks to him for motivating this cleanup. Reported-by: Hans Petter Selasky Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 650b2f479519..6cf22012079a 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -682,9 +682,9 @@ static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size) mce_request_packet(ir, data, size, MCEUSB_TX); } -static void mce_sync_in(struct mceusb_dev *ir, unsigned char *data, int size) +static void mce_flush_rx_buffer(struct mceusb_dev *ir, int size) { - mce_request_packet(ir, data, size, MCEUSB_RX); + mce_request_packet(ir, NULL, size, MCEUSB_RX); } /* Send data out the IR blaster port(s) */ @@ -970,7 +970,6 @@ static void mceusb_dev_recv(struct urb *urb, struct pt_regs *regs) static void mceusb_gen1_init(struct mceusb_dev *ir) { int ret; - int maxp = ir->len_in; struct device *dev = ir->dev; char *data; @@ -1012,55 +1011,40 @@ static void mceusb_gen1_init(struct mceusb_dev *ir) /* device reset */ mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET)); - mce_sync_in(ir, NULL, maxp); /* get hw/sw revision? */ mce_async_out(ir, GET_REVISION, sizeof(GET_REVISION)); - mce_sync_in(ir, NULL, maxp); kfree(data); }; static void mceusb_gen2_init(struct mceusb_dev *ir) { - int maxp = ir->len_in; - /* device reset */ mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET)); - mce_sync_in(ir, NULL, maxp); /* get hw/sw revision? */ mce_async_out(ir, GET_REVISION, sizeof(GET_REVISION)); - mce_sync_in(ir, NULL, maxp); /* unknown what the next two actually return... */ mce_async_out(ir, GET_UNKNOWN, sizeof(GET_UNKNOWN)); - mce_sync_in(ir, NULL, maxp); mce_async_out(ir, GET_UNKNOWN2, sizeof(GET_UNKNOWN2)); - mce_sync_in(ir, NULL, maxp); } static void mceusb_get_parameters(struct mceusb_dev *ir) { - int maxp = ir->len_in; - /* get the carrier and frequency */ mce_async_out(ir, GET_CARRIER_FREQ, sizeof(GET_CARRIER_FREQ)); - mce_sync_in(ir, NULL, maxp); - if (!ir->flags.no_tx) { + if (!ir->flags.no_tx) /* get the transmitter bitmask */ mce_async_out(ir, GET_TX_BITMASK, sizeof(GET_TX_BITMASK)); - mce_sync_in(ir, NULL, maxp); - } /* get receiver timeout value */ mce_async_out(ir, GET_RX_TIMEOUT, sizeof(GET_RX_TIMEOUT)); - mce_sync_in(ir, NULL, maxp); /* get receiver sensor setting */ mce_async_out(ir, GET_RX_SENSOR, sizeof(GET_RX_SENSOR)); - mce_sync_in(ir, NULL, maxp); } static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir) @@ -1224,16 +1208,16 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, if (!ir->rc) goto rc_dev_fail; - /* flush buffers on the device */ - mce_sync_in(ir, NULL, maxp); - mce_sync_in(ir, NULL, maxp); - /* wire up inbound data handler */ usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp, (usb_complete_t) mceusb_dev_recv, ir, ep_in->bInterval); ir->urb_in->transfer_dma = ir->dma_in; ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + /* flush buffers on the device */ + mce_dbg(&intf->dev, "Flushing receive buffers\n"); + mce_flush_rx_buffer(ir, maxp); + /* initialize device */ if (ir->flags.microsoft_gen1) mceusb_gen1_init(ir); -- cgit v1.2.3 From 0b43fcdff6495958c39e3575848edef4b685ddef Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Tue, 24 May 2011 16:44:54 -0300 Subject: [media] mceusb: plug memory leak on data transmit Hans Petter Selasky pointed out to me that we're leaking urbs when mce_async_out is called. Its used both for configuring the hardware and for transmitting IR data. In the tx case, mce_request_packet actually allocates both a urb and the transfer buffer, neither of which was being torn down. Do that in the tx callback. CC: Hans Petter Selasky Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 6cf22012079a..06dfe0957b5e 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -621,6 +621,9 @@ static void mce_async_callback(struct urb *urb, struct pt_regs *regs) mceusb_dev_printdata(ir, urb->transfer_buffer, 0, len, true); } + /* the transfer buffer and urb were allocated in mce_request_packet */ + kfree(urb->transfer_buffer); + usb_free_urb(urb); } /* request incoming or send outgoing usb packet - used to initialize remote */ -- cgit v1.2.3 From f3e456cb899304bed183247ed3228f7ff855eebd Mon Sep 17 00:00:00 2001 From: Rafi Rubin Date: Sun, 3 Jul 2011 17:13:52 -0300 Subject: [media] mceusb: Timeout unit corrections Unit missmatch in mceusb_handle_command. It should be converting to us, not 1/10th of ms. mceusb_dev_printdata 100us/ms -> 1000us/ms Alter format of fix slightly and update comment to match proper reality. Signed-off-by: Rafi Rubin Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 06dfe0957b5e..b928a61736d7 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -558,9 +558,10 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf, inout, data1); break; case MCE_CMD_S_TIMEOUT: - /* value is in units of 50us, so x*50/100 or x/2 ms */ + /* value is in units of 50us, so x*50/1000 ms */ dev_info(dev, "%s receive timeout of %d ms\n", - inout, ((data1 << 8) | data2) / 2); + inout, + ((data1 << 8) | data2) * MCE_TIME_UNIT / 1000); break; case MCE_CMD_G_TIMEOUT: dev_info(dev, "Get receive timeout\n"); @@ -847,7 +848,7 @@ static void mceusb_handle_command(struct mceusb_dev *ir, int index) switch (ir->buf_in[index]) { /* 2-byte return value commands */ case MCE_CMD_S_TIMEOUT: - ir->rc->timeout = US_TO_NS((hi << 8 | lo) / 2); + ir->rc->timeout = US_TO_NS((hi << 8 | lo) * MCE_TIME_UNIT); break; /* 1-byte return value commands */ -- cgit v1.2.3 From 9824ae4aff2793947ea78c4c8147bb6c59efdcba Mon Sep 17 00:00:00 2001 From: Rafi Rubin Date: Sun, 3 Jul 2011 17:13:53 -0300 Subject: [media] mceusb: increase default timeout to 100ms This matches the typical timeout advertised by hardware, once we're actually interpreting it correctly. Signed-off-by: Rafi Rubin Signed-off-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/media/rc/mceusb.c') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index b928a61736d7..ec972dc25790 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -1079,7 +1079,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir) rc->priv = ir; rc->driver_type = RC_DRIVER_IR_RAW; rc->allowed_protos = RC_TYPE_ALL; - rc->timeout = US_TO_NS(1000); + rc->timeout = MS_TO_NS(100); if (!ir->flags.no_tx) { rc->s_tx_mask = mceusb_set_tx_mask; rc->s_tx_carrier = mceusb_set_tx_carrier; -- cgit v1.2.3