Re: [PATCH v6 1/2] drivers/coresight: Add UltraSoc System Memory Buffer driver

Hi Qi

On 06/06/2022 14:02, Qi Liu wrote:

This patch adds driver for UltraSoc SMB(System Memory Buffer) device. SMB provides a way to buffer messages from ETM, and store these CPU instructions in system memory.

nit: "CPU instruction trace" in system memory.

SMB is developed by UltraSoc technology, which is acquired by Siemens, and we still use "UltraSoc" to name driver.

Signed-off-by: Qi Liu liuqi115@huawei.com Tested-by: JunHao He hejunhao2@hisilicon.com

---

drivers/hwtracing/coresight/Kconfig | 10 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/ultrasoc-smb.c | 663 +++++++++++++++++++++ drivers/hwtracing/coresight/ultrasoc-smb.h | 110 ++++ 4 files changed, 784 insertions(+) create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 514a9b8086e3..4380eb1a0a73 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig

...

diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index b6c4a48140ec..344dba8d6ff8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -27,3 +27,4 @@ obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ coresight-cti-sysfs.o +obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c new file mode 100644 index 000000000000..44b4bceb6f28 --- /dev/null +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c

...

+static int smb_open(struct inode *inode, struct file *file) +{

• struct smb_drv_data *drvdata = container_of(file->private_data,

• 			struct smb_drv_data, miscdev);
  

• unsigned long flags;
• int ret = 0;
• spin_lock_irqsave(&drvdata->spinlock, flags);
• if (local_read(&drvdata->reading)) {

• ret = -EBUSY;
  

• goto out;
  

• }
• if (drvdata->mode == CS_MODE_PERF) {

• ret = -EINVAL;
  

• goto out;
  

• }

+out:

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• return ret;

+}

+static ssize_t smb_read(struct file *file, char __user *data, size_t len,

• 	loff_t *ppos)
  

+{

• struct smb_drv_data *drvdata = container_of(file->private_data,

• 			struct smb_drv_data, miscdev);
  

• struct smb_data_buffer *sdb = &drvdata->sdb;
• struct device *dev = &drvdata->csdev->dev;
• unsigned long flags;
• int to_copy = 0;
• spin_lock_irqsave(&drvdata->spinlock, flags);
• if (atomic_read(drvdata->csdev->refcnt)) {

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
  

• return -EBUSY;
  

• }
• if (local_cmpxchg(&drvdata->reading, 0, 1)) {

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
  

• return -EBUSY;
  

• }

Shouldn't this be done in smb_open() ? Otherwise, a subsequent read will fail with EBUSY ? i.e, fd = open(/dev/ultra_smb0, ..);

# size of the SMB buffer = 1M, size = read(fd, buf, SZ_4K); /* Do something the buf */ if (size >= 0) size = read(fd, buf, SZ_4K);

^^ This would fail, isn't it ?

• if (!sdb->data_size) {

• smb_update_data_size(drvdata);
  

• if (!sdb->data_size)
  

• 	goto out;
  

• }
• to_copy = min(sdb->data_size, len);
• /* Copy parts of trace data when read pointer wrap around SMB buffer */
• if (sdb->rd_offset + to_copy > sdb->buf_size)

• to_copy = sdb->buf_size - sdb->rd_offset;
  

• if (copy_to_user(data, (void *)sdb->buf_base + sdb->rd_offset,

• 	 to_copy)) {
  

• dev_dbg(dev, "Failed to copy data to user.\n");
  

• to_copy = -EFAULT;
  

• goto out;
  

• }
• *ppos += to_copy;
• sdb->data_size -= to_copy;
• sdb->rd_offset += to_copy;
• sdb->rd_offset %= sdb->buf_size;
• writel(sdb->start_addr + sdb->rd_offset,

•       drvdata->base + SMB_LB_RD_ADDR);
  

• dev_dbg(dev, "%d bytes copied.\n", to_copy);

+out:

• if (!sdb->data_size)

• smb_reset_buffer_status(drvdata);
  

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• return to_copy;

+}

+static int smb_release(struct inode *inode, struct file *file) +{

• struct smb_drv_data *drvdata = container_of(file->private_data,

• 			struct smb_drv_data, miscdev);
  

• local_set(&drvdata->reading, 0);
• return 0;

+}

