[PATCH v4 0/2] IR driver for USB-UIRT device

[PATCH v4 0/2] IR driver for USB-UIRT device

[Sean Young]

This is a new rc-core driver for the USB-UIRT which you can see here
http://www.usbuirt.com/

This device is supported in lirc, via the usb serial kernel driver. This
driver is both for rc-core, which means it can use kernel/BPF decoding
ec. Also this implement is superior because it can:
 - support learning mode
 - setting transmit carrier
 - larger transmits using streaming tx command

Changes since v3:
 - Review comments from Johan Hovold
 - Do not move the ftdi_sio.h file an copy FTDI_* definitions instead

Changes since v2:
 - Fixed race condition is disconnect
 - Removed superfluous kmalloc in short tx

Changes since v1:
 - Review comments from Oliver Neukum
 - Simplified wideband read function

Sean Young (2):
  media: rc: new driver for USB-UIRT device
  USB: serial: blacklist USB-UIRT when driver is selected

 drivers/media/rc/Kconfig      |  11 +
 drivers/media/rc/Makefile     |   1 +
 drivers/media/rc/uirt.c       | 740 ++++++++++++++++++++++++++++++++++
 drivers/usb/serial/ftdi_sio.c |   6 +-
 4 files changed, 756 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/rc/uirt.c

-- 
2.31.1

[PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Sean Young]

This device uses an ftdi usb serial port, so this driver has a tiny
amount of usb ftdi code. It would be preferable to connect this driver via
serdev or line-discipline, but unfortunately neither support
hotplugging yet.

See http://www.usbuirt.com/

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/Kconfig  |  11 +
 drivers/media/rc/Makefile |   1 +
 drivers/media/rc/uirt.c   | 740 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 752 insertions(+)
 create mode 100644 drivers/media/rc/uirt.c

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d0a8326b75c2..cb1a2c65f920 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -471,6 +471,17 @@ config IR_TOY
 	   To compile this driver as a module, choose M here: the module will be
 	   called ir_toy.
 
+config IR_UIRT
+	tristate "USB-UIRT"
+	depends on RC_CORE
+	depends on USB_ARCH_HAS_HCD
+	help
+	   Say Y here if you want to use the USB-UIRT. See
+	   http://www.usbuirt.com/
+
+	   To compile this driver as a module, choose M here: the module will be
+	   called uirt.
+
 endif #RC_DEVICES
 
 endif #RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 692e9b6b203f..068036698747 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -50,3 +50,4 @@ obj-$(CONFIG_IR_SIR) += sir_ir.o
 obj-$(CONFIG_IR_MTK) += mtk-cir.o
 obj-$(CONFIG_RC_XBOX_DVD) += xbox_remote.o
 obj-$(CONFIG_IR_TOY) += ir_toy.o
+obj-$(CONFIG_IR_UIRT) += uirt.o
diff --git a/drivers/media/rc/uirt.c b/drivers/media/rc/uirt.c
new file mode 100644
index 000000000000..975cf464113e
--- /dev/null
+++ b/drivers/media/rc/uirt.c
@@ -0,0 +1,740 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * USB-UIRT
+ *
+ * Copyright (C) 2021 Sean Young <sean@mess.org>
+ *
+ * See http://www.usbuirt.com/USB-UIRT%20Command%20Protocol.doc
+ */
+
+#include <linux/completion.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/slab.h>
+#include <linux/usb/input.h>
+
+#include <media/rc-core.h>
+
+/* Copied from drivers/usb/serial/ftdi_sio.h */
+#define FTDI_SIO_SET_BAUDRATE_REQUEST_TYPE 0x40
+#define FTDI_SIO_SET_BAUDRATE_REQUEST 3
+
+#define  FTDI_SIO_SET_LATENCY_TIMER_REQUEST FTDI_SIO_SET_LATENCY_TIMER
+#define  FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE 0x40
+
+#define FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE 0x40
+#define FTDI_SIO_SET_FLOW_CTRL_REQUEST FTDI_SIO_SET_FLOW_CTRL
+
+#define FTDI_SIO_SET_FLOW_CTRL		2 /* Set flow control register */
+#define FTDI_SIO_SET_LATENCY_TIMER	9 /* Set the latency timer */
+
+#define FTDI_SIO_RTS_CTS_HS	BIT(8)
+#define FTDI_RS0_CTS		BIT(4)
+
+static const u8 CMD_GETVERSION[] = { 0x23, 0xdd };
+static const u8 CMD_SETMODERAW[] = { 0x21, 0xdf };
+static const u8 CMD_SETWIDEBAND[] = { 0x24, 0xdc };
+
+#define UNIT_US 50
+#define IR_TIMEOUT 12500
+#define MAX_PACKET 64
+
+enum cmd_state {
+	CMD_STATE_GETVERSION,
+	CMD_STATE_SETMODERAW,
+	CMD_STATE_SETMODEWIDEBAND,
+	CMD_STATE_IRDATA,
+	CMD_STATE_DOTXRAW,
+	CMD_STATE_STREAMING_TX,
+};
+
+enum rx_state {
+	RX_STATE_INTERSPACE_HIGH,
+	RX_STATE_INTERSPACE_LOW,
+	RX_STATE_ON_HIGH,
+	RX_STATE_ON_LOW,
+	RX_STATE_FREQ_HIGH,
+	RX_STATE_FREQ_LOW,
+	RX_STATE_OFF_HIGH,
+	RX_STATE_OFF_LOW,
+};
+
+struct uirt {
+	struct device *dev;
+	struct usb_device *usbdev;
+
+	struct rc_dev *rc;
+	struct urb *urb_in, *urb_out;
+
+	u8 *in;
+	u8 *out;
+	struct completion cmd_done;
+	u8 freq;
+	u8 high;
+	bool wideband;
+	u32 last_duration;
+
+	enum cmd_state cmd_state;
+	enum rx_state rx_state;
+
+	void *tx_buf;
+	u32 tx_len;
+
+	char phys[64];
+};
+
+// read IR in raw mode
+static void uirt_raw_mode(struct uirt *uirt, u32 offset, u32 len)
+{
+	uint i, duration;
+
+	for (i = offset; i < len; i++) {
+		switch (uirt->rx_state) {
+		case RX_STATE_INTERSPACE_HIGH:
+			uirt->rx_state = RX_STATE_INTERSPACE_LOW;
+			break;
+		case RX_STATE_INTERSPACE_LOW:
+			uirt->rx_state = RX_STATE_ON_HIGH;
+			break;
+		case RX_STATE_ON_HIGH:
+			duration = uirt->in[i];
+			if (duration == 0)
+				duration = 1;
+
+			ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+				.duration = duration * UNIT_US,
+				.pulse = true,
+			}));
+
+			uirt->rx_state = RX_STATE_OFF_HIGH;
+			break;
+		case RX_STATE_OFF_HIGH:
+			if (uirt->in[i] == 0xff) {
+				ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+					.duration = IR_TIMEOUT,
+					.timeout = true,
+				}));
+				uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
+				break;
+			}
+
+			duration = uirt->in[i];
+			if (duration == 0)
+				duration = 1;
+
+			ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+				.duration = duration * UNIT_US,
+				.pulse = false,
+			}));
+			uirt->rx_state = RX_STATE_ON_HIGH;
+			break;
+		default:
+			WARN(1, "unreachable state");
+			uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
+			break;
+		}
+	}
+
+	ir_raw_event_handle(uirt->rc);
+}
+
+// Read IR in wideband mode. The byte stream is delivered in packets,
+// and the values which come in two bytes may straddle two packets
+static void uirt_wideband(struct uirt *uirt, u32 offset, u32 len)
+{
+	uint i, duration, carrier, pulses;
+
+	for (i = offset; i < len; i++) {
+		switch (uirt->rx_state) {
+		case RX_STATE_INTERSPACE_HIGH:
+			uirt->rx_state = RX_STATE_INTERSPACE_LOW;
+			break;
+		case RX_STATE_INTERSPACE_LOW:
+			uirt->rx_state = RX_STATE_ON_HIGH;
+			break;
+		case RX_STATE_ON_HIGH:
+			uirt->high = uirt->in[i];
+			uirt->rx_state = RX_STATE_ON_LOW;
+			break;
+		case RX_STATE_ON_LOW:
+			// duration is in 400ns units
+			duration = (uirt->high << 8) | uirt->in[i];
+			if (duration < 3)
+				duration = 3;
+			uirt->last_duration = duration;
+			ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+				.duration = DIV_ROUND_CLOSEST(duration * 2, 5),
+				.pulse = true,
+			}));
+			uirt->rx_state = RX_STATE_FREQ_HIGH;
+			break;
+		case RX_STATE_FREQ_HIGH:
+			if (uirt->in[i] & 0x80) {
+				uirt->high = uirt->in[i] & 0x7f;
+				uirt->rx_state = RX_STATE_FREQ_LOW;
+				break;
+			}
+
+			uirt->high = 0;
+			fallthrough;
+		case RX_STATE_FREQ_LOW:
+			pulses = (uirt->high << 8) | uirt->in[i];
+			if (pulses && uirt->last_duration) {
+				dev_dbg(uirt->dev, "carrier duration %u pulses %u",
+					uirt->last_duration, pulses);
+
+				// calculate the Hz of pulses in duration 400ns units
+				carrier = DIV_ROUND_CLOSEST_ULL(pulses * 10000000ull,
+								uirt->last_duration * 4);
+				ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+					.carrier = carrier,
+					.carrier_report = true,
+				}));
+			}
+			uirt->rx_state = RX_STATE_OFF_HIGH;
+			break;
+		case RX_STATE_OFF_HIGH:
+			if (uirt->in[i] == 0xff) {
+				ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+					.duration = IR_TIMEOUT,
+					.timeout = true,
+				}));
+				uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
+			} else {
+				uirt->high = uirt->in[i];
+				uirt->rx_state = RX_STATE_OFF_LOW;
+			}
+			break;
+		case RX_STATE_OFF_LOW:
+			// duration is in 400ns units
+			duration = (uirt->high << 8) | uirt->in[i];
+			if (duration < 3)
+				duration = 3;
+			ir_raw_event_store(uirt->rc, &((struct ir_raw_event) {
+				.duration = DIV_ROUND_CLOSEST(duration * 2, 5),
+				.pulse = false,
+			}));
+			uirt->rx_state = RX_STATE_ON_HIGH;
+			break;
+		}
+	}
+
+	ir_raw_event_handle(uirt->rc);
+}
+
+static void uirt_response(struct uirt *uirt, u32 len)
+{
+	int offset = 2;
+	int i;
+
+	dev_dbg(uirt->dev, "state:%d data: %*phN\n", uirt->cmd_state, len, uirt->in);
+
+	// Do we have more IR to transmit
+	if (uirt->cmd_state == CMD_STATE_STREAMING_TX && len >= 2 &&
+	    uirt->tx_len && uirt->in[0] & FTDI_RS0_CTS) {
+		u32 len;
+		int err;
+
+		len = min_t(u32, uirt->tx_len, MAX_PACKET);
+
+		memcpy(uirt->out, uirt->tx_buf, len);
+		uirt->urb_out->transfer_buffer_length = len;
+
+		uirt->tx_len -= len;
+		uirt->tx_buf += len;
+
+		err = usb_submit_urb(uirt->urb_out, GFP_ATOMIC);
+		if (err != 0)
+			dev_warn(uirt->dev,
+				 "failed to submit out urb: %d\n", err);
+	}
+
+	// if we only have two bytes, it just gives us the serial line status
+	if (len <= 2)
+		return;
+
+	switch (uirt->cmd_state) {
+	case CMD_STATE_GETVERSION:
+		if (len >= 10) {
+			// check checksum
+			u8 checksum = 0;
+
+			for (i = 2; i < len; i++)
+				checksum += uirt->in[i];
+
+			if (checksum != 0) {
+				dev_err(uirt->dev, "checksum does not match: %*phN\n",
+					len, uirt->in);
+				return;
+			}
+
+			dev_info(uirt->dev,
+				 "USB-UIRT firmware v%u.%u protocol v%u.%u %02u-%02u-%04u",
+				 uirt->in[2], uirt->in[3], uirt->in[4],
+				 uirt->in[5], uirt->in[6], uirt->in[7],
+				 2000 + uirt->in[8]);
+
+			complete(&uirt->cmd_done);
+			uirt->cmd_state = CMD_STATE_IRDATA;
+			offset += 10;
+		}
+		break;
+	case CMD_STATE_DOTXRAW:
+	case CMD_STATE_STREAMING_TX:
+	case CMD_STATE_SETMODERAW:
+	case CMD_STATE_SETMODEWIDEBAND:
+		if (len >= 3) {
+			switch (uirt->in[2]) {
+			case 0x20:
+				// 0x20 transmitting is expected during streaming tx
+				if (uirt->cmd_state == CMD_STATE_STREAMING_TX)
+					return;
+
+				if (uirt->cmd_state == CMD_STATE_DOTXRAW)
+					complete(&uirt->cmd_done);
+				else
+					dev_err(uirt->dev, "device transmitting");
+				break;
+			case 0x21:
+				if (uirt->tx_len) {
+					dev_err(uirt->dev, "tx completed with %u left to send",
+						uirt->tx_len);
+				} else {
+					if (uirt->cmd_state == CMD_STATE_SETMODERAW)
+						uirt->wideband = false;
+					if (uirt->cmd_state == CMD_STATE_SETMODEWIDEBAND)
+						uirt->wideband = true;
+
+					complete(&uirt->cmd_done);
+				}
+				break;
+			case 0x80:
+				dev_err(uirt->dev, "checksum error");
+				break;
+			case 0x81:
+				dev_err(uirt->dev, "timeout");
+				break;
+			case 0x82:
+				dev_err(uirt->dev, "command error");
+				break;
+			default:
+				dev_err(uirt->dev, "unknown response");
+			}
+
+			uirt->cmd_state = CMD_STATE_IRDATA;
+			offset += 1;
+		}
+	default:
+		break;
+	}
+
+	if (uirt->wideband)
+		uirt_wideband(uirt, offset, len);
+	else
+		uirt_raw_mode(uirt, offset, len);
+}
+
+static void uirt_out_callback(struct urb *urb)
+{
+	struct uirt *uirt = urb->context;
+
+	switch (urb->status) {
+	case 0:
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+	case -EPIPE:
+	default:
+		dev_warn(uirt->dev, "out urb status: %d\n", urb->status);
+		break;
+	}
+}
+
+static void uirt_in_callback(struct urb *urb)
+{
+	struct uirt *uirt = urb->context;
+	int ret;
+
+	if (urb->status == 0)
+		uirt_response(uirt, urb->actual_length);
+	else
+		dev_dbg(uirt->dev, "in urb status: %d\n", urb->status);
+
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret && ret != -ENODEV)
+		dev_warn(uirt->dev, "failed to resubmit urb: %d\n", ret);
+}
+
+static int uirt_command(struct uirt *uirt, const u8 *cmd, u32 cmd_len,
+			enum cmd_state state)
+{
+	int err;
+
+	init_completion(&uirt->cmd_done);
+
+	uirt->cmd_state = state;
+
+	memcpy(uirt->out, cmd, cmd_len);
+	uirt->urb_out->transfer_buffer_length = cmd_len;
+
+	err = usb_submit_urb(uirt->urb_out, GFP_KERNEL);
+	if (err != 0) {
+		uirt->cmd_state = CMD_STATE_IRDATA;
+		return err;
+	}
+
+	if (!wait_for_completion_timeout(&uirt->cmd_done,
+					 msecs_to_jiffies(USB_CTRL_SET_TIMEOUT))) {
+		usb_kill_urb(uirt->urb_out);
+		uirt->cmd_state = CMD_STATE_IRDATA;
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int uirt_setup(struct uirt *uirt)
+{
+	int err;
+
+	err = uirt_command(uirt, CMD_SETMODERAW, sizeof(CMD_SETMODERAW),
+			   CMD_STATE_SETMODERAW);
+	if (err) {
+		dev_err(uirt->dev, "set mode raw command failed: %d\n",
+			err);
+		return err;
+	}
+
+	err = uirt_command(uirt, CMD_GETVERSION,
+			   sizeof(CMD_GETVERSION), CMD_STATE_GETVERSION);
+	if (err != 0)
+		dev_err(uirt->dev, "get version command failed: %d\n",
+			err);
+
+	return err;
+}
+
+// IR TX when the data can fit into a single packet (i.e 64 bytes).
+// count must be no larger than 24, else we might overflow the buffer.
+static int uirt_short_tx(struct rc_dev *rc, uint *txbuf, uint count)
+{
+	struct uirt *uirt = rc->priv;
+	u8 out[MAX_PACKET], checksum;
+	u32 i, dest, width, freq;
+	int err;
+
+	out[0] = 0x36; // DOTXRAW
+	out[2] = uirt->freq; // carrier frequency
+	out[3] = 1; // number of repeats
+
+	dest = 7;
+
+	freq = uirt->freq & 0x7f;
+
+	for (i = 0; i < count; i++) {
+		// width = (us / freq) * 2.5
+		width = DIV_ROUND_CLOSEST(txbuf[i] * 5, freq * 2);
+
+		if (width == 0)
+			width = 1;
+		else if (width > 127)
+			out[dest++] = (width >> 8) | 0x80;
+
+		out[dest++] = width;
+	}
+
+	// length of RAWSTRUCT + 1
+	out[1] = dest - 1;
+	// number of bytes in encoded pulse/space
+	out[6] = dest - 7;
+
+	// checksum
+	for (i = 0, checksum = 0; i < dest; i++)
+		checksum -= out[i];
+
+	out[dest++] = checksum;
+
+	uirt->tx_buf = NULL;
+	uirt->tx_len = 0;
+
+	err = uirt_command(uirt, out, dest, CMD_STATE_DOTXRAW);
+	if (err != 0)
+		return err;
+
+	return count;
+}
+
+static int uirt_tx(struct rc_dev *rc, uint *txbuf, uint count)
+{
+	struct uirt *uirt = rc->priv;
+	u8 *out;
+	u32 i, dest, unit_raw, freq, len;
+	int err;
+
+	// streaming tx does not work for short IR; use non-streaming
+	// tx for short IR
+	if (count <= 24)
+		return uirt_short_tx(rc, txbuf, count);
+
+	out = kmalloc(count * 2 + 3, GFP_KERNEL);
+	if (!out)
+		return -ENOMEM;
+
+	out[0] = 0x25; // Streaming Transmit
+	out[1] = 0xdb; // checksum over command (just the previous byte)
+	out[2] = uirt->freq; // carrier frequency
+
+	dest = 3;
+
+	freq = uirt->freq & 0x7f;
+
+	for (i = 0; i < count; i++) {
+		// width = (us / freq) * 2.5
+		unit_raw = DIV_ROUND_CLOSEST(txbuf[i] * 5, freq * 2);
+
+		if (unit_raw == 0)
+			unit_raw = 1;
+		else if (unit_raw > 127)
+			out[dest++] = (unit_raw >> 8) | 0x80;
+
+		out[dest++] = unit_raw;
+	}
+
+	len = min_t(u32, dest, MAX_PACKET);
+
+	uirt->tx_buf = out + len;
+	uirt->tx_len = dest - len;
+
+	err = uirt_command(uirt, out, len, CMD_STATE_STREAMING_TX);
+	kfree(out);
+	if (err != 0)
+		return err;
+
+	return count;
+}
+
+static int uirt_set_tx_carrier(struct rc_dev *dev, u32 carrier)
+{
+	struct uirt *uirt = dev->priv;
+
+	if (carrier == 0)
+		// bit 7 must be 1 for unmodulated, lower bits need to
+		// be something that makes sense for tx
+		uirt->freq = 0xc0;
+	else if (carrier >= 20000 && carrier <= 500000)
+		// bit 7 must be 0 for modulated
+		uirt->freq = 2500000 / carrier;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int uirt_set_rx_wideband(struct rc_dev *dev, int enable)
+{
+	struct uirt *uirt = dev->priv;
+	int err;
+
+	if (enable)
+		err = uirt_command(uirt, CMD_SETWIDEBAND,
+				   sizeof(CMD_SETWIDEBAND),
+				   CMD_STATE_SETMODEWIDEBAND);
+	else
+		err = uirt_command(uirt, CMD_SETMODERAW,
+				   sizeof(CMD_SETMODERAW),
+				   CMD_STATE_SETMODERAW);
+
+	if (err) {
+		dev_err(uirt->dev, "set mode command failed: %d\n",
+			err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int init_ftdi(struct usb_device *udev)
+{
+	int err;
+
+	// set the baud rate
+	err = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			      FTDI_SIO_SET_BAUDRATE_REQUEST,
+			      FTDI_SIO_SET_BAUDRATE_REQUEST_TYPE,
+			      0x4009, 0x0001,
+			      NULL, 0, USB_CTRL_SET_TIMEOUT);
+	if (err)
+		return err;
+
+	// enabling rts/cts flow control
+	err = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			      FTDI_SIO_SET_FLOW_CTRL_REQUEST,
+			      FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
+			      0, FTDI_SIO_RTS_CTS_HS,
+			      NULL, 0, USB_CTRL_SET_TIMEOUT);
+	if (err)
+		return err;
+
+	// Set latency in milliseconds. The USB-UIRT will generate a
+	// urb every latency milliseconds (IR or not), so this should be
+	// set as high as possible to reduce interrupts. However, setting
+	// this value too high will mean there is a preceptible delay for
+	// IR being processed and a key press being registered.
+	//
+	// Choose 50ms as a compromise.
+	return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			      FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
+			      FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
+			      50, 0,
+			      NULL, 0, USB_CTRL_SET_TIMEOUT);
+}
+
+static int uirt_probe(struct usb_interface *intf,
+		      const struct usb_device_id *id)
+{
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep_in;
+	struct usb_endpoint_descriptor *ep_out;
+	struct uirt *uirt;
+	struct rc_dev *rc;
+	struct urb *urb;
+	int pipe, err;
+
+	if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
+	    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
+	    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
+		dev_err(&intf->dev, "required endpoints not found\n");
+		return -ENODEV;
+	}
+
+	uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
+	if (!uirt)
+		return -ENOMEM;
+
+	uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
+	if (!uirt->in)
+		goto free_uirt;
+
+	uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
+	if (!uirt->out)
+		goto free_uirt;
+
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
+	if (!rc)
+		goto free_uirt;
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb)
+		goto free_rcdev;
+
+	pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
+	usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
+			  uirt_in_callback, uirt);
+	uirt->urb_in = urb;
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb)
+		goto free_rcdev;
+
+	pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
+	usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
+			  uirt_out_callback, uirt);
+
+	uirt->dev = &intf->dev;
+	uirt->usbdev = usbdev;
+	uirt->rc = rc;
+	uirt->urb_out = urb;
+	uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
+
+	err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
+	if (err != 0) {
+		dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
+		goto free_rcdev;
+	}
+
+	err = init_ftdi(usbdev);
+	if (err) {
+		dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
+		goto kill_urbs;
+	}
+
+	err = uirt_setup(uirt);
+	if (err)
+		goto kill_urbs;
+
+	usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
+
+	rc->device_name = "USB-UIRT";
+	rc->driver_name = KBUILD_MODNAME;
+	rc->input_phys = uirt->phys;
+	usb_to_input_id(usbdev, &rc->input_id);
+	rc->dev.parent = &intf->dev;
+	rc->priv = uirt;
+	rc->tx_ir = uirt_tx;
+	rc->s_tx_carrier = uirt_set_tx_carrier;
+	rc->s_learning_mode = uirt_set_rx_wideband;
+	rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
+	rc->map_name = RC_MAP_RC6_MCE;
+	rc->rx_resolution = UNIT_US;
+	rc->timeout = IR_TIMEOUT;
+
+	uirt_set_tx_carrier(rc, 38000);
+
+	err = rc_register_device(rc);
+	if (err)
+		goto kill_urbs;
+
+	usb_set_intfdata(intf, uirt);
+
+	return 0;
+
+kill_urbs:
+	usb_kill_urb(uirt->urb_in);
+	usb_kill_urb(uirt->urb_out);
+free_rcdev:
+	usb_free_urb(uirt->urb_in);
+	usb_free_urb(uirt->urb_out);
+	rc_free_device(rc);
+free_uirt:
+	kfree(uirt->in);
+	kfree(uirt->out);
+	kfree(uirt);
+	return err;
+}
+
+static void uirt_disconnect(struct usb_interface *intf)
+{
+	struct uirt *ir = usb_get_intfdata(intf);
+
+	rc_unregister_device(ir->rc);
+	usb_set_intfdata(intf, NULL);
+	usb_kill_urb(ir->urb_in);
+	usb_kill_urb(ir->urb_out);
+	usb_free_urb(ir->urb_in);
+	usb_free_urb(ir->urb_out);
+	kfree(ir->in);
+	kfree(ir->out);
+	kfree(ir);
+}
+
+static const struct usb_device_id uirt_table[] = {
+	{ USB_DEVICE(0x0403, 0xf850) },
+	{ }
+};
+
+static struct usb_driver uirt_driver = {
+	.name = KBUILD_MODNAME,
+	.probe = uirt_probe,
+	.disconnect = uirt_disconnect,
+	.id_table = uirt_table,
+};
+
+module_usb_driver(uirt_driver);
+
+MODULE_AUTHOR("Sean Young <sean@mess.org>");
+MODULE_DESCRIPTION("USB-UIRT driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(usb, uirt_table);
-- 
2.31.1

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8314 bytes --]

Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
base:   git://linuxtv.org/media_tree.git master
config: powerpc64-randconfig-r012-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
        git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/media/rc/uirt.c:11:
   In file included from include/linux/completion.h:12:
   In file included from include/linux/swait.h:5:
   In file included from include/linux/list.h:9:
   In file included from include/linux/kernel.h:12:
   In file included from include/linux/bitops.h:32:
   In file included from arch/powerpc/include/asm/bitops.h:62:
   arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
   #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
           ^
   <built-in>:310:9: note: previous definition is here
   #define __lwsync __builtin_ppc_lwsync
           ^
>> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!urb)
               ^~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
           if (!urb)
           ^~~~~~~~~
   drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!urb)
               ^~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
           if (!urb)
           ^~~~~~~~~
   drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!rc)
               ^~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
           if (!rc)
           ^~~~~~~~
   drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!uirt->out)
               ^~~~~~~~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
           if (!uirt->out)
           ^~~~~~~~~~~~~~~
   drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!uirt->in)
               ^~~~~~~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
           if (!uirt->in)
           ^~~~~~~~~~~~~~
   drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
           int pipe, err;
                        ^
                         = 0
   6 warnings generated.


vim +639 drivers/media/rc/uirt.c

   594	
   595	static int uirt_probe(struct usb_interface *intf,
   596			      const struct usb_device_id *id)
   597	{
   598		struct usb_device *usbdev = interface_to_usbdev(intf);
   599		struct usb_endpoint_descriptor *ep_in;
   600		struct usb_endpoint_descriptor *ep_out;
   601		struct uirt *uirt;
   602		struct rc_dev *rc;
   603		struct urb *urb;
   604		int pipe, err;
   605	
   606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
   607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
   608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
   609			dev_err(&intf->dev, "required endpoints not found\n");
   610			return -ENODEV;
   611		}
   612	
   613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
   614		if (!uirt)
   615			return -ENOMEM;
   616	
   617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
   618		if (!uirt->in)
   619			goto free_uirt;
   620	
   621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
   622		if (!uirt->out)
   623			goto free_uirt;
   624	
   625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
   626		if (!rc)
   627			goto free_uirt;
   628	
   629		urb = usb_alloc_urb(0, GFP_KERNEL);
   630		if (!urb)
   631			goto free_rcdev;
   632	
   633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
   634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
   635				  uirt_in_callback, uirt);
   636		uirt->urb_in = urb;
   637	
   638		urb = usb_alloc_urb(0, GFP_KERNEL);
 > 639		if (!urb)
   640			goto free_rcdev;
   641	
   642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
   643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
   644				  uirt_out_callback, uirt);
   645	
   646		uirt->dev = &intf->dev;
   647		uirt->usbdev = usbdev;
   648		uirt->rc = rc;
   649		uirt->urb_out = urb;
   650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
   651	
   652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
   653		if (err != 0) {
   654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
   655			goto free_rcdev;
   656		}
   657	
   658		err = init_ftdi(usbdev);
   659		if (err) {
   660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
   661			goto kill_urbs;
   662		}
   663	
   664		err = uirt_setup(uirt);
   665		if (err)
   666			goto kill_urbs;
   667	
   668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
   669	
   670		rc->device_name = "USB-UIRT";
   671		rc->driver_name = KBUILD_MODNAME;
   672		rc->input_phys = uirt->phys;
   673		usb_to_input_id(usbdev, &rc->input_id);
   674		rc->dev.parent = &intf->dev;
   675		rc->priv = uirt;
   676		rc->tx_ir = uirt_tx;
   677		rc->s_tx_carrier = uirt_set_tx_carrier;
   678		rc->s_learning_mode = uirt_set_rx_wideband;
   679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
   680		rc->map_name = RC_MAP_RC6_MCE;
   681		rc->rx_resolution = UNIT_US;
   682		rc->timeout = IR_TIMEOUT;
   683	
   684		uirt_set_tx_carrier(rc, 38000);
   685	
   686		err = rc_register_device(rc);
   687		if (err)
   688			goto kill_urbs;
   689	
   690		usb_set_intfdata(intf, uirt);
   691	
   692		return 0;
   693	
   694	kill_urbs:
   695		usb_kill_urb(uirt->urb_in);
   696		usb_kill_urb(uirt->urb_out);
   697	free_rcdev:
   698		usb_free_urb(uirt->urb_in);
   699		usb_free_urb(uirt->urb_out);
   700		rc_free_device(rc);
   701	free_uirt:
   702		kfree(uirt->in);
   703		kfree(uirt->out);
   704		kfree(uirt);
   705		return err;
   706	}
   707	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46994 bytes --]

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8532 bytes --]

Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
base:   git://linuxtv.org/media_tree.git master
config: powerpc64-randconfig-r012-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
        git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/media/rc/uirt.c:11:
   In file included from include/linux/completion.h:12:
   In file included from include/linux/swait.h:5:
   In file included from include/linux/list.h:9:
   In file included from include/linux/kernel.h:12:
   In file included from include/linux/bitops.h:32:
   In file included from arch/powerpc/include/asm/bitops.h:62:
   arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
   #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
           ^
   <built-in>:310:9: note: previous definition is here
   #define __lwsync __builtin_ppc_lwsync
           ^
>> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!urb)
               ^~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
           if (!urb)
           ^~~~~~~~~
   drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!urb)
               ^~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
           if (!urb)
           ^~~~~~~~~
   drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!rc)
               ^~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
           if (!rc)
           ^~~~~~~~
   drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!uirt->out)
               ^~~~~~~~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
           if (!uirt->out)
           ^~~~~~~~~~~~~~~
   drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!uirt->in)
               ^~~~~~~~~
   drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
           return err;
                  ^~~
   drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
           if (!uirt->in)
           ^~~~~~~~~~~~~~
   drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
           int pipe, err;
                        ^
                         = 0
   6 warnings generated.


vim +639 drivers/media/rc/uirt.c

   594	
   595	static int uirt_probe(struct usb_interface *intf,
   596			      const struct usb_device_id *id)
   597	{
   598		struct usb_device *usbdev = interface_to_usbdev(intf);
   599		struct usb_endpoint_descriptor *ep_in;
   600		struct usb_endpoint_descriptor *ep_out;
   601		struct uirt *uirt;
   602		struct rc_dev *rc;
   603		struct urb *urb;
   604		int pipe, err;
   605	
   606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
   607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
   608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
   609			dev_err(&intf->dev, "required endpoints not found\n");
   610			return -ENODEV;
   611		}
   612	
   613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
   614		if (!uirt)
   615			return -ENOMEM;
   616	
   617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
   618		if (!uirt->in)
   619			goto free_uirt;
   620	
   621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
   622		if (!uirt->out)
   623			goto free_uirt;
   624	
   625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
   626		if (!rc)
   627			goto free_uirt;
   628	
   629		urb = usb_alloc_urb(0, GFP_KERNEL);
   630		if (!urb)
   631			goto free_rcdev;
   632	
   633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
   634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
   635				  uirt_in_callback, uirt);
   636		uirt->urb_in = urb;
   637	
   638		urb = usb_alloc_urb(0, GFP_KERNEL);
 > 639		if (!urb)
   640			goto free_rcdev;
   641	
   642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
   643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
   644				  uirt_out_callback, uirt);
   645	
   646		uirt->dev = &intf->dev;
   647		uirt->usbdev = usbdev;
   648		uirt->rc = rc;
   649		uirt->urb_out = urb;
   650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
   651	
   652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
   653		if (err != 0) {
   654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
   655			goto free_rcdev;
   656		}
   657	
   658		err = init_ftdi(usbdev);
   659		if (err) {
   660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
   661			goto kill_urbs;
   662		}
   663	
   664		err = uirt_setup(uirt);
   665		if (err)
   666			goto kill_urbs;
   667	
   668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
   669	
   670		rc->device_name = "USB-UIRT";
   671		rc->driver_name = KBUILD_MODNAME;
   672		rc->input_phys = uirt->phys;
   673		usb_to_input_id(usbdev, &rc->input_id);
   674		rc->dev.parent = &intf->dev;
   675		rc->priv = uirt;
   676		rc->tx_ir = uirt_tx;
   677		rc->s_tx_carrier = uirt_set_tx_carrier;
   678		rc->s_learning_mode = uirt_set_rx_wideband;
   679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
   680		rc->map_name = RC_MAP_RC6_MCE;
   681		rc->rx_resolution = UNIT_US;
   682		rc->timeout = IR_TIMEOUT;
   683	
   684		uirt_set_tx_carrier(rc, 38000);
   685	
   686		err = rc_register_device(rc);
   687		if (err)
   688			goto kill_urbs;
   689	
   690		usb_set_intfdata(intf, uirt);
   691	
   692		return 0;
   693	
   694	kill_urbs:
   695		usb_kill_urb(uirt->urb_in);
   696		usb_kill_urb(uirt->urb_out);
   697	free_rcdev:
   698		usb_free_urb(uirt->urb_in);
   699		usb_free_urb(uirt->urb_out);
   700		rc_free_device(rc);
   701	free_uirt:
   702		kfree(uirt->in);
   703		kfree(uirt->out);
   704		kfree(uirt);
   705		return err;
   706	}
   707	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 46994 bytes --]

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Sean Young]

On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
> Hi Sean,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on linuxtv-media/master]
> [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> base:   git://linuxtv.org/media_tree.git master
> config: powerpc64-randconfig-r012-20210617 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install powerpc64 cross compiling tool for clang build
>         # apt-get install binutils-powerpc64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>         git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from drivers/media/rc/uirt.c:11:
>    In file included from include/linux/completion.h:12:
>    In file included from include/linux/swait.h:5:
>    In file included from include/linux/list.h:9:
>    In file included from include/linux/kernel.h:12:
>    In file included from include/linux/bitops.h:32:
>    In file included from arch/powerpc/include/asm/bitops.h:62:
>    arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
>    #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>            ^
>    <built-in>:310:9: note: previous definition is here
>    #define __lwsync __builtin_ppc_lwsync
>            ^
> >> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!urb)
>                ^~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;

This is interesting. clang is right here, there are error paths where err is
not initialized. gcc-11.1 does not pick this up for some reason. The error path
should be an immediate dominator so it shouldn't be complicated to detect.

I'll send out a v5 with this issue fixed.

Sean

>                   ^~~
>    drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
>            if (!urb)
>            ^~~~~~~~~
>    drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!urb)
>                ^~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
>            if (!urb)
>            ^~~~~~~~~
>    drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!rc)
>                ^~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
>            if (!rc)
>            ^~~~~~~~
>    drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!uirt->out)
>                ^~~~~~~~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
>            if (!uirt->out)
>            ^~~~~~~~~~~~~~~
>    drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!uirt->in)
>                ^~~~~~~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
>            if (!uirt->in)
>            ^~~~~~~~~~~~~~
>    drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
>            int pipe, err;
>                         ^
>                          = 0
>    6 warnings generated.
> 
> 
> vim +639 drivers/media/rc/uirt.c
> 
>    594	
>    595	static int uirt_probe(struct usb_interface *intf,
>    596			      const struct usb_device_id *id)
>    597	{
>    598		struct usb_device *usbdev = interface_to_usbdev(intf);
>    599		struct usb_endpoint_descriptor *ep_in;
>    600		struct usb_endpoint_descriptor *ep_out;
>    601		struct uirt *uirt;
>    602		struct rc_dev *rc;
>    603		struct urb *urb;
>    604		int pipe, err;
>    605	
>    606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
>    607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
>    608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
>    609			dev_err(&intf->dev, "required endpoints not found\n");
>    610			return -ENODEV;
>    611		}
>    612	
>    613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
>    614		if (!uirt)
>    615			return -ENOMEM;
>    616	
>    617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
>    618		if (!uirt->in)
>    619			goto free_uirt;
>    620	
>    621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
>    622		if (!uirt->out)
>    623			goto free_uirt;
>    624	
>    625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
>    626		if (!rc)
>    627			goto free_uirt;
>    628	
>    629		urb = usb_alloc_urb(0, GFP_KERNEL);
>    630		if (!urb)
>    631			goto free_rcdev;
>    632	
>    633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
>    634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
>    635				  uirt_in_callback, uirt);
>    636		uirt->urb_in = urb;
>    637	
>    638		urb = usb_alloc_urb(0, GFP_KERNEL);
>  > 639		if (!urb)
>    640			goto free_rcdev;
>    641	
>    642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
>    643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
>    644				  uirt_out_callback, uirt);
>    645	
>    646		uirt->dev = &intf->dev;
>    647		uirt->usbdev = usbdev;
>    648		uirt->rc = rc;
>    649		uirt->urb_out = urb;
>    650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
>    651	
>    652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
>    653		if (err != 0) {
>    654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
>    655			goto free_rcdev;
>    656		}
>    657	
>    658		err = init_ftdi(usbdev);
>    659		if (err) {
>    660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
>    661			goto kill_urbs;
>    662		}
>    663	
>    664		err = uirt_setup(uirt);
>    665		if (err)
>    666			goto kill_urbs;
>    667	
>    668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
>    669	
>    670		rc->device_name = "USB-UIRT";
>    671		rc->driver_name = KBUILD_MODNAME;
>    672		rc->input_phys = uirt->phys;
>    673		usb_to_input_id(usbdev, &rc->input_id);
>    674		rc->dev.parent = &intf->dev;
>    675		rc->priv = uirt;
>    676		rc->tx_ir = uirt_tx;
>    677		rc->s_tx_carrier = uirt_set_tx_carrier;
>    678		rc->s_learning_mode = uirt_set_rx_wideband;
>    679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>    680		rc->map_name = RC_MAP_RC6_MCE;
>    681		rc->rx_resolution = UNIT_US;
>    682		rc->timeout = IR_TIMEOUT;
>    683	
>    684		uirt_set_tx_carrier(rc, 38000);
>    685	
>    686		err = rc_register_device(rc);
>    687		if (err)
>    688			goto kill_urbs;
>    689	
>    690		usb_set_intfdata(intf, uirt);
>    691	
>    692		return 0;
>    693	
>    694	kill_urbs:
>    695		usb_kill_urb(uirt->urb_in);
>    696		usb_kill_urb(uirt->urb_out);
>    697	free_rcdev:
>    698		usb_free_urb(uirt->urb_in);
>    699		usb_free_urb(uirt->urb_out);
>    700		rc_free_device(rc);
>    701	free_uirt:
>    702		kfree(uirt->in);
>    703		kfree(uirt->out);
>    704		kfree(uirt);
>    705		return err;
>    706	}
>    707	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Sean Young]

