Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 2/5] Documentation: hid: intel-ish-hid: add section for firmware loading
From: Zhang Lixu @ 2024-05-06  1:30 UTC (permalink / raw)
  To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20240506013040.10700-1-lixu.zhang@intel.com>

From: Qianru Huang <qianru.huang@intel.com>

Add a section to describe the ISH firmware loading process for Lunar Lake
and later generations.

Signed-off-by: Qianru Huang <qianru.huang@intel.com>
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 Documentation/hid/intel-ish-hid.rst | 65 +++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/Documentation/hid/intel-ish-hid.rst b/Documentation/hid/intel-ish-hid.rst
index 12613cb2be43..55cbaa719a79 100644
--- a/Documentation/hid/intel-ish-hid.rst
+++ b/Documentation/hid/intel-ish-hid.rst
@@ -339,6 +339,71 @@ End to End HID transport Sequence Diagram
           |                        |                       |                               |
 
 
+ISH Firmware Loading from Host Flow
+-----------------------------------
+
+Starting from the Lunar Lake generation, the ISH firmware has been divided into two components for better space optimization and increased flexibility. These components include a bootloader that is integrated into the BIOS, and a main firmware that is stored within the operating system's file system.
+
+The process works as follows:
+
+- Initially, the ISHTP driver sends a command, HOST_START_REQ_CMD, to the ISH bootloader. In response, the bootloader sends back a HOST_START_RES_CMD. This response includes the ISHTP_SUPPORT_CAP_LOADER bit. Subsequently, the ISHTP driver checks if this bit is set. If it is, the firmware loading process from the host begins.
+
+- During this process, the ISHTP driver first invokes the request_firmware() function, followed by sending a LOADER_CMD_XFER_QUERY command. Upon receiving a response from the bootloader, the ISHTP driver sends a LOADER_CMD_XFER_FRAGMENT command. After receiving another response, the ISHTP driver sends a LOADER_CMD_START command. The bootloader responds and then proceeds to the Main Firmware.
+
+- After the process concludes, the ISHTP driver calls the release_firmware() function.
+
+For more detailed information, please refer to the flow descriptions provided below:
+
+::
+
+  +---------------+                                                    +-----------------+
+  | ISHTP Driver  |                                                    | ISH Bootloader  |
+  +---------------+                                                    +-----------------+
+          |                                                                     |
+          |~~~Send HOST_START_REQ_CMD~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>|
+          |                                                                     |
+          |<--Send HOST_START_RES_CMD(Includes ISHTP_SUPPORT_CAP_LOADER bit)----|
+          |                                                                     |
+  ****************************************************************************************
+  * if ISHTP_SUPPORT_CAP_LOADER bit is set                                               *
+  ****************************************************************************************
+          |                                                                     |
+          |~~~start loading firmware from host process~~~+                      |
+          |                                              |                      |
+          |<---------------------------------------------+                      |
+          |                                                                     |
+  ---------------------------                                                   |
+  | Call request_firmware() |                                                   |
+  ---------------------------                                                   |
+          |                                                                     |
+          |~~~Send LOADER_CMD_XFER_QUERY~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>|
+          |                                                                     |
+          |<--Send response-----------------------------------------------------|
+          |                                                                     |
+          |~~~Send LOADER_CMD_XFER_FRAGMENT~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>|
+          |                                                                     |
+          |<--Send response-----------------------------------------------------|
+          |                                                                     |
+          |~~~Send LOADER_CMD_START~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>|
+          |                                                                     |
+          |<--Send response-----------------------------------------------------|
+          |                                                                     |
+          |                                                                     |~~~Jump to Main Firmware~~~+
+          |                                                                     |                           |
+          |                                                                     |<--------------------------+
+          |                                                                     |
+  ---------------------------                                                   |
+  | Call release_firmware() |                                                   |
+  ---------------------------                                                   |
+          |                                                                     |
+  ****************************************************************************************
+  * end if                                                                               *
+  ****************************************************************************************
+          |                                                                     |
+  +---------------+                                                    +-----------------+
+  | ISHTP Driver  |                                                    | ISH Bootloader  |
+  +---------------+                                                    +-----------------+
+
 ISH Debugging
 -------------
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH 3/5] HID: intel-ish-hid: Add driver_data for specifying the firmware filename
From: Zhang Lixu @ 2024-05-06  1:30 UTC (permalink / raw)
  To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20240506013040.10700-1-lixu.zhang@intel.com>

Introduces a new structure, ishtp_driver_data, to hold driver-specific
data, including the firmware filename for different hardware variants of
the Intel Integrated Sensor Hub (ISH).

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c     | 18 +++++++++++++++++-
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 17 +++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index e79d72f7db2a..d487227085b2 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -23,6 +23,19 @@
 #include "ishtp-dev.h"
 #include "hw-ish.h"
 
+enum ishtp_driver_data_index {
+	ISHTP_DRIVER_DATA_NONE,
+	ISHTP_DRIVER_DATA_LNL_M,
+};
+
+#define ISH_FW_FILENAME_LNL_M "intel/ish/ish_lnlm.bin"
+
+static struct ishtp_driver_data ishtp_driver_data[] = {
+	[ISHTP_DRIVER_DATA_LNL_M] = {
+		.fw_filename = ISH_FW_FILENAME_LNL_M,
+	},
+};
+
 static const struct pci_device_id ish_pci_tbl[] = {
 	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_CHV)},
 	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_BXT_Ax)},
@@ -46,7 +59,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
 	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_MTL_P)},
 	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_ARL_H)},
 	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_ARL_S)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_LNL_M)},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ISH_LNL_M), .driver_data = ISHTP_DRIVER_DATA_LNL_M},
 	{}
 };
 MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
@@ -167,6 +180,7 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	hw = to_ish_hw(ishtp);
 	ishtp->print_log = ish_event_tracer;
+	ishtp->driver_data = &ishtp_driver_data[ent->driver_data];
 
 	/* mapping IO device memory */
 	hw->mem_addr = pcim_iomap_table(pdev)[0];
@@ -377,3 +391,5 @@ MODULE_AUTHOR("Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>");
 
 MODULE_DESCRIPTION("Intel(R) Integrated Sensor Hub PCI Device Driver");
 MODULE_LICENSE("GPL");
+
+MODULE_FIRMWARE(ISH_FW_FILENAME_LNL_M);
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index 32142c7d9a04..ed294bf0bc8f 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -122,12 +122,29 @@ struct ishtp_hw_ops {
 	bool	(*dma_no_cache_snooping)(struct ishtp_device *dev);
 };
 
+/**
+ * struct ishtp_driver_data - Driver-specific data for ISHTP devices
+ *
+ * This structure holds driver-specific data that can be associated with each
+ * ISHTP device instance. It allows for the storage of data that is unique to
+ * a particular driver or hardware variant.
+ *
+ * @fw_filename: The firmware filename associated with a specific hardware
+ *               variant of the Intel Integrated Sensor Hub (ISH). This allows
+ *               the driver to load the correct firmware based on the device's
+ *               hardware variant.
+ */
+struct ishtp_driver_data {
+	char *fw_filename;
+};
+
 /**
  * struct ishtp_device - ISHTP private device struct
  */
 struct ishtp_device {
 	struct device *devc;	/* pointer to lowest device */
 	struct pci_dev *pdev;	/* PCI device to get device ids */
+	struct ishtp_driver_data *driver_data; /* pointer to driver-specific data */
 
 	/* waitq for waiting for suspend response */
 	wait_queue_head_t suspend_wait;
-- 
2.34.1


^ permalink raw reply related

* [PATCH 4/5] HID: intel-ish-hid: Implement loading firmware from host feature
From: Zhang Lixu @ 2024-05-06  1:30 UTC (permalink / raw)
  To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20240506013040.10700-1-lixu.zhang@intel.com>

Starting from the Lunar Lake generation, the ISH firmware has been
divided into two components for better space optimization and increased
flexibility. These components include a bootloader that is integrated
into the BIOS, and a main firmware that is stored within the operating
system's file system.

Introduce support for loading ISH main firmware from host. This feature is
applicable for Lunar Lake and later generation.

Current intel-ishtp-loader, is designed for Chrome OS based systems which
uses core boot and has different firmware loading method. For non chrome
systems the ISH firmware loading uses different method.

Key differences include:
1. The new method utilizes ISHTP capability/fixed client to enumerate the
firmware loader function. It does not require a connection or flow control,
unlike the method used in Chrome OS, which is enumerated as an ISHTP
dynamic client driver, necessitating connect/disconnect operations and flow
control.

2. The new method employs a table to describe firmware fragments, which are
sent to ISH in a single operation. Conversely, the Chrome OS method sends
firmware fragments in multiple operations within a loop, sending only one
fragment at a time.

Additionally, address potential error scenarios to ensure graceful failure
handling.
- Firmware Not Found: Triggers if request_firmware() fails, leaving ISH in
  a waiting state.
  Recovery: Re-insmod the ISH drivers to retry.

- DMA Buffer Allocation Failure: Occurs during prepare_dma_bufs(), leading
  to ISH waiting state. Allocated resources are released.
  Recovery: Re-insmod the ISH drivers to retry.

- Incorrect Firmware Image: Causes ISH to refuse loading after three failed
  attempts.
  Recovery: A platform reset is required.

Please refer to the [Documentation](Documentation/hid/intel-ish-hid.rst)
for the details on flows.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/Makefile          |   1 +
 drivers/hid/intel-ish-hid/ishtp/hbm.c       |  21 ++
 drivers/hid/intel-ish-hid/ishtp/init.c      |   8 +
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h |  11 +
 drivers/hid/intel-ish-hid/ishtp/loader.c    | 275 ++++++++++++++++++++
 drivers/hid/intel-ish-hid/ishtp/loader.h    | 226 ++++++++++++++++
 6 files changed, 542 insertions(+)
 create mode 100644 drivers/hid/intel-ish-hid/ishtp/loader.c
 create mode 100644 drivers/hid/intel-ish-hid/ishtp/loader.h

diff --git a/drivers/hid/intel-ish-hid/Makefile b/drivers/hid/intel-ish-hid/Makefile
index f0a82b1c7cb9..a927b224cd44 100644
--- a/drivers/hid/intel-ish-hid/Makefile
+++ b/drivers/hid/intel-ish-hid/Makefile
@@ -11,6 +11,7 @@ intel-ishtp-objs += ishtp/client.o
 intel-ishtp-objs += ishtp/bus.o
 intel-ishtp-objs += ishtp/dma-if.o
 intel-ishtp-objs += ishtp/client-buffers.o
+intel-ishtp-objs += ishtp/loader.o
 
 obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-ipc.o
 intel-ish-ipc-objs := ipc/ipc.o
diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.c b/drivers/hid/intel-ish-hid/ishtp/hbm.c
index 9c031a06e4c4..8ee5467127d8 100644
--- a/drivers/hid/intel-ish-hid/ishtp/hbm.c
+++ b/drivers/hid/intel-ish-hid/ishtp/hbm.c
@@ -13,6 +13,7 @@
 #include "ishtp-dev.h"
 #include "hbm.h"
 #include "client.h"
+#include "loader.h"
 
 /**
  * ishtp_hbm_fw_cl_allocate() - Allocate FW clients
@@ -570,6 +571,10 @@ void ishtp_hbm_dispatch(struct ishtp_device *dev,
 			return;
 		}
 
+		/* Start firmware loading process if it has loader capability */
+		if (version_res->host_version_supported & ISHTP_SUPPORT_CAP_LOADER)
+			schedule_work(&dev->work_fw_loader);
+
 		dev->version.major_version = HBM_MAJOR_VERSION;
 		dev->version.minor_version = HBM_MINOR_VERSION;
 		if (dev->dev_state == ISHTP_DEV_INIT_CLIENTS &&
@@ -864,6 +869,20 @@ void	recv_hbm(struct ishtp_device *dev, struct ishtp_msg_hdr *ishtp_hdr)
 	return;
 }
 
+/**
+ * ishtp_loader_recv_msg() - Receive a message from the ISHTP device
+ * @dev: The ISHTP device
+ * @buf: The buffer containing the message
+ */
+static void ishtp_loader_recv_msg(struct ishtp_device *dev, void *buf)
+{
+	if (dev->fw_loader_rx_buf)
+		memcpy(dev->fw_loader_rx_buf, buf, dev->fw_loader_rx_size);
+
+	dev->fw_loader_received = true;
+	wake_up_interruptible(&dev->wait_loader_recvd_msg);
+}
+
 /**
  * recv_fixed_cl_msg() - Receive fixed client message
  * @dev: ISHTP device instance
@@ -890,6 +909,8 @@ void recv_fixed_cl_msg(struct ishtp_device *dev,
 		else
 			dev_err(dev->devc, "unknown fixed client msg [%02X]\n",
 				msg_hdr->cmd);
+	} else if (ishtp_hdr->fw_addr == ISHTP_LOADER_CLIENT_ADDR) {
+		ishtp_loader_recv_msg(dev, rd_msg_buf);
 	}
 }
 
diff --git a/drivers/hid/intel-ish-hid/ishtp/init.c b/drivers/hid/intel-ish-hid/ishtp/init.c
index 02a00cc2dd11..07fdd52e4c5e 100644
--- a/drivers/hid/intel-ish-hid/ishtp/init.c
+++ b/drivers/hid/intel-ish-hid/ishtp/init.c
@@ -5,12 +5,14 @@
  * Copyright (c) 2003-2016, Intel Corporation.
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include "ishtp-dev.h"
 #include "hbm.h"
 #include "client.h"
+#include "loader.h"
 
 /**
  * ishtp_dev_state_str() -Convert to string format
@@ -51,6 +53,8 @@ const char *ishtp_dev_state_str(int state)
  */
 void ishtp_device_init(struct ishtp_device *dev)
 {
+	int ret;
+
 	dev->dev_state = ISHTP_DEV_INITIALIZING;
 	INIT_LIST_HEAD(&dev->cl_list);
 	INIT_LIST_HEAD(&dev->device_list);
@@ -59,6 +63,7 @@ void ishtp_device_init(struct ishtp_device *dev)
 	spin_lock_init(&dev->rd_msg_spinlock);
 
 	init_waitqueue_head(&dev->wait_hbm_recvd_msg);
+	init_waitqueue_head(&dev->wait_loader_recvd_msg);
 	spin_lock_init(&dev->read_list_spinlock);
 	spin_lock_init(&dev->device_lock);
 	spin_lock_init(&dev->device_list_lock);
@@ -76,6 +81,9 @@ void ishtp_device_init(struct ishtp_device *dev)
 
 	INIT_LIST_HEAD(&dev->read_list.list);
 
+	ret = devm_work_autocancel(dev->devc, &dev->work_fw_loader, ishtp_loader_work);
+	if (ret)
+		dev_err_probe(dev->devc, ret, "Failed to initialise FW loader work\n");
 }
 EXPORT_SYMBOL(ishtp_device_init);
 
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index ed294bf0bc8f..181838c3d7ac 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -164,6 +164,17 @@ struct ishtp_device {
 	struct hbm_version version;
 	int transfer_path; /* Choice of transfer path: IPC or DMA */
 
+	/* work structure for scheduling firmware loading tasks */
+	struct work_struct work_fw_loader;
+	/* waitq for waiting for command response from the firmware loader */
+	wait_queue_head_t wait_loader_recvd_msg;
+	/* indicating whether a message from the firmware loader has been received */
+	bool fw_loader_received;
+	/* pointer to a buffer for receiving messages from the firmware loader */
+	void *fw_loader_rx_buf;
+	/* size of the buffer pointed to by fw_loader_rx_buf */
+	int fw_loader_rx_size;
+
 	/* ishtp device states */
 	enum ishtp_dev_state dev_state;
 	enum ishtp_hbm_state hbm_state;
diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-hid/ishtp/loader.c
new file mode 100644
index 000000000000..993f8b390e57
--- /dev/null
+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ISHTP firmware loader function
+ *
+ * Copyright (c) 2024, Intel Corporation.
+ *
+ * This module implements the functionality to load the main ISH firmware from the host, starting
+ * with the Lunar Lake generation. It leverages a new method that enhances space optimization and
+ * flexibility by dividing the ISH firmware into a bootloader and main firmware.
+ *
+ * Please refer to the [Documentation](Documentation/hid/intel-ish-hid.rst) for the details on
+ * flows.
+ *
+ * Additionally, address potential error scenarios to ensure graceful failure handling.
+ * - Firmware Image Not Found:
+ *   Occurs when `request_firmware()` cannot locate the firmware image. The ISH firmware will
+ *   remain in a state awaiting firmware loading from the host, with no further action from
+ *   the ISHTP driver.
+ *   Recovery: Re-insmod the ISH drivers allows for a retry of the firmware loading from the host.
+ *
+ * - DMA Buffer Allocation Failure:
+ *   This happens if allocating a DMA buffer during `prepare_dma_bufs()` fails. The ISH firmware
+ *   will stay in a waiting state, and the ISHTP driver will release any allocated DMA buffers and
+ *   firmware without further actions.
+ *   Recovery: Re-insmod the ISH drivers allows for a retry of the firmware loading from the host.
+ *
+ * - Incorrect Firmware Image:
+ *   Using an incorrect firmware image will initiate the firmware loading process but will
+ *   eventually be refused by the ISH firmware after three unsuccessful attempts, indicated by
+ *   returning an error code. The ISHTP driver will stop attempting after three tries.
+ *   Recovery: A platform reset is required to retry firmware loading from the host.
+ */
+
+#define dev_fmt(fmt) "ISH loader: " fmt
+
+#include <linux/cacheflush.h>
+#include <linux/container_of.h>
+#include <linux/dev_printk.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/firmware.h>
+#include <linux/gfp_types.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/pfn.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+
+#include "hbm.h"
+#include "loader.h"
+
+/**
+ * loader_write_message() - Write a message to the ISHTP device
+ * @dev: The ISHTP device
+ * @buf: The buffer containing the message
+ * @len: The length of the message
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int loader_write_message(struct ishtp_device *dev, void *buf, int len)
+{
+	struct ishtp_msg_hdr ishtp_hdr = {
+		.fw_addr = ISHTP_LOADER_CLIENT_ADDR,
+		.length = len,
+		.msg_complete = 1,
+	};
+
+	dev->fw_loader_received = false;
+
+	return ishtp_write_message(dev, &ishtp_hdr, buf);
+}
+
+/**
+ * loader_xfer_cmd() - Transfer a command to the ISHTP device
+ * @dev: The ISHTP device
+ * @req: The request buffer
+ * @req_len: The length of the request
+ * @resp: The response buffer
+ * @resp_len: The length of the response
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int loader_xfer_cmd(struct ishtp_device *dev, void *req, int req_len,
+			   void *resp, int resp_len)
+{
+	struct loader_msg_header *req_hdr = req;
+	struct loader_msg_header *resp_hdr = resp;
+	struct device *devc = dev->devc;
+	int rv;
+
+	dev->fw_loader_rx_buf = resp;
+	dev->fw_loader_rx_size = resp_len;
+
+	rv = loader_write_message(dev, req, req_len);
+	if (rv < 0) {
+		dev_err(devc, "write cmd %u failed:%d\n", req_hdr->command, rv);
+		return rv;
+	}
+
+	/* Wait the ACK */
+	wait_event_interruptible_timeout(dev->wait_loader_recvd_msg, dev->fw_loader_received,
+					 ISHTP_LOADER_TIMEOUT);
+	dev->fw_loader_rx_size = 0;
+	dev->fw_loader_rx_buf = NULL;
+	if (!dev->fw_loader_received) {
+		dev_err(devc, "wait response of cmd %u timeout\n", req_hdr->command);
+		return -ETIMEDOUT;
+	}
+
+	if (!resp_hdr->is_response) {
+		dev_err(devc, "not a response for %u\n", req_hdr->command);
+		return -EBADMSG;
+	}
+
+	if (req_hdr->command != resp_hdr->command) {
+		dev_err(devc, "unexpected cmd response %u:%u\n", req_hdr->command,
+			resp_hdr->command);
+		return -EBADMSG;
+	}
+
+	if (resp_hdr->status) {
+		dev_err(devc, "cmd %u failed %u\n", req_hdr->command, resp_hdr->status);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/**
+ * release_dma_bufs() - Release the DMA buffer for transferring firmware fragments
+ * @dev: The ISHTP device
+ * @fragment: The ISHTP firmware fragment descriptor
+ * @dma_bufs: The array of DMA fragment buffers
+ * @fragment_size: The size of a single DMA fragment
+ */
+static void release_dma_bufs(struct ishtp_device *dev,
+			     struct loader_xfer_dma_fragment *fragment,
+			     void **dma_bufs, u32 fragment_size)
+{
+	int i;
+
+	for (i = 0; i < FRAGMENT_MAX_NUM; i++) {
+		if (dma_bufs[i]) {
+			dma_free_coherent(dev->devc, fragment_size, dma_bufs[i],
+					  fragment->fragment_tbl[i].ddr_adrs);
+			dma_bufs[i] = NULL;
+		}
+	}
+}
+
+/**
+ * prepare_dma_bufs() - Prepare the DMA buffer for transferring firmware fragments
+ * @dev: The ISHTP device
+ * @ish_fw: The ISH firmware
+ * @fragment: The ISHTP firmware fragment descriptor
+ * @dma_bufs: The array of DMA fragment buffers
+ * @fragment_size: The size of a single DMA fragment
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int prepare_dma_bufs(struct ishtp_device *dev,
+			    const struct firmware *ish_fw,
+			    struct loader_xfer_dma_fragment *fragment,
+			    void **dma_bufs, u32 fragment_size)
+{
+	u32 offset = 0;
+	int i;
+
+	for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
+		dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
+						 &fragment->fragment_tbl[i].ddr_adrs, GFP_KERNEL);
+		if (!dma_bufs[i])
+			return -ENOMEM;
+
+		fragment->fragment_tbl[i].length = clamp(ish_fw->size - offset, 0, fragment_size);
+		fragment->fragment_tbl[i].fw_off = offset;
+		memcpy(dma_bufs[i], ish_fw->data + offset, fragment->fragment_tbl[i].length);
+		clflush_cache_range(dma_bufs[i], fragment_size);
+
+		offset += fragment->fragment_tbl[i].length;
+	}
+
+	return 0;
+}
+
+/**
+ * ishtp_loader_work() - Load the ISHTP firmware
+ * @work: The work structure
+ *
+ * The ISH Loader attempts to load firmware by sending a series of commands
+ * to the ISH device. If a command fails to be acknowledged by the ISH device,
+ * the loader will retry sending the command, up to a maximum of
+ * ISHTP_LOADER_RETRY_TIMES.
+ *
+ * After the maximum number of retries has been reached without success, the
+ * ISH bootloader will return an error status code and will no longer respond
+ * to the driver's commands. This behavior indicates that the ISH Loader has
+ * encountered a critical error during the firmware loading process.
+ *
+ * In such a case, where the ISH bootloader is unresponsive after all retries
+ * have been exhausted, a platform reset is required to restore communication
+ * with the ISH device and to recover from this error state.
+ */
+void ishtp_loader_work(struct work_struct *work)
+{
+	DEFINE_RAW_FLEX(struct loader_xfer_dma_fragment, fragment, fragment_tbl, FRAGMENT_MAX_NUM);
+	struct ishtp_device *dev = container_of(work, struct ishtp_device, work_fw_loader);
+	struct loader_xfer_query query = {
+		.header.command = LOADER_CMD_XFER_QUERY,
+	};
+	struct loader_start start = {
+		.header.command = LOADER_CMD_START,
+	};
+	union loader_recv_message recv_msg;
+	char *filename = dev->driver_data->fw_filename;
+	const struct firmware *ish_fw;
+	void *dma_bufs[FRAGMENT_MAX_NUM] = {};
+	u32 fragment_size;
+	int retry = ISHTP_LOADER_RETRY_TIMES;
+	int rv;
+
+	rv = request_firmware(&ish_fw, filename, dev->devc);
+	if (rv < 0) {
+		dev_err(dev->devc, "request firmware %s failed:%d\n", filename, rv);
+		return;
+	}
+
+	fragment->fragment.header.command = LOADER_CMD_XFER_FRAGMENT;
+	fragment->fragment.xfer_mode = LOADER_XFER_MODE_DMA;
+	fragment->fragment.is_last = 1;
+	fragment->fragment.size = ish_fw->size;
+	/* Calculate the size of a single DMA fragment */
+	fragment_size = PFN_ALIGN(DIV_ROUND_UP(ish_fw->size, FRAGMENT_MAX_NUM));
+	/* Calculate the count of DMA fragments */
+	fragment->fragment_cnt = DIV_ROUND_UP(ish_fw->size, fragment_size);
+
+	rv = prepare_dma_bufs(dev, ish_fw, fragment, dma_bufs, fragment_size);
+	if (rv) {
+		dev_err(dev->devc, "prepare DMA buffer failed.\n");
+		goto out;
+	}
+
+	do {
+		query.image_size = ish_fw->size;
+		rv = loader_xfer_cmd(dev, &query, sizeof(query), recv_msg.raw_data,
+				     sizeof(struct loader_xfer_query_ack));
+		if (rv)
+			continue; /* try again if failed */
+
+		dev_dbg(dev->devc, "ISH Version %u.%u.%u.%u\n",
+			recv_msg.query_ack.version_major,
+			recv_msg.query_ack.version_minor,
+			recv_msg.query_ack.version_hotfix,
+			recv_msg.query_ack.version_build);
+
+		rv = loader_xfer_cmd(dev, fragment,
+				     struct_size(fragment, fragment_tbl, fragment->fragment_cnt),
+				     recv_msg.raw_data, sizeof(struct loader_xfer_fragment_ack));
+		if (rv)
+			continue; /* try again if failed */
+
+		rv = loader_xfer_cmd(dev, &start, sizeof(start), recv_msg.raw_data,
+				     sizeof(struct loader_start_ack));
+		if (rv)
+			continue; /* try again if failed */
+
+		dev_info(dev->devc, "firmware loaded. size:%zu\n", ish_fw->size);
+		break;
+	} while (--retry);
+
+out:
+	release_dma_bufs(dev, fragment, dma_bufs, fragment_size);
+	release_firmware(ish_fw);
+}
diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.h b/drivers/hid/intel-ish-hid/ishtp/loader.h
new file mode 100644
index 000000000000..7aa45ebc3f7b
--- /dev/null
+++ b/drivers/hid/intel-ish-hid/ishtp/loader.h
@@ -0,0 +1,226 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ISHTP firmware loader header
+ *
+ * Copyright (c) 2024, Intel Corporation.
+ */
+
+#ifndef _ISHTP_LOADER_H_
+#define _ISHTP_LOADER_H_
+
+#include <linux/bits.h>
+#include <linux/jiffies.h>
+#include <linux/types.h>
+
+#include "ishtp-dev.h"
+
+struct work_struct;
+
+#define LOADER_MSG_SIZE \
+	(IPC_PAYLOAD_SIZE - sizeof(struct ishtp_msg_hdr))
+
+/*
+ * ISHTP firmware loader protocol definition
+ */
+#define LOADER_CMD_XFER_QUERY		0	/* SW -> FW */
+#define LOADER_CMD_XFER_FRAGMENT	1	/* SW -> FW */
+#define LOADER_CMD_START		2	/* SW -> FW */
+
+/* Only support DMA mode */
+#define LOADER_XFER_MODE_DMA BIT(0)
+
+/**
+ * struct loader_msg_header - ISHTP firmware loader message header
+ * @command: Command type
+ * @is_response: Indicates if the message is a response
+ * @has_next: Indicates if there is a next message
+ * @reserved: Reserved for future use
+ * @status: Status of the message
+ */
+struct loader_msg_header {
+	__le32 command:7;
+	__le32 is_response:1;
+	__le32 has_next:1;
+	__le32 reserved:15;
+	__le32 status:8;
+};
+
+/**
+ * struct loader_xfer_query - ISHTP firmware loader transfer query packet
+ * @header: Header of the message
+ * @image_size: Size of the image
+ */
+struct loader_xfer_query {
+	struct loader_msg_header header;
+	__le32 image_size;
+};
+
+/**
+ * struct loader_version - ISHTP firmware loader version
+ * @value: Value of the version
+ * @major: Major version
+ * @minor: Minor version
+ * @hotfix: Hotfix version
+ * @build: Build version
+ */
+struct loader_version {
+	union {
+		__le32 value;
+		struct {
+			__u8 major;
+			__u8 minor;
+			__u8 hotfix;
+			__u8 build;
+		};
+	};
+};
+
+/**
+ * struct loader_capability - ISHTP firmware loader capability
+ * @max_fw_image_size: Maximum firmware image size
+ * @support_mode: Support mode
+ * @reserved: Reserved for future use
+ * @platform: Platform
+ * @max_dma_buf_size: Maximum DMA buffer size, multiples of 4096
+ */
+struct loader_capability {
+	__le32 max_fw_image_size;
+	__le16 support_mode;
+	__u8 reserved;
+	__u8 platform;
+	__le32 max_dma_buf_size;
+};
+
+/**
+ * struct loader_xfer_query_ack - ISHTP firmware loader transfer query acknowledgment
+ * @header: Header of the message
+ * @version_major: ISH Major version
+ * @version_minor: ISH Minor version
+ * @version_hotfix: ISH Hotfix version
+ * @version_build: ISH Build version
+ * @protocol_version: Protocol version
+ * @loader_version: Loader version
+ * @capability: Loader capability
+ */
+struct loader_xfer_query_ack {
+	struct loader_msg_header header;
+	__le16 version_major;
+	__le16 version_minor;
+	__le16 version_hotfix;
+	__le16 version_build;
+	__le32 protocol_version;
+	struct loader_version loader_version;
+	struct loader_capability capability;
+};
+
+/**
+ * struct loader_xfer_fragment - ISHTP firmware loader transfer fragment
+ * @header: Header of the message
+ * @xfer_mode: Transfer mode
+ * @offset: Offset
+ * @size: Size
+ * @is_last: Is last
+ */
+struct loader_xfer_fragment {
+	struct loader_msg_header header;
+	__le32 xfer_mode;
+	__le32 offset;
+	__le32 size;
+	__le32 is_last;
+};
+
+/**
+ * struct loader_xfer_fragment_ack - ISHTP firmware loader transfer fragment acknowledgment
+ * @header: Header of the message
+ */
+struct loader_xfer_fragment_ack {
+	struct loader_msg_header header;
+};
+
+/**
+ * struct fragment_dscrpt - ISHTP firmware loader fragment descriptor
+ * @ddr_adrs: The address in host DDR
+ * @fw_off: The offset of the fragment in the fw image
+ * @length: The length of the fragment
+ */
+struct fragment_dscrpt {
+	__le64 ddr_adrs;
+	__le32 fw_off;
+	__le32 length;
+};
+
+#define FRAGMENT_MAX_NUM \
+	((LOADER_MSG_SIZE - sizeof(struct loader_xfer_dma_fragment)) / \
+	 sizeof(struct fragment_dscrpt))
+
+/**
+ * struct loader_xfer_dma_fragment - ISHTP firmware loader transfer DMA fragment
+ * @fragment: Fragment
+ * @fragment_cnt: How many descriptors in the fragment_tbl
+ * @fragment_tbl: Fragment table
+ */
+struct loader_xfer_dma_fragment {
+	struct loader_xfer_fragment fragment;
+	__le32 fragment_cnt;
+	struct fragment_dscrpt fragment_tbl[] __counted_by(fragment_cnt);
+};
+
+/**
+ * struct loader_start - ISHTP firmware loader start
+ * @header: Header of the message
+ */
+struct loader_start {
+	struct loader_msg_header header;
+};
+
+/**
+ * struct loader_start_ack - ISHTP firmware loader start acknowledgment
+ * @header: Header of the message
+ */
+struct loader_start_ack {
+	struct loader_msg_header header;
+};
+
+union loader_recv_message {
+	struct loader_xfer_query_ack query_ack;
+	struct loader_xfer_fragment_ack fragment_ack;
+	struct loader_start_ack start_ack;
+	__u8 raw_data[LOADER_MSG_SIZE];
+};
+
+/*
+ * ISHTP firmware loader internal use
+ */
+/* ISHTP firmware loader command timeout */
+#define ISHTP_LOADER_TIMEOUT msecs_to_jiffies(100)
+
+/* ISHTP firmware loader retry times */
+#define ISHTP_LOADER_RETRY_TIMES 3
+
+/**
+ * struct ish_firmware_variant - ISH firmware variant
+ * @device: PCI Device ID
+ * @filename: The firmware file name
+ */
+struct ish_firmware_variant {
+	unsigned short device;
+	const char *filename;
+};
+
+/*
+ * ISHTP firmware loader API for ISHTP hbm
+ */
+
+/* ISHTP capability bit for firmware loader */
+#define ISHTP_SUPPORT_CAP_LOADER BIT(4)
+
+/* Firmware loader address */
+#define ISHTP_LOADER_CLIENT_ADDR 16
+
+/**
+ * ishtp_loader_work - The work function to start the firmware loading process
+ * @work: The work structure
+ */
+void ishtp_loader_work(struct work_struct *work);
+
+#endif /* _ISHTP_LOADER_H_ */
-- 
2.34.1


^ permalink raw reply related

* [PATCH 5/5] HID: intel-ish-hid: handler multiple MNG_RESET_NOTIFY messages
From: Zhang Lixu @ 2024-05-06  1:30 UTC (permalink / raw)
  To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20240506013040.10700-1-lixu.zhang@intel.com>

This patch enhances the firmware reset handler in the Intel Integrated
Sensor Hub (ISH) driver. Previously, the ISH firmware would send a
MNG_RESET_NOTIFY message in response to an empty IPC message from the
ish_wakeup function. With the introduction of the feature to load ISH
firmware from the host on the LunarLake platform, the ISH bootloader
now involves the IPC function. This results in an additional
MNG_RESET_NOTIFY message being sent by ISH bootloader after power on.
Consequently, the driver receives two MNG_RESET_NOTIFY messages during
system boot up. This can disrupt the dev->dev_state during the first
reset flow due to the subsequent reset notify message.

To address this, the patch modifies the fw_reset_work_fn function to skip
the execution of ishtp_reset_compl_handler during the first reset flow if
a reset is pending. The ishtp_reset_compl_handler will then be executed
during the second reset flow, ensuring the dev->dev_state is not disrupted.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index adce30f8ebff..3cd53fc80634 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -546,11 +546,11 @@ static int ish_fw_reset_handler(struct ishtp_device *dev)
 
 /**
  * fw_reset_work_fn() - FW reset worker function
- * @unused: not used
+ * @work: Work item
  *
  * Call ish_fw_reset_handler to complete FW reset
  */
-static void fw_reset_work_fn(struct work_struct *unused)
+static void fw_reset_work_fn(struct work_struct *work)
 {
 	int	rv;
 
@@ -562,7 +562,8 @@ static void fw_reset_work_fn(struct work_struct *unused)
 		wake_up_interruptible(&ishtp_dev->wait_hw_ready);
 
 		/* ISHTP notification in IPC_RESET sequence completion */
-		ishtp_reset_compl_handler(ishtp_dev);
+		if (!work_pending(work))
+			ishtp_reset_compl_handler(ishtp_dev);
 	} else
 		dev_err(ishtp_dev->devc, "[ishtp-ish]: FW reset failed (%d)\n",
 			rv);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2 1/3] dt-bindings: touchscreen: convert elan,ektf2127 to json-schema
From: Krzysztof Kozlowski @ 2024-05-06  6:24 UTC (permalink / raw)
  To: Andreas Kemnade, dmitry.torokhov, robh, krzk+dt, conor+dt,
	u.kleine-koenig, hdegoede, andy.shevchenko, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20240505214754.891700-2-andreas@kemnade.info>

On 05/05/2024 23:47, Andreas Kemnade wrote:
> Convert EKTF2127 infrared touchscreen controller binding to DT schema
> and add ektf2232 compatible.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---

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

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: touchscreen: elan,ektf2127: Add EKTF2232
From: Krzysztof Kozlowski @ 2024-05-06  6:24 UTC (permalink / raw)
  To: Andreas Kemnade, dmitry.torokhov, robh, krzk+dt, conor+dt,
	u.kleine-koenig, hdegoede, andy.shevchenko, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20240505214754.891700-3-andreas@kemnade.info>

On 05/05/2024 23:47, Andreas Kemnade wrote:
> Add a compatible for the EKTF2232, which is similar to other chips in this
> document.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---

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

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 3/6] dt-bindings: HID: i2c-hid: elan: add 'no-reset-on-power-off' property
From: Linus Walleij @ 2024-05-06  6:29 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, Douglas Anderson, linux-input, devicetree,
	linux-arm-msm, linux-kernel
In-Reply-To: <ZjSksM07OlgpE3yO@hovoldconsulting.com>

On Fri, May 3, 2024 at 10:47 AM Johan Hovold <johan@kernel.org> wrote:

> > If the above holds true, the driver can then just check for the open drain flag
> > in the reset-gpios phandle, and if that is set, conclude that it should not
> > actively drive the line low in the poweroff state.
>
> That is an alternative I considered but rejected as just knowing that
> the gpio is open-drain is not necessarily sufficient, for example, if
> the reset line is pulled to always-on rail while power to the device can
> be cut.
>
> Perhaps no one would ever construct hardware like that, but it does not
> seem like the hardware property I'm trying to encode necessarily follows
> from having an open-drain reset line.
>
> And then the OS should probably not make assumptions like that either,
> especially since getting it wrong can potentially lead to damaged
> hardware.

OK it's a fair point.

I was worried about over-specification of behaviour, as that always
leads to contradictions.

+  no-reset-on-power-off:
+    type: boolean
+    description:
+      Reset line is wired so that it can be left deasserted when the power
+      supply is off.

To be nitpicky: *should* be left deasserted rather than *can* be left
deasserted, right? If the behaviour is desirable but not strictly
required.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] dt-bindings: Use full path to other schemas
From: Miquel Raynal @ 2024-05-06  6:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Ming Qian, Zhou Peng, Mauro Carvalho Chehab, Richard Weinberger,
	Vignesh Raghavendra, Jeff LaBundy, Shijie Qin, linux-input,
	devicetree, linux-kernel, linux-media, linux-mtd
In-Reply-To: <20240505084618.135705-1-krzysztof.kozlowski@linaro.org>

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Sun,  5 May 2024 10:46:18 +0200:

> When referencing other schema, it is preferred to use an absolute path
> (/schemas/....), which allows also an seamless move of particular schema
> out of Linux kernel to dtschema.

I didn't know about this. You probably want to add a check over uses of
relative paths in the tooling now :-)

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Rob, maybe you can take it directly? Should apply cleanly on your tree.

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

^ permalink raw reply

* [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers
From: Charles Wang @ 2024-05-06 11:47 UTC (permalink / raw)
  To: hadess, hdegoede, dmitry.torokhov
  Cc: linux-input, linux-kernel, neil.armstrong, charles.goodix

Export a sysfs interface that would allow reading and writing touchscreen
IC registers. With this interface many things can be done in usersapce
such as firmware updates. An example tool that utilizes this interface
for performing firmware updates can be found at [1].

[1] https://github.com/goodix/fwupdate_for_berlin_linux

Signed-off-by: Charles Wang <charles.goodix@gmail.com>
---
 drivers/input/touchscreen/goodix_berlin.h     |  2 +
 .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
 drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
 drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
 4 files changed, 66 insertions(+)

diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
index 1fd77eb69..1741f2d15 100644
--- a/drivers/input/touchscreen/goodix_berlin.h
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -19,6 +19,8 @@ struct regmap;
 int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 			struct regmap *regmap);
 
+void goodix_berlin_remove(struct device *dev);
+
 extern const struct dev_pm_ops goodix_berlin_pm_ops;
 
 #endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
index e7b41a926..e02160841 100644
--- a/drivers/input/touchscreen/goodix_berlin_core.c
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
 	goodix_berlin_power_off(cd);
 }
 
+static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
+	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
+{
+	struct goodix_berlin_core *cd;
+	struct device *dev;
+	int error;
+
+	dev = kobj_to_dev(kobj);
+	cd = dev_get_drvdata(dev);
+
+	error = regmap_raw_read(cd->regmap, (unsigned int)off,
+				buf, count);
+
+	return error ? error : count;
+}
+
+static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
+	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
+{
+	struct goodix_berlin_core *cd;
+	struct device *dev;
+	int error;
+
+	dev = kobj_to_dev(kobj);
+	cd = dev_get_drvdata(dev);
+
+	error = regmap_raw_write(cd->regmap, (unsigned int)off,
+				 buf, count);
+
+	return error ? error : count;
+}
+
+static struct bin_attribute goodix_berlin_registers_attr = {
+	.attr = {.name = "registers", .mode = 0600},
+	.read = goodix_berlin_registers_read,
+	.write = goodix_berlin_registers_write,
+};
+
 int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 			struct regmap *regmap)
 {
@@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 
 	dev_set_drvdata(dev, cd);
 
+	error = sysfs_create_bin_file(&cd->dev->kobj,
+				      &goodix_berlin_registers_attr);
+
+	if (error) {
+		dev_err(dev, "unable to create sysfs file, err=%d\n", error);
+		return error;
+	}
+
 	dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller",
 		cd->fw_version.patch_pid);
 
@@ -750,6 +796,12 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 }
 EXPORT_SYMBOL_GPL(goodix_berlin_probe);
 
+void goodix_berlin_remove(struct device *dev)
+{
+	sysfs_remove_bin_file(&dev->kobj, &goodix_berlin_registers_attr);
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_remove);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
 MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
index 6ed9aa808..35ef21cc8 100644
--- a/drivers/input/touchscreen/goodix_berlin_i2c.c
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -46,6 +46,11 @@ static int goodix_berlin_i2c_probe(struct i2c_client *client)
 	return 0;
 }
 
+static void goodix_berlin_i2c_remove(struct i2c_client *client)
+{
+	goodix_berlin_remove(&client->dev);
+}
+
 static const struct i2c_device_id goodix_berlin_i2c_id[] = {
 	{ "gt9916", 0 },
 	{ }
@@ -66,6 +71,7 @@ static struct i2c_driver goodix_berlin_i2c_driver = {
 		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
 	},
 	.probe = goodix_berlin_i2c_probe,
+	.remove = goodix_berlin_i2c_remove,
 	.id_table = goodix_berlin_i2c_id,
 };
 module_i2c_driver(goodix_berlin_i2c_driver);
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
index 4cc557da0..8ffbe1289 100644
--- a/drivers/input/touchscreen/goodix_berlin_spi.c
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -150,6 +150,11 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
 	return 0;
 }
 
+static void goodix_berlin_spi_remove(struct spi_device *spi)
+{
+	goodix_berlin_remove(&spi->dev);
+}
+
 static const struct spi_device_id goodix_berlin_spi_ids[] = {
 	{ "gt9916" },
 	{ },
@@ -169,6 +174,7 @@ static struct spi_driver goodix_berlin_spi_driver = {
 		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
 	},
 	.probe = goodix_berlin_spi_probe,
+	.remove = goodix_berlin_spi_remove,
 	.id_table = goodix_berlin_spi_ids,
 };
 module_spi_driver(goodix_berlin_spi_driver);
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers
From: Hans de Goede @ 2024-05-06 12:03 UTC (permalink / raw)
  To: Charles Wang, hadess, dmitry.torokhov, Richard Hughes
  Cc: linux-input, linux-kernel, neil.armstrong
In-Reply-To: <20240506114752.47204-1-charles.goodix@gmail.com>

Hi,

On 5/6/24 1:47 PM, Charles Wang wrote:
> Export a sysfs interface that would allow reading and writing touchscreen
> IC registers. With this interface many things can be done in usersapce
> such as firmware updates. An example tool that utilizes this interface
> for performing firmware updates can be found at [1].

I'm not sure if I'm a fan of adding an interface to export raw register
access for fwupdate.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c

Contains update support for older goodix touchscreens and it is not
that big. I think it might be better to just have an in kernel fwupdate
driver for this and have a sysfs file to which to write the new firmware.

Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
any input on the suggested method for firmware updating ?

If raw register access is seen as a good solution, then I think this
should use regmap + some generic helpers (to be written) to export
regmap r/w access to userspace.

> [1] https://github.com/goodix/fwupdate_for_berlin_linux

Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
LICENSE file, but the header of fwupdate.c claims it is confidential ...

Regards,

Hans


> Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> ---
>  drivers/input/touchscreen/goodix_berlin.h     |  2 +
>  .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
>  drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
>  drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
>  4 files changed, 66 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> index 1fd77eb69..1741f2d15 100644
> --- a/drivers/input/touchscreen/goodix_berlin.h
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -19,6 +19,8 @@ struct regmap;
>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  			struct regmap *regmap);
>  
> +void goodix_berlin_remove(struct device *dev);
> +
>  extern const struct dev_pm_ops goodix_berlin_pm_ops;
>  
>  #endif
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index e7b41a926..e02160841 100644
> --- a/drivers/input/touchscreen/goodix_berlin_core.c
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
>  	goodix_berlin_power_off(cd);
>  }
>  
> +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> +{
> +	struct goodix_berlin_core *cd;
> +	struct device *dev;
> +	int error;
> +
> +	dev = kobj_to_dev(kobj);
> +	cd = dev_get_drvdata(dev);
> +
> +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
> +				buf, count);
> +
> +	return error ? error : count;
> +}
> +
> +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> +{
> +	struct goodix_berlin_core *cd;
> +	struct device *dev;
> +	int error;
> +
> +	dev = kobj_to_dev(kobj);
> +	cd = dev_get_drvdata(dev);
> +
> +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
> +				 buf, count);
> +
> +	return error ? error : count;
> +}
> +
> +static struct bin_attribute goodix_berlin_registers_attr = {
> +	.attr = {.name = "registers", .mode = 0600},
> +	.read = goodix_berlin_registers_read,
> +	.write = goodix_berlin_registers_write,
> +};
> +
>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  			struct regmap *regmap)
>  {
> @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  
>  	dev_set_drvdata(dev, cd);
>  
> +	error = sysfs_create_bin_file(&cd->dev->kobj,
> +				      &goodix_berlin_registers_attr);
> +
> +	if (error) {
> +		dev_err(dev, "unable to create sysfs file, err=%d\n", error);
> +		return error;
> +	}
> +
>  	dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller",
>  		cd->fw_version.patch_pid);
>  
> @@ -750,6 +796,12 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  }
>  EXPORT_SYMBOL_GPL(goodix_berlin_probe);
>  
> +void goodix_berlin_remove(struct device *dev)
> +{
> +	sysfs_remove_bin_file(&dev->kobj, &goodix_berlin_registers_attr);
> +}
> +EXPORT_SYMBOL_GPL(goodix_berlin_remove);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
>  MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
> diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
> index 6ed9aa808..35ef21cc8 100644
> --- a/drivers/input/touchscreen/goodix_berlin_i2c.c
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -46,6 +46,11 @@ static int goodix_berlin_i2c_probe(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static void goodix_berlin_i2c_remove(struct i2c_client *client)
> +{
> +	goodix_berlin_remove(&client->dev);
> +}
> +
>  static const struct i2c_device_id goodix_berlin_i2c_id[] = {
>  	{ "gt9916", 0 },
>  	{ }
> @@ -66,6 +71,7 @@ static struct i2c_driver goodix_berlin_i2c_driver = {
>  		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
>  	},
>  	.probe = goodix_berlin_i2c_probe,
> +	.remove = goodix_berlin_i2c_remove,
>  	.id_table = goodix_berlin_i2c_id,
>  };
>  module_i2c_driver(goodix_berlin_i2c_driver);
> diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
> index 4cc557da0..8ffbe1289 100644
> --- a/drivers/input/touchscreen/goodix_berlin_spi.c
> +++ b/drivers/input/touchscreen/goodix_berlin_spi.c
> @@ -150,6 +150,11 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static void goodix_berlin_spi_remove(struct spi_device *spi)
> +{
> +	goodix_berlin_remove(&spi->dev);
> +}
> +
>  static const struct spi_device_id goodix_berlin_spi_ids[] = {
>  	{ "gt9916" },
>  	{ },
> @@ -169,6 +174,7 @@ static struct spi_driver goodix_berlin_spi_driver = {
>  		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
>  	},
>  	.probe = goodix_berlin_spi_probe,
> +	.remove = goodix_berlin_spi_remove,
>  	.id_table = goodix_berlin_spi_ids,
>  };
>  module_spi_driver(goodix_berlin_spi_driver);


^ permalink raw reply

* Re: [PATCH v2 3/3] Input: ektf2127 - add ektf2232 support
From: Andy Shevchenko @ 2024-05-06 12:05 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: dmitry.torokhov, robh, krzk+dt, conor+dt, u.kleine-koenig,
	hdegoede, siebren.vroegindeweij, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20240505214754.891700-4-andreas@kemnade.info>

On Mon, May 6, 2024 at 12:48 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> The chip is similar, but has status bits at different positions,
> so use the correct bits.

...

> @@ -46,6 +47,11 @@ struct ektf2127_ts {
>         struct input_dev *input;
>         struct gpio_desc *power_gpios;
>         struct touchscreen_properties prop;
> +       int status_shift;
> +};
> +
> +struct ektf2127_i2c_chip_data {
> +       int status_shift;
>  };
>
>  static void ektf2127_parse_coordinates(const u8 *buf, unsigned int touch_count,

I'm wondering if you are using --histogram diff algo when preparing the patches.

...

> +       chip_data = device_get_match_data(&client->dev);
> +       if (!chip_data)
> +               chip_data = (const struct ektf2127_i2c_chip_data *)id->driver_data;
> +       if (!chip_data) {
> +               dev_err(&client->dev, "missing chip data\n");
> +               return -EINVAL;

First of all, there is a special combined API for this, please use it
instead. Second, use dev_err_probe().

> +       }

...

>  #ifdef CONFIG_OF

Side note: Ideally we should kill this ugliness. But it can be done later on.


--
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [syzbot] [input?] WARNING in cm109_input_open/usb_submit_urb (3)
From: syzbot @ 2024-05-06 12:38 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-kernel, syzkaller-bugs
In-Reply-To: <000000000000ae4aa90614a58d7b@google.com>

syzbot has found a reproducer for the following issue on:

HEAD commit:    dd5a440a31fa Linux 6.9-rc7
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1792e8a7180000
kernel config:  https://syzkaller.appspot.com/x/.config?x=6d14c12b661fb43
dashboard link: https://syzkaller.appspot.com/bug?extid=ac0f9c4cc1e034160492
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=10b7d354980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/58e2a4900479/disk-dd5a440a.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/246a109c32d6/vmlinux-dd5a440a.xz
kernel image: https://storage.googleapis.com/syzbot-assets/d2719f7eb672/bzImage-dd5a440a.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+ac0f9c4cc1e034160492@syzkaller.appspotmail.com

------------[ cut here ]------------
URB ffff8880293d7f00 submitted while active
WARNING: CPU: 0 PID: 1141 at drivers/usb/core/urb.c:379 usb_submit_urb+0x1039/0x18c0 drivers/usb/core/urb.c:379
Modules linked in:
CPU: 0 PID: 1141 Comm: kworker/0:2 Not tainted 6.9.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: usb_hub_wq hub_event
RIP: 0010:usb_submit_urb+0x1039/0x18c0 drivers/usb/core/urb.c:379
Code: 00 eb 66 e8 59 f2 7c fa e9 79 f0 ff ff e8 4f f2 7c fa c6 05 8f 7e 7a 08 01 90 48 c7 c7 a0 74 6c 8c 4c 89 ee e8 58 64 3f fa 90 <0f> 0b 90 90 e9 40 f0 ff ff e8 29 f2 7c fa eb 12 e8 22 f2 7c fa 41
RSP: 0018:ffffc90004926b48 EFLAGS: 00010246
RAX: 12b779fa952e9100 RBX: 0000000000000cc0 RCX: ffff8880229e8000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: ffff8880293d7f08 R08: ffffffff81587d32 R09: 1ffffffff25e6abf
R10: dffffc0000000000 R11: fffffbfff25e6ac0 R12: 1ffff11005d6920a
R13: ffff8880293d7f00 R14: dffffc0000000000 R15: ffff88802eb49010
FS:  0000000000000000(0000) GS:ffff8880b9400000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055555b1af938 CR3: 000000007c014000 CR4: 0000000000350ef0
Call Trace:
 <TASK>
 cm109_input_open+0x1f9/0x470 drivers/input/misc/cm109.c:572
 input_open_device+0x193/0x2e0 drivers/input/input.c:654
 kbd_connect+0xe9/0x130 drivers/tty/vt/keyboard.c:1593
 input_attach_handler drivers/input/input.c:1064 [inline]
 input_register_device+0xcfc/0x1090 drivers/input/input.c:2396
 cm109_usb_probe+0x10cd/0x1600 drivers/input/misc/cm109.c:806
 usb_probe_interface+0x647/0xbb0 drivers/usb/core/driver.c:399
 really_probe+0x2ba/0xad0 drivers/base/dd.c:656
 __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:798
 driver_probe_device+0x50/0x430 drivers/base/dd.c:828
 __device_attach_driver+0x2d6/0x530 drivers/base/dd.c:956
 bus_for_each_drv+0x250/0x2e0 drivers/base/bus.c:457
 __device_attach+0x333/0x520 drivers/base/dd.c:1028
 bus_probe_device+0x189/0x260 drivers/base/bus.c:532
 device_add+0x8ff/0xca0 drivers/base/core.c:3720
 usb_set_configuration+0x1976/0x1fb0 drivers/usb/core/message.c:2210
 usb_generic_driver_probe+0x88/0x140 drivers/usb/core/generic.c:254
 usb_probe_device+0x1ba/0x380 drivers/usb/core/driver.c:294
 really_probe+0x2ba/0xad0 drivers/base/dd.c:656
 __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:798
 driver_probe_device+0x50/0x430 drivers/base/dd.c:828
 __device_attach_driver+0x2d6/0x530 drivers/base/dd.c:956
 bus_for_each_drv+0x250/0x2e0 drivers/base/bus.c:457
 __device_attach+0x333/0x520 drivers/base/dd.c:1028
 bus_probe_device+0x189/0x260 drivers/base/bus.c:532
 device_add+0x8ff/0xca0 drivers/base/core.c:3720
 usb_new_device+0x104a/0x19a0 drivers/usb/core/hub.c:2652
 hub_port_connect drivers/usb/core/hub.c:5522 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5662 [inline]
 port_event drivers/usb/core/hub.c:5822 [inline]
 hub_event+0x2d6a/0x5150 drivers/usb/core/hub.c:5904
 process_one_work kernel/workqueue.c:3267 [inline]
 process_scheduled_works+0xa12/0x17c0 kernel/workqueue.c:3348
 worker_thread+0x86d/0xd70 kernel/workqueue.c:3429
 kthread+0x2f2/0x390 kernel/kthread.c:388
 ret_from_fork+0x4d/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
 </TASK>


---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

^ permalink raw reply

* Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers
From: Richard Hughes @ 2024-05-06 13:20 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Charles Wang, hadess, dmitry.torokhov, linux-input, linux-kernel,
	neil.armstrong
In-Reply-To: <6362e889-7df2-4c61-8ad5-bfe199e451ec@redhat.com>

On Mon, 6 May 2024 at 13:03, Hans de Goede <hdegoede@redhat.com> wrote:
> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
> any input on the suggested method for firmware updating ?

I'm okay with either; it's obviously easier for fwupd to just squirt a
file into a sysfs file to update the firmware, but some devices also
need something a bit more nuanced -- e.g. putting the device into an
update mode, setting the power domains, fallback recovery and stuff
like that.

Richard

^ permalink raw reply

* [PATCH 19/18] selftests/hid: skip tests with HID-BPF if udev-hid-bpf is not installed
From: bentiss @ 2024-05-06 14:36 UTC (permalink / raw)
  To: Jiri Kosina, Peter Hutterer, Shuah Khan
  Cc: Benjamin Tissoires, linux-input, Martin Sivak, Ping Cheng,
	Jason Gerecke, Aaron Armstrong Skomra, Joshua Dickens,
	linux-kernel, linux-kselftest
In-Reply-To: <20240410-bpf_sources-v1-0-a8bf16033ef8@kernel.org>

From: Benjamin Tissoires <bentiss@kernel.org>

udev-hid-bpf is still not installed everywhere, and we should probably
not assume it is installed automatically.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---

I wanted to apply this series given that it wasn't reviewed in a month,
but I thought that maybe I should not enforce ude-hid-bpf to be
installed everywhere.

I'll probably push this series tomorrow so it makes the 6.10 cut.

Cheers,
Benjamin

 tools/testing/selftests/hid/tests/base.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/hid/tests/base.py b/tools/testing/selftests/hid/tests/base.py
index 2d006c0f5fcd..3a465768e507 100644
--- a/tools/testing/selftests/hid/tests/base.py
+++ b/tools/testing/selftests/hid/tests/base.py
@@ -8,6 +8,7 @@
 import libevdev
 import os
 import pytest
+import shutil
 import subprocess
 import time
 
@@ -240,6 +241,10 @@ class BaseTestCase:
             root_dir = (script_dir / "../../../../..").resolve()
             bpf_dir = root_dir / "drivers/hid/bpf/progs"
 
+            udev_hid_bpf = shutil.which("udev-hid-bpf")
+            if not udev_hid_bpf:
+                pytest.skip("udev-hid-bpf not found in $PATH, skipping")
+
             wait = False
             for _, rdesc_fixup in self.hid_bpfs:
                 if rdesc_fixup:
-- 
2.44.0


^ permalink raw reply related

* Re: (subset) [PATCH v2 0/7] regulator: new API for voltage reference supplies
From: Mark Brown @ 2024-05-06 14:59 UTC (permalink / raw)
  To: Liam Girdwood, Jean Delvare, Guenter Roeck, Jonathan Cameron,
	Dmitry Torokhov, David Lechner
  Cc: Jonathan Corbet, Support Opensource, Cosmin Tanislav,
	Lars-Peter Clausen, Michael Hennerich, Antoniu Miclaus,
	Greg Kroah-Hartman, linux-doc, linux-kernel, linux-hwmon,
	linux-iio, linux-staging, linux-input, Jonathan Cameron
In-Reply-To: <20240429-regulator-get-enable-get-votlage-v2-0-b1f11ab766c1@baylibre.com>

On Mon, 29 Apr 2024 18:40:08 -0500, David Lechner wrote:
> In the IIO subsystem, we noticed a pattern in many drivers where we need
> to get, enable and get the voltage of a supply that provides a reference
> voltage. In these cases, we only need the voltage and not a handle to
> the regulator. Another common pattern is for chips to have an internal
> reference voltage that is used when an external reference is not
> available. There are also a few drivers outside of IIO that do the same.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/7] regulator: devres: add API for reference voltage supplies
      commit: b250c20b64290808aa4b5cc6d68819a7ee28237f
[2/7] hwmon: (adc128d818) Use devm_regulator_get_enable_read_voltage()
      commit: cffb8d74bd4e9dd0653c7093c4a5164a72c52b1f
[3/7] hwmon: (da9052) Use devm_regulator_get_enable_read_voltage()
      commit: d72fd5228c9f2136a3143daf5c7822140211883a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply

* Re: (subset) [PATCH v2 0/7] regulator: new API for voltage reference supplies
From: David Lechner @ 2024-05-06 16:09 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jean Delvare, Guenter Roeck, Jonathan Cameron,
	Dmitry Torokhov, Jonathan Corbet, Support Opensource,
	Cosmin Tanislav, Lars-Peter Clausen, Michael Hennerich,
	Antoniu Miclaus, Greg Kroah-Hartman, linux-doc, linux-kernel,
	linux-hwmon, linux-iio, linux-staging, linux-input,
	Jonathan Cameron
In-Reply-To: <171500756002.1968386.17290951989557329800.b4-ty@kernel.org>

On Mon, May 6, 2024 at 9:59 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, 29 Apr 2024 18:40:08 -0500, David Lechner wrote:
> > In the IIO subsystem, we noticed a pattern in many drivers where we need
> > to get, enable and get the voltage of a supply that provides a reference
> > voltage. In these cases, we only need the voltage and not a handle to
> > the regulator. Another common pattern is for chips to have an internal
> > reference voltage that is used when an external reference is not
> > available. There are also a few drivers outside of IIO that do the same.
> >
> > [...]
>
> Applied to
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
>
> Thanks!
>
> [1/7] regulator: devres: add API for reference voltage supplies
>       commit: b250c20b64290808aa4b5cc6d68819a7ee28237f
> [2/7] hwmon: (adc128d818) Use devm_regulator_get_enable_read_voltage()
>       commit: cffb8d74bd4e9dd0653c7093c4a5164a72c52b1f
> [3/7] hwmon: (da9052) Use devm_regulator_get_enable_read_voltage()
>       commit: d72fd5228c9f2136a3143daf5c7822140211883a
>
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
>

Hi Jonathan, if Mark doesn't pick up the iio patches here, I'll resend
them after the next kernel release.

^ permalink raw reply

* Re: [PATCH v3 06/11] dt-bindings: net/can: Add serial (serdev) LIN adapter
From: Conor Dooley @ 2024-05-06 16:16 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
	Benjamin Tissoires, Greg Kroah-Hartman, Sebastian Reichel,
	Linus Walleij, Andreas Lauser, Jonathan Corbet, Pavel Pisa,
	linux-can, netdev, devicetree, linux-input, linux-serial
In-Reply-To: <a5b894f8dc2ab0cf087a5b4972d7f752e6c17c16.camel@hexdev.de>

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

On Fri, May 03, 2024 at 08:29:15PM +0200, Christoph Fritz wrote:
> Hello Conor,
> 
>  thanks for having an eye on this, please see my answer below.
> 
> On Fri, 2024-05-03 at 18:12 +0100, Conor Dooley wrote:
> > On Thu, May 02, 2024 at 08:27:59PM +0200, Christoph Fritz wrote:
> > > Add dt-bindings for serial LIN bus adapters. These adapters are
> > > basically just LIN transceivers that are hard-wired to serial devices.
> > > 
> > > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > > ---
> > >  .../bindings/net/can/hexdev,lin-serdev.yaml   | 32 +++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > > new file mode 100644
> > > index 0000000000000..c178eb9be1391
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > > @@ -0,0 +1,32 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/can/hexdev,lin-serdev.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Serial LIN Adapter
> > > +
> > > +description:
> > > +  LIN transceiver, mostly hard-wired to a serial device, used for communication
> > > +  on a LIN bus.
> > > +  For more details on an adapter, visit <https://hexdev.de/hexlin#tty>.
> > > +
> > > +maintainers:
> > > +  - Christoph Fritz <christoph.fritz@hexdev.de>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: hexdev,lin-serdev
> > 
> > Maybe I've just missed something on earlier versions that I didn't
> > read, but the name of the device on the website you link is "hexLIN",
> > so why is "lin-serdev" used here instead?
> 
> The USB one is called hexLIN and has it's own HID driver.
> 
> This serial LIN adapter doesn't really have a product name. Currently
> on our website it's generically called 'UART LIN Adapter'.
> 
> This LIN adapter is basically just a LIN transceiver and very generic,
> so that one could solder it to any single-board computer with an uart.
> 
> I think 'lin-serdev' for LIN and serial device fits great, also serdev
> is the name of the used kernel infrastructure (besides the LIN glue
> driver).
> 
> If you still don't like it, I'm open to other names. What about
> "hexlin-uart" or "linser"?

I dunno, I don't really care about it being called "hexlin,lin-serdev",
all that much, I just found it confusing that the link in the description
sent me to the ""Hello World" in LIN" section of your site. If it had
dropped me off at the "UART LIN adapter" section things woud've been less
confusing.

That said, calling the compatible after a linux-ism is a bit odd to me
when the device seems to be called a "UART LIN adapter" on the page, not
a "serdev".

Cheers,
Conor.

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

^ permalink raw reply

* gamecube adapter driver
From: Carson @ 2024-05-06 16:19 UTC (permalink / raw)
  To: dmitry.torokhov, milas.robin; +Cc: linux-input

Hi everyone! I wanted to ask if the driver for the gamecube adapter has been 
merged. I was going to create one but I see it's already done, is there a 
subsystem tree I can clone and build somewhere?



^ permalink raw reply

* Re: [PATCH v2 3/3] Input: ektf2127 - add ektf2232 support
From: Andreas Kemnade @ 2024-05-06 16:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: dmitry.torokhov, robh, krzk+dt, conor+dt, u.kleine-koenig,
	hdegoede, siebren.vroegindeweij, linux-input, devicetree,
	linux-kernel
In-Reply-To: <CAHp75Vdnwrxw96prr9hyLdZ2u6t1uNcj6pyxCp52UoVOpatTpg@mail.gmail.com>

On Mon, 6 May 2024 15:05:52 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> To: Andreas Kemnade <andreas@kemnade.info>
> Cc: dmitry.torokhov@gmail.com, robh@kernel.org, krzk+dt@kernel.org,   conor+dt@kernel.org, u.kleine-koenig@pengutronix.de, hdegoede@redhat.com,   siebren.vroegindeweij@hotmail.com, linux-input@vger.kernel.org,   devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 3/3] Input: ektf2127 - add ektf2232 support
> Date: Mon, 6 May 2024 15:05:52 +0300
> 
> On Mon, May 6, 2024 at 12:48 AM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > The chip is similar, but has status bits at different positions,
> > so use the correct bits.  
> 
> ...
> 
> > @@ -46,6 +47,11 @@ struct ektf2127_ts {
> >         struct input_dev *input;
> >         struct gpio_desc *power_gpios;
> >         struct touchscreen_properties prop;
> > +       int status_shift;
> > +};
> > +
> > +struct ektf2127_i2c_chip_data {
> > +       int status_shift;
> >  };
> >
> >  static void ektf2127_parse_coordinates(const u8 *buf, unsigned int touch_count,  
> 
> I'm wondering if you are using --histogram diff algo when preparing the patches.

No, I am not using that, it seems to not make that chunk nicer. 
Yes, we want

+       int status_shift;
 };
+
+struct ektf2127_i2c_chip_data {
+       int status_shift;
+};

But that is not shorter or simpler, just more readable.

Regards,
Andreas

^ permalink raw reply

* [PATCH v3 0/3] Input: Add ektf2232 support
From: Andreas Kemnade @ 2024-05-06 16:23 UTC (permalink / raw)
  To: dmitry.torokhov, robh, krzk+dt, conor+dt, andreas, hdegoede,
	andy.shevchenko, u.kleine-koenig, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel

Add support for the EKTF2232 to the ektf2127 driver which
contains support for similar chips.

Add the needed compatible to bindings and convert them.

Changes in v3:
- use dev_err_probe
- use i2c_get_match_data

Changes in v2:
- separate patch for adding compatible
- use match data for selecting status shift

Andreas Kemnade (3):
  dt-bindings: touchscreen: convert elan,ektf2127 to json-schema
  dt-bindings: touchscreen: elan,ektf2127: Add EKTF2232
  Input: ektf2127 - add ektf2232 support

 .../bindings/input/touchscreen/ektf2127.txt   | 25 --------
 .../input/touchscreen/elan,ektf2127.yaml      | 58 +++++++++++++++++++
 drivers/input/touchscreen/ektf2127.c          | 36 ++++++++++--
 3 files changed, 88 insertions(+), 31 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml

-- 
2.39.2


^ permalink raw reply

* [PATCH v3 3/3] Input: ektf2127 - add ektf2232 support
From: Andreas Kemnade @ 2024-05-06 16:23 UTC (permalink / raw)
  To: dmitry.torokhov, robh, krzk+dt, conor+dt, andreas, hdegoede,
	andy.shevchenko, u.kleine-koenig, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20240506162350.912950-1-andreas@kemnade.info>

The chip is similar, but has status bits at different positions,
so use the correct bits.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/input/touchscreen/ektf2127.c | 36 +++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/ektf2127.c b/drivers/input/touchscreen/ektf2127.c
index cc3103b9cbfba..b6f5046f4b917 100644
--- a/drivers/input/touchscreen/ektf2127.c
+++ b/drivers/input/touchscreen/ektf2127.c
@@ -13,6 +13,7 @@
  * Hans de Goede <hdegoede@redhat.com>
  */
 
+#include <linux/bits.h>
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
@@ -46,6 +47,11 @@ struct ektf2127_ts {
 	struct input_dev *input;
 	struct gpio_desc *power_gpios;
 	struct touchscreen_properties prop;
+	int status_shift;
+};
+
+struct ektf2127_i2c_chip_data {
+	int status_shift;
 };
 
 static void ektf2127_parse_coordinates(const u8 *buf, unsigned int touch_count,
@@ -112,8 +118,8 @@ static void ektf2127_report2_contact(struct ektf2127_ts *ts, int slot,
 
 static void ektf2127_report2_event(struct ektf2127_ts *ts, const u8 *buf)
 {
-	ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
-	ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));
+	ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & BIT(ts->status_shift)));
+	ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & BIT(ts->status_shift + 1)));
 
 	input_mt_sync_frame(ts->input);
 	input_sync(ts->input);
@@ -247,6 +253,7 @@ static int ektf2127_query_dimension(struct i2c_client *client, bool width)
 static int ektf2127_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	const struct ektf2127_i2c_chip_data *chip_data;
 	struct ektf2127_ts *ts;
 	struct input_dev *input;
 	u8 buf[4];
@@ -303,6 +310,13 @@ static int ektf2127_probe(struct i2c_client *client)
 		return error;
 
 	ts->input = input;
+
+	chip_data = i2c_get_match_data(client);
+	if (!chip_data)
+		return dev_err_probe(&client->dev, -EINVAL, "missing chip data\n");
+
+	ts->status_shift = chip_data->status_shift;
+
 	input_set_drvdata(input, ts);
 
 	error = devm_request_threaded_irq(dev, client->irq,
@@ -325,18 +339,28 @@ static int ektf2127_probe(struct i2c_client *client)
 	return 0;
 }
 
+static const struct ektf2127_i2c_chip_data ektf2127_data = {
+	.status_shift = 1,
+};
+
+static const struct ektf2127_i2c_chip_data ektf2232_data = {
+	.status_shift = 0,
+};
+
 #ifdef CONFIG_OF
 static const struct of_device_id ektf2127_of_match[] = {
-	{ .compatible = "elan,ektf2127" },
-	{ .compatible = "elan,ektf2132" },
+	{ .compatible = "elan,ektf2127",	.data = &ektf2127_data},
+	{ .compatible = "elan,ektf2132",	.data = &ektf2127_data},
+	{ .compatible = "elan,ektf2232",	.data = &ektf2232_data},
 	{}
 };
 MODULE_DEVICE_TABLE(of, ektf2127_of_match);
 #endif
 
 static const struct i2c_device_id ektf2127_i2c_id[] = {
-	{ "ektf2127", 0 },
-	{ "ektf2132", 0 },
+	{ .name = "ektf2127", .driver_data = (long)&ektf2127_data },
+	{ .name = "ektf2132", .driver_data = (long)&ektf2127_data },
+	{ .name = "ektf2232", .driver_data = (long)&ektf2232_data },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ektf2127_i2c_id);
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 2/3] dt-bindings: touchscreen: elan,ektf2127: Add EKTF2232
From: Andreas Kemnade @ 2024-05-06 16:23 UTC (permalink / raw)
  To: dmitry.torokhov, robh, krzk+dt, conor+dt, andreas, hdegoede,
	andy.shevchenko, u.kleine-koenig, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel
  Cc: Krzysztof Kozlowski
In-Reply-To: <20240506162350.912950-1-andreas@kemnade.info>

Add a compatible for the EKTF2232, which is similar to other chips in this
document.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/input/touchscreen/elan,ektf2127.yaml     | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
index 5c4c29da0b11d..ff0ec3fd24c5d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
@@ -17,6 +17,7 @@ properties:
     enum:
       - elan,ektf2127
       - elan,ektf2132
+      - elan,ektf2232
 
   reg:
     maxItems: 1
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 1/3] dt-bindings: touchscreen: convert elan,ektf2127 to json-schema
From: Andreas Kemnade @ 2024-05-06 16:23 UTC (permalink / raw)
  To: dmitry.torokhov, robh, krzk+dt, conor+dt, andreas, hdegoede,
	andy.shevchenko, u.kleine-koenig, siebren.vroegindeweij,
	linux-input, devicetree, linux-kernel
  Cc: Krzysztof Kozlowski
In-Reply-To: <20240506162350.912950-1-andreas@kemnade.info>

Convert EKTF2127 infrared touchscreen controller binding to DT schema
and add ektf2232 compatible.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/input/touchscreen/ektf2127.txt   | 25 --------
 .../input/touchscreen/elan,ektf2127.yaml      | 57 +++++++++++++++++++
 2 files changed, 57 insertions(+), 25 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
deleted file mode 100644
index c9f2c9f578e34..0000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-* Elan eKTF2127 I2C touchscreen controller
-
-Required properties:
- - compatible		  : "elan,ektf2127" or "elan,ektf2132"
- - reg			  : I2C slave address of the chip (0x40)
- - interrupts		  : interrupt specification for the ektf2127 interrupt
- - power-gpios		  : GPIO specification for the pin connected to the
-			    ektf2127's wake input. This needs to be driven high
-			    to take ektf2127 out of its low power state
-
-For additional optional properties see: touchscreen.txt
-
-Example:
-
-i2c@00000000 {
-	ektf2127: touchscreen@15 {
-		compatible = "elan,ektf2127";
-		reg = <0x15>;
-		interrupt-parent = <&pio>;
-		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
-		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
-		touchscreen-inverted-x;
-		touchscreen-swapped-x-y;
-	};
-};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
new file mode 100644
index 0000000000000..5c4c29da0b11d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/elan,ektf2127.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Elan eKTF2127 I2C touchscreen controller
+
+maintainers:
+  - Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    enum:
+      - elan,ektf2127
+      - elan,ektf2132
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        touchscreen@15 {
+            compatible = "elan,ektf2127";
+            reg = <0x15>;
+            interrupt-parent = <&pio>;
+            interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>;
+            power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
+            touchscreen-inverted-x;
+            touchscreen-swapped-x-y;
+        };
+    };
+...
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH v3 01/11] can: Add LIN bus as CAN abstraction
From: Ilpo Järvinen @ 2024-05-06 16:24 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
	Benjamin Tissoires, Greg Kroah-Hartman, Sebastian Reichel,
	Linus Walleij, Andreas Lauser, Jonathan Corbet, Pavel Pisa,
	linux-can, netdev, devicetree, linux-input, linux-serial
In-Reply-To: <20240502182804.145926-2-christoph.fritz@hexdev.de>

On Thu, 2 May 2024, Christoph Fritz wrote:

> This patch adds a LIN (local interconnect network) bus abstraction on
> top of CAN.  It is a glue driver adapting CAN on one side while offering
> LIN abstraction on the other side. So that upcoming LIN device drivers
> can make use of it.
> 
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  drivers/net/can/Kconfig          |  10 +
>  drivers/net/can/Makefile         |   1 +
>  drivers/net/can/lin.c            | 495 +++++++++++++++++++++++++++++++
>  include/net/lin.h                |  92 ++++++
>  include/uapi/linux/can/netlink.h |   1 +
>  5 files changed, 599 insertions(+)
>  create mode 100644 drivers/net/can/lin.c
>  create mode 100644 include/net/lin.h
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 2e31db55d9278..0934bbf8d03b2 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -171,6 +171,16 @@ config CAN_KVASER_PCIEFD
>  	    Kvaser M.2 PCIe 4xCAN
>  	    Kvaser PCIe 8xCAN
>  
> +config CAN_LIN
> +	tristate "LIN mode support"
> +	help
> +	  This is a glue driver for LIN-BUS support.
> +
> +	  The local interconnect (LIN) bus is a simple bus with a feature
> +	  subset of CAN. It is often combined with CAN for simple controls.
> +
> +	  Actual device drivers need to be enabled too.
> +
>  config CAN_SLCAN
>  	tristate "Serial / USB serial CAN Adaptors (slcan)"
>  	depends on TTY
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 4669cd51e7bf5..0093ee9219ca8 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
>  obj-$(CONFIG_CAN_IFI_CANFD)	+= ifi_canfd/
>  obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
>  obj-$(CONFIG_CAN_KVASER_PCIEFD)	+= kvaser_pciefd.o
> +obj-$(CONFIG_CAN_LIN)		+= lin.o
>  obj-$(CONFIG_CAN_MSCAN)		+= mscan/
>  obj-$(CONFIG_CAN_M_CAN)		+= m_can/
>  obj-$(CONFIG_CAN_PEAK_PCIEFD)	+= peak_canfd/
> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
> new file mode 100644
> index 0000000000000..95906003666fb
> --- /dev/null
> +++ b/drivers/net/can/lin.c
> @@ -0,0 +1,495 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/can/core.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/netdevice.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <net/lin.h>
> +
> +static const u8 lin_id_parity_tbl[] = {
> +	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
> +	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
> +	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
> +	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
> +	0x00, 0x40, 0xC0, 0x80, 0x40, 0x00, 0x80, 0xC0,
> +	0x80, 0xC0, 0x40, 0x00, 0xC0, 0x80, 0x00, 0x40,
> +	0xC0, 0x80, 0x00, 0x40, 0x80, 0xC0, 0x40, 0x00,
> +	0x40, 0x00, 0x80, 0xC0, 0x00, 0x40, 0xC0, 0x80,
> +};
> +
> +u8 lin_get_id_parity(u8 id)
> +{
> +	return lin_id_parity_tbl[id];
> +}
> +EXPORT_SYMBOL(lin_get_id_parity);
> +
> +static ssize_t lin_identifier_show(struct kobject *kobj,
> +				   struct kobj_attribute *attr, char *buf)
> +{
> +	struct lin_attr *lin_attr = container_of(attr, struct lin_attr, attr);
> +	struct lin_device *ldev = lin_attr->ldev;
> +	ssize_t count = 0;
> +	struct lin_responder_answer answ;
> +	int k, ret;
> +	long id;
> +
> +	if (!ldev->ldev_ops->get_responder_answer)
> +		return -EOPNOTSUPP;
> +
> +	ret = kstrtol(attr->attr.name, 16, &id);
> +	if (ret)
> +		return ret;
> +	if (id < 0 || id >= LIN_NUM_IDS)
> +		return -EINVAL;
> +
> +	count += scnprintf(buf + count, PAGE_SIZE - count,
> +			   "%-6s %-11s %-9s %-9s %-2s %-24s %-6s\n",
> +			   "state", "cksum-mode", "is_event", "event_id",
> +			   "n", "data", "cksum");

Onl use sysfs_emit() and sysfs_emit_at() in *_show functions.

> +
> +	ret = ldev->ldev_ops->get_responder_answer(ldev, id, &answ);
> +	if (ret)
> +		return ret;
> +
> +	count += scnprintf(buf + count, PAGE_SIZE - count,
> +			   "%-6s %-11s %-9s %-9u %-2u ",
> +			   answ.is_active ? "active" : "off",
> +			   answ.lf.checksum_mode ? "enhanced" : "classic",
> +			   answ.is_event_frame ? "yes" : "no",
> +			   answ.event_associated_id,
> +			   answ.lf.len);
> +
> +	for (k = 0; k < answ.lf.len; k++)
> +		count += scnprintf(buf + count, PAGE_SIZE - count,
> +				   "%02x ", answ.lf.data[k]);
> +	for (; k < 8; k++)
> +		count += scnprintf(buf + count, PAGE_SIZE - count,
> +				   "   ");
> +	if (answ.lf.len)
> +		count += scnprintf(buf + count, PAGE_SIZE - count,
> +				   " %02x", answ.lf.checksum);
> +
> +	count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
> +
> +	return count;
> +}
> +
> +static const char *parse_and_advance(const char *buf, long *result,
> +				     unsigned int base)
> +{
> +	char num_str[5] = {0};
> +	int num_len = 0;
> +
> +	while (*buf && isspace(*buf))
> +		buf++;
> +	while (*buf && isalnum(*buf) && num_len < sizeof(num_str) - 1)
> +		num_str[num_len++] = *buf++;
> +	if (kstrtol(num_str, base, result))
> +		return NULL;
> +
> +	return buf;
> +}
> +
> +static ssize_t lin_identifier_store(struct kobject *kobj,
> +				    struct kobj_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	struct lin_attr *lin_attr = container_of(attr, struct lin_attr, attr);
> +	struct lin_device *ldev = lin_attr->ldev;
> +	struct lin_responder_answer answ = { 0 };
> +	const char *ptr = buf;
> +	int ret;
> +	long v;
> +
> +	if (!ldev->ldev_ops->update_responder_answer)
> +		return -EOPNOTSUPP;
> +
> +	ret = kstrtol(attr->attr.name, 16, &v);
> +	if (ret)
> +		return ret;
> +	if (v < 0 || v >= LIN_NUM_IDS)
> +		return -EINVAL;
> +	answ.lf.lin_id = v;
> +
> +	ptr = parse_and_advance(ptr, &v, 2);
> +	if (!ptr)
> +		return -EINVAL;
> +	answ.is_active = v != 0;
> +
> +	ptr = parse_and_advance(ptr, &v, 2);
> +	if (!ptr)
> +		return -EINVAL;
> +	answ.lf.checksum_mode = v != 0;
> +
> +	ptr = parse_and_advance(ptr, &v, 2);
> +	if (!ptr)
> +		return -EINVAL;
> +	answ.is_event_frame = v != 0;
> +
> +	ptr = parse_and_advance(ptr, &v, 16);
> +	if (!ptr || v > LIN_ID_MASK)
> +		return -EINVAL;
> +	answ.event_associated_id = v;
> +
> +	ptr = parse_and_advance(ptr, &v, 16);
> +	if (!ptr || v > LIN_MAX_DLEN)
> +		return -EINVAL;
> +	answ.lf.len = v;
> +
> +	for (int i = 0; i < answ.lf.len; i++) {
> +		ptr = parse_and_advance(ptr, &v, 16);
> +		if (!ptr)
> +			return -EINVAL;
> +		answ.lf.data[i] = v;
> +	}
> +
> +	ret = ldev->ldev_ops->update_responder_answer(ldev, &answ);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static int lin_create_sysfs_id_files(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	struct kobj_attribute *attr;
> +	int ret;
> +
> +	for (int id = 0; id < LIN_NUM_IDS; id++) {
> +		ldev->sysfs_entries[id].ldev = ldev;
> +		attr = &ldev->sysfs_entries[id].attr;
> +		attr->attr.name = kasprintf(GFP_KERNEL, "%02x", id);
> +		if (!attr->attr.name)
> +			return -ENOMEM;
> +		attr->attr.mode = 0644;
> +		attr->show = lin_identifier_show;
> +		attr->store = lin_identifier_store;
> +
> +		sysfs_attr_init(&attr->attr);
> +		ret = sysfs_create_file(ldev->lin_ids_kobj, &attr->attr);
> +		if (ret) {
> +			kfree(attr->attr.name);
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	return 0;
> +}

Can you use .dev_groups instead ?

FWIW, this function doesn't do rollback when error occurs.

> +
> +static void lin_remove_sysfs_id_files(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	struct kobj_attribute *attr;
> +
> +	for (int id = 0; id < LIN_NUM_IDS; id++) {
> +		attr = &ldev->sysfs_entries[id].attr;
> +		sysfs_remove_file(ldev->lin_ids_kobj, &attr->attr);
> +		kfree(attr->attr.name);
> +	}
> +}
> +
> +static void lin_tx_work_handler(struct work_struct *ws)
> +{
> +	struct lin_device *ldev = container_of(ws, struct lin_device,
> +					       tx_work);
> +	struct net_device *ndev = ldev->ndev;
> +	struct canfd_frame *cfd;
> +	struct lin_frame lf;
> +	int ret;
> +
> +	ldev->tx_busy = true;
> +
> +	cfd = (struct canfd_frame *)ldev->tx_skb->data;
> +	lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
> +			   LINBUS_ENHANCED : LINBUS_CLASSIC;
> +	lf.lin_id = cfd->can_id & LIN_ID_MASK;
> +	lf.len = min(cfd->len, LIN_MAX_DLEN);
> +	memcpy(lf.data, cfd->data, lf.len);
> +
> +	ret = ldev->ldev_ops->ldo_tx(ldev, &lf);
> +	if (ret) {
> +		DEV_STATS_INC(ndev, tx_dropped);
> +		netdev_err_once(ndev, "transmission failure %d\n", ret);
> +		goto lin_tx_out;
> +	}
> +
> +	DEV_STATS_INC(ndev, tx_packets);
> +	DEV_STATS_ADD(ndev, tx_bytes, lf.len);
> +
> +lin_tx_out:
> +	ldev->tx_busy = false;
> +	netif_wake_queue(ndev);
> +}
> +
> +static netdev_tx_t lin_start_xmit(struct sk_buff *skb,
> +				  struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +
> +	if (ldev->tx_busy)
> +		return NETDEV_TX_BUSY;
> +
> +	netif_stop_queue(ndev);
> +	ldev->tx_skb = skb;
> +	queue_work(ldev->wq, &ldev->tx_work);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static int lin_open(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	int ret;
> +
> +	ldev->tx_busy = false;
> +
> +	ret = open_candev(ndev);
> +	if (ret)
> +		return ret;
> +
> +	netif_wake_queue(ndev);
> +
> +	ldev->can.state = CAN_STATE_ERROR_ACTIVE;
> +	ndev->mtu = CANFD_MTU;
> +
> +	return ldev->ldev_ops->ldo_open(ldev);
> +}
> +
> +static int lin_stop(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +
> +	close_candev(ndev);
> +
> +	flush_work(&ldev->tx_work);
> +
> +	ldev->can.state = CAN_STATE_STOPPED;
> +
> +	return ldev->ldev_ops->ldo_stop(ldev);
> +}
> +
> +static const struct net_device_ops lin_netdev_ops = {
> +	.ndo_open = lin_open,
> +	.ndo_stop = lin_stop,
> +	.ndo_start_xmit = lin_start_xmit,
> +	.ndo_change_mtu = can_change_mtu,
> +};
> +
> +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> +		    enum lin_checksum_mode cm)
> +{
> +	unsigned int csum = 0;
> +	int i;
> +
> +	if (cm == LINBUS_ENHANCED)
> +		csum += pid;
> +
> +	for (i = 0; i < n_of_bytes; i++) {
> +		csum += bytes[i];
> +		if (csum > 255)
> +			csum -= 255;
> +	}
> +
> +	return (~csum & 0xff);
> +}
> +EXPORT_SYMBOL_GPL(lin_get_checksum);
> +
> +static int lin_bump_rx_err(struct lin_device *ldev, const struct lin_frame *lf)
> +{
> +	struct net_device *ndev = ldev->ndev;
> +	struct can_frame cf = {0 };
> +
> +	if (lf->lin_id > LIN_ID_MASK) {
> +		netdev_dbg(ndev, "id exceeds LIN max id\n");
> +		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +		cf.data[3] = CAN_ERR_PROT_LOC_ID12_05;
> +	}
> +
> +	if (lf->len > LIN_MAX_DLEN) {
> +		netdev_dbg(ndev, "frame exceeds number of bytes\n");
> +		cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +		cf.data[3] = CAN_ERR_PROT_LOC_DLC;
> +	}
> +
> +	if (lf->len) {
> +		u8 checksum = lin_get_checksum(LIN_FORM_PID(lf->lin_id),
> +					       lf->len, lf->data,
> +					       lf->checksum_mode);
> +
> +		if (checksum != lf->checksum) {
> +			netdev_dbg(ndev, "expected cksm: 0x%02x got: 0x%02x\n",
> +				   checksum, lf->checksum);
> +			cf.can_id = CAN_ERR_FLAG | CAN_ERR_PROT;
> +			cf.data[2] = CAN_ERR_PROT_FORM;
> +		}
> +	}
> +
> +	if (cf.can_id & CAN_ERR_FLAG) {
> +		struct can_frame *err_cf;
> +		struct sk_buff *skb = alloc_can_err_skb(ndev, &err_cf);
> +
> +		if (unlikely(!skb))
> +			return -ENOMEM;
> +
> +		err_cf->can_id |= cf.can_id;
> +		memcpy(err_cf->data, cf.data, CAN_MAX_DLEN);
> +
> +		netif_rx(skb);
> +
> +		return -EREMOTEIO;
> +	}
> +
> +	return 0;
> +}
> +
> +int lin_rx(struct lin_device *ldev, const struct lin_frame *lf)
> +{
> +	struct net_device *ndev = ldev->ndev;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	if (ldev->can.state == CAN_STATE_STOPPED)
> +		return 0;
> +
> +	netdev_dbg(ndev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
> +		   lf->checksum_mode ? "enhanced" : "classic");
> +
> +	ret = lin_bump_rx_err(ldev, lf);
> +	if (ret) {
> +		DEV_STATS_INC(ndev, rx_dropped);
> +		return ret;
> +	}
> +
> +	skb = alloc_can_skb(ndev, &cf);
> +	if (unlikely(!skb)) {
> +		DEV_STATS_INC(ndev, rx_dropped);
> +		return -ENOMEM;
> +	}
> +
> +	cf->can_id = lf->lin_id;
> +	cf->len = min(lf->len, LIN_MAX_DLEN);
> +	memcpy(cf->data, lf->data, cf->len);
> +
> +	DEV_STATS_INC(ndev, rx_packets);
> +	DEV_STATS_ADD(ndev, rx_bytes, cf->len);
> +
> +	netif_receive_skb(skb);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lin_rx);
> +
> +static int lin_set_bittiming(struct net_device *ndev)
> +{
> +	struct lin_device *ldev = netdev_priv(ndev);
> +	unsigned int bitrate = ldev->can.bittiming.bitrate;
> +
> +	return ldev->ldev_ops->update_bitrate(ldev, bitrate);
> +}
> +
> +static const u32 lin_bitrate[] = { 1200, 2400, 4800, 9600, 19200 };
> +
> +struct lin_device *register_lin(struct device *dev,
> +				const struct lin_device_ops *ldops)
> +{
> +	struct net_device *ndev;
> +	struct lin_device *ldev;
> +	int ret;
> +
> +	if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate  ||
> +	    !ldops->ldo_open || !ldops->ldo_stop) {
> +		netdev_err(ndev, "missing mandatory lin_device_ops\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct lin_device), 1);
> +	if (!ndev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ldev = netdev_priv(ndev);
> +
> +	ldev->ldev_ops = ldops;
> +	ndev->netdev_ops = &lin_netdev_ops;
> +	ndev->flags |= IFF_ECHO;
> +	ndev->mtu = CANFD_MTU;
> +	ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
> +	ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
> +	ldev->can.ctrlmode_supported = 0;
> +	ldev->can.bitrate_const = lin_bitrate;
> +	ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
> +	ldev->can.do_set_bittiming = lin_set_bittiming;
> +	ldev->ndev = ndev;
> +	ldev->dev = dev;
> +
> +	SET_NETDEV_DEV(ndev, dev);
> +
> +	ret = lin_set_bittiming(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "set bittiming failed\n");
> +		goto exit_candev;
> +	}
> +
> +	ret = register_candev(ndev);
> +	if (ret)
> +		goto exit_candev;
> +
> +	ldev->lin_ids_kobj = kobject_create_and_add("lin_ids", &ndev->dev.kobj);
> +	if (!ldev->lin_ids_kobj) {
> +		netdev_err(ndev, "Failed to create sysfs directory\n");
> +		ret = -ENOMEM;
> +		goto exit_unreg;
> +	}
> +
> +	ret = lin_create_sysfs_id_files(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "Failed to create sysfs entry: %d\n", ret);
> +		goto exit_kobj_put;
> +	}
> +
> +	/* Using workqueue as tx over USB/SPI/... may sleep */
> +	ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> +				   0);
> +	if (!ldev->wq)
> +		goto exit_rm_files;
> +
> +	INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
> +
> +	netdev_info(ndev, "LIN initialized.\n");
> +
> +	return ldev;
> +
> +exit_rm_files:
> +	lin_remove_sysfs_id_files(ndev);
> +exit_kobj_put:
> +	kobject_put(ldev->lin_ids_kobj);
> +exit_unreg:
> +	unregister_candev(ndev);
> +exit_candev:
> +	free_candev(ndev);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(register_lin);
> +
> +void unregister_lin(struct lin_device *ldev)
> +{
> +	struct net_device *ndev = ldev->ndev;
> +
> +	lin_remove_sysfs_id_files(ndev);
> +	kobject_put(ldev->lin_ids_kobj);
> +	unregister_candev(ndev);
> +	destroy_workqueue(ldev->wq);
> +	free_candev(ndev);
> +}
> +EXPORT_SYMBOL_GPL(unregister_lin);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
> +MODULE_DESCRIPTION("LIN bus to CAN glue driver");
> diff --git a/include/net/lin.h b/include/net/lin.h
> new file mode 100644
> index 0000000000000..e7c7c820a6e18
> --- /dev/null
> +++ b/include/net/lin.h
> @@ -0,0 +1,92 @@
> +/* 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	0x00000200U
> +#define LIN_ENHANCED_CKSUM_FLAG	0x00000100U

BIT(x) x 2

> +
> +extern u8 lin_get_id_parity(u8 id);
> +
> +#define LIN_GET_ID(PID)		FIELD_GET(LIN_ID_MASK, PID)
> +#define LIN_FORM_PID(ID)	(LIN_GET_ID(ID) | \
> +					lin_get_id_parity(LIN_GET_ID(ID)))
> +#define LIN_GET_PARITY(PID)	((PID) & ~LIN_ID_MASK)
> +#define LIN_CHECK_PID(PID)	(LIN_GET_PARITY(PID) == \
> +					LIN_GET_PARITY(LIN_FORM_PID(PID)))
> +
> +struct lin_attr {
> +	struct kobj_attribute attr;
> +	struct lin_device *ldev;
> +};
> +
> +struct lin_device {
> +	struct can_priv can;  /* must be the first member */
> +	struct net_device *ndev;
> +	struct device *dev;
> +	const struct lin_device_ops *ldev_ops;
> +	struct workqueue_struct *wq;
> +	struct work_struct tx_work;
> +	bool tx_busy;
> +	struct sk_buff *tx_skb;
> +	struct kobject *lin_ids_kobj;
> +	struct lin_attr sysfs_entries[LIN_NUM_IDS];
> +};
> +
> +enum lin_checksum_mode {
> +	LINBUS_CLASSIC = 0,
> +	LINBUS_ENHANCED,
> +};
> +
> +struct lin_frame {
> +	u8 lin_id;
> +	u8 len;
> +	u8 data[LIN_MAX_DLEN];
> +	u8 checksum;
> +	enum lin_checksum_mode checksum_mode;
> +};
> +
> +struct lin_responder_answer {
> +	bool is_active;
> +	bool is_event_frame;
> +	u8 event_associated_id;
> +	struct lin_frame lf;
> +};
> +
> +struct lin_device_ops {
> +	int (*ldo_open)(struct lin_device *ldev);
> +	int (*ldo_stop)(struct lin_device *ldev);
> +	int (*ldo_tx)(struct lin_device *ldev, const struct lin_frame *frame);
> +	int (*update_bitrate)(struct lin_device *ldev, u16 bitrate);
> +	int (*update_responder_answer)(struct lin_device *ldev,
> +				       const struct lin_responder_answer *answ);
> +	int (*get_responder_answer)(struct lin_device *ldev, u8 id,
> +				    struct lin_responder_answer *answ);
> +};
> +
> +int lin_rx(struct lin_device *ldev, const struct lin_frame *lf);
> +
> +u8 lin_get_checksum(u8 pid, u8 n_of_bytes, const u8 *bytes,
> +		    enum lin_checksum_mode cm);
> +
> +struct lin_device *register_lin(struct device *dev,
> +				const struct lin_device_ops *ldops);
> +void unregister_lin(struct lin_device *ldev);
> +
> +#endif /* _NET_LIN_H_ */
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index 02ec32d694742..51b0e2a7624e4 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -103,6 +103,7 @@ struct can_ctrlmode {
>  #define CAN_CTRLMODE_CC_LEN8_DLC	0x100	/* Classic CAN DLC option */
>  #define CAN_CTRLMODE_TDC_AUTO		0x200	/* CAN transiver automatically calculates TDCV */
>  #define CAN_CTRLMODE_TDC_MANUAL		0x400	/* TDCV is manually set up by user */

BIT(x) is these days available also for uapi I think.

> +#define CAN_CTRLMODE_LIN		0x800	/* LIN bus mode */
>  
>  /*
>   * CAN device statistics
> 

-- 
 i.


^ permalink raw reply

* Re: [PATCH v3 02/11] HID: hexLIN: Add support for USB LIN bus adapter
From: Ilpo Järvinen @ 2024-05-06 16:53 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
	Benjamin Tissoires, Greg Kroah-Hartman, Sebastian Reichel,
	Linus Walleij, Andreas Lauser, Jonathan Corbet, Pavel Pisa,
	linux-can, Netdev, devicetree, linux-input, linux-serial
In-Reply-To: <20240502182804.145926-3-christoph.fritz@hexdev.de>

On Thu, 2 May 2024, Christoph Fritz wrote:

> This patch introduces driver support for the hexLIN USB LIN bus adapter,
> enabling LIN communication over USB for both controller and responder
> modes. The driver interfaces with the CAN_LIN framework for userland
> connectivity.
> 
> 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 | 611 ++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h           |   1 +
>  drivers/hid/hid-quirks.c        |   3 +
>  5 files changed, 635 insertions(+)
>  create mode 100644 drivers/hid/hid-hexdev-hexlin.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 4c682c6507040..d2fb35d83c640 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 082a728eac600..f9b13e6117e60 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_HID_GOOGLE_STADIA_FF)	+= hid-google-stadiaff.o
>  obj-$(CONFIG_HID_VIVALDI)	+= hid-vivaldi.o
>  obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
> +obj-$(CONFIG_HID_MCS_HEXDEV)	+= hid-hexdev-hexlin.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
> diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
> new file mode 100644
> index 0000000000000..1ddc1e00ab2da
> --- /dev/null
> +++ b/drivers/hid/hid-hexdev-hexlin.c
> @@ -0,0 +1,611 @@
> +// 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"
> +
> +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,

Could you align the values?

> +};
> +
> +#define HEXLIN_SUCCESS_SZ			1
> +#define HEXLIN_FRAME_SZ				17
> +#define HEXLIN_FAIL_SZ				1
> +#define HEXLIN_GET_RESPONDER_ANSWER_ID_SZ	20
> +#define HEXLIN_GET_BAUDRATE_SZ			3

Is this sizeof(hexlin_baudrate_req)? If so, don't add define for it.
This probably applies to other defines here too.

> +#define HEXLIN_BAUDRATE_SZ			2
> +#define HEXLIN_GET_VERSION_SZ			2
> +#define HEXLIN_PKGLEN_MAX_SZ			64
> +
> +struct hexlin_val8_req {
> +	u8 cmd;
> +	u8 v;
> +} __packed;
> +
> +struct hexlin_baudrate_req {
> +	u8 cmd;
> +	u16 baudrate;
> +} __packed;
> +
> +struct hexlin_frame {
> +	u32 flags;
> +	u8 len;
> +	u8 lin_id;
> +	u8 data[LIN_MAX_DLEN];
> +	u8 checksum;
> +	u8 checksum_mode;
> +} __packed;
> +
> +struct hexlin_unconditional_req {
> +	u8 cmd;
> +	struct hexlin_frame frm;
> +} __packed;
> +
> +struct hexlin_responder_answer {
> +	u8 is_active;
> +	u8 is_event_frame;
> +	u8 event_associated_id;
> +	struct hexlin_frame frm;
> +} __packed;
> +
> +struct hexlin_responder_answer_req {
> +	u8 cmd;
> +	struct hexlin_responder_answer answ;
> +} __packed;
> +
> +struct hexlin_priv_data {
> +	struct hid_device *hid_dev;
> +	struct lin_device *ldev;
> +	u16 baudrate;
> +	struct completion wait_in_report;
> +	bool is_error;
> +	struct mutex tx_lock;  /* protects hexlin_tx_report() */
> +	struct hexlin_responder_answer_req rar;
> +	u8 fw_version;
> +};
> +
> +static int hexlin_tx_report(struct hexlin_priv_data *priv,
> +			    const void *out_report, size_t len)
> +{
> +	u8 *buf;
> +	int ret;
> +
> +	buf = kmemdup(out_report, len, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_output_report(priv->hid_dev, buf, len);
> +	kfree(buf);

Is duplicatign the buffer necessary?

> +	if (ret < 0)
> +		return ret;
> +	if (ret != len)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int hexlin_tx_req_status(struct hexlin_priv_data *priv,
> +				const void *out_report, int len)
> +{
> +	int ret;
> +	unsigned long t;
> +
> +	mutex_lock(&priv->tx_lock);
> +
> +	reinit_completion(&priv->wait_in_report);
> +
> +	ret = hexlin_tx_report(priv, out_report, len);
> +	if (ret)
> +		goto tx_exit;
> +
> +	t = wait_for_completion_killable_timeout(&priv->wait_in_report,
> +						 msecs_to_jiffies(1000));

HZ?

> +	if (!t)
> +		ret = -ETIMEDOUT;
> +
> +	if (priv->is_error)
> +		ret = -EINVAL;
> +
> +tx_exit:
> +	mutex_unlock(&priv->tx_lock);
> +
> +	return ret;
> +}
> +
> +#define HEXLIN_GET_CMD(name, enum_cmd)					\
> +	static int hexlin_##name(struct hexlin_priv_data *priv)		\
> +	{								\
> +		u8 cmd = enum_cmd;					\
> +		int ret;						\
> +									\
> +		ret = hexlin_tx_req_status(priv, &cmd, sizeof(u8));	\

Take sizeof() of the relevant variable instead, so:

sizeof(cmd)

> +		if (ret)						\
> +			hid_err(priv->hid_dev, "%s failed with %d\n",	\
> +				__func__, ret);				\
> +									\
> +		return ret;						\
> +	}
> +
> +HEXLIN_GET_CMD(get_version, HEXLIN_GET_VERSION)
> +HEXLIN_GET_CMD(reset_dev, HEXLIN_RESET)
> +HEXLIN_GET_CMD(get_baudrate, HEXLIN_GET_BAUDRATE)
> +
> +#define HEXLIN_VAL_CMD(name, enum_cmd, struct_type, vtype)		\
> +	static int hexlin_##name(struct hexlin_priv_data *p, vtype val)	\
> +	{								\
> +		struct struct_type req;					\
> +		int ret;						\
> +									\
> +		req.cmd = enum_cmd;					\
> +		req.v = val;						\
> +									\
> +		ret = hexlin_tx_req_status(p, &req,			\
> +					   sizeof(struct struct_type));	\

sizeof(req)

> +		if (ret)						\
> +			hid_err(p->hid_dev, "%s failed with %d\n",	\
> +				__func__, ret);				\
> +									\
> +		return ret;						\
> +	}
> +
> +HEXLIN_VAL_CMD(send_break, HEXLIN_SEND_BREAK, hexlin_val8_req, u8)
> +
> +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> +				      const u8 *raw_data, int sz)
> +{
> +	struct hid_device *hdev = priv->hid_dev;
> +	struct hexlin_frame hxf;
> +	struct lin_frame lf;
> +
> +	if (sz != sizeof(struct hexlin_frame))
> +		return -EREMOTEIO;
> +
> +	memcpy(&hxf, raw_data, sz);

Why you cannot just cast the pointer to correct type?

> +	le32_to_cpus(hxf.flags);

You must use correct endianess typing. The struct hexlin_frame should have 
__le32 flags so sparse's endianness check is happy.

But .flags are not used at all so why is this required in the first place?

> +	lf.len = hxf.len;
> +	lf.lin_id = hxf.lin_id;
> +	memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
> +	lf.checksum = hxf.checksum;
> +	lf.checksum_mode = hxf.checksum_mode;
> +
> +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> +		   lf.checksum_mode ? "enhanced" : "classic");
> +
> +	lin_rx(priv->ldev, &lf);
> +
> +	return 0;
> +}
> +
> +static int hexlin_send_unconditional(struct hexlin_priv_data *priv,
> +			      const struct hexlin_frame *hxf)
> +{
> +	struct hexlin_unconditional_req req;
> +	int ret;
> +
> +	if (hxf->lin_id > LIN_ID_MASK)
> +		return -EINVAL;
> +
> +	req.cmd = HEXLIN_SEND_UNCONDITIONAL_FRAME;
> +	memcpy(&req.frm, hxf, sizeof(struct hexlin_frame));
> +
> +	ret = hexlin_tx_req_status(priv, &req,
> +				   sizeof(struct hexlin_unconditional_req));

sizeof(req)

> +
> +	if (ret)
> +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> +
> +	return ret;
> +}
> +
> +static int hexlin_set_baudrate(struct hexlin_priv_data *priv, u16 baudrate)
> +{
> +	struct hexlin_baudrate_req req;
> +	int ret;
> +
> +	if (baudrate < LIN_MIN_BAUDRATE || baudrate > LIN_MAX_BAUDRATE)
> +		return -EINVAL;
> +
> +	req.cmd = HEXLIN_SET_BAUDRATE;
> +	req.baudrate = cpu_to_le16(baudrate);

The struct should have __le16 baudrate.

> +
> +	ret = hexlin_tx_req_status(priv, &req,
> +				   sizeof(struct hexlin_baudrate_req));
> +	if (ret)
> +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> +
> +	return ret;
> +}
> +
> +static int hexlin_get_responder_answer_id(struct hexlin_priv_data *priv, u8 id,
> +					  struct hexlin_responder_answer_req *rar)
> +{
> +	u8 req[2] = { HEXLIN_GET_RESPONDER_ANSWER_ID, id };
> +	int ret;
> +
> +	if (id > LIN_ID_MASK)
> +		return -EINVAL;
> +
> +	ret = hexlin_tx_req_status(priv, &req, sizeof(req));
> +	if (ret) {
> +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);

Try to write error message that is meaningful to user, printing __func__ 
is not very helpful for user.

> +		return ret;
> +	}
> +
> +	memcpy(rar, &priv->rar, sizeof(struct hexlin_responder_answer_req));

sizeof(*rar)

> +	return 0;
> +}
> +
> +static int hexlin_set_responder_answer_id(struct hexlin_priv_data *priv,
> +					  const struct lin_responder_answer *answ)
> +{
> +	struct hexlin_responder_answer_req rar;
> +	int ret;
> +
> +	if (answ->lf.lin_id > LIN_ID_MASK ||
> +	    answ->event_associated_id > LIN_ID_MASK)
> +		return -EINVAL;
> +
> +	rar.cmd = HEXLIN_SET_RESPONDER_ANSWER_ID;
> +	rar.answ.is_active = answ->is_active;
> +	rar.answ.is_event_frame = answ->is_event_frame;
> +	rar.answ.event_associated_id = answ->event_associated_id;
> +	rar.answ.frm.len = answ->lf.len;
> +	rar.answ.frm.lin_id = answ->lf.lin_id;
> +	memcpy(rar.answ.frm.data, answ->lf.data, LIN_MAX_DLEN);
> +	rar.answ.frm.checksum = answ->lf.checksum;
> +	rar.answ.frm.checksum_mode = answ->lf.checksum_mode;
> +
> +	ret = hexlin_tx_req_status(priv, &rar,
> +				   sizeof(struct hexlin_responder_answer_req));

Ditto.

-- 
 i.


> +	if (ret)
> +		hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> +
> +	return ret;
> +}
> +
> +static int hexlin_open(struct lin_device *ldev)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +
> +	return hid_hw_open(hdev);
> +}
> +
> +static int hexlin_stop(struct lin_device *ldev)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	hid_hw_close(hdev);
> +
> +	priv->is_error = true;
> +	complete(&priv->wait_in_report);
> +
> +	return 0;
> +}
> +
> +static int hexlin_ldo_tx(struct lin_device *ldev,
> +			 const struct lin_frame *lf)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	int ret = -EINVAL;
> +
> +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> +		   lf->lin_id, lf->len, lf->len, lf->data, lf->checksum,
> +		   lf->checksum_mode ? "enhanced" : "classic");
> +
> +	if (lf->lin_id && lf->len == 0) {
> +		ret = hexlin_send_break(priv, lf->lin_id);
> +	} else if (lf->len <= LIN_MAX_DLEN) {
> +		struct hexlin_frame hxf;
> +
> +		hxf.len = lf->len;
> +		hxf.lin_id = lf->lin_id;
> +		memcpy(&hxf.data, lf->data, LIN_MAX_DLEN);
> +		hxf.checksum = lf->checksum;
> +		hxf.checksum_mode = lf->checksum_mode;
> +		ret = hexlin_send_unconditional(priv, &hxf);
> +	} else {
> +		hid_err(hdev, "unknown format\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int hexlin_update_bitrate(struct lin_device *ldev, u16 bitrate)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	hid_dbg(hdev, "update bitrate to: %u\n", bitrate);
> +
> +	ret = hexlin_open(ldev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hexlin_set_baudrate(priv, bitrate);
> +	if (ret)
> +		return ret;
> +
> +	ret = hexlin_get_baudrate(priv);
> +	if (ret)
> +		return ret;
> +
> +	if (priv->baudrate != bitrate) {
> +		hid_err(hdev, "update bitrate failed\n");
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int hexlin_get_responder_answer(struct lin_device *ldev, u8 id,
> +				       struct lin_responder_answer *answ)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +	struct hexlin_responder_answer_req rar;
> +	int ret;
> +
> +	if (answ == NULL)
> +		return -EINVAL;
> +
> +	ret = hexlin_get_responder_answer_id(priv, id, &rar);
> +	if (ret)
> +		return ret;
> +
> +	answ->is_active = rar.answ.is_active;
> +	answ->is_event_frame = rar.answ.is_event_frame;
> +	answ->event_associated_id = rar.answ.event_associated_id;
> +	answ->lf.len = rar.answ.frm.len;
> +	answ->lf.lin_id = rar.answ.frm.lin_id;
> +	memcpy(answ->lf.data, rar.answ.frm.data, LIN_MAX_DLEN);
> +	answ->lf.checksum = rar.answ.frm.checksum;
> +	answ->lf.checksum_mode = rar.answ.frm.checksum_mode;
> +
> +	return 0;
> +}
> +
> +static int hexlin_update_resp_answer(struct lin_device *ldev,
> +				     const struct lin_responder_answer *answ)
> +{
> +	struct hid_device *hdev = to_hid_device(ldev->dev);
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	if (answ == NULL)
> +		return -EINVAL;
> +
> +	return hexlin_set_responder_answer_id(priv, answ);
> +}
> +
> +static const struct lin_device_ops hexlin_ldo = {
> +	.ldo_open = hexlin_open,
> +	.ldo_stop = hexlin_stop,
> +	.ldo_tx = hexlin_ldo_tx,
> +	.update_bitrate = hexlin_update_bitrate,
> +	.get_responder_answer = hexlin_get_responder_answer,
> +	.update_responder_answer = hexlin_update_resp_answer,
> +};
> +
> +static int hexlin_raw_event(struct hid_device *hdev,
> +			    struct hid_report *report, u8 *data, int sz)
> +{
> +	struct hexlin_priv_data *priv;
> +	int ret;
> +
> +	if (sz < 1 || sz > HEXLIN_PKGLEN_MAX_SZ)
> +		return -EREMOTEIO;
> +
> +	priv = hid_get_drvdata(hdev);
> +
> +	hid_dbg(hdev, "%s, size:%i, data[0]: 0x%02x\n", __func__, sz, data[0]);
> +
> +	priv->is_error = false;
> +
> +	switch (data[0]) {
> +	case HEXLIN_SUCCESS:
> +		if (sz != HEXLIN_SUCCESS_SZ)
> +			return -EREMOTEIO;
> +		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_FAIL:
> +		if (sz != HEXLIN_FAIL_SZ)
> +			return -EREMOTEIO;
> +		hid_err(hdev, "HEXLIN_FAIL: 0x%02x\n", data[0]);
> +		priv->is_error = true;
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_VERSION:
> +		if (sz != HEXLIN_GET_VERSION_SZ)
> +			return -EREMOTEIO;
> +		priv->fw_version = data[1];
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_RESPONDER_ANSWER_ID:
> +		if (sz != HEXLIN_GET_RESPONDER_ANSWER_ID_SZ)
> +			return -EREMOTEIO;
> +		BUILD_BUG_ON(sizeof(priv->rar) !=
> +			HEXLIN_GET_RESPONDER_ANSWER_ID_SZ);
> +		memcpy(&priv->rar, data, sizeof(priv->rar));
> +		complete(&priv->wait_in_report);
> +		break;
> +	case HEXLIN_GET_BAUDRATE:
> +		if (sz != HEXLIN_GET_BAUDRATE_SZ)
> +			return -EREMOTEIO;
> +		BUILD_BUG_ON(sizeof(priv->baudrate) != HEXLIN_BAUDRATE_SZ);
> +		memcpy(&priv->baudrate, &data[1], sizeof(priv->baudrate));
> +		le16_to_cpus(priv->baudrate);
> +		complete(&priv->wait_in_report);
> +		break;
> +	/* following cases not initiated by us, so no complete() */
> +	case HEXLIN_FRAME:
> +		if (sz != HEXLIN_FRAME_SZ) {
> +			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
> +			return -EREMOTEIO;
> +		}
> +		ret = hexlin_queue_frames_insert(priv, &data[1], sz-1);
> +		if (ret) {
> +			hid_err(hdev, "failed to add frame: %i\n", ret);
> +			return ret;
> +		}
> +		break;
> +	case HEXLIN_ERROR:
> +		hid_err(hdev, "error from adapter\n");
> +		break;
> +	default:
> +		hid_err(hdev, "unknown event: 0x%02x\n", data[0]);
> +	}
> +
> +	return 0;
> +}
> +
> +static int init_hw(struct hexlin_priv_data *priv)
> +{
> +	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_OR_NULL(priv->ldev)) {
> +		ret = PTR_ERR(priv->ldev);
> +		goto fail_and_close;
> +	}
> +
> +	hid_hw_close(hdev);
> +
> +	hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> +
> +	return 0;
> +
> +fail_and_close:
> +	hid_hw_close(hdev);
> +fail_and_stop:
> +	hid_hw_stop(hdev);
> +fail_and_free:
> +	mutex_destroy(&priv->tx_lock);
> +	return ret;
> +}
> +
> +static void hexlin_remove(struct hid_device *hdev)
> +{
> +	struct hexlin_priv_data *priv = hid_get_drvdata(hdev);
> +
> +	unregister_lin(priv->ldev);
> +	hid_hw_stop(hdev);
> +}
> +
> +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 64164423b592b..8f46d37c2b499 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -907,6 +907,7 @@
>  
>  #define USB_VENDOR_ID_MCS		0x16d0
>  #define USB_DEVICE_ID_MCS_GAMEPADBLOCK	0x0bcc
> +#define USB_DEVICE_ID_MCS_HEXDEV_HEXLIN	0x0648
>  
>  #define USB_VENDOR_MEGAWORLD		0x07b5
>  #define USB_DEVICE_ID_MEGAWORLD_GAMEPAD	0x0312
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 1d1949d62dfaf..a514449c50047 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -438,6 +438,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE_2) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE_3) },
>  #endif
> +#if IS_ENABLED(CONFIG_HID_MCS_HEXDEV)
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_HEXDEV_HEXLIN) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_HOLTEK)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK, USB_DEVICE_ID_HOLTEK_ON_LINE_GRIP) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD) },
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox