| Message ID | 20241018032217.39728-4-quic_jinlmao@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/3] dt-bindings: arm: Add arm,static-trace-id for coresight dummy source | expand |
On Mon, 21 Oct 2024 at 13:36, Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > > On 21/10/2024 13:31, Suzuki K Poulose wrote: > > On 18/10/2024 04:22, Mao Jinlong wrote: > >> Some dummy source has static trace id configured in HW and it cannot > >> be changed via software programming. Configure the trace id in device > >> tree and reserve the id when device probe. > >> > >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > >> --- > >> .../sysfs-bus-coresight-devices-dummy-source | 15 +++++ > >> drivers/hwtracing/coresight/coresight-dummy.c | 59 +++++++++++++++++-- > >> 2 files changed, 70 insertions(+), 4 deletions(-) > >> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight- > >> devices-dummy-source > >> > >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- > >> dummy-source b/Documentation/ABI/testing/sysfs-bus-coresight-devices- > >> dummy-source > >> new file mode 100644 > >> index 000000000000..c7d975e75d85 > >> --- /dev/null > >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-dummy-source > >> @@ -0,0 +1,15 @@ > >> +What: /sys/bus/coresight/devices/dummy_source<N>/enable_source > >> +Date: Oct 2024 > >> +KernelVersion: 6.13 > >> +Contact: Mao Jinlong <quic_jinlmao@quicinc.com> > >> +Description: (RW) Enable/disable tracing of dummy source. A sink > >> should be activated > >> + before enabling the source. The path of coresight components > >> linking > >> + the source to the sink is configured and managed > >> automatically by the > >> + coresight framework. > >> + > >> +What: /sys/bus/coresight/devices/dummy_source<N>/traceid > >> +Date: Oct 2024 > >> +KernelVersion: 6.13 > >> +Contact: Mao Jinlong <quic_jinlmao@quicinc.com> > >> +Description: (R) Show the trace ID that will appear in the trace > >> stream > >> + coming from this trace entity. > >> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/ > >> hwtracing/coresight/coresight-dummy.c > >> index bb85fa663ffc..602a7e89e311 100644 > >> --- a/drivers/hwtracing/coresight/coresight-dummy.c > >> +++ b/drivers/hwtracing/coresight/coresight-dummy.c > >> @@ -11,10 +11,12 @@ > >> #include <linux/pm_runtime.h> > >> #include "coresight-priv.h" > >> +#include "coresight-trace-id.h" > >> struct dummy_drvdata { > >> struct device *dev; > >> struct coresight_device *csdev; > >> + u8 traceid; > >> }; > >> DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source"); > >> @@ -72,6 +74,32 @@ static const struct coresight_ops dummy_sink_cs_ops > >> = { > >> .sink_ops = &dummy_sink_ops, > >> }; > >> +/* User can get the trace id of dummy source from this node. */ > >> +static ssize_t traceid_show(struct device *dev, > >> + struct device_attribute *attr, char *buf) > >> +{ > >> + unsigned long val; > >> + struct dummy_drvdata *drvdata = dev_get_drvdata(dev->parent); > >> + > >> + val = drvdata->traceid; > >> + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); sysfs_emit() is the convention moving forwards - this handles all the PAGE_SIZE issues automatically > >> +} > >> +static DEVICE_ATTR_RO(traceid); > >> + > >> +static struct attribute *coresight_dummy_attrs[] = { > >> + &dev_attr_traceid.attr, > >> + NULL, > >> +}; > >> + > >> +static const struct attribute_group coresight_dummy_group = { > >> + .attrs = coresight_dummy_attrs, > >> +}; > >> + > >> +static const struct attribute_group *coresight_dummy_groups[] = { > >> + &coresight_dummy_group, > >> + NULL, > >> +}; > >> + > >> static int dummy_probe(struct platform_device *pdev) > >> { > >> struct device *dev = &pdev->dev; > >> @@ -79,6 +107,11 @@ static int dummy_probe(struct platform_device *pdev) > >> struct coresight_platform_data *pdata; > >> struct dummy_drvdata *drvdata; > >> struct coresight_desc desc = { 0 }; > >> + int ret, trace_id; > >> + > >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > >> + if (!drvdata) > >> + return -ENOMEM; > >> if (of_device_is_compatible(node, "arm,coresight-dummy-source")) { > >> @@ -90,6 +123,25 @@ static int dummy_probe(struct platform_device *pdev) > >> desc.subtype.source_subtype = > >> CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; > >> desc.ops = &dummy_source_cs_ops; > >> + desc.groups = coresight_dummy_groups; > >> + > >> + ret = coresight_get_static_trace_id(dev, &trace_id); > >> + if (!ret) { > >> + /* Get the static id if id is set in device tree. */ > >> + ret = coresight_trace_id_get_static_system_id(trace_id); > > This may be worth an error message, it is a rare one. Othewise, there is > no clue on what caused the failure. Or have a specific error code as a > result ? > The called function does actually emit an error message - but a comment to that effect would clarify this. > >> + if (ret < 0) > >> + return ret; > > e.g., return -EBUSY ? /* Device or resource not available */ > > >> + > >> + } else { > >> + /* Get next available id if id is not set in device tree. */ > >> + trace_id = coresight_trace_id_get_system_id(); > >> + if (trace_id < 0) { > >> + ret = trace_id; > >> + return ret; > >> + } > >> + } > >> + drvdata->traceid = (u8)trace_id; > >> + > >> } else if (of_device_is_compatible(node, "arm,coresight-dummy- > >> sink")) { > >> desc.name = coresight_alloc_device_name(&sink_devs, dev); > >> if (!desc.name) > >> @@ -108,10 +160,6 @@ static int dummy_probe(struct platform_device *pdev) > >> return PTR_ERR(pdata); > >> pdev->dev.platform_data = pdata; > >> - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > >> - if (!drvdata) > >> - return -ENOMEM; > >> - > >> drvdata->dev = &pdev->dev; > >> platform_set_drvdata(pdev, drvdata); > > Additionally we should drop the system_id if registering the coresight > device fails. > > > Suzuki > > >> @@ -131,7 +179,10 @@ static void dummy_remove(struct platform_device > >> *pdev) > >> { > >> struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); > >> struct device *dev = &pdev->dev; > >> + struct device_node *node = dev->of_node; > > > > ^^ Why is this needed ? The rest looks fine to me > > > >> + if (IS_VALID_CS_TRACE_ID(drvdata->traceid)) > >> + coresight_trace_id_put_system_id(drvdata->traceid); > >> pm_runtime_disable(dev); > >> coresight_unregister(drvdata->csdev); > >> } > > > Mike
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-dummy-source b/Documentation/ABI/testing/sysfs-bus-coresight-devices-dummy-source new file mode 100644 index 000000000000..c7d975e75d85 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-dummy-source @@ -0,0 +1,15 @@ +What: /sys/bus/coresight/devices/dummy_source<N>/enable_source +Date: Oct 2024 +KernelVersion: 6.13 +Contact: Mao Jinlong <quic_jinlmao@quicinc.com> +Description: (RW) Enable/disable tracing of dummy source. A sink should be activated + before enabling the source. The path of coresight components linking + the source to the sink is configured and managed automatically by the + coresight framework. + +What: /sys/bus/coresight/devices/dummy_source<N>/traceid +Date: Oct 2024 +KernelVersion: 6.13 +Contact: Mao Jinlong <quic_jinlmao@quicinc.com> +Description: (R) Show the trace ID that will appear in the trace stream + coming from this trace entity. diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c index bb85fa663ffc..602a7e89e311 100644 --- a/drivers/hwtracing/coresight/coresight-dummy.c +++ b/drivers/hwtracing/coresight/coresight-dummy.c @@ -11,10 +11,12 @@ #include <linux/pm_runtime.h> #include "coresight-priv.h" +#include "coresight-trace-id.h" struct dummy_drvdata { struct device *dev; struct coresight_device *csdev; + u8 traceid; }; DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source"); @@ -72,6 +74,32 @@ static const struct coresight_ops dummy_sink_cs_ops = { .sink_ops = &dummy_sink_ops, }; +/* User can get the trace id of dummy source from this node. */ +static ssize_t traceid_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + unsigned long val; + struct dummy_drvdata *drvdata = dev_get_drvdata(dev->parent); + + val = drvdata->traceid; + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); +} +static DEVICE_ATTR_RO(traceid); + +static struct attribute *coresight_dummy_attrs[] = { + &dev_attr_traceid.attr, + NULL, +}; + +static const struct attribute_group coresight_dummy_group = { + .attrs = coresight_dummy_attrs, +}; + +static const struct attribute_group *coresight_dummy_groups[] = { + &coresight_dummy_group, + NULL, +}; + static int dummy_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -79,6 +107,11 @@ static int dummy_probe(struct platform_device *pdev) struct coresight_platform_data *pdata; struct dummy_drvdata *drvdata; struct coresight_desc desc = { 0 }; + int ret, trace_id; + + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; if (of_device_is_compatible(node, "arm,coresight-dummy-source")) { @@ -90,6 +123,25 @@ static int dummy_probe(struct platform_device *pdev) desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; desc.ops = &dummy_source_cs_ops; + desc.groups = coresight_dummy_groups; + + ret = coresight_get_static_trace_id(dev, &trace_id); + if (!ret) { + /* Get the static id if id is set in device tree. */ + ret = coresight_trace_id_get_static_system_id(trace_id); + if (ret < 0) + return ret; + + } else { + /* Get next available id if id is not set in device tree. */ + trace_id = coresight_trace_id_get_system_id(); + if (trace_id < 0) { + ret = trace_id; + return ret; + } + } + drvdata->traceid = (u8)trace_id; + } else if (of_device_is_compatible(node, "arm,coresight-dummy-sink")) { desc.name = coresight_alloc_device_name(&sink_devs, dev); if (!desc.name) @@ -108,10 +160,6 @@ static int dummy_probe(struct platform_device *pdev) return PTR_ERR(pdata); pdev->dev.platform_data = pdata; - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); - if (!drvdata) - return -ENOMEM; - drvdata->dev = &pdev->dev; platform_set_drvdata(pdev, drvdata); @@ -131,7 +179,10 @@ static void dummy_remove(struct platform_device *pdev) { struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); struct device *dev = &pdev->dev; + struct device_node *node = dev->of_node; + if (IS_VALID_CS_TRACE_ID(drvdata->traceid)) + coresight_trace_id_put_system_id(drvdata->traceid); pm_runtime_disable(dev); coresight_unregister(drvdata->csdev); }
Some dummy source has static trace id configured in HW and it cannot be changed via software programming. Configure the trace id in device tree and reserve the id when device probe. Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> --- .../sysfs-bus-coresight-devices-dummy-source | 15 +++++ drivers/hwtracing/coresight/coresight-dummy.c | 59 +++++++++++++++++-- 2 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-dummy-source