[-- Attachment #1: Type: text/plain, Size: 9326 bytes --]

On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
> Hi Sean,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on linuxtv-media/master]
> [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> base:   git://linuxtv.org/media_tree.git master
> config: powerpc64-randconfig-r012-20210617 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install powerpc64 cross compiling tool for clang build
>         # apt-get install binutils-powerpc64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>         git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from drivers/media/rc/uirt.c:11:
>    In file included from include/linux/completion.h:12:
>    In file included from include/linux/swait.h:5:
>    In file included from include/linux/list.h:9:
>    In file included from include/linux/kernel.h:12:
>    In file included from include/linux/bitops.h:32:
>    In file included from arch/powerpc/include/asm/bitops.h:62:
>    arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
>    #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>            ^
>    <built-in>:310:9: note: previous definition is here
>    #define __lwsync __builtin_ppc_lwsync
>            ^
> >> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!urb)
>                ^~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;

This is interesting. clang is right here, there are error paths where err is
not initialized. gcc-11.1 does not pick this up for some reason. The error path
should be an immediate dominator so it shouldn't be complicated to detect.

I'll send out a v5 with this issue fixed.

Sean

>                   ^~~
>    drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
>            if (!urb)
>            ^~~~~~~~~
>    drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!urb)
>                ^~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
>            if (!urb)
>            ^~~~~~~~~
>    drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!rc)
>                ^~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
>            if (!rc)
>            ^~~~~~~~
>    drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!uirt->out)
>                ^~~~~~~~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
>            if (!uirt->out)
>            ^~~~~~~~~~~~~~~
>    drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!uirt->in)
>                ^~~~~~~~~
>    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>            return err;
>                   ^~~
>    drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
>            if (!uirt->in)
>            ^~~~~~~~~~~~~~
>    drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
>            int pipe, err;
>                         ^
>                          = 0
>    6 warnings generated.
> 
> 
> vim +639 drivers/media/rc/uirt.c
> 
>    594	
>    595	static int uirt_probe(struct usb_interface *intf,
>    596			      const struct usb_device_id *id)
>    597	{
>    598		struct usb_device *usbdev = interface_to_usbdev(intf);
>    599		struct usb_endpoint_descriptor *ep_in;
>    600		struct usb_endpoint_descriptor *ep_out;
>    601		struct uirt *uirt;
>    602		struct rc_dev *rc;
>    603		struct urb *urb;
>    604		int pipe, err;
>    605	
>    606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
>    607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
>    608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
>    609			dev_err(&intf->dev, "required endpoints not found\n");
>    610			return -ENODEV;
>    611		}
>    612	
>    613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
>    614		if (!uirt)
>    615			return -ENOMEM;
>    616	
>    617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
>    618		if (!uirt->in)
>    619			goto free_uirt;
>    620	
>    621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
>    622		if (!uirt->out)
>    623			goto free_uirt;
>    624	
>    625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
>    626		if (!rc)
>    627			goto free_uirt;
>    628	
>    629		urb = usb_alloc_urb(0, GFP_KERNEL);
>    630		if (!urb)
>    631			goto free_rcdev;
>    632	
>    633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
>    634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
>    635				  uirt_in_callback, uirt);
>    636		uirt->urb_in = urb;
>    637	
>    638		urb = usb_alloc_urb(0, GFP_KERNEL);
>  > 639		if (!urb)
>    640			goto free_rcdev;
>    641	
>    642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
>    643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
>    644				  uirt_out_callback, uirt);
>    645	
>    646		uirt->dev = &intf->dev;
>    647		uirt->usbdev = usbdev;
>    648		uirt->rc = rc;
>    649		uirt->urb_out = urb;
>    650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
>    651	
>    652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
>    653		if (err != 0) {
>    654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
>    655			goto free_rcdev;
>    656		}
>    657	
>    658		err = init_ftdi(usbdev);
>    659		if (err) {
>    660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
>    661			goto kill_urbs;
>    662		}
>    663	
>    664		err = uirt_setup(uirt);
>    665		if (err)
>    666			goto kill_urbs;
>    667	
>    668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
>    669	
>    670		rc->device_name = "USB-UIRT";
>    671		rc->driver_name = KBUILD_MODNAME;
>    672		rc->input_phys = uirt->phys;
>    673		usb_to_input_id(usbdev, &rc->input_id);
>    674		rc->dev.parent = &intf->dev;
>    675		rc->priv = uirt;
>    676		rc->tx_ir = uirt_tx;
>    677		rc->s_tx_carrier = uirt_set_tx_carrier;
>    678		rc->s_learning_mode = uirt_set_rx_wideband;
>    679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>    680		rc->map_name = RC_MAP_RC6_MCE;
>    681		rc->rx_resolution = UNIT_US;
>    682		rc->timeout = IR_TIMEOUT;
>    683	
>    684		uirt_set_tx_carrier(rc, 38000);
>    685	
>    686		err = rc_register_device(rc);
>    687		if (err)
>    688			goto kill_urbs;
>    689	
>    690		usb_set_intfdata(intf, uirt);
>    691	
>    692		return 0;
>    693	
>    694	kill_urbs:
>    695		usb_kill_urb(uirt->urb_in);
>    696		usb_kill_urb(uirt->urb_out);
>    697	free_rcdev:
>    698		usb_free_urb(uirt->urb_in);
>    699		usb_free_urb(uirt->urb_out);
>    700		rc_free_device(rc);
>    701	free_uirt:
>    702		kfree(uirt->in);
>    703		kfree(uirt->out);
>    704		kfree(uirt);
>    705		return err;
>    706	}
>    707	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Nathan Chancellor]

On 6/18/2021 1:44 AM, Sean Young wrote:
> On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
>> Hi Sean,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on linuxtv-media/master]
>> [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>> base:   git://linuxtv.org/media_tree.git master
>> config: powerpc64-randconfig-r012-20210617 (attached as .config)
>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # install powerpc64 cross compiling tool for clang build
>>          # apt-get install binutils-powerpc64-linux-gnu
>>          # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
>>          git remote add linux-review https://github.com/0day-ci/linux
>>          git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>>          git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
>>          # save the attached .config to linux build tree
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> All warnings (new ones prefixed by >>):
>>
>>     In file included from drivers/media/rc/uirt.c:11:
>>     In file included from include/linux/completion.h:12:
>>     In file included from include/linux/swait.h:5:
>>     In file included from include/linux/list.h:9:
>>     In file included from include/linux/kernel.h:12:
>>     In file included from include/linux/bitops.h:32:
>>     In file included from arch/powerpc/include/asm/bitops.h:62:
>>     arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
>>     #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>>             ^
>>     <built-in>:310:9: note: previous definition is here
>>     #define __lwsync __builtin_ppc_lwsync
>>             ^
>>>> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!urb)
>>                 ^~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
> 
> This is interesting. clang is right here, there are error paths where err is
> not initialized. gcc-11.1 does not pick this up for some reason. The error path
> should be an immediate dominator so it shouldn't be complicated to detect.

The reason GCC does not warn about this is due to commit 78a5255ffb6a 
("Stop the ad-hoc games with -Wno-maybe-initialized"), which disables 
the GCC version of this warning except with W=2, which very few people 
use. You could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the 
same warning.

Cheers,
Nathan

