[PATCH v4 00/11] LIN Bus support for Linux

[PATCH v4 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.

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#hexLINSER
[5]: https://github.com/raspberrypi/linux/issues/5985
[6]: https://github.com/lin-bus/linux-lin/blob/master/sllin/sllin.c
[7]: https://github.com/ch-f/iproute2/tree/lin-feature
[8]: https://github.com/ch-f/lin-utils

Changes in v4:
 - rebase to next-20240509
 - dt-bindings: rename lin-serdev to hexlinser
 - lin: don't use ndev uninitialized, use dev_err() instead
 - lin: fix missing failure return value
 - lin: use sysfs_emit() instead of scnprintf()
 - lin: use static init of sysfs files
 - hexlin: fix hid_dbg indent
 - hexlin: use __le16 for req.baudrate
 - hexlin: refactor to be able to drop kmemdup()
 - linser and hexlin: s/IS_ERR_OR_NULL()/IS_ERR()/
 - serdev treewide: refactor receive_buf() by adding argument flags
 - address minor changes from review

Changes in v3:
 - drop unneccessary mutex_lock/unlock and _destroy() in hexlin
 - add Kconfig depends for HID_MCS_HEXDEV
 - simplify reset retry in hexlin
 - simplify hid driver init
 - fix dt-bindings and its commit message
 - adapt and reorder includes
 - use unsigned long instead of ulong
 - use macro USEC_PER_SEC in linser_derive_timings()
 - drop goto in linser_tx_frame_as_responder() and use unlock+return
 - simplify linser_pop_fifo() by using kfifo_skip()
 - squash [PATCH v2 08/12] can: lin: Add special frame id for rx...

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

Christoph Fritz (11):
  can: Add LIN bus as CAN abstraction
  HID: hexLIN: Add support for USB LIN adapter
  treewide, serdev: add flags argument to receive_buf()
  tty: serdev: Add method to enable break flags
  dt-bindings: vendor-prefixes: Add hexDEV
  dt-bindings: net/can: Add serial LIN adapter hexLINSER
  can: Add support for hexDEV serial LIN adapter hexLINSER
  can: bcm: Add LIN answer offloading for responder mode
  can: lin: Handle rx offload config frames
  can: lin: Support setting LIN mode
  HID: hexLIN: Implement ability to update lin mode

 .../bindings/net/can/hexdev,hex-linser.yaml   |  32 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/bluetooth/btmtkuart.c                 |   3 +-
 drivers/bluetooth/btnxpuart.c                 |   3 +-
 drivers/bluetooth/hci_serdev.c                |   3 +-
 drivers/gnss/serial.c                         |   2 +-
 drivers/gnss/sirf.c                           |   2 +-
 drivers/greybus/gb-beagleplay.c               |   2 +-
 drivers/hid/Kconfig                           |  19 +
 drivers/hid/Makefile                          |   1 +
 drivers/hid/hid-hexdev-hexlin.c               | 620 ++++++++++++++++++
 drivers/hid/hid-ids.h                         |   1 +
 drivers/hid/hid-quirks.c                      |   3 +
 drivers/iio/chemical/pms7003.c                |   2 +-
 drivers/iio/chemical/scd30_serial.c           |   3 +-
 drivers/iio/chemical/sps30_serial.c           |   3 +-
 drivers/iio/imu/bno055/bno055_ser_core.c      |   3 +-
 drivers/mfd/rave-sp.c                         |   2 +-
 drivers/net/can/Kconfig                       |  25 +
 drivers/net/can/Makefile                      |   2 +
 drivers/net/can/hex-linser.c                  | 505 ++++++++++++++
 drivers/net/can/lin.c                         | 537 +++++++++++++++
 drivers/net/ethernet/qualcomm/qca_uart.c      |   3 +-
 drivers/nfc/pn533/uart.c                      |   2 +-
 drivers/nfc/s3fwrn5/uart.c                    |   3 +-
 drivers/platform/chrome/cros_ec_uart.c        |   3 +-
 drivers/platform/surface/aggregator/core.c    |   2 +-
 .../lenovo-yoga-tab2-pro-1380-fastcharger.c   |   3 +-
 drivers/tty/serdev/core.c                     |  11 +
 drivers/tty/serdev/serdev-ttyport.c           |  19 +-
 drivers/w1/masters/w1-uart.c                  |   3 +-
 include/linux/serdev.h                        |  12 +-
 include/net/lin.h                             |  97 +++
 include/uapi/linux/can/bcm.h                  |   5 +-
 include/uapi/linux/can/netlink.h              |   2 +
 net/can/bcm.c                                 |  72 ++
 sound/drivers/serial-generic.c                |   3 +-
 37 files changed, 1991 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,hex-linser.yaml
 create mode 100644 drivers/hid/hid-hexdev-hexlin.c
 create mode 100644 drivers/net/can/hex-linser.c
 create mode 100644 drivers/net/can/lin.c
 create mode 100644 include/net/lin.h

-- 
2.39.2

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

[Christoph Fritz]

Introduce a LIN (local interconnect network) abstraction on top of CAN.
This 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            | 470 +++++++++++++++++++++++++++++++
 include/net/lin.h                |  90 ++++++
 include/uapi/linux/can/netlink.h |   1 +
 5 files changed, 572 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..a22768c17e3f8
--- /dev/null
+++ b/drivers/net/can/lin.c
@@ -0,0 +1,470 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#include <linux/can/core.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <net/lin.h>
+
+static const u8 lin_id_parity_tbl[] = {
+	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
+	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
+	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
+	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
+	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
+	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
+	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
+	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
+};
+
+u8 lin_get_id_parity(u8 id)
+{
+	return lin_id_parity_tbl[id];
+}
+EXPORT_SYMBOL(lin_get_id_parity);
+
+static ssize_t lin_identifier_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct lin_device *ldev = netdev_priv(to_net_dev(dev));
+	struct lin_responder_answer answ;
+	int k, count, 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 = sysfs_emit(buf, "%-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 += sysfs_emit_at(buf, 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 += sysfs_emit_at(buf, count, "%02x ", answ.lf.data[k]);
+	for (; k < 8; k++)
+		count += sysfs_emit_at(buf, count, "   ");
+	if (answ.lf.len)
+		count += sysfs_emit_at(buf, count, " %02x", answ.lf.checksum);
+
+	count += sysfs_emit_at(buf, count, "\n");
+
+	return count;
+}
+
+static const char *parse_and_advance(const char *buf, long *result,
+				     unsigned int base)
+{
+	char num_str[5] = {0};
+	int num_len = 0;
+
+	while (*buf && isspace(*buf))
+		buf++;
+	while (*buf && isalnum(*buf) && num_len < sizeof(num_str) - 1)
+		num_str[num_len++] = *buf++;
+	if (kstrtol(num_str, base, result))
+		return NULL;
+
+	return buf;
+}
+
+static ssize_t lin_identifier_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct lin_device *ldev = netdev_priv(to_net_dev(dev));
+	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;
+}
+
+#define LID(_name) \
+	struct device_attribute linid_##_name = __ATTR(_name, 0644, \
+	lin_identifier_show, lin_identifier_store)
+
+LID(00); LID(01); LID(02); LID(03); LID(04); LID(05); LID(06); LID(07);
+LID(08); LID(09); LID(0a); LID(0b); LID(0c); LID(0d); LID(0e); LID(0f);
+LID(10); LID(11); LID(12); LID(13); LID(14); LID(15); LID(16); LID(17);
+LID(18); LID(19); LID(1a); LID(1b); LID(1c); LID(1d); LID(1e); LID(1f);
+LID(20); LID(21); LID(22); LID(23); LID(24); LID(25); LID(26); LID(27);
+LID(28); LID(29); LID(2a); LID(2b); LID(2c); LID(2d); LID(2e); LID(2f);
+LID(30); LID(31); LID(32); LID(33); LID(34); LID(35); LID(36); LID(37);
+LID(38); LID(39); LID(3a); LID(3b); LID(3c); LID(3d); LID(3e); LID(3f);
+
+static struct attribute *lin_sysfs_attrs[] = {
+	&linid_00.attr, &linid_01.attr, &linid_02.attr, &linid_03.attr,
+	&linid_04.attr, &linid_05.attr, &linid_06.attr, &linid_07.attr,
+	&linid_08.attr, &linid_09.attr, &linid_0a.attr, &linid_0b.attr,
+	&linid_0c.attr, &linid_0d.attr, &linid_0e.attr, &linid_0f.attr,
+	&linid_10.attr, &linid_11.attr, &linid_12.attr, &linid_13.attr,
+	&linid_14.attr, &linid_15.attr, &linid_16.attr, &linid_17.attr,
+	&linid_18.attr, &linid_19.attr, &linid_1a.attr, &linid_1b.attr,
+	&linid_1c.attr, &linid_1d.attr, &linid_1e.attr, &linid_1f.attr,
+	&linid_20.attr, &linid_21.attr, &linid_22.attr, &linid_23.attr,
+	&linid_24.attr, &linid_25.attr, &linid_26.attr, &linid_27.attr,
+	&linid_28.attr, &linid_29.attr, &linid_2a.attr, &linid_2b.attr,
+	&linid_2c.attr, &linid_2d.attr, &linid_2e.attr, &linid_2f.attr,
+	&linid_30.attr, &linid_31.attr, &linid_32.attr, &linid_33.attr,
+	&linid_34.attr, &linid_35.attr, &linid_36.attr, &linid_37.attr,
+	&linid_38.attr, &linid_39.attr, &linid_3a.attr, &linid_3b.attr,
+	&linid_3c.attr, &linid_3d.attr, &linid_3e.attr, &linid_3f.attr,
+	NULL
+};
+
+static const struct attribute_group lin_sysfs_group = {
+	.name = "lin_ids",
+	.attrs = lin_sysfs_attrs,
+};
+
+static void lin_tx_work_handler(struct work_struct *ws)
+{
+	struct lin_device *ldev = container_of(ws, struct lin_device,
+					       tx_work);
+	struct net_device *ndev = ldev->ndev;
+	struct canfd_frame *cfd;
+	struct lin_frame lf;
+	int ret;
+
+	ldev->tx_busy = true;
+
+	cfd = (struct canfd_frame *)ldev->tx_skb->data;
+	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
+			   LINBUS_ENHANCED : LINBUS_CLASSIC;
+	lf.lin_id = cfd->can_id & LIN_ID_MASK;
+	lf.len = min(cfd->len, LIN_MAX_DLEN);
+	memcpy(lf.data, cfd->data, lf.len);
+
+	ret = ldev->ldev_ops->ldo_tx(ldev, &lf);
+	if (ret) {
+		DEV_STATS_INC(ndev, tx_dropped);
+		netdev_err_once(ndev, "transmission failure %d\n", ret);
+		goto lin_tx_out;
+	}
+
+	DEV_STATS_INC(ndev, tx_packets);
+	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
+
+lin_tx_out:
+	ldev->tx_busy = false;
+	netif_wake_queue(ndev);
+}
+
+static netdev_tx_t lin_start_xmit(struct sk_buff *skb,
+				  struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+
+	if (ldev->tx_busy)
+		return NETDEV_TX_BUSY;
+
+	netif_stop_queue(ndev);
+	ldev->tx_skb = skb;
+	queue_work(ldev->wq, &ldev->tx_work);
+
+	return NETDEV_TX_OK;
+}
+
+static int lin_open(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	int ret;
+
+	ldev->tx_busy = false;
+
+	ret = open_candev(ndev);
+	if (ret)
+		return ret;
+
+	netif_wake_queue(ndev);
+
+	ldev->can.state = CAN_STATE_ERROR_ACTIVE;
+	ndev->mtu = CANFD_MTU;
+
+	return ldev->ldev_ops->ldo_open(ldev);
+}
+
+static int lin_stop(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+
+	close_candev(ndev);
+
+	flush_work(&ldev->tx_work);
+
+	ldev->can.state = CAN_STATE_STOPPED;
+
+	return ldev->ldev_ops->ldo_stop(ldev);
+}
+
+static const struct net_device_ops lin_netdev_ops = {
+	.ndo_open = lin_open,
+	.ndo_stop = lin_stop,
+	.ndo_start_xmit = lin_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
+};
+
+u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
+		    enum lin_checksum_mode cm)
+{
+	unsigned int csum = 0;
+	int i;
+
+	if (cm == LINBUS_ENHANCED)
+		csum += pid;
+
+	for (i = 0; i < n_of_bytes; i++) {
+		csum += bytes[i];
+		if (csum > 255)
+			csum -= 255;
+	}
+
+	return (~csum & 0xff);
+}
+EXPORT_SYMBOL_GPL(lin_get_checksum);
+
+static int lin_bump_rx_err(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct net_device *ndev = ldev->ndev;
+	struct can_frame cf = {0 };
+
+	if (lf->lin_id > LIN_ID_MASK) {
+		netdev_dbg(ndev, "id exceeds LIN max id\n");
+		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+		cf.data[3] = CAN_ERR_PROT_LOC_ID12_05;
+	}
+
+	if (lf->len > LIN_MAX_DLEN) {
+		netdev_dbg(ndev, "frame exceeds number of bytes\n");
+		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+		cf.data[3] = CAN_ERR_PROT_LOC_DLC;
+	}
+
+	if (lf->len) {
+		u8 checksum = lin_get_checksum(LIN_FORM_PID(lf->lin_id),
+					       lf->len, lf->data,
+					       lf->checksum_mode);
+
+		if (checksum != lf->checksum) {
+			netdev_dbg(ndev, "expected cksm: 0x%02x got: 0x%02x\n",
+				   checksum, lf->checksum);
+			cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
+			cf.data[2] = CAN_ERR_PROT_FORM;
+		}
+	}
+
+	if (cf.can_id & CAN_ERR_FLAG) {
+		struct can_frame *err_cf;
+		struct sk_buff *skb = alloc_can_err_skb(ndev, &err_cf);
+
+		if (unlikely(!skb))
+			return -ENOMEM;
+
+		err_cf->can_id |= cf.can_id;
+		memcpy(err_cf->data, cf.data, CAN_MAX_DLEN);
+
+		netif_rx(skb);
+
+		return -EREMOTEIO;
+	}
+
+	return 0;
+}
+
+int lin_rx(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct net_device *ndev = ldev->ndev;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	int ret;
+
+	if (ldev->can.state == CAN_STATE_STOPPED)
+		return 0;
+
+	netdev_dbg(ndev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
+		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
+		   lf->checksum_mode ? "enhanced" : "classic");
+
+	ret = lin_bump_rx_err(ldev, lf);
+	if (ret) {
+		DEV_STATS_INC(ndev, rx_dropped);
+		return ret;
+	}
+
+	skb = alloc_can_skb(ndev, &cf);
+	if (unlikely(!skb)) {
+		DEV_STATS_INC(ndev, rx_dropped);
+		return -ENOMEM;
+	}
+
+	cf->can_id = lf->lin_id;
+	cf->len = min(lf->len, LIN_MAX_DLEN);
+	memcpy(cf->data, lf->data, cf->len);
+
+	DEV_STATS_INC(ndev, rx_packets);
+	DEV_STATS_ADD(ndev, rx_bytes, cf->len);
+
+	netif_receive_skb(skb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lin_rx);
+
+static int lin_set_bittiming(struct net_device *ndev)
+{
+	struct lin_device *ldev = netdev_priv(ndev);
+	unsigned int bitrate = ldev->can.bittiming.bitrate;
+
+	return ldev->ldev_ops->update_bitrate(ldev, bitrate);
+}
+
+static const u32 lin_bitrate[] = { 1200, 2400, 4800, 9600, 19200 };
+
+struct lin_device *register_lin(struct device *dev,
+				const struct lin_device_ops *ldops)
+{
+	struct net_device *ndev;
+	struct lin_device *ldev;
+	int ret;
+
+	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||
+	    !ldops->ldo_open || !ldops->ldo_stop) {
+		dev_err(dev, "missing mandatory lin_device_ops\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	ndev = alloc_candev(sizeof(struct lin_device), 1);
+	if (!ndev)
+		return ERR_PTR(-ENOMEM);
+
+	ldev = netdev_priv(ndev);
+
+	ldev->ldev_ops = ldops;
+	ndev->netdev_ops = &lin_netdev_ops;
+	ndev->flags |= IFF_ECHO;
+	ndev->mtu = CANFD_MTU;
+	ndev->sysfs_groups[0] = &lin_sysfs_group;
+	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;
+
+	/* Using workqueue as tx over USB/SPI/... may sleep */
+	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
+				   0);
+	if (!ldev->wq) {
+		ret = -ENOMEM;
+		goto exit_unreg;
+	}
+
+	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
+
+	netdev_info(ndev, "LIN initialized\n");
+
+	return ldev;
+
+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;
+
+	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..31bb0feefd188
--- /dev/null
+++ b/include/net/lin.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#ifndef _NET_LIN_H_
+#define _NET_LIN_H_
+
+#include <linux/bitfield.h>
+#include <linux/can/dev.h>
+#include <linux/device.h>
+
+#define LIN_NUM_IDS		64
+#define LIN_HEADER_SIZE		3
+#define LIN_MAX_DLEN		8
+
+#define LIN_MAX_BAUDRATE	20000
+#define LIN_MIN_BAUDRATE	1000
+#define LIN_DEFAULT_BAUDRATE	9600
+#define LIN_SYNC_BYTE		0x55
+
+#define LIN_ID_MASK		GENMASK(5, 0)
+/* special ID descriptions for LIN */
+#define LIN_RXOFFLOAD_DATA_FLAG	BIT(9)
+#define LIN_ENHANCED_CKSUM_FLAG	BIT(8)
+
+extern u8 lin_get_id_parity(u8 id);
+
+#define LIN_GET_ID(PID)		FIELD_GET(LIN_ID_MASK, PID)
+#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
+					lin_get_id_parity(LIN_GET_ID(ID)))
+#define LIN_GET_PARITY(PID)	((PID) & ~LIN_ID_MASK)
+#define LIN_CHECK_PID(PID)	(LIN_GET_PARITY(PID) == \
+					LIN_GET_PARITY(LIN_FORM_PID(PID)))
+
+struct lin_attr {
+	struct kobj_attribute attr;
+	struct lin_device *ldev;
+};
+
+struct lin_device {
+	struct can_priv can;  /* must be the first member */
+	struct net_device *ndev;
+	struct device *dev;
+	const struct lin_device_ops *ldev_ops;
+	struct workqueue_struct *wq;
+	struct work_struct tx_work;
+	bool tx_busy;
+	struct sk_buff *tx_skb;
+};
+
+enum lin_checksum_mode {
+	LINBUS_CLASSIC = 0,
+	LINBUS_ENHANCED,
+};
+
+struct lin_frame {
+	u8 lin_id;
+	u8 len;
+	u8 data[LIN_MAX_DLEN];
+	u8 checksum;
+	enum lin_checksum_mode checksum_mode;
+};
+
+struct lin_responder_answer {
+	bool is_active;
+	bool is_event_frame;
+	u8 event_associated_id;
+	struct lin_frame lf;
+};
+
+struct lin_device_ops {
+	int (*ldo_open)(struct lin_device *ldev);
+	int (*ldo_stop)(struct lin_device *ldev);
+	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
+	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
+	int (*update_responder_answer)(struct lin_device *ldev,
+				       const struct lin_responder_answer *answ);
+	int (*get_responder_answer)(struct lin_device *ldev, u8 id,
+				    struct lin_responder_answer *answ);
+};
+
+int lin_rx(struct lin_device *ldev, const struct lin_frame *lf);
+
+u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
+		    enum lin_checksum_mode cm);
+
+struct lin_device *register_lin(struct device *dev,
+				const struct lin_device_ops *ldops);
+void unregister_lin(struct lin_device *ldev);
+
+#endif /* _NET_LIN_H_ */
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index 02ec32d694742..a37f56d86c5f2 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		BIT(11)	/* LIN bus mode */
 
 /*
  * CAN device statistics
-- 
2.39.2

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

[Christoph Fritz]

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

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

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

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 08446c89eff6e..682e3ab5fdfe5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -496,6 +496,25 @@ config HID_GYRATION
 	help
 	Support for Gyration remote control.
 
+config HID_MCS_HEXDEV
+	tristate "hexDEV LIN-BUS adapter support"
+	depends on HID && CAN_NETLINK && CAN_DEV
+	select CAN_LIN
+	help
+	  Support for hexDEV its hexLIN USB LIN bus adapter.
+
+	  Local Interconnect Network (LIN) to USB adapter for controller and
+	  responder usage.
+	  This device driver is using CAN_LIN for a userland connection on
+	  one side and USB HID for the actual hardware adapter on the other
+	  side.
+
+	  If you have such an adapter, say Y here and see
+	  <https://hexdev.de/hexlin>.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hid-hexlin.
+
 config HID_ICADE
 	tristate "ION iCade arcade controller"
 	help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index ce71b53ea6c54..6af678f283548 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_HID_GOOGLE_STADIA_FF)	+= hid-google-stadiaff.o
 obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
+obj-$(CONFIG_HID_MCS_HEXDEV)	+= hid-hexdev-hexlin.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
new file mode 100644
index 0000000000000..a9ed080b3e33e
--- /dev/null
+++ b/drivers/hid/hid-hexdev-hexlin.c
@@ -0,0 +1,609 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * LIN bus USB adapter driver https://hexdev.de/hexlin
+ *
+ * Copyright (C) 2024 hexDEV GmbH
+ */
+
+#include <linux/completion.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/wait.h>
+#include <net/lin.h>
+#include "hid-ids.h"
+
+#define HEXLIN_PKGLEN_MAX	64
+#define HEXLIN_LEN_RETCODE	1
+
+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;
+	__le16 baudrate;
+} __packed;
+
+struct hexlin_frame {
+	__le32 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 answ;
+	u8 fw_version;
+};
+
+static int hexlin_tx_req_status(struct hexlin_priv_data *priv,
+				const void *out_report, int len)
+{
+	unsigned long t;
+	int n, ret = 0;
+
+	mutex_lock(&priv->tx_lock);
+
+	reinit_completion(&priv->wait_in_report);
+
+	n = hid_hw_output_report(priv->hid_dev, (__u8 *) out_report, len);
+	if (n < 0) {
+		mutex_unlock(&priv->tx_lock);
+		return n;
+	}
+	if (n != len) {
+		mutex_unlock(&priv->tx_lock);
+		return -EIO;
+	}
+
+	t = wait_for_completion_killable_timeout(&priv->wait_in_report, HZ);
+	if (!t)
+		ret = -ETIMEDOUT;
+
+	if (priv->is_error)
+		ret = -EINVAL;
+
+	mutex_unlock(&priv->tx_lock);
+
+	return ret;
+}
+
+#define HEXLIN_GET_CMD(name, enum_cmd)					\
+	static int hexlin_##name(struct hexlin_priv_data *p)		\
+	{								\
+		u8 *req;						\
+		int ret;						\
+									\
+		req = kmalloc(sizeof(*req), GFP_KERNEL)	;		\
+		if (!req)						\
+			return -ENOMEM;					\
+									\
+		*req = enum_cmd;					\
+									\
+		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
+		if (ret)						\
+			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
+				#name, ret);				\
+									\
+		kfree(req);						\
+		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 = kmalloc(sizeof(*req), GFP_KERNEL)	;		\
+		if (!req)						\
+			return -ENOMEM;					\
+									\
+		req->cmd = enum_cmd;					\
+		req->v = val;						\
+									\
+		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
+		if (ret)						\
+			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
+				#name, ret);				\
+									\
+		kfree(req);						\
+		return ret;						\
+	}
+
+HEXLIN_VAL_CMD(send_break, HEXLIN_SEND_BREAK, hexlin_val8_req, u8)
+
+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 = kmalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->cmd = HEXLIN_SEND_UNCONDITIONAL_FRAME;
+	memcpy(&req->frm, hxf, sizeof(*hxf));
+
+	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
+	if (ret)
+		hid_err(priv->hid_dev, "unconditional tx failed: %d\n", ret);
+
+	kfree(req);
+	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 = kmalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->cmd = HEXLIN_SET_BAUDRATE;
+	req->baudrate = cpu_to_le16(baudrate);
+
+	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
+	if (ret)
+		hid_err(priv->hid_dev, "set baudrate failed: %d\n", ret);
+
+	kfree(req);
+	return ret;
+}
+
+static int hexlin_get_responder_answer_id(struct hexlin_priv_data *priv, u8 id,
+					  struct hexlin_responder_answer *ransw)
+{
+	struct hexlin_val8_req *req;
+	int ret;
+
+	if (id > LIN_ID_MASK)
+		return -EINVAL;
+
+	req = kmalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->cmd = HEXLIN_GET_RESPONDER_ANSWER_ID;
+	req->v = id;
+
+	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
+	if (ret) {
+		hid_err(priv->hid_dev, "get respond answer failed: %d\n", ret);
+		kfree(req);
+		return ret;
+	}
+
+	memcpy(ransw, &priv->answ, sizeof(priv->answ));
+
+	kfree(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 *req;
+	int ret;
+
+	if (answ->lf.lin_id > LIN_ID_MASK ||
+	    answ->event_associated_id > LIN_ID_MASK)
+		return -EINVAL;
+
+	req = kmalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->cmd = HEXLIN_SET_RESPONDER_ANSWER_ID;
+	req->answ.is_active = answ->is_active;
+	req->answ.is_event_frame = answ->is_event_frame;
+	req->answ.event_associated_id = answ->event_associated_id;
+	req->answ.frm.len = answ->lf.len;
+	req->answ.frm.lin_id = answ->lf.lin_id;
+	memcpy(req->answ.frm.data, answ->lf.data, LIN_MAX_DLEN);
+	req->answ.frm.checksum = answ->lf.checksum;
+	req->answ.frm.checksum_mode = answ->lf.checksum_mode;
+
+	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
+	if (ret)
+		hid_err(priv->hid_dev, "set respond answer failed: %d\n", ret);
+
+	kfree(req);
+	return ret;
+}
+
+static int hexlin_open(struct lin_device *ldev)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+
+	return hid_hw_open(hdev);
+}
+
+static int hexlin_stop(struct lin_device *ldev)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	hid_hw_close(hdev);
+
+	priv->is_error = true;
+	complete(&priv->wait_in_report);
+
+	return 0;
+}
+
+static int hexlin_ldo_tx(struct lin_device *ldev,
+			 const struct lin_frame *lf)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	int ret = -EINVAL;
+
+	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
+		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
+		   lf->checksum_mode ? "enhanced" : "classic");
+
+	if (lf->lin_id && lf->len == 0) {
+		ret = hexlin_send_break(priv, lf->lin_id);
+	} else if (lf->len <= LIN_MAX_DLEN) {
+		struct hexlin_frame hxf;
+
+		hxf.len = lf->len;
+		hxf.lin_id = lf->lin_id;
+		memcpy(&hxf.data, lf->data, LIN_MAX_DLEN);
+		hxf.checksum = lf->checksum;
+		hxf.checksum_mode = lf->checksum_mode;
+		ret = hexlin_send_unconditional(priv, &hxf);
+	} else {
+		hid_err(hdev, "unknown format\n");
+	}
+
+	return ret;
+}
+
+static int hexlin_update_bitrate(struct lin_device *ldev, u16 bitrate)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	int ret;
+
+	hid_dbg(hdev, "update bitrate to: %u\n", bitrate);
+
+	ret = hexlin_open(ldev);
+	if (ret)
+		return ret;
+
+	ret = hexlin_set_baudrate(priv, bitrate);
+	if (ret)
+		return ret;
+
+	ret = hexlin_get_baudrate(priv);
+	if (ret)
+		return ret;
+
+	if (priv->baudrate != bitrate) {
+		hid_err(hdev, "update bitrate failed\n");
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int hexlin_get_responder_answer(struct lin_device *ldev, u8 id,
+				       struct lin_responder_answer *answ)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+	struct hexlin_responder_answer ransw;
+	int ret;
+
+	if (answ == NULL)
+		return -EINVAL;
+
+	ret = hexlin_get_responder_answer_id(priv, id, &ransw);
+	if (ret)
+		return ret;
+
+	answ->is_active = ransw.is_active;
+	answ->is_event_frame = ransw.is_event_frame;
+	answ->event_associated_id = ransw.event_associated_id;
+	answ->lf.len = ransw.frm.len;
+	answ->lf.lin_id = ransw.frm.lin_id;
+	memcpy(answ->lf.data, ransw.frm.data, LIN_MAX_DLEN);
+	answ->lf.checksum = ransw.frm.checksum;
+	answ->lf.checksum_mode = ransw.frm.checksum_mode;
+
+	return 0;
+}
+
+static int hexlin_update_resp_answer(struct lin_device *ldev,
+				     const struct lin_responder_answer *answ)
+{
+	struct hid_device *hdev = to_hid_device(ldev->dev);
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	if (answ == NULL)
+		return -EINVAL;
+
+	return hexlin_set_responder_answer_id(priv, answ);
+}
+
+static const struct lin_device_ops hexlin_ldo = {
+	.ldo_open = hexlin_open,
+	.ldo_stop = hexlin_stop,
+	.ldo_tx = hexlin_ldo_tx,
+	.update_bitrate = hexlin_update_bitrate,
+	.get_responder_answer = hexlin_get_responder_answer,
+	.update_responder_answer = hexlin_update_resp_answer,
+};
+
+static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
+				      const struct hexlin_frame *hxf)
+{
+	struct hid_device *hdev = priv->hid_dev;
+	struct lin_frame lf;
+
+	lf.len = hxf->len;
+	lf.lin_id = hxf->lin_id;
+	memcpy(lf.data, hxf->data, LIN_MAX_DLEN);
+	lf.checksum = hxf->checksum;
+	lf.checksum_mode = hxf->checksum_mode;
+
+	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, chk:%02x (%s), flg:%08x\n",
+		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
+		lf.checksum_mode ? "enhanced" : "classic", hxf->flags);
+
+	lin_rx(priv->ldev, &lf);
+
+	return 0;
+}
+
+static int hexlin_raw_event(struct hid_device *hdev,
+			    struct hid_report *report, u8 *data, int sz)
+{
+	struct hexlin_priv_data *priv;
+	struct hexlin_baudrate_req *br;
+	struct hexlin_responder_answer_req *rar;
+	struct hexlin_unconditional_req *hfr;
+	struct hexlin_val8_req *vr;
+
+	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 != HEXLIN_LEN_RETCODE)
+			return -EREMOTEIO;
+		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_FAIL:
+		if (sz != HEXLIN_LEN_RETCODE)
+			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 != sizeof(*vr))
+			return -EREMOTEIO;
+		vr = (struct hexlin_val8_req *) data;
+		priv->fw_version = vr->v;
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_RESPONDER_ANSWER_ID:
+		if (sz != sizeof(*rar))
+			return -EREMOTEIO;
+		rar = (struct hexlin_responder_answer_req *) data;
+		memcpy(&priv->answ, &rar->answ, sizeof(priv->answ));
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_BAUDRATE:
+		if (sz != sizeof(*br))
+			return -EREMOTEIO;
+		br = (struct hexlin_baudrate_req *) data;
+		le16_to_cpus(br->baudrate);
+		priv->baudrate = br->baudrate;
+		complete(&priv->wait_in_report);
+		break;
+	/* following cases not initiated by us, so no complete() */
+	case HEXLIN_FRAME:
+		if (sz != sizeof(*hfr)) {
+			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
+			return -EREMOTEIO;
+		}
+		hfr = (struct hexlin_unconditional_req *) data;
+		le32_to_cpus(hfr->frm.flags);
+		hexlin_queue_frames_insert(priv, &hfr->frm);
+		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) {
+		/* if first reset fails, try one more time */
+		ret = hexlin_reset_dev(priv);
+		if (ret)
+			return ret;
+	}
+
+	ret = hexlin_get_version(priv);
+	if (ret)
+		return ret;
+
+	priv->baudrate = LIN_DEFAULT_BAUDRATE;
+	ret = hexlin_set_baudrate(priv, priv->baudrate);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int hexlin_probe(struct hid_device *hdev,
+			const struct hid_device_id *id)
+{
+	struct hexlin_priv_data *priv;
+	int ret;
+
+	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->hid_dev = hdev;
+	hid_set_drvdata(hdev, priv);
+
+	mutex_init(&priv->tx_lock);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "hid parse failed with %d\n", ret);
+		goto fail_and_free;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (ret) {
+		hid_err(hdev, "hid hw start failed with %d\n", ret);
+		goto fail_and_stop;
+	}
+
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_err(hdev, "hid hw open failed with %d\n", ret);
+		goto fail_and_close;
+	}
+
+	init_completion(&priv->wait_in_report);
+
+	hid_device_io_start(hdev);
+
+	ret = init_hw(priv);
+	if (ret)
+		goto fail_and_close;
+
+	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
+	if (IS_ERR(priv->ldev)) {
+		ret = PTR_ERR(priv->ldev);
+		goto fail_and_close;
+	}
+
+	hid_hw_close(hdev);
+
+	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
+
+	return 0;
+
+fail_and_close:
+	hid_hw_close(hdev);
+fail_and_stop:
+	hid_hw_stop(hdev);
+fail_and_free:
+	mutex_destroy(&priv->tx_lock);
+	return ret;
+}
+
+static void hexlin_remove(struct hid_device *hdev)
+{
+	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
+
+	unregister_lin(priv->ldev);
+	hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id hexlin_table[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(hid, hexlin_table);
+
+static struct hid_driver hexlin_driver = {
+	.name = "hexLIN",
+	.id_table = hexlin_table,
+	.probe = hexlin_probe,
+	.remove = hexlin_remove,
+	.raw_event = hexlin_raw_event,
+};
+
+module_hid_driver(hexlin_driver);
+
+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 61d2a21affa26..c6fe6f99a0e80 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -907,6 +907,7 @@
 
 #define USB_VENDOR_ID_MCS		0x16d0
 #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
+#define USB_DEVICE_ID_MCS_HEXDEV_HEXLIN	0x0648
 
 #define USB_VENDOR_MEGAWORLD		0x07b5
 #define USB_DEVICE_ID_MEGAWORLD_GAMEPAD	0x0312
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index e0bbf0c6345d6..d721110d0889b 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_MCS_HEXDEV)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
+#endif
 #if IS_ENABLED(CONFIG_HID_HOLTEK)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK, USB_DEVICE_ID_HOLTEK_ON_LINE_GRIP) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD) },
-- 
2.39.2

[PATCH v4 03/11] treewide, serdev: add flags argument to receive_buf()

[Christoph Fritz]

For serdev device drivers to be able to detect TTY_BREAK and other flags
in the buffer, pass the flag buffer pointer down to serdev its receive
function and update all drivers using it.

The changes were mostly done using the following Coccinelle
semantic patch:

// <smpl>
@ rule1 @
identifier fn;
identifier opsname;
@@
struct serdev_device_ops opsname = {
	.receive_buf = fn,
};
@@
identifier rule1.fn;
parameter E1, E2, E3;
typedef u8;
@@
  fn(E1, E2,
+ const u8 *flags,
  E3)
  { ... }
// </smpl>

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/bluetooth/btmtkuart.c                          |  3 ++-
 drivers/bluetooth/btnxpuart.c                          |  3 ++-
 drivers/bluetooth/hci_serdev.c                         |  3 ++-
 drivers/gnss/serial.c                                  |  2 +-
 drivers/gnss/sirf.c                                    |  2 +-
 drivers/greybus/gb-beagleplay.c                        |  2 +-
 drivers/iio/chemical/pms7003.c                         |  2 +-
 drivers/iio/chemical/scd30_serial.c                    |  3 ++-
 drivers/iio/chemical/sps30_serial.c                    |  3 ++-
 drivers/iio/imu/bno055/bno055_ser_core.c               |  3 ++-
 drivers/mfd/rave-sp.c                                  |  2 +-
 drivers/net/ethernet/qualcomm/qca_uart.c               |  3 ++-
 drivers/nfc/pn533/uart.c                               |  2 +-
 drivers/nfc/s3fwrn5/uart.c                             |  3 ++-
 drivers/platform/chrome/cros_ec_uart.c                 |  3 ++-
 drivers/platform/surface/aggregator/core.c             |  2 +-
 .../x86/lenovo-yoga-tab2-pro-1380-fastcharger.c        |  3 ++-
 drivers/tty/serdev/serdev-ttyport.c                    |  2 +-
 drivers/w1/masters/w1-uart.c                           |  3 ++-
 include/linux/serdev.h                                 | 10 +++++++---
 sound/drivers/serial-generic.c                         |  3 ++-
 21 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c
index e6bc4a73c9fc3..acb343931f83c 100644
--- a/drivers/bluetooth/btmtkuart.c
+++ b/drivers/bluetooth/btmtkuart.c
@@ -384,7 +384,8 @@ static void btmtkuart_recv(struct hci_dev *hdev, const u8 *data, size_t count)
 }
 
 static size_t btmtkuart_receive_buf(struct serdev_device *serdev,
-				    const u8 *data, size_t count)
+				    const u8 *data, const u8 *flags,
+				    size_t count)
 {
 	struct btmtkuart_dev *bdev = serdev_device_get_drvdata(serdev);
 
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 9d0c7e278114b..a14afc4ecdda5 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1286,7 +1286,8 @@ static const struct h4_recv_pkt nxp_recv_pkts[] = {
 };
 
 static size_t btnxpuart_receive_buf(struct serdev_device *serdev,
-				    const u8 *data, size_t count)
+				    const u8 *data, const u8 *flags,
+				    size_t count)
 {
 	struct btnxpuart_dev *nxpdev = serdev_device_get_drvdata(serdev);
 
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 89a22e9b3253a..9c3e8f5c136bf 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -272,7 +272,8 @@ static void hci_uart_write_wakeup(struct serdev_device *serdev)
  * Return: number of processed bytes
  */
 static size_t hci_uart_receive_buf(struct serdev_device *serdev,
-				   const u8 *data, size_t count)
+				   const u8 *data, const u8 *flags,
+				   size_t count)
 {
 	struct hci_uart *hu = serdev_device_get_drvdata(serdev);
 
diff --git a/drivers/gnss/serial.c b/drivers/gnss/serial.c
index 0e43bf6294f87..4af4d2c0624b3 100644
--- a/drivers/gnss/serial.c
+++ b/drivers/gnss/serial.c
@@ -81,7 +81,7 @@ static const struct gnss_operations gnss_serial_gnss_ops = {
 };
 
 static size_t gnss_serial_receive_buf(struct serdev_device *serdev,
-				       const u8 *buf, size_t count)
+		const u8 *buf, const u8 *flags, size_t count)
 {
 	struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
 	struct gnss_device *gdev = gserial->gdev;
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
index 79375d14bbb67..410c7bc6af43c 100644
--- a/drivers/gnss/sirf.c
+++ b/drivers/gnss/sirf.c
@@ -161,7 +161,7 @@ static const struct gnss_operations sirf_gnss_ops = {
 };
 
 static size_t sirf_receive_buf(struct serdev_device *serdev,
-				const u8 *buf, size_t count)
+				const u8 *buf, const u8 *flags, size_t count)
 {
 	struct sirf_data *data = serdev_device_get_drvdata(serdev);
 	struct gnss_device *gdev = data->gdev;
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c
index 33f8fad70260a..b56675a7b6b06 100644
--- a/drivers/greybus/gb-beagleplay.c
+++ b/drivers/greybus/gb-beagleplay.c
@@ -332,7 +332,7 @@ static void hdlc_deinit(struct gb_beagleplay *bg)
 }
 
 static size_t gb_tty_receive(struct serdev_device *sd, const u8 *data,
-			     size_t count)
+			     const u8 *flags, size_t count)
 {
 	struct gb_beagleplay *bg = serdev_device_get_drvdata(sd);
 
diff --git a/drivers/iio/chemical/pms7003.c b/drivers/iio/chemical/pms7003.c
index 43025866d5b79..4d7ed0dbd4ac4 100644
--- a/drivers/iio/chemical/pms7003.c
+++ b/drivers/iio/chemical/pms7003.c
@@ -212,7 +212,7 @@ static bool pms7003_frame_is_okay(struct pms7003_frame *frame)
 }
 
 static size_t pms7003_receive_buf(struct serdev_device *serdev, const u8 *buf,
-				  size_t size)
+				  const u8 *flags, size_t size)
 {
 	struct iio_dev *indio_dev = serdev_device_get_drvdata(serdev);
 	struct pms7003_state *state = iio_priv(indio_dev);
diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c
index 2adb76dbb0209..c67524a394378 100644
--- a/drivers/iio/chemical/scd30_serial.c
+++ b/drivers/iio/chemical/scd30_serial.c
@@ -175,7 +175,8 @@ static int scd30_serdev_command(struct scd30_state *state, enum scd30_cmd cmd, u
 }
 
 static size_t scd30_serdev_receive_buf(struct serdev_device *serdev,
-				       const u8 *buf, size_t size)
+				       const u8 *buf, const u8 *flags,
+				       size_t size)
 {
 	struct iio_dev *indio_dev = serdev_device_get_drvdata(serdev);
 	struct scd30_serdev_priv *priv;
diff --git a/drivers/iio/chemical/sps30_serial.c b/drivers/iio/chemical/sps30_serial.c
index a6dfbe28c914c..5e486e23110dc 100644
--- a/drivers/iio/chemical/sps30_serial.c
+++ b/drivers/iio/chemical/sps30_serial.c
@@ -211,7 +211,8 @@ static int sps30_serial_command(struct sps30_state *state, unsigned char cmd,
 }
 
 static size_t sps30_serial_receive_buf(struct serdev_device *serdev,
-				       const u8 *buf, size_t size)
+				       const u8 *buf, const u8 *flags,
+				       size_t size)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
 	struct sps30_serial_priv *priv;
