[PATCH 3/4] [media] mceusb: remove redundant function and defines

[PATCH 3/4] [media] mceusb: remove redundant function and defines

[Sean Young]

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/mceusb.c | 92 +++++++++++++++--------------------------------
 1 file changed, 28 insertions(+), 64 deletions(-)

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a25bb15..3a4f95f 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -166,15 +166,6 @@ static bool debug;
 			dev_info(dev, fmt, ## __VA_ARGS__);	\
 	} while (0)
 
-/* general constants */
-#define SEND_FLAG_IN_PROGRESS	1
-#define SEND_FLAG_COMPLETE	2
-#define RECV_FLAG_IN_PROGRESS	3
-#define RECV_FLAG_COMPLETE	4
-
-#define MCEUSB_RX		1
-#define MCEUSB_TX		2
-
 #define VENDOR_PHILIPS		0x0471
 #define VENDOR_SMK		0x0609
 #define VENDOR_TATUNG		0x1460
@@ -452,7 +443,6 @@ struct mceusb_dev {
 	} flags;
 
 	/* transmit support */
-	int send_flags;
 	u32 carrier;
 	unsigned char tx_mask;
 
@@ -731,45 +721,29 @@ static void mce_async_callback(struct urb *urb)
 
 /* request incoming or send outgoing usb packet - used to initialize remote */
 static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
-			       int size, int urb_type)
+			       int size)
 {
 	int res, pipe;
 	struct urb *async_urb;
 	struct device *dev = ir->dev;
 	unsigned char *async_buf;
 
-	if (urb_type == MCEUSB_TX) {
-		async_urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (unlikely(!async_urb)) {
-			dev_err(dev, "Error, couldn't allocate urb!\n");
-			return;
-		}
-
-		async_buf = kzalloc(size, GFP_KERNEL);
-		if (!async_buf) {
-			dev_err(dev, "Error, couldn't allocate buf!\n");
-			usb_free_urb(async_urb);
-			return;
-		}
+	async_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (unlikely(!async_urb))
+		return;
 
-		/* outbound data */
-		pipe = usb_sndintpipe(ir->usbdev,
-				      ir->usb_ep_out->bEndpointAddress);
-		usb_fill_int_urb(async_urb, ir->usbdev, pipe,
-			async_buf, size, mce_async_callback,
-			ir, ir->usb_ep_out->bInterval);
-		memcpy(async_buf, data, size);
-
-	} else if (urb_type == MCEUSB_RX) {
-		/* standard request */
-		async_urb = ir->urb_in;
-		ir->send_flags = RECV_FLAG_IN_PROGRESS;
-
-	} else {
-		dev_err(dev, "Error! Unknown urb type %d\n", urb_type);
+	async_buf = kmalloc(size, GFP_KERNEL);
+	if (!async_buf) {
+		usb_free_urb(async_urb);
 		return;
 	}
 
+	/* outbound data */
+	pipe = usb_sndintpipe(ir->usbdev, ir->usb_ep_out->bEndpointAddress);
+	usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf, size,
+			mce_async_callback, ir, ir->usb_ep_out->bInterval);
+	memcpy(async_buf, data, size);
+
 	mce_dbg(dev, "receive request called (size=%#x)\n", size);
 
 	async_urb->transfer_buffer_length = size;
@@ -789,19 +763,14 @@ static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size)
 
 	if (ir->need_reset) {
 		ir->need_reset = false;
-		mce_request_packet(ir, DEVICE_RESUME, rsize, MCEUSB_TX);
+		mce_request_packet(ir, DEVICE_RESUME, rsize);
 		msleep(10);
 	}
 
-	mce_request_packet(ir, data, size, MCEUSB_TX);
+	mce_request_packet(ir, data, size);
 	msleep(10);
 }
 
-static void mce_flush_rx_buffer(struct mceusb_dev *ir, int size)
-{
-	mce_request_packet(ir, NULL, size, MCEUSB_RX);
-}
-
 /* Send data out the IR blaster port(s) */
 static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 {
@@ -1040,7 +1009,6 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 static void mceusb_dev_recv(struct urb *urb)
 {
 	struct mceusb_dev *ir;
-	int buf_len;
 
 	if (!urb)
 		return;
@@ -1051,18 +1019,10 @@ static void mceusb_dev_recv(struct urb *urb)
 		return;
 	}
 
-	buf_len = urb->actual_length;
-
-	if (ir->send_flags == RECV_FLAG_IN_PROGRESS) {
-		ir->send_flags = SEND_FLAG_COMPLETE;
-		mce_dbg(ir->dev, "setup answer received %d bytes\n",
-			buf_len);
-	}
-
 	switch (urb->status) {
 	/* success */
 	case 0:
-		mceusb_process_ir_data(ir, buf_len);
+		mceusb_process_ir_data(ir, urb->actual_length);
 		break;
 
 	case -ECONNRESET:
@@ -1250,7 +1210,7 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *ep_in = NULL;
 	struct usb_endpoint_descriptor *ep_out = NULL;
 	struct mceusb_dev *ir = NULL;
-	int pipe, maxp, i;
+	int pipe, maxp, i, res;
 	char buf[63], name[128] = "";
 	enum mceusb_model_type model = id->driver_info;
 	bool is_gen3;
@@ -1346,19 +1306,21 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 		snprintf(name + strlen(name), sizeof(name) - strlen(name),
 			 " %s", buf);
 
-	ir->rc = mceusb_init_rc_dev(ir);
-	if (!ir->rc)
-		goto rc_dev_fail;
-
 	/* wire up inbound data handler */
 	usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
 				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);
+	res = usb_submit_urb(ir->urb_in, GFP_KERNEL);
+	if (res) {
+		dev_err(&intf->dev, "failed to submit urb: %d\n", res);
+		goto usb_submit_fail;
+	}
+
+	ir->rc = mceusb_init_rc_dev(ir);
+	if (!ir->rc)
+		goto rc_dev_fail;
 
 	/* figure out which firmware/emulator version this hardware has */
 	mceusb_get_emulator_version(ir);
@@ -1393,6 +1355,8 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 
 	/* Error-handling path */
 rc_dev_fail:
+	usb_kill_urb(ir->urb_in);
+usb_submit_fail:
 	usb_free_urb(ir->urb_in);
 urb_in_alloc_fail:
 	usb_free_coherent(dev, maxp, ir->buf_in, ir->dma_in);
-- 
1.8.4.2

[PATCH 4/4] [media] mceusb: improve error logging

[Sean Young]

A number of recent bug reports involve usb_submit_urb() failing which was
only reported with debug parameter on. In addition, remove custom debug
function.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/mceusb.c | 180 ++++++++++++++++++++++------------------------
 1 file changed, 84 insertions(+), 96 deletions(-)

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 3a4f95f..2df5ac0 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -84,7 +84,7 @@
 #define MCE_PORT_IR		0x4	/* (0x4 << 5) | MCE_CMD = 0x9f */
 #define MCE_PORT_SYS		0x7	/* (0x7 << 5) | MCE_CMD = 0xff */
 #define MCE_PORT_SER		0x6	/* 0xc0 thru 0xdf flush & 0x1f bytes */
-#define MCE_PORT_MASK	0xe0	/* Mask out command bits */
+#define MCE_PORT_MASK		0xe0	/* Mask out command bits */
 
 /* Command port headers */
 #define MCE_CMD_PORT_IR		0x9f	/* IR-related cmd/rsp */
@@ -153,19 +153,6 @@
 #define MCE_COMMAND_IRDATA	0x80
 #define MCE_PACKET_LENGTH_MASK	0x1f /* Packet length mask */
 
-/* module parameters */
-#ifdef CONFIG_USB_DEBUG
-static bool debug = 1;
-#else
-static bool debug;
-#endif
-
-#define mce_dbg(dev, fmt, ...)					\
-	do {							\
-		if (debug)					\
-			dev_info(dev, fmt, ## __VA_ARGS__);	\
-	} while (0)
-
 #define VENDOR_PHILIPS		0x0471
 #define VENDOR_SMK		0x0609
 #define VENDOR_TATUNG		0x1460
@@ -531,16 +518,13 @@ static int mceusb_cmd_datasize(u8 cmd, u8 subcmd)
 static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 				 int offset, int len, bool out)
 {
-	char codes[USB_BUFLEN * 3 + 1];
-	char inout[9];
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
+	char *inout;
 	u8 cmd, subcmd, data1, data2, data3, data4;
 	struct device *dev = ir->dev;
-	int i, start, skip = 0;
+	int start, skip = 0;
 	u32 carrier, period;
 
-	if (!debug)
-		return;
-
 	/* skip meaningless 0xb1 0x60 header bytes on orig receiver */
 	if (ir->flags.microsoft_gen1 && !out && !offset)
 		skip = 2;
@@ -548,16 +532,10 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 	if (len <= skip)
 		return;
 
-	for (i = 0; i < len && i < USB_BUFLEN; i++)
-		snprintf(codes + i * 3, 4, "%02x ", buf[i + offset] & 0xff);
-
-	dev_info(dev, "%sx data: %s(length=%d)\n",
-		 (out ? "t" : "r"), codes, len);
+	dev_dbg(dev, "%cx data: %*ph (length=%d)",
+		(out ? 't' : 'r'), min(len, USB_BUFLEN), buf, len);
 
-	if (out)
-		strcpy(inout, "Request\0");
-	else
-		strcpy(inout, "Got\0");
+	inout = out ? "Request" : "Got";
 
 	start  = offset + skip;
 	cmd    = buf[start] & 0xff;
@@ -573,50 +551,50 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 			break;
 		if ((subcmd == MCE_CMD_PORT_SYS) &&
 		    (data1 == MCE_CMD_RESUME))
-			dev_info(dev, "Device resume requested\n");
+			dev_dbg(dev, "Device resume requested");
 		else
-			dev_info(dev, "Unknown command 0x%02x 0x%02x\n",
+			dev_dbg(dev, "Unknown command 0x%02x 0x%02x",
 				 cmd, subcmd);
 		break;
 	case MCE_CMD_PORT_SYS:
 		switch (subcmd) {
 		case MCE_RSP_EQEMVER:
 			if (!out)
-				dev_info(dev, "Emulator interface version %x\n",
+				dev_dbg(dev, "Emulator interface version %x",
 					 data1);
 			break;
 		case MCE_CMD_G_REVISION:
 			if (len == 2)
-				dev_info(dev, "Get hw/sw rev?\n");
+				dev_dbg(dev, "Get hw/sw rev?");
 			else
-				dev_info(dev, "hw/sw rev 0x%02x 0x%02x "
-					 "0x%02x 0x%02x\n", data1, data2,
+				dev_dbg(dev, "hw/sw rev 0x%02x 0x%02x 0x%02x 0x%02x",
+					 data1, data2,
 					 buf[start + 4], buf[start + 5]);
 			break;
 		case MCE_CMD_RESUME:
-			dev_info(dev, "Device resume requested\n");
+			dev_dbg(dev, "Device resume requested");
 			break;
 		case MCE_RSP_CMD_ILLEGAL:
-			dev_info(dev, "Illegal PORT_SYS command\n");
+			dev_dbg(dev, "Illegal PORT_SYS command");
 			break;
 		case MCE_RSP_EQWAKEVERSION:
 			if (!out)
-				dev_info(dev, "Wake version, proto: 0x%02x, "
+				dev_dbg(dev, "Wake version, proto: 0x%02x, "
 					 "payload: 0x%02x, address: 0x%02x, "
-					 "version: 0x%02x\n",
+					 "version: 0x%02x",
 					 data1, data2, data3, data4);
 			break;
 		case MCE_RSP_GETPORTSTATUS:
 			if (!out)
 				/* We use data1 + 1 here, to match hw labels */
-				dev_info(dev, "TX port %d: blaster is%s connected\n",
+				dev_dbg(dev, "TX port %d: blaster is%s connected",
 					 data1 + 1, data4 ? " not" : "");
 			break;
 		case MCE_CMD_FLASHLED:
-			dev_info(dev, "Attempting to flash LED\n");
+			dev_dbg(dev, "Attempting to flash LED");
 			break;
 		default:
-			dev_info(dev, "Unknown command 0x%02x 0x%02x\n",
+			dev_dbg(dev, "Unknown command 0x%02x 0x%02x",
 				 cmd, subcmd);
 			break;
 		}
@@ -624,13 +602,13 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 	case MCE_CMD_PORT_IR:
 		switch (subcmd) {
 		case MCE_CMD_SIG_END:
-			dev_info(dev, "End of signal\n");
+			dev_dbg(dev, "End of signal");
 			break;
 		case MCE_CMD_PING:
-			dev_info(dev, "Ping\n");
+			dev_dbg(dev, "Ping");
 			break;
 		case MCE_CMD_UNKNOWN:
-			dev_info(dev, "Resp to 9f 05 of 0x%02x 0x%02x\n",
+			dev_dbg(dev, "Resp to 9f 05 of 0x%02x 0x%02x",
 				 data1, data2);
 			break;
 		case MCE_RSP_EQIRCFS:
@@ -639,51 +617,51 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 			if (!period)
 				break;
 			carrier = (1000 * 1000) / period;
-			dev_info(dev, "%s carrier of %u Hz (period %uus)\n",
+			dev_dbg(dev, "%s carrier of %u Hz (period %uus)",
 				 inout, carrier, period);
 			break;
 		case MCE_CMD_GETIRCFS:
-			dev_info(dev, "Get carrier mode and freq\n");
+			dev_dbg(dev, "Get carrier mode and freq");
 			break;
 		case MCE_RSP_EQIRTXPORTS:
-			dev_info(dev, "%s transmit blaster mask of 0x%02x\n",
+			dev_dbg(dev, "%s transmit blaster mask of 0x%02x",
 				 inout, data1);
 			break;
 		case MCE_RSP_EQIRTIMEOUT:
 			/* value is in units of 50us, so x*50/1000 ms */
 			period = ((data1 << 8) | data2) * MCE_TIME_UNIT / 1000;
-			dev_info(dev, "%s receive timeout of %d ms\n",
+			dev_dbg(dev, "%s receive timeout of %d ms",
 				 inout, period);
 			break;
 		case MCE_CMD_GETIRTIMEOUT:
-			dev_info(dev, "Get receive timeout\n");
+			dev_dbg(dev, "Get receive timeout");
 			break;
 		case MCE_CMD_GETIRTXPORTS:
-			dev_info(dev, "Get transmit blaster mask\n");
+			dev_dbg(dev, "Get transmit blaster mask");
 			break;
 		case MCE_RSP_EQIRRXPORTEN:
-			dev_info(dev, "%s %s-range receive sensor in use\n",
+			dev_dbg(dev, "%s %s-range receive sensor in use",
 				 inout, data1 == 0x02 ? "short" : "long");
 			break;
 		case MCE_CMD_GETIRRXPORTEN:
 		/* aka MCE_RSP_EQIRRXCFCNT */
 			if (out)
-				dev_info(dev, "Get receive sensor\n");
+				dev_dbg(dev, "Get receive sensor");
 			else if (ir->learning_enabled)
-				dev_info(dev, "RX pulse count: %d\n",
+				dev_dbg(dev, "RX pulse count: %d",
 					 ((data1 << 8) | data2));
 			break;
 		case MCE_RSP_EQIRNUMPORTS:
 			if (out)
 				break;
-			dev_info(dev, "Num TX ports: %x, num RX ports: %x\n",
+			dev_dbg(dev, "Num TX ports: %x, num RX ports: %x",
 				 data1, data2);
 			break;
 		case MCE_RSP_CMD_ILLEGAL:
-			dev_info(dev, "Illegal PORT_IR command\n");
+			dev_dbg(dev, "Illegal PORT_IR command");
 			break;
 		default:
-			dev_info(dev, "Unknown command 0x%02x 0x%02x\n",
+			dev_dbg(dev, "Unknown command 0x%02x 0x%02x",
 				 cmd, subcmd);
 			break;
 		}
@@ -693,10 +671,11 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 	}
 
 	if (cmd == MCE_IRDATA_TRAILER)
-		dev_info(dev, "End of raw IR data\n");
+		dev_dbg(dev, "End of raw IR data");
 	else if ((cmd != MCE_CMD_PORT_IR) &&
 		 ((cmd & MCE_PORT_MASK) == MCE_COMMAND_IRDATA))
-		dev_info(dev, "Raw IR data, %d pulse/space samples\n", ir->rem);
+		dev_dbg(dev, "Raw IR data, %d pulse/space samples", ir->rem);
+#endif
 }
 
 static void mce_async_callback(struct urb *urb)
@@ -708,10 +687,25 @@ static void mce_async_callback(struct urb *urb)
 		return;
 
 	ir = urb->context;
-	if (ir) {
+
+	switch (urb->status) {
+	/* success */
+	case 0:
 		len = urb->actual_length;
 
 		mceusb_dev_printdata(ir, urb->transfer_buffer, 0, len, true);
+		break;
+
+	case -ECONNRESET:
+	case -ENOENT:
+	case -EILSEQ:
+	case -ESHUTDOWN:
+		break;
+
+	case -EPIPE:
+	default:
+		dev_err(ir->dev, "Error: request urb status = %d", urb->status);
+		break;
 	}
 
 	/* the transfer buffer and urb were allocated in mce_request_packet */
@@ -744,17 +738,17 @@ static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
 			mce_async_callback, ir, ir->usb_ep_out->bInterval);
 	memcpy(async_buf, data, size);
 
-	mce_dbg(dev, "receive request called (size=%#x)\n", size);
+	dev_dbg(dev, "receive request called (size=%#x)", size);
 
 	async_urb->transfer_buffer_length = size;
 	async_urb->dev = ir->usbdev;
 
 	res = usb_submit_urb(async_urb, GFP_ATOMIC);
 	if (res) {
-		mce_dbg(dev, "receive request FAILED! (res=%d)\n", res);
+		dev_err(dev, "receive request FAILED! (res=%d)", res);
 		return;
 	}
-	mce_dbg(dev, "receive request complete (res=%d)\n", res);
+	dev_dbg(dev, "receive request complete (res=%d)", res);
 }
 
 static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size)
@@ -864,8 +858,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;
-			mce_dbg(ir->dev, "%s: disabling carrier "
-				"modulation\n", __func__);
+			dev_dbg(ir->dev, "disabling carrier modulation");
 			mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
 			return carrier;
 		}
@@ -876,8 +869,8 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32 carrier)
 				ir->carrier = carrier;
 				cmdbuf[2] = prescaler;
 				cmdbuf[3] = divisor;
-				mce_dbg(ir->dev, "%s: requesting %u HZ "
-					"carrier\n", __func__, carrier);
+				dev_dbg(ir->dev, "requesting %u HZ carrier",
+								carrier);
 
 				/* Transmit new carrier to mce device */
 				mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
@@ -967,7 +960,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);
 
-			mce_dbg(ir->dev, "Storing %s with duration %d\n",
+			dev_dbg(ir->dev, "Storing %s with duration %d",
 				rawir.pulse ? "pulse" : "space",
 				rawir.duration);
 
@@ -1001,7 +994,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 			ir->parser_state = CMD_HEADER;
 	}
 	if (event) {
-		mce_dbg(ir->dev, "processed IR data, calling ir_raw_event_handle\n");
+		dev_dbg(ir->dev, "processed IR data");
 		ir_raw_event_handle(ir->rc);
 	}
 }
@@ -1027,13 +1020,14 @@ static void mceusb_dev_recv(struct urb *urb)
 
 	case -ECONNRESET:
 	case -ENOENT:
+	case -EILSEQ:
 	case -ESHUTDOWN:
 		usb_unlink_urb(urb);
 		return;
 
 	case -EPIPE:
 	default:
-		mce_dbg(ir->dev, "Error: urb status = %d\n", urb->status);
+		dev_err(ir->dev, "Error: urb status = %d", urb->status);
 		break;
 	}
 
@@ -1055,7 +1049,7 @@ static void mceusb_gen1_init(struct mceusb_dev *ir)
 
 	data = kzalloc(USB_CTRL_MSG_SZ, GFP_KERNEL);
 	if (!data) {
-		dev_err(dev, "%s: memory allocation failed!\n", __func__);
+		dev_err(dev, "%s: memory allocation failed!", __func__);
 		return;
 	}
 
@@ -1066,28 +1060,28 @@ 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);
-	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]);
+	dev_dbg(dev, "set address - ret = %d", ret);
+	dev_dbg(dev, "set address - data[0] = %d, data[1] = %d",
+						data[0], data[1]);
 
 	/* set feature: bit rate 38400 bps */
 	ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0),
 			      USB_REQ_SET_FEATURE, USB_TYPE_VENDOR,
 			      0xc04e, 0x0000, NULL, 0, HZ * 3);
 
-	mce_dbg(dev, "%s - ret = %d\n", __func__, ret);
+	dev_dbg(dev, "set feature - ret = %d", 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);
-	mce_dbg(dev, "%s - retB = %d\n", __func__, ret);
+	dev_dbg(dev, "set char length - retB = %d", 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);
-	mce_dbg(dev, "%s - retC = %d\n", __func__, ret);
+	dev_dbg(dev, "set handshake  - retC = %d", ret);
 
 	/* device resume */
 	mce_async_out(ir, DEVICE_RESUME, sizeof(DEVICE_RESUME));
@@ -1158,7 +1152,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir)
 
 	rc = rc_allocate_device();
 	if (!rc) {
-		dev_err(dev, "remote dev allocation failed\n");
+		dev_err(dev, "remote dev allocation failed");
 		goto out;
 	}
 
@@ -1190,7 +1184,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir)
 
 	ret = rc_register_device(rc);
 	if (ret < 0) {
-		dev_err(dev, "remote dev registration failed\n");
+		dev_err(dev, "remote dev registration failed");
 		goto out;
 	}
 
@@ -1218,7 +1212,7 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 	bool tx_mask_normal;
 	int ir_intfnum;
 
-	mce_dbg(&intf->dev, "%s called\n", __func__);
+	dev_dbg(&intf->dev, "%s called", __func__);
 
 	idesc  = intf->cur_altsetting;
 
@@ -1246,8 +1240,7 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 			ep_in = ep;
 			ep_in->bmAttributes = USB_ENDPOINT_XFER_INT;
 			ep_in->bInterval = 1;
-			mce_dbg(&intf->dev, "acceptable inbound endpoint "
-				"found\n");
+			dev_dbg(&intf->dev, "acceptable inbound endpoint found");
 		}
 
 		if ((ep_out == NULL)
@@ -1261,12 +1254,11 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 			ep_out = ep;
 			ep_out->bmAttributes = USB_ENDPOINT_XFER_INT;
 			ep_out->bInterval = 1;
-			mce_dbg(&intf->dev, "acceptable outbound endpoint "
-				"found\n");
+			dev_dbg(&intf->dev, "acceptable outbound endpoint found");
 		}
 	}
 	if (ep_in == NULL) {
-		mce_dbg(&intf->dev, "inbound and/or endpoint not found\n");
+		dev_dbg(&intf->dev, "inbound and/or endpoint not found");
 		return -ENODEV;
 	}
 
@@ -1314,7 +1306,7 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 
 	res = usb_submit_urb(ir->urb_in, GFP_KERNEL);
 	if (res) {
-		dev_err(&intf->dev, "failed to submit urb: %d\n", res);
+		dev_err(&intf->dev, "failed to submit urb: %d", res);
 		goto usb_submit_fail;
 	}
 
@@ -1344,10 +1336,9 @@ static int mceusb_dev_probe(struct usb_interface *intf,
 	device_set_wakeup_capable(ir->dev, true);
 	device_set_wakeup_enable(ir->dev, true);
 
-	dev_info(&intf->dev, "Registered %s with mce emulator interface "
-		 "version %x\n", name, ir->emver);
-	dev_info(&intf->dev, "%x tx ports (0x%x cabled) and "
-		 "%x rx sensors (0x%x active)\n",
+	dev_info(&intf->dev, "Registered %s with mce emulator interface version %x",
+		name, ir->emver);
+	dev_info(&intf->dev, "%x tx ports (0x%x cabled) and %x rx sensors (0x%x active)",
 		 ir->num_txports, ir->txports_cabled,
 		 ir->num_rxports, ir->rxports_active);
 
@@ -1363,7 +1354,7 @@ urb_in_alloc_fail:
 buf_in_alloc_fail:
 	kfree(ir);
 mem_alloc_fail:
-	dev_err(&intf->dev, "%s: device setup failed!\n", __func__);
+	dev_err(&intf->dev, "%s: device setup failed!", __func__);
 
 	return -ENOMEM;
 }
@@ -1391,7 +1382,7 @@ static void mceusb_dev_disconnect(struct usb_interface *intf)
 static int mceusb_dev_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct mceusb_dev *ir = usb_get_intfdata(intf);
-	dev_info(ir->dev, "suspend\n");
+	dev_info(ir->dev, "suspend");
 	usb_kill_urb(ir->urb_in);
 	return 0;
 }
@@ -1399,7 +1390,7 @@ static int mceusb_dev_suspend(struct usb_interface *intf, pm_message_t message)
 static int mceusb_dev_resume(struct usb_interface *intf)
 {
 	struct mceusb_dev *ir = usb_get_intfdata(intf);
-	dev_info(ir->dev, "resume\n");
+	dev_info(ir->dev, "resume");
 	if (usb_submit_urb(ir->urb_in, GFP_ATOMIC))
 		return -EIO;
 	return 0;
@@ -1421,6 +1412,3 @@ MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(usb, mceusb_dev_table);
-
-module_param(debug, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Debug enabled or not");
-- 
1.8.4.2

Re: [PATCH 3/4] [media] mceusb: remove redundant function and defines

[Mauro Carvalho Chehab]

Hi Sean,

Em Mon, 20 Jan 2014 22:10:43 +0000
Sean Young <sean@mess.org> escreveu:

Could you please provide a patch description? Even simple ones should have,
and this one is everything but trivial...

Also, you should likely break it into smaller changesets. For example, the
last hunk adding a usb_kill_urb() looks more like a bugfix than a pure
cleanup change.

Thanks!
Mauro

> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  drivers/media/rc/mceusb.c | 92 +++++++++++++++--------------------------------
>  1 file changed, 28 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
> index a25bb15..3a4f95f 100644
> --- a/drivers/media/rc/mceusb.c
> +++ b/drivers/media/rc/mceusb.c
> @@ -166,15 +166,6 @@ static bool debug;
>  			dev_info(dev, fmt, ## __VA_ARGS__);	\
>  	} while (0)
>  
> -/* general constants */
> -#define SEND_FLAG_IN_PROGRESS	1
> -#define SEND_FLAG_COMPLETE	2
> -#define RECV_FLAG_IN_PROGRESS	3
> -#define RECV_FLAG_COMPLETE	4
> -
> -#define MCEUSB_RX		1
> -#define MCEUSB_TX		2
> -
>  #define VENDOR_PHILIPS		0x0471
>  #define VENDOR_SMK		0x0609
>  #define VENDOR_TATUNG		0x1460
> @@ -452,7 +443,6 @@ struct mceusb_dev {
>  	} flags;
>  
>  	/* transmit support */
> -	int send_flags;
>  	u32 carrier;
>  	unsigned char tx_mask;
>  
> @@ -731,45 +721,29 @@ static void mce_async_callback(struct urb *urb)
>  
>  /* request incoming or send outgoing usb packet - used to initialize remote */
>  static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
> -			       int size, int urb_type)
> +			       int size)
>  {
>  	int res, pipe;
>  	struct urb *async_urb;
>  	struct device *dev = ir->dev;
>  	unsigned char *async_buf;
>  
> -	if (urb_type == MCEUSB_TX) {
> -		async_urb = usb_alloc_urb(0, GFP_KERNEL);
> -		if (unlikely(!async_urb)) {
> -			dev_err(dev, "Error, couldn't allocate urb!\n");
> -			return;
> -		}
> -
> -		async_buf = kzalloc(size, GFP_KERNEL);
> -		if (!async_buf) {
> -			dev_err(dev, "Error, couldn't allocate buf!\n");
> -			usb_free_urb(async_urb);
> -			return;
> -		}
> +	async_urb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (unlikely(!async_urb))
> +		return;
>  
> -		/* outbound data */
> -		pipe = usb_sndintpipe(ir->usbdev,
> -				      ir->usb_ep_out->bEndpointAddress);
> -		usb_fill_int_urb(async_urb, ir->usbdev, pipe,
> -			async_buf, size, mce_async_callback,
> -			ir, ir->usb_ep_out->bInterval);
> -		memcpy(async_buf, data, size);
> -
> -	} else if (urb_type == MCEUSB_RX) {
> -		/* standard request */
> -		async_urb = ir->urb_in;
> -		ir->send_flags = RECV_FLAG_IN_PROGRESS;
> -
> -	} else {
> -		dev_err(dev, "Error! Unknown urb type %d\n", urb_type);
> +	async_buf = kmalloc(size, GFP_KERNEL);
> +	if (!async_buf) {
> +		usb_free_urb(async_urb);
>  		return;
>  	}
>  
> +	/* outbound data */
> +	pipe = usb_sndintpipe(ir->usbdev, ir->usb_ep_out->bEndpointAddress);
> +	usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf, size,
> +			mce_async_callback, ir, ir->usb_ep_out->bInterval);
> +	memcpy(async_buf, data, size);
> +
>  	mce_dbg(dev, "receive request called (size=%#x)\n", size);
>  
>  	async_urb->transfer_buffer_length = size;
> @@ -789,19 +763,14 @@ static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size)
>  
>  	if (ir->need_reset) {
>  		ir->need_reset = false;
> -		mce_request_packet(ir, DEVICE_RESUME, rsize, MCEUSB_TX);
> +		mce_request_packet(ir, DEVICE_RESUME, rsize);
>  		msleep(10);
>  	}
>  
> -	mce_request_packet(ir, data, size, MCEUSB_TX);
> +	mce_request_packet(ir, data, size);
>  	msleep(10);
>  }
>  
> -static void mce_flush_rx_buffer(struct mceusb_dev *ir, int size)
> -{
> -	mce_request_packet(ir, NULL, size, MCEUSB_RX);
> -}
> -
>  /* Send data out the IR blaster port(s) */
>  static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  {
> @@ -1040,7 +1009,6 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
>  static void mceusb_dev_recv(struct urb *urb)
>  {
>  	struct mceusb_dev *ir;
> -	int buf_len;
>  
>  	if (!urb)
>  		return;
> @@ -1051,18 +1019,10 @@ static void mceusb_dev_recv(struct urb *urb)
>  		return;
>  	}
>  
> -	buf_len = urb->actual_length;
> -
> -	if (ir->send_flags == RECV_FLAG_IN_PROGRESS) {
> -		ir->send_flags = SEND_FLAG_COMPLETE;
> -		mce_dbg(ir->dev, "setup answer received %d bytes\n",
> -			buf_len);
> -	}
> -
>  	switch (urb->status) {
>  	/* success */
>  	case 0:
> -		mceusb_process_ir_data(ir, buf_len);
> +		mceusb_process_ir_data(ir, urb->actual_length);
>  		break;
>  
>  	case -ECONNRESET:
> @@ -1250,7 +1210,7 @@ static int mceusb_dev_probe(struct usb_interface *intf,
>  	struct usb_endpoint_descriptor *ep_in = NULL;
>  	struct usb_endpoint_descriptor *ep_out = NULL;
>  	struct mceusb_dev *ir = NULL;
> -	int pipe, maxp, i;
> +	int pipe, maxp, i, res;
>  	char buf[63], name[128] = "";
>  	enum mceusb_model_type model = id->driver_info;
>  	bool is_gen3;
> @@ -1346,19 +1306,21 @@ static int mceusb_dev_probe(struct usb_interface *intf,
>  		snprintf(name + strlen(name), sizeof(name) - strlen(name),
>  			 " %s", buf);
>  
> -	ir->rc = mceusb_init_rc_dev(ir);
> -	if (!ir->rc)
> -		goto rc_dev_fail;
> -
>  	/* wire up inbound data handler */
>  	usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
>  				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);
> +	res = usb_submit_urb(ir->urb_in, GFP_KERNEL);
> +	if (res) {
> +		dev_err(&intf->dev, "failed to submit urb: %d\n", res);
> +		goto usb_submit_fail;
> +	}
> +
> +	ir->rc = mceusb_init_rc_dev(ir);
> +	if (!ir->rc)
> +		goto rc_dev_fail;
>  
>  	/* figure out which firmware/emulator version this hardware has */
>  	mceusb_get_emulator_version(ir);
> @@ -1393,6 +1355,8 @@ static int mceusb_dev_probe(struct usb_interface *intf,
>  
>  	/* Error-handling path */
>  rc_dev_fail:
> +	usb_kill_urb(ir->urb_in);
> +usb_submit_fail:
>  	usb_free_urb(ir->urb_in);
>  urb_in_alloc_fail:
>  	usb_free_coherent(dev, maxp, ir->buf_in, ir->dma_in);


-- 

Cheers,
Mauro