> I'll send out a v5 with this issue fixed.
> 
> Sean
> 
>>                    ^~~
>>     drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
>>             if (!urb)
>>             ^~~~~~~~~
>>     drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!urb)
>>                 ^~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
>>             if (!urb)
>>             ^~~~~~~~~
>>     drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!rc)
>>                 ^~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
>>             if (!rc)
>>             ^~~~~~~~
>>     drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!uirt->out)
>>                 ^~~~~~~~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
>>             if (!uirt->out)
>>             ^~~~~~~~~~~~~~~
>>     drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!uirt->in)
>>                 ^~~~~~~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
>>             if (!uirt->in)
>>             ^~~~~~~~~~~~~~
>>     drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
>>             int pipe, err;
>>                          ^
>>                           = 0
>>     6 warnings generated.
>>
>>
>> vim +639 drivers/media/rc/uirt.c
>>
>>     594	
>>     595	static int uirt_probe(struct usb_interface *intf,
>>     596			      const struct usb_device_id *id)
>>     597	{
>>     598		struct usb_device *usbdev = interface_to_usbdev(intf);
>>     599		struct usb_endpoint_descriptor *ep_in;
>>     600		struct usb_endpoint_descriptor *ep_out;
>>     601		struct uirt *uirt;
>>     602		struct rc_dev *rc;
>>     603		struct urb *urb;
>>     604		int pipe, err;
>>     605	
>>     606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
>>     607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
>>     608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
>>     609			dev_err(&intf->dev, "required endpoints not found\n");
>>     610			return -ENODEV;
>>     611		}
>>     612	
>>     613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
>>     614		if (!uirt)
>>     615			return -ENOMEM;
>>     616	
>>     617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
>>     618		if (!uirt->in)
>>     619			goto free_uirt;
>>     620	
>>     621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
>>     622		if (!uirt->out)
>>     623			goto free_uirt;
>>     624	
>>     625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
>>     626		if (!rc)
>>     627			goto free_uirt;
>>     628	
>>     629		urb = usb_alloc_urb(0, GFP_KERNEL);
>>     630		if (!urb)
>>     631			goto free_rcdev;
>>     632	
>>     633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
>>     634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
>>     635				  uirt_in_callback, uirt);
>>     636		uirt->urb_in = urb;
>>     637	
>>     638		urb = usb_alloc_urb(0, GFP_KERNEL);
>>   > 639		if (!urb)
>>     640			goto free_rcdev;
>>     641	
>>     642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
>>     643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
>>     644				  uirt_out_callback, uirt);
>>     645	
>>     646		uirt->dev = &intf->dev;
>>     647		uirt->usbdev = usbdev;
>>     648		uirt->rc = rc;
>>     649		uirt->urb_out = urb;
>>     650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
>>     651	
>>     652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
>>     653		if (err != 0) {
>>     654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
>>     655			goto free_rcdev;
>>     656		}
>>     657	
>>     658		err = init_ftdi(usbdev);
>>     659		if (err) {
>>     660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
>>     661			goto kill_urbs;
>>     662		}
>>     663	
>>     664		err = uirt_setup(uirt);
>>     665		if (err)
>>     666			goto kill_urbs;
>>     667	
>>     668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
>>     669	
>>     670		rc->device_name = "USB-UIRT";
>>     671		rc->driver_name = KBUILD_MODNAME;
>>     672		rc->input_phys = uirt->phys;
>>     673		usb_to_input_id(usbdev, &rc->input_id);
>>     674		rc->dev.parent = &intf->dev;
>>     675		rc->priv = uirt;
>>     676		rc->tx_ir = uirt_tx;
>>     677		rc->s_tx_carrier = uirt_set_tx_carrier;
>>     678		rc->s_learning_mode = uirt_set_rx_wideband;
>>     679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>>     680		rc->map_name = RC_MAP_RC6_MCE;
>>     681		rc->rx_resolution = UNIT_US;
>>     682		rc->timeout = IR_TIMEOUT;
>>     683	
>>     684		uirt_set_tx_carrier(rc, 38000);
>>     685	
>>     686		err = rc_register_device(rc);
>>     687		if (err)
>>     688			goto kill_urbs;
>>     689	
>>     690		usb_set_intfdata(intf, uirt);
>>     691	
>>     692		return 0;
>>     693	
>>     694	kill_urbs:
>>     695		usb_kill_urb(uirt->urb_in);
>>     696		usb_kill_urb(uirt->urb_out);
>>     697	free_rcdev:
>>     698		usb_free_urb(uirt->urb_in);
>>     699		usb_free_urb(uirt->urb_out);
>>     700		rc_free_device(rc);
>>     701	free_uirt:
>>     702		kfree(uirt->in);
>>     703		kfree(uirt->out);
>>     704		kfree(uirt);
>>     705		return err;
>>     706	}
>>     707	
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 
> 

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Nathan Chancellor]

[-- Attachment #1: Type: text/plain, Size: 10106 bytes --]

On 6/18/2021 1:44 AM, Sean Young wrote:
> On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
>> Hi Sean,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on linuxtv-media/master]
>> [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>> base:   git://linuxtv.org/media_tree.git master
>> config: powerpc64-randconfig-r012-20210617 (attached as .config)
>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # install powerpc64 cross compiling tool for clang build
>>          # apt-get install binutils-powerpc64-linux-gnu
>>          # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
>>          git remote add linux-review https://github.com/0day-ci/linux
>>          git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
>>          git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
>>          # save the attached .config to linux build tree
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> All warnings (new ones prefixed by >>):
>>
>>     In file included from drivers/media/rc/uirt.c:11:
>>     In file included from include/linux/completion.h:12:
>>     In file included from include/linux/swait.h:5:
>>     In file included from include/linux/list.h:9:
>>     In file included from include/linux/kernel.h:12:
>>     In file included from include/linux/bitops.h:32:
>>     In file included from arch/powerpc/include/asm/bitops.h:62:
>>     arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
>>     #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>>             ^
>>     <built-in>:310:9: note: previous definition is here
>>     #define __lwsync __builtin_ppc_lwsync
>>             ^
>>>> drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!urb)
>>                 ^~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
> 
> This is interesting. clang is right here, there are error paths where err is
> not initialized. gcc-11.1 does not pick this up for some reason. The error path
> should be an immediate dominator so it shouldn't be complicated to detect.

The reason GCC does not warn about this is due to commit 78a5255ffb6a 
("Stop the ad-hoc games with -Wno-maybe-initialized"), which disables 
the GCC version of this warning except with W=2, which very few people 
use. You could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the 
same warning.

Cheers,
Nathan

> I'll send out a v5 with this issue fixed.
> 
> Sean
> 
>>                    ^~~
>>     drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
>>             if (!urb)
>>             ^~~~~~~~~
>>     drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!urb)
>>                 ^~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
>>             if (!urb)
>>             ^~~~~~~~~
>>     drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!rc)
>>                 ^~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
>>             if (!rc)
>>             ^~~~~~~~
>>     drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!uirt->out)
>>                 ^~~~~~~~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
>>             if (!uirt->out)
>>             ^~~~~~~~~~~~~~~
>>     drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!uirt->in)
>>                 ^~~~~~~~~
>>     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
>>             return err;
>>                    ^~~
>>     drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
>>             if (!uirt->in)
>>             ^~~~~~~~~~~~~~
>>     drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
>>             int pipe, err;
>>                          ^
>>                           = 0
>>     6 warnings generated.
>>
>>
>> vim +639 drivers/media/rc/uirt.c
>>
>>     594	
>>     595	static int uirt_probe(struct usb_interface *intf,
>>     596			      const struct usb_device_id *id)
>>     597	{
>>     598		struct usb_device *usbdev = interface_to_usbdev(intf);
>>     599		struct usb_endpoint_descriptor *ep_in;
>>     600		struct usb_endpoint_descriptor *ep_out;
>>     601		struct uirt *uirt;
>>     602		struct rc_dev *rc;
>>     603		struct urb *urb;
>>     604		int pipe, err;
>>     605	
>>     606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
>>     607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
>>     608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
>>     609			dev_err(&intf->dev, "required endpoints not found\n");
>>     610			return -ENODEV;
>>     611		}
>>     612	
>>     613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
>>     614		if (!uirt)
>>     615			return -ENOMEM;
>>     616	
>>     617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
>>     618		if (!uirt->in)
>>     619			goto free_uirt;
>>     620	
>>     621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
>>     622		if (!uirt->out)
>>     623			goto free_uirt;
>>     624	
>>     625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
>>     626		if (!rc)
>>     627			goto free_uirt;
>>     628	
>>     629		urb = usb_alloc_urb(0, GFP_KERNEL);
>>     630		if (!urb)
>>     631			goto free_rcdev;
>>     632	
>>     633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
>>     634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
>>     635				  uirt_in_callback, uirt);
>>     636		uirt->urb_in = urb;
>>     637	
>>     638		urb = usb_alloc_urb(0, GFP_KERNEL);
>>   > 639		if (!urb)
>>     640			goto free_rcdev;
>>     641	
>>     642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
>>     643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
>>     644				  uirt_out_callback, uirt);
>>     645	
>>     646		uirt->dev = &intf->dev;
>>     647		uirt->usbdev = usbdev;
>>     648		uirt->rc = rc;
>>     649		uirt->urb_out = urb;
>>     650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
>>     651	
>>     652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
>>     653		if (err != 0) {
>>     654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
>>     655			goto free_rcdev;
>>     656		}
>>     657	
>>     658		err = init_ftdi(usbdev);
>>     659		if (err) {
>>     660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
>>     661			goto kill_urbs;
>>     662		}
>>     663	
>>     664		err = uirt_setup(uirt);
>>     665		if (err)
>>     666			goto kill_urbs;
>>     667	
>>     668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
>>     669	
>>     670		rc->device_name = "USB-UIRT";
>>     671		rc->driver_name = KBUILD_MODNAME;
>>     672		rc->input_phys = uirt->phys;
>>     673		usb_to_input_id(usbdev, &rc->input_id);
>>     674		rc->dev.parent = &intf->dev;
>>     675		rc->priv = uirt;
>>     676		rc->tx_ir = uirt_tx;
>>     677		rc->s_tx_carrier = uirt_set_tx_carrier;
>>     678		rc->s_learning_mode = uirt_set_rx_wideband;
>>     679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>>     680		rc->map_name = RC_MAP_RC6_MCE;
>>     681		rc->rx_resolution = UNIT_US;
>>     682		rc->timeout = IR_TIMEOUT;
>>     683	
>>     684		uirt_set_tx_carrier(rc, 38000);
>>     685	
>>     686		err = rc_register_device(rc);
>>     687		if (err)
>>     688			goto kill_urbs;
>>     689	
>>     690		usb_set_intfdata(intf, uirt);
>>     691	
>>     692		return 0;
>>     693	
>>     694	kill_urbs:
>>     695		usb_kill_urb(uirt->urb_in);
>>     696		usb_kill_urb(uirt->urb_out);
>>     697	free_rcdev:
>>     698		usb_free_urb(uirt->urb_in);
>>     699		usb_free_urb(uirt->urb_out);
>>     700		rc_free_device(rc);
>>     701	free_uirt:
>>     702		kfree(uirt->in);
>>     703		kfree(uirt->out);
>>     704		kfree(uirt);
>>     705		return err;
>>     706	}
>>     707	
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> 
> 

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Sean Young]

On Fri, Jun 18, 2021 at 10:04:58AM -0700, Nathan Chancellor wrote:
> On 6/18/2021 1:44 AM, Sean Young wrote:
> > On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
> > > Hi Sean,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on linuxtv-media/master]
> > > [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch]
> > > 
> > > url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > > base:   git://linuxtv.org/media_tree.git master
> > > config: powerpc64-randconfig-r012-20210617 (attached as .config)
> > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
> > > reproduce (this is a W=1 build):
> > >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >          chmod +x ~/bin/make.cross
> > >          # install powerpc64 cross compiling tool for clang build
> > >          # apt-get install binutils-powerpc64-linux-gnu
> > >          # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          git remote add linux-review https://github.com/0day-ci/linux
> > >          git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > >          git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          # save the attached .config to linux build tree
> > >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
> > > 
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >     In file included from drivers/media/rc/uirt.c:11:
> > >     In file included from include/linux/completion.h:12:
> > >     In file included from include/linux/swait.h:5:
> > >     In file included from include/linux/list.h:9:
> > >     In file included from include/linux/kernel.h:12:
> > >     In file included from include/linux/bitops.h:32:
> > >     In file included from arch/powerpc/include/asm/bitops.h:62:
> > >     arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
> > >     #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> > >             ^
> > >     <built-in>:310:9: note: previous definition is here
> > >     #define __lwsync __builtin_ppc_lwsync
> > >             ^
> > > > > drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > 
> > This is interesting. clang is right here, there are error paths where err is
> > not initialized. gcc-11.1 does not pick this up for some reason. The error path
> > should be an immediate dominator so it shouldn't be complicated to detect.
> 
> The reason GCC does not warn about this is due to commit 78a5255ffb6a ("Stop
> the ad-hoc games with -Wno-maybe-initialized"), which disables the GCC
> version of this warning except with W=2, which very few people use. You
> could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the same warning.

You're right, this is exactly the issue here.  When running with W=2 or
KCFLAGS=-Wmaybe-uninitialized you do get the warning.

There are paths where err are initialized, so gcc correctly says:

warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]


Thanks,

Sean

> 
> Cheers,
> Nathan
> 
> > I'll send out a v5 with this issue fixed.
> > 
> > Sean
> > 
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!rc)
> > >                 ^~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
> > >             if (!rc)
> > >             ^~~~~~~~
> > >     drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->out)
> > >                 ^~~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->out)
> > >             ^~~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->in)
> > >                 ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->in)
> > >             ^~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
> > >             int pipe, err;
> > >                          ^
> > >                           = 0
> > >     6 warnings generated.
> > > 
> > > 
> > > vim +639 drivers/media/rc/uirt.c
> > > 
> > >     594	
> > >     595	static int uirt_probe(struct usb_interface *intf,
> > >     596			      const struct usb_device_id *id)
> > >     597	{
> > >     598		struct usb_device *usbdev = interface_to_usbdev(intf);
> > >     599		struct usb_endpoint_descriptor *ep_in;
> > >     600		struct usb_endpoint_descriptor *ep_out;
> > >     601		struct uirt *uirt;
> > >     602		struct rc_dev *rc;
> > >     603		struct urb *urb;
> > >     604		int pipe, err;
> > >     605	
> > >     606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
> > >     607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
> > >     608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
> > >     609			dev_err(&intf->dev, "required endpoints not found\n");
> > >     610			return -ENODEV;
> > >     611		}
> > >     612	
> > >     613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
> > >     614		if (!uirt)
> > >     615			return -ENOMEM;
> > >     616	
> > >     617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     618		if (!uirt->in)
> > >     619			goto free_uirt;
> > >     620	
> > >     621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     622		if (!uirt->out)
> > >     623			goto free_uirt;
> > >     624	
> > >     625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> > >     626		if (!rc)
> > >     627			goto free_uirt;
> > >     628	
> > >     629		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >     630		if (!urb)
> > >     631			goto free_rcdev;
> > >     632	
> > >     633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
> > >     634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
> > >     635				  uirt_in_callback, uirt);
> > >     636		uirt->urb_in = urb;
> > >     637	
> > >     638		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >   > 639		if (!urb)
> > >     640			goto free_rcdev;
> > >     641	
> > >     642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
> > >     643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
> > >     644				  uirt_out_callback, uirt);
> > >     645	
> > >     646		uirt->dev = &intf->dev;
> > >     647		uirt->usbdev = usbdev;
> > >     648		uirt->rc = rc;
> > >     649		uirt->urb_out = urb;
> > >     650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
> > >     651	
> > >     652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
> > >     653		if (err != 0) {
> > >     654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
> > >     655			goto free_rcdev;
> > >     656		}
> > >     657	
> > >     658		err = init_ftdi(usbdev);
> > >     659		if (err) {
> > >     660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
> > >     661			goto kill_urbs;
> > >     662		}
> > >     663	
> > >     664		err = uirt_setup(uirt);
> > >     665		if (err)
> > >     666			goto kill_urbs;
> > >     667	
> > >     668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
> > >     669	
> > >     670		rc->device_name = "USB-UIRT";
> > >     671		rc->driver_name = KBUILD_MODNAME;
> > >     672		rc->input_phys = uirt->phys;
> > >     673		usb_to_input_id(usbdev, &rc->input_id);
> > >     674		rc->dev.parent = &intf->dev;
> > >     675		rc->priv = uirt;
> > >     676		rc->tx_ir = uirt_tx;
> > >     677		rc->s_tx_carrier = uirt_set_tx_carrier;
> > >     678		rc->s_learning_mode = uirt_set_rx_wideband;
> > >     679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> > >     680		rc->map_name = RC_MAP_RC6_MCE;
> > >     681		rc->rx_resolution = UNIT_US;
> > >     682		rc->timeout = IR_TIMEOUT;
> > >     683	
> > >     684		uirt_set_tx_carrier(rc, 38000);
> > >     685	
> > >     686		err = rc_register_device(rc);
> > >     687		if (err)
> > >     688			goto kill_urbs;
> > >     689	
> > >     690		usb_set_intfdata(intf, uirt);
> > >     691	
> > >     692		return 0;
> > >     693	
> > >     694	kill_urbs:
> > >     695		usb_kill_urb(uirt->urb_in);
> > >     696		usb_kill_urb(uirt->urb_out);
> > >     697	free_rcdev:
> > >     698		usb_free_urb(uirt->urb_in);
> > >     699		usb_free_urb(uirt->urb_out);
> > >     700		rc_free_device(rc);
> > >     701	free_uirt:
> > >     702		kfree(uirt->in);
> > >     703		kfree(uirt->out);
> > >     704		kfree(uirt);
> > >     705		return err;
> > >     706	}
> > >     707	
> > > 
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> > 
> > 

Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

[Sean Young]

[-- Attachment #1: Type: text/plain, Size: 11180 bytes --]

On Fri, Jun 18, 2021 at 10:04:58AM -0700, Nathan Chancellor wrote:
> On 6/18/2021 1:44 AM, Sean Young wrote:
> > On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
> > > Hi Sean,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on linuxtv-media/master]
> > > [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch]
> > > 
> > > url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > > base:   git://linuxtv.org/media_tree.git master
> > > config: powerpc64-randconfig-r012-20210617 (attached as .config)
> > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
> > > reproduce (this is a W=1 build):
> > >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >          chmod +x ~/bin/make.cross
> > >          # install powerpc64 cross compiling tool for clang build
> > >          # apt-get install binutils-powerpc64-linux-gnu
> > >          # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          git remote add linux-review https://github.com/0day-ci/linux
> > >          git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > >          git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          # save the attached .config to linux build tree
> > >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
> > > 
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >     In file included from drivers/media/rc/uirt.c:11:
> > >     In file included from include/linux/completion.h:12:
> > >     In file included from include/linux/swait.h:5:
> > >     In file included from include/linux/list.h:9:
> > >     In file included from include/linux/kernel.h:12:
> > >     In file included from include/linux/bitops.h:32:
> > >     In file included from arch/powerpc/include/asm/bitops.h:62:
> > >     arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
> > >     #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> > >             ^
> > >     <built-in>:310:9: note: previous definition is here
> > >     #define __lwsync __builtin_ppc_lwsync
> > >             ^
> > > > > drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > 
> > This is interesting. clang is right here, there are error paths where err is
> > not initialized. gcc-11.1 does not pick this up for some reason. The error path
> > should be an immediate dominator so it shouldn't be complicated to detect.
> 
> The reason GCC does not warn about this is due to commit 78a5255ffb6a ("Stop
> the ad-hoc games with -Wno-maybe-initialized"), which disables the GCC
> version of this warning except with W=2, which very few people use. You
> could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the same warning.

You're right, this is exactly the issue here.  When running with W=2 or
KCFLAGS=-Wmaybe-uninitialized you do get the warning.

There are paths where err are initialized, so gcc correctly says:

warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]


Thanks,

Sean

> 
> Cheers,
> Nathan
> 
> > I'll send out a v5 with this issue fixed.
> > 
> > Sean
> > 
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!rc)
> > >                 ^~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
> > >             if (!rc)
> > >             ^~~~~~~~
> > >     drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->out)
> > >                 ^~~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->out)
> > >             ^~~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->in)
> > >                 ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->in)
> > >             ^~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
> > >             int pipe, err;
> > >                          ^
> > >                           = 0
> > >     6 warnings generated.
> > > 
> > > 
> > > vim +639 drivers/media/rc/uirt.c
> > > 
> > >     594	
> > >     595	static int uirt_probe(struct usb_interface *intf,
> > >     596			      const struct usb_device_id *id)
> > >     597	{
> > >     598		struct usb_device *usbdev = interface_to_usbdev(intf);
> > >     599		struct usb_endpoint_descriptor *ep_in;
> > >     600		struct usb_endpoint_descriptor *ep_out;
> > >     601		struct uirt *uirt;
> > >     602		struct rc_dev *rc;
> > >     603		struct urb *urb;
> > >     604		int pipe, err;
> > >     605	
> > >     606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
> > >     607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
> > >     608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
> > >     609			dev_err(&intf->dev, "required endpoints not found\n");
> > >     610			return -ENODEV;
> > >     611		}
> > >     612	
> > >     613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
> > >     614		if (!uirt)
> > >     615			return -ENOMEM;
> > >     616	
> > >     617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     618		if (!uirt->in)
> > >     619			goto free_uirt;
> > >     620	
> > >     621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     622		if (!uirt->out)
> > >     623			goto free_uirt;
> > >     624	
> > >     625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> > >     626		if (!rc)
> > >     627			goto free_uirt;
> > >     628	
> > >     629		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >     630		if (!urb)
> > >     631			goto free_rcdev;
> > >     632	
> > >     633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
> > >     634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
> > >     635				  uirt_in_callback, uirt);
> > >     636		uirt->urb_in = urb;
> > >     637	
> > >     638		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >   > 639		if (!urb)
> > >     640			goto free_rcdev;
> > >     641	
> > >     642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
> > >     643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
> > >     644				  uirt_out_callback, uirt);
> > >     645	
> > >     646		uirt->dev = &intf->dev;
> > >     647		uirt->usbdev = usbdev;
> > >     648		uirt->rc = rc;
> > >     649		uirt->urb_out = urb;
> > >     650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
> > >     651	
> > >     652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
> > >     653		if (err != 0) {
> > >     654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
> > >     655			goto free_rcdev;
> > >     656		}
> > >     657	
> > >     658		err = init_ftdi(usbdev);
> > >     659		if (err) {
> > >     660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
> > >     661			goto kill_urbs;
> > >     662		}
> > >     663	
> > >     664		err = uirt_setup(uirt);
> > >     665		if (err)
> > >     666			goto kill_urbs;
> > >     667	
> > >     668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
> > >     669	
> > >     670		rc->device_name = "USB-UIRT";
> > >     671		rc->driver_name = KBUILD_MODNAME;
> > >     672		rc->input_phys = uirt->phys;
> > >     673		usb_to_input_id(usbdev, &rc->input_id);
> > >     674		rc->dev.parent = &intf->dev;
> > >     675		rc->priv = uirt;
> > >     676		rc->tx_ir = uirt_tx;
> > >     677		rc->s_tx_carrier = uirt_set_tx_carrier;
> > >     678		rc->s_learning_mode = uirt_set_rx_wideband;
> > >     679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> > >     680		rc->map_name = RC_MAP_RC6_MCE;
> > >     681		rc->rx_resolution = UNIT_US;
> > >     682		rc->timeout = IR_TIMEOUT;
> > >     683	
> > >     684		uirt_set_tx_carrier(rc, 38000);
> > >     685	
> > >     686		err = rc_register_device(rc);
> > >     687		if (err)
> > >     688			goto kill_urbs;
> > >     689	
> > >     690		usb_set_intfdata(intf, uirt);
> > >     691	
> > >     692		return 0;
> > >     693	
> > >     694	kill_urbs:
> > >     695		usb_kill_urb(uirt->urb_in);
> > >     696		usb_kill_urb(uirt->urb_out);
> > >     697	free_rcdev:
> > >     698		usb_free_urb(uirt->urb_in);
> > >     699		usb_free_urb(uirt->urb_out);
> > >     700		rc_free_device(rc);
> > >     701	free_uirt:
> > >     702		kfree(uirt->in);
> > >     703		kfree(uirt->out);
> > >     704		kfree(uirt);
> > >     705		return err;
> > >     706	}
> > >     707	
> > > 
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> > 
> > 

[PATCH v4 2/2] USB: serial: blacklist USB-UIRT when driver is selected

[Sean Young]

The USB-UIRT device has its own driver, so blacklist the fdti driver
from using it if the driver has been enabled.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/usb/serial/ftdi_sio.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 369ef140df78..2e9a9076a38d 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -106,7 +106,7 @@ static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
 	.probe	= ftdi_NDI_device_setup,
 };
 
-static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
+static __maybe_unused const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
 	.port_probe = ftdi_USB_UIRT_setup,
 };
 
@@ -568,8 +568,10 @@ static const struct usb_device_id id_table_combined[] = {
 	{ USB_DEVICE(OCT_VID, OCT_DK201_PID) },
 	{ USB_DEVICE(FTDI_VID, FTDI_HE_TIRA1_PID),
 		.driver_info = (kernel_ulong_t)&ftdi_HE_TIRA1_quirk },
+#if !IS_ENABLED(CONFIG_IR_UIRT)
 	{ USB_DEVICE(FTDI_VID, FTDI_USB_UIRT_PID),
 		.driver_info = (kernel_ulong_t)&ftdi_USB_UIRT_quirk },
+#endif
 	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_1) },
 	{ USB_DEVICE(FTDI_VID, PROTEGO_R2X0) },
 	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_3) },
@@ -2281,7 +2283,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 /* Setup for the USB-UIRT device, which requires hardwired
  * baudrate (38400 gets mapped to 312500) */
 /* Called from usbserial:serial_probe */
-static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
+static __maybe_unused void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
 {
 	priv->flags |= ASYNC_SPD_CUST;
 	priv->custom_divisor = 77;
-- 
2.31.1

Re: [PATCH v4 2/2] USB: serial: blacklist USB-UIRT when driver is selected

[Greg Kroah-Hartman]

On Thu, Jun 10, 2021 at 11:16:25AM +0100, Sean Young wrote:
> The USB-UIRT device has its own driver, so blacklist the fdti driver
> from using it if the driver has been enabled.
> 
> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  drivers/usb/serial/ftdi_sio.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 369ef140df78..2e9a9076a38d 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -106,7 +106,7 @@ static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
>  	.probe	= ftdi_NDI_device_setup,
>  };
>  
> -static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
> +static __maybe_unused const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {

Why make this change?

>  	.port_probe = ftdi_USB_UIRT_setup,
>  };
>  
> @@ -568,8 +568,10 @@ static const struct usb_device_id id_table_combined[] = {
>  	{ USB_DEVICE(OCT_VID, OCT_DK201_PID) },
>  	{ USB_DEVICE(FTDI_VID, FTDI_HE_TIRA1_PID),
>  		.driver_info = (kernel_ulong_t)&ftdi_HE_TIRA1_quirk },
> +#if !IS_ENABLED(CONFIG_IR_UIRT)
>  	{ USB_DEVICE(FTDI_VID, FTDI_USB_UIRT_PID),
>  		.driver_info = (kernel_ulong_t)&ftdi_USB_UIRT_quirk },
> +#endif
>  	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_1) },
>  	{ USB_DEVICE(FTDI_VID, PROTEGO_R2X0) },
>  	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_3) },
> @@ -2281,7 +2283,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
>  /* Setup for the USB-UIRT device, which requires hardwired
>   * baudrate (38400 gets mapped to 312500) */
>  /* Called from usbserial:serial_probe */
> -static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
> +static __maybe_unused void ftdi_USB_UIRT_setup(struct ftdi_private *priv)

Again, why this change here?

thanks,

greg k-h

Re: [PATCH v4 2/2] USB: serial: blacklist USB-UIRT when driver is selected

[Sean Young]

On Thu, Jun 10, 2021 at 12:23:53PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jun 10, 2021 at 11:16:25AM +0100, Sean Young wrote:
> > The USB-UIRT device has its own driver, so blacklist the fdti driver
> > from using it if the driver has been enabled.
> > 
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >  drivers/usb/serial/ftdi_sio.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> > index 369ef140df78..2e9a9076a38d 100644
> > --- a/drivers/usb/serial/ftdi_sio.c
> > +++ b/drivers/usb/serial/ftdi_sio.c
> > @@ -106,7 +106,7 @@ static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
> >  	.probe	= ftdi_NDI_device_setup,
> >  };
> >  
> > -static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
> > +static __maybe_unused const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
> 
> Why make this change?

If CONFIG_IR_UIRT is enabled, then this struct is not used. This generates
a warning when compiling with W=1 (clang or gcc):

  CC [M]  drivers/usb/serial/ftdi_sio.o
drivers/usb/serial/ftdi_sio.c:109:36: warning: ‘ftdi_USB_UIRT_quirk’ defined but not used [-Wunused-const-variable=]
  109 | static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
      |                                    ^~~~~~~~~~~~~~~~~~~

> 
> >  	.port_probe = ftdi_USB_UIRT_setup,
> >  };
> >  
> > @@ -568,8 +568,10 @@ static const struct usb_device_id id_table_combined[] = {
> >  	{ USB_DEVICE(OCT_VID, OCT_DK201_PID) },
> >  	{ USB_DEVICE(FTDI_VID, FTDI_HE_TIRA1_PID),
> >  		.driver_info = (kernel_ulong_t)&ftdi_HE_TIRA1_quirk },
> > +#if !IS_ENABLED(CONFIG_IR_UIRT)
> >  	{ USB_DEVICE(FTDI_VID, FTDI_USB_UIRT_PID),
> >  		.driver_info = (kernel_ulong_t)&ftdi_USB_UIRT_quirk },
> > +#endif
> >  	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_1) },
> >  	{ USB_DEVICE(FTDI_VID, PROTEGO_R2X0) },
> >  	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_3) },
> > @@ -2281,7 +2283,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
> >  /* Setup for the USB-UIRT device, which requires hardwired
> >   * baudrate (38400 gets mapped to 312500) */
> >  /* Called from usbserial:serial_probe */
> > -static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
> > +static __maybe_unused void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
> 
> Again, why this change here?

Same again.


Thanks for reviewing.

Sean