Re: [PATCH v4 11/11] coresight-tpdm: Add nodes for dsb msr support

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 27/04/2023 10:00, Tao Zhang wrote:
> Add the nodes for DSB subunit MSR(mux select register) support.
> The TPDM MSR (mux select register) interface is an optional
> interface and associated bank of registers per TPDM subunit.
> The intent of mux select registers is to control muxing structures
> driving the TPDM’s’ various subunit interfaces.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 15 ++++++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 53 ++++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-tpdm.h       |  3 ++
>   3 files changed, 71 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 639b6fb8..f746f25 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -170,3 +170,18 @@ Description:
>   		Accepts only one of the 2 values -  0 or 1.
>   		0 : Set the DSB pattern type to value.
>   		1 : Set the DSB pattern type to toggle.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_msr
> +Date:		March 2023
> +KernelVersion	6.3
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(Write) Set the MSR(mux select register) of DSB tpdm. Read
> +		the MSR(mux select register) of DSB tpdm.
> +
> +		Expected format is the following:
> +		<integer1> <integer2>
> +
> +		Where:
> +		<integer1> : Index number of MSR register
> +		<integer2> : The value need to be written
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 627de36..5fe0bd5c 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -240,6 +240,14 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
>   			if (!drvdata->dsb)
>   				return -ENOMEM;
>   		}
> +		if (!of_property_read_u32(drvdata->dev->of_node,
> +			   "qcom,dsb_msr_num", &drvdata->dsb->msr_num)) {
> +			drvdata->dsb->msr = devm_kzalloc(drvdata->dev,
> +				   (drvdata->dsb->msr_num * sizeof(*drvdata->dsb->msr)),
> +				   GFP_KERNEL);
> +			if (!drvdata->dsb->msr)
> +				return -ENOMEM;
> +		}
>   	}
>   
>   	return 0;
> @@ -765,6 +773,50 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(dsb_trig_ts);
>   
> +static ssize_t dsb_msr_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned int i;
> +	ssize_t size = 0;
> +
> +	if (drvdata->dsb->msr_num == 0)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {

Shouldn't this be  "i < drvdata->dsb->msr_num" ?


> +		size += sysfs_emit_at(buf, size,
> +				  "%u 0x%x\n", i, drvdata->dsb->msr[i]);
> +	}
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;
> +}
> +
> +static ssize_t dsb_msr_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf,
> +				  size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned int num, val;
> +	int nval;
> +
> +	if (drvdata->dsb->msr_num == 0)
> +		return -EINVAL;
> +
> +	nval = sscanf(buf, "%u %x", &num, &val);
> +	if ((nval != 2) || (num >= (drvdata->dsb->msr_num - 1)))

(num >= drvdata->dsb->msr_num) ?

> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->dsb->msr[num] = val;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_msr);
> +
>   static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_mode.attr,
>   	&dev_attr_dsb_edge_ctrl.attr,
> @@ -777,6 +829,7 @@ static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_trig_patt_mask.attr,
>   	&dev_attr_dsb_trig_ts.attr,
>   	&dev_attr_dsb_trig_type.attr,
> +	&dev_attr_dsb_msr.attr,
>   	NULL,
>   };
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index 9ad32a6..05e9f8e 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -18,6 +18,7 @@
>   #define TPDM_DSB_XPMR(n)	(0x7E8 + (n * 4))
>   #define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
>   #define TPDM_DSB_EDCMR(n)	(0x848 + (n * 4))
> +#define TPDM_DSB_MSR(n)		(0x980 + (n * 4))
>   
>   /* Enable bit for DSB subunit */
>   #define TPDM_DSB_CR_ENA		BIT(0)
> @@ -113,6 +114,8 @@ struct dsb_dataset {
>   	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
>   	bool			trig_ts;
>   	bool			trig_type;
> +	u32				msr_num;
> +	u32				*msr;
>   };
>   
>   /**


Where/when do we write to these registers in the DSB ?

Suzuki