+static const struct file_operations smb_fops = {

• .owner = THIS_MODULE,
• .open = smb_open,
• .read = smb_read,
• .release = smb_release,
• .llseek = no_llseek,

+};

+smb_reg(read_pos, SMB_LB_RD_ADDR); +smb_reg(write_pos, SMB_LB_WR_ADDR); +smb_reg(buf_status, SMB_LB_INT_STS);

+static ssize_t buf_size_show(struct device *dev, struct device_attribute *attr,

• 	     char *buf)
  

+{

• struct smb_drv_data *drvdata = dev_get_drvdata(dev->parent);
• return sysfs_emit(buf, "0x%lx\n", drvdata->sdb.buf_size);

+} +static DEVICE_ATTR_RO(buf_size);

+static struct attribute *smb_sink_attrs[] = {

• &dev_attr_read_pos.attr,
• &dev_attr_write_pos.attr,
• &dev_attr_buf_status.attr,
• &dev_attr_buf_size.attr,
• NULL,

+};

+static const struct attribute_group smb_sink_group = {

• .attrs = smb_sink_attrs,
• .name = "mgmt",

+};

+static const struct attribute_group *smb_sink_groups[] = {

• &smb_sink_group,
• NULL,

+};

+static int smb_set_perf_buffer(struct perf_output_handle *handle) +{

• struct cs_buffers *buf = etm_perf_sink_config(handle);
• u32 head;
• if (!buf)

• return -EINVAL;
  

• /* Wrap head around to the amount of space we have */
• head = handle->head & ((buf->nr_pages << PAGE_SHIFT) - 1);
• /* Find the page to write to and offset within that page */
• buf->cur = head / PAGE_SIZE;
• buf->offset = head % PAGE_SIZE;

I believe these fields should be dropped and instead use the handle->head, to determine the page number and offset. See the TMC-ETR driver for e.g.

• local_set(&buf->data_size, 0);
• return 0;

+}

+static void smb_enable_hw(struct smb_drv_data *drvdata) +{

• writel(0x1, drvdata->base + SMB_GLOBAL_EN);

+}

+static void smb_disable_hw(struct smb_drv_data *drvdata) +{

• writel(0x0, drvdata->base + SMB_GLOBAL_EN);

+}

+static int smb_enable_sysfs(struct coresight_device *csdev) +{

• struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
• unsigned long flags;
• int ret = 0;
• spin_lock_irqsave(&drvdata->spinlock, flags);
• if (drvdata->mode == CS_MODE_PERF) {

• ret = -EBUSY;
  

• goto out;
  

• }
• if (drvdata->mode == CS_MODE_DISABLED) {

• smb_enable_hw(drvdata);
  

• drvdata->mode = CS_MODE_SYSFS;
  

• }
• atomic_inc(csdev->refcnt);

+out:

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• return ret;

+}

+static int smb_enable_perf(struct coresight_device *csdev, void *data) +{

• struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
• struct device *dev = &drvdata->csdev->dev;
• struct perf_output_handle *handle = data;
• struct cs_buffers *buf = etm_perf_sink_config(handle);
• unsigned long flags;
• int ret = 0;
• pid_t pid;
• spin_lock_irqsave(&drvdata->spinlock, flags);
• if (drvdata->mode == CS_MODE_SYSFS) {

• dev_dbg(dev, "Device is already in used by sysfs.\n");
  

• ret = -EBUSY;
  

• goto out;
  

• }
• /* Get a handle on the pid of the target process */
• pid = buf->pid;
• if (drvdata->pid != -1 && drvdata->pid != pid) {

• dev_dbg(dev, "Device is already in used by other session.\n");
  

• ret = -EBUSY;
  

• goto out;
  

• }
• /* The sink is already enabled by this session */
• if (drvdata->pid == pid) {

• atomic_inc(csdev->refcnt);
  

• goto out;
  

• }
• ret = smb_set_perf_buffer(handle);
• if (ret)

• goto out;
  

• smb_enable_hw(drvdata);
• drvdata->pid = pid;
• drvdata->mode = CS_MODE_PERF;
• atomic_inc(csdev->refcnt);

+out:

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• return ret;

+}

+static int smb_enable(struct coresight_device *csdev, u32 mode, void *data) +{

• struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
• int ret;
• /* Do nothing if trace data is reading by other interface now */
• if (local_read(&drvdata->reading))

• return -EBUSY;
  

Is there global switch for all the inports ? Or is there a control for individual inports to the SMB ? Do we need additional information here to enable the corresponding port ?

• switch (mode) {
• case CS_MODE_SYSFS:

• ret = smb_enable_sysfs(csdev);
  

• break;
  

• case CS_MODE_PERF:

• ret = smb_enable_perf(csdev, data);
  

• break;
  

• default:

• ret = -EINVAL;
  

• break;
  

• }
• if (ret)

• return ret;
  

• dev_dbg(&csdev->dev, "Ultrasoc SMB enabled.\n");
• return 0;

+}

+static int smb_disable(struct coresight_device *csdev) +{

• struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
• unsigned long flags;
• spin_lock_irqsave(&drvdata->spinlock, flags);
• if (local_read(&drvdata->reading)) {

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
  

• return -EBUSY;
  

• }
• if (atomic_dec_return(csdev->refcnt)) {

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
  

• return -EBUSY;
  

• }
• WARN_ON_ONCE(drvdata->mode == CS_MODE_DISABLED);
• smb_disable_hw(drvdata);
• smb_purge_data(drvdata);
• /* Dissociate from the target process. */
• drvdata->pid = -1;
• drvdata->mode = CS_MODE_DISABLED;
• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• dev_dbg(&csdev->dev, "Ultrasoc SMB disabled.\n");
• return 0;

+}

+static void *smb_alloc_buffer(struct coresight_device *csdev,

• 	      struct perf_event *event, void **pages,
  

• 	      int nr_pages, bool overwrite)
  

+{

• struct cs_buffers *buf;
• int node;
• node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu);
• buf = kzalloc_node(sizeof(struct cs_buffers), GFP_KERNEL, node);
• if (!buf)

• return NULL;
  

• buf->snapshot = overwrite;
• buf->nr_pages = nr_pages;
• buf->data_pages = pages;
• buf->pid = task_pid_nr(event->owner);
• return buf;

+}

+static void smb_free_buffer(void *config) +{

• struct cs_buffers *buf = config;
• kfree(buf);

+}

+static void smb_sync_perf_buffer(struct smb_drv_data *drvdata,

• 		 struct cs_buffers *buf,
  

• 		 unsigned long data_size)
  

You may need to pass the aux_handle here, to determine the handle->head and the size. See my comments above.

+{

• struct smb_data_buffer *sdb = &drvdata->sdb;
• char **dst_pages = (char **)buf->data_pages;
• unsigned long page_offset = buf->offset;
• unsigned int cur = buf->cur;
• unsigned long to_copy;
• while (data_size) {

• unsigned long page_space = PAGE_SIZE - page_offset;
  

• /* Copy parts of trace data when read pointer wrap around */
  

• if (sdb->rd_offset + page_space > sdb->buf_size)
  

• 	to_copy = sdb->buf_size - sdb->rd_offset;
  

• else
  

• 	to_copy = min(data_size, page_space);
  

• memcpy_fromio(dst_pages[cur] + page_offset,
  

• 	      sdb->buf_base + sdb->rd_offset, to_copy);
  

• page_offset += to_copy;
  

• if (page_offset >= PAGE_SIZE) {
  

• 	page_offset = 0;
  

• 	cur++;
  

• 	cur %= buf->nr_pages;
  

• }
  

• data_size -= to_copy;
  

• sdb->rd_offset += to_copy;
  

• sdb->rd_offset %= sdb->buf_size;
  

• }
• sdb->data_size = 0;
• writel(sdb->start_addr + sdb->rd_offset, drvdata->base + SMB_LB_RD_ADDR);
• /*

• * Data remained in link cannot be purged when SMB is full, so
  

• * synchronize the read pointer to write pointer, to make sure
  

• * these remained data won't influence next trace.
  

• */
  

• if (sdb->full) {

• smb_purge_data(drvdata);
  

• writel(readl(drvdata->base + SMB_LB_WR_ADDR),
  

•        drvdata->base + SMB_LB_RD_ADDR);
  

• }
• smb_reset_buffer_status(drvdata);

+}

+static unsigned long smb_update_buffer(struct coresight_device *csdev,

• 		       struct perf_output_handle *handle,
  

• 		       void *sink_config)
  

+{

• struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
• struct smb_data_buffer *sdb = &drvdata->sdb;
• struct cs_buffers *buf = sink_config;
• unsigned long data_size = 0;
• unsigned long flags;
• bool lost = false;
• if (!buf)

• return 0;
  

• spin_lock_irqsave(&drvdata->spinlock, flags);
• /* Don't do anything if another tracer is using this sink. */
• if (atomic_read(csdev->refcnt) != 1)

• goto out;
  

• smb_disable_hw(drvdata);
• smb_update_data_size(drvdata);
• data_size = sdb->data_size;
• /*

• * The SMB buffer may be bigger than the space available in the
  

• * perf ring buffer (handle->size). If so advance the offset so
  

• * that we get the latest trace data.
  

• */
  

• if (data_size > handle->size) {

• sdb->rd_offset += data_size - handle->size;
  

• sdb->rd_offset %= sdb->buf_size;
  

• data_size = handle->size;
  

• lost = true;
  

• }
• smb_sync_perf_buffer(drvdata, buf, data_size);
• if (!buf->snapshot && lost)

• perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
  

• smb_enable_hw(drvdata);

+out:

• spin_unlock_irqrestore(&drvdata->spinlock, flags);
• return data_size;

+}

+static const struct coresight_ops_sink smb_cs_ops = {

• .enable = smb_enable,
• .disable = smb_disable,
• .alloc_buffer = smb_alloc_buffer,
• .free_buffer = smb_free_buffer,
• .update_buffer = smb_update_buffer,

+};

+static const struct coresight_ops cs_ops = {

• .sink_ops = &smb_cs_ops,

+};

+static int smb_init_data_buffer(struct platform_device *pdev,

• 		struct smb_data_buffer *sdb)
  

+{

• struct resource *res;
• void __iomem *base;
• res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
• if (IS_ERR(res)) {

• dev_err(&pdev->dev, "SMB device failed to get resource.\n");
  

• return -EINVAL;
  

• }
• sdb->start_addr = res->start & SMB_BASE_LOW_MASK;
• sdb->buf_size = resource_size(res);
• if (sdb->buf_size == 0)

• return -EINVAL;
  

• base = devm_ioremap_resource(&pdev->dev, res);
• if (IS_ERR(base))

• return PTR_ERR(base);
  

• sdb->buf_base = base;
• return 0;

+}

+static void smb_init_hw(struct smb_drv_data *drvdata) +{

• /* First disable smb and clear the status of SMB buffer */
• smb_reset_buffer_status(drvdata);
• smb_disable_hw(drvdata);
• smb_purge_data(drvdata);
• writel(SMB_BUF_CFG_STREAMING, drvdata->base + SMB_LB_CFG_LO);
• writel(SMB_MSG_FILTER, drvdata->base + SMB_LB_CFG_HI);
• writel(SMB_GLOBAL_CFG, drvdata->base + SMB_CFG_REG);
• writel(SMB_GLB_INT_CFG, drvdata->base + SMB_GLOBAL_INT);
• writel(SMB_BUF_INT_CFG, drvdata->base + SMB_LB_INT_CTRL);

+}

+static int smb_register_sink(struct platform_device *pdev,

• 	     struct smb_drv_data *drvdata)
  

+{

• struct coresight_platform_data *pdata = NULL;
• struct coresight_desc desc = { 0 };
• int ret;
• pdata = coresight_get_platform_data(&pdev->dev);
• if (IS_ERR(pdata))

• return PTR_ERR(pdata);
  

• desc.type = CORESIGHT_DEV_TYPE_SINK;
• desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
• desc.ops = &cs_ops;
• desc.pdata = pdata;
• desc.dev = &pdev->dev;
• desc.groups = smb_sink_groups;
• desc.name = coresight_alloc_device_name(&sink_devs, &pdev->dev);

Please fill in the csdev_access method for the device. desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);

• if (!desc.name) {

• dev_err(&pdev->dev, "Failed to alloc coresight device name.");
  

• return -ENOMEM;
  

• }
• drvdata->csdev = coresight_register(&desc);
• if (IS_ERR(drvdata->csdev))

• return PTR_ERR(drvdata->csdev);
  

• drvdata->miscdev.name = desc.name;
• drvdata->miscdev.minor = MISC_DYNAMIC_MINOR;
• drvdata->miscdev.fops = &smb_fops;
• ret = misc_register(&drvdata->miscdev);
• if (ret) {

• coresight_unregister(drvdata->csdev);
  

• dev_err(&pdev->dev, "Failed to register misc, ret=%d.\n", ret);
  

• }
• return ret;

+}

+static void smb_unregister_sink(struct smb_drv_data *drvdata) +{

• misc_deregister(&drvdata->miscdev);
• coresight_unregister(drvdata->csdev);

+}

+static int smb_config_inport(struct device *dev, bool enable) +{

• u64 func = enable ? 1 : 0;
• union acpi_object *obj;
• guid_t guid;
• u64 rev = 0;
• /*

• * Using DSM calls to enable/disable ultrasoc hardwares on
  

• * tracing path, to prevent ultrasoc packet format being exposed.
  

• */
  

• if (guid_parse(ULTRASOC_SMB_DSM_UUID, &guid)) {

• dev_err(dev, "Get GUID failed.\n");
  

• return -EINVAL;
  

• }
• obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &guid, rev, func, NULL);
• if (!obj)

• dev_err(dev, "ACPI handle failed!\n");
  

• else

• ACPI_FREE(obj);
  

• return 0;

+}

+static int smb_probe(struct platform_device *pdev) +{

• struct smb_drv_data *drvdata;
• int ret;
• drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
• if (!drvdata)

• return -ENOMEM;
  

• drvdata->base = devm_platform_ioremap_resource(pdev, 0);
• if (IS_ERR(drvdata->base)) {

• dev_err(&pdev->dev, "Failed to ioremap resource.\n");
  

• return PTR_ERR(drvdata->base);
  

• }
• ret = smb_init_data_buffer(pdev, &drvdata->sdb);
• if (ret) {

• dev_err(&pdev->dev, "Failed to init buffer, ret = %d.\n", ret);
  

• return ret;
  

• }
• smb_init_hw(drvdata);
• spin_lock_init(&drvdata->spinlock);
• drvdata->pid = -1;
• ret = smb_register_sink(pdev, drvdata);
• if (ret) {

• dev_err(&pdev->dev, "Failed to register smb sink.\n");
  

• return ret;
  

• }
• ret = smb_config_inport(&pdev->dev, true);
• if (ret) {

• smb_unregister_sink(drvdata);
  

• return ret;
  

• }
• platform_set_drvdata(pdev, drvdata);
• return 0;

+}

+static int smb_remove(struct platform_device *pdev) +{

• struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
• int ret;
• ret = smb_config_inport(&pdev->dev, false);
• if (ret)

• return ret;
  

• smb_unregister_sink(drvdata);
• return 0;

+}

+static const struct acpi_device_id ultrasoc_smb_acpi_match[] = {

• {"HISI03A1", 0},
• {},

+}; +MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match);

+static struct platform_driver smb_driver = {

• .driver = {

• .name = "ultrasoc-smb",
  

• .acpi_match_table = ACPI_PTR(ultrasoc_smb_acpi_match),
  

• .suppress_bind_attrs = true,
  

• },
• .probe = smb_probe,
• .remove = smb_remove,

+}; +module_platform_driver(smb_driver);

+MODULE_DESCRIPTION("UltraSoc smb driver");

"UltraSoc SMB CoreSight driver"

+MODULE_LICENSE("Dual MIT/GPL"); +MODULE_AUTHOR("Jonathan Zhou jonathan.zhouwen@huawei.com"); +MODULE_AUTHOR("Qi Liu liuqi115@huawei.com");

diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h new file mode 100644 index 000000000000..a01f869fe122 --- /dev/null +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h @@ -0,0 +1,110 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ +/*

• • Siemens System Memory Buffer driver.
• • Copyright(c) 2021, HiSilicon Limited.
• */

+#ifndef _ULTRASOC_SMB_H +#define _ULTRASOC_SMB_H

+#include <linux/coresight.h> +#include <linux/list.h> +#include <linux/miscdevice.h>

+#include "coresight-etm-perf.h" +#include "coresight-priv.h"

+/* Offset of SMB logical buffer registers */ +#define SMB_CFG_REG 0x00 +#define SMB_GLOBAL_EN 0x04 +#define SMB_GLOBAL_INT 0x08 +#define SMB_LB_CFG_LO 0x40 +#define SMB_LB_CFG_HI 0x44 +#define SMB_LB_INT_CTRL 0x48 +#define SMB_LB_INT_STS 0x4c +#define SMB_LB_LIMIT 0x58 +#define SMB_LB_RD_ADDR 0x5c +#define SMB_LB_WR_ADDR 0x60 +#define SMB_LB_PURGE 0x64

