* 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
* Re: [PATCH v3 07/11] can: Add support for serdev LIN adapters
From: Ilpo Järvinen @ 2024-05-06 17:03 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-8-christoph.fritz@hexdev.de>
On Thu, 2 May 2024, Christoph Fritz wrote:
> This commit introduces LIN-Bus support for UART devices equipped with
> LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
>
> For more details on an adapter, visit: https://hexdev.de/hexlin#tty
>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
> drivers/net/can/Kconfig | 16 ++
> drivers/net/can/Makefile | 1 +
> drivers/net/can/lin-serdev.c | 500 +++++++++++++++++++++++++++++++++++
> 3 files changed, 517 insertions(+)
> create mode 100644 drivers/net/can/lin-serdev.c
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 0934bbf8d03b2..91c6cdeefe440 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -181,6 +181,22 @@ config CAN_LIN
>
> Actual device drivers need to be enabled too.
>
> +config CAN_LIN_SERDEV
> + tristate "Serial LIN Adaptors"
> + depends on CAN_LIN && SERIAL_DEV_BUS && OF
> + help
> + LIN-Bus support for serial devices equipped with LIN transceievers.
> + This device driver is using CAN_LIN for a userland connection on
> + one side and the kernel its serial device bus (serdev) interface
> + on the other side.
> +
> + If you have a hexlin tty adapter, say Y here and see
> + <https://hexdev.de/hexlin#tty>.
> +
> + This driver can also be built as a module. If so, the module will be
> + called lin-serdev.ko.
> +
> +
> config CAN_SLCAN
> tristate "Serial / USB serial CAN Adaptors (slcan)"
> depends on TTY
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 0093ee9219ca8..21ca514a42439 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_CAN_IFI_CANFD) += ifi_canfd/
> obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o
> obj-$(CONFIG_CAN_KVASER_PCIEFD) += kvaser_pciefd.o
> obj-$(CONFIG_CAN_LIN) += lin.o
> +obj-$(CONFIG_CAN_LIN_SERDEV) += lin-serdev.o
> obj-$(CONFIG_CAN_MSCAN) += mscan/
> obj-$(CONFIG_CAN_M_CAN) += m_can/
> obj-$(CONFIG_CAN_PEAK_PCIEFD) += peak_canfd/
> diff --git a/drivers/net/can/lin-serdev.c b/drivers/net/can/lin-serdev.c
> new file mode 100644
> index 0000000000000..13fd9a54c6c93
> --- /dev/null
> +++ b/drivers/net/can/lin-serdev.c
> @@ -0,0 +1,500 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/kfifo.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <net/lin.h>
> +
> +#define LINSER_SAMPLES_PER_CHAR 10
> +#define LINSER_TX_BUFFER_SIZE 11
> +#define LINSER_RX_FIFO_SIZE 256
> +#define LINSER_PARSE_BUFFER 24
> +
> +struct linser_rx {
> + u8 data;
> + u8 flag;
> +};
> +
> +enum linser_rx_status {
> + NEED_MORE = -1,
> + MODE_OK = 0,
> + NEED_FORCE,
> +};
> +
> +struct linser_priv {
> + struct lin_device *lin_dev;
> + struct serdev_device *serdev;
> + DECLARE_KFIFO_PTR(rx_fifo, struct linser_rx);
> + struct delayed_work rx_work;
> + unsigned long break_usleep_min;
> + unsigned long break_usleep_max;
> + unsigned long post_break_usleep_min;
> + unsigned long post_break_usleep_max;
> + unsigned long force_timeout_jfs;
> + struct lin_responder_answer respond_answ[LIN_NUM_IDS];
> + struct mutex resp_lock; /* protects respond_answ */
> + bool is_stopped;
> +};
> +
> +static int linser_open(struct lin_device *ldev)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + int ret;
> +
> + if (priv->is_stopped) {
> + ret = serdev_device_open(serdev);
> + if (ret) {
> + dev_err(&serdev->dev, "Unable to open device\n");
> + return ret;
> + }
> +
> + serdev_device_set_flow_control(serdev, false);
> + serdev_device_set_break_detection(serdev, true);
> +
> + priv->is_stopped = false;
> + }
> +
> + return 0;
> +}
> +
> +static int linser_stop(struct lin_device *ldev)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +
> + if (priv->is_stopped)
> + return 0;
> +
> + serdev_device_close(serdev);
> + priv->is_stopped = true;
> +
> + return 0;
> +}
> +
> +static int linser_send_break(struct linser_priv *priv)
> +{
> + struct serdev_device *serdev = priv->serdev;
> + int ret;
> +
> + ret = serdev_device_break_ctl(serdev, -1);
> + if (ret)
> + return ret;
> +
> + usleep_range(priv->break_usleep_min, priv->break_usleep_max);
> +
> + ret = serdev_device_break_ctl(serdev, 0);
> + if (ret)
> + return ret;
> +
> + usleep_range(priv->post_break_usleep_min, priv->post_break_usleep_max);
> +
> + return 0;
> +}
> +
> +static int linser_ldo_tx(struct lin_device *ldev, const struct lin_frame *lf)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + u8 pid = LIN_FORM_PID(lf->lin_id);
> + u8 buf[LINSER_TX_BUFFER_SIZE];
> + ssize_t written_len, total_len;
> + u8 checksum;
> + int ret;
> +
> + if (lf->len + 3 > LINSER_TX_BUFFER_SIZE) {
> + dev_err(&serdev->dev, "Frame length %u exceeds buffer size\n", lf->len);
> + return -EINVAL;
> + }
> +
> + buf[0] = LIN_SYNC_BYTE;
> + buf[1] = pid;
> + total_len = 2;
> +
> + if (lf->len) {
> + memcpy(&buf[2], lf->data, lf->len);
> + checksum = lin_get_checksum(pid, lf->len, lf->data,
> + lf->checksum_mode);
> + buf[lf->len + 2] = checksum;
> + total_len += lf->len + 1;
> + }
> +
> + ret = linser_send_break(priv);
> + if (ret)
> + return ret;
> +
> + written_len = serdev_device_write(serdev, buf, total_len, 0);
> + if (written_len < total_len)
> + return written_len < 0 ? (int)written_len : -EIO;
> +
> + dev_dbg(&serdev->dev, "sent out: %*ph\n", (int)total_len, buf);
> +
> + serdev_device_wait_until_sent(serdev, 0);
> +
> + return 0;
> +}
> +
> +static void linser_derive_timings(struct linser_priv *priv, u16 bitrate)
> +{
> + unsigned long break_baud = (bitrate * 2) / 3;
> + unsigned long timeout_us;
> +
> + priv->break_usleep_min = (USEC_PER_SEC * LINSER_SAMPLES_PER_CHAR) /
> + break_baud;
> + priv->break_usleep_max = priv->break_usleep_min + 50;
> + priv->post_break_usleep_min = (USEC_PER_SEC * 1 /* 1 bit */) / break_baud;
Don't mix comments and code like this.
> + priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
> +
> + timeout_us = DIV_ROUND_CLOSEST(USEC_PER_SEC * 256 /* bit */, bitrate);
Ditto.
> + priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
> +}
> +
> +static int linser_update_bitrate(struct lin_device *ldev, u16 bitrate)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + unsigned int speed;
> + int ret;
> +
> + ret = linser_open(ldev);
> + if (ret)
> + return ret;
> +
> + speed = serdev_device_set_baudrate(serdev, bitrate);
> + if (!bitrate || speed != bitrate)
> + return -EINVAL;
> +
> + linser_derive_timings(priv, bitrate);
> +
> + return 0;
> +}
> +
> +static int linser_get_responder_answer(struct lin_device *ldev, u8 id,
> + struct lin_responder_answer *answ)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + struct lin_responder_answer *r = &priv->respond_answ[id];
> +
> + if (!answ)
> + return -EINVAL;
> +
> + mutex_lock(&priv->resp_lock);
guard(mutex)(&priv->resp_lock);
> + memcpy(answ, r, sizeof(struct lin_responder_answer));
sizeof(*answ);
> + mutex_unlock(&priv->resp_lock);
> +
> + return 0;
> +}
> +
> +static int linser_update_resp_answer(struct lin_device *ldev,
> + const struct lin_responder_answer *answ)
> +{
> + struct serdev_device *serdev = to_serdev_device(ldev->dev);
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + struct lin_responder_answer *r = &priv->respond_answ[answ->lf.lin_id];
> +
> + if (!answ)
> + return -EINVAL;
> +
> + mutex_lock(&priv->resp_lock);
> + memcpy(r, answ, sizeof(struct lin_responder_answer));
Ditto.
> + r->lf.checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> + answ->lf.len,
> + answ->lf.data,
> + answ->lf.checksum_mode);
> + mutex_unlock(&priv->resp_lock);
> +
> + return 0;
> +}
> +
> +static struct lin_device_ops linser_lindev_ops = {
> + .ldo_open = linser_open,
> + .ldo_stop = linser_stop,
> + .ldo_tx = linser_ldo_tx,
> + .update_bitrate = linser_update_bitrate,
> + .get_responder_answer = linser_get_responder_answer,
> + .update_responder_answer = linser_update_resp_answer,
> +};
> +
> +static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
> +{
> + struct lin_responder_answer *answ = &priv->respond_answ[id];
> + struct serdev_device *serdev = priv->serdev;
> + u8 buf[LINSER_TX_BUFFER_SIZE];
> + u8 checksum, count, n;
> + ssize_t write_len;
> +
> + mutex_lock(&priv->resp_lock);
scoped_guard() can be used here.
> +
> + if (!answ->is_active) {
> + mutex_unlock(&priv->resp_lock);
> + return false;
> + }
> +
> + if (answ->is_event_frame) {
> + struct lin_responder_answer *e_answ;
> +
> + e_answ = &priv->respond_answ[answ->event_associated_id];
> + n = min(e_answ->lf.len, LIN_MAX_DLEN);
> + if (memcmp(answ->lf.data, e_answ->lf.data, n) != 0) {
> + memcpy(answ->lf.data, e_answ->lf.data, n);
> + checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> + n, e_answ->lf.data,
> + answ->lf.checksum_mode);
> + answ = e_answ;
> + } else {
> + mutex_unlock(&priv->resp_lock);
> + return false;
Use reverse logic so you can reduce indent.
> + }
> + } else {
> + checksum = answ->lf.checksum;
> + }
> +
> + count = min(answ->lf.len, LIN_MAX_DLEN);
> + memcpy(&buf[0], answ->lf.data, count);
> + buf[count] = checksum;
> +
> + mutex_unlock(&priv->resp_lock);
> +
> + write_len = serdev_device_write(serdev, buf, count + 1, 0);
> + if (write_len < count + 1)
> + return false;
> +
> + serdev_device_wait_until_sent(serdev, 0);
> +
> + return true;
> +}
> +
> +static void linser_pop_fifo(struct linser_priv *priv, size_t n)
> +{
> + for (size_t i = 0; i < n; i++)
> + kfifo_skip(&priv->rx_fifo);
> +}
> +
> +static int linser_fill_frame(struct linser_priv *priv, struct lin_frame *lf)
> +{
> + struct serdev_device *serdev = priv->serdev;
> + struct linser_rx buf[LINSER_PARSE_BUFFER];
> + unsigned int count, i, brk = 0;
> +
> + count = kfifo_out_peek(&priv->rx_fifo, buf, LINSER_PARSE_BUFFER);
> +
> + memset(lf, 0, sizeof(struct lin_frame));
> +
> + for (i = 0; i < count; i++) {
> + dev_dbg(&serdev->dev, "buf[%d]: data=%02x, flag=%02x\n",
> + i, buf[i].data, buf[i].flag);
> + }
> +
> + if (count < 3)
> + return NEED_MORE;
> +
> + if (buf[0].flag != TTY_BREAK || buf[1].data != LIN_SYNC_BYTE) {
> + linser_pop_fifo(priv, 1); /* pop incorrect start */
> + return NEED_MORE;
> + } else if (!LIN_CHECK_PID(buf[2].data)) {
> + linser_pop_fifo(priv, 3); /* pop incorrect header */
> + return NEED_MORE;
> + }
> +
> + lf->lin_id = LIN_GET_ID(buf[2].data);
> +
> + /* from here on we do have a correct LIN header */
> +
> + if (count == 3)
> + return linser_tx_frame_as_responder(priv, lf->lin_id) ?
> + NEED_MORE : NEED_FORCE;
> +
> + for (i = 3; i < count && i < LINSER_PARSE_BUFFER && i < 12; i++) {
> + if (buf[i].flag == TTY_BREAK) {
> + brk = i;
> + break;
> + }
> + lf->len++;
> + }
> + if (lf->len)
> + lf->len -= 1; /* account for checksum */
> +
> + if (brk == 3)
> + return MODE_OK;
> +
> + if (brk == 4) {
> + /* suppress wrong answer data-byte in between PID and break
> + * because checksum is missing
> + */
> + return MODE_OK;
> + }
> +
> + for (i = 0; i < lf->len; i++)
> + lf->data[i] = buf[3 + i].data;
> + lf->checksum = buf[2 + lf->len + 1].data;
> + mutex_lock(&priv->resp_lock);
> + lf->checksum_mode = priv->respond_answ[lf->lin_id].lf.checksum_mode;
> + mutex_unlock(&priv->resp_lock);
> +
> + dev_dbg(&serdev->dev, "brk:%i, len:%u, data:%*ph, checksum:%x (%s)\n",
> + brk, lf->len, lf->len, lf->data, lf->checksum,
> + lf->checksum_mode ? "enhanced" : "classic");
> +
> + if (brk > 4)
> + return MODE_OK; /* frame in between two breaks: so complete */
> +
> + if (lf->len == 8)
> + return MODE_OK;
> +
> + return NEED_FORCE;
> +}
> +
> +static int linser_process_frame(struct linser_priv *priv, bool force)
> +{
> + struct serdev_device *serdev = priv->serdev;
> + struct lin_frame lf;
> + size_t bytes_to_pop;
> + int ret = NEED_MORE;
> +
> + while (kfifo_len(&priv->rx_fifo) >= LIN_HEADER_SIZE) {
> + ret = linser_fill_frame(priv, &lf);
> +
> + if (ret == MODE_OK || (ret == NEED_FORCE && force)) {
> + dev_dbg(&serdev->dev, "lin_rx: %s\n",
> + force ? "force" : "normal");
> + lin_rx(priv->lin_dev, &lf);
> + bytes_to_pop = LIN_HEADER_SIZE + lf.len +
> + (lf.len ? 1 : 0);
> + linser_pop_fifo(priv, bytes_to_pop);
> + force = false;
> + ret = MODE_OK;
> + } else {
> + return ret;
Reverse logic & deindent the main block.
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void linser_process_delayed(struct work_struct *work)
> +{
> + struct linser_priv *priv = container_of(work, struct linser_priv,
> + rx_work.work);
> +
> + linser_process_frame(priv, true);
> +}
> +
> +static ssize_t linser_receive_buf_fp(struct serdev_device *serdev,
> + const u8 *data, const u8 *fp,
> + size_t count)
> +{
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> + enum linser_rx_status rx_status;
> + ssize_t n = 0;
> + int i;
> +
> + cancel_delayed_work_sync(&priv->rx_work);
> +
> + for (i = 0; i < count; i++) {
> + struct linser_rx rx;
> +
> + rx.data = data[i];
> + rx.flag = (fp ? fp[i] : 0);
> + n += kfifo_in(&priv->rx_fifo, &rx, 1);
> + dev_dbg(&serdev->dev, "%s: n:%zd, flag:0x%02x, data:0x%02x\n",
> + __func__, n, rx.flag, data[i]);
> + }
> +
> + rx_status = linser_process_frame(priv, false);
> +
> + if (rx_status == NEED_FORCE)
> + schedule_delayed_work(&priv->rx_work, priv->force_timeout_jfs);
> +
> + return n;
> +}
> +
> +static const struct serdev_device_ops linser_ops = {
> + .receive_buf_fp = linser_receive_buf_fp,
> + .write_wakeup = serdev_device_write_wakeup,
> +};
> +
> +static int linser_probe(struct serdev_device *serdev)
> +{
> + struct linser_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&serdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = kfifo_alloc(&priv->rx_fifo, LINSER_RX_FIFO_SIZE, GFP_KERNEL);
> + if (ret)
> + return ret;
> +
> + INIT_DELAYED_WORK(&priv->rx_work, linser_process_delayed);
> +
> + priv->serdev = serdev;
> + serdev_device_set_drvdata(serdev, priv);
> + serdev_device_set_client_ops(serdev, &linser_ops);
> +
> + ret = serdev_device_open(serdev);
> + if (ret) {
> + dev_err(&serdev->dev, "Unable to open device\n");
> + goto err_open;
> + }
> +
> + serdev_device_set_flow_control(serdev, false);
> + serdev_device_set_break_detection(serdev, true);
> + linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
> +
> + mutex_init(&priv->resp_lock);
> +
> + priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
> + if (IS_ERR_OR_NULL(priv->lin_dev)) {
> + ret = PTR_ERR(priv->lin_dev);
> + goto err_register_lin;
> + }
> +
> + serdev_device_close(serdev);
> + priv->is_stopped = true;
> +
> + return 0;
> +
> +err_register_lin:
> + serdev_device_close(serdev);
> +err_open:
> + kfifo_free(&priv->rx_fifo);
> + return ret;
> +}
> +
> +static void linser_remove(struct serdev_device *serdev)
> +{
> + struct linser_priv *priv = serdev_device_get_drvdata(serdev);
> +
> + unregister_lin(priv->lin_dev);
> +}
> +
> +static const struct of_device_id linser_of_match[] = {
> + {
> + .compatible = "hexdev,lin-serdev",
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, linser_of_match);
> +
> +static struct serdev_device_driver linser_driver = {
> + .probe = linser_probe,
> + .remove = linser_remove,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = linser_of_match,
> + }
> +};
> +
> +module_serdev_device_driver(linser_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Christoph Fritz <christoph.fritz@hexdev.de>");
> +MODULE_DESCRIPTION("LIN-Bus serdev driver");
>
--
i.
^ permalink raw reply
* Re: [PATCH v3 08/11] can: bcm: Add LIN answer offloading for responder mode
From: Ilpo Järvinen @ 2024-05-06 17:08 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-9-christoph.fritz@hexdev.de>
On Thu, 2 May 2024, Christoph Fritz wrote:
> Enhance CAN broadcast manager with RX_LIN_SETUP and RX_LIN_DELETE
> operations to setup automatic LIN frame responses in responder mode.
>
> Additionally, the patch introduces the LIN_EVENT_FRAME flag to
> setup event-triggered LIN frames.
>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
> include/uapi/linux/can/bcm.h | 5 ++-
> net/can/bcm.c | 74 +++++++++++++++++++++++++++++++++++-
> 2 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h
> index f1e45f533a72c..c46268a114078 100644
> --- a/include/uapi/linux/can/bcm.h
> +++ b/include/uapi/linux/can/bcm.h
> @@ -86,7 +86,9 @@ enum {
> TX_EXPIRED, /* notification on performed transmissions (count=0) */
> RX_STATUS, /* reply to RX_READ request */
> RX_TIMEOUT, /* cyclic message is absent */
> - RX_CHANGED /* updated CAN frame (detected content change) */
> + RX_CHANGED, /* updated CAN frame (detected content change) */
> + RX_LIN_SETUP, /* create auto-response for LIN frame */
> + RX_LIN_DELETE, /* remove auto-response for LIN frame */
> };
>
> #define SETTIMER 0x0001
> @@ -101,5 +103,6 @@ enum {
> #define TX_RESET_MULTI_IDX 0x0200
> #define RX_RTR_FRAME 0x0400
> #define CAN_FD_FRAME 0x0800
> +#define LIN_EVENT_FRAME 0x1000
>
> #endif /* !_UAPI_CAN_BCM_H */
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 27d5fcf0eac9d..a717e594234d1 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -59,6 +59,7 @@
> #include <linux/can/bcm.h>
> #include <linux/slab.h>
> #include <net/sock.h>
> +#include <net/lin.h>
> #include <net/net_namespace.h>
>
> /*
> @@ -1330,6 +1331,59 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk,
> return cfsiz + MHSIZ;
> }
>
> +static int bcm_lin_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> + int ifindex, struct sock *sk, int cfsiz, int is_active)
> +{
> + struct lin_responder_answer answ;
> + struct net_device *dev;
> + struct sk_buff *skb;
> + struct canfd_frame cf;
> + netdevice_tracker tracker;
> + size_t sz;
> + int ret;
> +
> + if (msg_head->nframes > 1)
> + return -EINVAL;
> +
> + if (!(msg_head->flags & CAN_FD_FRAME))
> + return -EINVAL;
> +
> + ret = memcpy_from_msg(&cf, msg, cfsiz);
> + if (ret < 0)
> + return ret;
> +
> + answ.lf.lin_id = cf.can_id & LIN_ID_MASK;
> + answ.is_active = is_active;
> + answ.is_event_frame = !!(msg_head->flags & LIN_EVENT_FRAME);
> + answ.event_associated_id = msg_head->can_id;
> + answ.lf.len = min(cf.len, LIN_MAX_DLEN);
> + memcpy(answ.lf.data, cf.data, answ.lf.len);
> + sz = min(sizeof(struct lin_responder_answer), sizeof(cf.data));
> + cf.can_id |= LIN_RXOFFLOAD_DATA_FLAG;
> + memcpy(cf.data, &answ, sz);
> +
> + dev = netdev_get_by_index(sock_net(sk), ifindex, &tracker, GFP_KERNEL);
> + if (!dev)
> + return -ENODEV;
> +
> + skb = alloc_skb(cfsiz + sizeof(struct can_skb_priv), gfp_any());
You just called the other function with GFP_KERNEL and you now need
gfp_any(). Which is correct??
> + if (!skb)
> + goto lin_out;
> +
> + can_skb_reserve(skb);
> + can_skb_prv(skb)->ifindex = dev->ifindex;
> + can_skb_prv(skb)->skbcnt = 0;
> + skb_put_data(skb, &cf, cfsiz);
> +
> + skb->dev = dev;
> + can_skb_set_owner(skb, sk);
> + ret = can_send(skb, 1); /* send with loopback */
> +
> +lin_out:
> + netdev_put(dev, &tracker);
> + return ret;
> +}
> +
> /*
> * bcm_sendmsg - process BCM commands (opcodes) from the userspace
> */
> @@ -1429,12 +1483,30 @@ static int bcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
>
> case TX_SEND:
> /* we need exactly one CAN frame behind the msg head */
> - if ((msg_head.nframes != 1) || (size != cfsiz + MHSIZ))
> + if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
Unrelated style fix, doesn't belong to this patch.
> ret = -EINVAL;
> else
> ret = bcm_tx_send(msg, ifindex, sk, cfsiz);
> break;
>
> + case RX_LIN_SETUP:
> + /* we need exactly one CAN frame behind the msg head */
> + if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
> + ret = -EINVAL;
> + else
> + ret = bcm_lin_setup(&msg_head, msg, ifindex, sk, cfsiz,
> + 1);
> + break;
> +
> + case RX_LIN_DELETE:
> + /* we need exactly one CAN frame behind the msg head */
> + if (msg_head.nframes != 1 || size != cfsiz + MHSIZ)
> + ret = -EINVAL;
> + else
> + ret = bcm_lin_setup(&msg_head, msg, ifindex, sk, cfsiz,
> + 0);
> + break;
> +
> default:
> ret = -EINVAL;
> break;
>
--
i.
^ permalink raw reply
* Re: [PATCH v3 09/11] can: lin: Handle rx offload config frames
From: Ilpo Järvinen @ 2024-05-06 17:11 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-10-christoph.fritz@hexdev.de>
On Thu, 2 May 2024, Christoph Fritz wrote:
> The CAN Broadcast Manager now has the capability to dispatch CANFD
> frames marked with the id LINBUS_RXOFFLOAD_ID. This patch introduces
> functionality to interpret these specific frames, enabling the
> configuration of RX offloading within the LIN driver.
>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
> drivers/net/can/lin.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
> index 95906003666fb..ee2ebea2c865f 100644
> --- a/drivers/net/can/lin.c
> +++ b/drivers/net/can/lin.c
> @@ -194,6 +194,27 @@ static void lin_remove_sysfs_id_files(struct net_device *ndev)
> }
> }
>
> +static int lin_setup_rxoffload(struct lin_device *ldev,
> + struct canfd_frame *cfd)
> +{
> + struct lin_responder_answer answ;
> +
> + if (!(cfd->flags & CANFD_FDF))
> + return -EMSGSIZE;
This seems a bit odd error code.
> + BUILD_BUG_ON(sizeof(struct lin_responder_answer) > sizeof(cfd->data));
> + memcpy(&answ, cfd->data, sizeof(struct lin_responder_answer));
2x sizeof(answ)
> +
> + answ.lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
> + LINBUS_ENHANCED : LINBUS_CLASSIC;
> +
> + if (answ.lf.lin_id > LIN_ID_MASK ||
> + answ.event_associated_id > LIN_ID_MASK)
> + return -EINVAL;
> +
> + return ldev->ldev_ops->update_responder_answer(ldev, &answ);
> +}
> +
> static void lin_tx_work_handler(struct work_struct *ws)
> {
> struct lin_device *ldev = container_of(ws, struct lin_device,
> @@ -206,6 +227,14 @@ static void lin_tx_work_handler(struct work_struct *ws)
> ldev->tx_busy = true;
>
> cfd = (struct canfd_frame *)ldev->tx_skb->data;
> +
> + if (cfd->can_id & LIN_RXOFFLOAD_DATA_FLAG) {
> + ret = lin_setup_rxoffload(ldev, cfd);
> + if (ret < 0)
> + netdev_err(ndev, "setting up rx failed %d\n", ret);
> + goto lin_tx_out;
> + }
> +
> lf.checksum_mode = (cfd->can_id & LIN_ENHANCED_CKSUM_FLAG) ?
> LINBUS_ENHANCED : LINBUS_CLASSIC;
> lf.lin_id = cfd->can_id & LIN_ID_MASK;
>
--
i.
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: ektf2127 - add ektf2232 support
From: Andy Shevchenko @ 2024-05-06 18:43 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: <20240506182111.3c6673a0@aktux>
On Mon, May 6, 2024 at 7:21 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> 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>
> > Date: Mon, 6 May 2024 15:05:52 +0300
> > On Mon, May 6, 2024 at 12:48 AM Andreas Kemnade <andreas@kemnade.info> wrote:
...
> > 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.
And that's exactly what histogram is about. I suggest making it
default for the Linux kernel project (or globally in your
~/.gitconfig).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 06/11] dt-bindings: net/can: Add serial (serdev) LIN adapter
From: Krzysztof Kozlowski @ 2024-05-06 18:50 UTC (permalink / raw)
To: Conor Dooley, 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: <20240506-jaws-cheesy-bf94885651c1@spud>
On 06/05/2024 18:16, Conor Dooley wrote:
>>>> +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".
>
If there is no real, fixed model name, I would also propose to use
whatever is on the website currently and avoid Linuxism.
Best regards,
Krzysztof
^ permalink raw reply
* gamecube adapter driver
From: Milas Robin @ 2024-05-06 19:32 UTC (permalink / raw)
To: crange76; +Cc: dmitry.torokhov, linux-input, milas.robin
In-Reply-To: <4864668.OV4Wx5bFTl@mythra>
Hi,
I don't think so, I didn't received any reply so I guess it is not ?
If you have some time to review the code I would be grateful.
As I said in the cover letter (which didn't linked correctly), this is
my first kernel module so there might be some basic architecture notably
on race conditions.
Also due to the way I've implemented it, if it is build as a module
it lose priority over the usb-hid generic driver as the device report
supporting hid it.
One other thing which might make the code heavier to read is properly detecting
if the controller really support rumbles, I actually ignore the bit signalling
it cause I don't know if the adapter could change it without unplugging the
controller which might lead to have to recreate the device to switch it's
rumbling capability
The patch seems to have no conflict with the current input/next branch.
If you need a branch I've uploaded it on https://github.com/Hinara/linux.git
The branch is called ngc.
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: ektf2127 - add ektf2232 support
From: Andreas Kemnade @ 2024-05-06 20:29 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: <CAHp75VchLWQgmdxKPSbwH-m43zFHT9ADk4aH7-jvD5-MaVOtEw@mail.gmail.com>
On Mon, 6 May 2024 21:43:14 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Mon, May 6, 2024 at 7:21 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> > 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>
> > > Date: Mon, 6 May 2024 15:05:52 +0300
> > > On Mon, May 6, 2024 at 12:48 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> ...
>
> > > 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.
>
> And that's exactly what histogram is about. I suggest making it
> default for the Linux kernel project (or globally in your
> ~/.gitconfig).
>
>
again, it does not do anything helpful in this case, I tried to run
git format-patch --histogram with
no improvements. But it might help in other places.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH v2] HID: hid-steam: Add Deck IMU support
From: Jiri Kosina @ 2024-05-06 21:10 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Max Maisel, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <80573947-3f6b-4be7-b5cd-999a2113a434@endrift.com>
On Tue, 23 Apr 2024, Vicki Pfau wrote:
> > The Deck's controller features an accelerometer and gyroscope which
> > send their measurement values by default in the main HID input report.
> > Expose both sensors to userspace through a separate evdev node as it
> > is done by the hid-nintendo and hid-playstation drivers.
> >
> > Signed-off-by: Max Maisel <mmm-1@posteo.net>
[ .. snip ... ]
> Looks good.
>
> Reviewed-by: Vicki Pfau <vi@endrift.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Add check for pci_alloc_irq_vectors
From: Jiri Kosina @ 2024-05-06 21:11 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Chen Ni, bentiss, even.xu, lixu.zhang, kai.heng.feng,
hongyan.song, linux-input, linux-kernel
In-Reply-To: <95fadbf4772d575bff777ddb738cb6c25b85b779.camel@linux.intel.com>
On Thu, 2 May 2024, srinivas pandruvada wrote:
> On Mon, 2024-04-29 at 16:54 +0800, Chen Ni wrote:
> > Add check for the return value of pci_alloc_irq_vectors() and return
> > the error if it fails in order to catch the error.
> >
> You can write as
> "
> Add a check for the return value of pci_alloc_irq_vectors() and return
> error if it fails.
> "
Srinivas,
my understanding is that with the changelog rewroding this patch has your
Ack?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: Add quirk for Logitech Casa touchpad
From: Jiri Kosina @ 2024-05-06 21:14 UTC (permalink / raw)
To: Sean O'Brien; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20240429180804.1.Ie7e8d0ba595f9e39f71cbe4ab3468f79c00b4581@changeid>
On Mon, 29 Apr 2024, Sean O'Brien wrote:
> This device sometimes doesn't send touch release signals when moving
> from >=4 fingers to <4 fingers. Using MT_QUIRK_NOT_SEEN_MEANS_UP instead
> of MT_QUIRK_ALWAYS_VALID makes sure that no touches become stuck.
>
> MT_QUIRK_FORCE_MULTI_INPUT is not necessary for this device, but does no
> harm.
>
> Signed-off-by: Sean O'Brien <seobrien@chromium.org>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-debug: add EV_FF and FF_STATUS mappings
From: Jiri Kosina @ 2024-05-06 21:17 UTC (permalink / raw)
To: Thomas Kuehne; +Cc: bentiss, linux-input, linux-kernel
In-Reply-To: <Zhp89Q8k13Uua24f@black>
On Sat, 13 Apr 2024, Thomas Kuehne wrote:
> Currently hid-debug only output question marks for all force
> feedback related input mapping making debugging gamepads
> with force feedback a challenge.
>
> This adds the necessary mapping information to output
> EV_FF and FF_STATUS related information.
>
> Signed-off-by: Thomas Kuehne <thomas.kuehne@gmx.li>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-debug: fix Moir -> Moire typo
From: Jiri Kosina @ 2024-05-06 21:17 UTC (permalink / raw)
To: Thomas Kuehne; +Cc: bentiss, linux-input
In-Reply-To: <Zhp4o/lIvW3bJ2kF@black>
On Sat, 13 Apr 2024, Thomas Kuehne wrote:
> This adds the letter "e" to fix hid_usage_table' HorizontalMoir and
> VerticalMoir entries.
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-debug: more informative output for EV_KEY
From: Jiri Kosina @ 2024-05-06 21:17 UTC (permalink / raw)
To: Thomas Kuehne; +Cc: bentiss, linux-input, linux-kernel
In-Reply-To: <Zhp5QlMGazcW+RV5@black>
On Sat, 13 Apr 2024, Thomas Kuehne wrote:
> Currently hid-debug's hid_resolv_event prints questions marks for
> all entries without explicit mapping information. This makes
> debugging unnecessarily complicated as multiple different
> keys may simply result in the same uninformative output.
>
> Some common event codes are deliberately not defined in
> input-event-codes.h. For example the 16th gamepad key.
>
> Instead, print the hexadecimal codes for all events without symbolic
> names.
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Add Logitech HID++ devices
From: Jiri Kosina @ 2024-05-06 21:25 UTC (permalink / raw)
To: Allan Sandfeld Jensen
Cc: lains, hadess, benjamin.tissoires, linux-input, linux-kernel,
Allan Sandfeld Jensen
In-Reply-To: <20240424113130.60386-1-kde@carewolf.com>
On Wed, 24 Apr 2024, kde@carewolf.com wrote:
> From: Allan Sandfeld Jensen <allan.jensen@qt.io>
>
> Adds a few recognized Logitech HID++ capable mice over USB and Bluetooth
>
> Signed-off-by: Allan Sandfeld Jensen <kde@carewolf.com>
Thanks for the patch!
Two nits:
- the shortlog (subject) of the patch didn't really follow the pattern we
are using in HID subsystem. I have fixed it, but please take care of
that in your next submissions.
- the patch was against some old version of the code, so it didn't apply
cleanly (more entries are now present in hidpp_devices[]). I've fixed
that manually, please cross-check the end result in the git branch
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/4] HID: Asus
From: Jiri Kosina @ 2024-05-06 21:26 UTC (permalink / raw)
To: Luke D. Jones; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20240416090402.31057-1-luke@ljones.dev>
On Tue, 16 Apr 2024, Luke D. Jones wrote:
> This is a small series of patches that does some cleanup and also adds some
> initialisation support for the ROG Ally and the ROG X13 devices. The series is
> a partial prep fro a *much* larger 2k line patch adding full feature support
> for the ROG Ally - this patch isn't included as I don't want to hold up this
> small series during review.
>
> Luke D. Jones (4):
> HID: asus: fix more n-key report descriptors if n-key quirked
> HID: asus: make asus_kbd_init() generic, remove rog_nkey_led_init()
> HID: asus: add ROG Ally N-Key ID and keycodes
> HID: asus: add ROG Z13 lightbar
>
> drivers/hid/hid-asus.c | 132 ++++++++++++++++-------------------------
> drivers/hid/hid-ids.h | 2 +
> 2 files changed, 54 insertions(+), 80 deletions(-)
Now queued in hid.git#for-6.10/asus.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] HID: i2c-hid: Unify device wake-up logic
From: Jiri Kosina @ 2024-05-06 21:28 UTC (permalink / raw)
To: Kenny Levinsen
Cc: Dmitry Torokhov, Benjamin Tissoires, Douglas Anderson,
Hans de Goede, Maxime Ripard, Kai-Heng Feng, Johan Hovold,
linux-input, linux-kernel, Radoslaw Biernacki, Lukasz Majczak
In-Reply-To: <20240429233924.6453-1-kl@kl.wtf>
On Tue, 30 Apr 2024, Kenny Levinsen wrote:
> Third time was not the charm[0]. After Dmitry's comment[1], and after
> looking some more at the I2C drivers, I have given up on removing the
> smbus probe for now. We can always revisit this later if the situation
> improves enough, but there are more important things to fix.
>
> Instead, go all in on the address probe with retry and use it for both
> initial probe and resume, replacing the previous retry on power on
> commands. This gives us consistency and a single place to update and
> document.
>
> [0]: https://lore.kernel.org/all/20240426225739.2166-1-kl@kl.wtf/
> [1]: https://lore.kernel.org/all/ZixvUNooESC02cJK@google.com/
I really apologize for belated response to all this, I've been completely
swamped for the past two weeks.
This is now queued in hid.git#for-6.10/i2c-hid.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: kye: Change Device Usage from Puck to Mouse
From: Jiri Kosina @ 2024-05-06 21:30 UTC (permalink / raw)
To: David Yang; +Cc: linux-input, Benjamin Tissoires, linux-kernel, Milan Plzik
In-Reply-To: <20240502042335.15611-1-mmyangfl@gmail.com>
On Thu, 2 May 2024, David Yang wrote:
> Change device type because
> a. it is exactly a mouse, with left/right buttons and scroll wheel;
> b. it does not have visible marks or crosshairs, thus does not provide
> higher accuracy than stylus.
Let's CC Milan, who originally added all this in feb6faf1e5d46 ("HID: kye:
Fix report descriptor for Genius PenSketch M912") ... Milan, any concerns
about the below?
Thanks.
>
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
> drivers/hid/hid-kye.c | 75 +++++++++++++++++++++++++------------------
> 1 file changed, 44 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c
> index eb9bf2829937..70ceb9437332 100644
> --- a/drivers/hid/hid-kye.c
> +++ b/drivers/hid/hid-kye.c
> @@ -209,7 +209,7 @@ static const __u8 pensketch_t609a_control_rdesc[] = {
> 0xC0 /* End Collection */
> };
>
> -/* Fix indexes in kye_tablet_fixup if you change this */
> +/* Fix indexes in kye_tablet_fixup() if you change this */
> static const __u8 kye_tablet_rdesc[] = {
> 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> 0x09, 0x01, /* Usage (01h), */
> @@ -262,12 +262,16 @@ static const __u8 kye_tablet_rdesc[] = {
> 0x27, 0xFF, 0x07, 0x00, 0x00, /* Logical Maximum (2047), */
> 0x81, 0x02, /* Input (Variable), */
> 0xC0, /* End Collection, */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x21, /* Usage (Puck), */
> + 0xC0 /* End Collection, */
> +};
> +
> +/* Fix indexes in kye_tablet_fixup() if you change this */
> +static const __u8 kye_tablet_mouse_rdesc[] = {
> + 0x05, 0x01, /* Usage Page (Desktop), */
> + 0x09, 0x02, /* Usage (Mouse), */
> 0xA1, 0x01, /* Collection (Application), */
> 0x85, 0x11, /* Report ID (17), */
> - 0x09, 0x21, /* Usage (Puck), */
> + 0x09, 0x01, /* Usage (Pointer), */
> 0xA0, /* Collection (Physical), */
> 0x05, 0x09, /* Usage Page (Button), */
> 0x19, 0x01, /* Usage Minimum (01h), */
> @@ -280,7 +284,7 @@ static const __u8 kye_tablet_rdesc[] = {
> 0x95, 0x04, /* Report Count (4), */
> 0x81, 0x01, /* Input (Constant), */
> 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x32, /* Usage (In Range), */
> + 0x09, 0x37, /* Usage (Data Valid), */
> 0x95, 0x01, /* Report Count (1), */
> 0x81, 0x02, /* Input (Variable), */
> 0x05, 0x01, /* Usage Page (Desktop), */
> @@ -317,7 +321,7 @@ static const struct kye_tablet_info {
> __s32 y_physical_maximum;
> __s8 unit_exponent;
> __s8 unit;
> - bool has_punk;
> + bool has_mouse;
> unsigned int control_rsize;
> const __u8 *control_rdesc;
> } kye_tablets_info[] = {
> @@ -402,7 +406,7 @@ static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc,
> static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize)
> {
> const struct kye_tablet_info *info;
> - unsigned int newsize;
> + __u8 *newdesc = rdesc;
>
> if (*rsize < sizeof(kye_tablet_rdesc)) {
> hid_warn(hdev,
> @@ -420,36 +424,45 @@ static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int
> return rdesc;
> }
>
> - newsize = info->has_punk ? sizeof(kye_tablet_rdesc) : 112;
> - memcpy(rdesc, kye_tablet_rdesc, newsize);
> -
> - put_unaligned_le32(info->x_logical_maximum, rdesc + 66);
> - put_unaligned_le32(info->x_physical_maximum, rdesc + 72);
> - rdesc[77] = info->unit;
> - rdesc[79] = info->unit_exponent;
> - put_unaligned_le32(info->y_logical_maximum, rdesc + 87);
> - put_unaligned_le32(info->y_physical_maximum, rdesc + 92);
> - put_unaligned_le32(info->pressure_logical_maximum, rdesc + 104);
> -
> - if (info->has_punk) {
> - put_unaligned_le32(info->x_logical_maximum, rdesc + 156);
> - put_unaligned_le32(info->x_physical_maximum, rdesc + 162);
> - rdesc[167] = info->unit;
> - rdesc[169] = info->unit_exponent;
> - put_unaligned_le32(info->y_logical_maximum, rdesc + 177);
> - put_unaligned_le32(info->y_physical_maximum, rdesc + 182);
> + memcpy(newdesc, kye_tablet_rdesc, sizeof(kye_tablet_rdesc));
> +
> + put_unaligned_le32(info->x_logical_maximum, newdesc + 66);
> + put_unaligned_le32(info->x_physical_maximum, newdesc + 72);
> + newdesc[77] = info->unit;
> + newdesc[79] = info->unit_exponent;
> + put_unaligned_le32(info->y_logical_maximum, newdesc + 87);
> + put_unaligned_le32(info->y_physical_maximum, newdesc + 92);
> + put_unaligned_le32(info->pressure_logical_maximum, newdesc + 104);
> +
> + newdesc += sizeof(kye_tablet_rdesc);
> +
> + if (info->has_mouse) {
> + if (newdesc + sizeof(kye_tablet_mouse_rdesc) > rdesc + *rsize)
> + hid_err(hdev, "control desc unexpectedly large\n");
> + else {
> + memcpy(newdesc, kye_tablet_mouse_rdesc, sizeof(kye_tablet_mouse_rdesc));
> +
> + put_unaligned_le32(info->x_logical_maximum, newdesc + 44);
> + put_unaligned_le32(info->x_physical_maximum, newdesc + 50);
> + newdesc[55] = info->unit;
> + newdesc[57] = info->unit_exponent;
> + put_unaligned_le32(info->y_logical_maximum, newdesc + 65);
> + put_unaligned_le32(info->y_physical_maximum, newdesc + 70);
> +
> + newdesc += sizeof(kye_tablet_mouse_rdesc);
> + }
> }
>
> if (info->control_rsize) {
> - if (newsize + info->control_rsize > *rsize)
> - hid_err(hdev, "control rdesc unexpectedly large");
> + if (newdesc + info->control_rsize > rdesc + *rsize)
> + hid_err(hdev, "control desc unexpectedly large\n");
> else {
> - memcpy(rdesc + newsize, info->control_rdesc, info->control_rsize);
> - newsize += info->control_rsize;
> + memcpy(newdesc, info->control_rdesc, info->control_rsize);
> + newdesc += info->control_rsize;
> }
> }
>
> - *rsize = newsize;
> + *rsize = newdesc - rdesc;
> return rdesc;
> }
>
> --
> 2.43.0
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: playstation: DS4: Fix calibration workaround for clone devices
From: Jiri Kosina @ 2024-05-06 21:32 UTC (permalink / raw)
To: Max Staudt
Cc: Roderick Colenbrander, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20240504165531.21315-1-max@enpas.org>
On Sun, 5 May 2024, Max Staudt wrote:
> The logic in dualshock4_get_calibration_data() used uninitialised data
> in case of a failed kzalloc() for the transfer buffer.
Good catch, this completely escaped my attention during review.
>
> The solution is to group all business logic and all sanity checks
> together, and jump only to the latter in case of an error.
> While we're at it, factor out the axes' labelling, since it must happen
> either way for input_report_abs() to succeed later on.
>
> Thanks to Dan Carpenter for the Smatch static checker warning.
>
> Fixes: a48a7cd85f55 ("HID: playstation: DS4: Don't fail on calibration data request")
> Signed-off-by: Max Staudt <max@enpas.org>
Now queued on top of the for-6.10/playstation pile in hid.git.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/5] HID: intel-ish-hid: Implement loading firmware from host feature
From: Jiri Kosina @ 2024-05-06 21:34 UTC (permalink / raw)
To: Zhang Lixu; +Cc: linux-input, srinivas.pandruvada, benjamin.tissoires
In-Reply-To: <20240506013040.10700-1-lixu.zhang@intel.com>
On Mon, 6 May 2024, Zhang Lixu wrote:
> This patch series is comprised of 5 patches. The first two patches are to add documentation
> for firmware loading. The third and fourth patches introduce support for the 'Load Main
> Firmware from Host' feature in the ISHTP driver, applicable to Lunar Lake and subsequent
> platforms. The last patch enhances the firmware reset handler in the ISH driver. This
> addresses an issue where the driver receives two MNG_RESET_NOTIFY messages upon implementing
> the 'Load Main Firmware from Host' feature.
>
> This patch series is based on the following 3 commits, which have already been included in the linux-next/master branch.
> - HID: intel-ish-hid: ipc: Fix dev_err usage with uninitialized dev->devc
> - HID: intel-ish-hid: Use PCI_VDEVICE() and rename device ID macros
> - HID: intel-ish-hid: ipc: Add Lunar Lake-M PCI device ID
>
> Qianru Huang (2):
> Documentation: hid: intel-ish-hid: remove section numbering
> Documentation: hid: intel-ish-hid: add section for firmware loading
>
> Zhang Lixu (3):
> HID: intel-ish-hid: Add driver_data for specifying the firmware
> filename
> HID: intel-ish-hid: Implement loading firmware from host feature
> HID: intel-ish-hid: handler multiple MNG_RESET_NOTIFY messages
This is now queued in hid.git#for-6.10/intel-ish.
Thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] bpftool, selftests/hid/bpf: fix 29 clang warnings
From: patchwork-bot+netdevbpf @ 2024-05-06 21:50 UTC (permalink / raw)
To: John Hubbard
Cc: shuah, jikos, bentiss, justinstitt, peter.hutterer, jason.gerecke,
joshua, qmo, ast, daniel, andrii, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf,
linux-input, kernel, linux-kselftest, linux-kernel, llvm
In-Reply-To: <20240505230054.13813-1-jhubbard@nvidia.com>
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Sun, 5 May 2024 16:00:54 -0700 you wrote:
> When building either tools/bpf/bpftool, or tools/testing/selftests/hid,
> (the same Makefile is used for these), clang generates many instances of
> the following:
>
> "clang: warning: -lLLVM-17: 'linker' input unused"
>
> Quentin points out that the LLVM version is only required in $(LIBS),
> not in $(CFLAGS), so the fix is to remove it from CFLAGS.
>
> [...]
Here is the summary with links:
- [v2] bpftool, selftests/hid/bpf: fix 29 clang warnings
https://git.kernel.org/bpf/bpf-next/c/41b307ad756e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Add check for pci_alloc_irq_vectors
From: srinivas pandruvada @ 2024-05-06 21:52 UTC (permalink / raw)
To: Jiri Kosina
Cc: Chen Ni, bentiss, even.xu, lixu.zhang, kai.heng.feng,
hongyan.song, linux-input, linux-kernel
In-Reply-To: <nycvar.YFH.7.76.2405062311100.16865@cbobk.fhfr.pm>
On Mon, 2024-05-06 at 23:11 +0200, Jiri Kosina wrote:
> On Thu, 2 May 2024, srinivas pandruvada wrote:
>
> > On Mon, 2024-04-29 at 16:54 +0800, Chen Ni wrote:
> > > Add check for the return value of pci_alloc_irq_vectors() and
> > > return
> > > the error if it fails in order to catch the error.
> > >
> > You can write as
> > "
> > Add a check for the return value of pci_alloc_irq_vectors() and
> > return
> > error if it fails.
> > "
>
Hi Jiri,
> Srinivas,
>
> my understanding is that with the changelog rewroding this patch has
> your
> Ack?
Yes, just to make it more clear. With that.
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Thanks,
Srinivas
>
> Thanks,
>
^ permalink raw reply
* [PATCH v9 0/8] VMware hypercalls enhancements
From: Alexey Makhalov @ 2024-05-06 21:52 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, timothym, akaher,
dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov, Alexey Makhalov
In-Reply-To: <20240505182829.GBZjfPzeEijTsBUth5@fat_crate.local>
VMware hypercalls invocations were all spread out across the kernel
implementing same ABI as in-place asm-inline. With encrypted memory
and confidential computing it became harder to maintain every changes
in these hypercall implementations.
Intention of this patchset is to introduce arch independent VMware
hypercall API layer other subsystems such as device drivers can call
to, while hiding architecture specific implementation behind.
Second patch introduces the vmware_hypercall low and high bandwidth
families of functions, with little enhancements there.
Sixth patch adds tdx hypercall support
arm64 implementation of vmware_hypercalls is in drivers/gpu/drm/
vmwgfx/vmwgfx_msg_arm64.h and going to be moved to arch/arm64 with
a separate patchset with the introduction of VMware Linux guest
support for arm64.
No functional changes in drivers/input/mouse/vmmouse.c and
drivers/ptp/ptp_vmw.c
v8->v9 change:
First patch "x86/vmware: Move common macros to vmware.h" was split on 2 pieces:
"x86/vmware: Move common macros to vmware.h" - just code movement, and
"x86/vmware: Correct macro names" - macro renaming.
v7->v8 no functional changes. Updated authors and reviewers emails to
@broadcom.com
v6->v7 changes (only in patch 7):
- Addressed comments from H. Peter Anvin:
1. Removed vmware_tdx_hypercall_args(), moved args handling inside
vmware_tdx_hypercall().
2. Added pr_warn_once() for !hypervisor_is_type(X86_HYPER_VMWARE) case.
- Added ack by Dave Hansen.
v5->v6 change:
- Added ack by Kirill A. Shutemov in patch 7.
v4->v5 changes:
[patch 2]:
- Fixed the problem reported by Simon Horman where build fails after
patch 2 application. Do not undefine VMWARE_HYPERCALL for now, and
update vmwgfx, vmmouse and ptp_vmw code for new VMWARE_HYPERCALL macro.
- Introduce new patch 6 to undefine VMWARE_HYPERCALL, which is safe to do
after patches 3 to 5.
- [patch 7 (former patch 6)]: Add missing r15 (CPL) initialization.
v3->v4 changes: (no functional changes in patches 1-5)
[patch 2]:
- Added the comment with VMware hypercall ABI description.
[patch 6]:
- vmware_tdx_hypercall_args remove in6/out6 arguments as excessive.
- vmware_tdx_hypercall return ULONG_MAX on error to mimic bad hypercall
command error from the hypervisor.
- Replaced pr_warn by pr_warn_once as pointed by Kirill Shutemov.
- Fixed the warning reported by Intel's kernel test robot.
- Added the comment describing VMware TDX hypercall ABI.
v2->v3 changes: (no functional changes in patches 1-5)
- Improved commit message in patches 1, 2 and 5 as was suggested by
Borislav Petkov.
- To address Dave Hansen's concern, patch 6 was reorganized to avoid
exporting bare __tdx_hypercall and to make exported vmware_tdx_hypercall
VMWare guest specific.
v1->v2 changes (no functional changes):
- Improved commit message in patches 2 and 5.
- Added Reviewed-by for all patches.
- Added Ack from Dmitry Torokhov in patch 4. No fixes regarding reported
by Simon Horman gcc error in this patch.
Alexey Makhalov (8):
x86/vmware: Move common macros to vmware.h
x86/vmware: Correct macro names
x86/vmware: Introduce VMware hypercall API
ptp/vmware: Use VMware hypercall API
input/vmmouse: Use VMware hypercall API
drm/vmwgfx: Use VMware hypercall API
x86/vmware: Undefine VMWARE_HYPERCALL
x86/vmware: Add TDX hypercall support
arch/x86/include/asm/vmware.h | 331 +++++++++++++++++++---
arch/x86/kernel/cpu/vmware.c | 144 +++++-----
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 173 ++++-------
drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h | 197 +++++++++----
drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h | 185 ------------
drivers/input/mouse/vmmouse.c | 76 ++---
drivers/ptp/ptp_vmw.c | 12 +-
7 files changed, 593 insertions(+), 525 deletions(-)
--
2.39.0
^ permalink raw reply
* [PATCH v9 1/8] x86/vmware: Move common macros to vmware.h
From: Alexey Makhalov @ 2024-05-06 21:52 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, timothym, akaher,
dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov, Alexey Makhalov,
Nadav Amit
In-Reply-To: <20240506215305.30756-1-alexey.makhalov@broadcom.com>
Move VMware hypercall macros to vmware.h. This is a prerequisite for
the introduction of vmware_hypercall API. No functional changes besides
exporting vmware_hypercall_mode symbol.
Signed-off-by: Alexey Makhalov <alexey.makhalov@broadcom.com>
Reviewed-by: Nadav Amit <nadav.amit@gmail.com>
---
arch/x86/include/asm/vmware.h | 72 +++++++++++++++++++++++++++++------
arch/x86/kernel/cpu/vmware.c | 49 +-----------------------
2 files changed, 62 insertions(+), 59 deletions(-)
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index ac9fc51e2b18..de2533337611 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -8,25 +8,34 @@
/*
* The hypercall definitions differ in the low word of the %edx argument
- * in the following way: the old port base interface uses the port
- * number to distinguish between high- and low bandwidth versions.
+ * in the following way: the old I/O port based interface uses the port
+ * number to distinguish between high- and low bandwidth versions, and
+ * uses IN/OUT instructions to define transfer direction.
*
* The new vmcall interface instead uses a set of flags to select
* bandwidth mode and transfer direction. The flags should be loaded
* into %dx by any user and are automatically replaced by the port
- * number if the VMWARE_HYPERVISOR_PORT method is used.
- *
- * In short, new driver code should strictly use the new definition of
- * %dx content.
+ * number if the I/O port method is used.
*/
-/* Old port-based version */
-#define VMWARE_HYPERVISOR_PORT 0x5658
-#define VMWARE_HYPERVISOR_PORT_HB 0x5659
+#define VMWARE_HYPERVISOR_HB BIT(0)
+#define VMWARE_HYPERVISOR_OUT BIT(1)
+
+#define VMWARE_HYPERVISOR_PORT 0x5658
+#define VMWARE_HYPERVISOR_PORT_HB (VMWARE_HYPERVISOR_PORT | \
+ VMWARE_HYPERVISOR_HB)
+
+#define VMWARE_HYPERVISOR_MAGIC 0x564d5868U
+
+#define VMWARE_CMD_GETVERSION 10
+#define VMWARE_CMD_GETHZ 45
+#define VMWARE_CMD_GETVCPU_INFO 68
+#define VMWARE_CMD_STEALCLOCK 91
+
+#define CPUID_VMWARE_FEATURES_ECX_VMMCALL BIT(0)
+#define CPUID_VMWARE_FEATURES_ECX_VMCALL BIT(1)
-/* Current vmcall / vmmcall version */
-#define VMWARE_HYPERVISOR_HB BIT(0)
-#define VMWARE_HYPERVISOR_OUT BIT(1)
+extern u8 vmware_hypercall_mode;
/* The low bandwidth call. The low word of edx is presumed clear. */
#define VMWARE_HYPERCALL \
@@ -54,4 +63,43 @@
"rep insb", \
"vmcall", X86_FEATURE_VMCALL, \
"vmmcall", X86_FEATURE_VMW_VMMCALL)
+
+#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
+ __asm__("inl (%%dx), %%eax" : \
+ "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
+ "a"(VMWARE_HYPERVISOR_MAGIC), \
+ "c"(VMWARE_CMD_##cmd), \
+ "d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) : \
+ "memory")
+
+#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx) \
+ __asm__("vmcall" : \
+ "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
+ "a"(VMWARE_HYPERVISOR_MAGIC), \
+ "c"(VMWARE_CMD_##cmd), \
+ "d"(0), "b"(UINT_MAX) : \
+ "memory")
+
+#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx) \
+ __asm__("vmmcall" : \
+ "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
+ "a"(VMWARE_HYPERVISOR_MAGIC), \
+ "c"(VMWARE_CMD_##cmd), \
+ "d"(0), "b"(UINT_MAX) : \
+ "memory")
+
+#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do { \
+ switch (vmware_hypercall_mode) { \
+ case CPUID_VMWARE_FEATURES_ECX_VMCALL: \
+ VMWARE_VMCALL(cmd, eax, ebx, ecx, edx); \
+ break; \
+ case CPUID_VMWARE_FEATURES_ECX_VMMCALL: \
+ VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx); \
+ break; \
+ default: \
+ VMWARE_PORT(cmd, eax, ebx, ecx, edx); \
+ break; \
+ } \
+ } while (0)
+
#endif
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 11f83d07925e..68d812e12e52 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -41,60 +41,14 @@
#define CPUID_VMWARE_INFO_LEAF 0x40000000
#define CPUID_VMWARE_FEATURES_LEAF 0x40000010
-#define CPUID_VMWARE_FEATURES_ECX_VMMCALL BIT(0)
-#define CPUID_VMWARE_FEATURES_ECX_VMCALL BIT(1)
-#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
-
-#define VMWARE_CMD_GETVERSION 10
-#define VMWARE_CMD_GETHZ 45
-#define VMWARE_CMD_GETVCPU_INFO 68
#define VMWARE_CMD_LEGACY_X2APIC 3
#define VMWARE_CMD_VCPU_RESERVED 31
-#define VMWARE_CMD_STEALCLOCK 91
#define STEALCLOCK_NOT_AVAILABLE (-1)
#define STEALCLOCK_DISABLED 0
#define STEALCLOCK_ENABLED 1
-#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
- __asm__("inl (%%dx), %%eax" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx) \
- __asm__("vmcall" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(0), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx) \
- __asm__("vmmcall" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(0), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do { \
- switch (vmware_hypercall_mode) { \
- case CPUID_VMWARE_FEATURES_ECX_VMCALL: \
- VMWARE_VMCALL(cmd, eax, ebx, ecx, edx); \
- break; \
- case CPUID_VMWARE_FEATURES_ECX_VMMCALL: \
- VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx); \
- break; \
- default: \
- VMWARE_PORT(cmd, eax, ebx, ecx, edx); \
- break; \
- } \
- } while (0)
-
struct vmware_steal_time {
union {
uint64_t clock; /* stolen time counter in units of vtsc */
@@ -108,7 +62,8 @@ struct vmware_steal_time {
};
static unsigned long vmware_tsc_khz __ro_after_init;
-static u8 vmware_hypercall_mode __ro_after_init;
+u8 vmware_hypercall_mode __ro_after_init;
+EXPORT_SYMBOL_GPL(vmware_hypercall_mode);
static inline int __vmware_platform(void)
{
--
2.39.0
^ permalink raw reply related
* [PATCH v9 2/8] x86/vmware: Correct macro names
From: Alexey Makhalov @ 2024-05-06 21:52 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, timothym, akaher,
dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov, Alexey Makhalov
In-Reply-To: <20240506215305.30756-1-alexey.makhalov@broadcom.com>
VCPU_RESERVED and LEGACY_X2APIC are not VMware hypercall commands.
These are bits in return value of VMWARE_CMD_GETVCPU_INFO command.
Change VMWARE_CMD_ prefix to GETVCPU_INFO_ one. And move bit-shift
operation to the macro body.
Signed-off-by: Alexey Makhalov <alexey.makhalov@broadcom.com>
---
arch/x86/kernel/cpu/vmware.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 68d812e12e52..9d804d60a11f 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -42,8 +42,8 @@
#define CPUID_VMWARE_INFO_LEAF 0x40000000
#define CPUID_VMWARE_FEATURES_LEAF 0x40000010
-#define VMWARE_CMD_LEGACY_X2APIC 3
-#define VMWARE_CMD_VCPU_RESERVED 31
+#define GETVCPU_INFO_LEGACY_X2APIC BIT(3)
+#define GETVCPU_INFO_VCPU_RESERVED BIT(31)
#define STEALCLOCK_NOT_AVAILABLE (-1)
#define STEALCLOCK_DISABLED 0
@@ -431,8 +431,8 @@ static bool __init vmware_legacy_x2apic_available(void)
{
uint32_t eax, ebx, ecx, edx;
VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx);
- return !(eax & BIT(VMWARE_CMD_VCPU_RESERVED)) &&
- (eax & BIT(VMWARE_CMD_LEGACY_X2APIC));
+ return !(eax & GETVCPU_INFO_VCPU_RESERVED) &&
+ (eax & GETVCPU_INFO_LEGACY_X2APIC);
}
#ifdef CONFIG_AMD_MEM_ENCRYPT
--
2.39.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox