...

quoted

diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
new file mode 100644
index 0000000000000..a9ed080b3e33e
--- /dev/null
+++ b/drivers/hid/hid-hexdev-hexlin.c

...

quoted

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

Extra space.

Use:

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

quoted

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

Not needed after using __free().

quoted

+		return ret;						\
+	}
+
+HEXLIN_GET_CMD(get_version, HEXLIN_GET_VERSION)
+HEXLIN_GET_CMD(reset_dev, HEXLIN_RESET)
+HEXLIN_GET_CMD(get_baudrate, HEXLIN_GET_BAUDRATE)

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

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

...

quoted

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

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

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

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

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

...

quoted

+static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
+				      const struct hexlin_frame *hxf)
+{
+	struct hid_device *hdev = priv->hid_dev;
+	struct lin_frame lf;
+
+	lf.len = hxf->len;
+	lf.lin_id = hxf->lin_id;
+	memcpy(lf.data, hxf->data, LIN_MAX_DLEN);
+	lf.checksum = hxf->checksum;
+	lf.checksum_mode = hxf->checksum_mode;
+
+	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, chk:%02x (%s), flg:%08x\n",
+		lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
+		lf.checksum_mode ? "enhanced" : "classic", hxf->flags);
+
+	lin_rx(priv->ldev, &lf);
+
+	return 0;
+}
+
+static int hexlin_raw_event(struct hid_device *hdev,
+			    struct hid_report *report, u8 *data, int sz)
+{
+	struct hexlin_priv_data *priv;
+	struct hexlin_baudrate_req *br;
+	struct hexlin_responder_answer_req *rar;
+	struct hexlin_unconditional_req *hfr;
+	struct hexlin_val8_req *vr;
+
+	if (sz < 1 || sz > HEXLIN_PKGLEN_MAX)
+		return -EREMOTEIO;
+
+	priv = hid_get_drvdata(hdev);
+
+	hid_dbg(hdev, "%s, size:%i, data[0]: 0x%02x\n", __func__, sz, data[0]);
+
+	priv->is_error = false;
+
+	switch (data[0]) {
+	case HEXLIN_SUCCESS:
+		if (sz != HEXLIN_LEN_RETCODE)
+			return -EREMOTEIO;
+		hid_dbg(hdev, "HEXLIN_SUCCESS: 0x%02x\n", data[0]);
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_FAIL:
+		if (sz != HEXLIN_LEN_RETCODE)
+			return -EREMOTEIO;
+		hid_err(hdev, "HEXLIN_FAIL: 0x%02x\n", data[0]);
+		priv->is_error = true;
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_VERSION:
+		if (sz != sizeof(*vr))
+			return -EREMOTEIO;
+		vr = (struct hexlin_val8_req *) data;
+		priv->fw_version = vr->v;
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_RESPONDER_ANSWER_ID:
+		if (sz != sizeof(*rar))
+			return -EREMOTEIO;
+		rar = (struct hexlin_responder_answer_req *) data;
+		memcpy(&priv->answ, &rar->answ, sizeof(priv->answ));
+		complete(&priv->wait_in_report);
+		break;
+	case HEXLIN_GET_BAUDRATE:
+		if (sz != sizeof(*br))
+			return -EREMOTEIO;
+		br = (struct hexlin_baudrate_req *) data;
+		le16_to_cpus(br->baudrate);
+		priv->baudrate = br->baudrate;
+		complete(&priv->wait_in_report);
+		break;
+	/* following cases not initiated by us, so no complete() */
+	case HEXLIN_FRAME:
+		if (sz != sizeof(*hfr)) {
+			hid_err_once(hdev, "frame size mismatch: %i\n", sz);
+			return -EREMOTEIO;
+		}
+		hfr = (struct hexlin_unconditional_req *) data;
+		le32_to_cpus(hfr->frm.flags);

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

Can you propose a solution?

...

thanks
  -- Christoph