lkml.org 
[lkml]   [2022]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5] watchdog: add driver for StreamLabs USB watchdog device
On 7/24/22 20:06, Alexey Klimov wrote:
> 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 <klimov.linux@gmail.com>
> ---
> Changes since v4:
> -- added more comments;
> -- added nowayout check in ->disconnect();
> -- completely rework usb_streamlabs_wdt_cmd() ->
> __usb_streamlabs_wdt_cmd() and functions
> that handle validation of the response;
> -- usb sending and receiving msgs are moved to separate
> functions;
> -- added soft_unbind=1 flag in usb_driver struct
> to make it possible to send URBs in ->disconnect();
> -- buffer is declared as __le16 now;
> -- made checkpatch.pl --strict happy;
>
> Previous version:
> https://lore.kernel.org/linux-watchdog/20220715234442.3910204-1-klimov.linux@gmail.com/
>
>
> MAINTAINERS | 6 +
> drivers/watchdog/Kconfig | 15 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/streamlabs_wdt.c | 360 ++++++++++++++++++++++++++++++
> 4 files changed, 382 insertions(+)
> create mode 100644 drivers/watchdog/streamlabs_wdt.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 64379c699903..fb31c1823043 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19190,6 +19190,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 <klimov.linux@gmail.com>
> +L: linux-watchdog@vger.kernel.org
> +S: Maintained
> +F: drivers/watchdog/streamlabs_wdt.c
> +
> SUN3/3X
> M: Sam Creasey <sammy@sammy.net>
> S: Maintained
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 32fd37698932..64b7f947b196 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -2171,6 +2171,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)
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index c324e9d820e9..2d601da9b5bd 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -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
>
> diff --git a/drivers/watchdog/streamlabs_wdt.c b/drivers/watchdog/streamlabs_wdt.c
> new file mode 100644
> index 000000000000..f1130fe5559c
> --- /dev/null
> +++ b/drivers/watchdog/streamlabs_wdt.c
> @@ -0,0 +1,360 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * StreamLabs USB Watchdog driver
> + *
> + * Copyright (c) 2016-2017,2022 Alexey Klimov <klimov.linux@gmail.com>
> + */
> +
> +#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 <klimov.linux@gmail.com>"
> +#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
> +
Comment about the unit (ms) might be useful.

> +#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 mutex lock;
> + __le16 *buffer;
> +};
> +
> +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 (size != BUFFER_TRANSFER_LENGTH)
> + return -EIO;
> +

If usb_interrupt_msg() returns an error, it will likely not set size,
which may result in a random -EIO. I think this should be something like

if (retval)
return retval;
if (size != BUFFER_TRANSFER_LENGTH)
return -EIO;

return 0;


> + return retval;
> +}
> +
> +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 (size != BUFFER_LENGTH)
> + return -EIO;
> +
Same here.

> + return retval;
> +}
> +
> +/*
> + * 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;
> +
> + retval = usb_streamlabs_wdt_check_timeout(buf, timeout_msec);
> + return retval;

assignment to retval is unnecessary.

> +}
> +
> +static void usb_streamlabs_wdt_prepare_buf(__le16 *buf, u16 cmd,
> + unsigned long timeout_msec)
> +{
> + /*
> + * remaining elements expected to be zero everytime during
> + * communication
> + */
> + buf[0] = cpu_to_le16(cmd);
> + buf[1] = cpu_to_le16(0x8000);
> + buf[3] = cpu_to_le16(timeout_msec);

Not setting buf[2] and buf[4] contradicts the comment above. Maybe
those offsets are not _expected_ to be set by the device, but that
is not guaranteed. It may be safer to just use memset() at the
beginning of the function to clear the buffer.

> + buf[5] = 0x0;
> + buf[6] = 0x0;
> +}
> +
> +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;
> +
> + mutex_lock(&streamlabs_wdt->lock);
> + retval = __usb_streamlabs_wdt_cmd(streamlabs_wdt, cmd);
> + mutex_unlock(&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;
> +
> + streamlabs_wdt->buffer = devm_kzalloc(&intf->dev, BUFFER_LENGTH,
> + GFP_KERNEL);
> + if (!streamlabs_wdt->buffer)
> + return -ENOMEM;
> +

Nit: buffer could be made part of struct streamlabs_wdt and be tagged with
____cacheline_aligned to avoid the double allocation.

> + mutex_init(&streamlabs_wdt->lock);
> +
> + 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);
> +
> + retval = usb_streamlabs_wdt_stop(&streamlabs_wdt->wdt_dev);
> + if (retval)
> + return -ENODEV;
> +

A comment explaining why the watchdog is explicitly stopped when running
might be useful.

> + 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);
> +
> + mutex_lock(&streamlabs_wdt->lock);
> + /*
> + * 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;
> + mutex_unlock(&streamlabs_wdt->lock);
> +}
> +
> +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,
> + .id_table = usb_streamlabs_wdt_device_table,
> + .soft_unbind = 1,
> +};
> +
> +module_usb_driver(usb_streamlabs_wdt_driver);

\
 
 \ /
  Last update: 2022-07-25 16:03    [W:0.146 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site