[PATCH 00/11] LIN Bus support for Linux

[PATCH 00/11] 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

Christoph Fritz (11):
  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: net: can: Add serdev LIN bus dt bindings
  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/linux,lin-serdev.yaml    |  29 +
 drivers/hid/Kconfig                           |  19 +
 drivers/hid/Makefile                          |   1 +
 drivers/hid/hid-hexlin.c                      | 594 ++++++++++++++++++
 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                  | 467 ++++++++++++++
 drivers/net/can/lin.c                         | 547 ++++++++++++++++
 drivers/tty/serdev/core.c                     |  11 +
 drivers/tty/serdev/serdev-ttyport.c           |  19 +-
 include/linux/serdev.h                        |  19 +-
 include/net/lin.h                             | 105 ++++
 include/uapi/linux/can/bcm.h                  |   5 +-
 include/uapi/linux/can/netlink.h              |   2 +
 net/can/bcm.c                                 |  74 ++-
 17 files changed, 1918 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
 create mode 100644 drivers/hid/hid-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 01/11] 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            | 477 +++++++++++++++++++++++++++++++
 include/net/lin.h                |  97 +++++++
 include/uapi/linux/can/netlink.h |   1 +
 5 files changed, 586 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..a9b5d76708879
--- /dev/null
+++ b/drivers/net/can/lin.c
@@ -0,0 +1,477 @@
+// 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 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, uint 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);
+			kfree(attr);
+			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;
+
+	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 = (u8)(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);
+	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 0;
+}
+
+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 0;
+}
+
+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)
+{
+	uint 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 (u8)(~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 (!ndev)
+		return -ENODEV;
+
+	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;
+	int ret;
+
+	bitrate = ldev->can.bittiming.bitrate;
+	ret = ldev->ldev_ops->update_bitrate(ldev, bitrate);
+
+	return ret;
+}
+
+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) {
+		netdev_err(ndev, "missing mandatory lin_device_ops\n");
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
+	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;
+	}
+
+	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..2fe16e142db96
--- /dev/null
+++ b/include/net/lin.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#ifndef _NET_LIN_H_
+#define _NET_LIN_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		0x0000003FU
+/* special ID descriptions for LIN */
+#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
+
+static const unsigned char 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,
+};
+
+#define LIN_GET_ID(PID)		((PID) & LIN_ID_MASK)
+#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
+					lin_id_parity_tbl[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_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 02/11] 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-hexlin.c | 583 +++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h    |   1 +
 drivers/hid/hid-quirks.c |   3 +
 5 files changed, 607 insertions(+)
 create mode 100644 drivers/hid/hid-hexlin.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 08446c89eff6e..844c3565b397f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -496,6 +496,25 @@ config HID_GYRATION
 	help
 	Support for Gyration remote control.
 
+config HID_HEXLIN
+	tristate "hexLIN LIN Bus adapter"
+	depends on HID
+	select CAN_LIN
+	help
+	  Support for 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 ce71b53ea6c54..4a7e0a388c4c9 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_HEXLIN)	+= hid-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-hexlin.c b/drivers/hid/hid-hexlin.c
new file mode 100644
index 0000000000000..e1a16d79e3259
--- /dev/null
+++ b/drivers/hid/hid-hexlin.c
@@ -0,0 +1,583 @@
+// 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,
+};
+
+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 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;
+
+	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_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_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_tx = hexlin_ldo_tx,
+	.update_bitrate = hexlin_update_bitrate,
+	.get_responder_answer = hexlin_get_responder_answer,
+	.update_responder_answer = hexlin_update_resp_answer,
+};
+
+#define HEXLIN_PKGLEN_MAX	64
+
+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)
+		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 != 1)
+			return -EREMOTEIO;
+		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_FAIL:
+		if (sz != 1)
+			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 != 2)
+			return -EREMOTEIO;
+		priv->fw_version = data[1];
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_RESPONDER_ANSWER_ID:
+		if (sz != 20)
+			return -EREMOTEIO;
+		BUILD_BUG_ON(sizeof(priv->rar) != 20);
+		memcpy(&priv->rar, data, sizeof(priv->rar));
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_BAUDRATE:
+		if (sz != 3)
+			return -EREMOTEIO;
+		BUILD_BUG_ON(sizeof(priv->baudrate) != 2);
+		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 != 17) {
+			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)
+{
+	int 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_HIDRAW);
+	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_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);
+
+	complete(&priv->wait_in_report);
+	unregister_lin(priv->ldev);
+	hid_hw_close(hdev);
+	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_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 8376fb5e2d0b4..157d234e1d400 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -903,6 +903,7 @@
 #define USB_DEVICE_ID_MCC_PMD1208LS	0x007a
 
 #define USB_VENDOR_ID_MCS		0x16d0
+#define USB_DEVICE_ID_MCS_HEXLIN	0x0648
 #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
 
 #define USB_VENDOR_MEGAWORLD		0x07b5
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index e0bbf0c6345d6..328fcc61303f3 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -436,6 +436,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_HEXLIN)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_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 03/11] 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 04/11] 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 05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

[Christoph Fritz]

Add documentation of device tree bindings for serdev UART LIN-Bus
devices equipped with LIN transceivers.

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

diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
new file mode 100644
index 0000000000000..cb4e932ff249c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linux serdev LIN-Bus Support
+
+description: |
+  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
+
+properties:
+  compatible:
+    const: linux,lin-serdev
+
+required:
+  - compatible
+
+examples:
+  - |
+    &uart2 {
+      status = "okay";
+      linbus {
+        compatible = "linux,lin-serdev";
+      };
+    };
-- 
2.39.2

[PATCH 06/11] 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 | 467 +++++++++++++++++++++++++++++++++++
 3 files changed, 484 insertions(+)
 create mode 100644 drivers/net/can/lin-serdev.c

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 0934bbf8d03b2..b930eaea9b235 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 UART 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..18e842c39f111
--- /dev/null
+++ b/drivers/net/can/lin-serdev.c
@@ -0,0 +1,467 @@
+// 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 */
+};
+
+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 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;
+
+	speed = serdev_device_set_baudrate(serdev, bitrate);
+	if (!bitrate || speed != bitrate)
+		return -EINVAL;
+
+	linser_derive_timings(priv, bitrate);
+
+	return 0;
+}
+
+static struct lin_device_ops linser_lindev_ops = {
+	.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];
+	uint count = kfifo_out_peek(&priv->rx_fifo, buf, LINSER_PARSE_BUFFER);
+	uint i, brk = 0;
+
+	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;
+	}
+
+	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);
+
+	if (priv && priv->lin_dev)
+		unregister_lin(priv->lin_dev);
+
+	serdev_device_close(serdev);
+}
+
+static const struct of_device_id linser_of_match[] = {
+	{
+		.compatible = "linux,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 07/11] 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 2fe16e142db96..3f78cfda9c657 100644
--- a/include/net/lin.h
+++ b/include/net/lin.h
@@ -18,6 +18,7 @@
 
 #define LIN_ID_MASK		0x0000003FU
 /* special ID descriptions for LIN */
+#define LIN_RXOFFLOAD_DATA_FLAG	0x00000200U
 #define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
 
 static const unsigned char lin_id_parity_tbl[] = {
-- 
2.39.2

[PATCH 08/11] 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 09/11] 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 | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
index a9b5d76708879..56492952c8588 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -177,6 +177,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,
@@ -184,10 +205,19 @@ static void lin_tx_work_handler(struct work_struct *ws)
 	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;
+
+	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 = (u8)(cfd->can_id & LIN_ID_MASK);
@@ -203,6 +233,8 @@ static void lin_tx_work_handler(struct work_struct *ws)
 
 	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);
 }
-- 
2.39.2

[PATCH 10/11] 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 56492952c8588..472cadc87ad86 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -254,11 +254,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);
@@ -437,7 +466,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;
@@ -452,6 +481,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 3f78cfda9c657..4f064a2ce2ca8 100644
--- a/include/net/lin.h
+++ b/include/net/lin.h
@@ -44,6 +44,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;
@@ -55,6 +60,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 {
@@ -79,6 +85,7 @@ struct lin_responder_answer {
 
 struct lin_device_ops {
 	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 11/11] 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-hexlin.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-hexlin.c b/drivers/hid/hid-hexlin.c
index e1a16d79e3259..4c523e4cdefab 100644
--- a/drivers/hid/hid-hexlin.c
+++ b/drivers/hid/hid-hexlin.c
@@ -171,6 +171,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)
@@ -312,6 +314,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);
@@ -377,6 +387,7 @@ static int hexlin_update_resp_answer(struct lin_device *ldev,
 
 static const struct lin_device_ops hexlin_ldo = {
 	.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 05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

[Krzysztof Kozlowski]

On 22/04/2024 08:51, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.

A nit, subject: drop second/last, redundant "dt bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> new file mode 100644
> index 0000000000000..cb4e932ff249c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linux serdev LIN-Bus Support

This looks like Linux binding, but we expect here description of hardware.


> +
> +description: |
> +  LIN-Bus support for UART devices equipped with LIN transceivers,
> +  utilizing the Serial Device Bus (serdev) interface.

serdev is Linux thingy, AFAIR. Please describe the hardware.

> +
> +  For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> +
> +properties:
> +  compatible:
> +    const: linux,lin-serdev

Feels confusing. Your link describes real hardware, but you wrote
bindings for software construct.

If you add this to DT, then it is hard-wired on the board, right? If so,
how this could be a software construct?


> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    &uart2 {

& does not make much sense here. I think you wanted it to be serial bus,
so serial.

> +      status = "okay";

Drop, it was not disabled anywhere.


> +      linbus {
> +        compatible = "linux,lin-serdev";
> +      };
> +    };

Best regards,
Krzysztof

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

[Jiri Slaby]

On 22. 04. 24, 8:51, 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.
...
> --- /dev/null
> +++ b/drivers/net/can/lin.c
...> +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);
> +			kfree(attr);

Is the latter kfree() right? It appears not.

> +			return -ENOMEM;
> +		}
> +	}
> +
> +	return 0;
> +}
...
> +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;
> +
> +	ldev->tx_busy = true;

How is this store protected against reordering/race conditions?

> +
> +	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 = (u8)(cfd->can_id & LIN_ID_MASK);

Why is that cast necessary?

> +	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;

Where is this label?

> +	}
> +
> +	DEV_STATS_INC(ndev, tx_packets);
> +	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
> +	ldev->tx_busy = false;

The same as above.

> +	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;

And here too.

> +
> +	netif_stop_queue(ndev);
> +	ldev->tx_skb = skb;
> +	queue_work(ldev->wq, &ldev->tx_work);
> +
> +	return NETDEV_TX_OK;
> +}
...
> +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> +		    enum lin_checksum_mode cm)
> +{
> +	uint csum = 0;

Is "uint" of the preffered types in the kernel?

> +	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 (u8)(~csum & 0xff);

Unnecessary cast?

> +}


> +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 (!ndev)
> +		return -ENODEV;

Is this racy or is this only a sanity check?

> +	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;
> +	int ret;
> +
> +	bitrate = ldev->can.bittiming.bitrate;
> +	ret = ldev->ldev_ops->update_bitrate(ldev, bitrate);
> +
> +	return ret;

No need for ret then.

> +}
> +
> +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) {
> +		netdev_err(ndev, "missing mandatory lin_device_ops\n");
> +		return ERR_PTR(-EOPNOTSUPP);

Would EINVAL fit better?

> +	}
> +
> +	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;
> +	}
> +
> +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> +				   0);

It would be worth noting why you need your own WQ.

> +	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..2fe16e142db96
> --- /dev/null
> +++ b/include/net/lin.h
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#ifndef _NET_LIN_H_
> +#define _NET_LIN_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		0x0000003FU

GEN_MASK() ?

> +/* special ID descriptions for LIN */
> +#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
> +
> +static const unsigned char lin_id_parity_tbl[] = {

This ends up in every translation unit you include lin.h into. Bad.

And perhaps u8?

> +	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,
> +};
> +
> +#define LIN_GET_ID(PID)		((PID) & LIN_ID_MASK)

FIELD_GET() ?

> +#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
> +					lin_id_parity_tbl[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)))

thanks,
-- 
js
suse labs

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

[Benjamin Tissoires]

Hi Christoph,

On Apr 22 2024, 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.

Patch looks good from the HID point of view. I have a couple of
questions/nitpicks:

> 
> 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-hexlin.c | 583 +++++++++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h    |   1 +
>  drivers/hid/hid-quirks.c |   3 +
>  5 files changed, 607 insertions(+)
>  create mode 100644 drivers/hid/hid-hexlin.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 08446c89eff6e..844c3565b397f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -496,6 +496,25 @@ config HID_GYRATION
>  	help
>  	Support for Gyration remote control.
>  
> +config HID_HEXLIN
> +	tristate "hexLIN LIN Bus adapter"
> +	depends on HID
> +	select CAN_LIN
> +	help
> +	  Support for 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 ce71b53ea6c54..4a7e0a388c4c9 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_HEXLIN)	+= hid-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-hexlin.c b/drivers/hid/hid-hexlin.c
> new file mode 100644
> index 0000000000000..e1a16d79e3259
> --- /dev/null
> +++ b/drivers/hid/hid-hexlin.c
> @@ -0,0 +1,583 @@
> +// 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,
> +};
> +
> +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);

AFAICT, any operation using the device will use this function and
therefore this is enforcing a single user at the same time.

Is this a bus or a hw limitation?

> +
> +	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 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;
> +
> +	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_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_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_tx = hexlin_ldo_tx,
> +	.update_bitrate = hexlin_update_bitrate,
> +	.get_responder_answer = hexlin_get_responder_answer,
> +	.update_responder_answer = hexlin_update_resp_answer,
> +};
> +
> +#define HEXLIN_PKGLEN_MAX	64
> +
> +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)
> +		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 != 1)
> +			return -EREMOTEIO;

Could we have some #define for all of these sizes (here and in all of
the other branches)?

> +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> +		complete(&priv->wait_in_report);

Shouldn't you ensure that you currently have a request pending?
This works as long as no-one opens the hidraw node (see my remark
below).

> +		break;
> +	case HEXLIN_FAIL:
> +		if (sz != 1)
> +			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 != 2)
> +			return -EREMOTEIO;
> +		priv->fw_version = data[1];
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> +		if (sz != 20)
> +			return -EREMOTEIO;
> +		BUILD_BUG_ON(sizeof(priv->rar) != 20);

magical constants again

> +		memcpy(&priv->rar, data, sizeof(priv->rar));
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_BAUDRATE:
> +		if (sz != 3)
> +			return -EREMOTEIO;
> +		BUILD_BUG_ON(sizeof(priv->baudrate) != 2);
> +		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 != 17) {
> +			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)
> +{
> +	int 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_HIDRAW);

Are you sure you want HID_CONNECT_HIDRAW?

Given that your whole driver relies on the assumption that any command
sent to the device is guarded by the mutex, if one client opens the
hidraw node and starts sending commands behind your back you are
screwed...

Maybe use HID_CONNECT_DRIVER instead.

> +	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_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);

you are not calling hid_hw_close(hdev) here (on purpose I guess).

However, this prevents the device to enter any sleep mode as the kernel
will always consider it to be in use.
Is there some open/close mechanism in LIN or in CAN that can tell the
device that it needs to be opened or do we assume that the device needs
to be powered on all the time?

> +
> +	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);
> +
> +	complete(&priv->wait_in_report);

what if you get one LIN request just now, between those 2 calls?

You should probably disable the ability to take the mutex before sending
the complete call above or you might still have the mutex taken here.

Also shouldn't you set priv->is_error = true before the complete?

> +	unregister_lin(priv->ldev);
> +	hid_hw_close(hdev);
> +	hid_hw_stop(hdev);
> +	mutex_destroy(&priv->tx_lock);

Given how the device works, I think it would be safer to do this in the
following order:

// prevent any incoming event (assuming hidraw is not available)
hid_hw_close(hdev);
// ensure the device is powered off
hid_hw_stop(hdev);
// mark any pending request as failed
priv->is_error = true;
// mark the device as unusable
priv->removed = true;
complete(&priv->wait_in_report);
// unregister
unregister_lin(priv->ldev);
// mutex is not used anymore
mutex_destroy(&priv->tx_lock);

(I might be wrong but this seems more sensible to me).

Actually, instead of having a priv->removed boolean, you could also take
and release the mutex before releasing it, this way you are sure to not
be in the critical code section. This should work because you are using
wait_for_completion_killable_timeout() and so after 1 s you are
guaranteed to exit the mutex.

> +}
> +
> +static const struct hid_device_id hexlin_table[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_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 8376fb5e2d0b4..157d234e1d400 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -903,6 +903,7 @@
>  #define USB_DEVICE_ID_MCC_PMD1208LS	0x007a
>  
>  #define USB_VENDOR_ID_MCS		0x16d0
> +#define USB_DEVICE_ID_MCS_HEXLIN	0x0648
>  #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
>  
>  #define USB_VENDOR_MEGAWORLD		0x07b5
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e0bbf0c6345d6..328fcc61303f3 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -436,6 +436,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_HEXLIN)
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXLIN) },

Generally, the pattern for drivers in the HID subsystem is to rely on
the vendor name, not the product, in order to be able to extend it to
more than one product.

Is your vendor name MCS? Or Hexdev?

If so, the driver should likely be hid-hexdev.c...

> +#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
> 

Cheers,
Benjamin

Re: [PATCH 05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

[Rob Herring]

On Mon, 22 Apr 2024 08:51:08 +0200, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml: 'oneOf' conditional failed, one must be fixed:
	'unevaluatedProperties' is a required property
	'additionalProperties' is a required property
	hint: Either unevaluatedProperties or additionalProperties must be present
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Error: Documentation/devicetree/bindings/net/can/linux,lin-serdev.example.dts:18.9-15 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:427: Documentation/devicetree/bindings/net/can/linux,lin-serdev.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240422065114.3185505-6-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 05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

[Krzysztof Kozlowski]

On 22/04/2024 08:51, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---

Please test patches before sending...

Best regards,
Krzysztof

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

[Christoph Fritz]

...
> > --- /dev/null
> > +++ b/drivers/net/can/lin.c
> ...> +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);
> > +			kfree(attr);
> 
> Is the latter kfree() right? It appears not.

Thanks for the catch, it's wrong and will be removed in v2.

> 
> > +			return -ENOMEM;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> ...
> > +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;
> > +
> > +	ldev->tx_busy = true;
> 
> How is this store protected against reordering/race conditions?

Falsely it is not, like in mcp251x.c I'll add a mutex.

> 
> > +
> > +	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 = (u8)(cfd->can_id & LIN_ID_MASK);
> 
> Why is that cast necessary?

It's not.

> 
> > +	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;
> 
> Where is this label?

In a later patch, let me fix the patchset accordingly.

> 
> > +	}
> > +
> > +	DEV_STATS_INC(ndev, tx_packets);
> > +	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
> > +	ldev->tx_busy = false;
> 
> The same as above.

OK

> 
> > +	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;
> 
> And here too.

OK

> 
> > +
> > +	netif_stop_queue(ndev);
> > +	ldev->tx_skb = skb;
> > +	queue_work(ldev->wq, &ldev->tx_work);
> > +
> > +	return NETDEV_TX_OK;
> > +}
> ...
> > +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> > +		    enum lin_checksum_mode cm)
> > +{
> > +	uint csum = 0;
> 
> Is "uint" of the preffered types in the kernel?

OK, no sysv 'uint', will be changed in another patch too

> 
> > +	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 (u8)(~csum & 0xff);
> 
> Unnecessary cast?

Yes

> 
> > +}
> 
> 
> > +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 (!ndev)
> > +		return -ENODEV;
> 
> Is this racy or is this only a sanity check?

Just beeing cautious, I guess it can be removed

> 
> > +	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;
> > +	int ret;
> > +
> > +	bitrate = ldev->can.bittiming.bitrate;
> > +	ret = ldev->ldev_ops->update_bitrate(ldev, bitrate);
> > +
> > +	return ret;
> 
> No need for ret then.

Compact code, sure, will get adapted

> 
> > +}
> > +
> > +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) {
> > +		netdev_err(ndev, "missing mandatory lin_device_ops\n");
> > +		return ERR_PTR(-EOPNOTSUPP);
> 
> Would EINVAL fit better?

no preference over the other, will use 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;
> > +	}
> > +
> > +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> > +				   0);
> 
> It would be worth noting why you need your own WQ.

I'll add a comment stating:

/* Use workqueue as tx over USB/SPI/... may sleep */

> 
> > +	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..2fe16e142db96
> > --- /dev/null
> > +++ b/include/net/lin.h
> > @@ -0,0 +1,97 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> > +
> > +#ifndef _NET_LIN_H_
> > +#define _NET_LIN_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		0x0000003FU
> 
> GEN_MASK() ?

In the upcoming v2 I'll use:

#define LIN_ID_MASK   GENMASK(5, 0)

> 
> > +/* special ID descriptions for LIN */
> > +#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
> > +
> > +static const unsigned char lin_id_parity_tbl[] = {
> 
> This ends up in every translation unit you include lin.h into. Bad.

This is also being used by a serial lin driver. But I guess most of the drivers have no need for this. Mhm, ... any ideas?

> And perhaps u8?

OK

> 
> > +	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,
> > +};
> > +
> > +#define LIN_GET_ID(PID)		((PID) & LIN_ID_MASK)
> 
> FIELD_GET() ?

In the upcoming v2 I'll use:

#define LIN_GET_ID(PID)		FIELD_GET(LIN_ID_MASK, PID)

> 
> > +#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
> > +					lin_id_parity_tbl[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)))
> 

Thanks for the feedback
  -- Christoph

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

[Jiri Slaby]

On 23. 04. 24, 11:33, Christoph Fritz wrote:
>>> --- /dev/null
>>> +++ b/include/net/lin.h
>>> @@ -0,0 +1,97 @@
...
>>> +/* special ID descriptions for LIN */
>>> +#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U
>>> +
>>> +static const unsigned char lin_id_parity_tbl[] = {
>>
>> This ends up in every translation unit you include lin.h into. Bad.
> 
> This is also being used by a serial lin driver. But I guess most of the drivers have no need for this. Mhm, ... any ideas?

If needs be, put it to a .c and keep an extern here.

thanks,
-- 
js
suse labs

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

[Christoph Fritz]

Hi Benjamin,

 thanks for your review, please see my answers below.

...
> > +
> > +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);
> 
> AFAICT, any operation using the device will use this function and
> therefore this is enforcing a single user at the same time.
> 
> Is this a bus or a hw limitation?

It's a hw limitation.

> > +
> > +	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;
> > +}
...
> > +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)
> > +		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 != 1)
> > +			return -EREMOTEIO;
> 
> Could we have some #define for all of these sizes (here and in all of
> the other branches)?

OK

> 
> > +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> > +		complete(&priv->wait_in_report);
> 
> Shouldn't you ensure that you currently have a request pending?
> This works as long as no-one opens the hidraw node (see my remark
> below).

Thanks for the heads up, there is no need for hidraw.

> > +		break;
> > +	case HEXLIN_FAIL:
> > +		if (sz != 1)
> > +			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 != 2)
> > +			return -EREMOTEIO;
> > +		priv->fw_version = data[1];
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> > +		if (sz != 20)
> > +			return -EREMOTEIO;
> > +		BUILD_BUG_ON(sizeof(priv->rar) != 20);
> 
> magical constants again

OK

> 
> > +		memcpy(&priv->rar, data, sizeof(priv->rar));
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	case HEXLIN_GET_BAUDRATE:
> > +		if (sz != 3)
> > +			return -EREMOTEIO;
> > +		BUILD_BUG_ON(sizeof(priv->baudrate) != 2);
> > +		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 != 17) {
> > +			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)
> > +{
> > +	int 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_HIDRAW);
> 
> Are you sure you want HID_CONNECT_HIDRAW?
> 
> Given that your whole driver relies on the assumption that any command
> sent to the device is guarded by the mutex, if one client opens the
> hidraw node and starts sending commands behind your back you are
> screwed...
> 
> Maybe use HID_CONNECT_DRIVER instead.

HID_CONNECT_DRIVER it is

> 
> > +	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_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> 
> you are not calling hid_hw_close(hdev) here (on purpose I guess).
> 
> However, this prevents the device to enter any sleep mode as the kernel
> will always consider it to be in use.
> Is there some open/close mechanism in LIN or in CAN that can tell the
> device that it needs to be opened or do we assume that the device needs
> to be powered on all the time?

One can bring the LIN device up and down, just like any other Ethernet
or CAN device. So, for revision 2 of this patchset, I added open/stop
handling. This allows for hid_hw_close(hdev) here and also makes
remove() handling way easier. Thanks for the heads up.

