On 31/05/2024 05:27, Linu Cherian wrote:
Introduce a new mode CS_MODE_READ_CRASHDATA for reading trace captured in previous crash/watchdog reset.
Add special device files for reading ETR/ETF crash data.
User can read the crash data as below
For example, for reading crash data from tmc_etf sink
#dd if=/dev/crash_tmc_etfXX of=~/cstrace.bin
Signed-off-by: Anil Kumar Reddy areddy3@marvell.com Signed-off-by: Tanmay Jagdale tanmay@marvell.com Signed-off-by: Linu Cherian lcherian@marvell.com
Changelog from v7:
- Moved crash dev registration into new function, register_crash_dev_interface
- Removed redundant variable trace_addr in tmc_etr_setup_crashdata_buf
.../coresight/coresight-etm4x-core.c | 1 + .../hwtracing/coresight/coresight-tmc-core.c | 148 ++++++++++++++++- .../hwtracing/coresight/coresight-tmc-etf.c | 73 +++++++++ .../hwtracing/coresight/coresight-tmc-etr.c | 152 +++++++++++++++++- drivers/hwtracing/coresight/coresight-tmc.h | 11 +- include/linux/coresight.h | 13 ++ 6 files changed, 391 insertions(+), 7 deletions(-)
[...]
+static void register_crash_dev_interface(struct tmc_drvdata * drvdata,
const char *name)+{
- drvdata->crashdev.name =
devm_kasprintf(&drvdata->csdev->dev, GFP_KERNEL, "%s_%s", "crash", name);- drvdata->crashdev.minor = MISC_DYNAMIC_MINOR;
- drvdata->crashdev.fops = &tmc_crashdata_fops;
- if (misc_register(&drvdata->crashdev))
dev_dbg(&drvdata->csdev->dev,"Failed to setup user interface for crashdata\n");+}
static int tmc_probe(struct amba_device *adev, const struct amba_id *id) { int ret = 0; @@ -619,6 +752,10 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) coresight_unregister(drvdata->csdev); else pm_runtime_put(&adev->dev);
- if (is_tmc_reserved_region_valid(dev))
register_crash_dev_interface(drvdata, desc.name);
Now that you've added an extra step to the probe you need to add a "goto out" when the previous step fails:
if (ret) { coresight_unregister(drvdata->csdev); + goto out; }
That seems to be the pattern in the rest of the function. Otherwise you might register the interface on no device.
There's also a conflict here and in some other places. Since this has to go through the coresight branch you should base it off of coresight-next.
out: return ret; } @@ -630,7 +767,8 @@ static void tmc_shutdown(struct amba_device *adev) spin_lock_irqsave(&drvdata->spinlock, flags);
- if (coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED)
- if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED) ||
(coresight_get_mode(drvdata->csdev) == CS_MODE_READ_CRASHDATA))if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index f9569585e9f8..655c0c0ba54b 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -657,6 +657,56 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev) return 0; }
[...]
@@ -725,6 +789,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata) __tmc_etb_disable_hw(drvdata); } +mode_valid: drvdata->reading = true; out: spin_unlock_irqrestore(&drvdata->spinlock, flags); @@ -744,8 +809,16 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) drvdata->config_type != TMC_CONFIG_TYPE_ETF)) return -EINVAL;
Whitespace change. There are a couple of other minor checkpatch style warnings.
The rest looks good to me. All the tests are passing. I still think we should all the kself test for this mode, but that can be done later.