On Tue, Jan 21, 2025 at 02:39:00PM +0800, Mao Jinlong wrote:
> The system on chip (SoC) consists of main APSS(Applications
> processor subsytem) and additional processors like modem, lpass.
> Coresight remote etm(Embedded Trace Macrocell) driver is for
> enabling and disabling the etm trace of remote processors. It
> uses QMI interface to communicate with remote processors' software
> and uses coresight framework to configure the connection from
> remote etm source to TMC sinks.
>
> Signed-off-by: Mao Jinlong <quic_jinlmao@xxxxxxxxxxx>
> ---
> drivers/hwtracing/coresight/Kconfig | 13 +
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-qmi.h | 89 +++++
> .../coresight/coresight-remote-etm.c | 316 ++++++++++++++++++
> 4 files changed, 419 insertions(+)
> create mode 100644 drivers/hwtracing/coresight/coresight-qmi.h
> create mode 100644 drivers/hwtracing/coresight/coresight-remote-etm.c
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 06f0a7594169..871dd83649ea 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -247,4 +247,17 @@ config CORESIGHT_DUMMY
>
> To compile this driver as a module, choose M here: the module will be
> called coresight-dummy.
> +
> +config CORESIGHT_REMOTE_ETM
> + tristate "Remote processor ETM trace support"
> + depends on QCOM_QMI_HELPERS
> + help
> + Enables support for ETM trace collection on remote processor using
> + CoreSight framework. Enabling this will allow turning on ETM
> + tracing on remote processor via sysfs by configuring the required
> + CoreSight components.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called coresight-remote-etm.
> +
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 4ba478211b31..e0781d729eb3 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
> coresight-cti-sysfs.o
> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
> +obj-$(CONFIG_CORESIGHT_REMOTE_ETM) += coresight-remote-etm.o
> diff --git a/drivers/hwtracing/coresight/coresight-qmi.h b/drivers/hwtracing/coresight/coresight-qmi.h
> new file mode 100644
> index 000000000000..c9b49500d11c
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-qmi.h
> @@ -0,0 +1,89 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef _CORESIGHT_QMI_H
> +#define _CORESIGHT_QMI_H
> +
> +#include <linux/soc/qcom/qmi.h>
> +
> +#define CORESIGHT_QMI_SVC_ID (0x33)
> +#define CORESIGHT_QMI_VERSION (1)
> +
> +#define CORESIGHT_QMI_GET_ETM_REQ_V01 (0x002B)
> +#define CORESIGHT_QMI_GET_ETM_RESP_V01 (0x002B)
> +#define CORESIGHT_QMI_SET_ETM_REQ_V01 (0x002C)
> +#define CORESIGHT_QMI_SET_ETM_RESP_V01 (0x002C)
> +
> +#define CORESIGHT_QMI_GET_ETM_REQ_MAX_LEN (0)
> +#define CORESIGHT_QMI_GET_ETM_RESP_MAX_LEN (14)
> +#define CORESIGHT_QMI_SET_ETM_REQ_MAX_LEN (7)
> +#define CORESIGHT_QMI_SET_ETM_RESP_MAX_LEN (7)
> +
> +#define TIMEOUT_MS (10000)
> +
> +enum coresight_etm_state_enum_type_v01 {
> + /* To force a 32 bit signed enum. Do not change or use */
> + CORESIGHT_ETM_STATE_ENUM_TYPE_MIN_ENUM_VAL_V01 = INT_MIN,
> + CORESIGHT_ETM_STATE_DISABLED_V01 = 0,
> + CORESIGHT_ETM_STATE_ENABLED_V01 = 1,
> + CORESIGHT_ETM_STATE_ENUM_TYPE_MAX_ENUM_VAL_01 = INT_MAX,
> +};
> +
> +struct coresight_set_etm_req_msg_v01 {
> + /* Mandatory */
> + /* ETM output state */
> + enum coresight_etm_state_enum_type_v01 state;
> +};
> +
> +struct coresight_set_etm_resp_msg_v01 {
> + /* Mandatory */
> + struct qmi_response_type_v01 resp;
> +};
> +
> +static struct qmi_elem_info coresight_set_etm_req_msg_v01_ei[] = {
Why, unlike every other instance, this cannot be const?
Anyway, you cannot have DATA in the header. This makes little sense.
There is no such patterns so I don't understand from where did you get
this.
...
> + {
> + .data_type = QMI_UNSIGNED_4_BYTE,
> + .elem_len = 1,
> + .elem_size = sizeof(enum coresight_etm_state_enum_type_v01),
> + .array_type = NO_ARRAY,
> + .tlv_type = 0x01,
> + .offset = offsetof(struct coresight_set_etm_req_msg_v01,
> + state),
> + .ei_array = NULL,
> + },
> +static int remote_etm_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct coresight_platform_data *pdata;
> + struct remote_etm_drvdata *drvdata;
> + struct coresight_desc desc = {0 };
> + int ret;
> +
> + desc.name = coresight_alloc_device_name(&remote_etm_devs, dev);
> + if (!desc.name)
> + return -ENOMEM;
> + pdata = coresight_get_platform_data(dev);
> + if (IS_ERR(pdata))
> + return PTR_ERR(pdata);
> + pdev->dev.platform_data = pdata;
> +
> + pm_runtime_enable(dev);
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &pdev->dev;
Use the cached value, if you cache it.
= dev
> + platform_set_drvdata(pdev, drvdata);
> +
> + drvdata->qmi_id = (u32)(unsigned long)of_device_get_match_data(&pdev->dev);
Ditto, everywhere...
> +
> + mutex_init(&drvdata->mutex);
> +
> + ret = qmi_handle_init(&drvdata->handle,
> + CORESIGHT_QMI_SET_ETM_REQ_MAX_LEN,
> + &server_ops, NULL);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Remote ETM client init failed.\n");
Syntax is always: return dev_err_probe. I am pretty sure you got this
feedback before.
Best regards,
Krzysztof
