[PATCH 2/2] mei: Add MEI hardware support for IVSC device

Wentong Wu posted 2 patches 2 years ago
[PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Wentong Wu 2 years ago
The protocol used for the IVSC device to communicate with HOST is MEI.
The MEI hardware interfaces for the IVSC device are implemented.

The APIs are exposed by MEI framework to mei clients, e.g. mei_csi and
mei_ace.

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Reviewed-by: Alexander Usyskin <alexander.usyskin@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/misc/mei/Kconfig        |  13 ++
 drivers/misc/mei/Makefile       |   3 +
 drivers/misc/mei/platform-vsc.c | 442 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 458 insertions(+)
 create mode 100644 drivers/misc/mei/platform-vsc.c

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 470957a..2c5312b 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -71,6 +71,19 @@ config INTEL_MEI_VSC_HW
 	  This driver can also be built as a module. If so, the module
 	  will be called mei-vsc-hw.
 
+config INTEL_MEI_VSC
+	tristate "Intel visual sensing controller device with ME interface"
+	select INTEL_MEI_VSC_HW
+	depends on INTEL_MEI
+	help
+	  Intel MEI over SPI driver for Intel visual sensing controller
+	  (IVSC) device embedded in IA platform. It supports camera sharing
+	  between IVSC for context sensing and IPU for typical media usage.
+	  Select this config will enable transport layer for IVSC device.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called mei-vsc.
+
 source "drivers/misc/mei/hdcp/Kconfig"
 source "drivers/misc/mei/pxp/Kconfig"
 source "drivers/misc/mei/gsc_proxy/Kconfig"
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 3d0da19..6f9fdbf 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -35,3 +35,6 @@ obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
 obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
 mei-vsc-hw-y := vsc-tp.o
 mei-vsc-hw-y += vsc-fw-loader.o
+
+obj-$(CONFIG_INTEL_MEI_VSC) += mei-vsc.o
+mei-vsc-y := platform-vsc.o
diff --git a/drivers/misc/mei/platform-vsc.c b/drivers/misc/mei/platform-vsc.c
new file mode 100644
index 0000000..8bd016d
--- /dev/null
+++ b/drivers/misc/mei/platform-vsc.c
@@ -0,0 +1,442 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, Intel Corporation.
+ * Intel Visual Sensing Controller Interface Linux driver
+ */
+
+#include <linux/align.h>
+#include <linux/cache.h>
+#include <linux/cleanup.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/mei.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/timekeeping.h>
+#include <linux/types.h>
+
+#include <asm-generic/bug.h>
+#include <asm-generic/unaligned.h>
+
+#include "mei_dev.h"
+#include "vsc-tp.h"
+
+#define VSC_MEI_MAX_MSG_SIZE		512
+
+#define MEI_VSC_POLL_DELAY_US		(50 * USEC_PER_MSEC)
+#define MEI_VSC_POLL_TIMEOUT_US		(200 * USEC_PER_MSEC)
+
+#define mei_dev_to_vsc_hw(dev)		((struct mei_vsc_hw *)((dev)->hw))
+
+struct mei_vsc_host_timestamp {
+	u64 realtime;
+	u64 boottime;
+};
+
+struct mei_vsc_hw {
+	struct vsc_tp *tp;
+
+	bool fw_ready;
+	bool host_ready;
+
+	atomic_t write_lock_cnt;
+
+	u32 rx_len;
+	u32 rx_hdr;
+
+	/* buffer for tx */
+	char tx_buf[VSC_MEI_MAX_MSG_SIZE + sizeof(struct mei_msg_hdr)] ____cacheline_aligned;
+	/* buffer for rx */
+	char rx_buf[VSC_MEI_MAX_MSG_SIZE + sizeof(struct mei_msg_hdr)] ____cacheline_aligned;
+};
+
+static int mei_vsc_read_helper(struct mei_vsc_hw *hw, u8 *buf,
+			       u32 max_len)
+{
+	struct mei_vsc_host_timestamp ts = {
+		.realtime = ktime_to_ns(ktime_get_real()),
+		.boottime = ktime_to_ns(ktime_get_boottime()),
+	};
+
+	return vsc_tp_xfer(hw->tp, VSC_TP_CMD_READ, &ts, sizeof(ts),
+			   buf, max_len);
+}
+
+static int mei_vsc_write_helper(struct mei_vsc_hw *hw, u8 *buf, u32 len)
+{
+	u8 status;
+
+	return vsc_tp_xfer(hw->tp, VSC_TP_CMD_WRITE, buf, len, &status,
+			   sizeof(status));
+}
+
+static int mei_vsc_fw_status(struct mei_device *mei_dev,
+			     struct mei_fw_status *fw_status)
+{
+	if (!fw_status)
+		return -EINVAL;
+
+	fw_status->count = 0;
+
+	return 0;
+}
+
+static inline enum mei_pg_state mei_vsc_pg_state(struct mei_device *mei_dev)
+{
+	return MEI_PG_OFF;
+}
+
+static void mei_vsc_intr_enable(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	vsc_tp_intr_enable(hw->tp);
+}
+
+static void mei_vsc_intr_disable(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	vsc_tp_intr_disable(hw->tp);
+}
+
+/* mei framework requires this ops */
+static void mei_vsc_intr_clear(struct mei_device *mei_dev)
+{
+}
+
+/* wait for pending irq handler */
+static void mei_vsc_synchronize_irq(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	vsc_tp_intr_synchronize(hw->tp);
+}
+
+static int mei_vsc_hw_config(struct mei_device *mei_dev)
+{
+	return 0;
+}
+
+static bool mei_vsc_host_is_ready(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	return hw->host_ready;
+}
+
+static bool mei_vsc_hw_is_ready(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	return hw->fw_ready;
+}
+
+static int mei_vsc_hw_start(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+	int ret, rlen;
+	u8 buf;
+
+	hw->host_ready = true;
+
+	vsc_tp_intr_enable(hw->tp);
+
+	ret = read_poll_timeout(mei_vsc_read_helper, rlen,
+				rlen >= 0, MEI_VSC_POLL_DELAY_US,
+				MEI_VSC_POLL_TIMEOUT_US, true,
+				hw, &buf, sizeof(buf));
+	if (ret) {
+		dev_err(mei_dev->dev, "wait fw ready failed: %d\n", ret);
+		return ret;
+	}
+
+	hw->fw_ready = true;
+
+	return 0;
+}
+
+static bool mei_vsc_hbuf_is_ready(struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	return atomic_read(&hw->write_lock_cnt) == 0;
+}
+
+static int mei_vsc_hbuf_empty_slots(struct mei_device *mei_dev)
+{
+	return VSC_MEI_MAX_MSG_SIZE / MEI_SLOT_SIZE;
+}
+
+static u32 mei_vsc_hbuf_depth(const struct mei_device *mei_dev)
+{
+	return VSC_MEI_MAX_MSG_SIZE / MEI_SLOT_SIZE;
+}
+
+static int mei_vsc_write(struct mei_device *mei_dev,
+			 const void *hdr, size_t hdr_len,
+			 const void *data, size_t data_len)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+	char *buf = hw->tx_buf;
+	int ret;
+
+	if (WARN_ON(!hdr || !IS_ALIGNED(hdr_len, 4)))
+		return -EINVAL;
+
+	if (!data || data_len > VSC_MEI_MAX_MSG_SIZE)
+		return -EINVAL;
+
+	atomic_inc(&hw->write_lock_cnt);
+
+	memcpy(buf, hdr, hdr_len);
+	memcpy(buf + hdr_len, data, data_len);
+
+	ret = mei_vsc_write_helper(hw, buf, hdr_len + data_len);
+
+	atomic_dec_if_positive(&hw->write_lock_cnt);
+
+	return ret < 0 ? ret : 0;
+}
+
+static inline u32 mei_vsc_read(const struct mei_device *mei_dev)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+	int ret;
+
+	ret = mei_vsc_read_helper(hw, hw->rx_buf, sizeof(hw->rx_buf));
+	if (ret < 0 || ret < sizeof(u32))
+		return 0;
+	hw->rx_len = ret;
+
+	hw->rx_hdr = get_unaligned_le32(hw->rx_buf);
+
+	return hw->rx_hdr;
+}
+
+static int mei_vsc_count_full_read_slots(struct mei_device *mei_dev)
+{
+	return VSC_MEI_MAX_MSG_SIZE / MEI_SLOT_SIZE;
+}
+
+static int mei_vsc_read_slots(struct mei_device *mei_dev, unsigned char *buf,
+			      unsigned long len)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+	struct mei_msg_hdr *hdr;
+
+	hdr = (struct mei_msg_hdr *)&hw->rx_hdr;
+	if (len != hdr->length || hdr->length + sizeof(*hdr) != hw->rx_len)
+		return -EINVAL;
+
+	memcpy(buf, hw->rx_buf + sizeof(*hdr), len);
+
+	return 0;
+}
+
+static bool mei_vsc_pg_in_transition(struct mei_device *mei_dev)
+{
+	return mei_dev->pg_event >= MEI_PG_EVENT_WAIT &&
+	       mei_dev->pg_event <= MEI_PG_EVENT_INTR_WAIT;
+}
+
+static bool mei_vsc_pg_is_enabled(struct mei_device *mei_dev)
+{
+	return false;
+}
+
+static int mei_vsc_hw_reset(struct mei_device *mei_dev, bool intr_enable)
+{
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+
+	vsc_tp_reset(hw->tp);
+
+	vsc_tp_intr_disable(hw->tp);
+
+	return vsc_tp_init(hw->tp, mei_dev->dev);
+}
+
+static const struct mei_hw_ops mei_vsc_hw_ops = {
+	.fw_status = mei_vsc_fw_status,
+	.pg_state = mei_vsc_pg_state,
+
+	.host_is_ready = mei_vsc_host_is_ready,
+	.hw_is_ready = mei_vsc_hw_is_ready,
+	.hw_reset = mei_vsc_hw_reset,
+	.hw_config = mei_vsc_hw_config,
+	.hw_start = mei_vsc_hw_start,
+
+	.pg_in_transition = mei_vsc_pg_in_transition,
+	.pg_is_enabled = mei_vsc_pg_is_enabled,
+
+	.intr_clear = mei_vsc_intr_clear,
+	.intr_enable = mei_vsc_intr_enable,
+	.intr_disable = mei_vsc_intr_disable,
+	.synchronize_irq = mei_vsc_synchronize_irq,
+
+	.hbuf_free_slots = mei_vsc_hbuf_empty_slots,
+	.hbuf_is_ready = mei_vsc_hbuf_is_ready,
+	.hbuf_depth = mei_vsc_hbuf_depth,
+	.write = mei_vsc_write,
+
+	.rdbuf_full_slots = mei_vsc_count_full_read_slots,
+	.read_hdr = mei_vsc_read,
+	.read = mei_vsc_read_slots,
+};
+
+static void mei_vsc_event_cb(void *context)
+{
+	struct mei_device *mei_dev = context;
+	struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+	struct list_head cmpl_list;
+	s32 slots;
+	int ret;
+
+	if (mei_dev->dev_state == MEI_DEV_RESETTING ||
+	    mei_dev->dev_state == MEI_DEV_INITIALIZING)
+		return;
+
+	INIT_LIST_HEAD(&cmpl_list);
+
+	guard(mutex)(&mei_dev->device_lock);
+
+	while (vsc_tp_need_read(hw->tp)) {
+		/* check slots available for reading */
+		slots = mei_count_full_read_slots(mei_dev);
+
+		ret = mei_irq_read_handler(mei_dev, &cmpl_list, &slots);
+		if (ret) {
+			if (ret != -ENODATA) {
+				if (mei_dev->dev_state != MEI_DEV_RESETTING &&
+				    mei_dev->dev_state != MEI_DEV_POWER_DOWN)
+					schedule_work(&mei_dev->reset_work);
+			}
+
+			return;
+		}
+	}
+
+	mei_dev->hbuf_is_ready = mei_hbuf_is_ready(mei_dev);
+	ret = mei_irq_write_handler(mei_dev, &cmpl_list);
+	if (ret)
+		dev_err(mei_dev->dev, "dispatch write request failed: %d\n", ret);
+
+	mei_dev->hbuf_is_ready = mei_hbuf_is_ready(mei_dev);
+	mei_irq_compl_handler(mei_dev, &cmpl_list);
+}
+
+static int mei_vsc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mei_device *mei_dev;
+	struct mei_vsc_hw *hw;
+	struct vsc_tp *tp;
+	int ret;
+
+	tp = *(struct vsc_tp **)dev_get_platdata(dev);
+	if (!tp)
+		return dev_err_probe(dev, -ENODEV, "no platform data\n");
+
+	mei_dev = devm_kzalloc(dev, size_add(sizeof(*mei_dev), sizeof(*hw)),
+			       GFP_KERNEL);
+	if (!mei_dev)
+		return -ENOMEM;
+
+	mei_device_init(mei_dev, dev, false, &mei_vsc_hw_ops);
+	mei_dev->fw_f_fw_ver_supported = 0;
+	mei_dev->kind = "ivsc";
+
+	hw = mei_dev_to_vsc_hw(mei_dev);
+	atomic_set(&hw->write_lock_cnt, 0);
+	hw->tp = tp;
+
+	platform_set_drvdata(pdev, mei_dev);
+
+	vsc_tp_register_event_cb(tp, mei_vsc_event_cb, mei_dev);
+
+	ret = mei_start(mei_dev);
+	if (ret) {
+		dev_err_probe(dev, ret, "init hw failed\n");
+		goto err_cancel;
+	}
+
+	ret = mei_register(mei_dev, dev);
+	if (ret)
+		goto err_stop;
+
+	pm_runtime_enable(mei_dev->dev);
+
+	return 0;
+
+err_stop:
+	mei_stop(mei_dev);
+
+err_cancel:
+	mei_cancel_work(mei_dev);
+
+	mei_disable_interrupts(mei_dev);
+
+	return ret;
+}
+
+static int mei_vsc_remove(struct platform_device *pdev)
+{
+	struct mei_device *mei_dev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(mei_dev->dev);
+
+	mei_stop(mei_dev);
+
+	mei_disable_interrupts(mei_dev);
+
+	mei_deregister(mei_dev);
+
+	return 0;
+}
+
+static int mei_vsc_suspend(struct device *dev)
+{
+	struct mei_device *mei_dev = dev_get_drvdata(dev);
+
+	mei_stop(mei_dev);
+
+	return 0;
+}
+
+static int mei_vsc_resume(struct device *dev)
+{
+	struct mei_device *mei_dev = dev_get_drvdata(dev);
+	int ret;
+
+	ret = mei_restart(mei_dev);
+	if (ret)
+		return ret;
+
+	/* start timer if stopped in suspend */
+	schedule_delayed_work(&mei_dev->timer_work, HZ);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend, mei_vsc_resume);
+
+static struct platform_driver mei_vsc_drv = {
+	.probe = mei_vsc_probe,
+	.remove = mei_vsc_remove,
+	.driver = {
+		.name = "intel_vsc",
+		.pm = &mei_vsc_pm_ops,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+module_platform_driver(mei_vsc_drv);
+
+MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
+MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
+MODULE_DESCRIPTION("Intel Visual Sensing Controller Interface");
+MODULE_ALIAS("platform:intel_vsc");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(VSC_TP);
-- 
2.7.4
Re: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Krzysztof Kozlowski 2 years ago
On 28/11/2023 13:34, Wentong Wu wrote:
> The protocol used for the IVSC device to communicate with HOST is MEI.
> The MEI hardware interfaces for the IVSC device are implemented.
> 

...

> +static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend, mei_vsc_resume);
> +
> +static struct platform_driver mei_vsc_drv = {
> +	.probe = mei_vsc_probe,
> +	.remove = mei_vsc_remove,
> +	.driver = {
> +		.name = "intel_vsc",
> +		.pm = &mei_vsc_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +module_platform_driver(mei_vsc_drv);
> +
> +MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
> +MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
> +MODULE_DESCRIPTION("Intel Visual Sensing Controller Interface");
> +MODULE_ALIAS("platform:intel_vsc");

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.


Best regards,
Krzysztof
RE: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Wu, Wentong 2 years ago
> From: Krzysztof Kozlowski <krzk@kernel.org>
> On 28/11/2023 13:34, Wentong Wu wrote:
> > The protocol used for the IVSC device to communicate with HOST is MEI.
> > The MEI hardware interfaces for the IVSC device are implemented.
> >
> 
> ...
> 
> > +static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend,
> > +mei_vsc_resume);
> > +
> > +static struct platform_driver mei_vsc_drv = {
> > +	.probe = mei_vsc_probe,
> > +	.remove = mei_vsc_remove,
> > +	.driver = {
> > +		.name = "intel_vsc",
> > +		.pm = &mei_vsc_pm_ops,
> > +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > +	},
> > +};
> > +module_platform_driver(mei_vsc_drv);
> > +
> > +MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
> > +MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
> > +MODULE_DESCRIPTION("Intel Visual Sensing Controller Interface");
> > +MODULE_ALIAS("platform:intel_vsc");
> 
> You should not need MODULE_ALIAS() in normal cases. If you need it, usually it
> means your device ID table is wrong (e.g. misses either entries or
> MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete
> ID table.

Agree, I forgot the id table and it will be added in next version, thanks

#define MEI_VSC_DRV_NAME		"intel_vsc"

static const struct platform_device_id mei_vsc_id_table[] = {
	{ MEI_VSC_DRV_NAME },
	{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(platform, mei_vsc_id_table);

static struct platform_driver mei_vsc_drv = {
	.probe = mei_vsc_probe,
	.remove = mei_vsc_remove,
	.id_table = mei_vsc_id_table,
	.driver = {
		.name = MEI_VSC_DRV_NAME,
		.pm = &mei_vsc_pm_ops,
		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
	},
};
module_platform_driver(mei_vsc_drv);

MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
MODULE_DESCRIPTION("Intel Visual Sensing Controller Interface");
MODULE_LICENSE("GPL");
MODULE_IMPORT_NS(VSC_TP);

BR,
Wentong
> 
> 
> Best regards,
> Krzysztof
Re: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Sakari Ailus 2 years ago
Hi Wentong,

On Tue, Nov 28, 2023 at 08:34:06PM +0800, Wentong Wu wrote:
> The protocol used for the IVSC device to communicate with HOST is MEI.
> The MEI hardware interfaces for the IVSC device are implemented.
> 
> The APIs are exposed by MEI framework to mei clients, e.g. mei_csi and
> mei_ace.
> 
> Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> Reviewed-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/misc/mei/Kconfig        |  13 ++
>  drivers/misc/mei/Makefile       |   3 +
>  drivers/misc/mei/platform-vsc.c | 442 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 458 insertions(+)
>  create mode 100644 drivers/misc/mei/platform-vsc.c
> 
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
> index 470957a..2c5312b 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -71,6 +71,19 @@ config INTEL_MEI_VSC_HW
>  	  This driver can also be built as a module. If so, the module
>  	  will be called mei-vsc-hw.
>  
> +config INTEL_MEI_VSC
> +	tristate "Intel visual sensing controller device with ME interface"
> +	select INTEL_MEI_VSC_HW

Changing the select here to depends on addresses the Kconfig option
dependency issue (as select just blindly selects the options while ignoring
their dependencies).

I wouldn't mind having a single Kconfig option for the two drivers either.
They're always used together, aren't they?

> +	depends on INTEL_MEI
> +	help
> +	  Intel MEI over SPI driver for Intel visual sensing controller
> +	  (IVSC) device embedded in IA platform. It supports camera sharing
> +	  between IVSC for context sensing and IPU for typical media usage.
> +	  Select this config will enable transport layer for IVSC device.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called mei-vsc.
> +
>  source "drivers/misc/mei/hdcp/Kconfig"
>  source "drivers/misc/mei/pxp/Kconfig"
>  source "drivers/misc/mei/gsc_proxy/Kconfig"

-- 
Regards,

Sakari Ailus
RE: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Wu, Wentong 2 years ago
> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> On Tue, Nov 28, 2023 at 08:34:06PM +0800, Wentong Wu wrote:
> > The protocol used for the IVSC device to communicate with HOST is MEI.
> > The MEI hardware interfaces for the IVSC device are implemented.
> >
> > The APIs are exposed by MEI framework to mei clients, e.g. mei_csi and
> > mei_ace.
> >
> > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > Reviewed-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/misc/mei/Kconfig        |  13 ++
> >  drivers/misc/mei/Makefile       |   3 +
> >  drivers/misc/mei/platform-vsc.c | 442
> > ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 458 insertions(+)
> >  create mode 100644 drivers/misc/mei/platform-vsc.c
> >
> > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> > 470957a..2c5312b 100644
> > --- a/drivers/misc/mei/Kconfig
> > +++ b/drivers/misc/mei/Kconfig
> > @@ -71,6 +71,19 @@ config INTEL_MEI_VSC_HW
> >  	  This driver can also be built as a module. If so, the module
> >  	  will be called mei-vsc-hw.
> >
> > +config INTEL_MEI_VSC
> > +	tristate "Intel visual sensing controller device with ME interface"
> > +	select INTEL_MEI_VSC_HW
> 
> Changing the select here to depends on addresses the Kconfig option
> dependency issue 

Yes, this is the plan to address the warning. Thanks

> (as select just blindly selects the options while ignoring their
> dependencies).
> 
> I wouldn't mind having a single Kconfig option for the two drivers either.
> They're always used together, aren't they?

Thanks and yes, currently they're used together, but we may have more
flexibility if keep them separate to cover future hardware which may has
different transport layer.

BR,
Wentong
> 
> > +	depends on INTEL_MEI
> > +	help
> > +	  Intel MEI over SPI driver for Intel visual sensing controller
> > +	  (IVSC) device embedded in IA platform. It supports camera sharing
> > +	  between IVSC for context sensing and IPU for typical media usage.
> > +	  Select this config will enable transport layer for IVSC device.
> > +
> > +	  This driver can also be built as a module. If so, the module
> > +	  will be called mei-vsc.
> > +
> >  source "drivers/misc/mei/hdcp/Kconfig"
> >  source "drivers/misc/mei/pxp/Kconfig"
> >  source "drivers/misc/mei/gsc_proxy/Kconfig"
> 
> --
> Regards,
> 
> Sakari Ailus
Re: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Andy Shevchenko 2 years ago
On Wed, Nov 29, 2023 at 09:26:47AM +0000, Sakari Ailus wrote:
> On Tue, Nov 28, 2023 at 08:34:06PM +0800, Wentong Wu wrote:

...

> I wouldn't mind having a single Kconfig option for the two drivers either.
> They're always used together, aren't they?

As far as I understand the topology, here we have one "core" driver and
one "leaf" driver. So, in my understanding they may not go together as
it will bring unneeded code in the future in some configurations.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by Sakari Ailus 2 years ago
On Wed, Nov 29, 2023 at 12:21:32PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 29, 2023 at 09:26:47AM +0000, Sakari Ailus wrote:
> > On Tue, Nov 28, 2023 at 08:34:06PM +0800, Wentong Wu wrote:
> 
> ...
> 
> > I wouldn't mind having a single Kconfig option for the two drivers either.
> > They're always used together, aren't they?
> 
> As far as I understand the topology, here we have one "core" driver and
> one "leaf" driver. So, in my understanding they may not go together as
> it will bring unneeded code in the future in some configurations.

Yes, that is possible in the future if other clients appear. But will there
be any? Both drivers appear to be related to IVSC.

-- 
Sakari Ailus
Re: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
Posted by kernel test robot 2 years ago
Hi Wentong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.7-rc3 next-20231128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wentong-Wu/mei-Add-transport-driver-for-IVSC-device/20231128-203609
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/1701174846-16316-3-git-send-email-wentong.wu%40intel.com
patch subject: [PATCH 2/2] mei: Add MEI hardware support for IVSC device
config: x86_64-kismet-CONFIG_INTEL_MEI_VSC_HW-CONFIG_INTEL_MEI_VSC-0-0 (https://download.01.org/0day-ci/archive/20231129/202311290342.lucjHsTH-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20231129/202311290342.lucjHsTH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311290342.lucjHsTH-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for INTEL_MEI_VSC_HW when selected by INTEL_MEI_VSC
   
   WARNING: unmet direct dependencies detected for INTEL_MEI_VSC_HW
     Depends on [n]: ACPI [=y] && SPI [=y] && (GPIOLIB [=n] || COMPILE_TEST [=n])
     Selected by [y]:
     - INTEL_MEI_VSC [=y] && INTEL_MEI [=y]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki