[PATCH v2 00/12] LIN Bus support for Linux

[PATCH v2 00/12] LIN Bus support for Linux

[Christoph Fritz]

This series is introducing basic Local Interconnect Network (LIN) (ISO
17987) [0] support to the Linux kernel, along with two drivers that make
use of it: An advanced USB adapter and a lightweight serdev driver (for
UARTs equipped with a LIN transceiver).

The LIN bus is common in the automotive industry for connecting
low-level devices like side mirrors, seats, ambient lights, etc.

The LIN bus is a lower-cost bus system with a subset of features of CAN.
Its earlier specification (before ISO) is publicly accessible [1].

This series of patches follows up on a discussion initiated by an RFC
patch series [2].

The core of this series is the first patch, which implements the CAN_LIN
glue driver. It basically utilizes the CAN interface on one side and
for device drivers on the other side it creates a rx() function and
several callbacks.

This approach is non-invasive, as LIN frames (nearly identical to CAN
frames) are just treated as a special case of CAN frames. This approach
eliminates the need for a separate API for LIN, allowing the use of
existing CAN tools, including the CAN broadcast manager.

For the responder part of LIN, when a device responds to a controller
request, it can reply on up to LIN its 64 possible IDs (0...63) with a
maximum of 8 bytes payload.  The response must be sent relatively
quickly, so offloading is used (which is used by most devices anyway).
Devices that do not support offloading (like the lightweight serdev)
handle the list of responses in the driver on a best-effort basis.

The CAN broadcast manager (bcm) makes a good interface for the LIN
userland interface, bcm is therefore enhanced to handle the
configuration of these offload RX frames, so that the device can handle
the response on its own.  As a basic alternative, a sysfs file per LIN
identifier gets also introduced.

The USB device driver for the hexLIN [3] adapter uses the HID protocol
and is located in the drivers/hid directory. Which is a bit uncommon for
a CAN device, but this is a LIN device and mainly a hid driver (and all
hid drivers go into drivers/hid).

The other driver, the UART lin-serdev driver requires support for break
detection, this is addressed by two serdev patches.

The lin-serdev driver has been tested on an ARM SoC, on its uart
(uart-pl011) an adapter board (hexLIN-tty [4]) has been used.  As a
sidenote, in that tty serial driver (amba-pl011.c) it was necessary to
disable DMA_ENGINE to accurately detect breaks [5].

The functions for generating LIN-Breaks and checksums, originally from
a line discipline driver named sllin [6], have been adopted into the
lin-serdev driver.

To make use of the LIN mode configuration (commander or responder)
option, a patch for iproute2 [7] has been made.

The lin-utils [8] provide userland tools for reference, testing, and
evaluation. These utilities are currently separate but may potentially
be integrated into can-utils in the future.

[0]: https://en.wikipedia.org/wiki/Local_Interconnect_Network
[1]: https://www.lin-cia.org/fileadmin/microsites/lin-cia.org/resources/documents/LIN_2.2A.pdf
[2]: https://lwn.net/Articles/916049/
[3]: https://hexdev.de/hexlin
[4]: https://hexdev.de/hexlin#tty
[5]: https://github.com/raspberrypi/linux/issues/5985
[6]: https://github.com/lin-bus/linux-lin/blob/master/sllin/sllin.c
[7]: https://github.com/ch-f/iproute2/tree/lin-feature
[8]: https://github.com/ch-f/lin-utils

Changes in v2:
 - add open/stop functions to also address teardown issues
 - adapt dt-bindings description and add hexdev
 - use 'unsigned int' instead of 'uint'
 - add and adapt macros
 - address review comments

Christoph Fritz (12):
  can: Add LIN bus as CAN abstraction
  HID: hexLIN: Add support for USB LIN bus adapter
  tty: serdev: Add flag buffer aware receive_buf_fp()
  tty: serdev: Add method to enable break flags
  dt-bindings: vendor-prefixes: Add hexDEV
  dt-bindings: net/can: Add serial (serdev) LIN adapter
  can: Add support for serdev LIN adapters
  can: lin: Add special frame id for rx offload config
  can: bcm: Add LIN answer offloading for responder mode
  can: lin: Handle rx offload config frames
  can: lin: Support setting LIN mode
  HID: hexLIN: Implement ability to update lin mode

 .../bindings/net/can/hexdev,lin-serdev.yaml   |  32 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/hid/Kconfig                           |  19 +
 drivers/hid/Makefile                          |   1 +
 drivers/hid/hid-hexdev-hexlin.c               | 641 ++++++++++++++++++
 drivers/hid/hid-ids.h                         |   1 +
 drivers/hid/hid-quirks.c                      |   3 +
 drivers/net/can/Kconfig                       |  26 +
 drivers/net/can/Makefile                      |   2 +
 drivers/net/can/lin-serdev.c                  | 514 ++++++++++++++
 drivers/net/can/lin.c                         | 562 +++++++++++++++
 drivers/tty/serdev/core.c                     |  11 +
 drivers/tty/serdev/serdev-ttyport.c           |  19 +-
 include/linux/serdev.h                        |  19 +-
 include/net/lin.h                             |  99 +++
 include/uapi/linux/can/bcm.h                  |   5 +-
 include/uapi/linux/can/netlink.h              |   2 +
 net/can/bcm.c                                 |  74 +-
 18 files changed, 2026 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
 create mode 100644 drivers/hid/hid-hexdev-hexlin.c
 create mode 100644 drivers/net/can/lin-serdev.c
 create mode 100644 drivers/net/can/lin.c
 create mode 100644 include/net/lin.h

-- 
2.39.2

[PATCH v2 01/12] can: Add LIN bus as CAN abstraction

[Christoph Fritz]

This patch adds a LIN (local interconnect network) bus abstraction on
top of CAN.  It is a glue driver adapting CAN on one side while offering
LIN abstraction on the other side. So that upcoming LIN device drivers
can make use of it.

Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/net/can/Kconfig          |  10 +
 drivers/net/can/Makefile         |   1 +
 drivers/net/can/lin.c            | 495 +++++++++++++++++++++++++++++++
 include/net/lin.h                |  91 ++++++
 include/uapi/linux/can/netlink.h |   1 +
 5 files changed, 598 insertions(+)
 create mode 100644 drivers/net/can/lin.c
 create mode 100644 include/net/lin.h

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 2e31db55d9278..0934bbf8d03b2 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -171,6 +171,16 @@ config CAN_KVASER_PCIEFD
 	    Kvaser M.2 PCIe 4xCAN
 	    Kvaser PCIe 8xCAN
 
+config CAN_LIN
+	tristate "LIN mode support"
+	help
+	  This is a glue driver for LIN-BUS support.
+
+	  The local interconnect (LIN) bus is a simple bus with a feature
+	  subset of CAN. It is often combined with CAN for simple controls.
+
+	  Actual device drivers need to be enabled too.
+
 config CAN_SLCAN
 	tristate "Serial / USB serial CAN Adaptors (slcan)"
 	depends on TTY
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 4669cd51e7bf5..0093ee9219ca8 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
 obj-$(CONFIG_CAN_IFI_CANFD)	+= ifi_canfd/
 obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
 obj-$(CONFIG_CAN_KVASER_PCIEFD)	+= kvaser_pciefd.o
+obj-$(CONFIG_CAN_LIN)		+= lin.o
 obj-$(CONFIG_CAN_MSCAN)		+= mscan/
 obj-$(CONFIG_CAN_M_CAN)		+= m_can/
 obj-$(CONFIG_CAN_PEAK_PCIEFD)	+= peak_canfd/
diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
new file mode 100644
index 0000000000000..95906003666fb
--- /dev/null
+++ b/drivers/net/can/lin.c
@@ -0,0 +1,495 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#include <linux/can/core.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <net/lin.h>
+
+static const u8 lin_id_parity_tbl[] = {
+	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
+	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
+	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
+	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
+	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
+	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
+	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
+	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
+};
+
+u8 lin_get_id_parity(u8 id)
+{
+	return lin_id_parity_tbl[id];
+}
+EXPORT_SYMBOL(lin_get_id_parity);
+
+static ssize_t lin_identifier_show(struct kobject *kobj,
+				   struct kobj_attribute *attr, char *buf)
+{
+	struct lin_attr *lin_attr = container_of(attr, struct lin_attr, attr);
+	struct lin_device *ldev = lin_attr->ldev;
+	ssize_t count = 0;
+	struct lin_responder_answer answ;
+	int k, ret;
+	long id;
+
+	if (!ldev->ldev_ops->get_responder_answer)
+		return -EOPNOTSUPP;
+
+	ret = kstrtol(attr->attr.name, 16, &id);
+	if (ret)
+		return ret;
+	if (id < 0 || id >= LIN_NUM_IDS)
+		return -EINVAL;
+
+	count += scnprintf(buf + count, PAGE_SIZE - count,
+			   "%-6s %-11s %-9s %-9s %-2s %-24s %-6s\n",
+			   "state", "cksum-mode", "is_event", "event_id",
+			   "n", "data", "cksum");
+
+	ret = ldev->ldev_ops->get_responder_answer(ldev, id, &answ);
+	if (ret)
+		return ret;
+
+	count += scnprintf(buf + count, PAGE_SIZE - count,
+			   "%-6s %-11s %-9s %-9u %-2u ",
+			   answ.is_active ? "active" : "off",
+			   answ.lf.checksum_mode ? "enhanced" : "classic",
+			   answ.is_event_frame ? "yes" : "no",
+			   answ.event_associated_id,
+			   answ.lf.len);
+
+	for (k = 0; k < answ.lf.len; k++)
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "%02x ", answ.lf.data[k]);
+	for (; k < 8; k++)
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "   ");
+	if (answ.lf.len)
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   " %02x", answ.lf.checksum);
+
+	count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
+
+	return count;
+}
+
+static const char *parse_and_advance(const char *buf, long *result,
+				     unsigned int base)
+{
+	char num_str[5] = {0};
+	int num_len = 0;
+
+	while (*buf && isspace(*buf))
+		buf++;
+	while (*buf && isalnum(*buf) && num_len < sizeof(num_str) - 1)
+		num_str[num_len++] = *buf++;
+	if (kstrtol(num_str, base, result))
+		return NULL;
+
+	return buf;
+}
+
+static ssize_t lin_identifier_store(struct kobject *kobj,
+				    struct kobj_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct lin_attr *lin_attr = container_of(attr, struct lin_attr, attr);
+	struct lin_device *ldev = lin_attr->ldev;
+	struct lin_responder_answer answ = { 0 };
+	const char *ptr = buf;
+	int ret;
+	long v;
+
+	if (!ldev->ldev_ops->update_responder_answer)
+		return -EOPNOTSUPP;
+
+	ret = kstrtol(attr->attr.name, 16, &v);
+	if (ret)
+		return ret;
+	if (v < 0 || v >= LIN_NUM_IDS)
+		return -EINVAL;
+	answ.lf.lin_id = v;
+
+	ptr = parse_and_advance(ptr, &v, 2);
+	if (!ptr)
+		return -EINVAL;
+	answ.is_active = v != 0;
+
+	ptr = parse_and_advance(ptr, &v, 2);
+	if (!ptr)
+		return -EINVAL;
+	answ.lf.checksum_mode = v != 0;
+
+	ptr = parse_and_advance(ptr, &v, 2);
+	if (!ptr)
+		return -EINVAL;
+	answ.is_event_frame = v != 0;
+
+	ptr = parse_and_advance(ptr, &v, 16);
+	if (!ptr || v > LIN_ID_MASK)
+		return -EINVAL;
+	answ.event_associated_id = v;
+
+	ptr = parse_and_advance(ptr, &v, 16);
+	if (!ptr || v > LIN_MAX_DLEN)
+		return -EINVAL;
+	answ.lf.len = v;
+
+	for (int i = 0; i < answ.lf.len; i++) {
+		ptr = parse_and_advance(ptr, &v, 16);
+		if (!ptr)
+			return -EINVAL;
+		answ.lf.data[i] = v;
+	}
+
+	ret = ldev->ldev_ops->update_responder_answer(ldev, &answ);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static int lin_create_sysfs_id_files(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	struct kobj_attribute *attr;
+	int ret;
+
+	for (int id = 0; id < LIN_NUM_IDS; id++) {
+		ldev->sysfs_entries[id].ldev = ldev;
+		attr = &ldev->sysfs_entries[id].attr;
+		attr->attr.name = kasprintf(GFP_KERNEL, "%02x", id);
+		if (!attr->attr.name)
+			return -ENOMEM;
+		attr->attr.mode = 0644;
+		attr->show = lin_identifier_show;
+		attr->store = lin_identifier_store;
+
+		sysfs_attr_init(&attr->attr);
+		ret = sysfs_create_file(ldev->lin_ids_kobj, &attr->attr);
+		if (ret) {
+			kfree(attr->attr.name);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
+static void lin_remove_sysfs_id_files(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	struct kobj_attribute *attr;
+
+	for (int id = 0; id < LIN_NUM_IDS; id++) {
+		attr = &ldev->sysfs_entries[id].attr;
+		sysfs_remove_file(ldev->lin_ids_kobj, &attr->attr);
+		kfree(attr->attr.name);
+	}
+}
+
+static void lin_tx_work_handler(struct work_struct *ws)
+{
+	struct lin_device *ldev = container_of(ws, struct lin_device,
+					       tx_work);
+	struct net_device *ndev = ldev->ndev;
+	struct canfd_frame *cfd;
+	struct lin_frame lf;
+	int ret;
+
+	ldev->tx_busy = true;
+
+	cfd = (struct canfd_frame *)ldev->tx_skb->data;
+	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
+			   LINBUS_ENHANCED : LINBUS_CLASSIC;
+	lf.lin_id = cfd->can_id & LIN_ID_MASK;
+	lf.len = min(cfd->len, LIN_MAX_DLEN);
+	memcpy(lf.data, cfd->data, lf.len);
+
+	ret = ldev->ldev_ops->ldo_tx(ldev, &lf);
+	if (ret) {
+		DEV_STATS_INC(ndev, tx_dropped);
+		netdev_err_once(ndev, "transmission failure %d\n", ret);
+		goto lin_tx_out;
+	}
+
+	DEV_STATS_INC(ndev, tx_packets);
+	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
+
+lin_tx_out:
+	ldev->tx_busy = false;
+	netif_wake_queue(ndev);
+}
+
+static netdev_tx_t lin_start_xmit(struct sk_buff *skb,
+				  struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+
+	if (ldev->tx_busy)
+		return NETDEV_TX_BUSY;
+
+	netif_stop_queue(ndev);
+	ldev->tx_skb = skb;
+	queue_work(ldev->wq, &ldev->tx_work);
+
+	return NETDEV_TX_OK;
+}
+
+static int lin_open(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	int ret;
+
+	ldev->tx_busy = false;
+
+	ret = open_candev(ndev);
+	if (ret)
+		return ret;
+
+	netif_wake_queue(ndev);
+
+	ldev->can.state = CAN_STATE_ERROR_ACTIVE;
+	ndev->mtu = CANFD_MTU;
+
+	return ldev->ldev_ops->ldo_open(ldev);
+}
+
+static int lin_stop(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+
+	close_candev(ndev);
+
+	flush_work(&ldev->tx_work);
+
+	ldev->can.state = CAN_STATE_STOPPED;
+
+	return ldev->ldev_ops->ldo_stop(ldev);
+}
+
+static const struct net_device_ops lin_netdev_ops = {
+	.ndo_open = lin_open,
+	.ndo_stop = lin_stop,
+	.ndo_start_xmit = lin_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
+};
+
+u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
+		    enum lin_checksum_mode cm)
+{
+	unsigned int csum = 0;
+	int i;
+
+	if (cm == LINBUS_ENHANCED)
+		csum += pid;
+
+	for (i = 0; i < n_of_bytes; i++) {
+		csum += bytes[i];
+		if (csum > 255)
+			csum -= 255;
+	}
+
+	return (~csum & 0xff);
+}
+EXPORT_SYMBOL_GPL(lin_get_checksum);
+
+static int lin_bump_rx_err(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct net_device *ndev = ldev->ndev;
+	struct can_frame cf = {0 };
+
+	if (lf->lin_id > LIN_ID_MASK) {
+		netdev_dbg(ndev, "id exceeds LIN max id\n");
+		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+		cf.data[3] = CAN_ERR_PROT_LOC_ID12_05;
+	}
+
+	if (lf->len > LIN_MAX_DLEN) {
+		netdev_dbg(ndev, "frame exceeds number of bytes\n");
+		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+		cf.data[3] = CAN_ERR_PROT_LOC_DLC;
+	}
+
+	if (lf->len) {
+		u8 checksum = lin_get_checksum(LIN_FORM_PID(lf->lin_id),
+					       lf->len, lf->data,
+					       lf->checksum_mode);
+
+		if (checksum != lf->checksum) {
+			netdev_dbg(ndev, "expected cksm: 0x%02x got: 0x%02x\n",
+				   checksum, lf->checksum);
+			cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+			cf.data[2] = CAN_ERR_PROT_FORM;
+		}
+	}
+
+	if (cf.can_id & CAN_ERR_FLAG) {
+		struct can_frame *err_cf;
+		struct sk_buff *skb = alloc_can_err_skb(ndev, &err_cf);
+
+		if (unlikely(!skb))
+			return -ENOMEM;
+
+		err_cf->can_id |= cf.can_id;
+		memcpy(err_cf->data, cf.data, CAN_MAX_DLEN);
+
+		netif_rx(skb);
+
+		return -EREMOTEIO;
+	}
+
+	return 0;
+}
+
+int lin_rx(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct net_device *ndev = ldev->ndev;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	int ret;
+
+	if (ldev->can.state == CAN_STATE_STOPPED)
+		return 0;
+
+	netdev_dbg(ndev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
+		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
+		   lf->checksum_mode ? "enhanced" : "classic");
+
+	ret = lin_bump_rx_err(ldev, lf);
+	if (ret) {
+		DEV_STATS_INC(ndev, rx_dropped);
+		return ret;
+	}
+
+	skb = alloc_can_skb(ndev, &cf);
+	if (unlikely(!skb)) {
+		DEV_STATS_INC(ndev, rx_dropped);
+		return -ENOMEM;
+	}
+
+	cf->can_id = lf->lin_id;
+	cf->len = min(lf->len, LIN_MAX_DLEN);
+	memcpy(cf->data, lf->data, cf->len);
+
+	DEV_STATS_INC(ndev, rx_packets);
+	DEV_STATS_ADD(ndev, rx_bytes, cf->len);
+
+	netif_receive_skb(skb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lin_rx);
+
+static int lin_set_bittiming(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	unsigned int bitrate = ldev->can.bittiming.bitrate;
+
+	return ldev->ldev_ops->update_bitrate(ldev, bitrate);
+}
+
+static const u32 lin_bitrate[] = { 1200, 2400, 4800, 9600, 19200 };
+
+struct lin_device *register_lin(struct device *dev,
+				const struct lin_device_ops *ldops)
+{
+	struct net_device *ndev;
+	struct lin_device *ldev;
+	int ret;
+
+	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||
+	    !ldops->ldo_open || !ldops->ldo_stop) {
+		netdev_err(ndev, "missing mandatory lin_device_ops\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	ndev = alloc_candev(sizeof(struct lin_device), 1);
+	if (!ndev)
+		return ERR_PTR(-ENOMEM);
+
+	ldev = netdev_priv(ndev);
+
+	ldev->ldev_ops = ldops;
+	ndev->netdev_ops = &lin_netdev_ops;
+	ndev->flags |= IFF_ECHO;
+	ndev->mtu = CANFD_MTU;
+	ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
+	ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
+	ldev->can.ctrlmode_supported = 0;
+	ldev->can.bitrate_const = lin_bitrate;
+	ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
+	ldev->can.do_set_bittiming = lin_set_bittiming;
+	ldev->ndev = ndev;
+	ldev->dev = dev;
+
+	SET_NETDEV_DEV(ndev, dev);
+
+	ret = lin_set_bittiming(ndev);
+	if (ret) {
+		netdev_err(ndev, "set bittiming failed\n");
+		goto exit_candev;
+	}
+
+	ret = register_candev(ndev);
+	if (ret)
+		goto exit_candev;
+
+	ldev->lin_ids_kobj = kobject_create_and_add("lin_ids", &ndev->dev.kobj);
+	if (!ldev->lin_ids_kobj) {
+		netdev_err(ndev, "Failed to create sysfs directory\n");
+		ret = -ENOMEM;
+		goto exit_unreg;
+	}
+
+	ret = lin_create_sysfs_id_files(ndev);
+	if (ret) {
+		netdev_err(ndev, "Failed to create sysfs entry: %d\n", ret);
+		goto exit_kobj_put;
+	}
+
+	/* Using workqueue as tx over USB/SPI/... may sleep */
+	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
+				   0);
+	if (!ldev->wq)
+		goto exit_rm_files;
+
+	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
+
+	netdev_info(ndev, "LIN initialized.\n");
+
+	return ldev;
+
+exit_rm_files:
+	lin_remove_sysfs_id_files(ndev);
+exit_kobj_put:
+	kobject_put(ldev->lin_ids_kobj);
+exit_unreg:
+	unregister_candev(ndev);
+exit_candev:
+	free_candev(ndev);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(register_lin);
+
+void unregister_lin(struct lin_device *ldev)
+{
+	struct net_device *ndev = ldev->ndev;
+
+	lin_remove_sysfs_id_files(ndev);
+	kobject_put(ldev->lin_ids_kobj);
+	unregister_candev(ndev);
+	destroy_workqueue(ldev->wq);
+	free_candev(ndev);
+}
+EXPORT_SYMBOL_GPL(unregister_lin);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
+MODULE_DESCRIPTION("LIN bus to CAN glue driver");
diff --git a/include/net/lin.h b/include/net/lin.h
new file mode 100644
index 0000000000000..944e775c40e79
--- /dev/null
+++ b/include/net/lin.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#ifndef _NET_LIN_H_
+#define _NET_LIN_H_
+
+#include <linux/bitfield.h>
+#include <linux/can/dev.h>
+#include <linux/device.h>
+
+#define LIN_NUM_IDS		64
+#define LIN_HEADER_SIZE		3
+#define LIN_MAX_DLEN		8
+
+#define LIN_MAX_BAUDRATE	20000
+#define LIN_MIN_BAUDRATE	1000
+#define LIN_DEFAULT_BAUDRATE	9600
+#define LIN_SYNC_BYTE		0x55
+
+#define LIN_ID_MASK		GENMASK(5, 0)
+/* special ID descriptions for LIN */
+#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
+
+extern u8 lin_get_id_parity(u8 id);
+
+#define LIN_GET_ID(PID)		FIELD_GET(LIN_ID_MASK, PID)
+#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
+					lin_get_id_parity(LIN_GET_ID(ID)))
+#define LIN_GET_PARITY(PID)	((PID) & ~LIN_ID_MASK)
+#define LIN_CHECK_PID(PID)	(LIN_GET_PARITY(PID) == \
+					LIN_GET_PARITY(LIN_FORM_PID(PID)))
+
+struct lin_attr {
+	struct kobj_attribute attr;
+	struct lin_device *ldev;
+};
+
+struct lin_device {
+	struct can_priv can;  /* must be the first member */
+	struct net_device *ndev;
+	struct device *dev;
+	const struct lin_device_ops *ldev_ops;
+	struct workqueue_struct *wq;
+	struct work_struct tx_work;
+	bool tx_busy;
+	struct sk_buff *tx_skb;
+	struct kobject *lin_ids_kobj;
+	struct lin_attr sysfs_entries[LIN_NUM_IDS];
+};
+
+enum lin_checksum_mode {
+	LINBUS_CLASSIC = 0,
+	LINBUS_ENHANCED,
+};
+
+struct lin_frame {
+	u8 lin_id;
+	u8 len;
+	u8 data[LIN_MAX_DLEN];
+	u8 checksum;
+	enum lin_checksum_mode checksum_mode;
+};
+
+struct lin_responder_answer {
+	bool is_active;
+	bool is_event_frame;
+	u8 event_associated_id;
+	struct lin_frame lf;
+};
+
+struct lin_device_ops {
+	int (*ldo_open)(struct lin_device *ldev);
+	int (*ldo_stop)(struct lin_device *ldev);
+	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
+	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
+	int (*update_responder_answer)(struct lin_device *ldev,
+				       const struct lin_responder_answer *answ);
+	int (*get_responder_answer)(struct lin_device *ldev, u8 id,
+				    struct lin_responder_answer *answ);
+};
+
+int lin_rx(struct lin_device *ldev, const struct lin_frame *lf);
+
+u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
+		    enum lin_checksum_mode cm);
+
+struct lin_device *register_lin(struct device *dev,
+				const struct lin_device_ops *ldops);
+void unregister_lin(struct lin_device *ldev);
+
+#endif /* _NET_LIN_H_ */
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index 02ec32d694742..51b0e2a7624e4 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -103,6 +103,7 @@ struct can_ctrlmode {
 #define CAN_CTRLMODE_CC_LEN8_DLC	0x100	/* Classic CAN DLC option */
 #define CAN_CTRLMODE_TDC_AUTO		0x200	/* CAN transiver automatically calculates TDCV */
 #define CAN_CTRLMODE_TDC_MANUAL		0x400	/* TDCV is manually set up by user */
+#define CAN_CTRLMODE_LIN		0x800	/* LIN bus mode */
 
 /*
  * CAN device statistics
-- 
2.39.2

[PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter

[Christoph Fritz]

This patch introduces driver support for the hexLIN USB LIN bus adapter,
enabling LIN communication over USB for both controller and responder
modes. The driver interfaces with the CAN_LIN framework for userland
connectivity.

For more details on the adapter, visit: https://hexdev.de/hexlin/

Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/hid/Kconfig             |  19 +
 drivers/hid/Makefile            |   1 +
 drivers/hid/hid-hexdev-hexlin.c | 630 ++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h           |   1 +
 drivers/hid/hid-quirks.c        |   3 +
 5 files changed, 654 insertions(+)
 create mode 100644 drivers/hid/hid-hexdev-hexlin.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4c682c6507040..4caff63d9dcda 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -496,6 +496,25 @@ config HID_GYRATION
 	help
 	Support for Gyration remote control.
 
+config HID_MCS_HEXDEV
+	tristate "hexDEV LIN-BUS adapter support"
+	depends on HID
+	select CAN_LIN
+	help
+	  Support for hexDEV its hexLIN USB LIN bus adapter.
+
+	  Local Interconnect Network (LIN) to USB adapter for controller and
+	  responder usage.
+	  This device driver is using CAN_LIN for a userland connection on
+	  one side and USB HID for the actual hardware adapter on the other
+	  side.
+
+	  If you have such an adapter, say Y here and see
+	  <https://hexdev.de/hexlin>.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hid-hexlin.
+
 config HID_ICADE
 	tristate "ION iCade arcade controller"
 	help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 082a728eac600..f9b13e6117e60 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_HID_GOOGLE_STADIA_FF)	+= hid-google-stadiaff.o
 obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
+obj-$(CONFIG_HID_MCS_HEXDEV)	+= hid-hexdev-hexlin.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
new file mode 100644
index 0000000000000..34019043f329e
--- /dev/null
+++ b/drivers/hid/hid-hexdev-hexlin.c
@@ -0,0 +1,630 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * LIN bus USB adapter driver https://hexdev.de/hexlin
+ *
+ * Copyright (C) 2024 hexDEV GmbH
+ */
+
+#include <linux/module.h>
+#include <linux/wait.h>
+#include <linux/completion.h>
+#include <linux/hid.h>
+#include <net/lin.h>
+#include "hid-ids.h"
+
+enum {
+	/* answers */
+	HEXLIN_SUCCESS = 0x01,
+	HEXLIN_FRAME = 0x02,
+	HEXLIN_ERROR = 0x03,
+	HEXLIN_FAIL = 0x0F,
+
+	/* lin-responder */
+	HEXLIN_SET_MODE_RESPONDER = 0x10,
+	HEXLIN_SET_RESPONDER_ANSWER_ID = 0x11,
+	HEXLIN_GET_RESPONDER_ANSWER_ID = 0x12,
+
+	/* lin-controller */
+	HEXLIN_SET_MODE_CONTROLLER = 0x20,
+	HEXLIN_SEND_BREAK = 0x21,
+	HEXLIN_SEND_UNCONDITIONAL_FRAME = 0x22,
+
+	/* lin-div */
+	HEXLIN_SET_BAUDRATE = 0x34,
+	HEXLIN_GET_BAUDRATE = 0x35,
+
+	/* div */
+	HEXLIN_RESET = 0xF0,
+	HEXLIN_GET_VERSION = 0xF1,
+};
+
+#define HEXLIN_SUCCESS_SZ			1
+#define HEXLIN_FRAME_SZ				17
+#define HEXLIN_FAIL_SZ				1
+#define HEXLIN_GET_RESPONDER_ANSWER_ID_SZ	20
+#define HEXLIN_GET_BAUDRATE_SZ			3
+#define HEXLIN_BAUDRATE_SZ			2
+#define HEXLIN_GET_VERSION_SZ			2
+#define HEXLIN_PKGLEN_MAX_SZ			64
+
+struct hexlin_val8_req {
+	u8 cmd;
+	u8 v;
+} __packed;
+
+struct hexlin_baudrate_req {
+	u8 cmd;
+	u16 baudrate;
+} __packed;
+
+struct hexlin_frame {
+	u32 flags;
+	u8 len;
+	u8 lin_id;
+	u8 data[LIN_MAX_DLEN];
+	u8 checksum;
+	u8 checksum_mode;
+} __packed;
+
+struct hexlin_unconditional_req {
+	u8 cmd;
+	struct hexlin_frame frm;
+} __packed;
+
+struct hexlin_responder_answer {
+	u8 is_active;
+	u8 is_event_frame;
+	u8 event_associated_id;
+	struct hexlin_frame frm;
+} __packed;
+
+struct hexlin_responder_answer_req {
+	u8 cmd;
+	struct hexlin_responder_answer answ;
+} __packed;
+
+struct hexlin_priv_data {
+	struct hid_device *hid_dev;
+	struct lin_device *ldev;
+	u16 baudrate;
+	struct completion wait_in_report;
+	bool is_error;
+	struct mutex tx_lock;  /* protects hexlin_tx_report() */
+	struct hexlin_responder_answer_req rar;
+	u8 fw_version;
+};
+
+static int hexlin_tx_report(struct hexlin_priv_data *priv,
+			    const void *out_report, size_t len)
+{
+	u8 *buf;
+	int ret;
+
+	buf = kmemdup(out_report, len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = hid_hw_output_report(priv->hid_dev, buf, len);
+	kfree(buf);
+
+	if (ret < 0)
+		return ret;
+	if (ret != len)
+		return -EIO;
+
+	return 0;
+}
+
+static int hexlin_tx_req_status(struct hexlin_priv_data *priv,
+				const void *out_report, int len)
+{
+	int ret;
+	unsigned long t;
+
+	mutex_lock(&priv->tx_lock);
+
+	reinit_completion(&priv->wait_in_report);
+
+	ret = hexlin_tx_report(priv, out_report, len);
+	if (ret)
+		goto tx_exit;
+
+	t = wait_for_completion_killable_timeout(&priv->wait_in_report,
+						 msecs_to_jiffies(1000));
+	if (!t)
+		ret = -ETIMEDOUT;
+
+	if (priv->is_error)
+		ret = -EINVAL;
+
+tx_exit:
+	mutex_unlock(&priv->tx_lock);
+
+	return ret;
+}
+
+#define HEXLIN_GET_CMD(name, enum_cmd)					\
+	static int hexlin_##name(struct hexlin_priv_data *priv)		\
+	{								\
+		u8 cmd = enum_cmd;					\
+		int ret;						\
+									\
+		ret = hexlin_tx_req_status(priv, &cmd, sizeof(u8));	\
+		if (ret)						\
+			hid_err(priv->hid_dev, "%s failed with %d\n",	\
+				__func__, ret);				\
+									\
+		return ret;						\
+	}
+
+HEXLIN_GET_CMD(get_version, HEXLIN_GET_VERSION)
+HEXLIN_GET_CMD(reset_dev, HEXLIN_RESET)
+HEXLIN_GET_CMD(get_baudrate, HEXLIN_GET_BAUDRATE)
+
+#define HEXLIN_VAL_CMD(name, enum_cmd, struct_type, vtype)		\
+	static int hexlin_##name(struct hexlin_priv_data *p, vtype val)	\
+	{								\
+		struct struct_type req;					\
+		int ret;						\
+									\
+		req.cmd = enum_cmd;					\
+		req.v = val;						\
+									\
+		ret = hexlin_tx_req_status(p, &req,			\
+					   sizeof(struct struct_type));	\
+		if (ret)						\
+			hid_err(p->hid_dev, "%s failed with %d\n",	\
+				__func__, ret);				\
+									\
+		return ret;						\
+	}
+
+HEXLIN_VAL_CMD(send_break, HEXLIN_SEND_BREAK, hexlin_val8_req, u8)
+
+static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
+				      const u8 *raw_data, int sz)
+{
+	struct hid_device *hdev = priv->hid_dev;
+	struct hexlin_frame hxf;
+	struct lin_frame lf;
+
+	if (sz != sizeof(struct hexlin_frame))
+		return -EREMOTEIO;
+
+	memcpy(&hxf, raw_data, sz);
+	le32_to_cpus(hxf.flags);
+
+	lf.len = hxf.len;
+	lf.lin_id = hxf.lin_id;
+	memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
+	lf.checksum = hxf.checksum;
+	lf.checksum_mode = hxf.checksum_mode;
+
+	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
+		   lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
+		   lf.checksum_mode ? "enhanced" : "classic");
+
+	lin_rx(priv->ldev, &lf);
+
+	return 0;
+}
+
+static int hexlin_send_unconditional(struct hexlin_priv_data *priv,
+			      const struct hexlin_frame *hxf)
+{
+	struct hexlin_unconditional_req req;
+	int ret;
+
+	if (hxf->lin_id > LIN_ID_MASK)
+		return -EINVAL;
+
+	req.cmd = HEXLIN_SEND_UNCONDITIONAL_FRAME;
+	memcpy(&req.frm, hxf, sizeof(struct hexlin_frame));
+
+	ret = hexlin_tx_req_status(priv, &req,
+				   sizeof(struct hexlin_unconditional_req));
+
+	if (ret)
+		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int hexlin_set_baudrate(struct hexlin_priv_data *priv, u16 baudrate)
+{
+	struct hexlin_baudrate_req req;
+	int ret;
+
+	if (baudrate < LIN_MIN_BAUDRATE || baudrate > LIN_MAX_BAUDRATE)
+		return -EINVAL;
+
+	req.cmd = HEXLIN_SET_BAUDRATE;
+	req.baudrate = cpu_to_le16(baudrate);
+
+	ret = hexlin_tx_req_status(priv, &req,
+				   sizeof(struct hexlin_baudrate_req));
+	if (ret)
+		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int hexlin_get_responder_answer_id(struct hexlin_priv_data *priv, u8 id,
+					  struct hexlin_responder_answer_req *rar)
+{
+	u8 req[2] = { HEXLIN_GET_RESPONDER_ANSWER_ID, id };
+	int ret;
+
+	if (id > LIN_ID_MASK)
+		return -EINVAL;
+
+	ret = hexlin_tx_req_status(priv, &req, sizeof(req));
+	if (ret) {
+		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
+		return ret;
+	}
+
+	memcpy(rar, &priv->rar, sizeof(struct hexlin_responder_answer_req));
+
+	return 0;
+}
+
+static int hexlin_set_responder_answer_id(struct hexlin_priv_data *priv,
+					  const struct lin_responder_answer *answ)
+{
+	struct hexlin_responder_answer_req rar;
+	int ret;
+
+	if (answ->lf.lin_id > LIN_ID_MASK ||
+	    answ->event_associated_id > LIN_ID_MASK)
+		return -EINVAL;
+
+	rar.cmd = HEXLIN_SET_RESPONDER_ANSWER_ID;
+	rar.answ.is_active = answ->is_active;
+	rar.answ.is_event_frame = answ->is_event_frame;
+	rar.answ.event_associated_id = answ->event_associated_id;
+	rar.answ.frm.len = answ->lf.len;
+	rar.answ.frm.lin_id = answ->lf.lin_id;
+	memcpy(rar.answ.frm.data, answ->lf.data, LIN_MAX_DLEN);
+	rar.answ.frm.checksum = answ->lf.checksum;
+	rar.answ.frm.checksum_mode = answ->lf.checksum_mode;
+
+	ret = hexlin_tx_req_status(priv, &rar,
+				   sizeof(struct hexlin_responder_answer_req));
+	if (ret)
+		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int hexlin_open(struct lin_device *ldev)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+
+	return hid_hw_open(hdev);
+}
+
+static int hexlin_stop(struct lin_device *ldev)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	hid_hw_close(hdev);
+
+	priv->is_error = true;
+	complete(&priv->wait_in_report);
+
+	mutex_lock(&priv->tx_lock);
+	mutex_unlock(&priv->tx_lock);
+
+	return 0;
+}
+
+static int hexlin_ldo_tx(struct lin_device *ldev,
+			 const struct lin_frame *lf)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	int ret = -EINVAL;
+
+	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
+		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
+		   lf->checksum_mode ? "enhanced" : "classic");
+
+	if (lf->lin_id && lf->len == 0) {
+		ret = hexlin_send_break(priv, lf->lin_id);
+	} else if (lf->len <= LIN_MAX_DLEN) {
+		struct hexlin_frame hxf;
+
+		hxf.len = lf->len;
+		hxf.lin_id = lf->lin_id;
+		memcpy(&hxf.data, lf->data, LIN_MAX_DLEN);
+		hxf.checksum = lf->checksum;
+		hxf.checksum_mode = lf->checksum_mode;
+		ret = hexlin_send_unconditional(priv, &hxf);
+	} else {
+		hid_err(hdev, "unknown format\n");
+	}
+
+	return ret;
+}
+
+static int hexlin_update_bitrate(struct lin_device *ldev, u16 bitrate)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	int ret;
+
+	hid_dbg(hdev, "update bitrate to: %u\n", bitrate);
+
+	ret = hexlin_open(ldev);
+	if (ret)
+		return ret;
+
+	ret = hexlin_set_baudrate(priv, bitrate);
+	if (ret)
+		return ret;
+
+	ret = hexlin_get_baudrate(priv);
+	if (ret)
+		return ret;
+
+	if (priv->baudrate != bitrate) {
+		hid_err(hdev, "update bitrate failed\n");
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int hexlin_get_responder_answer(struct lin_device *ldev, u8 id,
+				       struct lin_responder_answer *answ)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	struct hexlin_responder_answer_req rar;
+	int ret;
+
+	if (answ == NULL)
+		return -EINVAL;
+
+	ret = hexlin_get_responder_answer_id(priv, id, &rar);
+	if (ret)
+		return ret;
+
+	answ->is_active = rar.answ.is_active;
+	answ->is_event_frame = rar.answ.is_event_frame;
+	answ->event_associated_id = rar.answ.event_associated_id;
+	answ->lf.len = rar.answ.frm.len;
+	answ->lf.lin_id = rar.answ.frm.lin_id;
+	memcpy(answ->lf.data, rar.answ.frm.data, LIN_MAX_DLEN);
+	answ->lf.checksum = rar.answ.frm.checksum;
+	answ->lf.checksum_mode = rar.answ.frm.checksum_mode;
+
+	return 0;
+}
+
+static int hexlin_update_resp_answer(struct lin_device *ldev,
+				     const struct lin_responder_answer *answ)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	if (answ == NULL)
+		return -EINVAL;
+
+	return hexlin_set_responder_answer_id(priv, answ);
+}
+
+static const struct lin_device_ops hexlin_ldo = {
+	.ldo_open = hexlin_open,
+	.ldo_stop = hexlin_stop,
+	.ldo_tx = hexlin_ldo_tx,
+	.update_bitrate = hexlin_update_bitrate,
+	.get_responder_answer = hexlin_get_responder_answer,
+	.update_responder_answer = hexlin_update_resp_answer,
+};
+
+static int hexlin_raw_event(struct hid_device *hdev,
+			    struct hid_report *report, u8 *data, int sz)
+{
+	struct hexlin_priv_data *priv;
+	int ret;
+
+	if (sz < 1 || sz > HEXLIN_PKGLEN_MAX_SZ)
+		return -EREMOTEIO;
+
+	priv = hid_get_drvdata(hdev);
+
+	hid_dbg(hdev, "%s, size:%i, data[0]: 0x%02x\n", __func__, sz, data[0]);
+
+	priv->is_error = false;
+
+	switch (data[0]) {
+	case HEXLIN_SUCCESS:
+		if (sz != HEXLIN_SUCCESS_SZ)
+			return -EREMOTEIO;
+		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_FAIL:
+		if (sz != HEXLIN_FAIL_SZ)
+			return -EREMOTEIO;
+		hid_err(hdev, "HEXLIN_FAIL: 0x%02x\n", data[0]);
+		priv->is_error = true;
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_VERSION:
+		if (sz != HEXLIN_GET_VERSION_SZ)
+			return -EREMOTEIO;
+		priv->fw_version = data[1];
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_RESPONDER_ANSWER_ID:
+		if (sz != HEXLIN_GET_RESPONDER_ANSWER_ID_SZ)
+			return -EREMOTEIO;
+		BUILD_BUG_ON(sizeof(priv->rar) !=
+			HEXLIN_GET_RESPONDER_ANSWER_ID_SZ);
+		memcpy(&priv->rar, data, sizeof(priv->rar));
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_BAUDRATE:
+		if (sz != HEXLIN_GET_BAUDRATE_SZ)
+			return -EREMOTEIO;
+		BUILD_BUG_ON(sizeof(priv->baudrate) != HEXLIN_BAUDRATE_SZ);
+		memcpy(&priv->baudrate, &data[1], sizeof(priv->baudrate));
+		le16_to_cpus(priv->baudrate);
+		complete(&priv->wait_in_report);
+		break;
+	/* following cases not initiated by us, so no complete() */
+	case HEXLIN_FRAME:
+		if (sz != HEXLIN_FRAME_SZ) {
+			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
+			return -EREMOTEIO;
+		}
+		ret = hexlin_queue_frames_insert(priv, &data[1], sz-1);
+		if (ret) {
+			hid_err(hdev, "failed to add frame: %i\n", ret);
+			return ret;
+		}
+		break;
+	case HEXLIN_ERROR:
+		hid_err(hdev, "error from adapter\n");
+		break;
+	default:
+		hid_err(hdev, "unknown event: 0x%02x\n", data[0]);
+	}
+
+	return 0;
+}
+
+static int init_hw(struct hexlin_priv_data *priv)
+{
+	u8 dummy[] = { 0 };
+	int ret;
+
+	ret = hexlin_tx_report(priv, dummy, 1);
+	if (ret)
+		return ret;
+
+	ret = hexlin_reset_dev(priv);
+	if (ret)
+		return ret;
+
+	ret = hexlin_get_version(priv);
+	if (ret)
+		return ret;
+
+	priv->baudrate = LIN_DEFAULT_BAUDRATE;
+	ret = hexlin_set_baudrate(priv, priv->baudrate);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int hexlin_probe(struct hid_device *hdev,
+			const struct hid_device_id *id)
+{
+	struct hexlin_priv_data *priv;
+	int ret;
+
+	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->hid_dev = hdev;
+	hid_set_drvdata(hdev, priv);
+
+	mutex_init(&priv->tx_lock);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "hid parse failed with %d\n", ret);
+		goto fail_and_free;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (ret) {
+		hid_err(hdev, "hid hw start failed with %d\n", ret);
+		goto fail_and_stop;
+	}
+
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_err(hdev, "hid hw open failed with %d\n", ret);
+		goto fail_and_close;
+	}
+
+	init_completion(&priv->wait_in_report);
+
+	hid_device_io_start(hdev);
+
+	ret = init_hw(priv);
+	if (ret)
+		goto fail_and_close;
+
+	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
+	if (IS_ERR_OR_NULL(priv->ldev)) {
+		ret = PTR_ERR(priv->ldev);
+		goto fail_and_close;
+	}
+
+	hid_hw_close(hdev);
+
+	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
+
+	return 0;
+
+fail_and_close:
+	hid_hw_close(hdev);
+fail_and_stop:
+	hid_hw_stop(hdev);
+fail_and_free:
+	mutex_destroy(&priv->tx_lock);
+	return ret;
+}
+
+static void hexlin_remove(struct hid_device *hdev)
+{
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	unregister_lin(priv->ldev);
+	hid_hw_stop(hdev);
+	mutex_destroy(&priv->tx_lock);
+}
+
+static const struct hid_device_id hexlin_table[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(hid, hexlin_table);
+
+static struct hid_driver hexlin_driver = {
+	.name = "hexLIN",
+	.id_table = hexlin_table,
+	.probe = hexlin_probe,
+	.remove = hexlin_remove,
+	.raw_event = hexlin_raw_event,
+};
+
+static int __init hexlin_init(void)
+{
+	return hid_register_driver(&hexlin_driver);
+}
+
+static void __exit hexlin_exit(void)
+{
+	hid_unregister_driver(&hexlin_driver);
+}
+
+/*
+ * When compiled into the kernel, initialize after the hid bus.
+ */
+late_initcall(hexlin_init);
+module_exit(hexlin_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
+MODULE_DESCRIPTION("LIN bus driver for hexLIN USB adapter");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 64164423b592b..8f46d37c2b499 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -907,6 +907,7 @@
 
 #define USB_VENDOR_ID_MCS		0x16d0
 #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
+#define USB_DEVICE_ID_MCS_HEXDEV_HEXLIN	0x0648
 
 #define USB_VENDOR_MEGAWORLD		0x07b5
 #define USB_DEVICE_ID_MEGAWORLD_GAMEPAD	0x0312
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 1d1949d62dfaf..a514449c50047 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -438,6 +438,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE_2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE_3) },
 #endif
+#if IS_ENABLED(CONFIG_HID_MCS_HEXDEV)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
+#endif
 #if IS_ENABLED(CONFIG_HID_HOLTEK)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK, USB_DEVICE_ID_HOLTEK_ON_LINE_GRIP) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD) },
-- 
2.39.2

[PATCH v2 03/12] tty: serdev: Add flag buffer aware receive_buf_fp()

[Christoph Fritz]

This patch introduces an additional receive buffer callback variation
besides the already existing receive_buf(). This new callback function
also passes the flag buffer (TTY_NORMAL, TTY_BREAK, and friends).

If defined, this function gets prioritized and called instead of the
standard receive_buf().

An alternative approach could have been to enhance the receive_buf()
function and update all drivers that use it.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/tty/serdev/serdev-ttyport.c |  2 +-
 include/linux/serdev.h              | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 3d7ae7fa50186..bb47691afdb21 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -32,7 +32,7 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
 	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
 		return 0;
 
-	ret = serdev_controller_receive_buf(ctrl, cp, count);
+	ret = serdev_controller_receive_buf(ctrl, cp, fp, count);
 
 	dev_WARN_ONCE(&ctrl->dev, ret > count,
 				"receive_buf returns %zu (count = %zu)\n",
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index ff78efc1f60df..c6ef5a8988e07 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -23,11 +23,17 @@ struct serdev_device;
  * struct serdev_device_ops - Callback operations for a serdev device
  * @receive_buf:	Function called with data received from device;
  *			returns number of bytes accepted; may sleep.
+ * @receive_buf_fp:	Function called with data and flag buffer received
+ *			from device; If defined, this function gets called
+ *			instead of @receive_buf;
+ *			returns number of bytes accepted; may sleep.
  * @write_wakeup:	Function called when ready to transmit more data; must
  *			not sleep.
  */
 struct serdev_device_ops {
 	size_t (*receive_buf)(struct serdev_device *, const u8 *, size_t);
+	ssize_t (*receive_buf_fp)(struct serdev_device *, const u8 *,
+				  const u8 *, size_t);
 	void (*write_wakeup)(struct serdev_device *);
 };
 
@@ -186,15 +192,20 @@ static inline void serdev_controller_write_wakeup(struct serdev_controller *ctrl
 }
 
 static inline size_t serdev_controller_receive_buf(struct serdev_controller *ctrl,
-						   const u8 *data,
+						   const u8 *data, const u8 *fp,
 						   size_t count)
 {
 	struct serdev_device *serdev = ctrl->serdev;
 
-	if (!serdev || !serdev->ops->receive_buf)
+	if (!serdev || !serdev->ops)
 		return 0;
 
-	return serdev->ops->receive_buf(serdev, data, count);
+	if (serdev->ops->receive_buf_fp)
+		return serdev->ops->receive_buf_fp(serdev, data, fp, count);
+	else if (serdev->ops->receive_buf)
+		return serdev->ops->receive_buf(serdev, data, count);
+
+	return 0;
 }
 
 #if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
-- 
2.39.2

[PATCH v2 04/12] tty: serdev: Add method to enable break flags

[Christoph Fritz]

The recently introduced callback function receive_buf_fp() brings flags
buffer support. To allow signaling of TTY_BREAK flags, this patch
introduces serdev_device_set_break_detection() and an implementation for
ttyport. This enables serdev devices to configure their underlying tty
port to signal or ignore break conditions.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 17 +++++++++++++++++
 include/linux/serdev.h              |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 613cb356b918d..23a1e76cb553b 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -339,6 +339,17 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *serdev, unsigned i
 }
 EXPORT_SYMBOL_GPL(serdev_device_set_baudrate);
 
+void serdev_device_set_break_detection(struct serdev_device *serdev, bool enable)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->set_break_detection)
+		return;
+
+	ctrl->ops->set_break_detection(ctrl, enable);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_break_detection);
+
 void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
 {
 	struct serdev_controller *ctrl = serdev->ctrl;
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index bb47691afdb21..e928bf4175c6f 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -192,6 +192,22 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable
 	tty_set_termios(tty, &ktermios);
 }
 
+static void ttyport_set_break_detection(struct serdev_controller *ctrl, bool enable)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+	struct ktermios ktermios = tty->termios;
+
+	ktermios.c_iflag &= ~(IGNBRK | BRKINT);
+
+	if (enable)
+		ktermios.c_iflag |= BRKINT;
+	else
+		ktermios.c_iflag |= IGNBRK;
+
+	tty_set_termios(tty, &ktermios);
+}
+
 static int ttyport_set_parity(struct serdev_controller *ctrl,
 			      enum serdev_parity parity)
 {
@@ -263,6 +279,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.open = ttyport_open,
 	.close = ttyport_close,
 	.set_flow_control = ttyport_set_flow_control,
+	.set_break_detection = ttyport_set_break_detection,
 	.set_parity = ttyport_set_parity,
 	.set_baudrate = ttyport_set_baudrate,
 	.wait_until_sent = ttyport_wait_until_sent,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index c6ef5a8988e07..d08aa7e16f9f8 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -94,6 +94,7 @@ struct serdev_controller_ops {
 	int (*open)(struct serdev_controller *);
 	void (*close)(struct serdev_controller *);
 	void (*set_flow_control)(struct serdev_controller *, bool);
+	void (*set_break_detection)(struct serdev_controller *, bool);
 	int (*set_parity)(struct serdev_controller *, enum serdev_parity);
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
 	void (*wait_until_sent)(struct serdev_controller *, long);
@@ -215,6 +216,7 @@ void serdev_device_close(struct serdev_device *);
 int devm_serdev_device_open(struct device *, struct serdev_device *);
 unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
+void serdev_device_set_break_detection(struct serdev_device *, bool);
 int serdev_device_write_buf(struct serdev_device *, const u8 *, size_t);
 void serdev_device_wait_until_sent(struct serdev_device *, long);
 int serdev_device_get_tiocm(struct serdev_device *);
-- 
2.39.2

[PATCH v2 05/12] dt-bindings: vendor-prefixes: Add hexDEV

[Christoph Fritz]

Add vendor prefix for hexDEV GmbH. Website: https://hexdev.de

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 08b241825f8b6..1a8cfc5c7350d 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -601,6 +601,8 @@ patternProperties:
     description: Hardkernel Co., Ltd
   "^hechuang,.*":
     description: Shenzhen Hechuang Intelligent Co.
+  "^hexdev,.*":
+    description: hexDEV GmbH
   "^hideep,.*":
     description: HiDeep Inc.
   "^himax,.*":
-- 
2.39.2

[PATCH v2 06/12] dt-bindings: net/can: Add serial (serdev) LIN adapter

[Christoph Fritz]

This patch adds dt-bindings for serial LIN bus adapters. These adapters are
basically just LIN transceivers that get hard-wired with serial devices.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 .../bindings/net/can/hexdev,lin-serdev.yaml   | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml

diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
new file mode 100644
index 0000000000000..cbf9f63a2a49f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/hexdev,lin-serdev.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Serial LIN Adapter
+
+description:
+  LIN transceiver, mostly hard-wired to a serial device, used for communication
+  on a LIN bus.
+  For more details on an adapter, visit <https://hexdev.de/hexlin#tty>.
+
+maintainers:
+  - Christoph Fritz <christoph.fritz@hexdev.de>
+
+properties:
+  compatible:
+    const: hexdev,lin-serdev
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    serial {
+        linbus {
+            compatible = "linux,lin-serdev";
+        };
+    };
-- 
2.39.2

[PATCH v2 07/12] can: Add support for serdev LIN adapters

[Christoph Fritz]

This commit introduces LIN-Bus support for UART devices equipped with
LIN transceivers, utilizing the Serial Device Bus (serdev) interface.

For more details on an adapter, visit: https://hexdev.de/hexlin#tty

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/net/can/Kconfig      |  16 ++
 drivers/net/can/Makefile     |   1 +
 drivers/net/can/lin-serdev.c | 514 +++++++++++++++++++++++++++++++++++
 3 files changed, 531 insertions(+)
 create mode 100644 drivers/net/can/lin-serdev.c

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 0934bbf8d03b2..91c6cdeefe440 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -181,6 +181,22 @@ config CAN_LIN
 
 	  Actual device drivers need to be enabled too.
 
+config CAN_LIN_SERDEV
+	tristate "Serial LIN Adaptors"
+	depends on CAN_LIN && SERIAL_DEV_BUS && OF
+	help
+	  LIN-Bus support for serial devices equipped with LIN transceievers.
+	  This device driver is using CAN_LIN for a userland connection on
+	  one side and the kernel its serial device bus (serdev) interface
+	  on the other side.
+
+	  If you have a hexlin tty adapter, say Y here and see
+	  <https://hexdev.de/hexlin#tty>.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called lin-serdev.ko.
+
+
 config CAN_SLCAN
 	tristate "Serial / USB serial CAN Adaptors (slcan)"
 	depends on TTY
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 0093ee9219ca8..21ca514a42439 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CAN_IFI_CANFD)	+= ifi_canfd/
 obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
 obj-$(CONFIG_CAN_KVASER_PCIEFD)	+= kvaser_pciefd.o
 obj-$(CONFIG_CAN_LIN)		+= lin.o
+obj-$(CONFIG_CAN_LIN_SERDEV)	+= lin-serdev.o
 obj-$(CONFIG_CAN_MSCAN)		+= mscan/
 obj-$(CONFIG_CAN_M_CAN)		+= m_can/
 obj-$(CONFIG_CAN_PEAK_PCIEFD)	+= peak_canfd/
diff --git a/drivers/net/can/lin-serdev.c b/drivers/net/can/lin-serdev.c
new file mode 100644
index 0000000000000..4e763c615159c
--- /dev/null
+++ b/drivers/net/can/lin-serdev.c
@@ -0,0 +1,514 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#include <linux/module.h>
+#include <linux/wait.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <net/lin.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/workqueue.h>
+#include <linux/tty.h>
+
+#define LINSER_SAMPLES_PER_CHAR		10
+#define LINSER_TX_BUFFER_SIZE		11
+#define LINSER_RX_FIFO_SIZE		256
+#define LINSER_PARSE_BUFFER		24
+
+struct linser_rx {
+	u8 data;
+	u8 flag;
+};
+
+enum linser_rx_status {
+	NEED_MORE = -1,
+	MODE_OK = 0,
+	NEED_FORCE,
+};
+
+struct linser_priv {
+	struct lin_device *lin_dev;
+	struct serdev_device *serdev;
+	DECLARE_KFIFO_PTR(rx_fifo, struct linser_rx);
+	struct delayed_work rx_work;
+	ulong break_usleep_min;
+	ulong break_usleep_max;
+	ulong post_break_usleep_min;
+	ulong post_break_usleep_max;
+	ulong force_timeout_jfs;
+	struct lin_responder_answer respond_answ[LIN_NUM_IDS];
+	struct mutex resp_lock; /* protects respond_answ */
+	bool is_stopped;
+};
+
+static int linser_open(struct lin_device *ldev)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	int ret;
+
+	if (priv->is_stopped) {
+		ret = serdev_device_open(serdev);
+		if (ret) {
+			dev_err(&serdev->dev, "Unable to open device\n");
+			return ret;
+		}
+
+		serdev_device_set_flow_control(serdev, false);
+		serdev_device_set_break_detection(serdev, true);
+
+		priv->is_stopped = false;
+	}
+
+	return 0;
+}
+
+static int linser_stop(struct lin_device *ldev)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+
+	if (priv->is_stopped)
+		return 0;
+
+	serdev_device_close(serdev);
+	priv->is_stopped = true;
+
+	return 0;
+}
+
+static int linser_send_break(struct linser_priv *priv)
+{
+	struct serdev_device *serdev = priv->serdev;
+	int ret;
+
+	ret = serdev_device_break_ctl(serdev, -1);
+	if (ret)
+		return ret;
+
+	usleep_range(priv->break_usleep_min, priv->break_usleep_max);
+
+	ret = serdev_device_break_ctl(serdev, 0);
+	if (ret)
+		return ret;
+
+	usleep_range(priv->post_break_usleep_min, priv->post_break_usleep_max);
+
+	return 0;
+}
+
+static int linser_ldo_tx(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	u8 pid = LIN_FORM_PID(lf->lin_id);
+	u8 buf[LINSER_TX_BUFFER_SIZE];
+	ssize_t written_len, total_len;
+	u8 checksum;
+	int ret;
+
+	if (lf->len + 3 > LINSER_TX_BUFFER_SIZE) {
+		dev_err(&serdev->dev, "Frame length %u exceeds buffer size\n", lf->len);
+		return -EINVAL;
+	}
+
+	buf[0] = LIN_SYNC_BYTE;
+	buf[1] = pid;
+	total_len = 2;
+
+	if (lf->len) {
+		memcpy(&buf[2], lf->data, lf->len);
+		checksum = lin_get_checksum(pid, lf->len, lf->data,
+					    lf->checksum_mode);
+		buf[lf->len + 2] = checksum;
+		total_len += lf->len + 1;
+	}
+
+	ret = linser_send_break(priv);
+	if (ret)
+		return ret;
+
+	written_len = serdev_device_write(serdev, buf, total_len, 0);
+	if (written_len < total_len)
+		return written_len < 0 ? (int)written_len : -EIO;
+
+	dev_dbg(&serdev->dev, "sent out: %*ph\n", (int)total_len, buf);
+
+	serdev_device_wait_until_sent(serdev, 0);
+
+	return 0;
+}
+
+static void linser_derive_timings(struct linser_priv *priv, u16 bitrate)
+{
+	unsigned long break_baud = (bitrate * 2) / 3;
+	unsigned long timeout_us;
+
+	priv->break_usleep_min = (1000000UL * LINSER_SAMPLES_PER_CHAR) /
+				 break_baud;
+	priv->break_usleep_max = priv->break_usleep_min + 50;
+	priv->post_break_usleep_min = (1000000UL * 1 /* 1 bit */) / break_baud;
+	priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
+
+	timeout_us = DIV_ROUND_CLOSEST(1000000UL * 256 /* bit */, bitrate);
+	priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
+}
+
+static int linser_update_bitrate(struct lin_device *ldev, u16 bitrate)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	unsigned int speed;
+	int ret;
+
+	ret = linser_open(ldev);
+	if (ret)
+		return ret;
+
+	speed = serdev_device_set_baudrate(serdev, bitrate);
+	if (!bitrate || speed != bitrate)
+		return -EINVAL;
+
+	linser_derive_timings(priv, bitrate);
+
+	return 0;
+}
+
+static int linser_get_responder_answer(struct lin_device *ldev, u8 id,
+				       struct lin_responder_answer *answ)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	struct lin_responder_answer *r = &priv->respond_answ[id];
+
+	if (!answ)
+		return -EINVAL;
+
+	mutex_lock(&priv->resp_lock);
+	memcpy(answ, r, sizeof(struct lin_responder_answer));
+	mutex_unlock(&priv->resp_lock);
+
+	return 0;
+}
+
+static int linser_update_resp_answer(struct lin_device *ldev,
+				     const struct lin_responder_answer *answ)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	struct lin_responder_answer *r = &priv->respond_answ[answ->lf.lin_id];
+
+	if (!answ)
+		return -EINVAL;
+
+	mutex_lock(&priv->resp_lock);
+	memcpy(r, answ, sizeof(struct lin_responder_answer));
+	r->lf.checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
+					  answ->lf.len,
+					  answ->lf.data,
+					  answ->lf.checksum_mode);
+	mutex_unlock(&priv->resp_lock);
+
+	return 0;
+}
+
+static struct lin_device_ops linser_lindev_ops = {
+	.ldo_open = linser_open,
+	.ldo_stop = linser_stop,
+	.ldo_tx = linser_ldo_tx,
+	.update_bitrate = linser_update_bitrate,
+	.get_responder_answer = linser_get_responder_answer,
+	.update_responder_answer = linser_update_resp_answer,
+};
+
+static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
+{
+	struct lin_responder_answer *answ = &priv->respond_answ[id];
+	struct serdev_device *serdev = priv->serdev;
+	u8 buf[LINSER_TX_BUFFER_SIZE];
+	u8 checksum, count, n;
+	ssize_t write_len;
+
+	mutex_lock(&priv->resp_lock);
+
+	if (!answ->is_active)
+		goto unlock_and_exit_false;
+
+	if (answ->is_event_frame) {
+		struct lin_responder_answer *e_answ;
+
+		e_answ = &priv->respond_answ[answ->event_associated_id];
+		n = min(e_answ->lf.len, LIN_MAX_DLEN);
+		if (memcmp(answ->lf.data, e_answ->lf.data, n) != 0) {
+			memcpy(answ->lf.data, e_answ->lf.data, n);
+			checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
+						    n, e_answ->lf.data,
+						    answ->lf.checksum_mode);
+			answ = e_answ;
+		} else {
+			goto unlock_and_exit_false;
+		}
+	} else {
+		checksum = answ->lf.checksum;
+	}
+
+	count = min(answ->lf.len, LIN_MAX_DLEN);
+	memcpy(&buf[0], answ->lf.data, count);
+	buf[count] = checksum;
+
+	mutex_unlock(&priv->resp_lock);
+
+	write_len = serdev_device_write(serdev, buf, count + 1, 0);
+	if (write_len < count + 1)
+		return false;
+
+	serdev_device_wait_until_sent(serdev, 0);
+
+	return true;
+
+unlock_and_exit_false:
+	mutex_unlock(&priv->resp_lock);
+	return false;
+}
+
+static void linser_pop_fifo(struct linser_priv *priv, size_t n)
+{
+	struct serdev_device *serdev = priv->serdev;
+	struct linser_rx dummy;
+	size_t ret, i;
+
+	for (i = 0; i < n; i++) {
+		ret = kfifo_out(&priv->rx_fifo, &dummy, 1);
+		if (ret != 1) {
+			dev_err(&serdev->dev, "Failed to pop from FIFO\n");
+			break;
+		}
+	}
+}
+
+static int linser_fill_frame(struct linser_priv *priv, struct lin_frame *lf)
+{
+	struct serdev_device *serdev = priv->serdev;
+	struct linser_rx buf[LINSER_PARSE_BUFFER];
+	unsigned int count, i, brk = 0;
+
+	count = kfifo_out_peek(&priv->rx_fifo, buf, LINSER_PARSE_BUFFER);
+
+	memset(lf, 0, sizeof(struct lin_frame));
+
+	for (i = 0; i < count; i++) {
+		dev_dbg(&serdev->dev, "buf[%d]: data=%02x, flag=%02x\n",
+			i, buf[i].data, buf[i].flag);
+	}
+
+	if (count < 3)
+		return NEED_MORE;
+
+	if (buf[0].flag != TTY_BREAK || buf[1].data != LIN_SYNC_BYTE) {
+		linser_pop_fifo(priv, 1); /* pop incorrect start */
+		return NEED_MORE;
+	} else if (!LIN_CHECK_PID(buf[2].data)) {
+		linser_pop_fifo(priv, 3); /* pop incorrect header */
+		return NEED_MORE;
+	}
+
+	lf->lin_id = LIN_GET_ID(buf[2].data);
+
+	/* from here on we do have a correct LIN header */
+
+	if (count == 3)
+		return linser_tx_frame_as_responder(priv, lf->lin_id) ?
+		       NEED_MORE : NEED_FORCE;
+
+	for (i = 3; i < count && i < LINSER_PARSE_BUFFER && i < 12; i++) {
+		if (buf[i].flag == TTY_BREAK) {
+			brk = i;
+			break;
+		}
+		lf->len++;
+	}
+	if (lf->len)
+		lf->len -= 1; /* account for checksum */
+
+	if (brk == 3)
+		return MODE_OK;
+
+	if (brk == 4) {
+		/* suppress wrong answer data-byte in between PID and break
+		 * because checksum is missing
+		 */
+		return MODE_OK;
+	}
+
+	for (i = 0; i < lf->len; i++)
+		lf->data[i] = buf[3 + i].data;
+	lf->checksum = buf[2 + lf->len + 1].data;
+	mutex_lock(&priv->resp_lock);
+	lf->checksum_mode = priv->respond_answ[lf->lin_id].lf.checksum_mode;
+	mutex_unlock(&priv->resp_lock);
+
+	dev_dbg(&serdev->dev, "brk:%i, len:%u, data:%*ph, checksum:%x (%s)\n",
+		brk, lf->len, lf->len, lf->data, lf->checksum,
+		lf->checksum_mode ? "enhanced" : "classic");
+
+	if (brk > 4)
+		return MODE_OK;	/* frame in between two breaks: so complete */
+
+	if (lf->len == 8)
+		return MODE_OK;
+
+	return NEED_FORCE;
+}
+
+static int linser_process_frame(struct linser_priv *priv, bool force)
+{
+	struct serdev_device *serdev = priv->serdev;
+	struct lin_frame lf;
+	size_t bytes_to_pop;
+	int ret = NEED_MORE;
+
+	while (kfifo_len(&priv->rx_fifo) >= LIN_HEADER_SIZE) {
+		ret = linser_fill_frame(priv, &lf);
+
+		if (ret == MODE_OK || (ret == NEED_FORCE && force)) {
+			dev_dbg(&serdev->dev, "lin_rx: %s\n",
+				force ? "force" : "normal");
+			lin_rx(priv->lin_dev, &lf);
+			bytes_to_pop = LIN_HEADER_SIZE + lf.len +
+				       (lf.len ? 1 : 0);
+			linser_pop_fifo(priv, bytes_to_pop);
+			force = false;
+			ret = MODE_OK;
+		} else {
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static void linser_process_delayed(struct work_struct *work)
+{
+	struct linser_priv *priv = container_of(work, struct linser_priv,
+						rx_work.work);
+
+	linser_process_frame(priv, true);
+}
+
+static ssize_t linser_receive_buf_fp(struct serdev_device *serdev,
+				     const u8 *data, const u8 *fp,
+				     size_t count)
+{
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	enum linser_rx_status rx_status;
+	ssize_t n = 0;
+	int i;
+
+	cancel_delayed_work_sync(&priv->rx_work);
+
+	for (i = 0; i < count; i++) {
+		struct linser_rx rx;
+
+		rx.data = data[i];
+		rx.flag = (fp ? fp[i] : 0);
+		n += kfifo_in(&priv->rx_fifo, &rx, 1);
+		dev_dbg(&serdev->dev, "%s: n:%zd, flag:0x%02x, data:0x%02x\n",
+			__func__, n, rx.flag, data[i]);
+	}
+
+	rx_status = linser_process_frame(priv, false);
+
+	if (rx_status == NEED_FORCE)
+		schedule_delayed_work(&priv->rx_work, priv->force_timeout_jfs);
+
+	return n;
+}
+
+static const struct serdev_device_ops linser_ops = {
+	.receive_buf_fp = linser_receive_buf_fp,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+static int linser_probe(struct serdev_device *serdev)
+{
+	struct linser_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&serdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = kfifo_alloc(&priv->rx_fifo, LINSER_RX_FIFO_SIZE, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	INIT_DELAYED_WORK(&priv->rx_work, linser_process_delayed);
+
+	priv->serdev = serdev;
+	serdev_device_set_drvdata(serdev, priv);
+	serdev_device_set_client_ops(serdev, &linser_ops);
+
+	ret = serdev_device_open(serdev);
+	if (ret) {
+		dev_err(&serdev->dev, "Unable to open device\n");
+		goto err_open;
+	}
+
+	serdev_device_set_flow_control(serdev, false);
+	serdev_device_set_break_detection(serdev, true);
+	linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
+
+	mutex_init(&priv->resp_lock);
+
+	priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
+	if (IS_ERR_OR_NULL(priv->lin_dev)) {
+		ret = PTR_ERR(priv->lin_dev);
+		goto err_register_lin;
+	}
+
+	serdev_device_close(serdev);
+	priv->is_stopped = true;
+
+	return 0;
+
+err_register_lin:
+	serdev_device_close(serdev);
+err_open:
+	kfifo_free(&priv->rx_fifo);
+	return ret;
+}
+
+static void linser_remove(struct serdev_device *serdev)
+{
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+
+	unregister_lin(priv->lin_dev);
+}
+
+static const struct of_device_id linser_of_match[] = {
+	{
+		.compatible = "hexdev,lin-serdev",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, linser_of_match);
+
+static struct serdev_device_driver linser_driver = {
+	.probe = linser_probe,
+	.remove = linser_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = linser_of_match,
+	}
+};
+
+module_serdev_device_driver(linser_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
+MODULE_DESCRIPTION("LIN-Bus serdev driver");
-- 
2.39.2

[PATCH v2 08/12] can: lin: Add special frame id for rx offload config

[Christoph Fritz]

A LIN bus supports up to 64 identifiers in one byte. This commit adds a
special frame ID, beyond the actual LIN identifiers, for signaling RX
offload configuration requests. This ID will be utilized in future LIN
enhancements to the CAN broadcast manager.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 include/net/lin.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/lin.h b/include/net/lin.h
index 944e775c40e79..e7c7c820a6e18 100644
--- a/include/net/lin.h
+++ b/include/net/lin.h
@@ -19,6 +19,7 @@
 
 #define LIN_ID_MASK		GENMASK(5, 0)
 /* special ID descriptions for LIN */
+#define LIN_RXOFFLOAD_DATA_FLAG	0x00000200U
 #define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
 
 extern u8 lin_get_id_parity(u8 id);
-- 
2.39.2

[PATCH v2 09/12] can: bcm: Add LIN answer offloading for responder mode

[Christoph Fritz]

Enhance CAN broadcast manager with RX_LIN_SETUP and RX_LIN_DELETE
operations to setup automatic LIN frame responses in responder mode.

Additionally, the patch introduces the LIN_EVENT_FRAME flag to
setup event-triggered LIN frames.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 include/uapi/linux/can/bcm.h |  5 ++-
 net/can/bcm.c                | 74 +++++++++++++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h
index f1e45f533a72c..c46268a114078 100644
--- a/include/uapi/linux/can/bcm.h
+++ b/include/uapi/linux/can/bcm.h
@@ -86,7 +86,9 @@ enum {
 	TX_EXPIRED,	/* notification on performed transmissions (count=0) */
 	RX_STATUS,	/* reply to RX_READ request */
 	RX_TIMEOUT,	/* cyclic message is absent */
-	RX_CHANGED	/* updated CAN frame (detected content change) */
+	RX_CHANGED,	/* updated CAN frame (detected content change) */
+	RX_LIN_SETUP,	/* create auto-response for LIN frame */
+	RX_LIN_DELETE,  /* remove auto-response for LIN frame */
 };
 
 #define SETTIMER            0x0001
@@ -101,5 +103,6 @@ enum {
 #define TX_RESET_MULTI_IDX  0x0200
 #define RX_RTR_FRAME        0x0400
 #define CAN_FD_FRAME        0x0800
+#define LIN_EVENT_FRAME     0x1000
 
 #endif /* !_UAPI_CAN_BCM_H */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 27d5fcf0eac9d..a717e594234d1 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -59,6 +59,7 @@
 #include <linux/can/bcm.h>
 #include <linux/slab.h>
 #include <net/sock.h>
+#include <net/lin.h>
 #include <net/net_namespace.h>
 
 /*
@@ -1330,6 +1331,59 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk,
 	return cfsiz + MHSIZ;
 }
 
+static int bcm_lin_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
+			 int ifindex, struct sock *sk, int cfsiz, int is_active)
+{
+	struct lin_responder_answer answ;
+	struct net_device *dev;
+	struct sk_buff *skb;
+	struct canfd_frame cf;
+	netdevice_tracker tracker;
+	size_t sz;
+	int ret;
+
+	if (msg_head->nframes > 1)
+		return -EINVAL;
+
+	if (!(msg_head->flags & CAN_FD_FRAME))
+		return -EINVAL;
+
+	ret = memcpy_from_msg(&cf, msg, cfsiz);
+	if (ret < 0)
+		return ret;
+
+	answ.lf.lin_id = cf.can_id & LIN_ID_MASK;
+	answ.is_active = is_active;
+	answ.is_event_frame = !!(msg_head->flags & LIN_EVENT_FRAME);
+	answ.event_associated_id = msg_head->can_id;
+	answ.lf.len = min(cf.len, LIN_MAX_DLEN);
+	memcpy(answ.lf.data, cf.data, answ.lf.len);
+	sz = min(sizeof(struct lin_responder_answer), sizeof(cf.data));
+	cf.can_id |= LIN_RXOFFLOAD_DATA_FLAG;
+	memcpy(cf.data, &answ, sz);
+
+	dev = netdev_get_by_index(sock_net(sk), ifindex, &tracker, GFP_KERNEL);
+	if (!dev)
+		return -ENODEV;
+
+	skb = alloc_skb(cfsiz + sizeof(struct can_skb_priv), gfp_any());
+	if (!skb)
+		goto lin_out;
+
+	can_skb_reserve(skb);
+	can_skb_prv(skb)->ifindex = dev->ifindex;
+	can_skb_prv(skb)->skbcnt = 0;
+	skb_put_data(skb, &cf, cfsiz);
+
+	skb->dev = dev;
+	can_skb_set_owner(skb, sk);
+	ret = can_send(skb, 1); /* send with loopback */
+
+lin_out:
+	netdev_put(dev, &tracker);
+	return ret;
+}
+
 /*
  * bcm_sendmsg - process BCM commands (opcodes) from the userspace
  */
@@ -1429,12 +1483,30 @@ static int bcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 
 	case TX_SEND:
 		/* we need exactly one CAN frame behind the msg head */
-		if ((msg_head.nframes != 1) || (size != cfsiz + MHSIZ))
+		if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
 			ret = -EINVAL;
 		else
 			ret = bcm_tx_send(msg, ifindex, sk, cfsiz);
 		break;
 
+	case RX_LIN_SETUP:
+		/* we need exactly one CAN frame behind the msg head */
+		if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
+			ret = -EINVAL;
+		else
+			ret = bcm_lin_setup(&msg_head, msg, ifindex, sk, cfsiz,
+					    1);
+		break;
+
+	case RX_LIN_DELETE:
+		/* we need exactly one CAN frame behind the msg head */
+		if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
+			ret = -EINVAL;
+		else
+			ret = bcm_lin_setup(&msg_head, msg, ifindex, sk, cfsiz,
+					    0);
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
-- 
2.39.2

[PATCH v2 10/12] can: lin: Handle rx offload config frames

[Christoph Fritz]

The CAN Broadcast Manager now has the capability to dispatch CANFD
frames marked with the id LINBUS_RXOFFLOAD_ID. This patch introduces
functionality to interpret these specific frames, enabling the
configuration of RX offloading within the LIN driver.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/net/can/lin.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
index 95906003666fb..ee2ebea2c865f 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -194,6 +194,27 @@ static void lin_remove_sysfs_id_files(struct net_device *ndev)
 	}
 }
 
+static int lin_setup_rxoffload(struct lin_device *ldev,
+			       struct canfd_frame *cfd)
+{
+	struct lin_responder_answer answ;
+
+	if (!(cfd->flags & CANFD_FDF))
+		return -EMSGSIZE;
+
+	BUILD_BUG_ON(sizeof(struct lin_responder_answer) > sizeof(cfd->data));
+	memcpy(&answ, cfd->data, sizeof(struct lin_responder_answer));
+
+	answ.lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
+			LINBUS_ENHANCED : LINBUS_CLASSIC;
+
+	if (answ.lf.lin_id > LIN_ID_MASK ||
+	    answ.event_associated_id > LIN_ID_MASK)
+		return -EINVAL;
+
+	return ldev->ldev_ops->update_responder_answer(ldev, &answ);
+}
+
 static void lin_tx_work_handler(struct work_struct *ws)
 {
 	struct lin_device *ldev = container_of(ws, struct lin_device,
@@ -206,6 +227,14 @@ static void lin_tx_work_handler(struct work_struct *ws)
 	ldev->tx_busy = true;
 
 	cfd = (struct canfd_frame *)ldev->tx_skb->data;
+
+	if (cfd->can_id & LIN_RXOFFLOAD_DATA_FLAG) {
+		ret = lin_setup_rxoffload(ldev, cfd);
+		if (ret < 0)
+			netdev_err(ndev, "setting up rx failed %d\n", ret);
+		goto lin_tx_out;
+	}
+
 	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
 			   LINBUS_ENHANCED : LINBUS_CLASSIC;
 	lf.lin_id = cfd->can_id & LIN_ID_MASK;
-- 
2.39.2

[PATCH v2 11/12] can: lin: Support setting LIN mode

[Christoph Fritz]

A LIN node can work as commander or responder. This patch is introducing
a new control mode (CAN_CTRLMODE_LIN_COMMANDER), so that e.g. the ip
tool from iproute2 can turn on commander mode when the device is being
brought up.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/net/can/lin.c            | 40 +++++++++++++++++++++++++++++++-
 include/net/lin.h                |  7 ++++++
 include/uapi/linux/can/netlink.h |  1 +
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
index ee2ebea2c865f..96cd016228fea 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -271,11 +271,40 @@ static netdev_tx_t lin_start_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
+static int lin_update_mode(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	u32 ctrlmode = ldev->can.ctrlmode;
+	enum lin_mode lm;
+	int ret = 0;
+
+	lm = (ctrlmode & CAN_CTRLMODE_LIN_COMMANDER) ? LINBUS_COMMANDER :
+						       LINBUS_RESPONDER;
+	if (ldev->lmode != lm) {
+		if (!ldev->ldev_ops->update_lin_mode) {
+			netdev_err(ndev, "setting lin mode unsupported\n");
+			return -EINVAL;
+		}
+		ret = ldev->ldev_ops->update_lin_mode(ldev, lm);
+		if (ret) {
+			netdev_err(ndev, "Failed to set lin mode: %d\n", ret);
+			return ret;
+		}
+		ldev->lmode = lm;
+	}
+
+	return ret;
+}
+
 static int lin_open(struct net_device *ndev)
 {
 	struct lin_device *ldev = netdev_priv(ndev);
 	int ret;
 
+	ret = lin_update_mode(ndev);
+	if (ret)
+		return ret;
+
 	ldev->tx_busy = false;
 
 	ret = open_candev(ndev);
@@ -451,7 +480,7 @@ struct lin_device *register_lin(struct device *dev,
 	ndev->mtu = CANFD_MTU;
 	ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
 	ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
-	ldev->can.ctrlmode_supported = 0;
+	ldev->can.ctrlmode_supported = CAN_CTRLMODE_LIN_COMMANDER;
 	ldev->can.bitrate_const = lin_bitrate;
 	ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
 	ldev->can.do_set_bittiming = lin_set_bittiming;
@@ -466,6 +495,15 @@ struct lin_device *register_lin(struct device *dev,
 		goto exit_candev;
 	}
 
+	ldev->lmode = LINBUS_RESPONDER;
+	if (ldev->ldev_ops->update_lin_mode) {
+		ret = ldev->ldev_ops->update_lin_mode(ldev, ldev->lmode);
+		if (ret) {
+			netdev_err(ndev, "updating lin mode failed\n");
+			goto exit_candev;
+		}
+	}
+
 	ret = register_candev(ndev);
 	if (ret)
 		goto exit_candev;
diff --git a/include/net/lin.h b/include/net/lin.h
index e7c7c820a6e18..e80a4509b7a8c 100644
--- a/include/net/lin.h
+++ b/include/net/lin.h
@@ -36,6 +36,11 @@ struct lin_attr {
 	struct lin_device *ldev;
 };
 
+enum lin_mode {
+	LINBUS_RESPONDER = 0,
+	LINBUS_COMMANDER,
+};
+
 struct lin_device {
 	struct can_priv can;  /* must be the first member */
 	struct net_device *ndev;
@@ -47,6 +52,7 @@ struct lin_device {
 	struct sk_buff *tx_skb;
 	struct kobject *lin_ids_kobj;
 	struct lin_attr sysfs_entries[LIN_NUM_IDS];
+	enum lin_mode lmode;
 };
 
 enum lin_checksum_mode {
@@ -73,6 +79,7 @@ struct lin_device_ops {
 	int (*ldo_open)(struct lin_device *ldev);
 	int (*ldo_stop)(struct lin_device *ldev);
 	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
+	int (*update_lin_mode)(struct lin_device *ldev, enum lin_mode lm);
 	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
 	int (*update_responder_answer)(struct lin_device *ldev,
 				       const struct lin_responder_answer *answ);
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index 51b0e2a7624e4..6c84a7666c646 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -104,6 +104,7 @@ struct can_ctrlmode {
 #define CAN_CTRLMODE_TDC_AUTO		0x200	/* CAN transiver automatically calculates TDCV */
 #define CAN_CTRLMODE_TDC_MANUAL		0x400	/* TDCV is manually set up by user */
 #define CAN_CTRLMODE_LIN		0x800	/* LIN bus mode */
+#define CAN_CTRLMODE_LIN_COMMANDER	0x1000	/* LIN bus specific commander mode */
 
 /*
  * CAN device statistics
-- 
2.39.2

[PATCH v2 12/12] HID: hexLIN: Implement ability to update lin mode

[Christoph Fritz]

This patch enhances the hexLIN driver by implementing the newly
introduced update_lin_mode() callback.  So that either commander or
responder mode can be configured on this hardware.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/hid/hid-hexdev-hexlin.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
index 34019043f329e..64c1afcba0550 100644
--- a/drivers/hid/hid-hexdev-hexlin.c
+++ b/drivers/hid/hid-hexdev-hexlin.c
@@ -180,6 +180,8 @@ HEXLIN_GET_CMD(get_baudrate, HEXLIN_GET_BAUDRATE)
 	}
 
 HEXLIN_VAL_CMD(send_break, HEXLIN_SEND_BREAK, hexlin_val8_req, u8)
+HEXLIN_VAL_CMD(set_mode_controller, HEXLIN_SET_MODE_CONTROLLER, hexlin_val8_req,
+	       bool)
 
 static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
 				      const u8 *raw_data, int sz)
@@ -349,6 +351,14 @@ static int hexlin_ldo_tx(struct lin_device *ldev,
 	return ret;
 }
 
+static int  hexlin_update_lin_mode(struct lin_device *ldev, enum lin_mode lm)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	return hexlin_set_mode_controller(priv, lm == LINBUS_COMMANDER);
+}
+
 static int hexlin_update_bitrate(struct lin_device *ldev, u16 bitrate)
 {
 	struct hid_device *hdev = to_hid_device(ldev->dev);
@@ -420,6 +430,7 @@ static const struct lin_device_ops hexlin_ldo = {
 	.ldo_open = hexlin_open,
 	.ldo_stop = hexlin_stop,
 	.ldo_tx = hexlin_ldo_tx,
+	.update_lin_mode = hexlin_update_lin_mode,
 	.update_bitrate = hexlin_update_bitrate,
 	.get_responder_answer = hexlin_get_responder_answer,
 	.update_responder_answer = hexlin_update_resp_answer,
-- 
2.39.2

Re: [PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter

[Jiri Slaby]

On 02. 05. 24, 9:55, Christoph Fritz wrote:
> This patch introduces driver support for the hexLIN USB LIN bus adapter,
> enabling LIN communication over USB for both controller and responder
> modes. The driver interfaces with the CAN_LIN framework for userland
> connectivity.
> 
> For more details on the adapter, visit: https://hexdev.de/hexlin/
> 
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
...
> --- /dev/null
> +++ b/drivers/hid/hid-hexdev-hexlin.c
> @@ -0,0 +1,630 @@
...
> +static int hexlin_stop(struct lin_device *ldev)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	hid_hw_close(hdev);
> +
> +	priv->is_error = true;
> +	complete(&priv->wait_in_report);
> +
> +	mutex_lock(&priv->tx_lock);
> +	mutex_unlock(&priv->tx_lock);

This is a weird way to implement a completion. It looks like you need 
another one.

> +	return 0;
> +}
...> +static int hexlin_probe(struct hid_device *hdev,
> +			const struct hid_device_id *id)
> +{
> +	struct hexlin_priv_data *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->hid_dev = hdev;
> +	hid_set_drvdata(hdev, priv);
> +
> +	mutex_init(&priv->tx_lock);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid parse failed with %d\n", ret);
> +		goto fail_and_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> +	if (ret) {
> +		hid_err(hdev, "hid hw start failed with %d\n", ret);
> +		goto fail_and_stop;
> +	}
> +
> +	ret = hid_hw_open(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid hw open failed with %d\n", ret);
> +		goto fail_and_close;
> +	}
> +
> +	init_completion(&priv->wait_in_report);
> +
> +	hid_device_io_start(hdev);
> +
> +	ret = init_hw(priv);
> +	if (ret)
> +		goto fail_and_close;
> +
> +	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> +	if (IS_ERR_OR_NULL(priv->ldev)) {
> +		ret = PTR_ERR(priv->ldev);
> +		goto fail_and_close;
> +	}
> +
> +	hid_hw_close(hdev);
> +
> +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> +
> +	return 0;
> +
> +fail_and_close:
> +	hid_hw_close(hdev);
> +fail_and_stop:
> +	hid_hw_stop(hdev);
> +fail_and_free:
> +	mutex_destroy(&priv->tx_lock);
> +	return ret;
> +}
> +
> +static void hexlin_remove(struct hid_device *hdev)
> +{
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	unregister_lin(priv->ldev);
> +	hid_hw_stop(hdev);
> +	mutex_destroy(&priv->tx_lock);

It is unusual to destroy a mutex. Why do you do that?

> +}
...
> +static int __init hexlin_init(void)
> +{
> +	return hid_register_driver(&hexlin_driver);
> +}
> +
> +static void __exit hexlin_exit(void)
> +{
> +	hid_unregister_driver(&hexlin_driver);
> +}



> +
> +/*
> + * When compiled into the kernel, initialize after the hid bus.
> + */
> +late_initcall(hexlin_init);

Hmm, why not module_init() then? (And module_hid_driver().)

> +module_exit(hexlin_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
> +MODULE_DESCRIPTION("LIN bus driver for hexLIN USB adapter");

thanks,
-- 
js
suse labs

Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters

[Jiri Slaby]

On 02. 05. 24, 9:55, Christoph Fritz wrote:
> This commit introduces LIN-Bus support for UART devices equipped with
> LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
> 
> For more details on an adapter, visit: https://hexdev.de/hexlin#tty
...
> --- /dev/null
> +++ b/drivers/net/can/lin-serdev.c
> @@ -0,0 +1,514 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/module.h>
> +#include <linux/wait.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>

What do you need kernel.h for? You should explicitly require what you 
need (you apparently do), so kernel.h should not be needed.

> +#include <net/lin.h>
> +#include <linux/of.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/kfifo.h>
> +#include <linux/workqueue.h>
> +#include <linux/tty.h>

Might be eaier to maintain if you sort them.

> +#define LINSER_SAMPLES_PER_CHAR		10
> +#define LINSER_TX_BUFFER_SIZE		11
> +#define LINSER_RX_FIFO_SIZE		256
> +#define LINSER_PARSE_BUFFER		24
> +
> +struct linser_rx {
> +	u8 data;
> +	u8 flag;
> +};
> +
> +enum linser_rx_status {
> +	NEED_MORE = -1,
> +	MODE_OK = 0,
> +	NEED_FORCE,
> +};
> +
> +struct linser_priv {
> +	struct lin_device *lin_dev;
> +	struct serdev_device *serdev;
> +	DECLARE_KFIFO_PTR(rx_fifo, struct linser_rx);
> +	struct delayed_work rx_work;
> +	ulong break_usleep_min;
> +	ulong break_usleep_max;
> +	ulong post_break_usleep_min;
> +	ulong post_break_usleep_max;
> +	ulong force_timeout_jfs;

The same as for uint :)

> +	struct lin_responder_answer respond_answ[LIN_NUM_IDS];
> +	struct mutex resp_lock; /* protects respond_answ */
> +	bool is_stopped;
> +};
...
> +static void linser_derive_timings(struct linser_priv *priv, u16 bitrate)
> +{
> +	unsigned long break_baud = (bitrate * 2) / 3;
> +	unsigned long timeout_us;
> +

Are those 1000000UL USEC_PER_SEC?

> +	priv->break_usleep_min = (1000000UL * LINSER_SAMPLES_PER_CHAR) /
> +				 break_baud;
> +	priv->break_usleep_max = priv->break_usleep_min + 50;
> +	priv->post_break_usleep_min = (1000000UL * 1 /* 1 bit */) / break_baud;
> +	priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
> +
> +	timeout_us = DIV_ROUND_CLOSEST(1000000UL * 256 /* bit */, bitrate);
> +	priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
> +}
...
> +static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
> +{
> +	struct lin_responder_answer *answ = &priv->respond_answ[id];
> +	struct serdev_device *serdev = priv->serdev;
> +	u8 buf[LINSER_TX_BUFFER_SIZE];
> +	u8 checksum, count, n;
> +	ssize_t write_len;
> +
> +	mutex_lock(&priv->resp_lock);
> +
> +	if (!answ->is_active)
> +		goto unlock_and_exit_false;
> +
> +	if (answ->is_event_frame) {
> +		struct lin_responder_answer *e_answ;
> +
> +		e_answ = &priv->respond_answ[answ->event_associated_id];
> +		n = min(e_answ->lf.len, LIN_MAX_DLEN);
> +		if (memcmp(answ->lf.data, e_answ->lf.data, n) != 0) {
> +			memcpy(answ->lf.data, e_answ->lf.data, n);
> +			checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> +						    n, e_answ->lf.data,
> +						    answ->lf.checksum_mode);
> +			answ = e_answ;
> +		} else {
> +			goto unlock_and_exit_false;

Can't you simply use guard(mutex) above and avoid the error-prone 
gotos/cleanup completely?

> +		}
> +	} else {
> +		checksum = answ->lf.checksum;
> +	}
> +
> +	count = min(answ->lf.len, LIN_MAX_DLEN);
> +	memcpy(&buf[0], answ->lf.data, count);
> +	buf[count] = checksum;
> +
> +	mutex_unlock(&priv->resp_lock);
> +
> +	write_len = serdev_device_write(serdev, buf, count + 1, 0);
> +	if (write_len < count + 1)
> +		return false;
> +
> +	serdev_device_wait_until_sent(serdev, 0);
> +
> +	return true;
> +
> +unlock_and_exit_false:
> +	mutex_unlock(&priv->resp_lock);
> +	return false;
> +}
> +
> +static void linser_pop_fifo(struct linser_priv *priv, size_t n)
> +{
> +	struct serdev_device *serdev = priv->serdev;
> +	struct linser_rx dummy;
> +	size_t ret, i;
> +
> +	for (i = 0; i < n; i++) {
> +		ret = kfifo_out(&priv->rx_fifo, &dummy, 1);

Does kfifo_skip() not work for records? (I added it recently for serial.)

> +		if (ret != 1) {
> +			dev_err(&serdev->dev, "Failed to pop from FIFO\n");
> +			break;
> +		}
> +	}
> +}


thanks,
-- 
js
suse labs

Re: [PATCH v2 05/12] dt-bindings: vendor-prefixes: Add hexDEV

[Krzysztof Kozlowski]

On 02/05/2024 09:55, Christoph Fritz wrote:
> Add vendor prefix for hexDEV GmbH. Website: https://hexdev.de
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>



---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof

Re: [PATCH v2 06/12] dt-bindings: net/can: Add serial (serdev) LIN adapter

[Krzysztof Kozlowski]

On 02/05/2024 09:55, Christoph Fritz wrote:
> This patch adds dt-bindings for serial LIN bus adapters. These adapters are

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> basically just LIN transceivers that get hard-wired with serial devices.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>

...

> +maintainers:
> +  - Christoph Fritz <christoph.fritz@hexdev.de>
> +
> +properties:
> +  compatible:
> +    const: hexdev,lin-serdev
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false

This must be additionalProperties: false.

Best regards,
Krzysztof

Re: [PATCH v2 06/12] dt-bindings: net/can: Add serial (serdev) LIN adapter

[Rob Herring (Arm)]

On Thu, 02 May 2024 09:55:28 +0200, Christoph Fritz wrote:
> This patch adds dt-bindings for serial LIN bus adapters. These adapters are
> basically just LIN transceivers that get hard-wired with serial devices.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/hexdev,lin-serdev.yaml   | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dtb: /example-0/serial/linbus: failed to match any schema with compatible: ['linux,lin-serdev']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240502075534.882628-7-christoph.fritz@hexdev.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 08/12] can: lin: Add special frame id for rx offload config

[Krzysztof Kozlowski]

On 02/05/2024 09:55, Christoph Fritz wrote:
> A LIN bus supports up to 64 identifiers in one byte. This commit adds a
> special frame ID, beyond the actual LIN identifiers, for signaling RX
> offload configuration requests. This ID will be utilized in future LIN
> enhancements to the CAN broadcast manager.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  include/net/lin.h | 1 +

You just added this file in other patch. What is the point of splitting
line-per-line additions? There is no user of this in this patch. Squash
it with the patch adding the file.

Best regards,
Krzysztof

Re: [PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter

[Christoph Fritz]

On Thu, 2024-05-02 at 10:30 +0200, Jiri Slaby wrote:
> On 02. 05. 24, 9:55, Christoph Fritz wrote:
> > This patch introduces driver support for the hexLIN USB LIN bus adapter,
> > enabling LIN communication over USB for both controller and responder
> > modes. The driver interfaces with the CAN_LIN framework for userland
> > connectivity.
> > 
> > For more details on the adapter, visit: https://hexdev.de/hexlin/
> > 
> > Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ...
> > --- /dev/null
> > +++ b/drivers/hid/hid-hexdev-hexlin.c
> > @@ -0,0 +1,630 @@
> ...
> > +static int hexlin_stop(struct lin_device *ldev)
> > +{
> > +	struct hid_device *hdev = to_hid_device(ldev->dev);
> > +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> > +
> > +	hid_hw_close(hdev);
> > +
> > +	priv->is_error = true;
> > +	complete(&priv->wait_in_report);
> > +
> > +	mutex_lock(&priv->tx_lock);
> > +	mutex_unlock(&priv->tx_lock);
> 
> This is a weird way to implement a completion. It looks like you need 
> another one.

They are not necessary, even more so when I can drop the
mutex_destroy() below.

> 
> > +	return 0;
> > +}
> ...> +static int hexlin_probe(struct hid_device *hdev,
> > +			const struct hid_device_id *id)
> > +{
> > +	struct hexlin_priv_data *priv;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->hid_dev = hdev;
> > +	hid_set_drvdata(hdev, priv);
> > +
> > +	mutex_init(&priv->tx_lock);
> > +
> > +	ret = hid_parse(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "hid parse failed with %d\n", ret);
> > +		goto fail_and_free;
> > +	}
> > +
> > +	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> > +	if (ret) {
> > +		hid_err(hdev, "hid hw start failed with %d\n", ret);
> > +		goto fail_and_stop;
> > +	}
> > +
> > +	ret = hid_hw_open(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "hid hw open failed with %d\n", ret);
> > +		goto fail_and_close;
> > +	}
> > +
> > +	init_completion(&priv->wait_in_report);
> > +
> > +	hid_device_io_start(hdev);
> > +
> > +	ret = init_hw(priv);
> > +	if (ret)
> > +		goto fail_and_close;
> > +
> > +	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> > +	if (IS_ERR_OR_NULL(priv->ldev)) {
> > +		ret = PTR_ERR(priv->ldev);
> > +		goto fail_and_close;
> > +	}
> > +
> > +	hid_hw_close(hdev);
> > +
> > +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> > +
> > +	return 0;
> > +
> > +fail_and_close:
> > +	hid_hw_close(hdev);
> > +fail_and_stop:
> > +	hid_hw_stop(hdev);
> > +fail_and_free:
> > +	mutex_destroy(&priv->tx_lock);
> > +	return ret;
> > +}
> > +
> > +static void hexlin_remove(struct hid_device *hdev)
> > +{
> > +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> > +
> > +	unregister_lin(priv->ldev);
> > +	hid_hw_stop(hdev);
> > +	mutex_destroy(&priv->tx_lock);
> 
> It is unusual to destroy a mutex. Why do you do that?
> 

Just for code clarity and it should help if someone wants to use
CONFIG_DEBUG_MUTEXES.

To be able to drop the lock/unlock from above, I could add the
lock/unlock here or just drop the mutex_destroy() completely.

I'll just drop it in upcoming v3.

> > +}
> ...
> > +static int __init hexlin_init(void)
> > +{
> > +	return hid_register_driver(&hexlin_driver);
> > +}
> > +
> > +static void __exit hexlin_exit(void)
> > +{
> > +	hid_unregister_driver(&hexlin_driver);
> > +}
> 
> 
> 
> > +
> > +/*
> > + * When compiled into the kernel, initialize after the hid bus.
> > + */
> > +late_initcall(hexlin_init);
> 
> Hmm, why not module_init() then? (And module_hid_driver().)

Looking at the other hid drivers and testing with just

module_hid_driver(hexlin_driver)

works here fine for compiled into the kernel and as a module.

> 
> > +module_exit(hexlin_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
> > +MODULE_DESCRIPTION("LIN bus driver for hexLIN USB adapter");

Thanks
  -- Christoph

Re: [PATCH v2 06/12] dt-bindings: net/can: Add serial (serdev) LIN adapter

[Christoph Fritz]

On Thu, 2024-05-02 at 04:31 -0500, Rob Herring (Arm) wrote:
> On Thu, 02 May 2024 09:55:28 +0200, Christoph Fritz wrote:
> > This patch adds dt-bindings for serial LIN bus adapters. These adapters are
> > basically just LIN transceivers that get hard-wired with serial devices.
> > 
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > ---
> >  .../bindings/net/can/hexdev,lin-serdev.yaml   | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dtb: /example-0/serial/linbus: failed to match any schema with compatible: ['linux,lin-serdev']

Yes, that's obviously still false and will be fixed in v3.

> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240502075534.882628-7-christoph.fritz@hexdev.de
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

I'm wondering why my local run of dt_binding_check does not catch this:

$ pip3 install dtschema --upgrade
Requirement already satisfied: dtschema in ./venv/lib/python3.11/site-packages (2024.4)
Requirement already satisfied: ruamel.yaml>0.15.69 in ./venv/lib/python3.11/site-packages (from dtschema) (0.18.6)
Requirement already satisfied: jsonschema<4.18,>=4.1.2 in ./venv/lib/python3.11/site-packages (from dtschema) (4.17.3)
Requirement already satisfied: rfc3987 in ./venv/lib/python3.11/site-packages (from dtschema) (1.3.8)
Requirement already satisfied: pylibfdt in ./venv/lib/python3.11/site-packages (from dtschema) (1.7.0.post1)
Requirement already satisfied: attrs>=17.4.0 in ./venv/lib/python3.11/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (23.2.0)
Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in ./venv/lib/python3.11/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (0.20.0)
Requirement already satisfied: ruamel.yaml.clib>=0.2.7 in ./venv/lib/python3.11/site-packages (from ruamel.yaml>0.15.69->dtschema) (0.2.8)

$ git diff
diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
index c178eb9be1391..385cbe132258d 100644
--- a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
+++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
@@ -27,6 +27,6 @@ examples:
   - |
     serial {
         linbus {
-            compatible = "hexdev,lin-serdev";
+            compatible = "linux,lin-serdev";
         };
     };

$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/dtc/dtc.o
  HOSTCC  scripts/dtc/flattree.o
  HOSTCC  scripts/dtc/fstree.o
  HOSTCC  scripts/dtc/data.o
  HOSTCC  scripts/dtc/livetree.o
  HOSTCC  scripts/dtc/treesource.o
  HOSTCC  scripts/dtc/srcpos.o
  HOSTCC  scripts/dtc/checks.o
  HOSTCC  scripts/dtc/util.o
  LEX     scripts/dtc/dtc-lexer.lex.c
  YACC    scripts/dtc/dtc-parser.tab.[ch]
  HOSTCC  scripts/dtc/dtc-lexer.lex.o
  HOSTCC  scripts/dtc/dtc-parser.tab.o
  HOSTLD  scripts/dtc/dtc
  HOSTCC  scripts/dtc/libfdt/fdt.o
  HOSTCC  scripts/dtc/libfdt/fdt_ro.o
  HOSTCC  scripts/dtc/libfdt/fdt_wip.o
  HOSTCC  scripts/dtc/libfdt/fdt_sw.o
  HOSTCC  scripts/dtc/libfdt/fdt_rw.o
  HOSTCC  scripts/dtc/libfdt/fdt_strerror.o
  HOSTCC  scripts/dtc/libfdt/fdt_empty_tree.o
  HOSTCC  scripts/dtc/libfdt/fdt_addresses.o
  HOSTCC  scripts/dtc/libfdt/fdt_overlay.o
  HOSTCC  scripts/dtc/fdtoverlay.o
  HOSTLD  scripts/dtc/fdtoverlay
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/home/ch/linux/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml: ignoring, error in schema: properties: brcm,tperst-clk-ms: type
/home/ch/linux/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: ignoring, error in schema: properties: emcs205,max-state: description
  DTEX    Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dts
  DTC_CHK Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dtb

Any ideas?

I'm using a python venv here, maybe this is related?

Thanks
  -- Christoph

Re: [PATCH v2 06/12] dt-bindings: net/can: Add serial (serdev) LIN adapter

[Rob Herring]

On Thu, May 2, 2024 at 6:03 AM Christoph Fritz
<christoph.fritz@hexdev.de> wrote:
>
> On Thu, 2024-05-02 at 04:31 -0500, Rob Herring (Arm) wrote:
> > On Thu, 02 May 2024 09:55:28 +0200, Christoph Fritz wrote:
> > > This patch adds dt-bindings for serial LIN bus adapters. These adapters are
> > > basically just LIN transceivers that get hard-wired with serial devices.
> > >
> > > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > > ---
> > >  .../bindings/net/can/hexdev,lin-serdev.yaml   | 32 +++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dtb: /example-0/serial/linbus: failed to match any schema with compatible: ['linux,lin-serdev']
>
> Yes, that's obviously still false and will be fixed in v3.
>
> >
> > doc reference errors (make refcheckdocs):
> >
> > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240502075534.882628-7-christoph.fritz@hexdev.de
> >
> > The base for the series is generally the latest rc1. A different dependency
> > should be noted in *this* patch.
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> >
> > pip3 install dtschema --upgrade
> >
> > Please check and re-submit after running the above command yourself. Note
> > that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> > your schema. However, it must be unset to test all examples with your schema.
> >
>
> I'm wondering why my local run of dt_binding_check does not catch this:
>
> $ pip3 install dtschema --upgrade
> Requirement already satisfied: dtschema in ./venv/lib/python3.11/site-packages (2024.4)
> Requirement already satisfied: ruamel.yaml>0.15.69 in ./venv/lib/python3.11/site-packages (from dtschema) (0.18.6)
> Requirement already satisfied: jsonschema<4.18,>=4.1.2 in ./venv/lib/python3.11/site-packages (from dtschema) (4.17.3)
> Requirement already satisfied: rfc3987 in ./venv/lib/python3.11/site-packages (from dtschema) (1.3.8)
> Requirement already satisfied: pylibfdt in ./venv/lib/python3.11/site-packages (from dtschema) (1.7.0.post1)
> Requirement already satisfied: attrs>=17.4.0 in ./venv/lib/python3.11/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (23.2.0)
> Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in ./venv/lib/python3.11/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (0.20.0)
> Requirement already satisfied: ruamel.yaml.clib>=0.2.7 in ./venv/lib/python3.11/site-packages (from ruamel.yaml>0.15.69->dtschema) (0.2.8)
>
> $ git diff
> diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> index c178eb9be1391..385cbe132258d 100644
> --- a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> +++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> @@ -27,6 +27,6 @@ examples:
>    - |
>      serial {
>          linbus {
> -            compatible = "hexdev,lin-serdev";
> +            compatible = "linux,lin-serdev";
>          };
>      };
>
> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/dtc/dtc.o
>   HOSTCC  scripts/dtc/flattree.o
>   HOSTCC  scripts/dtc/fstree.o
>   HOSTCC  scripts/dtc/data.o
>   HOSTCC  scripts/dtc/livetree.o
>   HOSTCC  scripts/dtc/treesource.o
>   HOSTCC  scripts/dtc/srcpos.o
>   HOSTCC  scripts/dtc/checks.o
>   HOSTCC  scripts/dtc/util.o
>   LEX     scripts/dtc/dtc-lexer.lex.c
>   YACC    scripts/dtc/dtc-parser.tab.[ch]
>   HOSTCC  scripts/dtc/dtc-lexer.lex.o
>   HOSTCC  scripts/dtc/dtc-parser.tab.o
>   HOSTLD  scripts/dtc/dtc
>   HOSTCC  scripts/dtc/libfdt/fdt.o
>   HOSTCC  scripts/dtc/libfdt/fdt_ro.o
>   HOSTCC  scripts/dtc/libfdt/fdt_wip.o
>   HOSTCC  scripts/dtc/libfdt/fdt_sw.o
>   HOSTCC  scripts/dtc/libfdt/fdt_rw.o
>   HOSTCC  scripts/dtc/libfdt/fdt_strerror.o
>   HOSTCC  scripts/dtc/libfdt/fdt_empty_tree.o
>   HOSTCC  scripts/dtc/libfdt/fdt_addresses.o
>   HOSTCC  scripts/dtc/libfdt/fdt_overlay.o
>   HOSTCC  scripts/dtc/fdtoverlay.o
>   HOSTLD  scripts/dtc/fdtoverlay
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /home/ch/linux/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml: ignoring, error in schema: properties: brcm,tperst-clk-ms: type
> /home/ch/linux/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: ignoring, error in schema: properties: emcs205,max-state: description
>   DTEX    Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dts
>   DTC_CHK Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.example.dtb
>
> Any ideas?
>
> I'm using a python venv here, maybe this is related?

No. There are 2 possibilities. What kernel version are you on? This
check is enabled with the '-m' option on dt-validate which was only
recently (6.9) enabled by default for the bindings. You can enable it
with 'DT_CHECKER_FLAGS="-m"'. The other possibility is I noticed that
the flag has an interaction with DT_SCHEMA_FILES in that we don't set
the flag by default if DT_SCHEMA_FILES is set. (If you explicitly set
DT_CHECKER_FLAGS in the newer kernels it should still give the
warning.)  I think we don't enable it because you would get false
positives if your example has compatible strings not documented within
the schema you are testing. I need to double check that as how the
tools work in this regard has evolved. In any case, DT_SCHEMA_FILES is
a shortcut and it is always possible your changes can introduce
warnings in other examples, so ultimately "make dt_binding_check" has
to be run without DT_SCHEMA_FILES set.

Rob

Re: [PATCH v2 08/12] can: lin: Add special frame id for rx offload config

[Christoph Fritz]

On Thu, 2024-05-02 at 12:27 +0200, Krzysztof Kozlowski wrote:
> On 02/05/2024 09:55, Christoph Fritz wrote:
> > A LIN bus supports up to 64 identifiers in one byte. This commit adds a
> > special frame ID, beyond the actual LIN identifiers, for signaling RX
> > offload configuration requests. This ID will be utilized in future LIN
> > enhancements to the CAN broadcast manager.
> > 
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > ---
> >  include/net/lin.h | 1 +
> 
> You just added this file in other patch. What is the point of splitting
> line-per-line additions?

My intention was to make the review process easier by separating the
BCM (Broadcast Manager) logic from the basic driver implementation.

> There is no user of this in this patch. Squash it with the patch adding
> the file.

OK

v3 is coming up

Thanks
  -- Christoph

Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters

[Christoph Fritz]

On Thu, 2024-05-02 at 10:44 +0200, Jiri Slaby wrote:
> On 02. 05. 24, 9:55, Christoph Fritz wrote:
> > This commit introduces LIN-Bus support for UART devices equipped with
> > LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
> > 
> > For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> ...
> > --- /dev/null
> > +++ b/drivers/net/can/lin-serdev.c
> > @@ -0,0 +1,514 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> > +
> > +#include <linux/module.h>
> > +#include <linux/wait.h>
> > +#include <linux/init.h>
> > +#include <linux/errno.h>
> > +#include <linux/string.h>
> > +#include <linux/kernel.h>
> 
> What do you need kernel.h for? You should explicitly require what you 
> need (you apparently do), so kernel.h should not be needed.

OK

> 
> > +#include <net/lin.h>
> > +#include <linux/of.h>
> > +#include <linux/serdev.h>
> > +#include <linux/slab.h>
> > +#include <linux/kfifo.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/tty.h>
> 
> Might be eaier to maintain if you sort them.

OK, hid driver gets also sorted

> 
> > +#define LINSER_SAMPLES_PER_CHAR		10
> > +#define LINSER_TX_BUFFER_SIZE		11
> > +#define LINSER_RX_FIFO_SIZE		256
> > +#define LINSER_PARSE_BUFFER		24
> > +
> > +struct linser_rx {
> > +	u8 data;
> > +	u8 flag;
> > +};
> > +
> > +enum linser_rx_status {
> > +	NEED_MORE = -1,
> > +	MODE_OK = 0,
> > +	NEED_FORCE,
> > +};
> > +
> > +struct linser_priv {
> > +	struct lin_device *lin_dev;
> > +	struct serdev_device *serdev;
> > +	DECLARE_KFIFO_PTR(rx_fifo, struct linser_rx);
> > +	struct delayed_work rx_work;
> > +	ulong break_usleep_min;
> > +	ulong break_usleep_max;
> > +	ulong post_break_usleep_min;
> > +	ulong post_break_usleep_max;
> > +	ulong force_timeout_jfs;
> 
> The same as for uint :)

OK

> 
> > +	struct lin_responder_answer respond_answ[LIN_NUM_IDS];
> > +	struct mutex resp_lock; /* protects respond_answ */
> > +	bool is_stopped;
> > +};
> ...
> > +static void linser_derive_timings(struct linser_priv *priv, u16 bitrate)
> > +{
> > +	unsigned long break_baud = (bitrate * 2) / 3;
> > +	unsigned long timeout_us;
> > +
> 
> Are those 1000000UL USEC_PER_SEC?

yes

> 
> > +	priv->break_usleep_min = (1000000UL * LINSER_SAMPLES_PER_CHAR) /
> > +				 break_baud;
> > +	priv->break_usleep_max = priv->break_usleep_min + 50;
> > +	priv->post_break_usleep_min = (1000000UL * 1 /* 1 bit */) / break_baud;
> > +	priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
> > +
> > +	timeout_us = DIV_ROUND_CLOSEST(1000000UL * 256 /* bit */, bitrate);
> > +	priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
> > +}
> ...
> > +static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
> > +{
> > +	struct lin_responder_answer *answ = &priv->respond_answ[id];
> > +	struct serdev_device *serdev = priv->serdev;
> > +	u8 buf[LINSER_TX_BUFFER_SIZE];
> > +	u8 checksum, count, n;
> > +	ssize_t write_len;
> > +
> > +	mutex_lock(&priv->resp_lock);
> > +
> > +	if (!answ->is_active)
> > +		goto unlock_and_exit_false;
> > +
> > +	if (answ->is_event_frame) {
> > +		struct lin_responder_answer *e_answ;
> > +
> > +		e_answ = &priv->respond_answ[answ->event_associated_id];
> > +		n = min(e_answ->lf.len, LIN_MAX_DLEN);
> > +		if (memcmp(answ->lf.data, e_answ->lf.data, n) != 0) {
> > +			memcpy(answ->lf.data, e_answ->lf.data, n);
> > +			checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> > +						    n, e_answ->lf.data,
> > +						    answ->lf.checksum_mode);
> > +			answ = e_answ;
> > +		} else {
> > +			goto unlock_and_exit_false;
> 
> Can't you simply use guard(mutex) above and avoid the error-prone 
> gotos/cleanup completely?

OK

> 
> > +		}
> > +	} else {
> > +		checksum = answ->lf.checksum;
> > +	}
> > +
> > +	count = min(answ->lf.len, LIN_MAX_DLEN);
> > +	memcpy(&buf[0], answ->lf.data, count);
> > +	buf[count] = checksum;
> > +
> > +	mutex_unlock(&priv->resp_lock);
> > +
> > +	write_len = serdev_device_write(serdev, buf, count + 1, 0);
> > +	if (write_len < count + 1)
> > +		return false;
> > +
> > +	serdev_device_wait_until_sent(serdev, 0);
> > +
> > +	return true;
> > +
> > +unlock_and_exit_false:
> > +	mutex_unlock(&priv->resp_lock);
> > +	return false;
> > +}
> > +
> > +static void linser_pop_fifo(struct linser_priv *priv, size_t n)
> > +{
> > +	struct serdev_device *serdev = priv->serdev;
> > +	struct linser_rx dummy;
> > +	size_t ret, i;
> > +
> > +	for (i = 0; i < n; i++) {
> > +		ret = kfifo_out(&priv->rx_fifo, &dummy, 1);
> 
> Does kfifo_skip() not work for records? (I added it recently for serial.)

Using kfifo_skip() greatly simplifies this function and it works for
records (uses __kfifo_skip_r), tests are successful.

Maybe the comment in kfifo.h could be made more clear from:

 "kfifo_skip - skip output data"
to
 "kfifo_skip - skip the next fifo record"

> 
> > +		if (ret != 1) {
> > +			dev_err(&serdev->dev, "Failed to pop from FIFO\n");
> > +			break;
> > +		}
> > +	}
> > +}
> 

Let me address these points and reroll in v3.

Thanks
  -- Christoph

Re: [PATCH v2 01/12] can: Add LIN bus as CAN abstraction

[Simon Horman]

On Thu, May 02, 2024 at 09:55:23AM +0200, Christoph Fritz wrote:
> This patch adds a LIN (local interconnect network) bus abstraction on
> top of CAN.  It is a glue driver adapting CAN on one side while offering
> LIN abstraction on the other side. So that upcoming LIN device drivers
> can make use of it.
> 
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>

...

> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c

...

> +struct lin_device *register_lin(struct device *dev,
> +				const struct lin_device_ops *ldops)
> +{
> +	struct net_device *ndev;
> +	struct lin_device *ldev;
> +	int ret;
> +
> +	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||
> +	    !ldops->ldo_open || !ldops->ldo_stop) {
> +		netdev_err(ndev, "missing mandatory lin_device_ops\n");

Hi Christoph,

The line above uses ndev, but ndev is not initialised
until a few lines further down.

Flagged by Smatch.

> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct lin_device), 1);
> +	if (!ndev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ldev = netdev_priv(ndev);
> +
> +	ldev->ldev_ops = ldops;
> +	ndev->netdev_ops = &lin_netdev_ops;
> +	ndev->flags |= IFF_ECHO;
> +	ndev->mtu = CANFD_MTU;
> +	ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
> +	ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
> +	ldev->can.ctrlmode_supported = 0;
> +	ldev->can.bitrate_const = lin_bitrate;
> +	ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
> +	ldev->can.do_set_bittiming = lin_set_bittiming;
> +	ldev->ndev = ndev;
> +	ldev->dev = dev;
> +
> +	SET_NETDEV_DEV(ndev, dev);
> +
> +	ret = lin_set_bittiming(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "set bittiming failed\n");
> +		goto exit_candev;
> +	}
> +
> +	ret = register_candev(ndev);
> +	if (ret)
> +		goto exit_candev;
> +
> +	ldev->lin_ids_kobj = kobject_create_and_add("lin_ids", &ndev->dev.kobj);
> +	if (!ldev->lin_ids_kobj) {
> +		netdev_err(ndev, "Failed to create sysfs directory\n");
> +		ret = -ENOMEM;
> +		goto exit_unreg;
> +	}
> +
> +	ret = lin_create_sysfs_id_files(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "Failed to create sysfs entry: %d\n", ret);
> +		goto exit_kobj_put;
> +	}
> +
> +	/* Using workqueue as tx over USB/SPI/... may sleep */
> +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> +				   0);
> +	if (!ldev->wq)
> +		goto exit_rm_files;

The goto above will result in: return ERR_PTR(ret)
But ret is 0 here. Should it be set to a negative error value?

Also flagged by Smatch.

> +
> +	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
> +
> +	netdev_info(ndev, "LIN initialized.\n");
> +
> +	return ldev;
> +
> +exit_rm_files:
> +	lin_remove_sysfs_id_files(ndev);
> +exit_kobj_put:
> +	kobject_put(ldev->lin_ids_kobj);
> +exit_unreg:
> +	unregister_candev(ndev);
> +exit_candev:
> +	free_candev(ndev);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(register_lin);

...

Re: [PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter

[Simon Horman]

On Thu, May 02, 2024 at 09:55:24AM +0200, Christoph Fritz wrote:
> This patch introduces driver support for the hexLIN USB LIN bus adapter,
> enabling LIN communication over USB for both controller and responder
> modes. The driver interfaces with the CAN_LIN framework for userland
> connectivity.
> 
> For more details on the adapter, visit: https://hexdev.de/hexlin/
> 
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>

Hi Christoph,

Some minor feedback from my side.

...

> diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c

...

> +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> +				      const u8 *raw_data, int sz)
> +{
> +	struct hid_device *hdev = priv->hid_dev;
> +	struct hexlin_frame hxf;
> +	struct lin_frame lf;
> +
> +	if (sz != sizeof(struct hexlin_frame))
> +		return -EREMOTEIO;
> +
> +	memcpy(&hxf, raw_data, sz);
> +	le32_to_cpus(hxf.flags);
> +
> +	lf.len = hxf.len;
> +	lf.lin_id = hxf.lin_id;
> +	memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
> +	lf.checksum = hxf.checksum;
> +	lf.checksum_mode = hxf.checksum_mode;
> +
> +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> +		   lf.checksum_mode ? "enhanced" : "classic");

nit: The indentation above is not quite right.
     It should align to inside the opening '(' like this.

	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
		lf.checksum_mode ? "enhanced" : "classic");

Flagged by checkpatch.

...

> +static int hexlin_set_baudrate(struct hexlin_priv_data *priv, u16 baudrate)
> +{
> +	struct hexlin_baudrate_req req;
> +	int ret;
> +
> +	if (baudrate < LIN_MIN_BAUDRATE || baudrate > LIN_MAX_BAUDRATE)
> +		return -EINVAL;
> +
> +	req.cmd = HEXLIN_SET_BAUDRATE;
> +	req.baudrate = cpu_to_le16(baudrate);

The type of req.baudrate is u16, a host byte-order type.
But it is being assigned a little endian value.
This does not seem right.

Flagged by Smatch.

> +
> +	ret = hexlin_tx_req_status(priv, &req,
> +				   sizeof(struct hexlin_baudrate_req));
> +	if (ret)
> +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> +
> +	return ret;
> +}

...

> +static int hexlin_probe(struct hid_device *hdev,
> +			const struct hid_device_id *id)
> +{
> +	struct hexlin_priv_data *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->hid_dev = hdev;
> +	hid_set_drvdata(hdev, priv);
> +
> +	mutex_init(&priv->tx_lock);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid parse failed with %d\n", ret);
> +		goto fail_and_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> +	if (ret) {
> +		hid_err(hdev, "hid hw start failed with %d\n", ret);
> +		goto fail_and_stop;
> +	}
> +
> +	ret = hid_hw_open(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid hw open failed with %d\n", ret);
> +		goto fail_and_close;
> +	}
> +
> +	init_completion(&priv->wait_in_report);
> +
> +	hid_device_io_start(hdev);
> +
> +	ret = init_hw(priv);
> +	if (ret)
> +		goto fail_and_close;
> +
> +	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> +	if (IS_ERR_OR_NULL(priv->ldev)) {
> +		ret = PTR_ERR(priv->ldev);
> +		goto fail_and_close;

Is it intentional that when priv->ldev is NULL here,
the function will return 0?

Flagged by Smatch.

> +	}
> +
> +	hid_hw_close(hdev);
> +
> +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> +
> +	return 0;
> +
> +fail_and_close:
> +	hid_hw_close(hdev);
> +fail_and_stop:
> +	hid_hw_stop(hdev);
> +fail_and_free:
> +	mutex_destroy(&priv->tx_lock);
> +	return ret;
> +}

...

Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters

[Simon Horman]

On Thu, May 02, 2024 at 09:55:29AM +0200, Christoph Fritz wrote:
> This commit introduces LIN-Bus support for UART devices equipped with
> LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
> 
> For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>

...

> diff --git a/drivers/net/can/lin-serdev.c b/drivers/net/can/lin-serdev.c

...

> +static int linser_probe(struct serdev_device *serdev)
> +{
> +	struct linser_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&serdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = kfifo_alloc(&priv->rx_fifo, LINSER_RX_FIFO_SIZE, GFP_KERNEL);
> +	if (ret)
> +		return ret;
> +
> +	INIT_DELAYED_WORK(&priv->rx_work, linser_process_delayed);
> +
> +	priv->serdev = serdev;
> +	serdev_device_set_drvdata(serdev, priv);
> +	serdev_device_set_client_ops(serdev, &linser_ops);
> +
> +	ret = serdev_device_open(serdev);
> +	if (ret) {
> +		dev_err(&serdev->dev, "Unable to open device\n");
> +		goto err_open;
> +	}
> +
> +	serdev_device_set_flow_control(serdev, false);
> +	serdev_device_set_break_detection(serdev, true);
> +	linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
> +
> +	mutex_init(&priv->resp_lock);
> +
> +	priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
> +	if (IS_ERR_OR_NULL(priv->lin_dev)) {
> +		ret = PTR_ERR(priv->lin_dev);

As per my feedback on an earlier patch in the series,
in the case where priv->lin_dev is NULL,
this will result in the function returning 0.
Is that ok?

Flagged by Smatch

> +		goto err_register_lin;
> +	}
> +
> +	serdev_device_close(serdev);
> +	priv->is_stopped = true;
> +
> +	return 0;
> +
> +err_register_lin:
> +	serdev_device_close(serdev);
> +err_open:
> +	kfifo_free(&priv->rx_fifo);
> +	return ret;
> +}

...

Re: [PATCH v2 01/12] can: Add LIN bus as CAN abstraction

[Christoph Fritz]

On Sat, 2024-05-04 at 13:49 +0100, Simon Horman wrote:
> On Thu, May 02, 2024 at 09:55:23AM +0200, Christoph Fritz wrote:
> > This patch adds a LIN (local interconnect network) bus abstraction on
> > top of CAN.  It is a glue driver adapting CAN on one side while offering
> > LIN abstraction on the other side. So that upcoming LIN device drivers
> > can make use of it.
> > 
> > Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> 
> ...
> 
> > diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
> 
> ...
> 
> > +struct lin_device *register_lin(struct device *dev,
> > +				const struct lin_device_ops *ldops)
> > +{
> > +	struct net_device *ndev;
> > +	struct lin_device *ldev;
> > +	int ret;
> > +
> > +	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||
> > +	    !ldops->ldo_open || !ldops->ldo_stop) {
> > +		netdev_err(ndev, "missing mandatory lin_device_ops\n");
> 
> Hi Christoph,

Hi Simon

> The line above uses ndev, but ndev is not initialised
> until a few lines further down.
> 
> Flagged by Smatch.

Despite netdev_err() checks validity of ndev, I agree with Smatch: In
upcoming v4 I'll use dev_err() here instead.

> > +		return ERR_PTR(-EINVAL);
> > +	}
> > +
> > +	ndev = alloc_candev(sizeof(struct lin_device), 1);
> > +	if (!ndev)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	ldev = netdev_priv(ndev);
> > +
> > +	ldev->ldev_ops = ldops;
> > +	ndev->netdev_ops = &lin_netdev_ops;
> > +	ndev->flags |= IFF_ECHO;
> > +	ndev->mtu = CANFD_MTU;
> > +	ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
> > +	ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
> > +	ldev->can.ctrlmode_supported = 0;
> > +	ldev->can.bitrate_const = lin_bitrate;
> > +	ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
> > +	ldev->can.do_set_bittiming = lin_set_bittiming;
> > +	ldev->ndev = ndev;
> > +	ldev->dev = dev;
> > +
> > +	SET_NETDEV_DEV(ndev, dev);
> > +
> > +	ret = lin_set_bittiming(ndev);
> > +	if (ret) {
> > +		netdev_err(ndev, "set bittiming failed\n");
> > +		goto exit_candev;
> > +	}
> > +
> > +	ret = register_candev(ndev);
> > +	if (ret)
> > +		goto exit_candev;
> > +
> > +	ldev->lin_ids_kobj = kobject_create_and_add("lin_ids", &ndev->dev.kobj);
> > +	if (!ldev->lin_ids_kobj) {
> > +		netdev_err(ndev, "Failed to create sysfs directory\n");
> > +		ret = -ENOMEM;
> > +		goto exit_unreg;
> > +	}
> > +
> > +	ret = lin_create_sysfs_id_files(ndev);
> > +	if (ret) {
> > +		netdev_err(ndev, "Failed to create sysfs entry: %d\n", ret);
> > +		goto exit_kobj_put;
> > +	}
> > +
> > +	/* Using workqueue as tx over USB/SPI/... may sleep */
> > +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> > +				   0);
> > +	if (!ldev->wq)
> > +		goto exit_rm_files;
> 
> The goto above will result in: return ERR_PTR(ret)
> But ret is 0 here. Should it be set to a negative error value?
> 
> Also flagged by Smatch.

OK, will get an

ret = -ENOMEM;

> 
> > +
> > +	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
> > +
> > +	netdev_info(ndev, "LIN initialized.\n");
> > +
> > +	return ldev;
> > +
> > +exit_rm_files:
> > +	lin_remove_sysfs_id_files(ndev);
> > +exit_kobj_put:
> > +	kobject_put(ldev->lin_ids_kobj);
> > +exit_unreg:
> > +	unregister_candev(ndev);
> > +exit_candev:
> > +	free_candev(ndev);
> > +	return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(register_lin);
> 
> ...

Thanks
  -- Christoph

Re: [PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter

[Christoph Fritz]

On Sat, 2024-05-04 at 13:58 +0100, Simon Horman wrote:
> On Thu, May 02, 2024 at 09:55:24AM +0200, Christoph Fritz wrote:
> > This patch introduces driver support for the hexLIN USB LIN bus adapter,
> > enabling LIN communication over USB for both controller and responder
> > modes. The driver interfaces with the CAN_LIN framework for userland
> > connectivity.
> > 
> > For more details on the adapter, visit: https://hexdev.de/hexlin/
> > 
> > Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> 
> Hi Christoph,
> 
> Some minor feedback from my side.
> 
> ...
> 
> > diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
> 
> ...
> 
> > +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> > +				      const u8 *raw_data, int sz)
> > +{
> > +	struct hid_device *hdev = priv->hid_dev;
> > +	struct hexlin_frame hxf;
> > +	struct lin_frame lf;
> > +
> > +	if (sz != sizeof(struct hexlin_frame))
> > +		return -EREMOTEIO;
> > +
> > +	memcpy(&hxf, raw_data, sz);
> > +	le32_to_cpus(hxf.flags);
> > +
> > +	lf.len = hxf.len;
> > +	lf.lin_id = hxf.lin_id;
> > +	memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
> > +	lf.checksum = hxf.checksum;
> > +	lf.checksum_mode = hxf.checksum_mode;
> > +
> > +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> > +		   lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> > +		   lf.checksum_mode ? "enhanced" : "classic");
> 
> nit: The indentation above is not quite right.
>      It should align to inside the opening '(' like this.
> 
> 	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> 		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> 		lf.checksum_mode ? "enhanced" : "classic");
> 
> Flagged by checkpatch.

OK

> 
> ...
> 
> > +static int hexlin_set_baudrate(struct hexlin_priv_data *priv, u16 baudrate)
> > +{
> > +	struct hexlin_baudrate_req req;
> > +	int ret;
> > +
> > +	if (baudrate < LIN_MIN_BAUDRATE || baudrate > LIN_MAX_BAUDRATE)
> > +		return -EINVAL;
> > +
> > +	req.cmd = HEXLIN_SET_BAUDRATE;
> > +	req.baudrate = cpu_to_le16(baudrate);
> 
> The type of req.baudrate is u16, a host byte-order type.
> But it is being assigned a little endian value.
> This does not seem right.
> 
> Flagged by Smatch.

In upcoming v4 I'll use __le16

> 
> > +
> > +	ret = hexlin_tx_req_status(priv, &req,
> > +				   sizeof(struct hexlin_baudrate_req));
> > +	if (ret)
> > +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> > +
> > +	return ret;
> > +}
> 
> ...
> 
> > +static int hexlin_probe(struct hid_device *hdev,
> > +			const struct hid_device_id *id)
> > +{
> > +	struct hexlin_priv_data *priv;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->hid_dev = hdev;
> > +	hid_set_drvdata(hdev, priv);
> > +
> > +	mutex_init(&priv->tx_lock);
> > +
> > +	ret = hid_parse(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "hid parse failed with %d\n", ret);
> > +		goto fail_and_free;
> > +	}
> > +
> > +	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> > +	if (ret) {
> > +		hid_err(hdev, "hid hw start failed with %d\n", ret);
> > +		goto fail_and_stop;
> > +	}
> > +
> > +	ret = hid_hw_open(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "hid hw open failed with %d\n", ret);
> > +		goto fail_and_close;
> > +	}
> > +
> > +	init_completion(&priv->wait_in_report);
> > +
> > +	hid_device_io_start(hdev);
> > +
> > +	ret = init_hw(priv);
> > +	if (ret)
> > +		goto fail_and_close;
> > +
> > +	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> > +	if (IS_ERR_OR_NULL(priv->ldev)) {
> > +		ret = PTR_ERR(priv->ldev);
> > +		goto fail_and_close;
> 
> Is it intentional that when priv->ldev is NULL here,
> the function will return 0?
> 
> Flagged by Smatch.

IS_ERR_OR_NULL() gets IS_ERR() in upcoming v4

> 
> > +	}
> > +
> > +	hid_hw_close(hdev);
> > +
> > +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> > +
> > +	return 0;
> > +
> > +fail_and_close:
> > +	hid_hw_close(hdev);
> > +fail_and_stop:
> > +	hid_hw_stop(hdev);
> > +fail_and_free:
> > +	mutex_destroy(&priv->tx_lock);
> > +	return ret;
> > +}
> 
> ...

Thanks
  -- Christoph

Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters

[Christoph Fritz]

On Sat, 2024-05-04 at 14:13 +0100, Simon Horman wrote:
> On Thu, May 02, 2024 at 09:55:29AM +0200, Christoph Fritz wrote:
> > This commit introduces LIN-Bus support for UART devices equipped with
> > LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
> > 
> > For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> > 
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> 
> ...
> 
> > diff --git a/drivers/net/can/lin-serdev.c b/drivers/net/can/lin-serdev.c
> 
> ...
> 
> > +static int linser_probe(struct serdev_device *serdev)
> > +{
> > +	struct linser_priv *priv;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&serdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	ret = kfifo_alloc(&priv->rx_fifo, LINSER_RX_FIFO_SIZE, GFP_KERNEL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	INIT_DELAYED_WORK(&priv->rx_work, linser_process_delayed);
> > +
> > +	priv->serdev = serdev;
> > +	serdev_device_set_drvdata(serdev, priv);
> > +	serdev_device_set_client_ops(serdev, &linser_ops);
> > +
> > +	ret = serdev_device_open(serdev);
> > +	if (ret) {
> > +		dev_err(&serdev->dev, "Unable to open device\n");
> > +		goto err_open;
> > +	}
> > +
> > +	serdev_device_set_flow_control(serdev, false);
> > +	serdev_device_set_break_detection(serdev, true);
> > +	linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
> > +
> > +	mutex_init(&priv->resp_lock);
> > +
> > +	priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
> > +	if (IS_ERR_OR_NULL(priv->lin_dev)) {
> > +		ret = PTR_ERR(priv->lin_dev);
> 
> As per my feedback on an earlier patch in the series,
> in the case where priv->lin_dev is NULL,
> this will result in the function returning 0.
> Is that ok?
> 
> Flagged by Smatch

IS_ERR_OR_NULL() gets IS_ERR() in upcoming v4

Thanks
  -- Christoph

> 
> > +		goto err_register_lin;
> > +	}
> > +
> > +	serdev_device_close(serdev);
> > +	priv->is_stopped = true;
> > +
> > +	return 0;
> > +
> > +err_register_lin:
> > +	serdev_device_close(serdev);
> > +err_open:
> > +	kfifo_free(&priv->rx_fifo);
> > +	return ret;
> > +}
> 
> ...