linux-kernel - Re: [PATCH 2/5] coresight: add coresight Trace NOC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <13cabef8-8e68-412e-a6eb-42d22604f941@quicinc.com>
Date: Wed, 26 Feb 2025 18:44:08 +0800
From: Yuanfang Zhang <quic_yuanfang@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
        Suzuki K Poulose
	<suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        James Clark
	<james.clark@...aro.org>,
        Alexander Shishkin
	<alexander.shishkin@...ux.intel.com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>
CC: <kernel@...cinc.com>, <linux-kernel@...r.kernel.org>,
        <coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
        <kernel@....qualcomm.com>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH 2/5] coresight: add coresight Trace NOC driver



On 2/22/2025 6:54 PM, Krzysztof Kozlowski wrote:
> On 21/02/2025 08:40, Yuanfang Zhang wrote:
>> Add driver to support Coresight device Trace NOC(Network On Chip).
>> Trace NOC is an integration hierarchy which is a replacement of
>> Dragonlink configuration. It brings together debug components like
>> TPDA, funnel and interconnect Trace Noc.
>>
>> It sits in the different subsystem of SOC and aggregates the trace
>> and transports to QDSS trace bus.
>>
>> Signed-off-by: Yuanfang Zhang <quic_yuanfang@...cinc.com>
>> ---
>>  drivers/hwtracing/coresight/Kconfig          |  10 ++
>>  drivers/hwtracing/coresight/Makefile         |   1 +
>>  drivers/hwtracing/coresight/coresight-tnoc.c | 191 +++++++++++++++++++++++++++
>>  drivers/hwtracing/coresight/coresight-tnoc.h |  53 ++++++++
>>  4 files changed, 255 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index 06f0a7594169c5f03ca5f893b7debd294587de78..712b2469e37610e6fc5f15cedb2535bf570f99aa 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -247,4 +247,14 @@ config CORESIGHT_DUMMY
>>  
>>  	  To compile this driver as a module, choose M here: the module will be
>>  	  called coresight-dummy.
>> +
>> +config CORESIGHT_TNOC
>> +	tristate "Coresight Trace Noc driver"
>> +	help
>> +	  This driver provides support for Trace NoC component.
>> +	  Trace NoC is a interconnect that is used to collect trace from
>> +	  various subsystems and transport it QDSS trace sink.It sits in
>> +	  the different tiles of SOC and aggregates the trace local to the
>> +	  tile and transports it another tile or to QDSS trace sink eventually.
>> +
>>  endif
>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>> index 4ba478211b318ea5305f9f98dda40a041759f09f..ab1cff8f027495fabe3872d52f8c0877e39f0ea8 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -51,3 +51,4 @@ 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
>> +obj-$(CONFIG_CORESIGHT_TNOC) += coresight-tnoc.o
> 
> Why do you keep adding entries to the end instead to some logically
> ordered place?
> 
> Dummy driver, before tpda (obviously tpda should go after tpdm) and now
> this... This is just unnecessarily making simultaneous edits difficult.
> 
sure, add it after funnel/replicator before etm, since it work as a link.

>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..11b9a7fd1efdc9fff7c1e9666bda14acb41786cb
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/amba/bus.h>
>> +#include <linux/io.h>
>> +#include <linux/coresight.h>
>> +#include <linux/of.h>
>> +
>> +#include "coresight-priv.h"
>> +#include "coresight-tnoc.h"
>> +#include "coresight-trace-id.h"
>> +
> 
> 
>> +
>> +	drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> +	if (!drvdata->base)
>> +		return -ENOMEM;
>> +
>> +	spin_lock_init(&drvdata->spinlock);
>> +
>> +	ret = trace_noc_init_default_data(drvdata);
>> +	if (ret)
>> +		return ret;
>> +
>> +	desc.ops = &trace_noc_cs_ops;
>> +	desc.type = CORESIGHT_DEV_TYPE_LINK;
>> +	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
>> +	desc.pdata = adev->dev.platform_data;
>> +	desc.dev = &adev->dev;
>> +	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
>> +	drvdata->csdev = coresight_register(&desc);
>> +	if (IS_ERR(drvdata->csdev))
>> +		return PTR_ERR(drvdata->csdev);
>> +
>> +	pm_runtime_put(&adev->dev);
>> +
>> +	dev_dbg(drvdata->dev, "Trace Noc initialized\n");
> 
> 
> Drop. There is really no need to tell that function finished.
> 
> Please run standard kernel tools for static analysis, like coccinelle,
> smatch and sparse, and fix reported warnings. Also please check for
> warnings when building with W=1. Most of these commands (checks or W=1
> build) can build specific targets, like some directory, to narrow the
> scope to only your code. The code here looks like it needs a fix. Feel
> free to get in touch if the warning is not clear.
> 
Done.
> 
> Best regards,
> Krzysztof
>