Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <0f3ac22f-59c9-6e04-c824-dd486c20bf37@arm.com>
Date:   Mon, 2 Oct 2023 16:03:40 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 2/7] coresight: tmc-etr: Add support to use reserved trace
 memory
Content-Language: en-US
To:     Linu Cherian <lcherian@marvell.com>
Cc:     linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org,
        linux-kernel@vger.kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        devicetree@vger.kernel.org, sgoutham@marvell.com,
        gcherian@marvell.com, Anil Kumar Reddy <areddy3@marvell.com>,
        suzuki.poulose@arm.com, mike.leach@linaro.org, leo.yan@linaro.org
References: <20230929133754.857678-1-lcherian@marvell.com>
 <20230929133754.857678-3-lcherian@marvell.com>
From:   James Clark <james.clark@arm.com>
In-Reply-To: <20230929133754.857678-3-lcherian@marvell.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 29/09/2023 14:37, Linu Cherian wrote:
> Add support to use reserved memory for coresight ETR trace buffer.
> 
> Introduce a new ETR buffer mode called ETR_MODE_RESRV, which
> becomes available when ETR device tree node is supplied with a valid
> reserved memory region.
> 
> ETR_MODE_RESRV can be selected only by explicit user request.
> 
> $ echo resrv >/sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred
> 
> Signed-off-by: Anil Kumar Reddy <areddy3@marvell.com>
> Signed-off-by: Linu Cherian <lcherian@marvell.com>
> ---
>  .../hwtracing/coresight/coresight-tmc-core.c  | 51 +++++++++++
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 87 ++++++++++++++++++-
>  drivers/hwtracing/coresight/coresight-tmc.h   | 24 +++++
>  3 files changed, 161 insertions(+), 1 deletion(-)

[...]

> 
> +static void tmc_get_reserved_region(struct device *parent)
> +{
> +	struct tmc_drvdata *drvdata = dev_get_drvdata(parent);
> +	struct device_node *node;
> +	struct resource res;
> +	int rc;
> +
> +	node = tmc_get_region_byname(parent->of_node, "tracedata-mem");

Is this a typo? The DT commit says the region is called "trace-mem". And
"metadata-mem" for the other region, but that one matches the other call
to tmc_get_region_byname() added in the later commit.

> +	if (IS_ERR_OR_NULL(node)) {
> +		dev_dbg(parent, "No reserved trace buffer specified\n");
> +		goto out;
> +	}
> +