+/* Set SMB_CFG_REG register */ +#define SMB_BURST_LEN GENMASK(7, 4) +#define SMB_IDLE_PRD GENMASK(15, 12) +#define SMB_MEM_WR GENMASK(17, 16) +#define SMB_MEM_RD (GENMASK(26, 25) | GENMASK(23, 22)) +#define SMB_GLOBAL_CFG (SMB_IDLE_PRD | SMB_MEM_WR | SMB_MEM_RD | \

• 		 SMB_BURST_LEN)
  

+/* Set SMB_GLOBAL_INT register */ +#define SMB_INT_EN BIT(0) +#define SMB_INT_TYPE_PULSE BIT(1) +#define SMB_INT_POLARITY_HIGH BIT(2) +#define SMB_GLB_INT_CFG (SMB_INT_EN | SMB_INT_TYPE_PULSE | \

• 		 SMB_INT_POLARITY_HIGH)
  

+/* Set SMB_LB_CFG_LO register */ +#define SMB_BUF_EN BIT(0) +#define SMB_BUF_SINGLE_END BIT(1) +#define SMB_BUF_INIT BIT(8) +#define SMB_BUF_CONTINUOUS BIT(11) +#define SMB_FLOW_MASK GENMASK(19, 16)

What does the value GENMASK(19, 16) indicate for SMB_FLOW ? That is the value being passed down. Please add a descriptive name. .e.g,

#define SMB_FLOW_MODE_X GENMASK(19, 16)

Similarly for SMB_BUF_NOTE_MASK, unless there is an explanation.

+#define SMB_BUF_CFG_STREAMING (SMB_BUF_INIT | SMB_BUF_CONTINUOUS | \

• 		 SMB_FLOW_MASK | SMB_BUF_SINGLE_END |	\
  

• 		 SMB_BUF_EN)
  

+#define SMB_BASE_LOW_MASK GENMASK(31, 0)

+/* Set SMB_LB_CFG_HI register */ +#define SMB_MSG_FILTER GENMASK(15, 8)

+/* Set SMB_LB_INT_CTRL */ +#define SMB_BUF_INT_EN BIT(0) +#define SMB_BUF_NOTE_MASK GENMASK(11, 8) +#define SMB_BUF_INT_CFG (SMB_BUF_INT_EN | SMB_BUF_NOTE_MASK)

+/**

• • struct smb_data_buffer - Details of the buffer used by SMB
• • @buf_base : Memory mapped base address of SMB.
• • @start_addr : SMB buffer start Physical address.
• • @buf_size : Size of the buffer.

Please align the field definitions.

• • @data_size : Size of Trace data copy to userspace.
• • @rd_offset : Offset of the read pointer in the buffer.
• • @wr_offset : Offset of the write pointer in the buffer.
• • @status : Status of SMB buffer.
• */

+struct smb_data_buffer {

• void __iomem *buf_base;
• u32 start_addr;
• unsigned long buf_size;
• unsigned long data_size;
• unsigned long rd_offset;
• unsigned long wr_offset;
• bool full;

+};

+/**

• • struct smb_drv_data - specifics associated to an SMB component
• • @base: Memory mapped base address for SMB component.
• • @csdev: Component vitals needed by the framework.
• • @sdb: Data buffer for SMB.
• • @miscdev: Specifics to handle "/dev/xyz.smb" entry.
• • @spinlock: Only one at a time pls.
• • @reading: Synchronise user space access to SMB buffer.
• • @pid: Process ID of the process being monitored by the session

• • that is using this component.
    

Same as above.

• • @mode: how this SMB is being used, perf mode or sysfs mode.
• */

+struct smb_drv_data {

• void __iomem *base;
• struct coresight_device *csdev;
• struct smb_data_buffer sdb;
• struct miscdevice miscdev;
• spinlock_t spinlock;
• local_t reading;
• pid_t pid;
• u32 mode;

+};

+#define smb_reg(name, offset) coresight_simple_reg32(struct smb_drv_data, name, offset)

+#endif

Suzuki