> 
> > +
> > +	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);
> > +
> > +	complete(&priv->wait_in_report);
> 
> what if you get one LIN request just now, between those 2 calls?
> 
> You should probably disable the ability to take the mutex before sending
> the complete call above or you might still have the mutex taken here.
> 
> Also shouldn't you set priv->is_error = true before the complete?
> 
> > +	unregister_lin(priv->ldev);
> > +	hid_hw_close(hdev);
> > +	hid_hw_stop(hdev);
> > +	mutex_destroy(&priv->tx_lock);
> 
> Given how the device works, I think it would be safer to do this in the
> following order:
> 
> // prevent any incoming event (assuming hidraw is not available)
> hid_hw_close(hdev);
> // ensure the device is powered off
> hid_hw_stop(hdev);
> // mark any pending request as failed
> priv->is_error = true;
> // mark the device as unusable
> priv->removed = true;
> complete(&priv->wait_in_report);
> // unregister
> unregister_lin(priv->ldev);
> // mutex is not used anymore
> mutex_destroy(&priv->tx_lock);
> 
> (I might be wrong but this seems more sensible to me).
> 
> Actually, instead of having a priv->removed boolean, you could also take
> and release the mutex before releasing it, this way you are sure to not
> be in the critical code section. This should work because you are using
> wait_for_completion_killable_timeout() and so after 1 s you are
> guaranteed to exit the mutex.

Thanks for the great explanation.

> > +}
> > +
> > +static const struct hid_device_id hexlin_table[] = {
> > +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_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 8376fb5e2d0b4..157d234e1d400 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -903,6 +903,7 @@
> >  #define USB_DEVICE_ID_MCC_PMD1208LS	0x007a
> >  
> >  #define USB_VENDOR_ID_MCS		0x16d0
> > +#define USB_DEVICE_ID_MCS_HEXLIN	0x0648
> >  #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
> >  
> >  #define USB_VENDOR_MEGAWORLD		0x07b5
> > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > index e0bbf0c6345d6..328fcc61303f3 100644
> > --- a/drivers/hid/hid-quirks.c
> > +++ b/drivers/hid/hid-quirks.c
> > @@ -436,6 +436,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_HEXLIN)
> > +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXLIN) },
> 
> Generally, the pattern for drivers in the HID subsystem is to rely on
> the vendor name, not the product, in order to be able to extend it to
> more than one product.
> 
> Is your vendor name MCS? Or Hexdev?
> 
> If so, the driver should likely be hid-hexdev.c...

We got the PID from MCS online shop here:

https://www.mcselec.com/index.php?page=shop.product_details&product_id=92&option=com_phpshop

Our vendor name is hexDEV, but the USB VID is MCS, and the product name
is hexLIN...

So is 'hid-hexlin.c' or 'hid-hexdev-hexlin.c' okay or does it need to
be named 'hid-mcs-hexlin.c' in spite MCS has nearly nothing to do with
it?


Cheers
  -- Christoph

Re: [PATCH 05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

[Christoph Fritz]

Hello Krzysztof,

 thanks for your feedback, please see my answers below.

On Mon, 2024-04-22 at 09:54 +0200, Krzysztof Kozlowski wrote:
> On 22/04/2024 08:51, Christoph Fritz wrote:
> > Add documentation of device tree bindings for serdev UART LIN-Bus
> > devices equipped with LIN transceivers.
> 
> A nit, subject: drop second/last, redundant "dt bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

OK

> 
> > 
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > ---
> >  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > new file mode 100644
> > index 0000000000000..cb4e932ff249c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > @@ -0,0 +1,29 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Linux serdev LIN-Bus Support
> 
> This looks like Linux binding, but we expect here description of hardware.

OK


> > +
> > +description: |
> > +  LIN-Bus support for UART devices equipped with LIN transceivers,
> > +  utilizing the Serial Device Bus (serdev) interface.
> 
> serdev is Linux thingy, AFAIR. Please describe the hardware.

OK, in v2 it will get changed to:

"""
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
> > +
> > +properties:
> > +  compatible:
> > +    const: linux,lin-serdev
> 
> Feels confusing. Your link describes real hardware, but you wrote
> bindings for software construct.
> 
> If you add this to DT, then it is hard-wired on the board, right?

Yes, it is hard-wired.

>  If so, how this could be a software construct?

It's not, but fairly generic, that's why I used 'linux,lin-serdev' as
compatible string. In v2 I'll change it to 'hexdev,lin-serdev'.

> > +
> > +required:
> > +  - compatible
> > +
> > +examples:
> > +  - |
> > +    &uart2 {
> 
> & does not make much sense here. I think you wanted it to be serial bus,
> so serial.

OK

> 
> > +      status = "okay";
> 
> Drop, it was not disabled anywhere.

OK

> 
> 
> > +      linbus {
> > +        compatible = "linux,lin-serdev";
> > +      };
> > +    };

Let me address these points, fix warnings from dt_binding_check and
reroll in v2.

Thanks
  -- Christoph