Messages in this thread | | | Date | Mon, 27 Mar 2023 13:43:52 +0800 | Subject | Re: [PATCH v2 1/3] Coresight: Add coresight dummy driver | From | Hao Zhang <> |
| |
Hi Suzuki,
On 3/24/2023 6:44 PM, Suzuki K Poulose wrote: > On 24/03/2023 06:16, Hao Zhang wrote: >> Some Coresight devices that HLOS don't have permission to access >> or configure. Such as Coresight sink EUD, some TPDMs etc. So there >> need driver to register dummy devices as Coresight devices. Provide >> Coresight API for dummy device operations, such as enabling and >> disabling dummy devices. Build the Coresight path for dummy sink or >> dummy source for debugging. >> >> Signed-off-by: Hao Zhang <quic_hazha@quicinc.com> >> --- >> drivers/hwtracing/coresight/Kconfig | 11 ++ >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/coresight-dummy.c | 176 ++++++++++++++++++ >> 3 files changed, 188 insertions(+) >> create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c >> >> diff --git a/drivers/hwtracing/coresight/Kconfig >> b/drivers/hwtracing/coresight/Kconfig >> index 2b5bbfffbc4f..06f0a7594169 100644 >> --- a/drivers/hwtracing/coresight/Kconfig >> +++ b/drivers/hwtracing/coresight/Kconfig >> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA >> To compile this driver as a module, choose M here: the module >> will be >> called coresight-tpda. >> + >> +config CORESIGHT_DUMMY >> + tristate "Dummy driver support" >> + help >> + Enables support for dummy driver. Dummy driver can be used for >> + CoreSight sources/sinks that are owned and configured by some >> + other subsystem and use Linux drivers to configure rest of trace >> + path. >> + >> + To compile this driver as a module, choose M here: the module >> will be >> + called coresight-dummy. >> endif >> diff --git a/drivers/hwtracing/coresight/Makefile >> b/drivers/hwtracing/coresight/Makefile >> index 33bcc3f7b8ae..995d3b2c76df 100644 >> --- a/drivers/hwtracing/coresight/Makefile >> +++ b/drivers/hwtracing/coresight/Makefile >> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o >> coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ >> coresight-cti-sysfs.o >> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o >> +obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o >> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c >> b/drivers/hwtracing/coresight/coresight-dummy.c >> new file mode 100644 >> index 000000000000..2d4eb3e546eb >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/coresight-dummy.c >> @@ -0,0 +1,176 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/coresight.h> >> +#include <linux/of.h> >> +#include <linux/pm_runtime.h> >> + >> +#include "coresight-priv.h" >> +#include "coresight-trace-id.h" >> + >> +struct dummy_drvdata { >> + struct device *dev; >> + struct coresight_device *csdev; >> + int traceid; >> +}; >> + >> +DEFINE_CORESIGHT_DEVLIST(dummy_devs, "dummy"); >> + >> +static int dummy_source_enable(struct coresight_device *csdev, >> + struct perf_event *event, u32 mode) >> +{ >> + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + dev_info(drvdata->dev, "Dummy source enabled\n"); > > Please use dev_dbg everywher.
Sure, I will change to dev_dbg in the next patch series.
> >> + >> + return 0; >> +} >> + >> +static void dummy_source_disable(struct coresight_device *csdev, >> + struct perf_event *event) >> +{ >> + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + dev_info(drvdata->dev, "Dummy source disabled\n"); >> +} >> + >> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode, >> + void *data) >> +{ >> + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + dev_info(drvdata->dev, "Dummy sink enabled\n"); >> + >> + return 0; >> +} >> + >> +static int dummy_sink_disable(struct coresight_device *csdev) >> +{ >> + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + dev_info(drvdata->dev, "Dummy sink disabled\n"); >> + >> + return 0; >> +} >> + >> +static const struct coresight_ops_source dummy_source_ops = { >> + .enable = dummy_source_enable, >> + .disable = dummy_source_disable, >> +}; >> + >> +static const struct coresight_ops_sink dummy_sink_ops = { >> + .enable = dummy_sink_enable, >> + .disable = dummy_sink_disable, >> +}; >> + >> +static const struct coresight_ops dummy_cs_ops = { >> + .source_ops = &dummy_source_ops, >> + .sink_ops = &dummy_sink_ops, >> +}; >> + >> +static int dummy_probe(struct platform_device *pdev) >> +{ >> + int ret, trace_id; >> + struct device *dev = &pdev->dev; >> + struct coresight_platform_data *pdata; >> + struct dummy_drvdata *drvdata; >> + struct coresight_desc desc = { 0 }; >> + >> + desc.name = coresight_alloc_device_name(&dummy_devs, dev); >> + if (!desc.name) >> + return -ENOMEM; >> + >> + pdata = coresight_get_platform_data(dev); >> + if (IS_ERR(pdata)) >> + return PTR_ERR(pdata); >> + pdev->dev.platform_data = pdata; >> + >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + drvdata->dev = &pdev->dev; >> + platform_set_drvdata(pdev, drvdata); >> + >> + if (of_property_read_bool(pdev->dev.of_node, "qcom,dummy-source")) { > > I don't see any reason why this should be qcom,... > > Please could we use : "arm,coresight-", everywhere including the "dt" > compatible ?
It's not only for qcom device, I will update this according to your advice in the next patch series.
>> + desc.type = CORESIGHT_DEV_TYPE_SOURCE; >> + desc.subtype.source_subtype = >> + CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; >> + } else if (of_property_read_bool(pdev->dev.of_node, >> + "qcom,dummy-sink")) { >> + desc.type = CORESIGHT_DEV_TYPE_SINK; >> + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; >> + } else { >> + dev_info(dev, "Device type not set\n"); >> + return -EINVAL; >> + } >> + >> + desc.ops = &dummy_cs_ops; >> + desc.pdata = pdev->dev.platform_data; >> + desc.dev = &pdev->dev; >> + drvdata->csdev = coresight_register(&desc); >> + if (IS_ERR(drvdata->csdev)) >> + return PTR_ERR(drvdata->csdev); >> + >> + trace_id = coresight_trace_id_get_system_id(); >> + if (trace_id < 0) { >> + ret = trace_id; >> + goto cs_unregister; >> + } >> + drvdata->traceid = (u8)trace_id; >> + >> + pm_runtime_enable(dev); >> + dev_info(dev, "Dummy device initialized\n"); >> + >> + return 0; >> + >> +cs_unregister: >> + coresight_unregister(drvdata->csdev); >> + >> + return ret; >> +} >> + >> +static int dummy_remove(struct platform_device *pdev) >> +{ >> + struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); >> + struct device *dev = &pdev->dev; >> + >> + coresight_trace_id_put_system_id(drvdata->traceid); >> + pm_runtime_disable(dev); >> + coresight_unregister(drvdata->csdev); >> + return 0; >> +} >> + >> +static const struct of_device_id dummy_match[] = { >> + {.compatible = "qcom,coresight-dummy"}, > > As mentioned above, "arm,coresight-dummy-device" ? This has > nothing to do with qcom IP. qcom has a use for this. So, I would > like to keep this "coresight" subsystem specific compatibles. > > May be we could even add other types too : i.e, > > arm,coresight-dummy-link-split, arm,coresight-dummy-link-merge > > Suzuki >
I will change this it to "arm,coresight-dummy-device" in the next patch series.
Thanks, Hao
>> + {}, >> +}; >> + >> +static struct platform_driver dummy_driver = { >> + .probe = dummy_probe, >> + .remove = dummy_remove, >> + .driver = { >> + .name = "coresight-dummy", >> + .of_match_table = dummy_match, >> + }, >> +}; >> + >> +static int __init dummy_init(void) >> +{ >> + return platform_driver_register(&dummy_driver); >> +} >> +module_init(dummy_init); >> + >> +static void __exit dummy_exit(void) >> +{ >> + platform_driver_unregister(&dummy_driver); >> +} >> +module_exit(dummy_exit); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_DESCRIPTION("CoreSight dummy source driver"); >
| |