CoreSight

coresight@lists.linaro.org

1 participants
2381 discussions

Re: [PATCH v12 2/2] Documentation: Add document for UltraSoc SMB drivers

by hejunhao

Hi Yicong. On 2022/11/10 20:08, Yicong Yang wrote: > On 2022/11/9 21:50, Junhao He wrote: >> From: Qi Liu <liuqi115(a)huawei.com> >> >> This patch bring in documentation for UltraSoc SMB drivers. >> It simply describes the device, sysfs interface and the >> firmware bindings. >> >> Signed-off-by: Qi Liu <liuqi115(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> --- >> .../sysfs-bus-coresight-devices-ultra_smb | 31 +++++++ >> .../trace/coresight/ultrasoc-smb.rst | 80 +++++++++++++++++++ >> 2 files changed, 111 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> new file mode 100644 >> index 000000000000..deaefd508105 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> @@ -0,0 +1,31 @@ >> +What: /sys/bus/coresight/devices/ultra_smb<N>/enable_sink >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (RW) Add/remove a SMB device from a trace path. There can be >> + multiple sources for a single SMB device. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/buf_size >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the buffer size of each UltraSoc SMB device. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/buf_status >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB status register. >> + BIT(0) is zero means buffer is empty. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/read_pos >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB Read Pointer register. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/write_pos >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB Write Pointer register. >> diff --git a/Documentation/trace/coresight/ultrasoc-smb.rst b/Documentation/trace/coresight/ultrasoc-smb.rst >> new file mode 100644 >> index 000000000000..6d28ef0f6c88 >> --- /dev/null >> +++ b/Documentation/trace/coresight/ultrasoc-smb.rst >> @@ -0,0 +1,80 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +====================================== >> +UltraSoc - HW Assisted Tracing on SoC >> +====================================== >> + :Author: Qi Liu <liuqi115(a)huawei.com> >> + :Date: March 2022 >> + >> +Introduction >> +------------ >> + >> +UltraSoc SMB is a per SCCL(Super CPU Cluster) hardware, and it provides a >> +way to buffer and store CPU trace messages in a region of shared system >> +memory. SMB is plugged as a coresight sink device and the corresponding >> +trace generators (ETM) are plugged in as source devices. >> + >> +Sysfs files and directories >> +--------------------------- >> + >> +The SMB devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> + $# ls /sys/bus/coresight/devices/ >> + ultra_smb0 ultra_smb1 ultra_smb2 ultra_smb3 >> + >> +The ``ultra_smb<N>`` named SMB associated with SCCL.:: >> + >> + $# ls /sys/bus/coresight/devices/ultra_smb0 >> + enable_sink mgmt >> + $# ls /sys/bus/coresight/devices/ultra_smb0/mgmt >> + buf_size buf_status read_pos write_pos >> + >> +*Key file items are:-* >> + * ``read_pos``: Shows the value held by UltraSoc SMB Read Pointer register. >> + * ``write_pos``: Shows the value held by UltraSoc SMB Write Pointer register. >> + * ``buf_status``: Shows the value held by UltraSoc SMB status register. >> + BIT(0) is zero means buffer is empty. >> + * ``buf_size``: Shows the buffer size of each UltraSoc SMB device. >> + >> +Firmware Bindings >> +--------------------------- >> + >> +SMB device is only supported with ACPI, and ACPI binding of SMB device >> +describes SMB device indentifier, resource information and graph structure. >> + >> +SMB is identified by ACPI HID "HISI03A1", resource of device is declared using >> +the _CRS method. Each SMB must present two base address, the first one is the >> +configuration base address of SMB device, the second one is the base address of >> +shared system memory. >> + >> +examples:: >> + >> + Device(USMB) { \ >> + Name(_HID, "HISI03A1") \ >> + Name(_CRS, ResourceTemplate() { \ >> + MEM_RESRC(0x95100000, 0x951FFFFF, 0x100000) \ >> + MEM_RESRC(0x50000000, 0x53FFFFFF, 0x4000000) \ > I cannot find MEM_RESRC in ACPI Spec 6.4, any references? If it's a self defined macro just expand it here. > > btw, you need to cc linux-doc(a)vger.kernel.org. > > Thanks. "MEM_RESERVE" is a macro. will update the doc and add cc. Thanks for the comments! >> + }) \ >> + Name(_DSD, Package() { \ >> + ToUUID("ab02a46b-74c7-45a2-bd68-f7d344ef2153"), \ >> + /* Use CoreSight Graph ACPI bindings to describe connections topology */ >> + Package() { \ >> + 0, \ >> + 1, \ >> + Package() { \ >> + 1, \ >> + ToUUID("3ecbc8b6-1d0e-4fb3-8107-e627f805c6cd"), \ >> + 8, \ >> + Package() {0x8, 0, \_SB.S00.SL11.CL28.F008, 0}, \ >> + Package() {0x9, 0, \_SB.S00.SL11.CL29.F009, 0}, \ >> + Package() {0xa, 0, \_SB.S00.SL11.CL2A.F010, 0}, \ >> + Package() {0xb, 0, \_SB.S00.SL11.CL2B.F011, 0}, \ >> + Package() {0xc, 0, \_SB.S00.SL11.CL2C.F012, 0}, \ >> + Package() {0xd, 0, \_SB.S00.SL11.CL2D.F013, 0}, \ >> + Package() {0xe, 0, \_SB.S00.SL11.CL2E.F014, 0}, \ >> + Package() {0xf, 0, \_SB.S00.SL11.CL2F.F015, 0}, \ >> + } \ >> + } \ >> + }) \ >> + } >> > . > Best regards, Junhao.

2 years, 7 months

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

by hejunhao

Hi Yicong, Thanks for your comments. On 2022/11/10 20:05, Yicong Yang wrote: > Hi Junhao, > > On 2022/11/9 21:50, Junhao He wrote: >> From: Qi Liu <liuqi115(a)huawei.com> >> >> 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 trace" in system memory. >> >> SMB is developed by UltraSoc technology, which is acquired by >> Siemens, and we still use "UltraSoc" to name driver. >> > Some comments inline. > >> Signed-off-by: Qi Liu <liuqi115(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> Tested-by: JunHao He <hejunhao3(a)huawei.com> >> --- >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/ultrasoc-smb.c | 635 +++++++++++++++++++++ >> drivers/hwtracing/coresight/ultrasoc-smb.h | 116 ++++ >> 4 files changed, 763 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 45c1eb5dfcb7..cb17c207a728 100644 >> --- a/drivers/hwtracing/coresight/Kconfig >> +++ b/drivers/hwtracing/coresight/Kconfig >> @@ -201,4 +201,15 @@ config CORESIGHT_TRBE >> >> To compile this driver as a module, choose M here: the module will be >> called coresight-trbe. >> + >> +config ULTRASOC_SMB >> + tristate "Ultrasoc system memory buffer drivers" >> + depends on ARM64 && CORESIGHT_LINKS_AND_SINKS >> + help >> + This driver provides support for the Ultrasoc system memory buffer (SMB). >> + SMB is responsible for receiving the trace data from Coresight ETM devices >> + and storing them to a system buffer. >> + >> + To compile this driver as a module, choose M here: the module will be >> + called ultrasoc-smb. >> endif >> 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..ea2552a98d28 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c >> @@ -0,0 +1,635 @@ >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >> +/* >> + * Siemens System Memory Buffer driver. >> + * Copyright(c) 2022, HiSilicon Limited. >> + */ >> + >> +#include <linux/atomic.h> >> +#include <linux/acpi.h> >> +#include <linux/circ_buf.h> >> +#include <linux/err.h> >> +#include <linux/fs.h> >> +#include <linux/module.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/platform_device.h> >> + >> +#include "coresight-etm-perf.h" >> +#include "coresight-priv.h" >> +#include "ultrasoc-smb.h" >> + >> +DEFINE_CORESIGHT_DEVLIST(sink_devs, "ultra_smb"); >> + >> +#define ULTRASOC_SMB_DSM_UUID "82ae1283-7f6a-4cbe-aa06-53e8fb24db18" >> + >> +static bool smb_buffer_is_empty(struct smb_drv_data *drvdata) >> +{ >> + u32 buf_status = readl(drvdata->base + SMB_LB_INT_STS_REG); >> + >> + return !FIELD_PREP(SMB_LB_INT_STS_NOT_EMPTY_MSK, buf_status); > Should be FIELD_GET() here? Sure, Will fix in next version. >> +} >> + >> +static void smb_buffer_sync_status(struct smb_drv_data *drvdata) >> +{ >> + struct smb_data_buffer *sdb = &drvdata->sdb; >> + >> + sdb->wr_offset = readl(drvdata->base + SMB_LB_WR_ADDR_REG) - sdb->start_addr; >> + sdb->rd_offset = readl(drvdata->base + SMB_LB_RD_ADDR_REG) - sdb->start_addr; >> + sdb->full = sdb->wr_offset == sdb->rd_offset && !smb_buffer_is_empty(drvdata); > Mistake here? Why to overwrite wr_offset? Here is a comparison of "wr_offset". It maybe too width, I will split the line. >> +} >> + >> +static void smb_reset_buffer_status(struct smb_drv_data *drvdata) >> +{ >> + writel(SMB_LB_INT_BUF_STS_RESET, drvdata->base + SMB_LB_INT_STS_REG); > You'll reset other fields of SMB_LB_INT_STS_REG since SMB_LB_INT_BUF_STS_RESET is only > BIT [3, 0], if it's not intended I'd suggest to only update the necessary fields. So is > other places. Other bits are reserved. For simpler code, we can configure all the REG bits. And I will add a comment here. >> +} >> + >> +/* Purge data remaining in hardware path in case them influence next trace */ >> +static void smb_purge_data(struct smb_drv_data *drvdata) >> +{ >> + writel(SMB_LB_PURGE_PURGED, drvdata->base + SMB_LB_PURGE_REG); >> +} >> + >> +static void smb_update_data_size(struct smb_drv_data *drvdata) >> +{ >> + struct smb_data_buffer *sdb = &drvdata->sdb; >> + >> + smb_purge_data(drvdata); >> + smb_buffer_sync_status(drvdata); >> + if (sdb->full) { >> + sdb->data_size = sdb->buf_size; >> + return; >> + } >> + >> + sdb->data_size = CIRC_CNT(sdb->wr_offset, sdb->rd_offset, sdb->buf_size); >> +} >> + >> +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); >> + int ret = 0; >> + >> + mutex_lock(&drvdata->mutex); >> + >> + if (local_read(&drvdata->reading)) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + if (atomic_read(drvdata->csdev->refcnt)) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + local_set(&drvdata->reading, 1); >> +out: >> + mutex_unlock(&drvdata->mutex); >> + >> + 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; >> + ssize_t to_copy = 0; >> + >> + mutex_lock(&drvdata->mutex); >> + >> + if (!sdb->data_size) { >> + smb_update_data_size(drvdata); >> + if (!sdb->data_size) >> + goto out; >> + } >> + > It'd be better to check this earlier in open(). When SMB is full, If the data has been read and the "rd_offset" and "wr_offset" are not equal, the data remained in the link can be purged. So we need to dynamically update the data size in sysfs mode, and make sure to read all data. >> + 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_REG); >> + dev_dbg(dev, "%zu bytes copied\n", to_copy); >> +out: >> + if (!sdb->data_size) >> + smb_reset_buffer_status(drvdata); >> + mutex_unlock(&drvdata->mutex); >> + >> + 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); >> + >> + mutex_lock(&drvdata->mutex); >> + local_set(&drvdata->reading, 0); >> + mutex_unlock(&drvdata->mutex); >> + >> + return 0; >> +} >> + >> +static const struct file_operations smb_fops = { >> + .owner = THIS_MODULE, >> + .open = smb_open, >> + .read = smb_read, >> + .release = smb_release, >> + .llseek = no_llseek, >> +}; >> + >> +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); > Need to update the data size before sysfs_emit(). This value does not change after SMB driver probe. >> +} >> +static DEVICE_ATTR_RO(buf_size); >> + >> +static struct attribute *smb_sink_attrs[] = { >> + coresight_simple_reg32(read_pos, SMB_LB_RD_ADDR_REG), >> + coresight_simple_reg32(write_pos, SMB_LB_WR_ADDR_REG), >> + coresight_simple_reg32(buf_status, SMB_LB_INT_STS_REG), >> + &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 void smb_enable_hw(struct smb_drv_data *drvdata) >> +{ >> + writel(SMB_GLB_EN_HW_ENABLE, drvdata->base + SMB_GLB_EN_REG); >> +} >> + >> +static void smb_disable_hw(struct smb_drv_data *drvdata) >> +{ >> + writel(0x0, drvdata->base + SMB_GLB_EN_REG); >> +} >> + >> +static void smb_enable_sysfs(struct coresight_device *csdev) >> +{ >> + struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + if (drvdata->mode != CS_MODE_DISABLED) >> + return; >> + >> + smb_enable_hw(drvdata); >> + drvdata->mode = CS_MODE_SYSFS; >> +} >> + >> +static int smb_enable_perf(struct coresight_device *csdev, void *data) >> +{ >> + struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent); >> + struct perf_output_handle *handle = data; >> + struct cs_buffers *buf = etm_perf_sink_config(handle); >> + pid_t pid; >> + >> + if (!buf) >> + return -EINVAL; >> + >> + /* Get a handle on the pid of the target process */ >> + pid = buf->pid; >> + >> + /* Device is already in used by other session */ >> + if (drvdata->pid != -1 && drvdata->pid != pid) >> + return -EBUSY; >> + >> + if (drvdata->pid == -1) { >> + smb_enable_hw(drvdata); >> + drvdata->pid = pid; >> + drvdata->mode = CS_MODE_PERF; >> + } >> + >> + return 0; >> +} >> + >> +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 = 0; >> + >> + mutex_lock(&drvdata->mutex); >> + >> + /* Do nothing, the trace data is reading by other interface now */ >> + if (local_read(&drvdata->reading)) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + /* Do nothing, the SMB is already enabled as other mode */ >> + if (drvdata->mode != CS_MODE_DISABLED && drvdata->mode != mode) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + switch (mode) { >> + case CS_MODE_SYSFS: >> + smb_enable_sysfs(csdev); >> + break; >> + case CS_MODE_PERF: >> + ret = smb_enable_perf(csdev, data); >> + break; >> + default: >> + ret = -EINVAL; >> + } >> + >> + if (ret) >> + goto out; >> + >> + atomic_inc(csdev->refcnt); >> + >> + dev_dbg(&csdev->dev, "Ultrasoc SMB enabled\n"); >> +out: >> + mutex_unlock(&drvdata->mutex); >> + >> + return ret; >> +} >> + >> +static int smb_disable(struct coresight_device *csdev) >> +{ >> + struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent); >> + int ret = 0; >> + >> + mutex_lock(&drvdata->mutex); >> + >> + if (local_read(&drvdata->reading)) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + if (atomic_dec_return(csdev->refcnt)) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + /* Complain if we (somehow) got out of sync */ >> + WARN_ON_ONCE(drvdata->mode == CS_MODE_DISABLED); > Is there any conditions we can reach here? I see we always update > drvdata->mode within the mutex's protection. To be the same as other coresight sink driver. >> + >> + smb_disable_hw(drvdata); >> + smb_purge_data(drvdata); >> + >> + /* Dissociate from the target process. */ >> + drvdata->pid = -1; >> + drvdata->mode = CS_MODE_DISABLED; >> + >> + dev_dbg(&csdev->dev, "Ultrasoc SMB disabled\n"); >> +out: >> + mutex_unlock(&drvdata->mutex); >> + >> + return ret; >> +} >> + >> +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 head, >> + unsigned long data_size) >> +{ >> + struct smb_data_buffer *sdb = &drvdata->sdb; >> + char **dst_pages = (char **)buf->data_pages; >> + unsigned long to_copy; >> + long pg_idx, pg_offset; >> + >> + pg_idx = head >> PAGE_SHIFT; >> + pg_offset = head & (PAGE_SIZE - 1); >> + >> + while (data_size) { >> + unsigned long pg_space = PAGE_SIZE - pg_offset; >> + >> + /* Copy parts of trace data when read pointer wrap around */ >> + if (sdb->rd_offset + pg_space > sdb->buf_size) >> + to_copy = sdb->buf_size - sdb->rd_offset; >> + else >> + to_copy = min(data_size, pg_space); >> + >> + memcpy(dst_pages[pg_idx] + pg_offset, >> + sdb->buf_base + sdb->rd_offset, to_copy); >> + >> + pg_offset += to_copy; >> + if (pg_offset >= PAGE_SIZE) { >> + pg_offset = 0; >> + pg_idx++; >> + pg_idx %= 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_REG); >> + >> + /* >> + * 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_REG), >> + drvdata->base + SMB_LB_RD_ADDR_REG); >> + } >> + 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; >> + bool lost = false; >> + >> + if (!buf) >> + return 0; >> + >> + mutex_lock(&drvdata->mutex); >> + >> + /* 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, handle->head, data_size); >> + if (!buf->snapshot && lost) >> + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); >> + >> + smb_enable_hw(drvdata); >> +out: >> + mutex_unlock(&drvdata->mutex); >> + >> + 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, SMB_BUF_ADDR_RES); >> + if (IS_ERR(res)) { >> + dev_err(&pdev->dev, "SMB device failed to get resource\n"); >> + return -EINVAL; >> + } >> + >> + sdb->start_addr = FIELD_PREP(SMB_BUF_ADDR_LO_MSK, res->start); > also FIELD_GET()? Sure, Will fix in next version. >> + sdb->buf_size = resource_size(res); >> + if (sdb->buf_size == 0) >> + return -EINVAL; >> + >> + /* >> + * This is a chunk of memory, use classic mapping with better >> + * performance. >> + */ >> + base = devm_memremap(&pdev->dev, sdb->start_addr, sdb->buf_size, >> + MEMREMAP_WB); >> + 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_LB_CFG_LO_DEFAULT, drvdata->base + SMB_LB_CFG_LO_REG); >> + writel(SMB_LB_CFG_HI_DEFAULT, drvdata->base + SMB_LB_CFG_HI_REG); >> + writel(SMB_GLB_CFG_DEFAULT, drvdata->base + SMB_GLB_CFG_REG); >> + writel(SMB_GLB_INT_CFG, drvdata->base + SMB_GLB_INT_REG); >> + writel(SMB_LB_INT_CTRL_CFG, drvdata->base + SMB_LB_INT_CTRL_REG); >> +} >> + >> +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); >> + if (!desc.name) { >> + dev_err(&pdev->dev, "Failed to alloc coresight device name"); >> + return -ENOMEM; >> + } >> + desc.access = CSDEV_ACCESS_IOMEM(drvdata->base); >> + >> + 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 device *dev = &pdev->dev; >> + struct smb_drv_data *drvdata; >> + int ret; >> + >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + drvdata->base = devm_platform_ioremap_resource(pdev, SMB_REG_ADDR_RES); >> + if (IS_ERR(drvdata->base)) { >> + dev_err(dev, "Failed to ioremap resource\n"); >> + return PTR_ERR(drvdata->base); >> + } >> + >> + ret = smb_init_data_buffer(pdev, &drvdata->sdb); >> + if (ret) { >> + dev_err(dev, "Failed to init buffer, ret = %d\n", ret); >> + return ret; >> + } >> + >> + smb_init_hw(drvdata); >> + mutex_init(&drvdata->mutex); >> + drvdata->pid = -1; >> + >> + ret = smb_register_sink(pdev, drvdata); >> + if (ret) { >> + dev_err(dev, "Failed to register SMB sink\n"); >> + return ret; >> + } >> + >> + ret = smb_config_inport(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 CoreSight driver"); >> +MODULE_LICENSE("Dual MIT/GPL"); >> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen(a)huawei.com>"); >> +MODULE_AUTHOR("Qi Liu <liuqi115(a)huawei.com>"); >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h >> new file mode 100644 >> index 000000000000..2e2f9f8fe54b >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h >> @@ -0,0 +1,116 @@ >> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ >> +/* >> + * Siemens System Memory Buffer driver. >> + * Copyright(c) 2022, HiSilicon Limited. >> + */ >> + >> +#ifndef _ULTRASOC_SMB_H >> +#define _ULTRASOC_SMB_H >> + >> +#include <linux/miscdevice.h> >> +#include <linux/mutex.h> >> + >> +/* Offset of SMB global registers */ >> +#define SMB_GLB_CFG_REG 0x00 >> +#define SMB_GLB_EN_REG 0x04 >> +#define SMB_GLB_INT_REG 0x08 >> + >> +/* Offset of SMB logical buffer registers */ >> +#define SMB_LB_CFG_LO_REG 0x40 >> +#define SMB_LB_CFG_HI_REG 0x44 >> +#define SMB_LB_INT_CTRL_REG 0x48 >> +#define SMB_LB_INT_STS_REG 0x4c >> +#define SMB_LB_RD_ADDR_REG 0x5c >> +#define SMB_LB_WR_ADDR_REG 0x60 >> +#define SMB_LB_PURGE_REG 0x64 >> + >> +/* Set global config register */ >> +#define SMB_CFG_BURST_LEN_MSK GENMASK(11, 4) >> +#define SMB_CFG_IDLE_PRD_MSK GENMASK(15, 12) >> +#define SMB_CFG_MEM_WR_MSK GENMASK(21, 16) >> +#define SMB_CFG_MEM_RD_MSK GENMASK(27, 22) >> +#define SMB_GLB_CFG_DEFAULT (FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \ >> + FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \ >> + FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \ >> + FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b)) >> + >> +/* Set global interrupt control register */ >> +#define SMB_INT_EN BIT(0) >> +#define SMB_INT_PULSE BIT(1) /* Interrupt type: 1 - Pulse */ >> +#define SMB_INT_ACT_H BIT(2) /* Interrupt polarity: 1 - Active high */ >> +#define SMB_GLB_INT_CFG (SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H) >> + >> +/* Set logical buffer config register lower 32 bits */ >> +#define SMB_CFG_LO_EN BIT(0) >> +#define SMB_CFG_LO_SINGLE_END BIT(1) >> +#define SMB_CFG_LO_INIT BIT(8) >> +#define SMB_CFG_LO_CONT BIT(11) >> +#define SMB_CFG_LO_FLOW_MSK GENMASK(19, 16) >> +#define SMB_LB_CFG_LO_DEFAULT (SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \ >> + SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \ >> + FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf)) >> + >> +/* Set logical buffer config register upper 32 bits */ >> +#define SMB_CFG_HI_RANGE_UP_MSK GENMASK(15, 8) >> +#define SMB_LB_CFG_HI_DEFAULT FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff) >> + >> +/* Set logical buffer interrupt control register */ >> +#define SMB_INT_CTRL_EN BIT(0) >> +#define SMB_INT_CTRL_BUF_NOTE_MSK GENMASK(11, 8) >> +#define SMB_LB_INT_CTRL_CFG (SMB_INT_CTRL_EN | \ >> + FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf)) >> + >> +#define SMB_LB_INT_STS_NOT_EMPTY_MSK BIT(0) >> +#define SMB_LB_STS_RESET_MSK GENMASK(3, 0) >> +#define SMB_LB_INT_BUF_STS_RESET FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf) >> +#define SMB_LB_PURGE_PURGED BIT(0) >> +#define SMB_GLB_EN_HW_ENABLE BIT(0) >> + >> +#define SMB_REG_ADDR_RES 0 >> +#define SMB_BUF_ADDR_RES 1 >> +#define SMB_BUF_ADDR_LO_MSK GENMASK(31, 0) >> + >> +/** >> + * 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. >> + * @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. >> + * @full: Trace data overflow. >> + */ >> +struct smb_data_buffer { >> + void __iomem *buf_base; > The backend of buffer is a chunk of memory, so I guess we don't need > this __iomem here. Sure, Will fix in next version. >> + u32 start_addr; > Is u32 enough if we're running on a 64bit platform? You can use phys_addr_t > instead since you comment it to be "start Physical address". The system buffer start physical address point is declared by ACPI using the _CRS method, and will holds to 32bit registers "..._RD_ADDR_REG" and "..._WRADDR_REG". The hardware specification limits the address range and does not need to be compatible with 64-bit addresses, and this address is guaranteed to be 32 bits in ACPI. So u32 can be used. >> + 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. >> + * @mutex: Control data access to one at a time. >> + * @reading: Synchronise user space access to SMB buffer. >> + * @pid: Process ID of the process being monitored by the >> + * session that is using this component. >> + * @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; >> + struct mutex mutex; >> + local_t reading; > Why do we need to use local_t here? Considering the whole structure is > already protected by the mutex, maybe a boolean is enough? Yes, I will do that, the mutex already ensures "reading" consistency. > Thanks. > . > Best regards, Junhao.

