[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK9=C2Vpd8EajVXdnwMjSjSFkt4OFkn8OT4XHr-Nfs6ipjynpg@mail.gmail.com>
Date: Thu, 30 Oct 2025 14:07:01 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Bo Gan <ganboing@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Greg KH <gregkh@...uxfoundation.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Ian Rogers <irogers@...gle.com>,
Mark Rutland <mark.rutland@....com>, devicetree@...r.kernel.org,
Alexandre Ghiti <alex@...ti.fr>, Atish Patra <atish.patra@...ux.dev>,
Peter Zijlstra <peterz@...radead.org>, Anup Patel <anup@...infault.org>,
Adrian Hunter <adrian.hunter@...el.com>, linux-kernel@...r.kernel.org,
Mayuresh Chitale <mchitale@...tanamicro.com>, Ingo Molnar <mingo@...hat.com>,
Jiri Olsa <jolsa@...nel.org>, Mayuresh Chitale <mchitale@...il.com>,
Namhyung Kim <namhyung@...nel.org>, linux-riscv@...ts.infradead.org,
Andrew Jones <ajones@...tanamicro.com>, Liang Kan <kan.liang@...ux.intel.com>
Subject: Re: [PATCH 02/11] rvtrace: Initial implementation of driver framework
On Sat, Oct 11, 2025 at 6:33 AM Bo Gan <ganboing@...il.com> wrote:
>
> On 10/1/25 23:07, Anup Patel wrote:
> > The RISC-V Trace Control Interface Specification [1] defines a standard
> > way of implementing RISC-V trace related modular components irrespective
> > to underlying trace format (E-trace or N-trace). These RISC-V trace
> > components are organized in a graph-like topology where each RISC-V
> > hart has its own RISC-V trace encoder component.
> >
> > Implement a basic driver framework for RISC-V trace where RISC-V trace
> > components are instantiated by a common platform driver and a separate
> > RISC-V trace driver for each type of RISC-V trace component.
> >
> > [1] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf
> >
> > Co-developed-by: Mayuresh Chitale <mchitale@...tanamicro.com>
> > Signed-off-by: Mayuresh Chitale <mchitale@...tanamicro.com>
> > Signed-off-by: Anup Patel <apatel@...tanamicro.com>
> > ---
> > drivers/Makefile | 1 +
> > drivers/hwtracing/Kconfig | 2 +
> > drivers/hwtracing/rvtrace/Kconfig | 16 +
> > drivers/hwtracing/rvtrace/Makefile | 4 +
> > drivers/hwtracing/rvtrace/rvtrace-core.c | 484 +++++++++++++++++++
> > drivers/hwtracing/rvtrace/rvtrace-platform.c | 174 +++++++
> > include/linux/rvtrace.h | 272 +++++++++++
> > 7 files changed, 953 insertions(+)
> > create mode 100644 drivers/hwtracing/rvtrace/Kconfig
> > create mode 100644 drivers/hwtracing/rvtrace/Makefile
> > create mode 100644 drivers/hwtracing/rvtrace/rvtrace-core.c
> > create mode 100644 drivers/hwtracing/rvtrace/rvtrace-platform.c
> > create mode 100644 include/linux/rvtrace.h
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index b5749cf67044..466a55580f60 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -178,6 +178,7 @@ obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> > obj-y += hwtracing/intel_th/
> > obj-$(CONFIG_STM) += hwtracing/stm/
> > obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
> > +obj-$(CONFIG_RVTRACE) += hwtracing/rvtrace/
> > obj-y += android/
> > obj-$(CONFIG_NVMEM) += nvmem/
> > obj-$(CONFIG_FPGA) += fpga/
> > diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
> > index 911ee977103c..daeb38fe332d 100644
> > --- a/drivers/hwtracing/Kconfig
> > +++ b/drivers/hwtracing/Kconfig
> > @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
> >
> > source "drivers/hwtracing/ptt/Kconfig"
> >
> > +source "drivers/hwtracing/rvtrace/Kconfig"
> > +
> > endmenu
> > diff --git a/drivers/hwtracing/rvtrace/Kconfig b/drivers/hwtracing/rvtrace/Kconfig
> > new file mode 100644
> > index 000000000000..f8f6feea1953
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/Kconfig
> > @@ -0,0 +1,16 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +menuconfig RVTRACE
> > + tristate "RISC-V Trace Support"
> > + depends on RISCV
> > + depends on OF
> > + default RISCV
> > + help
> > + This framework provides a kernel interface for the RISC-V trace
> > + drivers (including both e-trace and n-trace). It's intended to
> > + build a topological view of the RISC-V trace components and
> > + configure the right series of components when trace is enabled
> > + on a CPU.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called rvtrace.
> > diff --git a/drivers/hwtracing/rvtrace/Makefile b/drivers/hwtracing/rvtrace/Makefile
> > new file mode 100644
> > index 000000000000..988525a379cf
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/Makefile
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_RVTRACE) += rvtrace.o
> > +rvtrace-y := rvtrace-core.o rvtrace-platform.o
> > diff --git a/drivers/hwtracing/rvtrace/rvtrace-core.c b/drivers/hwtracing/rvtrace/rvtrace-core.c
> > new file mode 100644
> > index 000000000000..52ea931745fc
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/rvtrace-core.c
> > @@ -0,0 +1,484 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2025 Ventana Micro Systems Inc.
> > + */
> > +
> > +#include <linux/cpumask.h>
> > +#include <linux/delay.h>
> > +#include <linux/export.h>
> > +#include <linux/idr.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/percpu.h>
> > +#include <linux/rvtrace.h>
> > +
> > +/* Mutex to serialize component registration/unregistration */
> > +static DEFINE_MUTEX(rvtrace_mutex);
> > +
> > +/* Per-CPU encoder instances */
> > +static DEFINE_PER_CPU(struct rvtrace_component *, rvtrace_cpu_encoder);
> > +
> > +/* Component type based id generator */
> > +struct rvtrace_type_idx {
> > + /* Lock to protect the type ID generator */
> > + struct mutex lock;
> > + struct idr idr;
> > +};
> > +
> > +/* Array of component type based id generator */
> > +static struct rvtrace_type_idx rvtrace_type_idx_array[RVTRACE_COMPONENT_TYPE_MAX];
> > +
> > +static int rvtrace_alloc_type_idx(struct rvtrace_component *comp)
> > +{
> > + struct rvtrace_type_idx *rvidx = &rvtrace_type_idx_array[comp->id.type];
> > + int idx;
> > +
> > + mutex_lock(&rvidx->lock);
> > + idx = idr_alloc(&rvidx->idr, comp, 0, 0, GFP_KERNEL);
> > + mutex_unlock(&rvidx->lock);
> > + if (idx < 0)
> > + return idx;
> > +
> > + comp->type_idx = idx;
> > + return 0;
> > +}
> > +
> > +static void rvtrace_free_type_idx(struct rvtrace_component *comp)
> > +{
> > + struct rvtrace_type_idx *rvidx = &rvtrace_type_idx_array[comp->id.type];
> > +
> > + mutex_lock(&rvidx->lock);
> > + idr_remove(&rvidx->idr, comp->type_idx);
> > + mutex_unlock(&rvidx->lock);
> > +}
> > +
> > +static void __init rvtrace_init_type_idx(void)
> > +{
> > + struct rvtrace_type_idx *rvidx;
> > + int i;
> > +
> > + for (i = 0; i < RVTRACE_COMPONENT_TYPE_MAX; i++) {
> > + rvidx = &rvtrace_type_idx_array[i];
> > + mutex_init(&rvidx->lock);
> > + idr_init(&rvidx->idr);
> > + }
> > +}
> > +
> > +const struct rvtrace_component_id *rvtrace_match_id(struct rvtrace_component *comp,
> > + const struct rvtrace_component_id *ids)
> > +{
> > + const struct rvtrace_component_id *id;
> > +
> > + for (id = ids; id->version && id->type; id++) {
> > + if (comp->id.type == id->type &&
> > + comp->id.version == id->version)
> > + return id;
> > + }
> > +
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_match_id);
> > +
> > +static int rvtrace_match_device(struct device *dev, const struct device_driver *drv)
> > +{
> > + const struct rvtrace_driver *rtdrv = to_rvtrace_driver(drv);
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > +
> > + return rvtrace_match_id(comp, rtdrv->id_table) ? 1 : 0;
> > +}
> > +
> > +static int rvtrace_probe(struct device *dev)
> > +{
> > + const struct rvtrace_driver *rtdrv = to_rvtrace_driver(dev->driver);
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > + int ret = -ENODEV;
> > +
> > + if (!rtdrv->probe)
> > + return ret;
> > +
> > + ret = rtdrv->probe(comp);
> > + if (!ret)
> > + comp->ready = true;
> > +
> > + return ret;
> > +}
> > +
> > +static void rvtrace_remove(struct device *dev)
> > +{
> > + const struct rvtrace_driver *rtdrv = to_rvtrace_driver(dev->driver);
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > +
> > + comp->ready = false;
> > + if (rtdrv->remove)
> > + rtdrv->remove(comp);
> > +}
> > +
> > +const struct bus_type rvtrace_bustype = {
> > + .name = "rvtrace",
> > + .match = rvtrace_match_device,
> > + .probe = rvtrace_probe,
> > + .remove = rvtrace_remove,
> > +};
> > +
> > +struct rvtrace_fwnode_match_data {
> > + struct fwnode_handle *fwnode;
> > + struct rvtrace_component *match;
> > +};
> > +
> > +static int rvtrace_match_fwnode(struct device *dev, void *data)
> > +{
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > + struct rvtrace_fwnode_match_data *d = data;
> > +
> > + if (device_match_fwnode(&comp->dev, d->fwnode)) {
> > + d->match = comp;
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +struct rvtrace_component *rvtrace_find_by_fwnode(struct fwnode_handle *fwnode)
> > +{
> > + struct rvtrace_fwnode_match_data d = { .fwnode = fwnode, .match = NULL };
> > + int ret;
> > +
> > + ret = bus_for_each_dev(&rvtrace_bustype, NULL, &d, rvtrace_match_fwnode);
> > + if (ret < 0)
> > + return ERR_PTR(ret);
> > +
> > + return d.match;
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_find_by_fwnode);
> > +
> > +int rvtrace_poll_bit(struct rvtrace_platform_data *pdata, int offset,
> > + int bit, int bitval, int timeout)
> > +{
> > + int i = 10;
> > + u32 val;
> > +
> > + while (i--) {
> > + val = rvtrace_read32(pdata, offset);
> > + if (((val >> bit) & 0x1) == bitval)
> > + break;
> > + udelay(timeout);
> > + }
> > +
> > + return (i < 0) ? -ETIMEDOUT : 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_poll_bit);
> > +
> > +int rvtrace_enable_component(struct rvtrace_component *comp)
> > +{
> > + u32 val;
> > +
> > + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> > + val |= BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
> > + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> > + return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> > + RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 1,
> > + comp->pdata->control_poll_timeout_usecs);
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_enable_component);
> > +
> > +int rvtrace_disable_component(struct rvtrace_component *comp)
> > +{
> > + u32 val;
> > +
> > + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> > + val &= ~BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
> > + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> > + return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> > + RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 0,
> > + comp->pdata->control_poll_timeout_usecs);
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_disable_component);
> > +
> > +struct rvtrace_component *rvtrace_cpu_source(unsigned int cpu)
> > +{
> > + if (!cpu_present(cpu))
> > + return NULL;
> > +
> > + return per_cpu(rvtrace_cpu_encoder, cpu);
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_cpu_source);
> > +
> > +static int rvtrace_cleanup_inconn(struct device *dev, void *data)
> > +{
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > + struct rvtrace_platform_data *pdata = comp->pdata;
> > + struct rvtrace_connection *conn = data;
> > + int i;
> > +
> > + if (device_match_fwnode(&comp->dev, conn->dest_fwnode)) {
> > + for (i = 0; i < pdata->nr_inconns; i++) {
> > + if (pdata->inconns[i] != conn)
> > + continue;
> > + pdata->inconns[i] = NULL;
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void rvtrace_cleanup_inconns_from_outconns(struct rvtrace_component *comp)
> > +{
> > + struct rvtrace_platform_data *pdata = comp->pdata;
> > + struct rvtrace_connection *conn;
> > + int i;
> > +
> > + lockdep_assert_held(&rvtrace_mutex);
> > +
> > + for (i = 0; i < pdata->nr_outconns; i++) {
> > + conn = pdata->outconns[i];
> > + bus_for_each_dev(&rvtrace_bustype, NULL, conn, rvtrace_cleanup_inconn);
> > + }
> > +}
> > +
> > +static int rvtrace_setup_inconn(struct device *dev, void *data)
> > +{
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > + struct rvtrace_platform_data *pdata = comp->pdata;
> > + struct rvtrace_connection *conn = data;
> > + int i;
> > +
> > + if (device_match_fwnode(&comp->dev, conn->dest_fwnode)) {
> > + for (i = 0; i < pdata->nr_inconns; i++) {
> > + if (pdata->inconns[i])
> > + continue;
> > + pdata->inconns[i] = conn;
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rvtrace_setup_inconns_from_outconns(struct rvtrace_component *comp)
> > +{
> > + struct rvtrace_platform_data *pdata = comp->pdata;
> > + struct rvtrace_connection *conn;
> > + int i, ret;
> > +
> > + lockdep_assert_held(&rvtrace_mutex);
> > +
> > + for (i = 0; i < pdata->nr_outconns; i++) {
> > + conn = pdata->outconns[i];
> > + ret = bus_for_each_dev(&rvtrace_bustype, NULL, conn, rvtrace_setup_inconn);
> > + if (ret < 0) {
> > + rvtrace_cleanup_inconns_from_outconns(comp);
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void rvtrace_component_release(struct device *dev)
> > +{
> > + struct rvtrace_component *comp = to_rvtrace_component(dev);
> > +
> > + fwnode_handle_put(comp->dev.fwnode);
> > + rvtrace_free_type_idx(comp);
> > + kfree(comp);
> > +}
> > +
> > +static int rvtrace_component_reset(struct rvtrace_platform_data *pdata)
> > +{
> > + int ret;
> > +
> > + rvtrace_write32(pdata, 0, RVTRACE_COMPONENT_CTRL_OFFSET);
> > + ret = rvtrace_poll_bit(pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> > + RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT, 0,
> > + pdata->control_poll_timeout_usecs);
> > + if (ret)
> > + return ret;
> > +
> > + rvtrace_write32(pdata, RVTRACE_COMPONENT_CTRL_ACTIVE_MASK,
> > + RVTRACE_COMPONENT_CTRL_OFFSET);
> > + return rvtrace_poll_bit(pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> > + RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT, 1,
> > + pdata->control_poll_timeout_usecs);
> > +}
> > +
> > +struct rvtrace_component *rvtrace_register_component(struct rvtrace_platform_data *pdata)
> > +{
> > + struct rvtrace_connection *conn;
> > + struct rvtrace_component *comp;
> > + u32 impl, type, major, minor;
> > + int i, ret = 0;
> > +
> > + if (!pdata || !pdata->dev) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> > +
> > + for (i = 0; i < pdata->nr_inconns; i++) {
> > + if (pdata->inconns[i]) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> > + }
> > +
> > + for (i = 0; i < pdata->nr_outconns; i++) {
> > + conn = pdata->outconns[i];
> > + if (!conn || conn->src_port < 0 || conn->src_comp ||
> > + !device_match_fwnode(pdata->dev, conn->src_fwnode) ||
> > + conn->dest_port < 0 || !conn->dest_fwnode || !conn->dest_comp) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> > + }
> > +
> > + ret = rvtrace_component_reset(pdata);
> > + if (ret)
> > + goto err_out;
> > +
> > + impl = rvtrace_read32(pdata, RVTRACE_COMPONENT_IMPL_OFFSET);
> > + type = (impl >> RVTRACE_COMPONENT_IMPL_TYPE_SHIFT) &
> > + RVTRACE_COMPONENT_IMPL_TYPE_MASK;
> > + major = (impl >> RVTRACE_COMPONENT_IMPL_VERMAJOR_SHIFT) &
> > + RVTRACE_COMPONENT_IMPL_VERMAJOR_MASK;
> > + minor = (impl >> RVTRACE_COMPONENT_IMPL_VERMINOR_SHIFT) &
> > + RVTRACE_COMPONENT_IMPL_VERMINOR_MASK;
> > +
> > + if (pdata->bound_cpu >= 0 && !cpu_present(pdata->bound_cpu)) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> > + if (type == RVTRACE_COMPONENT_TYPE_ENCODER && pdata->bound_cpu < 0) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
> > +
> > + comp = kzalloc(sizeof(*comp), GFP_KERNEL);
> > + if (!comp) {
> > + ret = -ENOMEM;
> > + goto err_out;
> > + }
> > + comp->pdata = pdata;
> > + comp->id.type = type;
> > + comp->id.version = rvtrace_component_mkversion(major, minor);
> > + ret = rvtrace_alloc_type_idx(comp);
> > + if (ret) {
> > + kfree(comp);
> > + goto err_out;
> > + }
> > +
> > + comp->dev.parent = pdata->dev;
> > + comp->dev.coherent_dma_mask = pdata->dev->coherent_dma_mask;
> > + comp->dev.release = rvtrace_component_release;
> > + comp->dev.bus = &rvtrace_bustype;
> > + comp->dev.fwnode = fwnode_handle_get(dev_fwnode(pdata->dev));
> > + switch (comp->id.type) {
> > + case RVTRACE_COMPONENT_TYPE_ENCODER:
> > + dev_set_name(&comp->dev, "encoder-%d", comp->type_idx);
> > + break;
> > + case RVTRACE_COMPONENT_TYPE_FUNNEL:
> > + dev_set_name(&comp->dev, "funnel-%d", comp->type_idx);
> > + break;
> > + case RVTRACE_COMPONENT_TYPE_RAMSINK:
> > + dev_set_name(&comp->dev, "ramsink-%d", comp->type_idx);
> > + break;
> > + case RVTRACE_COMPONENT_TYPE_PIBSINK:
> > + dev_set_name(&comp->dev, "pibsink-%d", comp->type_idx);
> > + break;
> > + case RVTRACE_COMPONENT_TYPE_ATBBRIDGE:
> > + dev_set_name(&comp->dev, "atbbridge-%d", comp->type_idx);
> > + break;
> > + default:
> > + dev_set_name(&comp->dev, "type%d-%d", comp->id.type, comp->type_idx);
> > + break;
> > + }
> > +
> > + mutex_lock(&rvtrace_mutex);
> > +
> > + ret = device_register(&comp->dev);
> > + if (ret) {
> > + put_device(&comp->dev);
> > + goto err_out_unlock;
> > + }
> > +
> > + for (i = 0; i < pdata->nr_outconns; i++) {
> > + conn = pdata->outconns[i];
> > + conn->src_comp = comp;
> > + }
> > +
> > + ret = rvtrace_setup_inconns_from_outconns(comp);
> > + if (ret < 0) {
> > + device_unregister(&comp->dev);
> > + goto err_out_unlock;
> > + }
> > +
> > + if (comp->id.type == RVTRACE_COMPONENT_TYPE_ENCODER) {
> > + rvtrace_get_component(comp);
> > + per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu) = comp;
> > + }
> > +
> > + mutex_unlock(&rvtrace_mutex);
> > +
> > + return comp;
> > +
> > +err_out_unlock:
> > + mutex_unlock(&rvtrace_mutex);
> > +err_out:
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_register_component);
> > +
> > +void rvtrace_unregister_component(struct rvtrace_component *comp)
> > +{
> > + struct rvtrace_component *c;
> > +
> > + mutex_lock(&rvtrace_mutex);
> > +
> > + if (comp->id.type == RVTRACE_COMPONENT_TYPE_ENCODER) {
> > + c = per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu);
> > + per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu) = NULL;
> > + rvtrace_put_component(c);
> > + }
> > +
> > + rvtrace_cleanup_inconns_from_outconns(comp);
> > + device_unregister(&comp->dev);
> > +
> > + mutex_unlock(&rvtrace_mutex);
> > +}
> > +EXPORT_SYMBOL_GPL(rvtrace_unregister_component);
> > +
> > +int __rvtrace_register_driver(struct module *owner, struct rvtrace_driver *rtdrv)
> > +{
> > + rtdrv->driver.owner = owner;
> > + rtdrv->driver.bus = &rvtrace_bustype;
> > +
> > + return driver_register(&rtdrv->driver);
> > +}
> > +EXPORT_SYMBOL_GPL(__rvtrace_register_driver);
> > +
> > +static int __init rvtrace_init(void)
> > +{
> > + int ret;
> > +
> > + rvtrace_init_type_idx();
> > +
> > + ret = bus_register(&rvtrace_bustype);
> > + if (ret)
> > + return ret;
> > +
> > + ret = platform_driver_register(&rvtrace_platform_driver);
> > + if (ret) {
> > + bus_unregister(&rvtrace_bustype);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit rvtrace_exit(void)
> > +{
> > + platform_driver_unregister(&rvtrace_platform_driver);
> > + bus_unregister(&rvtrace_bustype);
> > +}
> > +
> > +module_init(rvtrace_init);
> > +module_exit(rvtrace_exit);
> > diff --git a/drivers/hwtracing/rvtrace/rvtrace-platform.c b/drivers/hwtracing/rvtrace/rvtrace-platform.c
> > new file mode 100644
> > index 000000000000..a110ff1f2f08
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/rvtrace-platform.c
> > @@ -0,0 +1,174 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2025 Ventana Micro Systems Inc.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/rvtrace.h>
> > +#include <linux/types.h>
> > +
> > +static int rvtrace_of_parse_outconns(struct rvtrace_platform_data *pdata)
> > +{
> > + struct device_node *parent, *ep_node, *rep_node, *rdev_node;
> > + struct rvtrace_connection *conn;
> > + struct of_endpoint ep, rep;
> > + int ret = 0, i = 0;
> > +
> > + parent = of_get_child_by_name(dev_of_node(pdata->dev), "out-ports");
> > + if (!parent)
> > + return 0;
> > +
> > + pdata->nr_outconns = of_graph_get_endpoint_count(parent);
> > + pdata->outconns = devm_kcalloc(pdata->dev, pdata->nr_outconns,
> > + sizeof(*pdata->outconns), GFP_KERNEL);
> > + if (!pdata->outconns) {
> > + ret = -ENOMEM;
> > + goto done;
> > + }
> > +
> > + for_each_endpoint_of_node(parent, ep_node) {
> > + conn = devm_kzalloc(pdata->dev, sizeof(*conn), GFP_KERNEL);
> > + if (!conn) {
> > + of_node_put(ep_node);
> > + ret = -ENOMEM;
> > + break;
> > + }
> > +
> > + ret = of_graph_parse_endpoint(ep_node, &ep);
> > + if (ret) {
> > + of_node_put(ep_node);
> > + break;
> > + }
> > +
> > + rep_node = of_graph_get_remote_endpoint(ep_node);
> > + if (!rep_node) {
> > + ret = -ENODEV;
> > + of_node_put(ep_node);
> > + break;
> > + }
> > + rdev_node = of_graph_get_port_parent(rep_node);
> > +
> > + ret = of_graph_parse_endpoint(rep_node, &rep);
> > + if (ret) {
> > + of_node_put(ep_node);
> > + break;
> > + }
> > +
> > + conn->src_port = ep.port;
> > + conn->src_fwnode = dev_fwnode(pdata->dev);
> > + /* The 'src_comp' is set by rvtrace_register_component() */
> > + conn->src_comp = NULL;
> > + conn->dest_port = rep.port;
> > + conn->dest_fwnode = of_fwnode_handle(rdev_node);
>
> Don't you need the fwnode_handle_get the dest_fwnode to hold the reference
> to the sink like coresight does?
>
> https://elixir.bootlin.com/linux/v6.17.1/source/drivers/hwtracing/coresight/coresight-platform.c#L243
Okay, I will update.
>
> > + conn->dest_comp = rvtrace_find_by_fwnode(conn->dest_fwnode);
> > + if (!conn->dest_comp) {
> > + ret = -EPROBE_DEFER;
> > + of_node_put(ep_node);
>
> Missing a break; here.
>
> I finally figured out the hang issue on my p550. It's caused by this missing
> "break;". It caused the "ep_node" to get of_node_put twice. Please check.
Good catch. Thanks for reporting.
>
> > + }
> > +
> > + pdata->outconns[i] = conn;
> > + i++;
> > + }
> > +
> > +done:
> > + of_node_put(parent);
> > + return ret;
> > +}
> > +
> > +static int rvtrace_of_parse_inconns(struct rvtrace_platform_data *pdata)
> > +{
> > + struct device_node *parent;
> > + int ret = 0;
> > +
> > + parent = of_get_child_by_name(dev_of_node(pdata->dev), "in-ports");
> > + if (!parent)
> > + return 0;
> > +
> > + pdata->nr_inconns = of_graph_get_endpoint_count(parent);
> > + pdata->inconns = devm_kcalloc(pdata->dev, pdata->nr_inconns,
> > + sizeof(*pdata->inconns), GFP_KERNEL);
> > + if (!pdata->inconns)
> > + ret = -ENOMEM;
> > +
> > + of_node_put(parent);
> > + return ret;
> > +}
> > +
> > +static int rvtrace_platform_probe(struct platform_device *pdev)
> > +{
> > + struct rvtrace_platform_data *pdata;
> > + struct device *dev = &pdev->dev;
> > + struct rvtrace_component *comp;
> > + struct device_node *node;
> > + struct resource *res;
> > + int ret;
> > +
> > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > + if (!pdata)
> > + return -ENOMEM;
> > + pdata->dev = dev;
> > + pdata->impid = RVTRACE_COMPONENT_IMPID_UNKNOWN;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res)
> > + return -EINVAL;
> > +
> > + pdata->io_mem = true;
> > + pdata->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> > + if (!pdata->base)
> > + return dev_err_probe(dev, -ENOMEM, "failed to ioremap %pR\n", res);
> > +
> > + pdata->bound_cpu = -1;
> > + node = of_parse_phandle(dev_of_node(dev), "cpu", 0);
> > + if (node) {
> > + ret = of_cpu_node_to_id(node);
> > + of_node_put(node);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "failed to get CPU id for %pOF\n", node);
> > + pdata->bound_cpu = ret;
> > + }
> > +
> > + /* Default control poll timeout */
> > + pdata->control_poll_timeout_usecs = 10;
> > +
> > + ret = rvtrace_of_parse_outconns(pdata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to parse output connections\n");
> > +
> > + ret = rvtrace_of_parse_inconns(pdata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to parse input connections\n");
> > +
> > + comp = rvtrace_register_component(pdata);
> > + if (IS_ERR(comp))
> > + return PTR_ERR(comp);
> > +
> > + platform_set_drvdata(pdev, comp);
> > + return 0;
> > +}
> > +
> > +static void rvtrace_platform_remove(struct platform_device *pdev)
> > +{
> > + struct rvtrace_component *comp = platform_get_drvdata(pdev);
> > +
> > + rvtrace_unregister_component(comp);> +}
> > +
> > +static const struct of_device_id rvtrace_platform_match[] = {
> > + { .compatible = "riscv,trace-component" },
> > + {}
> > +};
> > +
> > +struct platform_driver rvtrace_platform_driver = {
> > + .driver = {
> > + .name = "rvtrace",
> > + .of_match_table = rvtrace_platform_match,
> > + },
> > + .probe = rvtrace_platform_probe,
> > + .remove = rvtrace_platform_remove,
> > +};
> > diff --git a/include/linux/rvtrace.h b/include/linux/rvtrace.h
> > new file mode 100644
> > index 000000000000..04eb03e62601
> > --- /dev/null
> > +++ b/include/linux/rvtrace.h
> > @@ -0,0 +1,272 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2025 Ventana Micro Systems Inc.
> > + */
> > +
> > +#ifndef __LINUX_RVTRACE_H__
> > +#define __LINUX_RVTRACE_H__
> > +
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/types.h>
> > +
> > +/* Control register common across all RISC-V trace components */
> > +#define RVTRACE_COMPONENT_CTRL_OFFSET 0x000
> > +#define RVTRACE_COMPONENT_CTRL_ACTIVE_MASK 0x1
> > +#define RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT 0
> > +#define RVTRACE_COMPONENT_CTRL_ENABLE_MASK 0x1
> > +#define RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT 1
> > +
> > +/* Implementation register common across all RISC-V trace components */
> > +#define RVTRACE_COMPONENT_IMPL_OFFSET 0x004
> > +#define RVTRACE_COMPONENT_IMPL_VERMAJOR_MASK 0xf
> > +#define RVTRACE_COMPONENT_IMPL_VERMAJOR_SHIFT 0
> > +#define RVTRACE_COMPONENT_IMPL_VERMINOR_MASK 0xf
> > +#define RVTRACE_COMPONENT_IMPL_VERMINOR_SHIFT 4
> > +#define RVTRACE_COMPONENT_IMPL_TYPE_MASK 0xf
> > +#define RVTRACE_COMPONENT_IMPL_TYPE_SHIFT 8
> > +
> > +/* Possible component types defined by the RISC-V Trace Control Interface */
> > +enum rvtrace_component_type {
> > + RVTRACE_COMPONENT_TYPE_RESV0,
> > + RVTRACE_COMPONENT_TYPE_ENCODER, /* 0x1 */
> > + RVTRACE_COMPONENT_TYPE_RESV2,
> > + RVTRACE_COMPONENT_TYPE_RESV3,
> > + RVTRACE_COMPONENT_TYPE_RESV4,
> > + RVTRACE_COMPONENT_TYPE_RESV5,
> > + RVTRACE_COMPONENT_TYPE_RESV6,
> > + RVTRACE_COMPONENT_TYPE_RESV7,
> > + RVTRACE_COMPONENT_TYPE_FUNNEL, /* 0x8 */
> > + RVTRACE_COMPONENT_TYPE_RAMSINK, /* 0x9 */
> > + RVTRACE_COMPONENT_TYPE_PIBSINK, /* 0xA */
> > + RVTRACE_COMPONENT_TYPE_RESV11,
> > + RVTRACE_COMPONENT_TYPE_RESV12,
> > + RVTRACE_COMPONENT_TYPE_RESV13,
> > + RVTRACE_COMPONENT_TYPE_ATBBRIDGE, /* 0xE */
> > + RVTRACE_COMPONENT_TYPE_RESV15,
> > + RVTRACE_COMPONENT_TYPE_MAX
> > +};
> > +
> > +/* Encoding/decoding macros for RISC-V trace component version */
> > +#define rvtrace_component_version_major(__version) \
> > + (((__version) >> 16) & 0xffff)
> > +#define rvtrace_component_version_minor(__version) \
> > + ((__version) & 0xffff)
> > +#define rvtrace_component_mkversion(__major, __minor) \
> > + ((((__major) & 0xffff) << 16) | ((__minor) & 0xffff))
> > +
> > +/*
> > + * Possible component implementation IDs discovered from DT or ACPI
> > + * shared across the RISC-V trace drivers to infer trace parameters,
> > + * quirks, and work-arounds. These component implementation IDs are
> > + * internal to Linux and must not be exposed to user-space.
> > + *
> > + * The component implementation ID should be named as follows:
> > + * RVTRACE_COMPONENT_IMPID_<vendor>_<part>
> > + */
> > +enum rvtrace_component_impid {
> > + RVTRACE_COMPONENT_IMPID_UNKNOWN,
> > + RVTRACE_COMPONENT_IMPID_MAX
> > +};
> > +
> > +/**
> > + * struct rvtrace_connection - Representation of a physical connection between
> > + * two RISC-V trace components.
> > + * @src_port: A connection's source port number.
> > + * @src_fwnode: Source component's fwnode handle..
> > + * @src_comp: Source component's pointer.
> > + * @dest_port: A connection's destination port number.
> > + * @dest_fwnode: Destination component's fwnode handle.
> > + * @dest_comp: Destination component's pointer.
> > + */
> > +struct rvtrace_connection {
> > + int src_port;
> > + struct fwnode_handle *src_fwnode;
> > + int dest_port;
> > + struct fwnode_handle *dest_fwnode;
> > + struct rvtrace_component *src_comp;
> > + struct rvtrace_component *dest_comp;
> > +};
> > +
> > +/**
> > + * struct rvtrace_platform_data - Platform-level data for a RISC-V trace component
> > + * discovered from DT or ACPI.
> > + * @dev: Parent device.
> > + * @impid: Component implementation ID
> > + * @io_mem: Flag showing whether component registers are memory mapped.
> > + * @base: If io_mem == true then base address of the memory mapped registers.
> > + * @read: If io_mem == false then read register from the given "offset".
> > + * @write: If io_mem == false then write register to the given "offset".
> > + * @bound_cpu: CPU to which the component is bound. This should be -1 if
> > + * the component is not bound to any CPU. For encoder component
> > + * type this must not be -1.
> > + * @nr_inconns: Number of input connections.
> > + * @inconns: Array of pointers to input connections.
> > + * @nr_outconns: Number of output connections.
> > + * @outconns: Array of pointers to output connections.
> > + */
> > +struct rvtrace_platform_data {
> > + struct device *dev;
> > +
> > + enum rvtrace_component_impid impid;
> > +
> > + bool io_mem;
> > + union {
> > + void __iomem *base;
> > + struct {
> > + u32 (*read)(struct rvtrace_platform_data *pdata,
> > + u32 offset, bool relaxed);
> > + void (*write)(struct rvtrace_platform_data *pdata,
> > + u32 val, u32 offset, bool relaxed);
> > + };
> > + };
> > +
> > + int bound_cpu;
> > +
> > + /* Delay in microseconds when polling control register bits */
> > + int control_poll_timeout_usecs;
> > +
> > + /*
> > + * Platform driver must only populate empty pointer array without
> > + * any actual input connections.
> > + */
> > + unsigned int nr_inconns;
> > + struct rvtrace_connection **inconns;
> > +
> > + /*
> > + * Platform driver must fully populate pointer array with individual
> > + * array elements pointing to actual output connections. The src_comp
> > + * of each output connection is automatically updated at the time of
> > + * registering component.
> > + */
> > + unsigned int nr_outconns;
> > + struct rvtrace_connection **outconns;
> > +};
> > +
> > +static inline u32 rvtrace_read32(struct rvtrace_platform_data *pdata, u32 offset)
> > +{
> > + if (likely(pdata->io_mem))
> > + return readl(pdata->base + offset);
> > +
> > + return pdata->read(pdata, offset, false);
> > +}
> > +
> > +static inline u32 rvtrace_relaxed_read32(struct rvtrace_platform_data *pdata, u32 offset)
> > +{
> > + if (likely(pdata->io_mem))
> > + return readl_relaxed(pdata->base + offset);
> > +
> > + return pdata->read(pdata, offset, true);
> > +}
> > +
> > +static inline void rvtrace_write32(struct rvtrace_platform_data *pdata, u32 val, u32 offset)
> > +{
> > + if (likely(pdata->io_mem))
> > + writel(val, pdata->base + offset);
> > + else
> > + pdata->write(pdata, val, offset, false);
> > +}
> > +
> > +static inline void rvtrace_relaxed_write32(struct rvtrace_platform_data *pdata,
> > + u32 val, u32 offset)
> > +{
> > + if (likely(pdata->io_mem))
> > + writel_relaxed(val, pdata->base + offset);
> > + else
> > + pdata->write(pdata, val, offset, true);
> > +}
> > +
> > +static inline bool rvtrace_is_source(struct rvtrace_platform_data *pdata)
> > +{
> > + return !pdata->nr_inconns ? true : false;
> > +}
> > +
> > +static inline bool rvtrace_is_sink(struct rvtrace_platform_data *pdata)
> > +{
> > + return !pdata->nr_outconns ? true : false;
> > +}
> > +
> > +/**
> > + * struct rvtrace_component_id - Details to identify or match a RISC-V trace component
> > + * @type: Type of the component
> > + * @version: Version of the component
> > + * @data: Data pointer for driver use
> > + */
> > +struct rvtrace_component_id {
> > + enum rvtrace_component_type type;
> > + u32 version;
> > + void *data;
> > +};
> > +
> > +/**
> > + * struct rvtrace_component - Representation of a RISC-V trace component
> > + * pdata: Pointer to underlying platform data
> > + * id: Details to match the component
> > + * type_idx: Unique number based on component type
> > + * dev: Device instance
> > + * ready: Flag showing whether RISC-V trace driver was probed successfully
> > + */
> > +struct rvtrace_component {
> > + struct rvtrace_platform_data *pdata;
> > + struct rvtrace_component_id id;
> > + u32 type_idx;
> > + struct device dev;
> > + bool ready;
> > +};
> > +
> > +#define to_rvtrace_component(__dev) container_of_const(__dev, struct rvtrace_component, dev)
> > +
> > +static inline void rvtrace_get_component(struct rvtrace_component *comp)
> > +{
> > + get_device(&comp->dev);
> > +}
> > +
> > +static inline void rvtrace_put_component(struct rvtrace_component *comp)
> > +{
> > + put_device(&comp->dev);
> > +}
> > +
> > +const struct rvtrace_component_id *rvtrace_match_id(struct rvtrace_component *comp,
> > + const struct rvtrace_component_id *ids);
> > +struct rvtrace_component *rvtrace_find_by_fwnode(struct fwnode_handle *fwnode);
> > +
> > +int rvtrace_poll_bit(struct rvtrace_platform_data *pdata, int offset,
> > + int bit, int bitval, int timeout);
> > +int rvtrace_enable_component(struct rvtrace_component *comp);
> > +int rvtrace_disable_component(struct rvtrace_component *comp);
> > +
> > +struct rvtrace_component *rvtrace_cpu_source(unsigned int cpu);
> > +
> > +struct rvtrace_component *rvtrace_register_component(struct rvtrace_platform_data *pdata);
> > +void rvtrace_unregister_component(struct rvtrace_component *comp);
> > +
> > +/**
> > + * struct rvtrace_driver - Representation of a RISC-V trace driver
> > + * id_table: Table to match components handled by the driver
> > + * probe: Driver probe() function
> > + * remove: Driver remove() function
> > + * driver: Device driver instance
> > + */
> > +struct rvtrace_driver {
> > + const struct rvtrace_component_id *id_table;
> > + int (*probe)(struct rvtrace_component *comp);
> > + void (*remove)(struct rvtrace_component *comp);
> > + struct device_driver driver;
> > +};
> > +
> > +#define to_rvtrace_driver(__drv) \
> > + ((__drv) ? container_of_const((__drv), struct rvtrace_driver, driver) : NULL)
> > +
> > +extern struct platform_driver rvtrace_platform_driver;
> > +
> > +int __rvtrace_register_driver(struct module *owner, struct rvtrace_driver *rtdrv);
> > +#define rvtrace_register_driver(driver) __rvtrace_register_driver(THIS_MODULE, driver)
> > +static inline void rvtrace_unregister_driver(struct rvtrace_driver *rtdrv)
> > +{
> > + if (rtdrv)
> > + driver_unregister(&rtdrv->driver);
> > +}
> > +
> > +#endif
> I suggest stress-testing it by creating some negative cases such as having
> dangling endpoint links in DT, e.g., RAM sinks disabled, but source encoder
> enabled, and check if the driver can properly handle it. Also because there
> can be 3 modules rvtrace/rvtrace-encoder/rvtrace-ramsink (and probably more
> for funnel later), check different module load orders and other edge cases
> such as only loading rvtrace+encoder and see if the driver can safely handle
> it without crash. In addition, try to load/unload the driver in a loop and
> see if resources are free'ed.
Testing infrastructure will develop over-time and there are a lot of topology
configurations to cover.
The current goal is to add a reasonably flexible rvtrace framework which
can be extended and hardened over time.
Regards,
Anup
Powered by blists - more mailing lists