Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Mon, 2 May 2022 10:49:11 -0600
From:   Mathieu Poirier <mathieu.poirier@linaro.org>
To:     German Gomez <german.gomez@arm.com>
Cc:     coresight@lists.linaro.org, suzuki.poulose@arm.com,
        james.clark@arm.com, leo.yan@linaro.org, mike.leach@linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] coresight: etm4x: Expose default timestamp source in
 sysfs
Message-ID: <20220502164911.GB2498226@p14s>
References: <20220429123100.268059-1-german.gomez@arm.com>
 <20220429123100.268059-2-german.gomez@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220429123100.268059-2-german.gomez@arm.com>
Precedence: bulk

On Fri, Apr 29, 2022 at 01:30:59PM +0100, German Gomez wrote:
> Add a new sysfs interface in /sys/bus/coresight/devices/etm<N>/ts_source
> indicating the configured timestamp source when the ETM device driver
> was probed.
> 
> The perf tool will use this information to detect if the trace data
> timestamp matches the kernel time, enabling correlation of CoreSight
> trace with perf events.
> 
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: German Gomez <german.gomez@arm.com>
> ---
>  arch/arm64/include/asm/sysreg.h               |  1 +
>  .../coresight/coresight-etm4x-sysfs.c         | 34 +++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 38508e507d73a..263a7bee06f9a 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1146,6 +1146,7 @@
>  #define SYS_MPIDR_SAFE_VAL	(BIT(31))
>  
>  #define TRFCR_ELx_TS_SHIFT		5
> +#define TRFCR_ELx_TS_MASK		((0x3UL) << TRFCR_ELx_TS_SHIFT)
>  #define TRFCR_ELx_TS_VIRTUAL		((0x1UL) << TRFCR_ELx_TS_SHIFT)
>  #define TRFCR_ELx_TS_GUEST_PHYSICAL	((0x2UL) << TRFCR_ELx_TS_SHIFT)
>  #define TRFCR_ELx_TS_PHYSICAL		((0x3UL) << TRFCR_ELx_TS_SHIFT)
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a0640fa5c55bd..c0c375c0cfde2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2264,6 +2264,39 @@ static ssize_t cpu_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(cpu);
>  
> +static int etmv4_to_ts_source(struct etmv4_drvdata *drvdata)
> +{
> +	int val;
> +
> +	if (!drvdata->trfcr)
> +		return -1;
> +
> +	switch (drvdata->trfcr & TRFCR_ELx_TS_MASK) {
> +	case TRFCR_ELx_TS_VIRTUAL:
> +	case TRFCR_ELx_TS_GUEST_PHYSICAL:
> +	case TRFCR_ELx_TS_PHYSICAL:
> +		val = FIELD_GET(TRFCR_ELx_TS_MASK, drvdata->trfcr);
> +		break;
> +	default:
> +		val = -1;
> +		break;
> +	}

I don't think the above is big and/or complex enough to mandate a separate
function - please move to ts_source_show().

> +
> +	return val;
> +}
> +
> +static ssize_t ts_source_show(struct device *dev,
> +			      struct device_attribute *attr,
> +			      char *buf)
> +{
> +	int val;
> +	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	val = etmv4_to_ts_source(drvdata);
> +	return sysfs_emit(buf, "%d\n", val);
> +}
> +static DEVICE_ATTR_RO(ts_source);
> +
>  static struct attribute *coresight_etmv4_attrs[] = {
>  	&dev_attr_nr_pe_cmp.attr,
>  	&dev_attr_nr_addr_cmp.attr,
> @@ -2318,6 +2351,7 @@ static struct attribute *coresight_etmv4_attrs[] = {
>  	&dev_attr_vmid_val.attr,
>  	&dev_attr_vmid_masks.attr,
>  	&dev_attr_cpu.attr,
> +	&dev_attr_ts_source.attr,
>  	NULL,
>  };
>  
> -- 
> 2.25.1
>