Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[media] mceusb: improve error logging

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.

[m.chehab@samsung.com: patch rebased, as one of the patches on this
series need changes]
Signed-off-by: Sean Young <sean@mess.org>

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

authored by

Sean Young and committed by
Mauro Carvalho Chehab
6b4a16c3 776eced0

+85 -97
+85 -97
drivers/media/rc/mceusb.c
··· 84 84 #define MCE_PORT_IR 0x4 /* (0x4 << 5) | MCE_CMD = 0x9f */ 85 85 #define MCE_PORT_SYS 0x7 /* (0x7 << 5) | MCE_CMD = 0xff */ 86 86 #define MCE_PORT_SER 0x6 /* 0xc0 thru 0xdf flush & 0x1f bytes */ 87 - #define MCE_PORT_MASK 0xe0 /* Mask out command bits */ 87 + #define MCE_PORT_MASK 0xe0 /* Mask out command bits */ 88 88 89 89 /* Command port headers */ 90 90 #define MCE_CMD_PORT_IR 0x9f /* IR-related cmd/rsp */ ··· 152 152 * then we're looking at a raw IR data sample */ 153 153 #define MCE_COMMAND_IRDATA 0x80 154 154 #define MCE_PACKET_LENGTH_MASK 0x1f /* Packet length mask */ 155 - 156 - /* module parameters */ 157 - #ifdef CONFIG_USB_DEBUG 158 - static bool debug = 1; 159 - #else 160 - static bool debug; 161 - #endif 162 - 163 - #define mce_dbg(dev, fmt, ...) \ 164 - do { \ 165 - if (debug) \ 166 - dev_info(dev, fmt, ## __VA_ARGS__); \ 167 - } while (0) 168 155 169 156 /* general constants */ 170 157 #define SEND_FLAG_IN_PROGRESS 1 ··· 528 541 static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf, 529 542 int offset, int len, bool out) 530 543 { 531 - char codes[USB_BUFLEN * 3 + 1]; 532 - char inout[9]; 544 + #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) 545 + char *inout; 533 546 u8 cmd, subcmd, data1, data2, data3, data4; 534 547 struct device *dev = ir->dev; 535 - int i, start, skip = 0; 548 + int start, skip = 0; 536 549 u32 carrier, period; 537 - 538 - if (!debug) 539 - return; 540 550 541 551 /* skip meaningless 0xb1 0x60 header bytes on orig receiver */ 542 552 if (ir->flags.microsoft_gen1 && !out && !offset) ··· 542 558 if (len <= skip) 543 559 return; 544 560 545 - for (i = 0; i < len && i < USB_BUFLEN; i++) 546 - snprintf(codes + i * 3, 4, "%02x ", buf[i + offset] & 0xff); 561 + dev_dbg(dev, "%cx data: %*ph (length=%d)", 562 + (out ? 't' : 'r'), min(len, USB_BUFLEN), buf, len); 547 563 548 - dev_info(dev, "%sx data: %s(length=%d)\n", 549 - (out ? "t" : "r"), codes, len); 550 - 551 - if (out) 552 - strcpy(inout, "Request\0"); 553 - else 554 - strcpy(inout, "Got\0"); 564 + inout = out ? "Request" : "Got"; 555 565 556 566 start = offset + skip; 557 567 cmd = buf[start] & 0xff; ··· 561 583 break; 562 584 if ((subcmd == MCE_CMD_PORT_SYS) && 563 585 (data1 == MCE_CMD_RESUME)) 564 - dev_info(dev, "Device resume requested\n"); 586 + dev_dbg(dev, "Device resume requested"); 565 587 else 566 - dev_info(dev, "Unknown command 0x%02x 0x%02x\n", 588 + dev_dbg(dev, "Unknown command 0x%02x 0x%02x", 567 589 cmd, subcmd); 568 590 break; 569 591 case MCE_CMD_PORT_SYS: 570 592 switch (subcmd) { 571 593 case MCE_RSP_EQEMVER: 572 594 if (!out) 573 - dev_info(dev, "Emulator interface version %x\n", 595 + dev_dbg(dev, "Emulator interface version %x", 574 596 data1); 575 597 break; 576 598 case MCE_CMD_G_REVISION: 577 599 if (len == 2) 578 - dev_info(dev, "Get hw/sw rev?\n"); 600 + dev_dbg(dev, "Get hw/sw rev?"); 579 601 else 580 - dev_info(dev, "hw/sw rev 0x%02x 0x%02x " 581 - "0x%02x 0x%02x\n", data1, data2, 602 + dev_dbg(dev, "hw/sw rev 0x%02x 0x%02x 0x%02x 0x%02x", 603 + data1, data2, 582 604 buf[start + 4], buf[start + 5]); 583 605 break; 584 606 case MCE_CMD_RESUME: 585 - dev_info(dev, "Device resume requested\n"); 607 + dev_dbg(dev, "Device resume requested"); 586 608 break; 587 609 case MCE_RSP_CMD_ILLEGAL: 588 - dev_info(dev, "Illegal PORT_SYS command\n"); 610 + dev_dbg(dev, "Illegal PORT_SYS command"); 589 611 break; 590 612 case MCE_RSP_EQWAKEVERSION: 591 613 if (!out) 592 - dev_info(dev, "Wake version, proto: 0x%02x, " 614 + dev_dbg(dev, "Wake version, proto: 0x%02x, " 593 615 "payload: 0x%02x, address: 0x%02x, " 594 - "version: 0x%02x\n", 616 + "version: 0x%02x", 595 617 data1, data2, data3, data4); 596 618 break; 597 619 case MCE_RSP_GETPORTSTATUS: 598 620 if (!out) 599 621 /* We use data1 + 1 here, to match hw labels */ 600 - dev_info(dev, "TX port %d: blaster is%s connected\n", 622 + dev_dbg(dev, "TX port %d: blaster is%s connected", 601 623 data1 + 1, data4 ? " not" : ""); 602 624 break; 603 625 case MCE_CMD_FLASHLED: 604 - dev_info(dev, "Attempting to flash LED\n"); 626 + dev_dbg(dev, "Attempting to flash LED"); 605 627 break; 606 628 default: 607 - dev_info(dev, "Unknown command 0x%02x 0x%02x\n", 629 + dev_dbg(dev, "Unknown command 0x%02x 0x%02x", 608 630 cmd, subcmd); 609 631 break; 610 632 } ··· 612 634 case MCE_CMD_PORT_IR: 613 635 switch (subcmd) { 614 636 case MCE_CMD_SIG_END: 615 - dev_info(dev, "End of signal\n"); 637 + dev_dbg(dev, "End of signal"); 616 638 break; 617 639 case MCE_CMD_PING: 618 - dev_info(dev, "Ping\n"); 640 + dev_dbg(dev, "Ping"); 619 641 break; 620 642 case MCE_CMD_UNKNOWN: 621 - dev_info(dev, "Resp to 9f 05 of 0x%02x 0x%02x\n", 643 + dev_dbg(dev, "Resp to 9f 05 of 0x%02x 0x%02x", 622 644 data1, data2); 623 645 break; 624 646 case MCE_RSP_EQIRCFS: ··· 627 649 if (!period) 628 650 break; 629 651 carrier = (1000 * 1000) / period; 630 - dev_info(dev, "%s carrier of %u Hz (period %uus)\n", 652 + dev_dbg(dev, "%s carrier of %u Hz (period %uus)", 631 653 inout, carrier, period); 632 654 break; 633 655 case MCE_CMD_GETIRCFS: 634 - dev_info(dev, "Get carrier mode and freq\n"); 656 + dev_dbg(dev, "Get carrier mode and freq"); 635 657 break; 636 658 case MCE_RSP_EQIRTXPORTS: 637 - dev_info(dev, "%s transmit blaster mask of 0x%02x\n", 659 + dev_dbg(dev, "%s transmit blaster mask of 0x%02x", 638 660 inout, data1); 639 661 break; 640 662 case MCE_RSP_EQIRTIMEOUT: 641 663 /* value is in units of 50us, so x*50/1000 ms */ 642 664 period = ((data1 << 8) | data2) * MCE_TIME_UNIT / 1000; 643 - dev_info(dev, "%s receive timeout of %d ms\n", 665 + dev_dbg(dev, "%s receive timeout of %d ms", 644 666 inout, period); 645 667 break; 646 668 case MCE_CMD_GETIRTIMEOUT: 647 - dev_info(dev, "Get receive timeout\n"); 669 + dev_dbg(dev, "Get receive timeout"); 648 670 break; 649 671 case MCE_CMD_GETIRTXPORTS: 650 - dev_info(dev, "Get transmit blaster mask\n"); 672 + dev_dbg(dev, "Get transmit blaster mask"); 651 673 break; 652 674 case MCE_RSP_EQIRRXPORTEN: 653 - dev_info(dev, "%s %s-range receive sensor in use\n", 675 + dev_dbg(dev, "%s %s-range receive sensor in use", 654 676 inout, data1 == 0x02 ? "short" : "long"); 655 677 break; 656 678 case MCE_CMD_GETIRRXPORTEN: 657 679 /* aka MCE_RSP_EQIRRXCFCNT */ 658 680 if (out) 659 - dev_info(dev, "Get receive sensor\n"); 681 + dev_dbg(dev, "Get receive sensor"); 660 682 else if (ir->learning_enabled) 661 - dev_info(dev, "RX pulse count: %d\n", 683 + dev_dbg(dev, "RX pulse count: %d", 662 684 ((data1 << 8) | data2)); 663 685 break; 664 686 case MCE_RSP_EQIRNUMPORTS: 665 687 if (out) 666 688 break; 667 - dev_info(dev, "Num TX ports: %x, num RX ports: %x\n", 689 + dev_dbg(dev, "Num TX ports: %x, num RX ports: %x", 668 690 data1, data2); 669 691 break; 670 692 case MCE_RSP_CMD_ILLEGAL: 671 - dev_info(dev, "Illegal PORT_IR command\n"); 693 + dev_dbg(dev, "Illegal PORT_IR command"); 672 694 break; 673 695 default: 674 - dev_info(dev, "Unknown command 0x%02x 0x%02x\n", 696 + dev_dbg(dev, "Unknown command 0x%02x 0x%02x", 675 697 cmd, subcmd); 676 698 break; 677 699 } ··· 681 703 } 682 704 683 705 if (cmd == MCE_IRDATA_TRAILER) 684 - dev_info(dev, "End of raw IR data\n"); 706 + dev_dbg(dev, "End of raw IR data"); 685 707 else if ((cmd != MCE_CMD_PORT_IR) && 686 708 ((cmd & MCE_PORT_MASK) == MCE_COMMAND_IRDATA)) 687 - dev_info(dev, "Raw IR data, %d pulse/space samples\n", ir->rem); 709 + dev_dbg(dev, "Raw IR data, %d pulse/space samples", ir->rem); 710 + #endif 688 711 } 689 712 690 713 static void mce_async_callback(struct urb *urb) ··· 697 718 return; 698 719 699 720 ir = urb->context; 700 - if (ir) { 721 + 722 + switch (urb->status) { 723 + /* success */ 724 + case 0: 701 725 len = urb->actual_length; 702 726 703 727 mceusb_dev_printdata(ir, urb->transfer_buffer, 0, len, true); 728 + break; 729 + 730 + case -ECONNRESET: 731 + case -ENOENT: 732 + case -EILSEQ: 733 + case -ESHUTDOWN: 734 + break; 735 + 736 + case -EPIPE: 737 + default: 738 + dev_err(ir->dev, "Error: request urb status = %d", urb->status); 739 + break; 704 740 } 705 741 706 742 /* the transfer buffer and urb were allocated in mce_request_packet */ ··· 764 770 return; 765 771 } 766 772 767 - mce_dbg(dev, "receive request called (size=%#x)\n", size); 773 + dev_dbg(dev, "receive request called (size=%#x)", size); 768 774 769 775 async_urb->transfer_buffer_length = size; 770 776 async_urb->dev = ir->usbdev; 771 777 772 778 res = usb_submit_urb(async_urb, GFP_ATOMIC); 773 779 if (res) { 774 - mce_dbg(dev, "receive request FAILED! (res=%d)\n", res); 780 + dev_err(dev, "receive request FAILED! (res=%d)", res); 775 781 return; 776 782 } 777 - mce_dbg(dev, "receive request complete (res=%d)\n", res); 783 + dev_dbg(dev, "receive request complete (res=%d)", res); 778 784 } 779 785 780 786 static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size) ··· 889 895 ir->carrier = carrier; 890 896 cmdbuf[2] = MCE_CMD_SIG_END; 891 897 cmdbuf[3] = MCE_IRDATA_TRAILER; 892 - mce_dbg(ir->dev, "%s: disabling carrier " 893 - "modulation\n", __func__); 898 + dev_dbg(ir->dev, "disabling carrier modulation"); 894 899 mce_async_out(ir, cmdbuf, sizeof(cmdbuf)); 895 900 return carrier; 896 901 } ··· 900 907 ir->carrier = carrier; 901 908 cmdbuf[2] = prescaler; 902 909 cmdbuf[3] = divisor; 903 - mce_dbg(ir->dev, "%s: requesting %u HZ " 904 - "carrier\n", __func__, carrier); 910 + dev_dbg(ir->dev, "requesting %u HZ carrier", 911 + carrier); 905 912 906 913 /* Transmit new carrier to mce device */ 907 914 mce_async_out(ir, cmdbuf, sizeof(cmdbuf)); ··· 991 998 rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK) 992 999 * US_TO_NS(MCE_TIME_UNIT); 993 1000 994 - mce_dbg(ir->dev, "Storing %s with duration %d\n", 1001 + dev_dbg(ir->dev, "Storing %s with duration %d", 995 1002 rawir.pulse ? "pulse" : "space", 996 1003 rawir.duration); 997 1004 ··· 1025 1032 ir->parser_state = CMD_HEADER; 1026 1033 } 1027 1034 if (event) { 1028 - mce_dbg(ir->dev, "processed IR data, calling ir_raw_event_handle\n"); 1035 + dev_dbg(ir->dev, "processed IR data"); 1029 1036 ir_raw_event_handle(ir->rc); 1030 1037 } 1031 1038 } ··· 1048 1055 1049 1056 if (ir->send_flags == RECV_FLAG_IN_PROGRESS) { 1050 1057 ir->send_flags = SEND_FLAG_COMPLETE; 1051 - mce_dbg(ir->dev, "setup answer received %d bytes\n", 1058 + dev_dbg(ir->dev, "setup answer received %d bytes\n", 1052 1059 buf_len); 1053 1060 } 1054 1061 ··· 1060 1067 1061 1068 case -ECONNRESET: 1062 1069 case -ENOENT: 1070 + case -EILSEQ: 1063 1071 case -ESHUTDOWN: 1064 1072 usb_unlink_urb(urb); 1065 1073 return; 1066 1074 1067 1075 case -EPIPE: 1068 1076 default: 1069 - mce_dbg(ir->dev, "Error: urb status = %d\n", urb->status); 1077 + dev_err(ir->dev, "Error: urb status = %d", urb->status); 1070 1078 break; 1071 1079 } 1072 1080 ··· 1089 1095 1090 1096 data = kzalloc(USB_CTRL_MSG_SZ, GFP_KERNEL); 1091 1097 if (!data) { 1092 - dev_err(dev, "%s: memory allocation failed!\n", __func__); 1098 + dev_err(dev, "%s: memory allocation failed!", __func__); 1093 1099 return; 1094 1100 } 1095 1101 ··· 1100 1106 ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0), 1101 1107 USB_REQ_SET_ADDRESS, USB_TYPE_VENDOR, 0, 0, 1102 1108 data, USB_CTRL_MSG_SZ, HZ * 3); 1103 - mce_dbg(dev, "%s - ret = %d\n", __func__, ret); 1104 - mce_dbg(dev, "%s - data[0] = %d, data[1] = %d\n", 1105 - __func__, data[0], data[1]); 1109 + dev_dbg(dev, "set address - ret = %d", ret); 1110 + dev_dbg(dev, "set address - data[0] = %d, data[1] = %d", 1111 + data[0], data[1]); 1106 1112 1107 1113 /* set feature: bit rate 38400 bps */ 1108 1114 ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0), 1109 1115 USB_REQ_SET_FEATURE, USB_TYPE_VENDOR, 1110 1116 0xc04e, 0x0000, NULL, 0, HZ * 3); 1111 1117 1112 - mce_dbg(dev, "%s - ret = %d\n", __func__, ret); 1118 + dev_dbg(dev, "set feature - ret = %d", ret); 1113 1119 1114 1120 /* bRequest 4: set char length to 8 bits */ 1115 1121 ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0), 1116 1122 4, USB_TYPE_VENDOR, 1117 1123 0x0808, 0x0000, NULL, 0, HZ * 3); 1118 - mce_dbg(dev, "%s - retB = %d\n", __func__, ret); 1124 + dev_dbg(dev, "set char length - retB = %d", ret); 1119 1125 1120 1126 /* bRequest 2: set handshaking to use DTR/DSR */ 1121 1127 ret = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0), 1122 1128 2, USB_TYPE_VENDOR, 1123 1129 0x0000, 0x0100, NULL, 0, HZ * 3); 1124 - mce_dbg(dev, "%s - retC = %d\n", __func__, ret); 1130 + dev_dbg(dev, "set handshake - retC = %d", ret); 1125 1131 1126 1132 /* device resume */ 1127 1133 mce_async_out(ir, DEVICE_RESUME, sizeof(DEVICE_RESUME)); ··· 1192 1198 1193 1199 rc = rc_allocate_device(); 1194 1200 if (!rc) { 1195 - dev_err(dev, "remote dev allocation failed\n"); 1201 + dev_err(dev, "remote dev allocation failed"); 1196 1202 goto out; 1197 1203 } 1198 1204 ··· 1224 1230 1225 1231 ret = rc_register_device(rc); 1226 1232 if (ret < 0) { 1227 - dev_err(dev, "remote dev registration failed\n"); 1233 + dev_err(dev, "remote dev registration failed"); 1228 1234 goto out; 1229 1235 } 1230 1236 ··· 1252 1258 bool tx_mask_normal; 1253 1259 int ir_intfnum; 1254 1260 1255 - mce_dbg(&intf->dev, "%s called\n", __func__); 1261 + dev_dbg(&intf->dev, "%s called", __func__); 1256 1262 1257 1263 idesc = intf->cur_altsetting; 1258 1264 ··· 1280 1286 ep_in = ep; 1281 1287 ep_in->bmAttributes = USB_ENDPOINT_XFER_INT; 1282 1288 ep_in->bInterval = 1; 1283 - mce_dbg(&intf->dev, "acceptable inbound endpoint " 1284 - "found\n"); 1289 + dev_dbg(&intf->dev, "acceptable inbound endpoint found"); 1285 1290 } 1286 1291 1287 1292 if ((ep_out == NULL) ··· 1294 1301 ep_out = ep; 1295 1302 ep_out->bmAttributes = USB_ENDPOINT_XFER_INT; 1296 1303 ep_out->bInterval = 1; 1297 - mce_dbg(&intf->dev, "acceptable outbound endpoint " 1298 - "found\n"); 1304 + dev_dbg(&intf->dev, "acceptable outbound endpoint found"); 1299 1305 } 1300 1306 } 1301 1307 if (ep_in == NULL) { 1302 - mce_dbg(&intf->dev, "inbound and/or endpoint not found\n"); 1308 + dev_dbg(&intf->dev, "inbound and/or endpoint not found"); 1303 1309 return -ENODEV; 1304 1310 } 1305 1311 ··· 1349 1357 ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; 1350 1358 1351 1359 /* flush buffers on the device */ 1352 - mce_dbg(&intf->dev, "Flushing receive buffers\n"); 1360 + dev_dbg(&intf->dev, "Flushing receive buffers\n"); 1353 1361 mce_flush_rx_buffer(ir, maxp); 1354 1362 1355 1363 /* figure out which firmware/emulator version this hardware has */ ··· 1374 1382 device_set_wakeup_capable(ir->dev, true); 1375 1383 device_set_wakeup_enable(ir->dev, true); 1376 1384 1377 - dev_info(&intf->dev, "Registered %s with mce emulator interface " 1378 - "version %x\n", name, ir->emver); 1379 - dev_info(&intf->dev, "%x tx ports (0x%x cabled) and " 1380 - "%x rx sensors (0x%x active)\n", 1385 + dev_info(&intf->dev, "Registered %s with mce emulator interface version %x", 1386 + name, ir->emver); 1387 + dev_info(&intf->dev, "%x tx ports (0x%x cabled) and %x rx sensors (0x%x active)", 1381 1388 ir->num_txports, ir->txports_cabled, 1382 1389 ir->num_rxports, ir->rxports_active); 1383 1390 ··· 1390 1399 buf_in_alloc_fail: 1391 1400 kfree(ir); 1392 1401 mem_alloc_fail: 1393 - dev_err(&intf->dev, "%s: device setup failed!\n", __func__); 1402 + dev_err(&intf->dev, "%s: device setup failed!", __func__); 1394 1403 1395 1404 return -ENOMEM; 1396 1405 } ··· 1418 1427 static int mceusb_dev_suspend(struct usb_interface *intf, pm_message_t message) 1419 1428 { 1420 1429 struct mceusb_dev *ir = usb_get_intfdata(intf); 1421 - dev_info(ir->dev, "suspend\n"); 1430 + dev_info(ir->dev, "suspend"); 1422 1431 usb_kill_urb(ir->urb_in); 1423 1432 return 0; 1424 1433 } ··· 1426 1435 static int mceusb_dev_resume(struct usb_interface *intf) 1427 1436 { 1428 1437 struct mceusb_dev *ir = usb_get_intfdata(intf); 1429 - dev_info(ir->dev, "resume\n"); 1438 + dev_info(ir->dev, "resume"); 1430 1439 if (usb_submit_urb(ir->urb_in, GFP_ATOMIC)) 1431 1440 return -EIO; 1432 1441 return 0; ··· 1448 1457 MODULE_AUTHOR(DRIVER_AUTHOR); 1449 1458 MODULE_LICENSE("GPL"); 1450 1459 MODULE_DEVICE_TABLE(usb, mceusb_dev_table); 1451 - 1452 - module_param(debug, bool, S_IRUGO | S_IWUSR); 1453 - MODULE_PARM_DESC(debug, "Debug enabled or not");