diff --git a/drivers/iio/imu/bno055/bno055_ser_core.c b/drivers/iio/imu/bno055/bno055_ser_core.c
index 694ff14a3aa27..68729e9e98796 100644
--- a/drivers/iio/imu/bno055/bno055_ser_core.c
+++ b/drivers/iio/imu/bno055/bno055_ser_core.c
@@ -379,7 +379,8 @@ static void bno055_ser_handle_rx(struct bno055_ser_priv *priv, int status)
  * unless we require to AND we don't queue more than one request per time).
  */
 static size_t bno055_ser_receive_buf(struct serdev_device *serdev,
-				     const u8 *buf, size_t size)
+				     const u8 *buf, const u8 *flags,
+				     size_t size)
 {
 	int status;
 	struct bno055_ser_priv *priv = serdev_device_get_drvdata(serdev);
diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
index ef326d6d566e6..5229b251d698f 100644
--- a/drivers/mfd/rave-sp.c
+++ b/drivers/mfd/rave-sp.c
@@ -472,7 +472,7 @@ static void rave_sp_receive_frame(struct rave_sp *sp,
 }
 
 static size_t rave_sp_receive_buf(struct serdev_device *serdev,
-				  const u8 *buf, size_t size)
+				  const u8 *buf, const u8 *flags, size_t size)
 {
 	struct device *dev = &serdev->dev;
 	struct rave_sp *sp = dev_get_drvdata(dev);
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
index 37efb1ea9fcd9..a59d88a8f9d91 100644
--- a/drivers/net/ethernet/qualcomm/qca_uart.c
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -46,7 +46,8 @@ struct qcauart {
 };
 
 static size_t
-qca_tty_receive(struct serdev_device *serdev, const u8 *data, size_t count)
+qca_tty_receive(struct serdev_device *serdev, const u8 *data, const u8 *flags,
+		size_t count)
 {
 	struct qcauart *qca = serdev_device_get_drvdata(serdev);
 	struct net_device *netdev = qca->net_dev;
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
index cfbbe0713317f..9686e354c0d56 100644
--- a/drivers/nfc/pn533/uart.c
+++ b/drivers/nfc/pn533/uart.c
@@ -204,7 +204,7 @@ static int pn532_uart_rx_is_frame(struct sk_buff *skb)
 }
 
 static size_t pn532_receive_buf(struct serdev_device *serdev,
-				const u8 *data, size_t count)
+				const u8 *data, const u8 *flags, size_t count)
 {
 	struct pn532_uart_phy *dev = serdev_device_get_drvdata(serdev);
 	size_t i;
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 9c09c10c2a464..b55734ad1b0f3 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -52,7 +52,8 @@ static const struct s3fwrn5_phy_ops uart_phy_ops = {
 };
 
 static size_t s3fwrn82_uart_read(struct serdev_device *serdev,
-				 const u8 *data, size_t count)
+				 const u8 *data, const u8 *flags,
+				 size_t count)
 {
 	struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
 	size_t i;
diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
index 62bc24f6dcc7a..349b0638420e6 100644
--- a/drivers/platform/chrome/cros_ec_uart.c
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -82,7 +82,8 @@ struct cros_ec_uart {
 };
 
 static size_t cros_ec_uart_rx_bytes(struct serdev_device *serdev,
-				    const u8 *data, size_t count)
+				    const u8 *data, const u8 *flags,
+				    size_t count)
 {
 	struct ec_host_response *host_response;
 	struct cros_ec_device *ec_dev = serdev_device_get_drvdata(serdev);
diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c
index ba550eaa06fcf..4efd2a8226789 100644
--- a/drivers/platform/surface/aggregator/core.c
+++ b/drivers/platform/surface/aggregator/core.c
@@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(ssam_client_bind);
 /* -- Glue layer (serdev_device -> ssam_controller). ------------------------ */
 
 static size_t ssam_receive_buf(struct serdev_device *dev, const u8 *buf,
-			       size_t n)
+			       const u8 *flags, size_t n)
 {
 	struct ssam_controller *ctrl;
 	int ret;
diff --git a/drivers/platform/x86/lenovo-yoga-tab2-pro-1380-fastcharger.c b/drivers/platform/x86/lenovo-yoga-tab2-pro-1380-fastcharger.c
index d525bdc8ca9b3..c20c1be27227f 100644
--- a/drivers/platform/x86/lenovo-yoga-tab2-pro-1380-fastcharger.c
+++ b/drivers/platform/x86/lenovo-yoga-tab2-pro-1380-fastcharger.c
@@ -133,7 +133,8 @@ static int yt2_1380_fc_extcon_evt(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static size_t yt2_1380_fc_receive(struct serdev_device *serdev, const u8 *data, size_t len)
+static size_t yt2_1380_fc_receive(struct serdev_device *serdev, const u8 *data,
+				  const u8 *flags, size_t len)
 {
 	/*
 	 * Since the USB data lines are shorted for DCP detection, echos of
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/drivers/w1/masters/w1-uart.c b/drivers/w1/masters/w1-uart.c
index a31782e56ba75..ed465b6f491fc 100644
--- a/drivers/w1/masters/w1-uart.c
+++ b/drivers/w1/masters/w1-uart.c
@@ -290,7 +290,8 @@ static int w1_uart_serdev_tx_rx(struct w1_uart_device *w1dev,
 }
 
 static size_t w1_uart_serdev_receive_buf(struct serdev_device *serdev,
-					  const u8 *buf, size_t count)
+					 const u8 *buf, const u8 *flags,
+					 size_t count)
 {
 	struct w1_uart_device *w1dev = serdev_device_get_drvdata(serdev);
 
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index ff78efc1f60df..94fc81a1de933 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -21,13 +21,16 @@ struct serdev_device;
 
 /**
  * struct serdev_device_ops - Callback operations for a serdev device
- * @receive_buf:	Function called with data received from device;
+ * @receive_buf:	Function called with data received from device (char
+ *			buffer), flags buffer (%TTY_NORMAL, %TTY_BREAK, etc)
+ *			and number of bytes;
  *			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);
+	size_t (*receive_buf)(struct serdev_device *, const u8 *, const u8 *,
+			      size_t);
 	void (*write_wakeup)(struct serdev_device *);
 };
 
@@ -187,6 +190,7 @@ 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 *flags,
 						   size_t count)
 {
 	struct serdev_device *serdev = ctrl->serdev;
@@ -194,7 +198,7 @@ static inline size_t serdev_controller_receive_buf(struct serdev_controller *ctr
 	if (!serdev || !serdev->ops->receive_buf)
 		return 0;
 
-	return serdev->ops->receive_buf(serdev, data, count);
+	return serdev->ops->receive_buf(serdev, data, flags, count);
 }
 
 #if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
diff --git a/sound/drivers/serial-generic.c b/sound/drivers/serial-generic.c
index 36409a56c675e..3569ef40d35a2 100644
--- a/sound/drivers/serial-generic.c
+++ b/sound/drivers/serial-generic.c
@@ -101,7 +101,8 @@ static void snd_serial_generic_write_wakeup(struct serdev_device *serdev)
 }
 
 static size_t snd_serial_generic_receive_buf(struct serdev_device *serdev,
-					     const u8 *buf, size_t count)
+					     const u8 *buf, const u8 *flags,
+					     size_t count)
 {
 	int ret;
 	struct snd_serial_generic *drvdata = serdev_device_get_drvdata(serdev);
-- 
2.39.2

[PATCH v4 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 94fc81a1de933..84805762a67cc 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -91,6 +91,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);
@@ -208,6 +209,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 v4 05/11] dt-bindings: vendor-prefixes: Add hexDEV

[Christoph Fritz]

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

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

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

[PATCH v4 06/11] dt-bindings: net/can: Add serial LIN adapter hexLINSER

[Christoph Fritz]

Add dt-bindings for hexDEV hexLINSER serial LIN adapters. These adapters
are basically just LIN transceivers that are mostly hard-wired to serial
devices.

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

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

[PATCH v4 07/11] can: Add support for hexDEV serial LIN adapter hexLINSER

[Christoph Fritz]

Introduce support for the hexDEV serial LIN adapter hexLINSER. These
devices are equipped with LIN transceivers and are mostly hard-wired to
serial devices.

This device driver uses CAN_LIN on one side and the serial device bus
(serdev) interface on the other.

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

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 drivers/net/can/Kconfig      |  15 ++
 drivers/net/can/Makefile     |   1 +
 drivers/net/can/hex-linser.c | 505 +++++++++++++++++++++++++++++++++++
 3 files changed, 521 insertions(+)
 create mode 100644 drivers/net/can/hex-linser.c

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 0934bbf8d03b2..141972d6bbf1e 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -181,6 +181,21 @@ config CAN_LIN
 
 	  Actual device drivers need to be enabled too.
 
+config CAN_LIN_HEXLINSER
+	tristate "hexDEV hexLINSER serial LIN Adaptors"
+	depends on CAN_LIN && SERIAL_DEV_BUS && OF
+	help
+	  LIN support for serial devices equipped with LIN transceivers.
+	  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 hexLINSER tty adapter, say Y here and see
+	  <https://hexdev.de/hexlin#hexLINSER>.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called hex-linser.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..9fdad4a0fd12a 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_HEXLINSER)	+= hex-linser.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/hex-linser.c b/drivers/net/can/hex-linser.c
new file mode 100644
index 0000000000000..9c2d11d2ed0c0
--- /dev/null
+++ b/drivers/net/can/hex-linser.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <net/lin.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;
+	unsigned long break_usleep_min;
+	unsigned long break_usleep_max;
+	unsigned long post_break_usleep_min;
+	unsigned long post_break_usleep_max;
+	unsigned long force_timeout_jfs;
+	struct lin_responder_answer respond_answ[LIN_NUM_IDS];
+	struct mutex resp_lock; /* protects respond_answ */
+	bool is_stopped;
+};
+
+static int linser_open(struct lin_device *ldev)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	int ret;
+
+	if (priv->is_stopped) {
+		ret = serdev_device_open(serdev);
+		if (ret) {
+			dev_err(&serdev->dev, "Unable to open device\n");
+			return ret;
+		}
+
+		serdev_device_set_flow_control(serdev, false);
+		serdev_device_set_break_detection(serdev, true);
+
+		priv->is_stopped = false;
+	}
+
+	return 0;
+}
+
+static int linser_stop(struct lin_device *ldev)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+
+	if (priv->is_stopped)
+		return 0;
+
+	serdev_device_close(serdev);
+	priv->is_stopped = true;
+
+	return 0;
+}
+
+static int linser_send_break(struct linser_priv *priv)
+{
+	struct serdev_device *serdev = priv->serdev;
+	int ret;
+
+	ret = serdev_device_break_ctl(serdev, -1);
+	if (ret)
+		return ret;
+
+	usleep_range(priv->break_usleep_min, priv->break_usleep_max);
+
+	ret = serdev_device_break_ctl(serdev, 0);
+	if (ret)
+		return ret;
+
+	usleep_range(priv->post_break_usleep_min, priv->post_break_usleep_max);
+
+	return 0;
+}
+
+static int linser_ldo_tx(struct lin_device *ldev, const struct lin_frame *lf)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	u8 pid = LIN_FORM_PID(lf->lin_id);
+	u8 buf[LINSER_TX_BUFFER_SIZE];
+	ssize_t written_len, total_len;
+	u8 checksum;
+	int ret;
+
+	if (lf->len + 3 > LINSER_TX_BUFFER_SIZE) {
+		dev_err(&serdev->dev, "Frame length %u exceeds buffer size\n", lf->len);
+		return -EINVAL;
+	}
+
+	buf[0] = LIN_SYNC_BYTE;
+	buf[1] = pid;
+	total_len = 2;
+
+	if (lf->len) {
+		memcpy(&buf[2], lf->data, lf->len);
+		checksum = lin_get_checksum(pid, lf->len, lf->data,
+					    lf->checksum_mode);
+		buf[lf->len + 2] = checksum;
+		total_len += lf->len + 1;
+	}
+
+	ret = linser_send_break(priv);
+	if (ret)
+		return ret;
+
+	written_len = serdev_device_write(serdev, buf, total_len, 0);
+	if (written_len < total_len)
+		return written_len < 0 ? (int)written_len : -EIO;
+
+	dev_dbg(&serdev->dev, "sent out: %*ph\n", (int)total_len, buf);
+
+	serdev_device_wait_until_sent(serdev, 0);
+
+	return 0;
+}
+
+static int linser_derive_timings(struct linser_priv *priv, u16 bitrate)
+{
+	unsigned long break_baud = (bitrate * 2) / 3;
+	struct serdev_device *serdev = priv->serdev;
+	unsigned long timeout_us;
+
+	if (bitrate < LIN_MIN_BAUDRATE || bitrate > LIN_MAX_BAUDRATE) {
+		dev_err(&serdev->dev, "Bitrate %u out of bounds (%u to %u)\n",
+			bitrate, LIN_MIN_BAUDRATE, LIN_MAX_BAUDRATE);
+		return -EINVAL;
+	}
+
+	priv->break_usleep_min = (USEC_PER_SEC * LINSER_SAMPLES_PER_CHAR) /
+				 break_baud;
+	priv->break_usleep_max = priv->break_usleep_min + 50;
+	priv->post_break_usleep_min = USEC_PER_SEC / break_baud;
+	priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
+
+	timeout_us = DIV_ROUND_CLOSEST(USEC_PER_SEC * 256, bitrate);
+	priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
+
+	return 0;
+}
+
+static int linser_update_bitrate(struct lin_device *ldev, u16 bitrate)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	unsigned int speed;
+	int ret;
+
+	ret = linser_open(ldev);
+	if (ret)
+		return ret;
+
+	speed = serdev_device_set_baudrate(serdev, bitrate);
+	if (!bitrate || speed != bitrate)
+		return -EINVAL;
+
+	ret = linser_derive_timings(priv, bitrate);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int linser_get_responder_answer(struct lin_device *ldev, u8 id,
+				       struct lin_responder_answer *answ)
+{
+	struct serdev_device *serdev = to_serdev_device(ldev->dev);
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	struct lin_responder_answer *r = &priv->respond_answ[id];
+
+	if (!answ)
+		return -EINVAL;
+
+	guard(mutex)(&priv->resp_lock);
+	memcpy(answ, r, sizeof(*answ));
+
+	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(*answ));
+	r->lf.checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
+					  answ->lf.len,
+					  answ->lf.data,
+					  answ->lf.checksum_mode);
+	mutex_unlock(&priv->resp_lock);
+
+	return 0;
+}
+
+static struct lin_device_ops linser_lindev_ops = {
+	.ldo_open = linser_open,
+	.ldo_stop = linser_stop,
+	.ldo_tx = linser_ldo_tx,
+	.update_bitrate = linser_update_bitrate,
+	.get_responder_answer = linser_get_responder_answer,
+	.update_responder_answer = linser_update_resp_answer,
+};
+
+static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
+{
+	struct lin_responder_answer *answ = &priv->respond_answ[id];
+	struct serdev_device *serdev = priv->serdev;
+	u8 buf[LINSER_TX_BUFFER_SIZE];
+	u8 checksum, count, n;
+	ssize_t write_len;
+
+	scoped_guard(mutex, &priv->resp_lock) {
+		if (!answ->is_active)
+			return 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)
+				return false;
+
+			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 {
+			checksum = answ->lf.checksum;
+		}
+
+		count = min(answ->lf.len, LIN_MAX_DLEN);
+		memcpy(&buf[0], answ->lf.data, count);
+		buf[count] = checksum;
+	}
+
+	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;
+}
+
+static void linser_pop_fifo(struct linser_priv *priv, size_t n)
+{
+	for (size_t i = 0; i < n; i++)
+		kfifo_skip(&priv->rx_fifo);
+}
+
+static int linser_fill_frame(struct linser_priv *priv, struct lin_frame *lf)
+{
+	struct serdev_device *serdev = priv->serdev;
+	struct linser_rx buf[LINSER_PARSE_BUFFER];
+	unsigned int count, i, brk = 0;
+
+	count = kfifo_out_peek(&priv->rx_fifo, buf, LINSER_PARSE_BUFFER);
+
+	memset(lf, 0, sizeof(*lf));
+
+	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)))
+			return ret;
+
+		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;
+	}
+
+	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 size_t linser_receive_buf(struct serdev_device *serdev, const u8 *data,
+				 const u8 *flags, size_t count)
+{
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+	enum linser_rx_status rx_status;
+	size_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 = (flags ? flags[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 = linser_receive_buf,
+	.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);
+	ret = linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
+	if (ret)
+		goto err_register_lin;
+
+	mutex_init(&priv->resp_lock);
+
+	priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
+	if (IS_ERR(priv->lin_dev)) {
+		ret = PTR_ERR(priv->lin_dev);
+		goto err_register_lin;
+	}
+
+	serdev_device_close(serdev);
+	priv->is_stopped = true;
+
+	return 0;
+
+err_register_lin:
+	serdev_device_close(serdev);
+err_open:
+	kfifo_free(&priv->rx_fifo);
+	return ret;
+}
+
+static void linser_remove(struct serdev_device *serdev)
+{
+	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
+
+	unregister_lin(priv->lin_dev);
+}
+
+static const struct of_device_id linser_of_match[] = {
+	{
+		.compatible = "hexdev,hex-linser",
+	},
+	{}
+};
+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 v4 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                | 72 ++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h
index f1e45f533a72c..d27bc79f924c1 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     BIT(12)
 
 #endif /* !_UAPI_CAN_BCM_H */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 27d5fcf0eac9d..6dc8b9877db4c 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_any());
+	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
  */
@@ -1435,6 +1489,24 @@ static int bcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			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 v4 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.

Introduce functionality to interpret these specific frames, enabling the
configuration of RX offloading within the LIN driver.

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

diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
index a22768c17e3f8..f77abd7d7d21c 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -185,6 +185,27 @@ static const struct attribute_group lin_sysfs_group = {
 	.attrs = lin_sysfs_attrs,
 };
 
+static int lin_setup_rxoffload(struct lin_device *ldev,
+			       struct canfd_frame *cfd)
+{
+	struct lin_responder_answer answ;
+
+	if (!(cfd->flags & CANFD_FDF))
+		return -EINVAL;
+
+	BUILD_BUG_ON(sizeof(answ) > sizeof(cfd->data));
+	memcpy(&answ, cfd->data, sizeof(answ));
+
+	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,
@@ -197,6 +218,14 @@ static void lin_tx_work_handler(struct work_struct *ws)
 	ldev->tx_busy = true;
 
 	cfd = (struct canfd_frame *)ldev->tx_skb->data;
+
+	if (cfd->can_id & LIN_RXOFFLOAD_DATA_FLAG) {
+		ret = lin_setup_rxoffload(ldev, cfd);
+		if (ret < 0)
+			netdev_err(ndev, "setting up rx failed %d\n", ret);
+		goto lin_tx_out;
+	}
+
 	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
 			   LINBUS_ENHANCED : LINBUS_CLASSIC;
 	lf.lin_id = cfd->can_id & LIN_ID_MASK;
-- 
2.39.2

[PATCH v4 10/11] can: lin: Support setting LIN mode

[Christoph Fritz]

A LIN node can work as commander or responder, so introduce a new
control mode (CAN_CTRLMODE_LIN_COMMANDER) for configuration.

This enables e.g. the userland tool ip from iproute2 to 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 f77abd7d7d21c..03ddf5d5a31b8 100644
--- a/drivers/net/can/lin.c
+++ b/drivers/net/can/lin.c
@@ -262,11 +262,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);
@@ -443,7 +472,7 @@ struct lin_device *register_lin(struct device *dev,
 	ndev->sysfs_groups[0] = &lin_sysfs_group;
 	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;
@@ -458,6 +487,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 31bb0feefd188..63ac870a0ab6f 100644
--- a/include/net/lin.h
+++ b/include/net/lin.h
@@ -36,6 +36,11 @@ struct lin_attr {
 	struct lin_device *ldev;
 };
 
+enum lin_mode {
+	LINBUS_RESPONDER = 0,
+	LINBUS_COMMANDER,
+};
+
 struct lin_device {
 	struct can_priv can;  /* must be the first member */
 	struct net_device *ndev;
@@ -45,6 +50,7 @@ struct lin_device {
 	struct work_struct tx_work;
 	bool tx_busy;
 	struct sk_buff *tx_skb;
+	enum lin_mode lmode;
 };
 
 enum lin_checksum_mode {
@@ -71,6 +77,7 @@ struct lin_device_ops {
 	int (*ldo_open)(struct lin_device *ldev);
 	int (*ldo_stop)(struct lin_device *ldev);
 	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
+	int (*update_lin_mode)(struct lin_device *ldev, enum lin_mode lm);
 	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
 	int (*update_responder_answer)(struct lin_device *ldev,
 				       const struct lin_responder_answer *answ);
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index a37f56d86c5f2..cc390f6444d59 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		BIT(11)	/* LIN bus mode */
+#define CAN_CTRLMODE_LIN_COMMANDER	BIT(12)	/* LIN bus specific commander mode */
 
 /*
  * CAN device statistics
-- 
2.39.2

[PATCH v4 11/11] HID: hexLIN: Implement ability to update lin mode

[Christoph Fritz]

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

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

diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
index a9ed080b3e33e..48fcb1e5b6e41 100644
--- a/drivers/hid/hid-hexdev-hexlin.c
+++ b/drivers/hid/hid-hexdev-hexlin.c
@@ -164,6 +164,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_send_unconditional(struct hexlin_priv_data *priv,
 				     const struct hexlin_frame *hxf)
@@ -322,6 +324,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);
@@ -393,6 +403,7 @@ static const struct lin_device_ops hexlin_ldo = {
 	.ldo_open = hexlin_open,
 	.ldo_stop = hexlin_stop,
 	.ldo_tx = hexlin_ldo_tx,
+	.update_lin_mode = hexlin_update_lin_mode,
 	.update_bitrate = hexlin_update_bitrate,
 	.get_responder_answer = hexlin_get_responder_answer,
 	.update_responder_answer = hexlin_update_resp_answer,
-- 
2.39.2

Re: [PATCH v4 06/11] dt-bindings: net/can: Add serial LIN adapter hexLINSER

[Conor Dooley]

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

On Thu, May 09, 2024 at 07:17:31PM +0200, Christoph Fritz wrote:
> Add dt-bindings for hexDEV hexLINSER serial LIN adapters. These adapters
> are basically just LIN transceivers that are mostly hard-wired to serial
> devices.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/hexdev,hex-linser.yaml   | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,hex-linser.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/can/hexdev,hex-linser.yaml b/Documentation/devicetree/bindings/net/can/hexdev,hex-linser.yaml
> new file mode 100644
> index 0000000000000..42dce3348f73c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/hexdev,hex-linser.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/hexdev,hex-linser.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: hexDEV hexLINSER serial LIN adapter
> +
> +description:
> +  LIN transceiver, mostly hard-wired to a serial device, used for communication
> +  on a LIN bus.
> +  For more details on the adapter, visit <https://hexdev.de/hexlin#hexLINSER>.

I figured I should check in firefox this time, link works fine :)

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

> Introduce a LIN (local interconnect network) abstraction on top of CAN.
> This 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            | 470 +++++++++++++++++++++++++++++++
>  include/net/lin.h                |  90 ++++++
>  include/uapi/linux/can/netlink.h |   1 +
>  5 files changed, 572 insertions(+)
>  create mode 100644 drivers/net/can/lin.c
>  create mode 100644 include/net/lin.h
> 
> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
> new file mode 100644
> index 0000000000000..a22768c17e3f8
> --- /dev/null
> +++ b/drivers/net/can/lin.c
> @@ -0,0 +1,470 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/can/core.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/netdevice.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <net/lin.h>
> +
> +static const u8 lin_id_parity_tbl[] = {
> +	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
> +	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
> +	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
> +	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
> +	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
> +	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
> +	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
> +	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
> +};
> +
> +u8 lin_get_id_parity(u8 id)
> +{
> +	return lin_id_parity_tbl[id];
> +}
> +EXPORT_SYMBOL(lin_get_id_parity);
> +
> +static ssize_t lin_identifier_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct lin_device *ldev = netdev_priv(to_net_dev(dev));
> +	struct lin_responder_answer answ;
> +	int k, count, 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)

How can this happen?

Perhaps you can determine the id arithmetically from a pointer within an 
array?

> +		return -EINVAL;
> +
> +	count = sysfs_emit(buf, "%-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 += sysfs_emit_at(buf, 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 += sysfs_emit_at(buf, count, "%02x ", answ.lf.data[k]);
> +	for (; k < 8; k++)
> +		count += sysfs_emit_at(buf, count, "   ");
> +	if (answ.lf.len)
> +		count += sysfs_emit_at(buf, count, " %02x", answ.lf.checksum);
> +
> +	count += sysfs_emit_at(buf, count, "\n");
> +
> +	return count;
> +}
> +
> +static const char *parse_and_advance(const char *buf, long *result,
> +				     unsigned int base)
> +{
> +	char num_str[5] = {0};
> +	int num_len = 0;
> +
> +	while (*buf && isspace(*buf))

Missing #include for isspace().

> +		buf++;
> +	while (*buf && isalnum(*buf) && num_len < sizeof(num_str) - 1)
> +		num_str[num_len++] = *buf++;

This is likely broken when the number is too long because the next 
number is not correctly parsed? I think you should bail out with an error 
when a number is invalid (too long).

> +	if (kstrtol(num_str, base, result))

Missing #include for kstrtol() but see b.

Do you care for signed numbers? If not use unsigned kstrtox variant and 
typing.

> +		return NULL;
> +
> +	return buf;
> +}
> +
> +static ssize_t lin_identifier_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	struct lin_device *ldev = netdev_priv(to_net_dev(dev));
> +	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;

Can this happen?

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

When adding sysfs entries, they should be documented under 
Documentation/ABI/

> +#define LID(_name) \
> +	struct device_attribute linid_##_name = __ATTR(_name, 0644, \
> +	lin_identifier_show, lin_identifier_store)
> +
> +LID(00); LID(01); LID(02); LID(03); LID(04); LID(05); LID(06); LID(07);
> +LID(08); LID(09); LID(0a); LID(0b); LID(0c); LID(0d); LID(0e); LID(0f);
> +LID(10); LID(11); LID(12); LID(13); LID(14); LID(15); LID(16); LID(17);
> +LID(18); LID(19); LID(1a); LID(1b); LID(1c); LID(1d); LID(1e); LID(1f);
> +LID(20); LID(21); LID(22); LID(23); LID(24); LID(25); LID(26); LID(27);
> +LID(28); LID(29); LID(2a); LID(2b); LID(2c); LID(2d); LID(2e); LID(2f);
> +LID(30); LID(31); LID(32); LID(33); LID(34); LID(35); LID(36); LID(37);
> +LID(38); LID(39); LID(3a); LID(3b); LID(3c); LID(3d); LID(3e); LID(3f);
> +
> +static struct attribute *lin_sysfs_attrs[] = {
> +	&linid_00.attr, &linid_01.attr, &linid_02.attr, &linid_03.attr,
> +	&linid_04.attr, &linid_05.attr, &linid_06.attr, &linid_07.attr,
> +	&linid_08.attr, &linid_09.attr, &linid_0a.attr, &linid_0b.attr,
> +	&linid_0c.attr, &linid_0d.attr, &linid_0e.attr, &linid_0f.attr,
> +	&linid_10.attr, &linid_11.attr, &linid_12.attr, &linid_13.attr,
> +	&linid_14.attr, &linid_15.attr, &linid_16.attr, &linid_17.attr,
> +	&linid_18.attr, &linid_19.attr, &linid_1a.attr, &linid_1b.attr,
> +	&linid_1c.attr, &linid_1d.attr, &linid_1e.attr, &linid_1f.attr,
> +	&linid_20.attr, &linid_21.attr, &linid_22.attr, &linid_23.attr,
> +	&linid_24.attr, &linid_25.attr, &linid_26.attr, &linid_27.attr,
> +	&linid_28.attr, &linid_29.attr, &linid_2a.attr, &linid_2b.attr,
> +	&linid_2c.attr, &linid_2d.attr, &linid_2e.attr, &linid_2f.attr,
> +	&linid_30.attr, &linid_31.attr, &linid_32.attr, &linid_33.attr,
> +	&linid_34.attr, &linid_35.attr, &linid_36.attr, &linid_37.attr,
> +	&linid_38.attr, &linid_39.attr, &linid_3a.attr, &linid_3b.attr,
> +	&linid_3c.attr, &linid_3d.attr, &linid_3e.attr, &linid_3f.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group lin_sysfs_group = {
> +	.name = "lin_ids",
> +	.attrs = lin_sysfs_attrs,
> +};
> +
> +static void lin_tx_work_handler(struct work_struct *ws)
> +{
> +	struct lin_device *ldev = container_of(ws, struct lin_device,
> +					       tx_work);
> +	struct net_device *ndev = ldev->ndev;
> +	struct canfd_frame *cfd;
> +	struct lin_frame lf;
> +	int ret;
> +
> +	ldev->tx_busy = true;
> +
> +	cfd = (struct canfd_frame *)ldev->tx_skb->data;
> +	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
> +			   LINBUS_ENHANCED : LINBUS_CLASSIC;
> +	lf.lin_id = cfd->can_id & LIN_ID_MASK;
> +	lf.len = min(cfd->len, LIN_MAX_DLEN);

I wonder if it is the correct approach to continue in this case? Something 
is getting truncated here, right?

> +	memcpy(lf.data, cfd->data, lf.len);
> +
> +	ret = ldev->ldev_ops->ldo_tx(ldev, &lf);
> +	if (ret) {
> +		DEV_STATS_INC(ndev, tx_dropped);
> +		netdev_err_once(ndev, "transmission failure %d\n", ret);
> +		goto lin_tx_out;
> +	}
> +
> +	DEV_STATS_INC(ndev, tx_packets);
> +	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
> +
> +lin_tx_out:
> +	ldev->tx_busy = false;
> +	netif_wake_queue(ndev);
> +}
> +
> +static netdev_tx_t lin_start_xmit(struct sk_buff *skb,
> +				  struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +
> +	if (ldev->tx_busy)

I'm skeptical this could be correct without any concurrency control 
related primitives.

> +		return NETDEV_TX_BUSY;
> +
> +	netif_stop_queue(ndev);
> +	ldev->tx_skb = skb;
> +	queue_work(ldev->wq, &ldev->tx_work);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static int lin_open(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	int ret;
> +
> +	ldev->tx_busy = false;
> +
> +	ret = open_candev(ndev);
> +	if (ret)
> +		return ret;
> +
> +	netif_wake_queue(ndev);
> +
> +	ldev->can.state = CAN_STATE_ERROR_ACTIVE;
> +	ndev->mtu = CANFD_MTU;
> +
> +	return ldev->ldev_ops->ldo_open(ldev);
> +}
> +
> +static int lin_stop(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +
> +	close_candev(ndev);
> +
> +	flush_work(&ldev->tx_work);
> +
> +	ldev->can.state = CAN_STATE_STOPPED;
> +
> +	return ldev->ldev_ops->ldo_stop(ldev);
> +}
> +
> +static const struct net_device_ops lin_netdev_ops = {
> +	.ndo_open = lin_open,
> +	.ndo_stop = lin_stop,
> +	.ndo_start_xmit = lin_start_xmit,
> +	.ndo_change_mtu = can_change_mtu,
> +};
> +
> +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> +		    enum lin_checksum_mode cm)
> +{
> +	unsigned int csum = 0;
> +	int i;

Use unsigned int for loop variables that are counting positive things.

> +	if (cm == LINBUS_ENHANCED)
> +		csum += pid;
> +
> +	for (i = 0; i < n_of_bytes; i++) {
> +		csum += bytes[i];
> +		if (csum > 255)
> +			csum -= 255;
> +	}
> +
> +	return (~csum & 0xff);

Unnecessary parenthesis.

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

Extra space.

I think ... = {} is enough if you want to just to fully initialize it.

> +
> +	if (lf->lin_id > LIN_ID_MASK) {
> +		netdev_dbg(ndev, "id exceeds LIN max id\n");
> +		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +		cf.data[3] = CAN_ERR_PROT_LOC_ID12_05;
> +	}
> +
> +	if (lf->len > LIN_MAX_DLEN) {
> +		netdev_dbg(ndev, "frame exceeds number of bytes\n");
> +		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +		cf.data[3] = CAN_ERR_PROT_LOC_DLC;
> +	}
> +
> +	if (lf->len) {
> +		u8 checksum = lin_get_checksum(LIN_FORM_PID(lf->lin_id),
> +					       lf->len, lf->data,
> +					       lf->checksum_mode);
> +
> +		if (checksum != lf->checksum) {
> +			netdev_dbg(ndev, "expected cksm: 0x%02x got: 0x%02x\n",
> +				   checksum, lf->checksum);
> +			cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +			cf.data[2] = CAN_ERR_PROT_FORM;
> +		}
> +	}
> +
> +	if (cf.can_id & CAN_ERR_FLAG) {
> +		struct can_frame *err_cf;
> +		struct sk_buff *skb = alloc_can_err_skb(ndev, &err_cf);
> +
> +		if (unlikely(!skb))
> +			return -ENOMEM;
> +
> +		err_cf->can_id |= cf.can_id;
> +		memcpy(err_cf->data, cf.data, CAN_MAX_DLEN);
> +
> +		netif_rx(skb);
> +
> +		return -EREMOTEIO;
> +	}
> +
> +	return 0;
> +}
> +
> +int lin_rx(struct lin_device *ldev, const struct lin_frame *lf)
> +{
> +	struct net_device *ndev = ldev->ndev;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	if (ldev->can.state == CAN_STATE_STOPPED)
> +		return 0;
> +
> +	netdev_dbg(ndev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
> +		   lf->checksum_mode ? "enhanced" : "classic");
> +
> +	ret = lin_bump_rx_err(ldev, lf);
> +	if (ret) {
> +		DEV_STATS_INC(ndev, rx_dropped);
> +		return ret;
> +	}
> +
> +	skb = alloc_can_skb(ndev, &cf);
> +	if (unlikely(!skb)) {
> +		DEV_STATS_INC(ndev, rx_dropped);
> +		return -ENOMEM;
> +	}
> +
> +	cf->can_id = lf->lin_id;
> +	cf->len = min(lf->len, LIN_MAX_DLEN);
> +	memcpy(cf->data, lf->data, cf->len);
> +
> +	DEV_STATS_INC(ndev, rx_packets);
> +	DEV_STATS_ADD(ndev, rx_bytes, cf->len);
> +
> +	netif_receive_skb(skb);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lin_rx);
> +
> +static int lin_set_bittiming(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	unsigned int bitrate = ldev->can.bittiming.bitrate;
> +
> +	return ldev->ldev_ops->update_bitrate(ldev, bitrate);
> +}
> +
> +static const u32 lin_bitrate[] = { 1200, 2400, 4800, 9600, 19200 };
> +
> +struct lin_device *register_lin(struct device *dev,
> +				const struct lin_device_ops *ldops)
> +{
> +	struct net_device *ndev;
> +	struct lin_device *ldev;
> +	int ret;
> +
> +	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||

Extra space.

> +	    !ldops->ldo_open || !ldops->ldo_stop) {
> +		dev_err(dev, "missing mandatory lin_device_ops\n");

Is this a programming error? WARN_ON_ONCE() around the condition would be 
warranted in that case.

> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct lin_device), 1);
> +	if (!ndev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ldev = netdev_priv(ndev);
> +
> +	ldev->ldev_ops = ldops;
> +	ndev->netdev_ops = &lin_netdev_ops;
> +	ndev->flags |= IFF_ECHO;
> +	ndev->mtu = CANFD_MTU;
> +	ndev->sysfs_groups[0] = &lin_sysfs_group;
> +	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;
> +
> +	/* Using workqueue as tx over USB/SPI/... may sleep */
> +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> +				   0);
> +	if (!ldev->wq) {
> +		ret = -ENOMEM;
> +		goto exit_unreg;
> +	}
> +
> +	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
> +
> +	netdev_info(ndev, "LIN initialized\n");
> +
> +	return ldev;
> +
> +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;
> +
> +	unregister_candev(ndev);
> +	destroy_workqueue(ldev->wq);

Why is the order different from that in register side?

> +	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..31bb0feefd188
> --- /dev/null
> +++ b/include/net/lin.h
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#ifndef _NET_LIN_H_
> +#define _NET_LIN_H_
> +
> +#include <linux/bitfield.h>
> +#include <linux/can/dev.h>
> +#include <linux/device.h>
> +
> +#define LIN_NUM_IDS		64
> +#define LIN_HEADER_SIZE		3
> +#define LIN_MAX_DLEN		8
> +
> +#define LIN_MAX_BAUDRATE	20000
> +#define LIN_MIN_BAUDRATE	1000
> +#define LIN_DEFAULT_BAUDRATE	9600
> +#define LIN_SYNC_BYTE		0x55
> +
> +#define LIN_ID_MASK		GENMASK(5, 0)
> +/* special ID descriptions for LIN */
> +#define LIN_RXOFFLOAD_DATA_FLAG	BIT(9)
> +#define LIN_ENHANCED_CKSUM_FLAG	BIT(8)
> +
> +extern u8 lin_get_id_parity(u8 id);
> +
> +#define LIN_GET_ID(PID)		FIELD_GET(LIN_ID_MASK, PID)
> +#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
> +					lin_get_id_parity(LIN_GET_ID(ID)))
> +#define LIN_GET_PARITY(PID)	((PID) & ~LIN_ID_MASK)
> +#define LIN_CHECK_PID(PID)	(LIN_GET_PARITY(PID) == \
> +					LIN_GET_PARITY(LIN_FORM_PID(PID)))
> +
> +struct lin_attr {
> +	struct kobj_attribute attr;
> +	struct lin_device *ldev;
> +};
> +
> +struct lin_device {
> +	struct can_priv can;  /* must be the first member */
> +	struct net_device *ndev;
> +	struct device *dev;
> +	const struct lin_device_ops *ldev_ops;
> +	struct workqueue_struct *wq;
> +	struct work_struct tx_work;
> +	bool tx_busy;
> +	struct sk_buff *tx_skb;
> +};
> +
> +enum lin_checksum_mode {
> +	LINBUS_CLASSIC = 0,
> +	LINBUS_ENHANCED,
> +};
> +
> +struct lin_frame {
> +	u8 lin_id;
> +	u8 len;
> +	u8 data[LIN_MAX_DLEN];
> +	u8 checksum;
> +	enum lin_checksum_mode checksum_mode;
> +};
> +
> +struct lin_responder_answer {
> +	bool is_active;
> +	bool is_event_frame;
> +	u8 event_associated_id;
> +	struct lin_frame lf;
> +};
> +
> +struct lin_device_ops {
> +	int (*ldo_open)(struct lin_device *ldev);
> +	int (*ldo_stop)(struct lin_device *ldev);
> +	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
> +	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
> +	int (*update_responder_answer)(struct lin_device *ldev,
> +				       const struct lin_responder_answer *answ);
> +	int (*get_responder_answer)(struct lin_device *ldev, u8 id,
> +				    struct lin_responder_answer *answ);
> +};
> +
> +int lin_rx(struct lin_device *ldev, const struct lin_frame *lf);
> +
> +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> +		    enum lin_checksum_mode cm);
> +
> +struct lin_device *register_lin(struct device *dev,
> +				const struct lin_device_ops *ldops);
> +void unregister_lin(struct lin_device *ldev);
> +
> +#endif /* _NET_LIN_H_ */
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index 02ec32d694742..a37f56d86c5f2 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		BIT(11)	/* LIN bus mode */
>  
>  /*
>   * CAN device statistics
> 

-- 
 i.

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

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

> Introduce driver support for hexDEV hexLIN USB LIN adapter, enabling LIN
> communication over USB for both controller and responder modes. The
> driver interfaces with the CAN_LIN framework for userland connectivity.
> 
> For more details on the adapter, visit: https://hexdev.de/hexlin/
> 
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  drivers/hid/Kconfig             |  19 +
>  drivers/hid/Makefile            |   1 +
>  drivers/hid/hid-hexdev-hexlin.c | 609 ++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h           |   1 +
>  drivers/hid/hid-quirks.c        |   3 +
>  5 files changed, 633 insertions(+)
>  create mode 100644 drivers/hid/hid-hexdev-hexlin.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 08446c89eff6e..682e3ab5fdfe5 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -496,6 +496,25 @@ config HID_GYRATION
>  	help
>  	Support for Gyration remote control.
>  
> +config HID_MCS_HEXDEV
> +	tristate "hexDEV LIN-BUS adapter support"
> +	depends on HID && CAN_NETLINK && CAN_DEV
> +	select CAN_LIN
> +	help
> +	  Support for hexDEV its hexLIN USB LIN bus adapter.
> +
> +	  Local Interconnect Network (LIN) to USB adapter for controller and
> +	  responder usage.
> +	  This device driver is using CAN_LIN for a userland connection on
> +	  one side and USB HID for the actual hardware adapter on the other
> +	  side.
> +
> +	  If you have such an adapter, say Y here and see
> +	  <https://hexdev.de/hexlin>.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called hid-hexlin.
> +
>  config HID_ICADE
>  	tristate "ION iCade arcade controller"
>  	help
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index ce71b53ea6c54..6af678f283548 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_HID_GOOGLE_STADIA_FF)	+= hid-google-stadiaff.o
>  obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
>  obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
> +obj-$(CONFIG_HID_MCS_HEXDEV)	+= hid-hexdev-hexlin.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
> diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
> new file mode 100644
> index 0000000000000..a9ed080b3e33e
> --- /dev/null
> +++ b/drivers/hid/hid-hexdev-hexlin.c
> @@ -0,0 +1,609 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * LIN bus USB adapter driver https://hexdev.de/hexlin
> + *
> + * Copyright (C) 2024 hexDEV GmbH
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/wait.h>
> +#include <net/lin.h>
> +#include "hid-ids.h"
> +
> +#define HEXLIN_PKGLEN_MAX	64
> +#define HEXLIN_LEN_RETCODE	1
> +
> +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;
> +	__le16 baudrate;
> +} __packed;
> +
> +struct hexlin_frame {
> +	__le32 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 answ;
> +	u8 fw_version;
> +};
> +
> +static int hexlin_tx_req_status(struct hexlin_priv_data *priv,
> +				const void *out_report, int len)
> +{
> +	unsigned long t;
> +	int n, ret = 0;
> +
> +	mutex_lock(&priv->tx_lock);

Use guard().

In general btw, if you're instructed to change something, it's useful to 
check your own patch series for similar cases as review pass is likely to 
miss some or not even spend time to point out the same thing over and 
over again. Then they are raised on the next review round which could have 
been avoided.

> +	reinit_completion(&priv->wait_in_report);
> +
> +	n = hid_hw_output_report(priv->hid_dev, (__u8 *) out_report, len);

The usual is to not leave space between cast and what is being cast. I 
know hid functions seem to use __u8 but that's intended for uapi and in 
kernel, u8 should be used (somebody should eventually cleanup the hid 
function types too).

> +	if (n < 0) {
> +		mutex_unlock(&priv->tx_lock);
> +		return n;
> +	}
> +	if (n != len) {
> +		mutex_unlock(&priv->tx_lock);
> +		return -EIO;
> +	}
> +
> +	t = wait_for_completion_killable_timeout(&priv->wait_in_report, HZ);
> +	if (!t)
> +		ret = -ETIMEDOUT;
> +
> +	if (priv->is_error)
> +		ret = -EINVAL;

These can return directly when you use guard().

> +	mutex_unlock(&priv->tx_lock);
> +
> +	return ret;

return 0; + drop ret variable.

> +}
> +
> +#define HEXLIN_GET_CMD(name, enum_cmd)					\
> +	static int hexlin_##name(struct hexlin_priv_data *p)		\
> +	{								\
> +		u8 *req;						\
> +		int ret;						\
> +									\
> +		req = kmalloc(sizeof(*req), GFP_KERNEL)	;		\

Extra space.

Use:

u8 *req __free(kfree) = kmalloc(sizeof(*req), GFP_KERNEL);

> +		if (!req)						\
> +			return -ENOMEM;					\
> +									\
> +		*req = enum_cmd;					\
> +									\
> +		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
> +		if (ret)						\
> +			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
> +				#name, ret);				\
> +									\
> +		kfree(req);						\

Not needed after using __free().

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

Could you convert the function in the macro into a helper function which 
is just called by a simple function with the relevant parameters for 
these 3 cases?

> +#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 = kmalloc(sizeof(*req), GFP_KERNEL)	;		\

Extra space.

Use __free().

> +		if (!req)						\
> +			return -ENOMEM;					\
> +									\
> +		req->cmd = enum_cmd;					\
> +		req->v = val;						\
> +									\
> +		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
> +		if (ret)						\
> +			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
> +				#name, ret);				\
> +									\
> +		kfree(req);						\
> +		return ret;						\
> +	}
> +
> +HEXLIN_VAL_CMD(send_break, HEXLIN_SEND_BREAK, hexlin_val8_req, u8)
> +
> +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 = kmalloc(sizeof(*req), GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->cmd = HEXLIN_SEND_UNCONDITIONAL_FRAME;
> +	memcpy(&req->frm, hxf, sizeof(*hxf));
> +
> +	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
> +	if (ret)
> +		hid_err(priv->hid_dev, "unconditional tx failed: %d\n", ret);
> +
> +	kfree(req);
> +	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 = kmalloc(sizeof(*req), GFP_KERNEL);

Hmm... Why do you alloc this small structure (3 bytes?) with kmalloc() and 
not just have it in stack as a local variable?

> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->cmd = HEXLIN_SET_BAUDRATE;
> +	req->baudrate = cpu_to_le16(baudrate);
> +
> +	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
> +	if (ret)
> +		hid_err(priv->hid_dev, "set baudrate failed: %d\n", ret);
> +
> +	kfree(req);
> +	return ret;
> +}
> +
> +static int hexlin_get_responder_answer_id(struct hexlin_priv_data *priv, u8 id,
> +					  struct hexlin_responder_answer *ransw)
> +{
> +	struct hexlin_val8_req *req;
> +	int ret;
> +
> +	if (id > LIN_ID_MASK)
> +		return -EINVAL;
> +
> +	req = kmalloc(sizeof(*req), GFP_KERNEL);

Use a stack variable instead?

> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->cmd = HEXLIN_GET_RESPONDER_ANSWER_ID;
> +	req->v = id;
> +
> +	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
> +	if (ret) {
> +		hid_err(priv->hid_dev, "get respond answer failed: %d\n", ret);
> +		kfree(req);
> +		return ret;
> +	}
> +
> +	memcpy(ransw, &priv->answ, sizeof(priv->answ));
> +
> +	kfree(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 *req;
> +	int ret;
> +
> +	if (answ->lf.lin_id > LIN_ID_MASK ||
> +	    answ->event_associated_id > LIN_ID_MASK)
> +		return -EINVAL;
> +
> +	req = kmalloc(sizeof(*req), GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->cmd = HEXLIN_SET_RESPONDER_ANSWER_ID;
> +	req->answ.is_active = answ->is_active;
> +	req->answ.is_event_frame = answ->is_event_frame;
> +	req->answ.event_associated_id = answ->event_associated_id;
> +	req->answ.frm.len = answ->lf.len;
> +	req->answ.frm.lin_id = answ->lf.lin_id;
> +	memcpy(req->answ.frm.data, answ->lf.data, LIN_MAX_DLEN);
> +	req->answ.frm.checksum = answ->lf.checksum;
> +	req->answ.frm.checksum_mode = answ->lf.checksum_mode;
> +
> +	ret = hexlin_tx_req_status(priv, req, sizeof(*req));
> +	if (ret)
> +		hid_err(priv->hid_dev, "set respond answer failed: %d\n", ret);
> +
> +	kfree(req);
> +	return ret;
> +}
> +
> +static int hexlin_open(struct lin_device *ldev)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +
> +	return hid_hw_open(hdev);
> +}
> +
> +static int hexlin_stop(struct lin_device *ldev)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	hid_hw_close(hdev);
> +
> +	priv->is_error = true;
> +	complete(&priv->wait_in_report);
> +
> +	return 0;
> +}
> +
> +static int hexlin_ldo_tx(struct lin_device *ldev,
> +			 const struct lin_frame *lf)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	int ret = -EINVAL;
> +
> +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
> +		   lf->checksum_mode ? "enhanced" : "classic");
> +
> +	if (lf->lin_id && lf->len == 0) {
> +		ret = hexlin_send_break(priv, lf->lin_id);
> +	} else if (lf->len <= LIN_MAX_DLEN) {
> +		struct hexlin_frame hxf;
> +
> +		hxf.len = lf->len;
> +		hxf.lin_id = lf->lin_id;
> +		memcpy(&hxf.data, lf->data, LIN_MAX_DLEN);
> +		hxf.checksum = lf->checksum;
> +		hxf.checksum_mode = lf->checksum_mode;
> +		ret = hexlin_send_unconditional(priv, &hxf);
> +	} else {
> +		hid_err(hdev, "unknown format\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int hexlin_update_bitrate(struct lin_device *ldev, u16 bitrate)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	hid_dbg(hdev, "update bitrate to: %u\n", bitrate);
> +
> +	ret = hexlin_open(ldev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hexlin_set_baudrate(priv, bitrate);
> +	if (ret)
> +		return ret;
> +
> +	ret = hexlin_get_baudrate(priv);
> +	if (ret)
> +		return ret;
> +
> +	if (priv->baudrate != bitrate) {
> +		hid_err(hdev, "update bitrate failed\n");
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int hexlin_get_responder_answer(struct lin_device *ldev, u8 id,
> +				       struct lin_responder_answer *answ)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	struct hexlin_responder_answer ransw;
> +	int ret;
> +
> +	if (answ == NULL)
> +		return -EINVAL;
> +
> +	ret = hexlin_get_responder_answer_id(priv, id, &ransw);
> +	if (ret)
> +		return ret;
> +
> +	answ->is_active = ransw.is_active;
> +	answ->is_event_frame = ransw.is_event_frame;
> +	answ->event_associated_id = ransw.event_associated_id;
> +	answ->lf.len = ransw.frm.len;
> +	answ->lf.lin_id = ransw.frm.lin_id;
> +	memcpy(answ->lf.data, ransw.frm.data, LIN_MAX_DLEN);
> +	answ->lf.checksum = ransw.frm.checksum;
> +	answ->lf.checksum_mode = ransw.frm.checksum_mode;
> +
> +	return 0;
> +}
> +
> +static int hexlin_update_resp_answer(struct lin_device *ldev,
> +				     const struct lin_responder_answer *answ)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	if (answ == NULL)
> +		return -EINVAL;
> +
> +	return hexlin_set_responder_answer_id(priv, answ);
> +}
> +
> +static const struct lin_device_ops hexlin_ldo = {
> +	.ldo_open = hexlin_open,
> +	.ldo_stop = hexlin_stop,
> +	.ldo_tx = hexlin_ldo_tx,
> +	.update_bitrate = hexlin_update_bitrate,
> +	.get_responder_answer = hexlin_get_responder_answer,
> +	.update_responder_answer = hexlin_update_resp_answer,
> +};
> +
> +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> +				      const struct hexlin_frame *hxf)
> +{
> +	struct hid_device *hdev = priv->hid_dev;
> +	struct lin_frame lf;
> +
> +	lf.len = hxf->len;
> +	lf.lin_id = hxf->lin_id;
> +	memcpy(lf.data, hxf->data, LIN_MAX_DLEN);
> +	lf.checksum = hxf->checksum;
> +	lf.checksum_mode = hxf->checksum_mode;
> +
> +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, chk:%02x (%s), flg:%08x\n",
> +		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> +		lf.checksum_mode ? "enhanced" : "classic", hxf->flags);
> +
> +	lin_rx(priv->ldev, &lf);
> +
> +	return 0;
> +}
> +
> +static int hexlin_raw_event(struct hid_device *hdev,
> +			    struct hid_report *report, u8 *data, int sz)
> +{
> +	struct hexlin_priv_data *priv;
> +	struct hexlin_baudrate_req *br;
> +	struct hexlin_responder_answer_req *rar;
> +	struct hexlin_unconditional_req *hfr;
> +	struct hexlin_val8_req *vr;
> +
> +	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 != HEXLIN_LEN_RETCODE)
> +			return -EREMOTEIO;
> +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_FAIL:
> +		if (sz != HEXLIN_LEN_RETCODE)
> +			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 != sizeof(*vr))
> +			return -EREMOTEIO;
> +		vr = (struct hexlin_val8_req *) data;
> +		priv->fw_version = vr->v;
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> +		if (sz != sizeof(*rar))
> +			return -EREMOTEIO;
> +		rar = (struct hexlin_responder_answer_req *) data;
> +		memcpy(&priv->answ, &rar->answ, sizeof(priv->answ));
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_BAUDRATE:
> +		if (sz != sizeof(*br))
> +			return -EREMOTEIO;
> +		br = (struct hexlin_baudrate_req *) data;
> +		le16_to_cpus(br->baudrate);
> +		priv->baudrate = br->baudrate;
> +		complete(&priv->wait_in_report);
> +		break;
> +	/* following cases not initiated by us, so no complete() */
> +	case HEXLIN_FRAME:
> +		if (sz != sizeof(*hfr)) {
> +			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
> +			return -EREMOTEIO;
> +		}
> +		hfr = (struct hexlin_unconditional_req *) data;
> +		le32_to_cpus(hfr->frm.flags);

I'm bit worried about this from endianness perspective. Perhaps there's 
some struct reusing that shouldn't be happening because the same field 
cannot be __le32 and u32 at the same time.

> +		hexlin_queue_frames_insert(priv, &hfr->frm);
> +		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) {
> +		/* if first reset fails, try one more time */
> +		ret = hexlin_reset_dev(priv);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = hexlin_get_version(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->baudrate = LIN_DEFAULT_BAUDRATE;
> +	ret = hexlin_set_baudrate(priv, priv->baudrate);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int hexlin_probe(struct hid_device *hdev,
> +			const struct hid_device_id *id)
> +{
> +	struct hexlin_priv_data *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->hid_dev = hdev;
> +	hid_set_drvdata(hdev, priv);
> +
> +	mutex_init(&priv->tx_lock);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid parse failed with %d\n", ret);
> +		goto fail_and_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> +	if (ret) {
> +		hid_err(hdev, "hid hw start failed with %d\n", ret);
> +		goto fail_and_stop;
> +	}
> +
> +	ret = hid_hw_open(hdev);
> +	if (ret) {
> +		hid_err(hdev, "hid hw open failed with %d\n", ret);
> +		goto fail_and_close;
> +	}
> +
> +	init_completion(&priv->wait_in_report);
> +
> +	hid_device_io_start(hdev);
> +
> +	ret = init_hw(priv);
> +	if (ret)
> +		goto fail_and_close;
> +
> +	priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> +	if (IS_ERR(priv->ldev)) {
> +		ret = PTR_ERR(priv->ldev);
> +		goto fail_and_close;
> +	}
> +
> +	hid_hw_close(hdev);
> +
> +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);

By custom, the success path in probe should not print anything so make 
this dbg level message if the fw version information is valuable in some 
debugging scenario.

> +	return 0;
> +
> +fail_and_close:
> +	hid_hw_close(hdev);
> +fail_and_stop:
> +	hid_hw_stop(hdev);
> +fail_and_free:
> +	mutex_destroy(&priv->tx_lock);
> +	return ret;
> +}
> +
> +static void hexlin_remove(struct hid_device *hdev)
> +{
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	unregister_lin(priv->ldev);
> +	hid_hw_stop(hdev);
> +}
> +
> +static const struct hid_device_id hexlin_table[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(hid, hexlin_table);
> +
> +static struct hid_driver hexlin_driver = {
> +	.name = "hexLIN",
> +	.id_table = hexlin_table,
> +	.probe = hexlin_probe,
> +	.remove = hexlin_remove,
> +	.raw_event = hexlin_raw_event,
> +};
> +
> +module_hid_driver(hexlin_driver);
> +
> +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 61d2a21affa26..c6fe6f99a0e80 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -907,6 +907,7 @@
>  
>  #define USB_VENDOR_ID_MCS		0x16d0
>  #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
> +#define USB_DEVICE_ID_MCS_HEXDEV_HEXLIN	0x0648
>  
>  #define USB_VENDOR_MEGAWORLD		0x07b5
>  #define USB_DEVICE_ID_MEGAWORLD_GAMEPAD	0x0312
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e0bbf0c6345d6..d721110d0889b 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_MCS_HEXDEV)
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_HOLTEK)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK, USB_DEVICE_ID_HOLTEK_ON_LINE_GRIP) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD) },
> 

-- 
 i.

Re: [PATCH v4 03/11] treewide, serdev: add flags argument to receive_buf()

[Ilpo Järvinen]

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

On Thu, 9 May 2024, Christoph Fritz wrote:

> For serdev device drivers to be able to detect TTY_BREAK and other flags
> in the buffer, pass the flag buffer pointer down to serdev its receive
> function and update all drivers using it.
> 
> The changes were mostly done using the following Coccinelle
> semantic patch:
> 
> // <smpl>
> @ rule1 @
> identifier fn;
> identifier opsname;
> @@
> struct serdev_device_ops opsname = {
> 	.receive_buf = fn,
> };
> @@
> identifier rule1.fn;
> parameter E1, E2, E3;
> typedef u8;
> @@
>   fn(E1, E2,
> + const u8 *flags,
>   E3)
>   { ... }
> // </smpl>
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>

> + * @receive_buf:	Function called with data received from device (char
> + *			buffer), flags buffer (%TTY_NORMAL, %TTY_BREAK, etc)
> + *			and number of bytes;
>   *			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);
> +	size_t (*receive_buf)(struct serdev_device *, const u8 *, const u8 *,
> +			      size_t);

These parameters should be named now that they're being touched.

With that done,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v4 04/11] tty: serdev: Add method to enable break flags

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

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

Why you need to test for !ctrl?

> +	ctrl->ops->set_break_detection(ctrl, enable);

I'd use positive logic here:

	if (ctrl->ops->set_break_detection)
		ctrl->ops->set_break_detection(ctrl, enable);

> +}
> +EXPORT_SYMBOL_GPL(serdev_device_set_break_detection);


-- 
 i.

Re: [PATCH v4 07/11] can: Add support for hexDEV serial LIN adapter hexLINSER

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

> Introduce support for the hexDEV serial LIN adapter hexLINSER. These
> devices are equipped with LIN transceivers and are mostly hard-wired to
> serial devices.
> 
> This device driver uses CAN_LIN on one side and the serial device bus
> (serdev) interface on the other.
> 
> For more details on the adapter, visit: https://hexdev.de/hexlin#hexLINSER
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  drivers/net/can/Kconfig      |  15 ++
>  drivers/net/can/Makefile     |   1 +
>  drivers/net/can/hex-linser.c | 505 +++++++++++++++++++++++++++++++++++
>  3 files changed, 521 insertions(+)
>  create mode 100644 drivers/net/can/hex-linser.c
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 0934bbf8d03b2..141972d6bbf1e 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -181,6 +181,21 @@ config CAN_LIN
>  
>  	  Actual device drivers need to be enabled too.
>  
> +config CAN_LIN_HEXLINSER
> +	tristate "hexDEV hexLINSER serial LIN Adaptors"
> +	depends on CAN_LIN && SERIAL_DEV_BUS && OF
> +	help
> +	  LIN support for serial devices equipped with LIN transceivers.
> +	  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 hexLINSER tty adapter, say Y here and see
> +	  <https://hexdev.de/hexlin#hexLINSER>.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called hex-linser.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..9fdad4a0fd12a 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_HEXLINSER)	+= hex-linser.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/hex-linser.c b/drivers/net/can/hex-linser.c
> new file mode 100644
> index 0000000000000..9c2d11d2ed0c0
> --- /dev/null
> +++ b/drivers/net/can/hex-linser.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/kfifo.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <net/lin.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;
> +	unsigned long break_usleep_min;
> +	unsigned long break_usleep_max;
> +	unsigned long post_break_usleep_min;
> +	unsigned long post_break_usleep_max;
> +	unsigned long force_timeout_jfs;
> +	struct lin_responder_answer respond_answ[LIN_NUM_IDS];
> +	struct mutex resp_lock; /* protects respond_answ */
> +	bool is_stopped;
> +};
> +
> +static int linser_open(struct lin_device *ldev)
> +{
> +	struct serdev_device *serdev = to_serdev_device(ldev->dev);
> +	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +	int ret;
> +
> +	if (priv->is_stopped) {
> +		ret = serdev_device_open(serdev);
> +		if (ret) {
> +			dev_err(&serdev->dev, "Unable to open device\n");
> +			return ret;
> +		}
> +
> +		serdev_device_set_flow_control(serdev, false);
> +		serdev_device_set_break_detection(serdev, true);
> +
> +		priv->is_stopped = false;
> +	}
> +
> +	return 0;
> +}
> +
> +static int linser_stop(struct lin_device *ldev)
> +{
> +	struct serdev_device *serdev = to_serdev_device(ldev->dev);
> +	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +
> +	if (priv->is_stopped)
> +		return 0;
> +
> +	serdev_device_close(serdev);
> +	priv->is_stopped = true;
> +
> +	return 0;
> +}
> +
> +static int linser_send_break(struct linser_priv *priv)
> +{
> +	struct serdev_device *serdev = priv->serdev;
> +	int ret;
> +
> +	ret = serdev_device_break_ctl(serdev, -1);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(priv->break_usleep_min, priv->break_usleep_max);
> +
> +	ret = serdev_device_break_ctl(serdev, 0);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(priv->post_break_usleep_min, priv->post_break_usleep_max);
> +
> +	return 0;
> +}
> +
> +static int linser_ldo_tx(struct lin_device *ldev, const struct lin_frame *lf)
> +{
> +	struct serdev_device *serdev = to_serdev_device(ldev->dev);
> +	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +	u8 pid = LIN_FORM_PID(lf->lin_id);
> +	u8 buf[LINSER_TX_BUFFER_SIZE];
> +	ssize_t written_len, total_len;
> +	u8 checksum;
> +	int ret;
> +
> +	if (lf->len + 3 > LINSER_TX_BUFFER_SIZE) {
> +		dev_err(&serdev->dev, "Frame length %u exceeds buffer size\n", lf->len);
> +		return -EINVAL;
> +	}
> +
> +	buf[0] = LIN_SYNC_BYTE;
> +	buf[1] = pid;
> +	total_len = 2;
> +
> +	if (lf->len) {
> +		memcpy(&buf[2], lf->data, lf->len);
> +		checksum = lin_get_checksum(pid, lf->len, lf->data,
> +					    lf->checksum_mode);
> +		buf[lf->len + 2] = checksum;
> +		total_len += lf->len + 1;
> +	}
> +
> +	ret = linser_send_break(priv);
> +	if (ret)
> +		return ret;
> +
> +	written_len = serdev_device_write(serdev, buf, total_len, 0);
> +	if (written_len < total_len)
> +		return written_len < 0 ? (int)written_len : -EIO;
> +
> +	dev_dbg(&serdev->dev, "sent out: %*ph\n", (int)total_len, buf);
> +
> +	serdev_device_wait_until_sent(serdev, 0);
> +
> +	return 0;
> +}
> +
> +static int linser_derive_timings(struct linser_priv *priv, u16 bitrate)
> +{
> +	unsigned long break_baud = (bitrate * 2) / 3;
> +	struct serdev_device *serdev = priv->serdev;
> +	unsigned long timeout_us;
> +
> +	if (bitrate < LIN_MIN_BAUDRATE || bitrate > LIN_MAX_BAUDRATE) {
> +		dev_err(&serdev->dev, "Bitrate %u out of bounds (%u to %u)\n",
> +			bitrate, LIN_MIN_BAUDRATE, LIN_MAX_BAUDRATE);
> +		return -EINVAL;
> +	}
> +
> +	priv->break_usleep_min = (USEC_PER_SEC * LINSER_SAMPLES_PER_CHAR) /
> +				 break_baud;
> +	priv->break_usleep_max = priv->break_usleep_min + 50;
> +	priv->post_break_usleep_min = USEC_PER_SEC / break_baud;
> +	priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
> +
> +	timeout_us = DIV_ROUND_CLOSEST(USEC_PER_SEC * 256, bitrate);
> +	priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
> +
> +	return 0;
> +}
> +
> +static int linser_update_bitrate(struct lin_device *ldev, u16 bitrate)
> +{
> +	struct serdev_device *serdev = to_serdev_device(ldev->dev);
> +	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +	unsigned int speed;
> +	int ret;
> +
> +	ret = linser_open(ldev);
> +	if (ret)
> +		return ret;
> +
> +	speed = serdev_device_set_baudrate(serdev, bitrate);
> +	if (!bitrate || speed != bitrate)
> +		return -EINVAL;
> +
> +	ret = linser_derive_timings(priv, bitrate);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int linser_get_responder_answer(struct lin_device *ldev, u8 id,
> +				       struct lin_responder_answer *answ)
> +{
> +	struct serdev_device *serdev = to_serdev_device(ldev->dev);
> +	struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +	struct lin_responder_answer *r = &priv->respond_answ[id];
> +
> +	if (!answ)
> +		return -EINVAL;
> +
> +	guard(mutex)(&priv->resp_lock);
> +	memcpy(answ, r, sizeof(*answ));
> +
> +	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(*answ));
> +	r->lf.checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> +					  answ->lf.len,
> +					  answ->lf.data,
> +					  answ->lf.checksum_mode);

Can this checksum occur outside of lock using the copy in r?

In anycase, use guard() (or scoped_guard() if the checksum can happen 
outside of the lock.)

> +	mutex_unlock(&priv->resp_lock);
> +
> +	return 0;
> +}
> +
> +static struct lin_device_ops linser_lindev_ops = {
> +	.ldo_open = linser_open,
> +	.ldo_stop = linser_stop,
> +	.ldo_tx = linser_ldo_tx,
> +	.update_bitrate = linser_update_bitrate,
> +	.get_responder_answer = linser_get_responder_answer,
> +	.update_responder_answer = linser_update_resp_answer,
> +};
> +
> +static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
> +{
> +	struct lin_responder_answer *answ = &priv->respond_answ[id];
> +	struct serdev_device *serdev = priv->serdev;
> +	u8 buf[LINSER_TX_BUFFER_SIZE];
> +	u8 checksum, count, n;
> +	ssize_t write_len;
> +
> +	scoped_guard(mutex, &priv->resp_lock) {
> +		if (!answ->is_active)
> +			return 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)
> +				return false;
> +
> +			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 {
> +			checksum = answ->lf.checksum;
> +		}
> +
> +		count = min(answ->lf.len, LIN_MAX_DLEN);
> +		memcpy(&buf[0], answ->lf.data, count);
> +		buf[count] = checksum;
> +	}
> +
> +	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;
> +}
> +
> +static void linser_pop_fifo(struct linser_priv *priv, size_t n)
> +{
> +	for (size_t i = 0; i < n; i++)
> +		kfifo_skip(&priv->rx_fifo);
> +}
> +
> +static int linser_fill_frame(struct linser_priv *priv, struct lin_frame *lf)
> +{
> +	struct serdev_device *serdev = priv->serdev;
> +	struct linser_rx buf[LINSER_PARSE_BUFFER];
> +	unsigned int count, i, brk = 0;
> +
> +	count = kfifo_out_peek(&priv->rx_fifo, buf, LINSER_PARSE_BUFFER);
> +
> +	memset(lf, 0, sizeof(*lf));
> +
> +	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)))
> +			return ret;
> +
> +		dev_dbg(&serdev->dev, "lin_rx: %s\n", force ?
> +			"force" : "normal");

Put to one line.

> +		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;
> +	}
> +
> +	return ret;
> +}


-- 
 i.

Re: [PATCH v4 09/11] can: lin: Handle rx offload config frames

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

> The CAN Broadcast Manager now has the capability to dispatch CANFD
> frames marked with the id LINBUS_RXOFFLOAD_ID.
> 
> Introduce functionality to interpret these specific frames, enabling the
> configuration of RX offloading within the LIN driver.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  drivers/net/can/lin.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
> index a22768c17e3f8..f77abd7d7d21c 100644
> --- a/drivers/net/can/lin.c
> +++ b/drivers/net/can/lin.c
> @@ -185,6 +185,27 @@ static const struct attribute_group lin_sysfs_group = {
>  	.attrs = lin_sysfs_attrs,
>  };
>  
> +static int lin_setup_rxoffload(struct lin_device *ldev,
> +			       struct canfd_frame *cfd)
> +{
> +	struct lin_responder_answer answ;
> +
> +	if (!(cfd->flags & CANFD_FDF))
> +		return -EINVAL;
> +
> +	BUILD_BUG_ON(sizeof(answ) > sizeof(cfd->data));
> +	memcpy(&answ, cfd->data, sizeof(answ));
> +
> +	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;

These can be reverse so that error check occur before the checksum_mode 
assignment? It would feel more natural that way.

...Even better, if the error check could be done before the memcpy().

-- 
 i.


> +	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,
> @@ -197,6 +218,14 @@ static void lin_tx_work_handler(struct work_struct *ws)
>  	ldev->tx_busy = true;
>  
>  	cfd = (struct canfd_frame *)ldev->tx_skb->data;
> +
> +	if (cfd->can_id & LIN_RXOFFLOAD_DATA_FLAG) {
> +		ret = lin_setup_rxoffload(ldev, cfd);
> +		if (ret < 0)
> +			netdev_err(ndev, "setting up rx failed %d\n", ret);
> +		goto lin_tx_out;
> +	}
> +
>  	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
>  			   LINBUS_ENHANCED : LINBUS_CLASSIC;
>  	lf.lin_id = cfd->can_id & LIN_ID_MASK;
> 

Re: [PATCH v4 10/11] can: lin: Support setting LIN mode

[Ilpo Järvinen]

On Thu, 9 May 2024, Christoph Fritz wrote:

> A LIN node can work as commander or responder, so introduce a new
> control mode (CAN_CTRLMODE_LIN_COMMANDER) for configuration.
> 
> This enables e.g. the userland tool ip from iproute2 to 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 f77abd7d7d21c..03ddf5d5a31b8 100644
> --- a/drivers/net/can/lin.c
> +++ b/drivers/net/can/lin.c
> @@ -262,11 +262,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");

In user visible messages, it would be best to use the expected 
capitalization, which I suppose is LIN given you use capitals in the 
commit message yourself?

> +			return -EINVAL;
> +		}
> +		ret = ldev->ldev_ops->update_lin_mode(ldev, lm);
> +		if (ret) {
> +			netdev_err(ndev, "Failed to set lin mode: %d\n", ret);

Ditto.

There might be other cases in any of the patches, please check.

> +			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);
> @@ -443,7 +472,7 @@ struct lin_device *register_lin(struct device *dev,
>  	ndev->sysfs_groups[0] = &lin_sysfs_group;
>  	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;
> @@ -458,6 +487,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");

Ditto.

> +			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 31bb0feefd188..63ac870a0ab6f 100644
> --- a/include/net/lin.h
> +++ b/include/net/lin.h
> @@ -36,6 +36,11 @@ struct lin_attr {
>  	struct lin_device *ldev;
>  };
>  
> +enum lin_mode {
> +	LINBUS_RESPONDER = 0,
> +	LINBUS_COMMANDER,
> +};
> +
>  struct lin_device {
>  	struct can_priv can;  /* must be the first member */
>  	struct net_device *ndev;
> @@ -45,6 +50,7 @@ struct lin_device {
>  	struct work_struct tx_work;
>  	bool tx_busy;
>  	struct sk_buff *tx_skb;
> +	enum lin_mode lmode;
>  };
>  
>  enum lin_checksum_mode {
> @@ -71,6 +77,7 @@ struct lin_device_ops {
>  	int (*ldo_open)(struct lin_device *ldev);
>  	int (*ldo_stop)(struct lin_device *ldev);
>  	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
> +	int (*update_lin_mode)(struct lin_device *ldev, enum lin_mode lm);
>  	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
>  	int (*update_responder_answer)(struct lin_device *ldev,
>  				       const struct lin_responder_answer *answ);
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index a37f56d86c5f2..cc390f6444d59 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		BIT(11)	/* LIN bus mode */
> +#define CAN_CTRLMODE_LIN_COMMANDER	BIT(12)	/* LIN bus specific commander mode */
>  
>  /*
>   * CAN device statistics
> 

-- 
 i.

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

[Christoph Fritz]

...
> > diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
> > new file mode 100644
> > index 0000000000000..a9ed080b3e33e
> > --- /dev/null
> > +++ b/drivers/hid/hid-hexdev-hexlin.c
> > 
...
> 
> > +}
> > +
> > +#define HEXLIN_GET_CMD(name, enum_cmd)					\
> > +	static int hexlin_##name(struct hexlin_priv_data *p)		\
> > +	{								\
> > +		u8 *req;						\
> > +		int ret;						\
> > +									\
> > +		req = kmalloc(sizeof(*req), GFP_KERNEL)	;		\
> 
> Extra space.
> 
> Use:
> 
> u8 *req __free(kfree) = kmalloc(sizeof(*req), GFP_KERNEL);
> 
> > +		if (!req)						\
> > +			return -ENOMEM;					\
> > +									\
> > +		*req = enum_cmd;					\
> > +									\
> > +		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
> > +		if (ret)						\
> > +			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
> > +				#name, ret);				\
> > +									\
> > +		kfree(req);						\
> 
> Not needed after using __free().
> 
> > +		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)
> 
> Could you convert the function in the macro into a helper function which 
> is just called by a simple function with the relevant parameters for 
> these 3 cases?

The device actually has a lot more features that I'm using in my debug
version and which might end up here in the future. So I would like to
keep it. Any objections?

...
> > +
> > +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 = kmalloc(sizeof(*req), GFP_KERNEL);
> 
> Hmm... Why do you alloc this small structure (3 bytes?) with kmalloc() and 
> not just have it in stack as a local variable?


This buffer must be suitable for DMA (see docu for struct urb).

So with a stack variable we would need to use kmemdup() before the
actual sending call, but that's what you did not like since v3 so I
changed it to this which now you also don't like.

Let's dial it back to the original kmemdup() usage, ok?

...
> > +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> > +				      const struct hexlin_frame *hxf)
> > +{
> > +	struct hid_device *hdev = priv->hid_dev;
> > +	struct lin_frame lf;
> > +
> > +	lf.len = hxf->len;
> > +	lf.lin_id = hxf->lin_id;
> > +	memcpy(lf.data, hxf->data, LIN_MAX_DLEN);
> > +	lf.checksum = hxf->checksum;
> > +	lf.checksum_mode = hxf->checksum_mode;
> > +
> > +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, chk:%02x (%s), flg:%08x\n",
> > +		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> > +		lf.checksum_mode ? "enhanced" : "classic", hxf->flags);
> > +
> > +	lin_rx(priv->ldev, &lf);
> > +
> > +	return 0;
> > +}
> > +
> > +static int hexlin_raw_event(struct hid_device *hdev,
> > +			    struct hid_report *report, u8 *data, int sz)
> > +{
> > +	struct hexlin_priv_data *priv;
> > +	struct hexlin_baudrate_req *br;
> > +	struct hexlin_responder_answer_req *rar;
> > +	struct hexlin_unconditional_req *hfr;
> > +	struct hexlin_val8_req *vr;
> > +
> > +	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 != HEXLIN_LEN_RETCODE)
> > +			return -EREMOTEIO;
> > +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	case HEXLIN_FAIL:
> > +		if (sz != HEXLIN_LEN_RETCODE)
> > +			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 != sizeof(*vr))
> > +			return -EREMOTEIO;
> > +		vr = (struct hexlin_val8_req *) data;
> > +		priv->fw_version = vr->v;
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> > +		if (sz != sizeof(*rar))
> > +			return -EREMOTEIO;
> > +		rar = (struct hexlin_responder_answer_req *) data;
> > +		memcpy(&priv->answ, &rar->answ, sizeof(priv->answ));
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	case HEXLIN_GET_BAUDRATE:
> > +		if (sz != sizeof(*br))
> > +			return -EREMOTEIO;
> > +		br = (struct hexlin_baudrate_req *) data;
> > +		le16_to_cpus(br->baudrate);
> > +		priv->baudrate = br->baudrate;
> > +		complete(&priv->wait_in_report);
> > +		break;
> > +	/* following cases not initiated by us, so no complete() */
> > +	case HEXLIN_FRAME:
> > +		if (sz != sizeof(*hfr)) {
> > +			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
> > +			return -EREMOTEIO;
> > +		}
> > +		hfr = (struct hexlin_unconditional_req *) data;
> > +		le32_to_cpus(hfr->frm.flags);
> 
> I'm bit worried about this from endianness perspective. Perhaps there's 
> some struct reusing that shouldn't be happening because the same field 
> cannot be __le32 and u32 at the same time.

Can you propose a solution?

> 
...

thanks
  -- Christoph

Re: [PATCH v4 04/11] tty: serdev: Add method to enable break flags

[Christoph Fritz]

On Fri, 2024-05-10 at 17:21 +0300, Ilpo Järvinen wrote:
> On Thu, 9 May 2024, Christoph Fritz wrote:
...
> > 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;
> 
> Why you need to test for !ctrl?

In our case we don't, it's an extra check like all the other functions
here:

https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/tty/serdev/core.c#L330

> 
> > +	ctrl->ops->set_break_detection(ctrl, enable);
> 
> I'd use positive logic here:
> 
> 	if (ctrl->ops->set_break_detection)
> 		ctrl->ops->set_break_detection(ctrl, enable);

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

[Jiri Slaby]

On 10. 05. 24, 15:46, Ilpo Järvinen wrote:
>> +	reinit_completion(&priv->wait_in_report);
>> +
>> +	n = hid_hw_output_report(priv->hid_dev, (__u8 *) out_report, len);
> 
> The usual is to not leave space between cast and what is being cast. I
> know hid functions seem to use __u8 but that's intended for uapi and in
> kernel, u8 should be used (somebody should eventually cleanup the hid
> function types too).

Apart from that, you are attached to USB, so this goes down to usbhid 
(the ll driver). Are you sure the put-const-away cast is right thing to 
do here? (usbhid passes it to usb_interrupt_msg().)

That is the only reason why you have the cast in there in the first place…

regards,
-- 
js

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

[Simon Horman]

On Thu, May 09, 2024 at 07:17:26PM +0200, Christoph Fritz wrote:
> Introduce a LIN (local interconnect network) abstraction on top of CAN.
> This 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>

...

> +#define LID(_name) \
> +	struct device_attribute linid_##_name = __ATTR(_name, 0644, \
> +	lin_identifier_show, lin_identifier_store)
> +
> +LID(00); LID(01); LID(02); LID(03); LID(04); LID(05); LID(06); LID(07);
> +LID(08); LID(09); LID(0a); LID(0b); LID(0c); LID(0d); LID(0e); LID(0f);
> +LID(10); LID(11); LID(12); LID(13); LID(14); LID(15); LID(16); LID(17);
> +LID(18); LID(19); LID(1a); LID(1b); LID(1c); LID(1d); LID(1e); LID(1f);
> +LID(20); LID(21); LID(22); LID(23); LID(24); LID(25); LID(26); LID(27);
> +LID(28); LID(29); LID(2a); LID(2b); LID(2c); LID(2d); LID(2e); LID(2f);
> +LID(30); LID(31); LID(32); LID(33); LID(34); LID(35); LID(36); LID(37);
> +LID(38); LID(39); LID(3a); LID(3b); LID(3c); LID(3d); LID(3e); LID(3f);

Hi Christoph,

Sparse flags that the structures defined by the above code are not
declared elsewhere, and therefore likely should be static.
> +
> +static struct attribute *lin_sysfs_attrs[] = {
> +	&linid_00.attr, &linid_01.attr, &linid_02.attr, &linid_03.attr,
> +	&linid_04.attr, &linid_05.attr, &linid_06.attr, &linid_07.attr,
> +	&linid_08.attr, &linid_09.attr, &linid_0a.attr, &linid_0b.attr,
> +	&linid_0c.attr, &linid_0d.attr, &linid_0e.attr, &linid_0f.attr,
> +	&linid_10.attr, &linid_11.attr, &linid_12.attr, &linid_13.attr,
> +	&linid_14.attr, &linid_15.attr, &linid_16.attr, &linid_17.attr,
> +	&linid_18.attr, &linid_19.attr, &linid_1a.attr, &linid_1b.attr,
> +	&linid_1c.attr, &linid_1d.attr, &linid_1e.attr, &linid_1f.attr,
> +	&linid_20.attr, &linid_21.attr, &linid_22.attr, &linid_23.attr,
> +	&linid_24.attr, &linid_25.attr, &linid_26.attr, &linid_27.attr,
> +	&linid_28.attr, &linid_29.attr, &linid_2a.attr, &linid_2b.attr,
> +	&linid_2c.attr, &linid_2d.attr, &linid_2e.attr, &linid_2f.attr,
> +	&linid_30.attr, &linid_31.attr, &linid_32.attr, &linid_33.attr,
> +	&linid_34.attr, &linid_35.attr, &linid_36.attr, &linid_37.attr,
> +	&linid_38.attr, &linid_39.attr, &linid_3a.attr, &linid_3b.attr,
> +	&linid_3c.attr, &linid_3d.attr, &linid_3e.attr, &linid_3f.attr,
> +	NULL
> +};

...

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

[Ilpo Järvinen]

On Sat, 11 May 2024, Christoph Fritz wrote:

> ...
> > > diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
> > > new file mode 100644
> > > index 0000000000000..a9ed080b3e33e
> > > --- /dev/null
> > > +++ b/drivers/hid/hid-hexdev-hexlin.c
> > > 
> ...
> > 
> > > +}
> > > +
> > > +#define HEXLIN_GET_CMD(name, enum_cmd)					\
> > > +	static int hexlin_##name(struct hexlin_priv_data *p)		\
> > > +	{								\
> > > +		u8 *req;						\
> > > +		int ret;						\
> > > +									\
> > > +		req = kmalloc(sizeof(*req), GFP_KERNEL)	;		\
> > 
> > Extra space.
> > 
> > Use:
> > 
> > u8 *req __free(kfree) = kmalloc(sizeof(*req), GFP_KERNEL);
> > 
> > > +		if (!req)						\
> > > +			return -ENOMEM;					\
> > > +									\
> > > +		*req = enum_cmd;					\
> > > +									\
> > > +		ret = hexlin_tx_req_status(p, req, sizeof(*req));	\
> > > +		if (ret)						\
> > > +			hid_err(p->hid_dev, "%s failed, error: %d\n",	\
> > > +				#name, ret);				\
> > > +									\
> > > +		kfree(req);						\
> > 
> > Not needed after using __free().
> > 
> > > +		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)
> > 
> > Could you convert the function in the macro into a helper function which 
> > is just called by a simple function with the relevant parameters for 
> > these 3 cases?
> 
> The device actually has a lot more features that I'm using in my debug
> version and which might end up here in the future. So I would like to
> keep it. Any objections?

I don't follow, HEXLIN_GET_CMD() will always produce a copy of that same 
function even if you use it 1000 times?? (Except for the enum and string 
which can easily be passed as arguments to a common function).

You can still keep HEXLIN_GET_CMD() which just calls that common function
if you want to avoid giving those two parameters directly.

> > > +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 = kmalloc(sizeof(*req), GFP_KERNEL);
> > 
> > Hmm... Why do you alloc this small structure (3 bytes?) with kmalloc() and 
> > not just have it in stack as a local variable?
> 
> This buffer must be suitable for DMA (see docu for struct urb).
> 
> So with a stack variable we would need to use kmemdup() before the
> actual sending call, but that's what you did not like since v3 so I
> changed it to this which now you also don't like.
>
> Let's dial it back to the original kmemdup() usage, ok?

I asked a question. Since you have a good reason for alloc, just keep the 
alloc then.

Now I notice it's kmalloc(), why not kzalloc()?

> > > +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> > > +				      const struct hexlin_frame *hxf)
> > > +{
> > > +	struct hid_device *hdev = priv->hid_dev;
> > > +	struct lin_frame lf;
> > > +
> > > +	lf.len = hxf->len;
> > > +	lf.lin_id = hxf->lin_id;
> > > +	memcpy(lf.data, hxf->data, LIN_MAX_DLEN);
> > > +	lf.checksum = hxf->checksum;
> > > +	lf.checksum_mode = hxf->checksum_mode;
> > > +
> > > +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, chk:%02x (%s), flg:%08x\n",
> > > +		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> > > +		lf.checksum_mode ? "enhanced" : "classic", hxf->flags);
> > > +
> > > +	lin_rx(priv->ldev, &lf);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int hexlin_raw_event(struct hid_device *hdev,
> > > +			    struct hid_report *report, u8 *data, int sz)
> > > +{
> > > +	struct hexlin_priv_data *priv;
> > > +	struct hexlin_baudrate_req *br;
> > > +	struct hexlin_responder_answer_req *rar;
> > > +	struct hexlin_unconditional_req *hfr;
> > > +	struct hexlin_val8_req *vr;
> > > +
> > > +	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 != HEXLIN_LEN_RETCODE)
> > > +			return -EREMOTEIO;
> > > +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> > > +		complete(&priv->wait_in_report);
> > > +		break;
> > > +	case HEXLIN_FAIL:
> > > +		if (sz != HEXLIN_LEN_RETCODE)
> > > +			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 != sizeof(*vr))
> > > +			return -EREMOTEIO;
> > > +		vr = (struct hexlin_val8_req *) data;
> > > +		priv->fw_version = vr->v;
> > > +		complete(&priv->wait_in_report);
> > > +		break;
> > > +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> > > +		if (sz != sizeof(*rar))
> > > +			return -EREMOTEIO;
> > > +		rar = (struct hexlin_responder_answer_req *) data;
> > > +		memcpy(&priv->answ, &rar->answ, sizeof(priv->answ));
> > > +		complete(&priv->wait_in_report);
> > > +		break;
> > > +	case HEXLIN_GET_BAUDRATE:
> > > +		if (sz != sizeof(*br))
> > > +			return -EREMOTEIO;
> > > +		br = (struct hexlin_baudrate_req *) data;
> > > +		le16_to_cpus(br->baudrate);
> > > +		priv->baudrate = br->baudrate;
> > > +		complete(&priv->wait_in_report);
> > > +		break;
> > > +	/* following cases not initiated by us, so no complete() */
> > > +	case HEXLIN_FRAME:
> > > +		if (sz != sizeof(*hfr)) {
> > > +			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
> > > +			return -EREMOTEIO;
> > > +		}
> > > +		hfr = (struct hexlin_unconditional_req *) data;
> > > +		le32_to_cpus(hfr->frm.flags);
> > 
> > I'm bit worried about this from endianness perspective. Perhaps there's 
> > some struct reusing that shouldn't be happening because the same field 
> > cannot be __le32 and u32 at the same time.
> 
> Can you propose a solution?

Just do a le32_to_cpu(hfr->frm.flags) in the debug print's arguments?

-- 
 i.

Re: [PATCH v4 04/11] tty: serdev: Add method to enable break flags

[Ilpo Järvinen]

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

On Sun, 12 May 2024, Christoph Fritz wrote:

> On Fri, 2024-05-10 at 17:21 +0300, Ilpo Järvinen wrote:
> > On Thu, 9 May 2024, Christoph Fritz wrote:
> ...
> > > 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;
> > 
> > Why you need to test for !ctrl?
> 
> In our case we don't, it's an extra check like all the other functions
> here:
> 
> https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/tty/serdev/core.c#L330

Okay. It might be I just don't understand all the structs in serdev well 
enough to understand when ctrl actually end ups being NULL so I don't 
object leaving it as is.

-- 
 i.

Re: [PATCH v4 00/11] LIN Bus support for Linux

[Christoph Fritz]

> This series is introducing basic Local Interconnect Network (LIN)
> (ISO 17987) [0] support

I just wanted to let you know that I'm unfortunately personally busy at
the moment. I will resume addressing the remaining issues with v5
sometime after the current merge window.

thanks
  -- Christoph