2 years, 7 months

Re: [PATCH v12 2/2] Documentation: Add document for UltraSoc SMB drivers

by hejunhao

On 2022/11/10 0:57, Jonathan Cameron wrote: > On Wed, 9 Nov 2022 21:50:08 +0800 > Junhao He <hejunhao3(a)huawei.com> wrote: > >> From: Qi Liu <liuqi115(a)huawei.com> >> >> This patch bring in documentation for UltraSoc SMB drivers. >> It simply describes the device, sysfs interface and the >> firmware bindings. >> >> Signed-off-by: Qi Liu <liuqi115(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> > LTGM > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com> > > Thanks, > > Jonathan Hi Jonathan, Thanks for your review! Best regards, Junhao. >> --- >> .../sysfs-bus-coresight-devices-ultra_smb | 31 +++++++ >> .../trace/coresight/ultrasoc-smb.rst | 80 +++++++++++++++++++ >> 2 files changed, 111 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> new file mode 100644 >> index 000000000000..deaefd508105 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> @@ -0,0 +1,31 @@ >> +What: /sys/bus/coresight/devices/ultra_smb<N>/enable_sink >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (RW) Add/remove a SMB device from a trace path. There can be >> + multiple sources for a single SMB device. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/buf_size >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the buffer size of each UltraSoc SMB device. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/buf_status >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB status register. >> + BIT(0) is zero means buffer is empty. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/read_pos >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB Read Pointer register. >> + >> +What: /sys/bus/coresight/devices/ultra_smb<N>/mgmt/write_pos >> +Date: November 2022 >> +KernelVersion: 6.2 >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (Read) Shows the value held by UltraSoc SMB Write Pointer register. >> diff --git a/Documentation/trace/coresight/ultrasoc-smb.rst b/Documentation/trace/coresight/ultrasoc-smb.rst >> new file mode 100644 >> index 000000000000..6d28ef0f6c88 >> --- /dev/null >> +++ b/Documentation/trace/coresight/ultrasoc-smb.rst >> @@ -0,0 +1,80 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +====================================== >> +UltraSoc - HW Assisted Tracing on SoC >> +====================================== >> + :Author: Qi Liu <liuqi115(a)huawei.com> >> + :Date: March 2022 >> + >> +Introduction >> +------------ >> + >> +UltraSoc SMB is a per SCCL(Super CPU Cluster) hardware, and it provides a >> +way to buffer and store CPU trace messages in a region of shared system >> +memory. SMB is plugged as a coresight sink device and the corresponding >> +trace generators (ETM) are plugged in as source devices. >> + >> +Sysfs files and directories >> +--------------------------- >> + >> +The SMB devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> + $# ls /sys/bus/coresight/devices/ >> + ultra_smb0 ultra_smb1 ultra_smb2 ultra_smb3 >> + >> +The ``ultra_smb<N>`` named SMB associated with SCCL.:: >> + >> + $# ls /sys/bus/coresight/devices/ultra_smb0 >> + enable_sink mgmt >> + $# ls /sys/bus/coresight/devices/ultra_smb0/mgmt >> + buf_size buf_status read_pos write_pos >> + >> +*Key file items are:-* >> + * ``read_pos``: Shows the value held by UltraSoc SMB Read Pointer register. >> + * ``write_pos``: Shows the value held by UltraSoc SMB Write Pointer register. >> + * ``buf_status``: Shows the value held by UltraSoc SMB status register. >> + BIT(0) is zero means buffer is empty. >> + * ``buf_size``: Shows the buffer size of each UltraSoc SMB device. >> + >> +Firmware Bindings >> +--------------------------- >> + >> +SMB device is only supported with ACPI, and ACPI binding of SMB device >> +describes SMB device indentifier, resource information and graph structure. >> + >> +SMB is identified by ACPI HID "HISI03A1", resource of device is declared using >> +the _CRS method. Each SMB must present two base address, the first one is the >> +configuration base address of SMB device, the second one is the base address of >> +shared system memory. >> + >> +examples:: >> + >> + Device(USMB) { \ >> + Name(_HID, "HISI03A1") \ >> + Name(_CRS, ResourceTemplate() { \ >> + MEM_RESRC(0x95100000, 0x951FFFFF, 0x100000) \ >> + MEM_RESRC(0x50000000, 0x53FFFFFF, 0x4000000) \ >> + }) \ >> + Name(_DSD, Package() { \ >> + ToUUID("ab02a46b-74c7-45a2-bd68-f7d344ef2153"), \ >> + /* Use CoreSight Graph ACPI bindings to describe connections topology */ >> + Package() { \ >> + 0, \ >> + 1, \ >> + Package() { \ >> + 1, \ >> + ToUUID("3ecbc8b6-1d0e-4fb3-8107-e627f805c6cd"), \ >> + 8, \ >> + Package() {0x8, 0, \_SB.S00.SL11.CL28.F008, 0}, \ >> + Package() {0x9, 0, \_SB.S00.SL11.CL29.F009, 0}, \ >> + Package() {0xa, 0, \_SB.S00.SL11.CL2A.F010, 0}, \ >> + Package() {0xb, 0, \_SB.S00.SL11.CL2B.F011, 0}, \ >> + Package() {0xc, 0, \_SB.S00.SL11.CL2C.F012, 0}, \ >> + Package() {0xd, 0, \_SB.S00.SL11.CL2D.F013, 0}, \ >> + Package() {0xe, 0, \_SB.S00.SL11.CL2E.F014, 0}, \ >> + Package() {0xf, 0, \_SB.S00.SL11.CL2F.F015, 0}, \ >> + } \ >> + } \ >> + }) \ >> + } > . >

2 years, 7 months

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

by hejunhao

On 2022/11/10 0:56, Jonathan Cameron wrote: > On Wed, 9 Nov 2022 21:50:07 +0800 > Junhao He <hejunhao3(a)huawei.com> wrote: > >> From: Qi Liu <liuqi115(a)huawei.com> >> >> 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 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(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> Tested-by: JunHao He <hejunhao3(a)huawei.com> > Hi JunHao, > > One trivial side effect of dropping the ACPI dependency. > > Also, I think (at the cost of a slightly lengthening of lines) > you can rename the register fields to avoid any potential > confusion between GLB and LB registers. > > With those fixed feel free to add > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com> > Hi Jonathan, Thanks for you comments! I will fix these in the next version. Thanks. > > ... > >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c >> new file mode 100644 >> index 000000000000..ea2552a98d28 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c > ... > >> +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), > Now the driver build isn't dependent on CONFIG_ACPI > if !CONFIG_ACPI ACPI_PTR() doesn't reference the parameter. > As such you'll get unused warnings. > > 1 options to fix this > a) Drop ACPI_PTR() and just have .acpi_match_data = ultrasoc_smb_acpi_match > b) ifdef magic around the acpi_match table. > > In theory the first option results in bloat, but in this case I doubt we care. Ok, will fix it, as follows ``` #ifdef CONFIG_ACPI static const struct acpi_device_id ultrasoc_smb_acpi_match[] = { {"HISI03A1", 0}, {} }; MODULE_DEVICE_TABLE(acpi, ultrasoc_smb_acpi_match); #endif ``` >> + .suppress_bind_attrs = true, >> + }, >> + .probe = smb_probe, >> + .remove = smb_remove, >> +}; >> +module_platform_driver(smb_driver); >> + >> +MODULE_DESCRIPTION("UltraSoc SMB CoreSight driver"); >> +MODULE_LICENSE("Dual MIT/GPL"); >> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen(a)huawei.com>"); >> +MODULE_AUTHOR("Qi Liu <liuqi115(a)huawei.com>"); >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h >> new file mode 100644 >> index 000000000000..2e2f9f8fe54b >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h >> @@ -0,0 +1,116 @@ >> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ >> +/* >> + * Siemens System Memory Buffer driver. >> + * Copyright(c) 2022, HiSilicon Limited. >> + */ >> + >> +#ifndef _ULTRASOC_SMB_H >> +#define _ULTRASOC_SMB_H >> + >> +#include <linux/miscdevice.h> >> +#include <linux/mutex.h> >> + >> +/* Offset of SMB global registers */ >> +#define SMB_GLB_CFG_REG 0x00 >> +#define SMB_GLB_EN_REG 0x04 >> +#define SMB_GLB_INT_REG 0x08 >> + >> +/* Offset of SMB logical buffer registers */ >> +#define SMB_LB_CFG_LO_REG 0x40 >> +#define SMB_LB_CFG_HI_REG 0x44 >> +#define SMB_LB_INT_CTRL_REG 0x48 >> +#define SMB_LB_INT_STS_REG 0x4c >> +#define SMB_LB_RD_ADDR_REG 0x5c >> +#define SMB_LB_WR_ADDR_REG 0x60 >> +#define SMB_LB_PURGE_REG 0x64 >> + >> +/* Set global config register */ >> +#define SMB_CFG_BURST_LEN_MSK GENMASK(11, 4) > Given there are several CFG registers, possibly worth > prefix of SMB_GLB_CFG_ ... Sure, I will do that. >> +#define SMB_CFG_IDLE_PRD_MSK GENMASK(15, 12) >> +#define SMB_CFG_MEM_WR_MSK GENMASK(21, 16) >> +#define SMB_CFG_MEM_RD_MSK GENMASK(27, 22) >> +#define SMB_GLB_CFG_DEFAULT (FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | \ >> + FIELD_PREP(SMB_CFG_IDLE_PRD_MSK, 0xf) | \ >> + FIELD_PREP(SMB_CFG_MEM_WR_MSK, 0x3) | \ >> + FIELD_PREP(SMB_CFG_MEM_RD_MSK, 0x1b)) >> + >> +/* Set global interrupt control register */ >> +#define SMB_INT_EN BIT(0) > Again, multiple INT registers, so SMB_INT_GLB_* perhaps? Ok, will fix it. >> +#define SMB_INT_PULSE BIT(1) /* Interrupt type: 1 - Pulse */ >> +#define SMB_INT_ACT_H BIT(2) /* Interrupt polarity: 1 - Active high */ >> +#define SMB_GLB_INT_CFG (SMB_INT_EN | SMB_INT_PULSE | SMB_INT_ACT_H) >> + >> +/* Set logical buffer config register lower 32 bits */ >> +#define SMB_CFG_LO_EN BIT(0) > SMB_LB_CFG_... > > etc for other cases. Sure, I will do that. >> +#define SMB_CFG_LO_SINGLE_END BIT(1) >> +#define SMB_CFG_LO_INIT BIT(8) >> +#define SMB_CFG_LO_CONT BIT(11) >> +#define SMB_CFG_LO_FLOW_MSK GENMASK(19, 16) >> +#define SMB_LB_CFG_LO_DEFAULT (SMB_CFG_LO_EN | SMB_CFG_LO_SINGLE_END | \ >> + SMB_CFG_LO_INIT | SMB_CFG_LO_CONT | \ >> + FIELD_PREP(SMB_CFG_LO_FLOW_MSK, 0xf)) >> + >> +/* Set logical buffer config register upper 32 bits */ >> +#define SMB_CFG_HI_RANGE_UP_MSK GENMASK(15, 8) >> +#define SMB_LB_CFG_HI_DEFAULT FIELD_PREP(SMB_CFG_HI_RANGE_UP_MSK, 0xff) >> + >> +/* Set logical buffer interrupt control register */ >> +#define SMB_INT_CTRL_EN BIT(0) >> +#define SMB_INT_CTRL_BUF_NOTE_MSK GENMASK(11, 8) >> +#define SMB_LB_INT_CTRL_CFG (SMB_INT_CTRL_EN | \ >> + FIELD_PREP(SMB_INT_CTRL_BUF_NOTE_MSK, 0xf)) >> + >> +#define SMB_LB_INT_STS_NOT_EMPTY_MSK BIT(0) >> +#define SMB_LB_STS_RESET_MSK GENMASK(3, 0) >> +#define SMB_LB_INT_BUF_STS_RESET FIELD_PREP(SMB_LB_STS_RESET_MSK, 0xf) >> +#define SMB_LB_PURGE_PURGED BIT(0) >> +#define SMB_GLB_EN_HW_ENABLE BIT(0) >> + >> +#define SMB_REG_ADDR_RES 0 >> +#define SMB_BUF_ADDR_RES 1 >> +#define SMB_BUF_ADDR_LO_MSK GENMASK(31, 0) > ... > >> + >> +/** >> + * 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. >> + * @mutex: Control data access to one at a time. >> + * @reading: Synchronise user space access to SMB buffer. >> + * @pid: Process ID of the process being monitored by the >> + * session that is using this component. >> + * @mode: how this SMB is being used, perf mode or sysfs mode. > Trivial, but for consistency should be: How this... Yes, I will do that. >> + */ >> +struct smb_drv_data { >> + void __iomem *base; >> + struct coresight_device *csdev; >> + struct smb_data_buffer sdb; >> + struct miscdevice miscdev; >> + struct mutex mutex; >> + local_t reading; >> + pid_t pid; >> + u32 mode; >> +}; >> + >> +#endif > . > Best regards, Junhao.

2 years, 7 months

[PATCH v12 0/2] Add support for UltraSoc System Memory Buffer

by Junhao He

Add support for UltraSoc System Memory Buffer. Change since v11: - Modify the code style and rename the register macro according to Jonathan's comment. - Link: https://lore.kernel.org/lkml/20221107130624.59886-1-hejunhao3@huawei.com/ Change since v10: - Rebase onto v6.1-rc4, included similar sysfs register accessors (as same as James's patch) - Link: https://lore.kernel.org/lkml/20221022115929.7503-1-hejunhao3@huawei.com/ Change since v9: - Update the Contact tag in SMB document. - Replace the spinlock with mutex. - Do some clean-ups in "smb_enable()" and "smb_release()". - Use classic memory mapped interface. - Link: https://lore.kernel.org/linux-arm-kernel/20220818132231.28240-1-hejunhao3@h… Change since v8: - Insert a blank line at the end of the config tag in Kconfig to Randy's comment. - Link: https://lore.kernel.org/linux-arm-kernel/20220816131634.38195-1-hejunhao3@h… Change since v7: - Use the macros for register bit flags and numbers of resource. - Cleanup punctuation. - Update the Date tag and the KernelVersion tag in the document. - Link: https://lore.kernel.org/lkml/20220712091353.34540-1-hejunhao3@huawei.com/ Change since v6: - Modify the code style and driver description according to Suzuki's comment. - Modify configuration of "drvdata->reading", to void problems in open/read concurrency scenario. - Rename the macro of "SMB_FLOW_MASK". - Use the "handle->head" to determine the page number and offset. - Link: https://lore.kernel.org/linux-arm-kernel/20220606130223.57354-1-liuqi115@hu… Change since v5: - Address the comments from Suzuki, add some comments in SMB document, and modify configuration of "drvdata->reading", to void problems in multi-core concurrency scenario - Link: https://lore.kernel.org/linux-arm-kernel/20220416083953.52610-1-liuqi115@hu… Change since v4: - Add a simple document of SMB driver according to Suzuki's comment. - Address the comments from Suzuki. - Link: https://lore.kernel.org/linux-arm-kernel/20220128061755.31909-1-liuqi115@hu… Change since v3: - Modify the file header according to community specifications. - Address the comments from Mathieu. - Link: https://lore.kernel.org/linux-arm-kernel/20211118110016.40398-1-liuqi115@hu… Change since v2: - Move ultrasoc driver to drivers/hwtracing/coresight by Mathieu's comment. - Link: https://lists.linaro.org/pipermail/coresight/2021-November/007310.html Change since v1: - Drop the document of UltraSoc according to Mathieu's comment. - Add comments to explain some private hardware settings. - Address the comments from Mathieu. - Link: https://lists.linaro.org/pipermail/coresight/2021-August/006842.html Change since RFC: - Move driver to drivers/hwtracing/coresight/ultrasoc. - Remove ultrasoc-axi-com.c, as AXI-COM doesn't need to be configured in basic tracing function. - Remove ultrasoc.c as SMB does not need to register with the ultrasoc core. - Address the comments from Mathieu and Suzuki. - Link: https://lists.linaro.org/pipermail/coresight/2021-June/006535.html Qi Liu (2): drivers/coresight: Add UltraSoc System Memory Buffer driver Documentation: Add document for UltraSoc SMB drivers .../sysfs-bus-coresight-devices-ultra_smb | 31 + .../trace/coresight/ultrasoc-smb.rst | 80 +++ drivers/hwtracing/coresight/Kconfig | 11 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/ultrasoc-smb.c | 635 ++++++++++++++++++ drivers/hwtracing/coresight/ultrasoc-smb.h | 117 ++++ 6 files changed, 875 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h -- 2.33.0

2 years, 7 months

[PATCH v14 0/9] Coresight: Add support for TPDM and TPDA

by Mao Jinlong

This series adds support for the trace performance monitoring and diagnostics hardware (TPDM and TPDA). It is composed of two major elements. a) Changes for original coresight framework to support for TPDM and TPDA. b) Add driver code for TPDM and TPDA. Introduction of changes for original coresight framework Support TPDM as new coresight source. Since only STM and ETM are supported as coresight source originally. TPDM is a newly added coresight source. We need to change the original way of saving coresight path to support more types source for coresight driver. The following patch is to add support more coresight sources. coresight: core: Use IDR for non-cpu bound sources' paths. Introduction of TPDM and TPDA TPDM - The trace performance monitoring and diagnostics monitor or TPDM in short serves as data collection component for various dataset types specified in the QPMDA(Qualcomm performance monitoring and diagnostics architecture) spec. The primary use case of the TPDM is to collect data from different data sources and send it to a TPDA for packetization, timestamping and funneling. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware definitions coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support docs: sysfs: coresight: Add sysfs ABI documentation for TPDM TPDA - The trace performance monitoring and diagnostics aggregator or TPDA in short serves as an arbitration and packetization engine for the performance monitoring and diagnostics network as specified in the QPMDA (Qualcomm performance monitoring and diagnostics architecture) specification. The primary use case of the TPDA is to provide packetization, funneling and timestamping of Monitor data as specified in the QPMDA specification. The following patch is to add driver for TPDA. Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions The last patch of this series is a device tree modification, which add the TPDM and TPDA configuration to device tree for validating. ARM: dts: msm: Add coresight components for SM8250 ARM: dts: msm: Add tpdm mm/prng for sm8250 Once this series patches are applied properly, the tpdm and tpda nodes should be observed at the coresight path /sys/bus/coresight/devices e.g. /sys/bus/coresight/devices # ls -l | grep tpd tpda0 -> ../../../devices/platform/soc(a)0/6004000.tpda/tpda0 tpdm0 -> ../../../devices/platform/soc(a)0/6c08000.mm.tpdm/tpdm0 We can use the commands are similar to the below to validate TPDMs. Enable coresight sink first. echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test The test data will be collected in the coresight sink which is enabled. If rwp register of the sink is keeping updating when do integration_test (by cat tmc_etf0/mgmt/rwp), it means there is data generated from TPDM to sink. There must be a tpda between tpdm and the sink. When there are some other trace event hw components in the same HW block with tpdm, tpdm and these hw components will connect to the coresight funnel. When there is only tpdm trace hw in the HW block, tpdm will connect to tpda directly. +---------------+ +-------------+ | tpdm@6c08000 | |tpdm@684C000 | +-------|-------+ +------|------+ | | +-------|-------+ | | funnel@6c0b000| | +-------|-------+ | | | +-------|-------+ | |funnel@6c2d000 | | +-------|-------+ | | | | +---------------+ | +----- tpda@6004000 -----------+ +-------|-------+ | +-------|-------+ |funnel@6005000 | +---------------+ This patch series depends on patch series: "[v5,00/14] coresight: Add new API to allocate trace source ID values" https://patchwork.kernel.org/project/linux-arm-kernel/cover/20221101163103.… Mao Jinlong (9): coresight: core: Use IDR for non-cpu bound sources' paths. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions arm64: dts: qcom: sm8250: Add coresight components arm64: dts: qcom: sm8250: Add tpdm mm/prng .../testing/sysfs-bus-coresight-devices-tpdm | 13 + .../bindings/arm/qcom,coresight-tpda.yaml | 111 +++ .../bindings/arm/qcom,coresight-tpdm.yaml | 93 +++ MAINTAINERS | 1 + arch/arm64/boot/dts/qcom/sm8250.dtsi | 671 ++++++++++++++++++ drivers/hwtracing/coresight/Kconfig | 23 + drivers/hwtracing/coresight/Makefile | 2 + drivers/hwtracing/coresight/coresight-core.c | 42 +- drivers/hwtracing/coresight/coresight-tpda.c | 208 ++++++ drivers/hwtracing/coresight/coresight-tpda.h | 35 + drivers/hwtracing/coresight/coresight-tpdm.c | 259 +++++++ drivers/hwtracing/coresight/coresight-tpdm.h | 62 ++ include/linux/coresight.h | 1 + 13 files changed, 1509 insertions(+), 12 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h -- 2.17.1

2 years, 7 months

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

by hejunhao

Hi Jonathan, Thanks for your comments. On 2022/11/7 22:30, Jonathan Cameron wrote: > On Mon, 7 Nov 2022 21:06:23 +0800 > Junhao He <hejunhao3(a)huawei.com> wrote: > >> From: Qi Liu <liuqi115(a)huawei.com> >> >> 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 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(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> Tested-by: JunHao He <hejunhao3(a)huawei.com> > Hi JunHao, > > It's been a while since I last looked at this driver, so I may have > forgotten or missed previous discussions. > > All the comments inline are fairly superficial and mostly concerned > with making the code easy to review / maintain rather than correctness. > > Jonathan Yes, I will do that. Junhao. >> --- >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/ultrasoc-smb.c | 631 +++++++++++++++++++++ >> drivers/hwtracing/coresight/ultrasoc-smb.h | 113 ++++ >> 4 files changed, 756 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 45c1eb5dfcb7..05d791cb05e3 100644 >> --- a/drivers/hwtracing/coresight/Kconfig >> +++ b/drivers/hwtracing/coresight/Kconfig >> @@ -201,4 +201,15 @@ config CORESIGHT_TRBE >> >> To compile this driver as a module, choose M here: the module will be >> called coresight-trbe. >> + >> +config ULTRASOC_SMB >> + tristate "Ultrasoc system memory buffer drivers" >> + depends on ACPI && ARM64 && CORESIGHT_LINKS_AND_SINKS > Can you relax this at all in the interests of getting better CI build coverage > from random configs etc. > > >From a quick look, I think you can safely drop the ACPI dependency on basis > relevant functions are stubbed out in acpi.h > > However, it looks like coresight more generally uses such depends, so perhaps > better to just leave them here for consistency. Ok, Will check it. Thanks. >> + help >> + This driver provides support for the Ultrasoc system memory buffer (SMB). >> + SMB is responsible for receiving the trace data from Coresight ETM devices >> + and storing them to a system buffer. >> + >> + To compile this driver as a module, choose M here: the module will be >> + called ultrasoc-smb. >> endif > >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c >> new file mode 100644 >> index 000000000000..7fe8bf9623e8 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c >> @@ -0,0 +1,631 @@ > ... > > >> + >> +static void smb_buffer_sync_status(struct smb_drv_data *drvdata) >> +{ >> + struct smb_data_buffer *sdb = &drvdata->sdb; >> + >> + sdb->wr_offset = readl(drvdata->base + SMB_LB_WR_ADDR) - sdb->start_addr; >> + sdb->rd_offset = readl(drvdata->base + SMB_LB_RD_ADDR) - sdb->start_addr; >> + if (sdb->wr_offset == sdb->rd_offset && !smb_buffer_is_empty(drvdata)) >> + sdb->full = true; >> + else >> + sdb->full = false; > Could do as > sdb->full = sdb->wr_offset == sdb->rd_offset && !smb_buffer_is_empty(drvdata); > up to you on which you think is more readable. Ok, I will drop this "if else" block. Thanks. > >> +} >> + > > >> +static struct attribute *smb_sink_attrs[] = { >> + coresight_simple_reg32(read_pos, SMB_LB_RD_ADDR), >> + coresight_simple_reg32(write_pos, SMB_LB_WR_ADDR), >> + coresight_simple_reg32(buf_status, SMB_LB_INT_STS), >> + &dev_attr_buf_size.attr, >> + NULL, > As below. Sure, Will fix in next version. Thanks. > >> +}; >> + >> +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, > Generally no comma after a NULL terminator. Having a comma > implies it may make sense to put something after it, which is never > the case for these. Sure, Will fix in next version. Thanks. >> +}; >> + > > ... > >> +static void smb_sync_perf_buffer(struct smb_drv_data *drvdata, >> + struct cs_buffers *buf, >> + unsigned long head, >> + unsigned long data_size) >> +{ >> + struct smb_data_buffer *sdb = &drvdata->sdb; >> + char **dst_pages = (char **)buf->data_pages; > Do you need the cast? It's void ** so implicit cast should work I think. > char **dst_pages = buf->data_pages; Ok, Will fix in next version. Thanks. >> + unsigned long to_copy; >> + long pg_idx, pg_offset; >> + >> + pg_idx = head >> PAGE_SHIFT; >> + pg_offset = head & (PAGE_SIZE - 1); >> + >> + while (data_size) { >> + unsigned long pg_space = PAGE_SIZE - pg_offset; >> + >> + /* Copy parts of trace data when read pointer wrap around */ >> + if (sdb->rd_offset + pg_space > sdb->buf_size) >> + to_copy = sdb->buf_size - sdb->rd_offset; >> + else >> + to_copy = min(data_size, pg_space); >> + >> + memcpy(dst_pages[pg_idx] + pg_offset, >> + sdb->buf_base + sdb->rd_offset, to_copy); >> + >> + pg_offset += to_copy; >> + if (pg_offset >= PAGE_SIZE) { >> + pg_offset = 0; >> + pg_idx++; >> + pg_idx %= 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 void smb_init_hw(struct smb_drv_data *drvdata) >> +{ >> + /* First disable smb and clear the status of SMB buffer */ > Check for consistency in capitalization of SMB in all comments. Sure, Will fix in next version. Thanks. > >> + 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_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct smb_drv_data *drvdata; >> + int ret; >> + >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + drvdata->base = devm_platform_ioremap_resource(pdev, SMB_BASE_ADDR_RES); >> + if (IS_ERR(drvdata->base)) { >> + dev_err(dev, "Failed to ioremap resource\n"); >> + return PTR_ERR(drvdata->base); >> + } >> + >> + ret = smb_init_data_buffer(pdev, &drvdata->sdb); >> + if (ret) { >> + dev_err(dev, "Failed to init buffer, ret = %d\n", ret); >> + return ret; >> + } >> + >> + smb_init_hw(drvdata); >> + mutex_init(&drvdata->mutex); >> + drvdata->pid = -1; >> + >> + ret = smb_register_sink(pdev, drvdata); >> + if (ret) { >> + dev_err(dev, "Failed to register smb sink\n"); >> + return ret; >> + } >> + >> + ret = smb_config_inport(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); > Trivial: I find a blank line before plane returns like this helps > a little with readability. Up to you though! Ok, Will fix it. Thanks. > >> + return 0; >> +} >> + >> +static const struct acpi_device_id ultrasoc_smb_acpi_match[] = { >> + {"HISI03A1", 0}, >> + {}, > Trivial, but little point in a trailing comma on a NULL terminator. Sure, Will fix in next version. Thanks. > > {} > }; > > is normally fine - note this is a bit subsystem specific so maintainer > may say otherwise. Ok, Thanks. > >> +}; >> +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 CoreSight driver"); >> +MODULE_LICENSE("Dual MIT/GPL"); >> +MODULE_AUTHOR("Jonathan Zhou <jonathan.zhouwen(a)huawei.com>"); >> +MODULE_AUTHOR("Qi Liu <liuqi115(a)huawei.com>"); >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.h b/drivers/hwtracing/coresight/ultrasoc-smb.h >> new file mode 100644 >> index 000000000000..56170e1a883d >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.h >> @@ -0,0 +1,113 @@ >> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ >> +/* >> + * Siemens System Memory Buffer driver. >> + * Copyright(c) 2022, HiSilicon Limited. >> + */ >> + >> +#ifndef _ULTRASOC_SMB_H >> +#define _ULTRASOC_SMB_H >> + >> +#include <linux/coresight.h> > I think you could move this down into the c files and provide > a forwards definition of struct coresight device > > Always good to keep scope of includes to minimum necessary. Sure, Will fix in next version. Thanks. > >> +#include <linux/miscdevice.h> >> +#include <linux/mutex.h> >> + >> +/* Offset of SMB logical buffer registers */ >> +#define SMB_CFG_REG 0x00 > To avoid any naming confusion I would postfix all the register > addresses with _REG > Then rethink the naming so the field names make it clear which > register they are in. OK, I'll check all REG names. Thanks. > >> +#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)) > Are these masks, or default values? Ideally express them as > a field mask then the value via FIELD_PREP > > e.g. > #define SMB_CFG_BURST_LEN_MSK GENMASK(7, 4) > #define SMB_GLOBAL_CFG_DEFAULT ... | FIELD_PREP(SMB_CFG_BURST_LEN_MSK, 0xf) | etc Sure, I will do that. Thanks. > > >> +#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_FILTER_FLOW GENMASK(19, 16) >> +#define SMB_BUF_CFG_STREAMING (SMB_BUF_INIT | SMB_BUF_CONTINUOUS | \ >> + SMB_FILTER_FLOW | 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) >> + >> +#define SMB_BUF_NOT_EMPTY BIT(0) >> +#define SMB_RESET_BUF_STS GENMASK(3, 0) >> +#define SMB_PURGED BIT(0) >> +#define SMB_HW_ENABLE BIT(0) > It is useful to give fields names that reflect which register they are in. > Perhaps > SMB_GLOBAL_EN_HW_ENABLE for this one. Sure, I will do that. Thanks. > >> + >> +#define SMB_BASE_ADDR_RES 0 >> +#define SMB_BUF_INFO_RES 1 >> + >> +/** >> + * 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. >> + * @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. > Naming wrong. Sure, Will fix in next version. Thanks. > >> + */ >> +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. >> + * @mutex: Control data access to one at a time. >> + * @reading: Synchronise user space access to SMB buffer. >> + * @pid: Process ID of the process being monitored by the >> + * session that is using this component. >> + * @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; >> + struct mutex mutex; >> + local_t reading; >> + pid_t pid; >> + u32 mode; >> +}; >> + >> +#endif > . > Best regards, Junhao.

2 years, 7 months

Re: [PATCH v11 2/2] Documentation: Add document for UltraSoc SMB drivers

by hejunhao

Hi Jonathan, On 2022/11/7 22:33, Jonathan Cameron wrote: > On Mon, 7 Nov 2022 21:06:24 +0800 > Junhao He <hejunhao3(a)huawei.com> wrote: > >> From: Qi Liu <liuqi115(a)huawei.com> >> >> This patch bring in documentation for UltraSoc SMB drivers. >> It simply describes the device, sysfs interface and the >> firmware bindings. >> >> Signed-off-by: Qi Liu <liuqi115(a)huawei.com> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> --- >> .../sysfs-bus-coresight-devices-ultra_smb | 31 +++++++ >> .../trace/coresight/ultrasoc-smb.rst | 80 +++++++++++++++++++ >> 2 files changed, 111 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> new file mode 100644 >> index 000000000000..7804aef4b40b >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb >> @@ -0,0 +1,31 @@ >> +What: /sys/bus/coresight/devices/ultra_smb<N>/enable_sink >> +Date: October 2022 >> +KernelVersion: 6.1 > Update to 6.2 as this will hopefully go in next merge window and it makes > life easier for the maintainer if they don't need to fix all these up. Ok, i will fix in next version. Thanks. >> +Contact: Junhao He <hejunhao3(a)huawei.com> >> +Description: (RW) Add/remove a SMB device from a trace path. There can be >> + multiple sources for a single SMB device. > Rest looks good to me, (not an expert on this stuff though!) > > Jonathan > . > Best regards, Junhao.

2 years, 7 months

[PATCH v3] coresight: etm4x: add CPU hotplug support for probing

by Tamas Zsoldos

etm4x devices cannot be successfully probed when their CPU is offline. For example, when booting with maxcpus=n, ETM probing will fail on CPUs >n, and the probing won't be reattempted once the CPUs come online. This will leave those CPUs unable to make use of ETM. This change adds a mechanism to delay the probing if the corresponding CPU is offline, and to try it again when the CPU comes online. Signed-off-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com> --- Changes since v2: * Fix tear down issue (thanks to Suzuki). --- .../coresight/coresight-etm4x-core.c | 153 +++++++++++++----- 1 file changed, 113 insertions(+), 40 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 87299e99dabb..71ab1d6e72dc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -66,10 +66,13 @@ static enum cpuhp_state hp_online; struct etm4_init_arg { unsigned int pid; - struct etmv4_drvdata *drvdata; + struct device *dev; struct csdev_access *csa; }; +static DEFINE_PER_CPU(struct etm4_init_arg *, delayed_probe); +static int etm4_probe_cpu(unsigned int cpu); + /* * Check if TRCSSPCICRn(i) is implemented for a given instance. * @@ -1071,7 +1074,7 @@ static void etm4_init_arch_data(void *info) struct csdev_access *csa; int i; - drvdata = init_arg->drvdata; + drvdata = dev_get_drvdata(init_arg->dev); csa = init_arg->csa; /* @@ -1514,7 +1517,7 @@ void etm4_config_trace_mode(struct etmv4_config *config) static int etm4_online_cpu(unsigned int cpu) { if (!etmdrvdata[cpu]) - return 0; + return etm4_probe_cpu(cpu); if (etmdrvdata[cpu]->boot_enable && !etmdrvdata[cpu]->sticky_enable) coresight_enable(etmdrvdata[cpu]->csdev); @@ -1890,48 +1893,20 @@ static void etm4_pm_clear(void) } } -static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) { int ret; struct coresight_platform_data *pdata = NULL; - struct etmv4_drvdata *drvdata; + struct device *dev = init_arg->dev; + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct coresight_desc desc = { 0 }; - struct etm4_init_arg init_arg = { 0 }; u8 major, minor; char *type_name; - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); if (!drvdata) - return -ENOMEM; - - dev_set_drvdata(dev, drvdata); - - if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) - pm_save_enable = coresight_loses_context_with_cpu(dev) ? - PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER; - - if (pm_save_enable != PARAM_PM_SAVE_NEVER) { - drvdata->save_state = devm_kmalloc(dev, - sizeof(struct etmv4_save_state), GFP_KERNEL); - if (!drvdata->save_state) - return -ENOMEM; - } - - drvdata->base = base; - - spin_lock_init(&drvdata->spinlock); - - drvdata->cpu = coresight_get_cpu(dev); - if (drvdata->cpu < 0) - return drvdata->cpu; - - init_arg.drvdata = drvdata; - init_arg.csa = &desc.access; - init_arg.pid = etm_pid; + return -EINVAL; - if (smp_call_function_single(drvdata->cpu, - etm4_init_arch_data, &init_arg, 1)) - dev_err(dev, "ETM arch init failed\n"); + desc.access = *init_arg->csa; if (!drvdata->arch) return -EINVAL; @@ -2002,6 +1977,68 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) return 0; } +static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +{ + struct etmv4_drvdata *drvdata; + struct csdev_access access = { 0 }; + struct etm4_init_arg init_arg = { 0 }; + struct etm4_init_arg *delayed; + + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + dev_set_drvdata(dev, drvdata); + + if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) + pm_save_enable = coresight_loses_context_with_cpu(dev) ? + PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER; + + if (pm_save_enable != PARAM_PM_SAVE_NEVER) { + drvdata->save_state = devm_kmalloc(dev, + sizeof(struct etmv4_save_state), GFP_KERNEL); + if (!drvdata->save_state) + return -ENOMEM; + } + + drvdata->base = base; + + spin_lock_init(&drvdata->spinlock); + + drvdata->cpu = coresight_get_cpu(dev); + if (drvdata->cpu < 0) + return drvdata->cpu; + + init_arg.dev = dev; + init_arg.csa = &access; + init_arg.pid = etm_pid; + + /* + * Serialize against CPUHP callbacks to avoid race condition + * between the smp call and saving the delayed probe. + */ + cpus_read_lock(); + if (smp_call_function_single(drvdata->cpu, + etm4_init_arch_data, &init_arg, 1)) { + /* The CPU was offline, try again once it comes online. */ + delayed = devm_kmalloc(dev, sizeof(*delayed), GFP_KERNEL); + if (!delayed) { + cpus_read_unlock(); + return -ENOMEM; + } + + *delayed = init_arg; + + per_cpu(delayed_probe, drvdata->cpu) = delayed; + + cpus_read_unlock(); + return 0; + } + cpus_read_unlock(); + + return etm4_add_coresight_dev(&init_arg); +} + static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) { void __iomem *base; @@ -2040,6 +2077,35 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) return ret; } +static int etm4_probe_cpu(unsigned int cpu) +{ + int ret; + struct etm4_init_arg init_arg; + struct csdev_access access = { 0 }; + struct etm4_init_arg *iap = *this_cpu_ptr(&delayed_probe); + + if (!iap) + return 0; + + init_arg = *iap; + devm_kfree(init_arg.dev, iap); + *this_cpu_ptr(&delayed_probe) = NULL; + + ret = pm_runtime_resume_and_get(init_arg.dev); + if (ret < 0) { + dev_err(init_arg.dev, "Failed to get PM runtime!\n"); + return 0; + } + + init_arg.csa = &access; + etm4_init_arch_data(&init_arg); + + etm4_add_coresight_dev(&init_arg); + + pm_runtime_put(init_arg.dev); + return 0; +} + static struct amba_cs_uci_id uci_id_etm4[] = { { /* ETMv4 UCI data */ @@ -2054,16 +2120,20 @@ static void clear_etmdrvdata(void *info) int cpu = *(int *)info; etmdrvdata[cpu] = NULL; + per_cpu(delayed_probe, cpu) = NULL; } static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) { - etm_perf_symlink(drvdata->csdev, false); + bool fully_registered; /* * Taking hotplug lock here to avoid racing between etm4_remove_dev() * and CPU hotplug call backs. */ cpus_read_lock(); + + fully_registered = (etmdrvdata[drvdata->cpu] == drvdata); + /* * The readers for etmdrvdata[] are CPU hotplug call backs * and PM notification call backs. Change etmdrvdata[i] on @@ -2071,12 +2141,15 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) * inside one call back function. */ if (smp_call_function_single(drvdata->cpu, clear_etmdrvdata, &drvdata->cpu, 1)) - etmdrvdata[drvdata->cpu] = NULL; + clear_etmdrvdata(&drvdata->cpu); cpus_read_unlock(); - cscfg_unregister_csdev(drvdata->csdev); - coresight_unregister(drvdata->csdev); + if (fully_registered) { + etm_perf_symlink(drvdata->csdev, false); + cscfg_unregister_csdev(drvdata->csdev); + coresight_unregister(drvdata->csdev); + } return 0; } -- 2.17.1

2 years, 7 months

Re: [PATCH v2] coresight: etm4x: Fix crash observed on Qcom ETM parts with 'Low power override'

by Leo Yan

Hi Bhupesh, On Thu, Aug 25, 2022 at 10:52:32AM +0530, Bhupesh Sharma wrote: > Some Qualcomm ETM implementations require skipping powering up > the trace unit, as the ETMs are in the same power domain as > their CPU cores. > > Via commit 5214b563588e ("coresight: etm4x: Add support for > sysreg only devices"), the setting of 'skip_power_up' flag was > moved after the 'etm4_init_arch_data' function is called, whereas > the flag value is itself used inside the function. This causes > a crash when ETM mode 'Low-power state behavior override' is set > on some Qualcomm parts. > > Fix the same. > > Fixes: 5214b563588e ("coresight: etm4x: Add support for sysreg only devices") > Cc: Mike Leach <mike.leach(a)linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> > Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> > Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma(a)linaro.org> > --- > - v1 can be seen here: https://lore.kernel.org/lkml/20220803191236.3037591-1-bhupesh.sharma@linaro… > - Addressed the review comments from Suzuki. > - Rebased on linux-next. > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index d39660a3e50c..14c1c7869795 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -977,6 +977,16 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, > if (!cpu_supports_sysreg_trace()) > return false; > > + /* > + * Some Qualcomm implementations require skipping powering up the trace unit, > + * as the ETMs are in the same power domain as their CPU cores. > + * > + * Since the 'skip_power_up' flag is used inside 'etm4_init_arch_data' function, > + * initialize it before the function is called. > + */ > + if (fwnode_property_present(dev_fwnode(dev), "qcom,skip-power-up")) > + drvdata->skip_power_up = true; > + I personally think this sentence should be placed in the function etm4_probe(), you need to move it just before smp call etm4_init_arch_data(), this can allow DT property "qcom,skip-power-up" to be respected. > /* > * ETMs implementing sysreg access must implement TRCDEVARCH. > */ > @@ -1951,8 +1961,7 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) > return -EINVAL; > > /* TRCPDCR is not accessible with system instructions. */ > - if (!desc.access.io_mem || > - fwnode_property_present(dev_fwnode(dev), "qcom,skip-power-up")) > + if (!desc.access.io_mem) > drvdata->skip_power_up = true; I prefer to move the condition checking for "desc.access.io_mem" to etm4_init_sysreg_access(), this can make sure the flag skip_power_up is set correctly based on property of system register access. A side topic, in the mainline kernel I found the value "desc.access.io_mem" is always zero (see the initialized value in etm4_probe() and etm4_init_sysreg_access()). Should we initialize desc.access.io_mem to true in etm4_probe()? diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index d39660a3e50c..cf2555c50abb 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1939,6 +1939,7 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) if (drvdata->cpu < 0) return drvdata->cpu; + desc.access.io_mem = true; init_arg.drvdata = drvdata; init_arg.csa = &desc.access; init_arg.pid = etm_pid; Thanks, Leo > major = ETM_ARCH_MAJOR_VERSION(drvdata->arch); > -- > 2.35.3 >

2 years, 7 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight