watchdog: add driver for StreamLabs USB watchdog device

Hi Wim/Guenter,

For me it seems that there could be a potential race condition. I have to rely on watchdog_active(&streamlabs_wdt->wdt_dev) function which tests the WDOG_ACTIVE bit in struct watchdog_device->status member.
The watchdog_dev changes the state of the device with ->start() or ->ping() and ->stop() methods and updates the WDOG_ACTIVE accordingly.
In {pre,post}_reset methods here I have to change the state of the device from running to stopped and back to running conditionally, however WDOG_ACTIVE bit could be updated in between these callbacks execution or starting/stopping the device can race.
For instance, I see the potential dangerous race like this:

	CPUX					CPUY

	..				watchdog_stop() {
	..					if (wdd->ops->stop) {
							...
							err = wdd->ops->stop(wdd)
						}
usb_streamlabs_wdt_pre_reset() {
	if (watchdog_active())
		stop_command();			/* WDOG_ACTIVE bit is still set
	...					 here indicating that watchdog is
}						 started, but ->stop() has already
						 finished */
	...
usb_streamlabs_wdt_post_reset() {
	if (watchdog_active())
		start_command();
}
	...					/* WDOG_ACTIVE is updated here */
						clear_bit(WDOG_ACTIVE, &wdd->status);
					}

As a result, the watchdog subsystem "thinks" that watchdog is not active and should not be pinged. However, the driver observed using watchdog_active() that watchdog was active during {pre,post}_reset and restarted the device which will lead to unexpected reset. It is very unlikely race to happen but consequence is fatal. In other words, there are two independent paths leading to driver changing the state of the watchdog device and one path relies on status that can be changed by another path.

Thinking about that I see the following approaches:

1. Introduce a variable in struct streamlabs_wdt that tracks the state of the watchdog device itself and checking/updating the state of a device happens under semaphore lock.
Obviously, this "internal" to the driver state variable should be used in {pre,post}_reset. In case there will be other drivers (say, USB ones) they also need to implement this.

or

2. The updates to wdd->status should happen under wd_data->lock. Currently, it is mutex-based. The acquiring and releasing the lock could be exported for the drivers to use. The mutex lock probably should be switched to a binary semaphore for that.

In such case, in pre_reset() for example, I would need to do:
static int pre_reset()

{
	lock_wdd();
	acquire_internal_driver_lock();
	
	if (watchdog_active())
		stop_command();
}

static int post_reset()
{

	if (watchdog_active())
		start_command();

	release_internal_driver_lock();
	unlock_wdd();
}

There should be an order that we have to acquire subsystem wdd lock first, then internal driver lock. Otherwise there could be deadlocks.

This could be done if you think it's more wiser move.

or

3. The {pre,post}_reset callbacks should execute watchdog_dev.c subsystem functions (not sure which functions exactly). Eventually, it will look similar to what is described in the previous point with respect to locks order.
I meant something like this:

static int pre_reset()

{
	watchdog_dev_pre_reset_prepare();
}

static int post_reset()

{
	watchdog_dev_post_reset_done();
}

In watchdog_dev.c:
void watchdog_dev_pre_reset_prepare()
{
mutex_lock(&wd_data->lock); <-- should be changed to semaphore too?

watchdog_stop(wdd); <-- without updating WDOG_ACTIVE bit?
or there should be a way to indicate to watchdog_dev_post_reset_done() if watchdog should be started or not
}

void watchdog_dev_post_reset_done()

{
	if (watchdog_active())
		watchdog_start(wdd);

         mutex_unlock(&wd_data->lock);
}

I didn't really thought about point 3 yet. For me personally the point 2 seems the like right way to go but you have more experience with that. The exported locks could be re-used by other drivers if needed in future.
In case of point 1 each USB driver should deal with {pre,post}_reset by themselves.

Any thoughts?

Thanks,
Alexey

Driver supports StreamLabs usb watchdog device. The device plugs into 9-pin usb header and connects to reset pins and reset button pins on desktop PC.

USB commands used to communicate with device were reverse engineered using usbmon.

Signed-off-by Alexey Klimov <[email protected]>

---

Changes since v5:

	-- moved to semaphore lock: down_killable()/up();
	If I have to go with down_timeout() option, then
	I am not sure which timeout value to use, apart
	from maximum or current timeout from device;
	-- implemented {pre,post}_reset methods (thanks, Oliver!);
	-- back to the version where buffer is a part of
	struct streamlabs_wdt and now also ____cacheline_aligned;
	-- watchdog_start() in probe instead of watchdog_stop();
	-- added watchdog_stop_on_reboot() in probe;
	-- other comments from Guenter.

Previous version:
https://lore.kernel.org/linux-usb/CALW4P+LmMJAm=3PWQfiEgYS7d_q0M0rdSJVhpZw9x9w3fQQMRA@mail.gmail.com/T/

  MAINTAINERS                       |   6 +
  drivers/watchdog/Kconfig          |  15 ++
  drivers/watchdog/Makefile         |   1 +
  drivers/watchdog/streamlabs_wdt.c | 412 ++++++++++++++++++++++++++++++
  4 files changed, 434 insertions(+)

create mode 100644 drivers/watchdog/streamlabs_wdt.c

MAINTAINERS +6 -0

index 936490dcc97b..e56fd4d636d1 100644
@@ -19480,6 +19480,12 @@ W:	http://www.stlinux.com
  F:	Documentation/networking/device_drivers/ethernet/stmicro/
  F:	drivers/net/ethernet/stmicro/stmmac/
 
+STREAMLABS USB WATCHDOG DRIVER
+M:	Alexey Klimov <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/watchdog/streamlabs_wdt.c
+
  SUN3/3X
  M:	Sam Creasey <[email protected]>
  S:	Maintained

drivers/watchdog/Kconfig +15 -0

index 9295492d24f7..8baf53fef3cc 100644
@@ -2180,6 +2180,21 @@ config USBPCWATCHDOG
 
  	  Most people will say N.
 
+config USB_STREAMLABS_WATCHDOG
+	tristate "StreamLabs USB watchdog driver"
+	depends on USB
+	help
+	  This is the driver for the USB Watchdog dongle from StreamLabs.
+	  If you correctly connect reset pins to motherboard Reset pin and
+	  to Reset button then this device will simply watch your kernel to make
+	  sure it doesn't freeze, and if it does, it reboots your computer
+	  after a certain amount of time.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called streamlabs_wdt.
+
+	  Most people will say N. Say yes or M if you want to use such usb device.
+
  config KEEMBAY_WATCHDOG
  	tristate "Intel Keem Bay SoC non-secure watchdog"
  	depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST)

drivers/watchdog/Makefile +1 -0

index cdeb119e6e61..afc6fdc1b4d9 100644
@@ -33,6 +33,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o
 
  # USB-based Watchdog Cards
  obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
+obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o
 
  # ALPHA Architecture
 

drivers/watchdog/streamlabs_wdt.c new file +357 -0

new file mode 100644
index 000000000000..cc4f48f1fd36
@@ -0,0 +1,412 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StreamLabs USB Watchdog driver
+ *
+ * Copyright (c) 2016-2017,2022 Alexey Klimov <[email protected]>
+ */
+
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/watchdog.h>
+#include <asm/byteorder.h>
+
+/*
+ * USB Watchdog device from Streamlabs:
+ * https://www.stream-labs.com/products/devices/watch-dog/
+ *
+ * USB commands have been reverse engineered using usbmon.
+ */
+
+#define DRIVER_AUTHOR "Alexey Klimov <[email protected]>"
+#define DRIVER_DESC "StreamLabs USB watchdog driver"
+#define DRIVER_NAME "usb_streamlabs_wdt"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+#define USB_STREAMLABS_WATCHDOG_VENDOR	0x13c0
+#define USB_STREAMLABS_WATCHDOG_PRODUCT	0x0011
+
+/*
+ * one buffer is used for communication, however transmitted message is only
+ * 32 bytes long
+ */
+#define BUFFER_TRANSFER_LENGTH	32
+#define BUFFER_LENGTH		64
+#define USB_TIMEOUT		350	/* 350 ms */
+
+#define STREAMLABS_CMD_START	0xaacc
+#define STREAMLABS_CMD_STOP	0xbbff
+
+/* timeout values are taken from windows program */
+#define STREAMLABS_WDT_MIN_TIMEOUT	1
+#define STREAMLABS_WDT_MAX_TIMEOUT	46
+
+struct streamlabs_wdt {
+	struct watchdog_device wdt_dev;
+	struct usb_interface *intf;
+	/* Serialises usb communication with a device */
+	struct semaphore lock;
+	__le16 buffer[BUFFER_LENGTH] ____cacheline_aligned;
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+			__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+/* USB call wrappers to send and receive messages to/from the device. */
+static int usb_streamlabs_send_msg(struct usb_device *usbdev, __le16 *buf)
+{
+	int retval;
+	int size;
+
+	retval = usb_interrupt_msg(usbdev, usb_sndintpipe(usbdev, 0x02),
+				   buf, BUFFER_TRANSFER_LENGTH,
+				   &size, USB_TIMEOUT);
+
+	if (retval)
+		return retval;
+
+	if (size != BUFFER_TRANSFER_LENGTH)
+		return -EIO;
+
+	return 0;
+}
+
+static int usb_streamlabs_get_msg(struct usb_device *usbdev, __le16 *buf)
+{
+	int retval;
+	int size;
+
+	retval = usb_interrupt_msg(usbdev, usb_rcvintpipe(usbdev, 0x81),
+				   buf, BUFFER_LENGTH,
+				   &size, USB_TIMEOUT);
+
+	if (retval)
+		return retval;
+
+	if (size != BUFFER_LENGTH)
+		return -EIO;
+
+	return 0;
+}
+
+/*
+ * This function is used to check if watchdog timeout in the received buffer
+ * matches the timeout passed from watchdog subsystem.
+ */
+static int usb_streamlabs_wdt_check_timeout(__le16 *buf, unsigned long timeout)
+{
+	if (buf[3] != cpu_to_le16(timeout))
+		return -EPROTO;
+
+	return 0;
+}
+
+static int usb_streamlabs_wdt_check_response(u8 *buf)
+{
+	/*
+	 * If watchdog device understood the command it will acknowledge
+	 * with values 1,2,3,4 at indexes 10, 11, 12, 13 in response message
+	 * when response treated as 8bit message.
+	 */
+	if (buf[10] != 1 || buf[11] != 2 || buf[12] != 3 || buf[13] != 4)
+		return -EPROTO;
+
+	return 0;
+}
+
+/*
+ * This function is used to check if watchdog command in the received buffer
+ * matches the command passed to the device.
+ */
+static int usb_streamlabs_wdt_check_command(__le16 *buf, u16 cmd)
+{
+	if (buf[0] != cpu_to_le16(cmd))
+		return -EPROTO;
+
+	return 0;
+}
+
+static int usb_streamlabs_wdt_validate_response(__le16 *buf, u16 cmd,
+						unsigned long timeout_msec)
+{
+	int retval;
+
+	retval = usb_streamlabs_wdt_check_response((u8 *)buf);
+	if (retval)
+		return retval;
+
+	retval = usb_streamlabs_wdt_check_command(buf, cmd);
+	if (retval)
+		return retval;
+
+	return usb_streamlabs_wdt_check_timeout(buf, timeout_msec);
+}
+
+static void usb_streamlabs_wdt_prepare_buf(__le16 *buf, u16 cmd,
+					   unsigned long timeout_msec)
+{
+	memset(buf, 0, BUFFER_LENGTH);
+
+	buf[0] = cpu_to_le16(cmd);
+	buf[1] = cpu_to_le16(0x8000);
+	buf[3] = cpu_to_le16(timeout_msec);
+}
+
+static int __usb_streamlabs_wdt_cmd(struct streamlabs_wdt *wdt, u16 cmd)
+{
+	struct usb_device *usbdev;
+	unsigned long timeout_msec;
+	/* how many times to re-try getting the state of the device */
+	unsigned int retry_counter = 10;
+	int retval;
+
+	if (unlikely(!wdt->intf))
+		return -ENODEV;
+
+	usbdev = interface_to_usbdev(wdt->intf);
+	timeout_msec = wdt->wdt_dev.timeout * MSEC_PER_SEC;
+
+	usb_streamlabs_wdt_prepare_buf(wdt->buffer, cmd, timeout_msec);
+
+	/* send command to watchdog */
+	retval = usb_streamlabs_send_msg(usbdev, wdt->buffer);
+	if (retval)
+		return retval;
+
+	/*
+	 * Transition from one state to another in this device
+	 * doesn't happen immediately, especially stopping the device
+	 * is not observed on the first reading of the response.
+	 * Plus to avoid potentially stale response message in the device
+	 * we keep reading the state of the device until we see:
+	 * -- that device recognised the sent command;
+	 * -- the received state (started or stopped) matches the state
+	 * that was requested;
+	 * -- the timeout passed matches the timeout value read from
+	 * the device.
+	 * Keep retrying 10 times and if watchdog device doesn't respond
+	 * correctly as expected we bail out and return an error.
+	 */
+	do {
+		retval = usb_streamlabs_get_msg(usbdev, wdt->buffer);
+		if (retval)
+			break;
+
+		retval = usb_streamlabs_wdt_validate_response(wdt->buffer, cmd,
+							      timeout_msec);
+	} while (retval && retry_counter--);
+
+	return retry_counter ? retval : -EIO;
+}
+
+static int usb_streamlabs_wdt_cmd(struct streamlabs_wdt *streamlabs_wdt, u16 cmd)
+{
+	int retval;
+
+	retval = down_killable(&streamlabs_wdt->lock);
+	if (retval)
+		return retval;
+	retval = __usb_streamlabs_wdt_cmd(streamlabs_wdt, cmd);
+	up(&streamlabs_wdt->lock);
+
+	return retval;
+}
+
+static int usb_streamlabs_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
+
+	return usb_streamlabs_wdt_cmd(streamlabs_wdt, STREAMLABS_CMD_START);
+}
+
+static int usb_streamlabs_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
+
+	return usb_streamlabs_wdt_cmd(streamlabs_wdt, STREAMLABS_CMD_STOP);
+}
+
+static const struct watchdog_info streamlabs_wdt_ident = {
+	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity	= DRIVER_NAME,
+};
+
+static const struct watchdog_ops usb_streamlabs_wdt_ops = {
+	.owner	= THIS_MODULE,
+	.start	= usb_streamlabs_wdt_start,
+	.stop	= usb_streamlabs_wdt_stop,
+};
+
+static int usb_streamlabs_wdt_probe(struct usb_interface *intf,
+				    const struct usb_device_id *id)
+{
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct streamlabs_wdt *streamlabs_wdt;
+	int retval;
+
+	/*
+	 * USB IDs of this device appear to be weird/unregistered. Hence, do
+	 * an additional check on product and manufacturer.
+	 * If there is similar device in the field with same values then
+	 * there is stop command in probe() below that checks if the device
+	 * behaves as a watchdog.
+	 */
+	if (!usbdev->product || !usbdev->manufacturer ||
+	    strncmp(usbdev->product, "USBkit", 6) ||
+	    strncmp(usbdev->manufacturer, "STREAM LABS", 11))
+		return -ENODEV;
+
+	streamlabs_wdt = devm_kzalloc(&intf->dev, sizeof(struct streamlabs_wdt),
+				      GFP_KERNEL);
+	if (!streamlabs_wdt)
+		return -ENOMEM;
+
+	sema_init(&streamlabs_wdt->lock, 1);
+
+	streamlabs_wdt->wdt_dev.info = &streamlabs_wdt_ident;
+	streamlabs_wdt->wdt_dev.ops = &usb_streamlabs_wdt_ops;
+	streamlabs_wdt->wdt_dev.timeout = STREAMLABS_WDT_MAX_TIMEOUT;
+	streamlabs_wdt->wdt_dev.max_timeout = STREAMLABS_WDT_MAX_TIMEOUT;
+	streamlabs_wdt->wdt_dev.min_timeout = STREAMLABS_WDT_MIN_TIMEOUT;
+	streamlabs_wdt->wdt_dev.parent = &intf->dev;
+
+	streamlabs_wdt->intf = intf;
+	usb_set_intfdata(intf, &streamlabs_wdt->wdt_dev);
+	watchdog_set_drvdata(&streamlabs_wdt->wdt_dev, streamlabs_wdt);
+	watchdog_set_nowayout(&streamlabs_wdt->wdt_dev, nowayout);
+	watchdog_stop_on_reboot(&streamlabs_wdt->wdt_dev);
+
+	retval = usb_streamlabs_wdt_start(&streamlabs_wdt->wdt_dev);
+	if (retval)
+		return -ENODEV;
+	set_bit(WDOG_HW_RUNNING, &streamlabs_wdt->wdt_dev.status);
+
+	retval = devm_watchdog_register_device(&intf->dev,
+					       &streamlabs_wdt->wdt_dev);
+	if (retval) {
+		dev_err(&intf->dev, "failed to register watchdog device\n");
+		return retval;
+	}
+
+	dev_info(&intf->dev, "StreamLabs USB watchdog driver loaded.\n");
+	return 0;
+}
+
+static int usb_streamlabs_wdt_suspend(struct usb_interface *intf,
+				      pm_message_t message)
+{
+	struct streamlabs_wdt *streamlabs_wdt = usb_get_intfdata(intf);
+
+	if (watchdog_active(&streamlabs_wdt->wdt_dev))
+		return usb_streamlabs_wdt_stop(&streamlabs_wdt->wdt_dev);
+
+	return 0;
+}
+
+static int usb_streamlabs_wdt_resume(struct usb_interface *intf)
+{
+	struct streamlabs_wdt *streamlabs_wdt = usb_get_intfdata(intf);
+
+	if (watchdog_active(&streamlabs_wdt->wdt_dev))
+		return usb_streamlabs_wdt_start(&streamlabs_wdt->wdt_dev);
+
+	return 0;
+}
+
+static void usb_streamlabs_wdt_disconnect(struct usb_interface *intf)
+{
+	struct streamlabs_wdt *streamlabs_wdt = usb_get_intfdata(intf);
+	int retval;
+
+	retval = down_killable(&streamlabs_wdt->lock);
+	if (retval) {
+		dev_warn(&intf->dev,
+			 "failed to acquire semaphore in disconnect\n");
+		return;
+	}
+
+	/*
+	 * If disconnect happens via sysfs or on rmmod, then try to stop
+	 * the watchdog. In case of physical detachment of the device this call
+	 * will fail but we continue.
+	 */
+	if (!nowayout)
+		__usb_streamlabs_wdt_cmd(streamlabs_wdt, STREAMLABS_CMD_STOP);
+
+	/* Stop sending (new) messages to the device */
+	streamlabs_wdt->intf = NULL;
+	up(&streamlabs_wdt->lock);
+}
+
+static int usb_streamlabs_wdt_pre_reset(struct usb_interface *intf)
+{
+	struct streamlabs_wdt *streamlabs_wdt = usb_get_intfdata(intf);
+	int retval;
+
+	retval = down_killable(&streamlabs_wdt->lock);
+	if (retval) {
+		dev_warn(&intf->dev,

drivers/watchdog/streamlabs_wdt.c new file +5 -0

+		 * unlock semaphore here.
+		 */
+		if (retval)
+			up(&streamlabs_wdt->lock);
+	}

drivers/watchdog/streamlabs_wdt.c new file +30 -1

+	if (watchdog_active(&streamlabs_wdt->wdt_dev))
+		retval = __usb_streamlabs_wdt_cmd(streamlabs_wdt,
+						  STREAMLABS_CMD_START);
+
+	up(&streamlabs_wdt->lock);
+
+	return retval;
+}
+
+static const struct usb_device_id usb_streamlabs_wdt_device_table[] = {
+	{ USB_DEVICE(USB_STREAMLABS_WATCHDOG_VENDOR, USB_STREAMLABS_WATCHDOG_PRODUCT) },
+	{ }	/* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, usb_streamlabs_wdt_device_table);
+
+static struct usb_driver usb_streamlabs_wdt_driver = {
+	.name		= DRIVER_NAME,
+	.probe		= usb_streamlabs_wdt_probe,
+	.disconnect	= usb_streamlabs_wdt_disconnect,
+	.suspend	= usb_streamlabs_wdt_suspend,
+	.resume		= usb_streamlabs_wdt_resume,
+	.reset_resume	= usb_streamlabs_wdt_resume,
+	.pre_reset	= usb_streamlabs_wdt_pre_reset,
+	.post_reset	= usb_streamlabs_wdt_post_reset,
+	.id_table	= usb_streamlabs_wdt_device_table,
+	.soft_unbind	= 1,
+};
+
+module_usb_driver(usb_streamlabs_wdt_driver);
--

2.37.2