[PATCH v8 00/13] Add support to configure TPDM DSB subunit

[PATCH v8 00/13] Add support to configure TPDM DSB subunit

[Tao Zhang]

Introduction of TPDM DSB subunit
DSB subunit is responsible for creating a dataset element, and is also
optionally responsible for packing it to fit multiple elements on a
single ATB transfer if possible in the configuration. The TPDM Core
Datapath requests timestamps be stored by the TPDA and then delivering
ATB sized data (depending on ATB width and element size, this could
be smaller or larger than a dataset element) to the ATB Mast FSM.

The DSB subunit must be configured prior to enablement. This series
adds support for TPDM to configure the configure DSB subunit.

Once this series patches are applied properly, the new tpdm nodes for
should be observed at the tpdm path /sys/bus/coresight/devices/tpdm*
which supports DSB subunit.
e.g.
root@qemuarm64:/sys/devices/platform/soc@0/6c08000.tpdm/tpdm1# ls -l
drwxr-xr-x    2 root     root             0 Jan  1 00:00 connections
drwxr-xr-x    2 root     root             0 Jan  1 00:00 dsb_edge
-rw-r--r--    1 root     root          4096 Jan  1 00:00 dsb_mode
drwxr-xr-x    2 root     root             0 Jan  1 00:00 dsb_msr
drwxr-xr-x    2 root     root             0 Jan  1 00:00 dsb_patt
-rw-r--r--    1 root     root          4096 Jan  1 00:00 dsb_patt_ts
-rw-r--r--    1 root     root          4096 Jan  1 00:00 dsb_patt_type
drwxr-xr-x    2 root     root             0 Jan  1 00:00 dsb_trig_patt
-rw-r--r--    1 root     root          4096 Jan  1 00:00 dsb_trig_ts
-rw-r--r--    1 root     root          4096 Jan  1 00:00 dsb_trig_type
-rw-r--r--    1 root     root          4096 Jan  1 00:02 enable_source
--w-------    1 root     root          4096 Jan  1 00:00 integration_test
drwxr-xr-x    2 root     root             0 Jan  1 00:00 power
--w-------    1 root     root          4096 Jan  1 00:02 reset_dataset
lrwxrwxrwx    1 root     root             0 Apr  5  2021 subsystem -> ../../../../../bus/coresight
-rw-r--r--    1 root     root          4096 Apr  5  2021 uevent
-r--r--r--    1 root     root          4096 Jan  1 00:00 waiting_for_supplier

We can use the commands are similar to the below to configure the
TPDMs which support DSB subunit. Enable coresight sink first.
echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
echo 1 > /sys/bus/coresight/devices/tpdm1/reset_dataset
echo 0x3 > /sys/bus/coresight/devices/tpdm1/dsb_edge/ctrl_idx
echo 0x1 > /sys/bus/coresight/devices/tpdm1/dsb_edge/ctrl_mask
echo 0x0 > /sys/bus/coresight/devices/tpdm1/dsb_edge/ctrl_val
echo 1 > /sys/bus/coresight/devices/tpdm1/dsb_patt_ts
echo 1 > /sys/bus/coresight/devices/tpdm1/dsb_patt_type
echo 0 > /sys/bus/coresight/devices/tpdm1/dsb_trig_ts
echo 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm1/dsb_patt/tpmr5
echo 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm1/dsb_trig_patt/xpr2
echo 1 > /sys/bus/coresight/devices/tpdm1/enable_source

TPDM_DSB commit tree:
https://git.codelinaro.org/clo/linux-kernel/coresight/-/tree/tpdm-dsb-v8
https://git.codelinaro.org/clo/linux-kernel/coresight/-/commits/tpdm-dsb-v8

Changes in V8:
1. Refine the function "tpda_set_element_size" and rename it
to "tpda_get_element_size" in the patch#4.
-- Suzuki K Poulose
2. Refine the functioin "tpda_enable_port" in the patch#4.
-- Suzuki K Poulose
3. Write a helper to check if the TPDM has DSB dataset in the
patch#5.
-- Suzuki K Poulose
4. Move the function "tpdm_reset_datasets" to "datasets_setup"
to call in the patch#5.
-- Suzuki K Poulose
5. Refine the comment of DSB in "tpdm_drvdata" in the patch#5.
-- Suzuki K Poulose
6. Refine the comments in the documents for this patch series.
-- Suzuki K Poulose
7. Adjust the code alignment in this patch series.
-- Suzuki K Poulose
8. Combine the mode related functions to one in the patch#8.
-- Suzuki K Poulose
9. Refine the R/W functions of "dsb_mode" in the patch#8.
-- Suzuki K Poulose
10. Adjust the macros of mode in the TPDM header file in the
patch#8.
-- Suzuki K Poulose
11. Remove the unused code and fix the warnings in compiling
for the patch#9.
-- kernel test robot
12. Use the following sysfs nodes to read/set edge control
related value in the patch#9.
dsb_edge/
	\- ctrl_idx		-> Set the index number
	\- ctrl_val		-> Set the edge control value
	\- ctrl_mask	-> Set the edge control mask
	\- edcr0 ... edcr15		-> Read the edge control value
	\- edcmr0 ... edcmr7	-> Read the edge control mask
-- Suzuki K Poulose
13. Use the following sysfs nodes to read/set DSB trigger
pattern value and mask in the patch#10.
dsb_trig_patt/
	\- xpr0 ... xpr15		-> (RW) Set/Get the value
	\- xpmr0 ... xpmr7		-> (RW) Set/Get the mask
-- Suzuki K Poulose
14. Use the following sysfs nodes to read/set DSB pattern
value and mask in the patch#11.
dsb_patt/
	\- tpr0 ... tpr15		-> (RW) Set/Get the value
	\- tpmr0 ... tpmr7		-> (RW) Set/Get the mask
-- Suzuki K Poulose
15. Add "Acked-by" tag to the patch#12.
-- Rob Herring
16. Use the following sysfs nodes to read/set DSB MSR in
the patch#13.
dsb_msr/
	\- msr0 ... msr31		-> (RW) Set/Get the value
-- Suzuki K Poulose
17. Create the maximal number of DSB MSR sysfs nodes if the
TPDM supports DSB MSR. Write the values set by user space to
the DSB MSR according to the number of MSR supported by the
TPDM.
-- Suzuki K Poulose

Changes in V7:
1. Since the "One value" limitation on SysFs file usage, add
the nodes to read/write the index number for configuring the
DSB TPDM. The following index number nodes are added.
"dsb_edge_ctrl_idx" in the patch #9
"dsb_trig_patt_idx" in the patch #10
"dsb_patt_idx" in the patch #11
"dsb_msr_idx" in the patch #13
-- Suzuki K Poulose

Changes in V6:
1. Align the code to fix the styling issue.
-- Suzuki K Poulose

Changes in V5:
1. Correct data type for DSB element size in dt-bindings patch.
2. Refine the recursive function "tpda_set_element_size".
-- Suzuki K Poulose
3. Get return value of the function "__tpda_enable" in
"tpda_enable".
-- Suzuki K Poulose
4. Refine the comments on "dsb_esize".
-- Suzuki K Poulose
5. Split the chage that introduce the subtype
"SUBTYPE_SOURCE_TPDM" to Coresight driver.
-- Suzuki K Poulose
6. Inline the trigger type setting to "tpdm_enable_dsb" simply.
-- Suzuki K Poulose
7. Split the change that remove the needless CS_{UN,}LOCK in
the function "tpdm_datasets_setup".
-- Suzuki K Poulose
8. Remove the disablement step in the reset node.
-- Suzuki K Poulose
9. Update the kernel version to 6.5 in the sysfs document.
-- Suzuki K Poulose
10. Remove the needless check in "tpdm_dsb_is_visible".
-- Suzuki K Poulose
11. Change the macro to mask the mode of DSB TPDM.
-- Suzuki K Poulose
12. Add a check to make sure "sysfs_emit_at" calling will not
cause overflow.
-- Suzuki K Poulose
13. Change the macro to get "edge_ctrl" value.
-- Suzuki K Poulose
14. Remove the needless comments in the sysfs document.
-- Suzuki K Poulose
15. Replace "TPDM_DSB_MAX_PATT" with "drvdata->dsb->msr_num" in
"dsb_msr_show".
-- Suzuki K Poulose
16. Update the check of MSR number in "dsb_msr_store".
-- Suzuki K Poulose
17. Write data to the MSR registers in the DSB TPDM enablement
function.
-- Suzuki K Poulose

Changes in V4:
1. Change the range of the property "qcom,dsb-element-size", and
change the type to enumeration.
-- Suzuki K Poulose, Krzysztof Kozlowski
2. Change dsb_esize from 32 bits to 8 bits.
-- Suzuki K Poulose
3. Update the function tpda_set_element_size since James has
updated the dependency series. Meanwhile, it will send out a
warning if it detects more than one TPDM from the same TPDA
input port.
-- Suzuki K Poulose
4. Add a source_sub_type for TPDM to distinguish TPDM from
the other coresight source.
-- Suzuki K Poulose
5. Return error if the element size is not configured on
devicetree in TPDA enablement.
-- Suzuki K Poulose
6. Move memory allocation from "tpdm_init_datasets" to
"tpdm_datasets_setup". Rename "tpdm_init_datasets" as
"tpdm_reset_datasets".
-- Suzuki K Poulose
7. Replace "coresight_disable" with "coresight_disable_source"
to disable the TPDM in resetting.
-- Suzuki K Poulose
8. Make sure "drvdata" is not NULL pointer before using it.
-- Suzuki K Poulose
9. Change "set_dsb_cycacc_mode" to "set_dsb_test_mode" since
cycle accurate mode is not supported on the current targets.
It is replaced by test mode.
10. Document the value of "dsb_mode".
-- Suzuki K Poulose
11. Macros are used to replace the formulas on dsb edge control
nodes.
-- Suzuki K Poulose
12. Document the values of "dsb_trig_patt_val" and
"dsb_trig_patt_mask".
-- Suzuki K Poulose
13. Combine two pattern related loops to one. And move DSB TIER
register configurations to the new function "set_dsb_tier".
-- Suzuki K Poulose
14. Rename the property "qcom,dsb_msr_num" to "qcom,dsb-msrs-num".
-- Suzuki K Poulose, Krzysztof Kozlowski

Changes in V3:
1. Move the property "qcom,dsb-element-size" to TPDM
devicetree and update the TPDM yaml file for this item.
-- Suzuki K Poulose
2. Add the error message when the DSB element size is not set to
32-bit or 64-bit. -- Suzuki K Poulose
3. Add more information to the comments of patch #3
-- Suzuki K Poulose
4. Combine the value updates to the TPDM_DSB_CR for TPDM.
-- Suzuki K Poulose
5. Remove the function "tpdm_datasets_alloc", and fold its code
to a new function "tpdm_init_datasets". It will complete the
initialization of TPDM.  -- Suzuki K Poulose
6. Change the method of qualifying input values.
-- Suzuki K Poulose
7. Add the documentation of the new sysfs handles.
-- Suzuki K Poulose
8. Provide the separate handles for the "mode bits".
-- Suzuki K Poulose

Changes in V2:
1. Change the name of the property "qcom,dsb-elem-size" to
"qcom,dsb-element-size" -- Suzuki K Poulose
2. Update the TPDA yaml file for the item "qcom,dsb-elem-size".
-- Krzysztof Kozlowski
3. Add the full name of DSB in the description of the item
"qcom,dsb-elem-size". -- Rob Herring

Changes in V1:
1. Change the definition of the property "qcom,dsb-elem-size" from
"uint32-array" to "uint32-matrix". -- Krzysztof Kozlowski
2. Add the full name of DSB. -- Rob Herring
3. Deal with 2 entries in an iteration in TPDA driver. -- Suzuki K Poulose
4. Divide the function "tpdm_datasets_alloc" into two functions,
"tpdm_datasets_setup" and "tpdm_datasets_alloc".
5. Detecte the input string with the conventional semantics automatically,
and constrain the size of the input value. -- Suzuki K Poulose
6. Use the hook function "is_visible()" to hide the DSB related knobs if
the data sets are missing. -- Suzuki K Poulose
7. Use the macros "FIELD_GET" and "FIELD_PREP" to set the values.
-- Suzuki K Poulose
8. Update the definition of the macros in TPDM driver.
9. Update the comments of the values for the nodes which are for DSB
element creation and onfigure pattern match output. -- Suzuki K Poulose
10. Use API "sysfs_emit" to "replace scnprintf". -- Suzuki K Poulose

Tao Zhang (13):
  coresight-tpdm: Remove the unnecessary lock
  dt-bindings: arm: Add support for DSB element size
  coresight-tpdm: Introduce TPDM subtype to TPDM driver
  coresight-tpda: Add DSB dataset support
  coresight-tpdm: Initialize DSB subunit configuration
  coresight-tpdm: Add reset node to TPDM node
  coresight-tpdm: Add nodes to set trigger timestamp and type
  coresight-tpdm: Add node to set dsb programming mode
  coresight-tpdm: Add nodes for dsb edge control
  coresight-tpdm: Add nodes to configure pattern match output
  coresight-tpdm: Add nodes for timestamp request
  dt-bindings: arm: Add support for DSB MSR register
  coresight-tpdm: Add nodes for dsb msr support

 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 159 +++++
 .../bindings/arm/qcom,coresight-tpdm.yaml          |  20 +
 drivers/hwtracing/coresight/coresight-core.c       |   3 +
 drivers/hwtracing/coresight/coresight-tpda.c       | 126 +++-
 drivers/hwtracing/coresight/coresight-tpda.h       |   2 +
 drivers/hwtracing/coresight/coresight-tpdm.c       | 682 ++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tpdm.h       | 165 +++++
 include/linux/coresight.h                          |   1 +
 8 files changed, 1136 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH v8 01/13] coresight-tpdm: Remove the unnecessary lock

[Tao Zhang]

Remove the unnecessary lock "CS_{UN,}LOCK" in TPDM driver. This
lock is only needed while writing the data to Coresight registers.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-tpdm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index f4854af..b645612 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -114,11 +114,9 @@ static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
 {
 	u32 pidr;
 
-	CS_UNLOCK(drvdata->base);
 	/*  Get the datasets present on the TPDM. */
 	pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
 	drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
-	CS_LOCK(drvdata->base);
 }
 
 /*
-- 
2.7.4

[PATCH v8 02/13] dt-bindings: arm: Add support for DSB element size

[Tao Zhang]

Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
Bit) element for TPDM. The associated aggregator will read this
size before it is enabled. DSB element size currently only
supports 32-bit and 64-bit.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
index 5c08342..931ee8f 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
@@ -44,6 +44,14 @@ properties:
     minItems: 1
     maxItems: 2
 
+  qcom,dsb-element-size:
+    description:
+      Specifies the DSB(Discrete Single Bit) element size supported by
+      the monitor. The associated aggregator will read this size before it
+      is enabled. DSB element size currently only supports 32-bit and 64-bit.
+    $ref: /schemas/types.yaml#/definitions/uint8
+    enum: [32, 64]
+
   clocks:
     maxItems: 1
 
@@ -77,6 +85,8 @@ examples:
       compatible = "qcom,coresight-tpdm", "arm,primecell";
       reg = <0x0684c000 0x1000>;
 
+      qcom,dsb-element-size = /bits/ 8 <32>;
+
       clocks = <&aoss_qmp>;
       clock-names = "apb_pclk";
 
-- 
2.7.4

[PATCH v8 03/13] coresight-tpdm: Introduce TPDM subtype to TPDM driver

[Tao Zhang]

Introduce the new subtype of "CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM"
for TPDM components in driver.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 3 +++
 drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
 include/linux/coresight.h                    | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 118fcf2..661a5ea 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1093,6 +1093,7 @@ static int coresight_validate_source(struct coresight_device *csdev,
 
 	if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC &&
 	    subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE &&
+	    subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM &&
 	    subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS) {
 		dev_err(&csdev->dev, "wrong device subtype in %s\n", function);
 		return -EINVAL;
@@ -1162,6 +1163,7 @@ int coresight_enable(struct coresight_device *csdev)
 		per_cpu(tracer_path, cpu) = path;
 		break;
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
+	case CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM:
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
 		/*
 		 * Use the hash of source's device name as ID
@@ -1212,6 +1214,7 @@ void coresight_disable(struct coresight_device *csdev)
 		per_cpu(tracer_path, cpu) = NULL;
 		break;
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE:
+	case CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM:
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS:
 		hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev)));
 		/* Find the path by the hash. */
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index b645612..abaff0b 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -203,7 +203,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 	if (!desc.name)
 		return -ENOMEM;
 	desc.type = CORESIGHT_DEV_TYPE_SOURCE;
-	desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
+	desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM;
 	desc.ops = &tpdm_cs_ops;
 	desc.pdata = adev->dev.platform_data;
 	desc.dev = &adev->dev;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index bf70987..dc19a44 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -62,6 +62,7 @@ enum coresight_dev_subtype_source {
 	CORESIGHT_DEV_SUBTYPE_SOURCE_PROC,
 	CORESIGHT_DEV_SUBTYPE_SOURCE_BUS,
 	CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE,
+	CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM,
 	CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS,
 };
 
-- 
2.7.4

[PATCH v8 04/13] coresight-tpda: Add DSB dataset support

[Tao Zhang]

Read the DSB element size from the device tree. Set the register
bit that controls the DSB element size of the corresponding port.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-tpda.c | 126 ++++++++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-tpda.h |   2 +
 2 files changed, 118 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 8d2b9d2..0f21cd1 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -21,6 +21,80 @@
 
 DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
 
+static bool coresight_device_is_tpdm(struct coresight_device *csdev)
+{
+	return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
+			(csdev->subtype.source_subtype ==
+			CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
+}
+
+/*
+ * Read the DSB element size from the TPDM device
+ * Returns
+ *    The dsb element size read from the devicetree if available.
+ *    0 - Otherwise, with a warning once.
+ */
+static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
+{
+	int rc = 0;
+	u8 size = 0;
+
+	rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
+			"qcom,dsb-element-size", &size);
+	if (rc)
+		dev_warn_once(&csdev->dev,
+			"Failed to read TPDM DSB Element size: %d\n", rc);
+
+	return size;
+}
+
+/*
+ * Search and read element data size from the TPDM node in
+ * the devicetree. Each input port of TPDA is connected to
+ * a TPDM. Different TPDM supports different types of dataset,
+ * and some may support more than one type of dataset.
+ * Parameter "inport" is used to pass in the input port number
+ * of TPDA, and it is set to -1 in the recursize call.
+ */
+static int tpda_get_element_size(struct coresight_device *csdev,
+								 int inport)
+{
+	int dsb_size = -ENOENT;
+	int i, size;
+	struct coresight_device *in;
+
+	for (i = 0; i < csdev->pdata->nr_inconns; i++) {
+		in = csdev->pdata->in_conns[i]->src_dev;
+		if (!in)
+			continue;
+
+		/* Ignore the paths that do not match port */
+		if (inport > 0 &&
+			(csdev->pdata->in_conns[i]->dest_port != inport))
+			continue;
+
+		if (coresight_device_is_tpdm(in)) {
+			size = tpdm_read_dsb_element_size(in);
+		} else {
+			/* Recurse down the path */
+			size = tpda_get_element_size(in, -1);
+		}
+
+		if (size < 0)
+			return size;
+
+		if (dsb_size < 0) {
+			/* Found a size, save it. */
+			dsb_size = size;
+		} else {
+			/* Found duplicate TPDMs */
+			return -EEXIST;
+		}
+	}
+
+	return dsb_size;
+}
+
 /* Settings pre enabling port control register */
 static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
 {
@@ -32,26 +106,55 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
 	writel_relaxed(val, drvdata->base + TPDA_CR);
 }
 
-static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
+static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
 {
 	u32 val;
+	int size;
 
 	val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
+	/*
+	 * Configure aggregator port n DSB data set element size
+	 * Set the bit to 0 if the size is 32
+	 * Set the bit to 1 if the size is 64
+	 */
+	size = tpda_get_element_size(drvdata->csdev, port);
+	switch (size) {
+	case 32:
+		val &= ~TPDA_Pn_CR_DSBSIZE;
+		break;
+	case 64:
+		val |= TPDA_Pn_CR_DSBSIZE;
+		break;
+	case 0:
+		return -EEXIST;
+	case -EEXIST:
+		dev_warn_once(&drvdata->csdev->dev,
+			"Detected multiple TPDMs on port %d", -EEXIST);
+		return -EEXIST;
+	default:
+		return -EINVAL;
+	}
+
 	/* Enable the port */
 	val |= TPDA_Pn_CR_ENA;
 	writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
+
+	return 0;
 }
 
-static void __tpda_enable(struct tpda_drvdata *drvdata, int port)
+static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
 {
+	int ret;
+
 	CS_UNLOCK(drvdata->base);
 
 	if (!drvdata->csdev->enable)
 		tpda_enable_pre_port(drvdata);
 
-	tpda_enable_port(drvdata, port);
-
+	ret = tpda_enable_port(drvdata, port);
 	CS_LOCK(drvdata->base);
+
+	return ret;
 }
 
 static int tpda_enable(struct coresight_device *csdev,
@@ -59,16 +162,19 @@ static int tpda_enable(struct coresight_device *csdev,
 		       struct coresight_connection *out)
 {
 	struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+	int ret;
 
 	spin_lock(&drvdata->spinlock);
-	if (atomic_read(&in->dest_refcnt) == 0)
-		__tpda_enable(drvdata, in->dest_port);
+	if (atomic_read(&in->dest_refcnt) == 0) {
+		ret = __tpda_enable(drvdata, in->dest_port);
+		if (!ret) {
+			atomic_inc(&in->dest_refcnt);
+			dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
+		}
+	}
 
-	atomic_inc(&in->dest_refcnt);
 	spin_unlock(&drvdata->spinlock);
-
-	dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
-	return 0;
+	return ret;
 }
 
 static void __tpda_disable(struct tpda_drvdata *drvdata, int port)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
index 0399678..b3b38fd 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.h
+++ b/drivers/hwtracing/coresight/coresight-tpda.h
@@ -10,6 +10,8 @@
 #define TPDA_Pn_CR(n)		(0x004 + (n * 4))
 /* Aggregator port enable bit */
 #define TPDA_Pn_CR_ENA		BIT(0)
+/* Aggregator port DSB data set element size bit */
+#define TPDA_Pn_CR_DSBSIZE		BIT(8)
 
 #define TPDA_MAX_INPORTS	32
 
-- 
2.7.4

[PATCH v8 05/13] coresight-tpdm: Initialize DSB subunit configuration

[Tao Zhang]

DSB is used for monitoring “events”. Events are something that
occurs at some point in time. It could be a state decode, the
act of writing/reading a particular address, a FIFO being empty,
etc. This decoding of the event desired is done outside TPDM.
DSB subunit need to be configured in enablement and disablement.
A struct that specifics associated to dsb dataset is needed. It
saves the configuration and parameters of the dsb datasets. This
change is to add this struct and initialize the configuration of
DSB subunit.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-tpdm.c | 64 ++++++++++++++++++++++++----
 drivers/hwtracing/coresight/coresight-tpdm.h | 18 ++++++++
 2 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index abaff0b..951ad4d 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -20,23 +20,57 @@
 
 DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
 
+static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
+{
+	return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
+}
+
+static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
+{
+	if (tpdm_has_dsb_dataset(drvdata)) {
+		memset(drvdata->dsb, 0, sizeof(struct dsb_dataset));
+
+		drvdata->dsb->trig_ts = true;
+		drvdata->dsb->trig_type = false;
+	}
+}
+
 static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 {
 	u32 val;
 
-	/* Set the enable bit of DSB control register to 1 */
+	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
+	/* Set trigger timestamp */
+	if (drvdata->dsb->trig_ts)
+		val |= TPDM_DSB_TIER_XTRIG_TSENAB;
+	else
+		val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
+	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
+
 	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
+	/* Set trigger type */
+	if (drvdata->dsb->trig_type)
+		val |= TPDM_DSB_CR_TRIG_TYPE;
+	else
+		val &= ~TPDM_DSB_CR_TRIG_TYPE;
+	/* Set the enable bit of DSB control register to 1 */
 	val |= TPDM_DSB_CR_ENA;
 	writel_relaxed(val, drvdata->base + TPDM_DSB_CR);
 }
 
-/* TPDM enable operations */
+/*
+ * TPDM enable operations
+ * The TPDM or Monitor serves as data collection component for various
+ * dataset types. It covers Basic Counts(BC), Tenure Counts(TC),
+ * Continuous Multi-Bit(CMB), Multi-lane CMB(MCMB) and Discrete Single
+ * Bit(DSB). This function will initialize the configuration according
+ * to the dataset type supported by the TPDM.
+ */
 static void __tpdm_enable(struct tpdm_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
-	/* Check if DSB datasets is present for TPDM. */
-	if (drvdata->datasets & TPDM_PIDR0_DS_DSB)
+	if (tpdm_has_dsb_dataset(drvdata))
 		tpdm_enable_dsb(drvdata);
 
 	CS_LOCK(drvdata->base);
@@ -76,8 +110,7 @@ static void __tpdm_disable(struct tpdm_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
-	/* Check if DSB datasets is present for TPDM. */
-	if (drvdata->datasets & TPDM_PIDR0_DS_DSB)
+	if (tpdm_has_dsb_dataset(drvdata))
 		tpdm_disable_dsb(drvdata);
 
 	CS_LOCK(drvdata->base);
@@ -110,13 +143,23 @@ static const struct coresight_ops tpdm_cs_ops = {
 	.source_ops	= &tpdm_source_ops,
 };
 
-static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
+static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
 {
 	u32 pidr;
 
 	/*  Get the datasets present on the TPDM. */
 	pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
 	drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
+
+	if (tpdm_has_dsb_dataset(drvdata) && (!drvdata->dsb)) {
+		drvdata->dsb = devm_kzalloc(drvdata->dev,
+						sizeof(*drvdata->dsb), GFP_KERNEL);
+		if (!drvdata->dsb)
+			return -ENOMEM;
+	}
+	tpdm_reset_datasets(drvdata);
+
+	return 0;
 }
 
 /*
@@ -179,6 +222,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 	struct coresight_platform_data *pdata;
 	struct tpdm_drvdata *drvdata;
 	struct coresight_desc desc = { 0 };
+	int ret;
 
 	pdata = coresight_get_platform_data(dev);
 	if (IS_ERR(pdata))
@@ -198,6 +242,10 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 
 	drvdata->base = base;
 
+	ret = tpdm_datasets_setup(drvdata);
+	if (ret)
+		return ret;
+
 	/* Set up coresight component description */
 	desc.name = coresight_alloc_device_name(&tpdm_devs, dev);
 	if (!desc.name)
@@ -214,7 +262,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 		return PTR_ERR(drvdata->csdev);
 
 	spin_lock_init(&drvdata->spinlock);
-	tpdm_init_default_data(drvdata);
+
 	/* Decrease pm refcount when probe is done.*/
 	pm_runtime_put(&adev->dev);
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 5438540..f59e751 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -11,8 +11,14 @@
 
 /* DSB Subunit Registers */
 #define TPDM_DSB_CR		(0x780)
+#define TPDM_DSB_TIER		(0x784)
+
 /* Enable bit for DSB subunit */
 #define TPDM_DSB_CR_ENA		BIT(0)
+/* Enable bit for DSB subunit trigger type */
+#define TPDM_DSB_CR_TRIG_TYPE		BIT(12)
+/* Enable bit for DSB subunit trigger timestamp */
+#define TPDM_DSB_TIER_XTRIG_TSENAB		BIT(1)
 
 /* TPDM integration test registers */
 #define TPDM_ITATBCNTRL		(0xEF0)
@@ -41,6 +47,16 @@
 #define TPDM_PIDR0_DS_DSB	BIT(1)
 
 /**
+ * struct dsb_dataset - specifics associated to dsb dataset
+ * @trig_ts:          Enable/Disable trigger timestamp.
+ * @trig_type:        Enable/Disable trigger type.
+ */
+struct dsb_dataset {
+	bool			trig_ts;
+	bool			trig_type;
+};
+
+/**
  * struct tpdm_drvdata - specifics associated to an TPDM component
  * @base:       memory mapped base address for this component.
  * @dev:        The device entity associated to this component.
@@ -48,6 +64,7 @@
  * @spinlock:   lock for the drvdata value.
  * @enable:     enable status of the component.
  * @datasets:   The datasets types present of the TPDM.
+ * @dsb         Specifics associated to TPDM DSB.
  */
 
 struct tpdm_drvdata {
@@ -57,6 +74,7 @@ struct tpdm_drvdata {
 	spinlock_t		spinlock;
 	bool			enable;
 	unsigned long		datasets;
+	struct dsb_dataset	*dsb;
 };
 
 #endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
-- 
2.7.4

[PATCH v8 06/13] coresight-tpdm: Add reset node to TPDM node

[Tao Zhang]

TPDM device need a node to reset the configurations and status of
it. This change provides a node to reset the configurations and
disable the TPDM if it has been enabled.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 10 ++++++++++
 drivers/hwtracing/coresight/coresight-tpdm.c       | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 4a58e64..2936226 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -11,3 +11,13 @@ Description:
 		Accepts only one of the 2 values -  1 or 2.
 		1 : Generate 64 bits data
 		2 : Generate 32 bits data
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/reset_dataset
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(Write) Reset the dataset of the tpdm.
+
+		Accepts only one value -  1.
+		1 : Reset the dataset of the tpdm
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 951ad4d..d6e7c8c 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -162,6 +162,27 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
 	return 0;
 }
 
+static ssize_t reset_dataset_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf,
+					  size_t size)
+{
+	int ret = 0;
+	unsigned long val;
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret || val != 1)
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	tpdm_reset_datasets(drvdata);
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_WO(reset_dataset);
+
 /*
  * value 1: 64 bits test data
  * value 2: 32 bits test data
@@ -202,6 +223,7 @@ static ssize_t integration_test_store(struct device *dev,
 static DEVICE_ATTR_WO(integration_test);
 
 static struct attribute *tpdm_attrs[] = {
+	&dev_attr_reset_dataset.attr,
 	&dev_attr_integration_test.attr,
 	NULL,
 };
-- 
2.7.4

[PATCH v8 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

[Tao Zhang]

The nodes are needed to set or show the trigger timestamp and
trigger type. This change is to add these nodes to achieve these
function.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 22 +++++
 drivers/hwtracing/coresight/coresight-tpdm.c       | 95 ++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 2936226..9e26e30 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -21,3 +21,25 @@ Description:
 
 		Accepts only one value -  1.
 		1 : Reset the dataset of the tpdm
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_type
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the trigger type of the DSB for tpdm.
+
+		Accepts only one of the 2 values -  0 or 1.
+		0 : Set the DSB trigger type to false
+		1 : Set the DSB trigger type to true
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the trigger timestamp of the DSB for tpdm.
+
+		Accepts only one of the 2 values -  0 or 1.
+		0 : Set the DSB trigger type to false
+		1 : Set the DSB trigger type to true
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index d6e7c8c..8e11c9b 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -25,6 +25,18 @@ static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
 	return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
 }
 
+static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
+					   struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	if (drvdata && tpdm_has_dsb_dataset(drvdata))
+		return attr->mode;
+
+	return 0;
+}
+
 static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
 {
 	if (tpdm_has_dsb_dataset(drvdata)) {
@@ -232,8 +244,91 @@ static struct attribute_group tpdm_attr_grp = {
 	.attrs = tpdm_attrs,
 };
 
+static ssize_t dsb_trig_type_show(struct device *dev,
+		  struct device_attribute *attr, char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%u\n",
+			 (unsigned int)drvdata->dsb->trig_type);
+}
+
+/*
+ * Trigger type (boolean):
+ * false - Disable trigger type.
+ * true  - Enable trigger type.
+ */
+static ssize_t dsb_trig_type_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf,
+					  size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	if (val)
+		drvdata->dsb->trig_type = true;
+	else
+		drvdata->dsb->trig_type = false;
+	spin_unlock(&drvdata->spinlock);
+	return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_type);
+
+static ssize_t dsb_trig_ts_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%u\n",
+			 (unsigned int)drvdata->dsb->trig_ts);
+}
+
+/*
+ * Trigger timestamp (boolean):
+ * false - Disable trigger timestamp.
+ * true  - Enable trigger timestamp.
+ */
+static ssize_t dsb_trig_ts_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf,
+				   size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	if (val)
+		drvdata->dsb->trig_ts = true;
+	else
+		drvdata->dsb->trig_ts = false;
+	spin_unlock(&drvdata->spinlock);
+	return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_ts);
+
+static struct attribute *tpdm_dsb_attrs[] = {
+	&dev_attr_dsb_trig_ts.attr,
+	&dev_attr_dsb_trig_type.attr,
+	NULL,
+};
+
+static struct attribute_group tpdm_dsb_attr_grp = {
+	.attrs = tpdm_dsb_attrs,
+	.is_visible = tpdm_dsb_is_visible,
+};
+
 static const struct attribute_group *tpdm_attr_grps[] = {
 	&tpdm_attr_grp,
+	&tpdm_dsb_attr_grp,
 	NULL,
 };
 
-- 
2.7.4

[PATCH v8 08/13] coresight-tpdm: Add node to set dsb programming mode

[Tao Zhang]

Add node to set and show programming mode for TPDM DSB subunit.
Once the DSB programming mode is set, it will be written to the
register DSB_CR.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 14 ++++++
 drivers/hwtracing/coresight/coresight-tpdm.c       | 53 ++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tpdm.h       | 19 ++++++++
 3 files changed, 86 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 9e26e30..e17d1b4 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -43,3 +43,17 @@ Description:
 		Accepts only one of the 2 values -  0 or 1.
 		0 : Set the DSB trigger type to false
 		1 : Set the DSB trigger type to true
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_mode
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the programming mode of the DSB for tpdm.
+
+		Accepts the value needs to be greater than 0. What data
+		bits do is listed below.
+		Bit[0:1] : Test mode control bit for choosing the inputs.
+		Bit[3] : Set to 0 for low performance mode.
+				 Set to 1 for high performance mode.
+		Bit[4:8] : Select byte lane for high performance mode.
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 8e11c9b..2424eb7 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/amba/bus.h>
+#include <linux/bitfield.h>
 #include <linux/bitmap.h>
 #include <linux/coresight.h>
 #include <linux/coresight-pmu.h>
@@ -47,6 +48,27 @@ static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
 	}
 }
 
+static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
+{
+	u32 mode;
+
+	/* Set the test accurate mode */
+	mode = TPDM_DSB_MODE_TEST(drvdata->dsb->mode);
+	*val &= ~TPDM_DSB_CR_TEST_MODE;
+	*val |= FIELD_PREP(TPDM_DSB_CR_TEST_MODE, mode);
+
+	/* Set the byte lane for high-performance mode */
+	mode = TPDM_DSB_MODE_HPBYTESEL(drvdata->dsb->mode);
+	*val &= ~TPDM_DSB_CR_HPSEL;
+	*val |= FIELD_PREP(TPDM_DSB_CR_HPSEL, mode);
+
+	/* Set the performance mode */
+	if (drvdata->dsb->mode & TPDM_DSB_MODE_PERF)
+		*val |= TPDM_DSB_CR_MODE;
+	else
+		*val &= ~TPDM_DSB_CR_MODE;
+}
+
 static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 {
 	u32 val;
@@ -60,6 +82,8 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
 
 	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
+	/* Set the mode of DSB dataset */
+	set_dsb_mode(drvdata, &val);
 	/* Set trigger type */
 	if (drvdata->dsb->trig_type)
 		val |= TPDM_DSB_CR_TRIG_TYPE;
@@ -244,6 +268,34 @@ static struct attribute_group tpdm_attr_grp = {
 	.attrs = tpdm_attrs,
 };
 
+static ssize_t dsb_mode_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%x\n", drvdata->dsb->mode);
+}
+
+static ssize_t dsb_mode_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf,
+				size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val < 0) ||
+			(val & ~TPDM_DSB_MODE_MASK))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->dsb->mode = val & TPDM_DSB_MODE_MASK;
+	spin_unlock(&drvdata->spinlock);
+	return size;
+}
+static DEVICE_ATTR_RW(dsb_mode);
+
 static ssize_t dsb_trig_type_show(struct device *dev,
 		  struct device_attribute *attr, char *buf)
 {
@@ -316,6 +368,7 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
 static DEVICE_ATTR_RW(dsb_trig_ts);
 
 static struct attribute *tpdm_dsb_attrs[] = {
+	&dev_attr_dsb_mode.attr,
 	&dev_attr_dsb_trig_ts.attr,
 	&dev_attr_dsb_trig_type.attr,
 	NULL,
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index f59e751..f57c9fe 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -15,11 +15,28 @@
 
 /* Enable bit for DSB subunit */
 #define TPDM_DSB_CR_ENA		BIT(0)
+/* Enable bit for DSB subunit perfmance mode */
+#define TPDM_DSB_CR_MODE		BIT(1)
 /* Enable bit for DSB subunit trigger type */
 #define TPDM_DSB_CR_TRIG_TYPE		BIT(12)
+/* Data bits for DSB high performace mode */
+#define TPDM_DSB_CR_HPSEL		GENMASK(6, 2)
+/* Data bits for DSB test mode */
+#define TPDM_DSB_CR_TEST_MODE		GENMASK(10, 9)
+
 /* Enable bit for DSB subunit trigger timestamp */
 #define TPDM_DSB_TIER_XTRIG_TSENAB		BIT(1)
 
+/* DSB programming modes */
+/* DSB mode bits mask */
+#define TPDM_DSB_MODE_MASK			GENMASK(8, 0)
+/* Test mode control bit*/
+#define TPDM_DSB_MODE_TEST(val)	(val & GENMASK(1, 0))
+/* Performance mode */
+#define TPDM_DSB_MODE_PERF		BIT(3)
+/* High performance mode */
+#define TPDM_DSB_MODE_HPBYTESEL(val)	(val & GENMASK(8, 4))
+
 /* TPDM integration test registers */
 #define TPDM_ITATBCNTRL		(0xEF0)
 #define TPDM_ITCNTRL		(0xF00)
@@ -48,10 +65,12 @@
 
 /**
  * struct dsb_dataset - specifics associated to dsb dataset
+ * @mode:             DSB programming mode
  * @trig_ts:          Enable/Disable trigger timestamp.
  * @trig_type:        Enable/Disable trigger type.
  */
 struct dsb_dataset {
+	u32				mode;
 	bool			trig_ts;
 	bool			trig_type;
 };
-- 
2.7.4

[PATCH v8 09/13] coresight-tpdm: Add nodes for dsb edge control

[Tao Zhang]

Add the nodes to set value for DSB edge control and DSB edge
control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
resgisters to configure edge control. DSB edge detection control
00: Rising edge detection
01: Falling edge detection
10: Rising and falling edge detection (toggle detection)
And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
configure mask. Eight 32 bit registers providing DSB interface
edge detection mask control.

Add the nodes to configure DSB edge control and DSB edge control
mask. Each DSB subunit TPDM maximum of 256 edge detections can be
configured. The index and value sysfs files need to be paired and
written to order. The index sysfs file is to set the index number
of the edge detection which needs to be configured. And the value
sysfs file is to set the control or mask for the edge detection.
DSB edge detection control should be set as the following values.
00: Rising edge detection
01: Falling edge detection
10: Rising and falling edge detection (toggle detection)
And DSB edge mask should be set as 0 or 1.
Each DSB subunit TPDM has maximum of n(n<16) EDCR resgisters to
configure edge control. And each DSB subunit TPDM has maximum of
m(m<8) ECDMR registers to configure mask.

Add the nodes to read a set of the edge control value and mask
of the DSB in TPDM.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  51 ++++++
 drivers/hwtracing/coresight/coresight-tpdm.c       | 177 ++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tpdm.h       |  63 ++++++++
 3 files changed, 288 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index e17d1b4..097fdc4 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -57,3 +57,54 @@ Description:
 		Bit[3] : Set to 0 for low performance mode.
 				 Set to 1 for high performance mode.
 		Bit[4:8] : Select byte lane for high performance mode.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_idx
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the index number of the edge detection for the DSB
+		subunit TPDM. Since there are at most 256 edge detections, this
+		value ranges from 0 to 255.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_val
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		Write a data to control the edge detection corresponding to
+		the index number. Before writing data to this sysfs file,
+		"ctrl_idx" should be written first to configure the index
+		number of the edge detection which needs to be controlled.
+
+		Accepts only one of the following values.
+		0 - Rising edge detection
+		1 - Falling edge detection
+		2 - Rising and falling edge detection (toggle detection)
+
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_mask
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		Write a data to mask the edge detection corresponding to the index
+		number. Before writing data to this sysfs file, "ctrl_idx" should
+		be written first to configure the index number of the edge detection
+		which needs to be masked.
+
+		Accepts only one of the 2 values -  0 or 1.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcr[0:15]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		Read a set of the edge control value of the DSB in TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcmr[0:7]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		Read a set of the edge control mask of the DSB in TPDM.
\ No newline at end of file
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 2424eb7..ba61e6a 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -21,6 +21,29 @@
 
 DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
 
+/* Read dataset array member with the index number */
+static ssize_t tpdm_simple_dataset_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	struct tpdm_dataset_attribute *tpdm_attr =
+		container_of(attr, struct tpdm_dataset_attribute, attr);
+
+	if (tpdm_attr->idx >= tpdm_attr->max)
+		return -EINVAL;
+
+	switch (tpdm_attr->mem) {
+	case DSB_EDGE_CTRL:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->edge_ctrl[tpdm_attr->idx]);
+	case DSB_EDGE_CTRL_MASK:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
+	default:
+		return -EINVAL;
+	}
+}
+
 static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
 {
 	return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
@@ -71,7 +94,14 @@ static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
 
 static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 {
-	u32 val;
+	u32 val, i;
+
+	for (i = 0; i < TPDM_DSB_MAX_EDCR; i++)
+		writel_relaxed(drvdata->dsb->edge_ctrl[i],
+			   drvdata->base + TPDM_DSB_EDCR(i));
+	for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
+		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
+			   drvdata->base + TPDM_DSB_EDCMR(i));
 
 	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
 	/* Set trigger timestamp */
@@ -296,6 +326,109 @@ static ssize_t dsb_mode_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(dsb_mode);
 
+static ssize_t ctrl_idx_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%u\n",
+			(unsigned int)drvdata->dsb->edge_ctrl_idx);
+}
+
+/*
+ * The EDCR registers can include up to 16 32-bit registers, and each
+ * one can be configured to control up to 16 edge detections(2 bits
+ * control one edge detection). So a total 256 edge detections can be
+ * configured. This function provides a way to set the index number of
+ * the edge detection which needs to be configured.
+ */
+static ssize_t ctrl_idx_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf,
+				size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val >= TPDM_DSB_MAX_LINES))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->dsb->edge_ctrl_idx = val;
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_RW(ctrl_idx);
+
+/*
+ * This function is used to control the edge detection according
+ * to the index number that has been set.
+ * "edge_ctrl" should be one of the following values.
+ * 0 - Rising edge detection
+ * 1 - Falling edge detection
+ * 2 - Rising and falling edge detection (toggle detection)
+ */
+static ssize_t ctrl_val_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf,
+				size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val, edge_ctrl;
+	int reg;
+
+	if ((kstrtoul(buf, 0, &edge_ctrl)) || (edge_ctrl > 0x2))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	/*
+	 * There are 2 bit per DSB Edge Control line.
+	 * Thus we have 16 lines in a 32bit word.
+	 */
+	reg = EDCR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
+	val = drvdata->dsb->edge_ctrl[reg];
+	val &= ~EDCR_TO_WORD_MASK(drvdata->dsb->edge_ctrl_idx);
+	val |= EDCR_TO_WORD_VAL(edge_ctrl, drvdata->dsb->edge_ctrl_idx);
+	drvdata->dsb->edge_ctrl[reg] = val;
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_WO(ctrl_val);
+
+static ssize_t ctrl_mask_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+	u32 set;
+	int reg;
+
+	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	/*
+	 * There is 1 bit per DSB Edge Control Mark line.
+	 * Thus we have 32 lines in a 32bit word.
+	 */
+	reg = EDCMR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
+	set = drvdata->dsb->edge_ctrl_mask[reg];
+	if (val)
+		set |= BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
+	else
+		set &= ~BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
+	drvdata->dsb->edge_ctrl_mask[reg] = set;
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
+}
+static DEVICE_ATTR_WO(ctrl_mask);
+
 static ssize_t dsb_trig_type_show(struct device *dev,
 		  struct device_attribute *attr, char *buf)
 {
@@ -367,6 +500,37 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(dsb_trig_ts);
 
+static struct attribute *tpdm_dsb_edge_attrs[] = {
+	&dev_attr_ctrl_idx.attr,
+	&dev_attr_ctrl_val.attr,
+	&dev_attr_ctrl_mask.attr,
+	DSB_EDGE_CTRL_ATTR(0),
+	DSB_EDGE_CTRL_ATTR(1),
+	DSB_EDGE_CTRL_ATTR(2),
+	DSB_EDGE_CTRL_ATTR(3),
+	DSB_EDGE_CTRL_ATTR(4),
+	DSB_EDGE_CTRL_ATTR(5),
+	DSB_EDGE_CTRL_ATTR(6),
+	DSB_EDGE_CTRL_ATTR(7),
+	DSB_EDGE_CTRL_ATTR(8),
+	DSB_EDGE_CTRL_ATTR(9),
+	DSB_EDGE_CTRL_ATTR(10),
+	DSB_EDGE_CTRL_ATTR(11),
+	DSB_EDGE_CTRL_ATTR(12),
+	DSB_EDGE_CTRL_ATTR(13),
+	DSB_EDGE_CTRL_ATTR(14),
+	DSB_EDGE_CTRL_ATTR(15),
+	DSB_EDGE_CTRL_MASK_ATTR(0),
+	DSB_EDGE_CTRL_MASK_ATTR(1),
+	DSB_EDGE_CTRL_MASK_ATTR(2),
+	DSB_EDGE_CTRL_MASK_ATTR(3),
+	DSB_EDGE_CTRL_MASK_ATTR(4),
+	DSB_EDGE_CTRL_MASK_ATTR(5),
+	DSB_EDGE_CTRL_MASK_ATTR(6),
+	DSB_EDGE_CTRL_MASK_ATTR(7),
+	NULL,
+};
+
 static struct attribute *tpdm_dsb_attrs[] = {
 	&dev_attr_dsb_mode.attr,
 	&dev_attr_dsb_trig_ts.attr,
@@ -374,14 +538,21 @@ static struct attribute *tpdm_dsb_attrs[] = {
 	NULL,
 };
 
-static struct attribute_group tpdm_dsb_attr_grp = {
+static struct attribute_group tpdm_dsb_attrs_grp = {
 	.attrs = tpdm_dsb_attrs,
 	.is_visible = tpdm_dsb_is_visible,
 };
 
+static struct attribute_group tpdm_dsb_edge_grp = {
+	.attrs = tpdm_dsb_edge_attrs,
+	.is_visible = tpdm_dsb_is_visible,
+	.name = "dsb_edge",
+};
+
 static const struct attribute_group *tpdm_attr_grps[] = {
 	&tpdm_attr_grp,
-	&tpdm_dsb_attr_grp,
+	&tpdm_dsb_attrs_grp,
+	&tpdm_dsb_edge_grp,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index f57c9fe..9736e00 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -12,6 +12,8 @@
 /* DSB Subunit Registers */
 #define TPDM_DSB_CR		(0x780)
 #define TPDM_DSB_TIER		(0x784)
+#define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
+#define TPDM_DSB_EDCMR(n)	(0x848 + (n * 4))
 
 /* Enable bit for DSB subunit */
 #define TPDM_DSB_CR_ENA		BIT(0)
@@ -37,6 +39,16 @@
 /* High performance mode */
 #define TPDM_DSB_MODE_HPBYTESEL(val)	(val & GENMASK(8, 4))
 
+#define EDCRS_PER_WORD				16
+#define EDCR_TO_WORD_IDX(r)			((r) / EDCRS_PER_WORD)
+#define EDCR_TO_WORD_SHIFT(r)		((r % EDCRS_PER_WORD) * 2)
+#define EDCR_TO_WORD_VAL(val, r)	(val << EDCR_TO_WORD_SHIFT(r))
+#define EDCR_TO_WORD_MASK(r)		EDCR_TO_WORD_VAL(0x3, r)
+
+#define EDCMRS_PER_WORD				32
+#define EDCMR_TO_WORD_IDX(r)		((r) / EDCMRS_PER_WORD)
+#define EDCMR_TO_WORD_SHIFT(r)		((r) % EDCMRS_PER_WORD)
+
 /* TPDM integration test registers */
 #define TPDM_ITATBCNTRL		(0xEF0)
 #define TPDM_ITCNTRL		(0xF00)
@@ -63,14 +75,44 @@
 #define TPDM_PIDR0_DS_IMPDEF	BIT(0)
 #define TPDM_PIDR0_DS_DSB	BIT(1)
 
+#define TPDM_DSB_MAX_LINES	256
+/* MAX number of EDCR registers */
+#define TPDM_DSB_MAX_EDCR	16
+/* MAX number of EDCMR registers */
+#define TPDM_DSB_MAX_EDCMR	8
+
+#define tpdm_simple_dataset_ro(name, mem, idx, max)			\
+	(&((struct tpdm_dataset_attribute[]) {			\
+	   {								\
+		__ATTR(name, 0444, tpdm_simple_dataset_show, NULL),	\
+		mem,							\
+		idx,							\
+		max,							\
+	   }								\
+	})[0].attr.attr)
+
+#define DSB_EDGE_CTRL_ATTR(nr)					\
+		tpdm_simple_dataset_ro(edcr##nr,		\
+		DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
+
+#define DSB_EDGE_CTRL_MASK_ATTR(nr)				\
+		tpdm_simple_dataset_ro(edcmr##nr,		\
+		DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
+
 /**
  * struct dsb_dataset - specifics associated to dsb dataset
  * @mode:             DSB programming mode
+ * @edge_ctrl_idx     Index number of the edge control
+ * @edge_ctrl:        Save value for edge control
+ * @edge_ctrl_mask:   Save value for edge control mask
  * @trig_ts:          Enable/Disable trigger timestamp.
  * @trig_type:        Enable/Disable trigger type.
  */
 struct dsb_dataset {
 	u32				mode;
+	u32				edge_ctrl_idx;
+	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
+	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
 	bool			trig_ts;
 	bool			trig_type;
 };
@@ -96,4 +138,25 @@ struct tpdm_drvdata {
 	struct dsb_dataset	*dsb;
 };
 
+/* Enumerate members of various datasets */
+enum dataset_mem {
+	DSB_EDGE_CTRL,
+	DSB_EDGE_CTRL_MASK,
+};
+
+/**
+ * struct tpdm_dataset_attribute - Record the member variables and
+ * index number of datasets that need to be operated by sysfs file
+ * @attr:       The device attribute
+ * @mem:        The member in the dataset data structure
+ * @idx:        The index number of the array data
+ * @max:        The maximal number of the array data
+ */
+struct tpdm_dataset_attribute {
+	struct device_attribute attr;
+	enum dataset_mem mem;
+	u32 idx;
+	u32 max;
+};
+
 #endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
-- 
2.7.4

[PATCH v8 10/13] coresight-tpdm: Add nodes to configure pattern match output

[Tao Zhang]

Add nodes to configure trigger pattern and trigger pattern mask.
Each DSB subunit TPDM has maximum of n(n<7) XPR registers to
configure trigger pattern match output. Eight 32 bit registers
providing DSB interface trigger output pattern match comparison.
And each DSB subunit TPDM has maximum of m(m<7) XPMR registers to
configure trigger pattern mask match output. Eight 32 bit
registers providing DSB interface trigger output pattern match
mask.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 18 +++++-
 drivers/hwtracing/coresight/coresight-tpdm.c       | 71 +++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tpdm.h       | 29 +++++++++
 3 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 097fdc4..f5cd302 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -107,4 +107,20 @@ Date:		March 2023
 KernelVersion	6.5
 Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
 Description:
-		Read a set of the edge control mask of the DSB in TPDM.
\ No newline at end of file
+		Read a set of the edge control mask of the DSB in TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpr[0:7]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the value of the trigger pattern for the DSB
+		subunit TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpmr[0:7]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the mask of the trigger pattern for the DSB
+		subunit TPDM.
\ No newline at end of file
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index ba61e6a..6521019 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -39,9 +39,46 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
 	case DSB_EDGE_CTRL_MASK:
 		return sysfs_emit(buf, "0x%x\n",
 				drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
+	case DSB_TRIG_PATT:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->trig_patt[tpdm_attr->idx]);
+	case DSB_TRIG_PATT_MASK:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Write dataset array member with the index number */
+static ssize_t tpdm_simple_dataset_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf,
+					  size_t size)
+{
+	unsigned long val;
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	struct tpdm_dataset_attribute *tpdm_attr =
+		container_of(attr, struct tpdm_dataset_attribute, attr);
+
+	if (kstrtoul(buf, 0, &val) || (tpdm_attr->idx >= tpdm_attr->max))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	switch (tpdm_attr->mem) {
+	case DSB_TRIG_PATT:
+		drvdata->dsb->trig_patt[tpdm_attr->idx] = val;
+		break;
+	case DSB_TRIG_PATT_MASK:
+		drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
+		break;
 	default:
+		spin_unlock(&drvdata->spinlock);
 		return -EINVAL;
 	}
+	spin_unlock(&drvdata->spinlock);
+
+	return size;
 }
 
 static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
@@ -102,7 +139,12 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 	for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
 		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
 			   drvdata->base + TPDM_DSB_EDCMR(i));
-
+	for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+		writel_relaxed(drvdata->dsb->trig_patt[i],
+			    drvdata->base + TPDM_DSB_XPR(i));
+		writel_relaxed(drvdata->dsb->trig_patt_mask[i],
+			    drvdata->base + TPDM_DSB_XPMR(i));
+	}
 	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
 	/* Set trigger timestamp */
 	if (drvdata->dsb->trig_ts)
@@ -531,6 +573,26 @@ static struct attribute *tpdm_dsb_edge_attrs[] = {
 	NULL,
 };
 
+static struct attribute *tpdm_dsb_trig_patt_attrs[] = {
+	DSB_TRIG_PATT_ATTR(0),
+	DSB_TRIG_PATT_ATTR(1),
+	DSB_TRIG_PATT_ATTR(2),
+	DSB_TRIG_PATT_ATTR(3),
+	DSB_TRIG_PATT_ATTR(4),
+	DSB_TRIG_PATT_ATTR(5),
+	DSB_TRIG_PATT_ATTR(6),
+	DSB_TRIG_PATT_ATTR(7),
+	DSB_TRIG_PATT_MASK_ATTR(0),
+	DSB_TRIG_PATT_MASK_ATTR(1),
+	DSB_TRIG_PATT_MASK_ATTR(2),
+	DSB_TRIG_PATT_MASK_ATTR(3),
+	DSB_TRIG_PATT_MASK_ATTR(4),
+	DSB_TRIG_PATT_MASK_ATTR(5),
+	DSB_TRIG_PATT_MASK_ATTR(6),
+	DSB_TRIG_PATT_MASK_ATTR(7),
+	NULL,
+};
+
 static struct attribute *tpdm_dsb_attrs[] = {
 	&dev_attr_dsb_mode.attr,
 	&dev_attr_dsb_trig_ts.attr,
@@ -549,10 +611,17 @@ static struct attribute_group tpdm_dsb_edge_grp = {
 	.name = "dsb_edge",
 };
 
+static struct attribute_group tpdm_dsb_trig_patt_grp = {
+	.attrs = tpdm_dsb_trig_patt_attrs,
+	.is_visible = tpdm_dsb_is_visible,
+	.name = "dsb_trig_patt",
+};
+
 static const struct attribute_group *tpdm_attr_grps[] = {
 	&tpdm_attr_grp,
 	&tpdm_dsb_attrs_grp,
 	&tpdm_dsb_edge_grp,
+	&tpdm_dsb_trig_patt_grp,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 9736e00..9e1b0a4 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -12,6 +12,8 @@
 /* DSB Subunit Registers */
 #define TPDM_DSB_CR		(0x780)
 #define TPDM_DSB_TIER		(0x784)
+#define TPDM_DSB_XPR(n)		(0x7C8 + (n * 4))
+#define TPDM_DSB_XPMR(n)	(0x7E8 + (n * 4))
 #define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
 #define TPDM_DSB_EDCMR(n)	(0x848 + (n * 4))
 
@@ -80,6 +82,8 @@
 #define TPDM_DSB_MAX_EDCR	16
 /* MAX number of EDCMR registers */
 #define TPDM_DSB_MAX_EDCMR	8
+/* MAX number of DSB pattern */
+#define TPDM_DSB_MAX_PATT	8
 
 #define tpdm_simple_dataset_ro(name, mem, idx, max)			\
 	(&((struct tpdm_dataset_attribute[]) {			\
@@ -91,6 +95,17 @@
 	   }								\
 	})[0].attr.attr)
 
+#define tpdm_simple_dataset_rw(name, mem, idx, max)			\
+	(&((struct tpdm_dataset_attribute[]) {			\
+	   {								\
+		__ATTR(name, 0644, tpdm_simple_dataset_show,		\
+		tpdm_simple_dataset_store),		\
+		mem,							\
+		idx,							\
+		max								\
+	   }								\
+	})[0].attr.attr)
+
 #define DSB_EDGE_CTRL_ATTR(nr)					\
 		tpdm_simple_dataset_ro(edcr##nr,		\
 		DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
@@ -99,12 +114,22 @@
 		tpdm_simple_dataset_ro(edcmr##nr,		\
 		DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
 
+#define DSB_TRIG_PATT_ATTR(nr)					\
+		tpdm_simple_dataset_rw(xpr##nr,			\
+		DSB_TRIG_PATT, nr, TPDM_DSB_MAX_PATT)
+
+#define DSB_TRIG_PATT_MASK_ATTR(nr)				\
+		tpdm_simple_dataset_rw(xpmr##nr,		\
+		DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
+
 /**
  * struct dsb_dataset - specifics associated to dsb dataset
  * @mode:             DSB programming mode
  * @edge_ctrl_idx     Index number of the edge control
  * @edge_ctrl:        Save value for edge control
  * @edge_ctrl_mask:   Save value for edge control mask
+ * @trig_patt:        Save value for trigger pattern
+ * @trig_patt_mask:   Save value for trigger pattern mask
  * @trig_ts:          Enable/Disable trigger timestamp.
  * @trig_type:        Enable/Disable trigger type.
  */
@@ -113,6 +138,8 @@ struct dsb_dataset {
 	u32				edge_ctrl_idx;
 	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
 	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
+	u32				trig_patt[TPDM_DSB_MAX_PATT];
+	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
 	bool			trig_ts;
 	bool			trig_type;
 };
@@ -142,6 +169,8 @@ struct tpdm_drvdata {
 enum dataset_mem {
 	DSB_EDGE_CTRL,
 	DSB_EDGE_CTRL_MASK,
+	DSB_TRIG_PATT,
+	DSB_TRIG_PATT_MASK,
 };
 
 /**
-- 
2.7.4

[PATCH v8 11/13] coresight-tpdm: Add nodes for timestamp request

[Tao Zhang]

Add nodes to configure the timestamp request based on input
pattern match. Each TPDM that support DSB subunit has maximum of
n(n<7) TPR registers to configure value for timestamp request
based on input pattern match. Eight 32 bit registers providing
DSB interface timestamp request  pattern match comparison. And
each TPDM that support DSB subunit has maximum of m(m<7) TPMR
registers to configure pattern mask for timestamp request. Eight
32 bit registers providing DSB interface timestamp request
pattern match mask generation. Add nodes to enable/disable
pattern timestamp and set pattern timestamp type.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  40 ++++++-
 drivers/hwtracing/coresight/coresight-tpdm.c       | 133 ++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tpdm.h       |  24 ++++
 3 files changed, 191 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index f5cd302..46a5535 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -123,4 +123,42 @@ KernelVersion	6.5
 Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
 Description:
 		(RW) Set/Get the mask of the trigger pattern for the DSB
-		subunit TPDM.
\ No newline at end of file
+		subunit TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpr[0:7]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the value of the pattern for the DSB subunit TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpmr[0:7]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the mask of the pattern for the DSB subunit TPDM.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(Write) Set the pattern timestamp of DSB tpdm. Read
+		the pattern timestamp of DSB tpdm.
+
+		Accepts only one of the 2 values -  0 or 1.
+		0 : Disable DSB pattern timestamp.
+		1 : Enable DSB pattern timestamp.
+
+What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt_type
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(Write) Set the pattern type of DSB tpdm. Read
+		the pattern type of DSB tpdm.
+
+		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.
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 6521019..9b0e060 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -45,6 +45,12 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
 	case DSB_TRIG_PATT_MASK:
 		return sysfs_emit(buf, "0x%x\n",
 				drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
+	case DSB_PATT:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->patt_val[tpdm_attr->idx]);
+	case DSB_PATT_MASK:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->patt_mask[tpdm_attr->idx]);
 	default:
 		return -EINVAL;
 	}
@@ -72,6 +78,12 @@ static ssize_t tpdm_simple_dataset_store(struct device *dev,
 	case DSB_TRIG_PATT_MASK:
 		drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
 		break;
+	case DSB_PATT:
+		drvdata->dsb->patt_val[tpdm_attr->idx] = val;
+		break;
+	case DSB_PATT_MASK:
+		drvdata->dsb->patt_mask[tpdm_attr->idx] = val;
+		break;
 	default:
 		spin_unlock(&drvdata->spinlock);
 		return -EINVAL;
@@ -129,6 +141,27 @@ static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
 		*val &= ~TPDM_DSB_CR_MODE;
 }
 
+static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val)
+{
+	/* Set pattern timestamp type and enablement */
+	if (drvdata->dsb->patt_ts) {
+		*val |= TPDM_DSB_TIER_PATT_TSENAB;
+		if (drvdata->dsb->patt_type)
+			*val |= TPDM_DSB_TIER_PATT_TYPE;
+		else
+			*val &= ~TPDM_DSB_TIER_PATT_TYPE;
+	} else {
+		*val &= ~TPDM_DSB_TIER_PATT_TSENAB;
+	}
+
+	/* Set trigger timestamp */
+	if (drvdata->dsb->trig_ts)
+		*val |= TPDM_DSB_TIER_XTRIG_TSENAB;
+	else
+		*val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
+
+}
+
 static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 {
 	u32 val, i;
@@ -140,17 +173,17 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
 			   drvdata->base + TPDM_DSB_EDCMR(i));
 	for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+		writel_relaxed(drvdata->dsb->patt_val[i],
+			    drvdata->base + TPDM_DSB_TPR(i));
+		writel_relaxed(drvdata->dsb->patt_mask[i],
+			    drvdata->base + TPDM_DSB_TPMR(i));
 		writel_relaxed(drvdata->dsb->trig_patt[i],
 			    drvdata->base + TPDM_DSB_XPR(i));
 		writel_relaxed(drvdata->dsb->trig_patt_mask[i],
 			    drvdata->base + TPDM_DSB_XPMR(i));
 	}
 	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
-	/* Set trigger timestamp */
-	if (drvdata->dsb->trig_ts)
-		val |= TPDM_DSB_TIER_XTRIG_TSENAB;
-	else
-		val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
+	set_dsb_tier(drvdata, &val);
 	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
 
 	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
@@ -471,6 +504,67 @@ static ssize_t ctrl_mask_store(struct device *dev,
 }
 static DEVICE_ATTR_WO(ctrl_mask);
 
+static ssize_t dsb_patt_ts_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%u\n",
+			 (unsigned int)drvdata->dsb->patt_ts);
+}
+
+/*
+ * value 1: Enable/Disable DSB pattern timestamp
+ */
+static ssize_t dsb_patt_ts_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf,
+				   size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->dsb->patt_ts = !!val;
+	spin_unlock(&drvdata->spinlock);
+	return size;
+}
+static DEVICE_ATTR_RW(dsb_patt_ts);
+
+static ssize_t dsb_patt_type_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	return sysfs_emit(buf, "%u\n",
+			 (unsigned int)drvdata->dsb->patt_type);
+}
+
+/*
+ * value 1: Set DSB pattern type
+ */
+static ssize_t dsb_patt_type_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf, size_t size)
+{
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	unsigned long val;
+
+	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+		return -EINVAL;
+
+	spin_lock(&drvdata->spinlock);
+	drvdata->dsb->patt_type = val;
+	spin_unlock(&drvdata->spinlock);
+	return size;
+}
+static DEVICE_ATTR_RW(dsb_patt_type);
+
 static ssize_t dsb_trig_type_show(struct device *dev,
 		  struct device_attribute *attr, char *buf)
 {
@@ -593,8 +687,30 @@ static struct attribute *tpdm_dsb_trig_patt_attrs[] = {
 	NULL,
 };
 
+static struct attribute *tpdm_dsb_patt_attrs[] = {
+	DSB_PATT_ATTR(0),
+	DSB_PATT_ATTR(1),
+	DSB_PATT_ATTR(2),
+	DSB_PATT_ATTR(3),
+	DSB_PATT_ATTR(4),
+	DSB_PATT_ATTR(5),
+	DSB_PATT_ATTR(6),
+	DSB_PATT_ATTR(7),
+	DSB_PATT_MASK_ATTR(0),
+	DSB_PATT_MASK_ATTR(1),
+	DSB_PATT_MASK_ATTR(2),
+	DSB_PATT_MASK_ATTR(3),
+	DSB_PATT_MASK_ATTR(4),
+	DSB_PATT_MASK_ATTR(5),
+	DSB_PATT_MASK_ATTR(6),
+	DSB_PATT_MASK_ATTR(7),
+	NULL,
+};
+
 static struct attribute *tpdm_dsb_attrs[] = {
 	&dev_attr_dsb_mode.attr,
+	&dev_attr_dsb_patt_ts.attr,
+	&dev_attr_dsb_patt_type.attr,
 	&dev_attr_dsb_trig_ts.attr,
 	&dev_attr_dsb_trig_type.attr,
 	NULL,
@@ -617,11 +733,18 @@ static struct attribute_group tpdm_dsb_trig_patt_grp = {
 	.name = "dsb_trig_patt",
 };
 
+static struct attribute_group tpdm_dsb_patt_grp = {
+	.attrs = tpdm_dsb_patt_attrs,
+	.is_visible = tpdm_dsb_is_visible,
+	.name = "dsb_patt",
+};
+
 static const struct attribute_group *tpdm_attr_grps[] = {
 	&tpdm_attr_grp,
 	&tpdm_dsb_attrs_grp,
 	&tpdm_dsb_edge_grp,
 	&tpdm_dsb_trig_patt_grp,
+	&tpdm_dsb_patt_grp,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 9e1b0a4..9173e80 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -12,6 +12,8 @@
 /* DSB Subunit Registers */
 #define TPDM_DSB_CR		(0x780)
 #define TPDM_DSB_TIER		(0x784)
+#define TPDM_DSB_TPR(n)		(0x788 + (n * 4))
+#define TPDM_DSB_TPMR(n)	(0x7A8 + (n * 4))
 #define TPDM_DSB_XPR(n)		(0x7C8 + (n * 4))
 #define TPDM_DSB_XPMR(n)	(0x7E8 + (n * 4))
 #define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
@@ -28,8 +30,12 @@
 /* Data bits for DSB test mode */
 #define TPDM_DSB_CR_TEST_MODE		GENMASK(10, 9)
 
+/* Enable bit for DSB subunit pattern timestamp */
+#define TPDM_DSB_TIER_PATT_TSENAB		BIT(0)
 /* Enable bit for DSB subunit trigger timestamp */
 #define TPDM_DSB_TIER_XTRIG_TSENAB		BIT(1)
+/* Bit for DSB subunit pattern type */
+#define TPDM_DSB_TIER_PATT_TYPE		BIT(2)
 
 /* DSB programming modes */
 /* DSB mode bits mask */
@@ -122,14 +128,26 @@
 		tpdm_simple_dataset_rw(xpmr##nr,		\
 		DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
 
+#define DSB_PATT_ATTR(nr)						\
+		tpdm_simple_dataset_rw(tpr##nr,			\
+		DSB_PATT, nr, TPDM_DSB_MAX_PATT)
+
+#define DSB_PATT_MASK_ATTR(nr)					\
+		tpdm_simple_dataset_rw(tpmr##nr,		\
+		DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
+
 /**
  * struct dsb_dataset - specifics associated to dsb dataset
  * @mode:             DSB programming mode
  * @edge_ctrl_idx     Index number of the edge control
  * @edge_ctrl:        Save value for edge control
  * @edge_ctrl_mask:   Save value for edge control mask
+ * @patt_val:         Save value for pattern
+ * @patt_mask:        Save value for pattern mask
  * @trig_patt:        Save value for trigger pattern
  * @trig_patt_mask:   Save value for trigger pattern mask
+ * @patt_ts:          Enable/Disable pattern timestamp
+ * @patt_type:        Set pattern type
  * @trig_ts:          Enable/Disable trigger timestamp.
  * @trig_type:        Enable/Disable trigger type.
  */
@@ -138,8 +156,12 @@ struct dsb_dataset {
 	u32				edge_ctrl_idx;
 	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
 	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
+	u32				patt_val[TPDM_DSB_MAX_PATT];
+	u32				patt_mask[TPDM_DSB_MAX_PATT];
 	u32				trig_patt[TPDM_DSB_MAX_PATT];
 	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
+	bool			patt_ts;
+	bool			patt_type;
 	bool			trig_ts;
 	bool			trig_type;
 };
@@ -171,6 +193,8 @@ enum dataset_mem {
 	DSB_EDGE_CTRL_MASK,
 	DSB_TRIG_PATT,
 	DSB_TRIG_PATT_MASK,
+	DSB_PATT,
+	DSB_PATT_MASK,
 };
 
 /**
-- 
2.7.4

[PATCH v8 12/13] dt-bindings: arm: Add support for DSB MSR register

[Tao Zhang]

Add property "qcom,dsb-msrs-num" to support DSB(Discrete Single
Bit) MSR(mux select register) for TPDM. It specifies the number
of MSR registers supported by the DSB TDPM.

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
index 931ee8f..d1d66bc 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
@@ -52,6 +52,15 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint8
     enum: [32, 64]
 
+  qcom,dsb-msrs-num:
+    description:
+      Specifies the number of DSB(Discrete Single Bit) MSR(mux select register)
+      registers supported by the monitor. If this property is not configured
+      or set to 0, it means this DSB TPDM doesn't support MSR.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 32
+
   clocks:
     maxItems: 1
 
@@ -86,6 +95,7 @@ examples:
       reg = <0x0684c000 0x1000>;
 
       qcom,dsb-element-size = /bits/ 8 <32>;
+      qcom,dsb-msrs-num = <16>;
 
       clocks = <&aoss_qmp>;
       clock-names = "apb_pclk";
-- 
2.7.4

[PATCH v8 13/13] coresight-tpdm: Add nodes for dsb msr support

[Tao Zhang]

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   |  8 +++
 drivers/hwtracing/coresight/coresight-tpdm.c       | 79 ++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tpdm.h       | 12 ++++
 3 files changed, 99 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 46a5535..9b7e172 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -162,3 +162,11 @@ 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/msr[0:31]
+Date:		March 2023
+KernelVersion	6.5
+Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
+Description:
+		(RW) Set/Get the MSR(mux select register) for the DSB subunit
+		TPDM.
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 9b0e060..367c5e2 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -51,6 +51,9 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
 	case DSB_PATT_MASK:
 		return sysfs_emit(buf, "0x%x\n",
 				drvdata->dsb->patt_mask[tpdm_attr->idx]);
+	case DSB_MSR:
+		return sysfs_emit(buf, "0x%x\n",
+				drvdata->dsb->msr[tpdm_attr->idx]);
 	default:
 		return -EINVAL;
 	}
@@ -84,6 +87,9 @@ static ssize_t tpdm_simple_dataset_store(struct device *dev,
 	case DSB_PATT_MASK:
 		drvdata->dsb->patt_mask[tpdm_attr->idx] = val;
 		break;
+	case DSB_MSR:
+		drvdata->dsb->msr[tpdm_attr->idx] = val;
+		break;
 	default:
 		spin_unlock(&drvdata->spinlock);
 		return -EINVAL;
@@ -110,6 +116,22 @@ static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
 	return 0;
 }
 
+static umode_t tpdm_dsb_msr_is_visible(struct kobject *kobj,
+					   struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	if (drvdata && tpdm_has_dsb_dataset(drvdata)) {
+		of_property_read_u32(drvdata->dev->of_node,
+			   "qcom,dsb_msr_num", &drvdata->dsb->msr_num);
+		if (drvdata->dsb->msr_num != 0)
+			return attr->mode;
+	}
+
+	return 0;
+}
+
 static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
 {
 	if (tpdm_has_dsb_dataset(drvdata)) {
@@ -162,6 +184,18 @@ static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val)
 
 }
 
+static void set_dsb_msr(struct tpdm_drvdata *drvdata)
+{
+	int i;
+
+	if (drvdata->dsb->msr_num == 0)
+		return;
+
+	for (i = 0; i < drvdata->dsb->msr_num; i++)
+		writel_relaxed(drvdata->dsb->msr[i],
+			   drvdata->base + TPDM_DSB_MSR(i));
+}
+
 static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 {
 	u32 val, i;
@@ -186,6 +220,8 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
 	set_dsb_tier(drvdata, &val);
 	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
 
+	set_dsb_msr(drvdata);
+
 	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
 	/* Set the mode of DSB dataset */
 	set_dsb_mode(drvdata, &val);
@@ -707,6 +743,42 @@ static struct attribute *tpdm_dsb_patt_attrs[] = {
 	NULL,
 };
 
+static struct attribute *tpdm_dsb_msr_attrs[] = {
+	DSB_MSR_ATTR(0),
+	DSB_MSR_ATTR(1),
+	DSB_MSR_ATTR(2),
+	DSB_MSR_ATTR(3),
+	DSB_MSR_ATTR(4),
+	DSB_MSR_ATTR(5),
+	DSB_MSR_ATTR(6),
+	DSB_MSR_ATTR(7),
+	DSB_MSR_ATTR(8),
+	DSB_MSR_ATTR(9),
+	DSB_MSR_ATTR(10),
+	DSB_MSR_ATTR(11),
+	DSB_MSR_ATTR(12),
+	DSB_MSR_ATTR(13),
+	DSB_MSR_ATTR(14),
+	DSB_MSR_ATTR(15),
+	DSB_MSR_ATTR(16),
+	DSB_MSR_ATTR(17),
+	DSB_MSR_ATTR(18),
+	DSB_MSR_ATTR(19),
+	DSB_MSR_ATTR(20),
+	DSB_MSR_ATTR(21),
+	DSB_MSR_ATTR(22),
+	DSB_MSR_ATTR(23),
+	DSB_MSR_ATTR(24),
+	DSB_MSR_ATTR(25),
+	DSB_MSR_ATTR(26),
+	DSB_MSR_ATTR(27),
+	DSB_MSR_ATTR(28),
+	DSB_MSR_ATTR(29),
+	DSB_MSR_ATTR(30),
+	DSB_MSR_ATTR(31),
+	NULL,
+};
+
 static struct attribute *tpdm_dsb_attrs[] = {
 	&dev_attr_dsb_mode.attr,
 	&dev_attr_dsb_patt_ts.attr,
@@ -739,12 +811,19 @@ static struct attribute_group tpdm_dsb_patt_grp = {
 	.name = "dsb_patt",
 };
 
+static struct attribute_group tpdm_dsb_msr_grp = {
+	.attrs = tpdm_dsb_msr_attrs,
+	.is_visible = tpdm_dsb_msr_is_visible,
+	.name = "dsb_msr",
+};
+
 static const struct attribute_group *tpdm_attr_grps[] = {
 	&tpdm_attr_grp,
 	&tpdm_dsb_attrs_grp,
 	&tpdm_dsb_edge_grp,
 	&tpdm_dsb_trig_patt_grp,
 	&tpdm_dsb_patt_grp,
+	&tpdm_dsb_msr_grp,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 9173e80..270e946 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)
@@ -90,6 +91,8 @@
 #define TPDM_DSB_MAX_EDCMR	8
 /* MAX number of DSB pattern */
 #define TPDM_DSB_MAX_PATT	8
+/* MAX number of DSB MSR */
+#define TPDM_DSB_MAX_MSR 32
 
 #define tpdm_simple_dataset_ro(name, mem, idx, max)			\
 	(&((struct tpdm_dataset_attribute[]) {			\
@@ -136,16 +139,22 @@
 		tpdm_simple_dataset_rw(tpmr##nr,		\
 		DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
 
+#define DSB_MSR_ATTR(nr)						\
+		tpdm_simple_dataset_rw(msr##nr,			\
+		DSB_MSR, nr, TPDM_DSB_MAX_MSR)
+
 /**
  * struct dsb_dataset - specifics associated to dsb dataset
  * @mode:             DSB programming mode
  * @edge_ctrl_idx     Index number of the edge control
+ * @msr_num           Number of MSR supported by DSB TPDM
  * @edge_ctrl:        Save value for edge control
  * @edge_ctrl_mask:   Save value for edge control mask
  * @patt_val:         Save value for pattern
  * @patt_mask:        Save value for pattern mask
  * @trig_patt:        Save value for trigger pattern
  * @trig_patt_mask:   Save value for trigger pattern mask
+ * @msr               Save value for MSR
  * @patt_ts:          Enable/Disable pattern timestamp
  * @patt_type:        Set pattern type
  * @trig_ts:          Enable/Disable trigger timestamp.
@@ -154,12 +163,14 @@
 struct dsb_dataset {
 	u32				mode;
 	u32				edge_ctrl_idx;
+	u32				msr_num;
 	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
 	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
 	u32				patt_val[TPDM_DSB_MAX_PATT];
 	u32				patt_mask[TPDM_DSB_MAX_PATT];
 	u32				trig_patt[TPDM_DSB_MAX_PATT];
 	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
+	u32				msr[TPDM_DSB_MAX_MSR];
 	bool			patt_ts;
 	bool			patt_type;
 	bool			trig_ts;
@@ -195,6 +206,7 @@ enum dataset_mem {
 	DSB_TRIG_PATT_MASK,
 	DSB_PATT,
 	DSB_PATT_MASK,
+	DSB_MSR,
 };
 
 /**
-- 
2.7.4

Re: [PATCH v8 04/13] coresight-tpda: Add DSB dataset support

[kernel test robot]

Hi Tao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.5-rc7 next-20230822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tao-Zhang/coresight-tpdm-Remove-the-unnecessary-lock/20230822-132946
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/1692681973-20764-5-git-send-email-quic_taozha%40quicinc.com
patch subject: [PATCH v8 04/13] coresight-tpda: Add DSB dataset support
config: arm64-randconfig-r031-20230823 (https://download.01.org/0day-ci/archive/20230823/202308230927.JBX1uKOE-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230927.JBX1uKOE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308230927.JBX1uKOE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwtracing/coresight/coresight-tpda.c:168:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (atomic_read(&in->dest_refcnt) == 0) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-tpda.c:177:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/hwtracing/coresight/coresight-tpda.c:168:2: note: remove the 'if' if its condition is always true
           if (atomic_read(&in->dest_refcnt) == 0) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-tpda.c:165:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +168 drivers/hwtracing/coresight/coresight-tpda.c

   159	
   160	static int tpda_enable(struct coresight_device *csdev,
   161			       struct coresight_connection *in,
   162			       struct coresight_connection *out)
   163	{
   164		struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
   165		int ret;
   166	
   167		spin_lock(&drvdata->spinlock);
 > 168		if (atomic_read(&in->dest_refcnt) == 0) {
   169			ret = __tpda_enable(drvdata, in->dest_port);
   170			if (!ret) {
   171				atomic_inc(&in->dest_refcnt);
   172				dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
   173			}
   174		}
   175	
   176		spin_unlock(&drvdata->spinlock);
   177		return ret;
   178	}
   179	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v8 04/13] coresight-tpda: Add DSB dataset support

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> Read the DSB element size from the device tree. Set the register
> bit that controls the DSB element size of the corresponding port.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   drivers/hwtracing/coresight/coresight-tpda.c | 126 ++++++++++++++++++++++++---
>   drivers/hwtracing/coresight/coresight-tpda.h |   2 +
>   2 files changed, 118 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 8d2b9d2..0f21cd1 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -21,6 +21,80 @@
>   
>   DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>   
> +static bool coresight_device_is_tpdm(struct coresight_device *csdev)
> +{
> +	return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
> +			(csdev->subtype.source_subtype ==

minor nit: Please align

	return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
	       (csdev->subtype.source_subtype ==
> +			CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);


> +}
> +
> +/*
> + * Read the DSB element size from the TPDM device
> + * Returns
> + *    The dsb element size read from the devicetree if available.
> + *    0 - Otherwise, with a warning once.
> + */
> +static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
> +{
> +	int rc = 0;
> +	u8 size = 0;
> +
> +	rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
> +			"qcom,dsb-element-size", &size);
> +	if (rc)
> +		dev_warn_once(&csdev->dev,
> +			"Failed to read TPDM DSB Element size: %d\n", rc);
> +
> +	return size;
> +}
> +
> +/*
> + * Search and read element data size from the TPDM node in
> + * the devicetree. Each input port of TPDA is connected to
> + * a TPDM. Different TPDM supports different types of dataset,
> + * and some may support more than one type of dataset.
> + * Parameter "inport" is used to pass in the input port number
> + * of TPDA, and it is set to -1 in the recursize call.
> + */
> +static int tpda_get_element_size(struct coresight_device *csdev,
> +								 int inport)

nit: Again, please align.

static int tpda_get_element_size(struct coresight_device *csdev,
				 int inport)
{

	
> +{
> +	int dsb_size = -ENOENT;
> +	int i, size;
> +	struct coresight_device *in;
> +
> +	for (i = 0; i < csdev->pdata->nr_inconns; i++) {
> +		in = csdev->pdata->in_conns[i]->src_dev;
> +		if (!in)
> +			continue;
> +
> +		/* Ignore the paths that do not match port */
> +		if (inport > 0 &&
> +			(csdev->pdata->in_conns[i]->dest_port != inport))

Align please :

		if (inport > 0 &&
		    (csdev->pdata->in_conns[i]->dest_port != inport))

> +			continue;
> +
> +		if (coresight_device_is_tpdm(in)) {
> +			size = tpdm_read_dsb_element_size(in);
> +		} else {
> +			/* Recurse down the path */
> +			size = tpda_get_element_size(in, -1);
> +		}
> +
> +		if (size < 0)
> +			return size;
> +
> +		if (dsb_size < 0) {
> +			/* Found a size, save it. */
> +			dsb_size = size;
> +		} else {
> +			/* Found duplicate TPDMs */
> +			return -EEXIST;
> +		}
> +	}
> +
> +	return dsb_size;
> +}
> +
>   /* Settings pre enabling port control register */
>   static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
>   {
> @@ -32,26 +106,55 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
>   	writel_relaxed(val, drvdata->base + TPDA_CR);
>   }
>   
> -static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
> +static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
>   {
>   	u32 val;
> +	int size;
>   
>   	val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
> +	/*
> +	 * Configure aggregator port n DSB data set element size
> +	 * Set the bit to 0 if the size is 32
> +	 * Set the bit to 1 if the size is 64
> +	 */
> +	size = tpda_get_element_size(drvdata->csdev, port);
> +	switch (size) {
> +	case 32:
> +		val &= ~TPDA_Pn_CR_DSBSIZE;
> +		break;
> +	case 64:
> +		val |= TPDA_Pn_CR_DSBSIZE;
> +		break;
> +	case 0:
> +		return -EEXIST;
> +	case -EEXIST:
> +		dev_warn_once(&drvdata->csdev->dev,
> +			"Detected multiple TPDMs on port %d", -EEXIST);
> +		return -EEXIST;
> +	default:
> +		return -EINVAL;
> +	}
> +
>   	/* Enable the port */
>   	val |= TPDA_Pn_CR_ENA;
>   	writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> +
> +	return 0;
>   }
>   
> -static void __tpda_enable(struct tpda_drvdata *drvdata, int port)
> +static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
>   {
> +	int ret;
> +
>   	CS_UNLOCK(drvdata->base);
>   
>   	if (!drvdata->csdev->enable)
>   		tpda_enable_pre_port(drvdata);
>   
> -	tpda_enable_port(drvdata, port);
> -
> +	ret = tpda_enable_port(drvdata, port);
>   	CS_LOCK(drvdata->base);
> +
> +	return ret;
>   }
>   
>   static int tpda_enable(struct coresight_device *csdev,
> @@ -59,16 +162,19 @@ static int tpda_enable(struct coresight_device *csdev,
>   		       struct coresight_connection *out)
>   {
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	int ret;

As reported by the build robot, please initialise this to 0.

>   
>   	spin_lock(&drvdata->spinlock);
> -	if (atomic_read(&in->dest_refcnt) == 0)
> -		__tpda_enable(drvdata, in->dest_port);
> +	if (atomic_read(&in->dest_refcnt) == 0) {
> +		ret = __tpda_enable(drvdata, in->dest_port);
> +		if (!ret) {
> +			atomic_inc(&in->dest_refcnt);
> +			dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
> +		}
> +	}
>   
> -	atomic_inc(&in->dest_refcnt);
>   	spin_unlock(&drvdata->spinlock);
> -
> -	dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
> -	return 0;
> +	return ret;
>   }
>   
>   static void __tpda_disable(struct tpda_drvdata *drvdata, int port)
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
> index 0399678..b3b38fd 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.h
> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
> @@ -10,6 +10,8 @@
>   #define TPDA_Pn_CR(n)		(0x004 + (n * 4))
>   /* Aggregator port enable bit */
>   #define TPDA_Pn_CR_ENA		BIT(0)
> +/* Aggregator port DSB data set element size bit */
> +#define TPDA_Pn_CR_DSBSIZE		BIT(8)
>   
>   #define TPDA_MAX_INPORTS	32
>   

Rest looks fine to me.

Suzuki

Re: [PATCH v8 06/13] coresight-tpdm: Add reset node to TPDM node

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> TPDM device need a node to reset the configurations and status of
> it. This change provides a node to reset the configurations and
> disable the TPDM if it has been enabled.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 10 ++++++++++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 22 ++++++++++++++++++++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 4a58e64..2936226 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -11,3 +11,13 @@ Description:
>   		Accepts only one of the 2 values -  1 or 2.
>   		1 : Generate 64 bits data
>   		2 : Generate 32 bits data
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/reset_dataset
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(Write) Reset the dataset of the tpdm.
> +
> +		Accepts only one value -  1.
> +		1 : Reset the dataset of the tpdm
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 951ad4d..d6e7c8c 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -162,6 +162,27 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
>   	return 0;
>   }
>   
> +static ssize_t reset_dataset_store(struct device *dev,
> +					  struct device_attribute *attr,
> +					  const char *buf,
> +					  size_t size)
> +{
> +	int ret = 0;
> +	unsigned long val;
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret || val != 1)
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	tpdm_reset_datasets(drvdata);
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;
> +}
> +static DEVICE_ATTR_WO(reset_dataset);
> +
>   /*
>    * value 1: 64 bits test data
>    * value 2: 32 bits test data
> @@ -202,6 +223,7 @@ static ssize_t integration_test_store(struct device *dev,
>   static DEVICE_ATTR_WO(integration_test);
>   
>   static struct attribute *tpdm_attrs[] = {
> +	&dev_attr_reset_dataset.attr,

Should this be only visible when DSB data set is present ?

Suzuki

>   	&dev_attr_integration_test.attr,
>   	NULL,
>   };

Re: [PATCH v8 06/13] coresight-tpdm: Add reset node to TPDM node

[Tao Zhang]

On 9/1/2023 4:34 AM, Suzuki K Poulose wrote:
> On 22/08/2023 06:26, Tao Zhang wrote:
>> TPDM device need a node to reset the configurations and status of
>> it. This change provides a node to reset the configurations and
>> disable the TPDM if it has been enabled.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 10 ++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 22 
>> ++++++++++++++++++++++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git 
>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index 4a58e64..2936226 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -11,3 +11,13 @@ Description:
>>           Accepts only one of the 2 values -  1 or 2.
>>           1 : Generate 64 bits data
>>           2 : Generate 32 bits data
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/reset_dataset
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (Write) Reset the dataset of the tpdm.
>> +
>> +        Accepts only one value -  1.
>> +        1 : Reset the dataset of the tpdm
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 951ad4d..d6e7c8c 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -162,6 +162,27 @@ static int tpdm_datasets_setup(struct 
>> tpdm_drvdata *drvdata)
>>       return 0;
>>   }
>>   +static ssize_t reset_dataset_store(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      const char *buf,
>> +                      size_t size)
>> +{
>> +    int ret = 0;
>> +    unsigned long val;
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    ret = kstrtoul(buf, 0, &val);
>> +    if (ret || val != 1)
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    tpdm_reset_datasets(drvdata);
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    return size;
>> +}
>> +static DEVICE_ATTR_WO(reset_dataset);
>> +
>>   /*
>>    * value 1: 64 bits test data
>>    * value 2: 32 bits test data
>> @@ -202,6 +223,7 @@ static ssize_t integration_test_store(struct 
>> device *dev,
>>   static DEVICE_ATTR_WO(integration_test);
>>     static struct attribute *tpdm_attrs[] = {
>> +    &dev_attr_reset_dataset.attr,
>
> Should this be only visible when DSB data set is present ?

This sysfs file doesn't only reset DSB dataset, and it needs to reset 
all the datasets the TPDM supported.

So I think it should not be only visible for DSB TPDM


Best,

Tao

>
> Suzuki
>
>>       &dev_attr_integration_test.attr,
>>       NULL,
>>   };
>

Re: [PATCH v8 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> The nodes are needed to set or show the trigger timestamp and
> trigger type. This change is to add these nodes to achieve these
> function.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 22 +++++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 95 ++++++++++++++++++++++
>   2 files changed, 117 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 2936226..9e26e30 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -21,3 +21,25 @@ Description:
>   
>   		Accepts only one value -  1.
>   		1 : Reset the dataset of the tpdm
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_type
> +Date:		March 2023
> +KernelVersion	6.5

6.7

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the trigger type of the DSB for tpdm.
> +
> +		Accepts only one of the 2 values -  0 or 1.
> +		0 : Set the DSB trigger type to false
> +		1 : Set the DSB trigger type to true
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
> +Date:		March 2023
> +KernelVersion	6.5

Same here

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the trigger timestamp of the DSB for tpdm.
> +
> +		Accepts only one of the 2 values -  0 or 1.
> +		0 : Set the DSB trigger type to false
> +		1 : Set the DSB trigger type to true
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index d6e7c8c..8e11c9b 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -25,6 +25,18 @@ static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
>   	return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
>   }
>   
> +static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
> +					   struct attribute *attr, int n)

minor nit: please align.

static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
				   struct attribute *attr, int n)

I don't know if you have a different setting for tabs in your editor.
Please refer to the coding style document.

> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	if (drvdata && tpdm_has_dsb_dataset(drvdata))
> +		return attr->mode;
> +
> +	return 0;
> +}
> +
>   static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
>   {
>   	if (tpdm_has_dsb_dataset(drvdata)) {
> @@ -232,8 +244,91 @@ static struct attribute_group tpdm_attr_grp = {
>   	.attrs = tpdm_attrs,
>   };
>   
> +static ssize_t dsb_trig_type_show(struct device *dev,
> +		  struct device_attribute *attr, char *buf)

same here.

> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%u\n",
> +			 (unsigned int)drvdata->dsb->trig_type);
> +}
> +
> +/*
> + * Trigger type (boolean):
> + * false - Disable trigger type.
> + * true  - Enable trigger type.
> + */
> +static ssize_t dsb_trig_type_store(struct device *dev,
> +					  struct device_attribute *attr,
> +					  const char *buf,
> +					  size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	if (val)
> +		drvdata->dsb->trig_type = true;
> +	else
> +		drvdata->dsb->trig_type = false;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_trig_type);
> +
> +static ssize_t dsb_trig_ts_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%u\n",
> +			 (unsigned int)drvdata->dsb->trig_ts);
> +}
> +
> +/*
> + * Trigger timestamp (boolean):
> + * false - Disable trigger timestamp.
> + * true  - Enable trigger timestamp.
> + */
> +static ssize_t dsb_trig_ts_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf,
> +				   size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	if (val)
> +		drvdata->dsb->trig_ts = true;
> +	else
> +		drvdata->dsb->trig_ts = false;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_trig_ts);
> +
> +static struct attribute *tpdm_dsb_attrs[] = {
> +	&dev_attr_dsb_trig_ts.attr,
> +	&dev_attr_dsb_trig_type.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group tpdm_dsb_attr_grp = {
> +	.attrs = tpdm_dsb_attrs,
> +	.is_visible = tpdm_dsb_is_visible,
> +};
> +
>   static const struct attribute_group *tpdm_attr_grps[] = {
>   	&tpdm_attr_grp,
> +	&tpdm_dsb_attr_grp,
>   	NULL,
>   };
>   

Rest looks fine to me

Suzuki

Re: [PATCH v8 08/13] coresight-tpdm: Add node to set dsb programming mode

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> Add node to set and show programming mode for TPDM DSB subunit.
> Once the DSB programming mode is set, it will be written to the
> register DSB_CR.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 14 ++++++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 53 ++++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-tpdm.h       | 19 ++++++++
>   3 files changed, 86 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 9e26e30..e17d1b4 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -43,3 +43,17 @@ Description:
>   		Accepts only one of the 2 values -  0 or 1.
>   		0 : Set the DSB trigger type to false
>   		1 : Set the DSB trigger type to true
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_mode
> +Date:		March 2023
> +KernelVersion	6.5

Please fix this to 6.7

Rest looks fine to me

Suzuki


> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the programming mode of the DSB for tpdm.
> +
> +		Accepts the value needs to be greater than 0. What data
> +		bits do is listed below.
> +		Bit[0:1] : Test mode control bit for choosing the inputs.
> +		Bit[3] : Set to 0 for low performance mode.
> +				 Set to 1 for high performance mode.
> +		Bit[4:8] : Select byte lane for high performance mode.
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 8e11c9b..2424eb7 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <linux/amba/bus.h>
> +#include <linux/bitfield.h>
>   #include <linux/bitmap.h>
>   #include <linux/coresight.h>
>   #include <linux/coresight-pmu.h>
> @@ -47,6 +48,27 @@ static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
>   	}
>   }
>   
> +static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
> +{
> +	u32 mode;
> +
> +	/* Set the test accurate mode */
> +	mode = TPDM_DSB_MODE_TEST(drvdata->dsb->mode);
> +	*val &= ~TPDM_DSB_CR_TEST_MODE;
> +	*val |= FIELD_PREP(TPDM_DSB_CR_TEST_MODE, mode);
> +
> +	/* Set the byte lane for high-performance mode */
> +	mode = TPDM_DSB_MODE_HPBYTESEL(drvdata->dsb->mode);
> +	*val &= ~TPDM_DSB_CR_HPSEL;
> +	*val |= FIELD_PREP(TPDM_DSB_CR_HPSEL, mode);
> +
> +	/* Set the performance mode */
> +	if (drvdata->dsb->mode & TPDM_DSB_MODE_PERF)
> +		*val |= TPDM_DSB_CR_MODE;
> +	else
> +		*val &= ~TPDM_DSB_CR_MODE;
> +}
> +
>   static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   {
>   	u32 val;
> @@ -60,6 +82,8 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
>   
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
> +	/* Set the mode of DSB dataset */
> +	set_dsb_mode(drvdata, &val);
>   	/* Set trigger type */
>   	if (drvdata->dsb->trig_type)
>   		val |= TPDM_DSB_CR_TRIG_TYPE;
> @@ -244,6 +268,34 @@ static struct attribute_group tpdm_attr_grp = {
>   	.attrs = tpdm_attrs,
>   };
>   
> +static ssize_t dsb_mode_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%x\n", drvdata->dsb->mode);
> +}
> +
> +static ssize_t dsb_mode_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf,
> +				size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val < 0) ||
> +			(val & ~TPDM_DSB_MODE_MASK))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->dsb->mode = val & TPDM_DSB_MODE_MASK;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_mode);
> +
>   static ssize_t dsb_trig_type_show(struct device *dev,
>   		  struct device_attribute *attr, char *buf)
>   {
> @@ -316,6 +368,7 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
>   static DEVICE_ATTR_RW(dsb_trig_ts);
>   
>   static struct attribute *tpdm_dsb_attrs[] = {
> +	&dev_attr_dsb_mode.attr,
>   	&dev_attr_dsb_trig_ts.attr,
>   	&dev_attr_dsb_trig_type.attr,
>   	NULL,
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index f59e751..f57c9fe 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -15,11 +15,28 @@
>   
>   /* Enable bit for DSB subunit */
>   #define TPDM_DSB_CR_ENA		BIT(0)
> +/* Enable bit for DSB subunit perfmance mode */
> +#define TPDM_DSB_CR_MODE		BIT(1)
>   /* Enable bit for DSB subunit trigger type */
>   #define TPDM_DSB_CR_TRIG_TYPE		BIT(12)
> +/* Data bits for DSB high performace mode */
> +#define TPDM_DSB_CR_HPSEL		GENMASK(6, 2)
> +/* Data bits for DSB test mode */
> +#define TPDM_DSB_CR_TEST_MODE		GENMASK(10, 9)
> +
>   /* Enable bit for DSB subunit trigger timestamp */
>   #define TPDM_DSB_TIER_XTRIG_TSENAB		BIT(1)
>   
> +/* DSB programming modes */
> +/* DSB mode bits mask */
> +#define TPDM_DSB_MODE_MASK			GENMASK(8, 0)
> +/* Test mode control bit*/
> +#define TPDM_DSB_MODE_TEST(val)	(val & GENMASK(1, 0))
> +/* Performance mode */
> +#define TPDM_DSB_MODE_PERF		BIT(3)
> +/* High performance mode */
> +#define TPDM_DSB_MODE_HPBYTESEL(val)	(val & GENMASK(8, 4))
> +
>   /* TPDM integration test registers */
>   #define TPDM_ITATBCNTRL		(0xEF0)
>   #define TPDM_ITCNTRL		(0xF00)
> @@ -48,10 +65,12 @@
>   
>   /**
>    * struct dsb_dataset - specifics associated to dsb dataset
> + * @mode:             DSB programming mode
>    * @trig_ts:          Enable/Disable trigger timestamp.
>    * @trig_type:        Enable/Disable trigger type.
>    */
>   struct dsb_dataset {
> +	u32				mode;
>   	bool			trig_ts;
>   	bool			trig_type;
>   };

Re: [PATCH v8 09/13] coresight-tpdm: Add nodes for dsb edge control

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> Add the nodes to set value for DSB edge control and DSB edge
> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
> resgisters to configure edge control. DSB edge detection control
> 00: Rising edge detection
> 01: Falling edge detection
> 10: Rising and falling edge detection (toggle detection)
> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
> configure mask. Eight 32 bit registers providing DSB interface
> edge detection mask control.
> 
> Add the nodes to configure DSB edge control and DSB edge control
> mask. Each DSB subunit TPDM maximum of 256 edge detections can be
> configured. The index and value sysfs files need to be paired and
> written to order. The index sysfs file is to set the index number
> of the edge detection which needs to be configured. And the value
> sysfs file is to set the control or mask for the edge detection.
> DSB edge detection control should be set as the following values.
> 00: Rising edge detection
> 01: Falling edge detection
> 10: Rising and falling edge detection (toggle detection)
> And DSB edge mask should be set as 0 or 1.
> Each DSB subunit TPDM has maximum of n(n<16) EDCR resgisters to
> configure edge control. And each DSB subunit TPDM has maximum of
> m(m<8) ECDMR registers to configure mask.
> 
> Add the nodes to read a set of the edge control value and mask
> of the DSB in TPDM.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  51 ++++++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 177 ++++++++++++++++++++-
>   drivers/hwtracing/coresight/coresight-tpdm.h       |  63 ++++++++
>   3 files changed, 288 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index e17d1b4..097fdc4 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -57,3 +57,54 @@ Description:
>   		Bit[3] : Set to 0 for low performance mode.
>   				 Set to 1 for high performance mode.
>   		Bit[4:8] : Select byte lane for high performance mode.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_idx
> +Date:		March 2023
> +KernelVersion	6.5

s/6.5/6.7

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the index number of the edge detection for the DSB
> +		subunit TPDM. Since there are at most 256 edge detections, this
> +		value ranges from 0 to 255.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_val
> +Date:		March 2023
> +KernelVersion	6.5

same as above

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		Write a data to control the edge detection corresponding to
> +		the index number. Before writing data to this sysfs file,
> +		"ctrl_idx" should be written first to configure the index
> +		number of the edge detection which needs to be controlled.
> +
> +		Accepts only one of the following values.
> +		0 - Rising edge detection
> +		1 - Falling edge detection
> +		2 - Rising and falling edge detection (toggle detection)
> +
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_mask
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		Write a data to mask the edge detection corresponding to the index
> +		number. Before writing data to this sysfs file, "ctrl_idx" should
> +		be written first to configure the index number of the edge detection
> +		which needs to be masked.
> +
> +		Accepts only one of the 2 values -  0 or 1.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcr[0:15]
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		Read a set of the edge control value of the DSB in TPDM.

Read edge control register n for edcr<n>.

> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcmr[0:7]
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		Read a set of the edge control mask of the DSB in TPDM.
> \ No newline at end of file
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 2424eb7..ba61e6a 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -21,6 +21,29 @@
>   
>   DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>   
> +/* Read dataset array member with the index number */
> +static ssize_t tpdm_simple_dataset_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	struct tpdm_dataset_attribute *tpdm_attr =
> +		container_of(attr, struct tpdm_dataset_attribute, attr);
> +
> +	if (tpdm_attr->idx >= tpdm_attr->max)

minor nit: See my comment on max below. We could skip max.

> +		return -EINVAL;
> +
> +	switch (tpdm_attr->mem) {
> +	case DSB_EDGE_CTRL:
		if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
			break;

> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->edge_ctrl[tpdm_attr->idx]);
> +	case DSB_EDGE_CTRL_MASK:
		if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
			break;

> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
	}

	return -EINVAL;


> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>   static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
>   {
>   	return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
> @@ -71,7 +94,14 @@ static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
>   
>   static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   {
> -	u32 val;
> +	u32 val, i;
> +
> +	for (i = 0; i < TPDM_DSB_MAX_EDCR; i++)
> +		writel_relaxed(drvdata->dsb->edge_ctrl[i],
> +			   drvdata->base + TPDM_DSB_EDCR(i));
> +	for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
> +		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
> +			   drvdata->base + TPDM_DSB_EDCMR(i));
>   
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
>   	/* Set trigger timestamp */
> @@ -296,6 +326,109 @@ static ssize_t dsb_mode_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(dsb_mode);
>   
> +static ssize_t ctrl_idx_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%u\n",
> +			(unsigned int)drvdata->dsb->edge_ctrl_idx);
> +}
> +
> +/*
> + * The EDCR registers can include up to 16 32-bit registers, and each
> + * one can be configured to control up to 16 edge detections(2 bits
> + * control one edge detection). So a total 256 edge detections can be
> + * configured. This function provides a way to set the index number of
> + * the edge detection which needs to be configured.
> + */
> +static ssize_t ctrl_idx_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf,
> +				size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val >= TPDM_DSB_MAX_LINES))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->dsb->edge_ctrl_idx = val;
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;
> +}
> +static DEVICE_ATTR_RW(ctrl_idx);
> +
> +/*
> + * This function is used to control the edge detection according
> + * to the index number that has been set.
> + * "edge_ctrl" should be one of the following values.
> + * 0 - Rising edge detection
> + * 1 - Falling edge detection
> + * 2 - Rising and falling edge detection (toggle detection)
> + */
> +static ssize_t ctrl_val_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf,
> +				size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val, edge_ctrl;
> +	int reg;
> +
> +	if ((kstrtoul(buf, 0, &edge_ctrl)) || (edge_ctrl > 0x2))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	/*
> +	 * There are 2 bit per DSB Edge Control line.
> +	 * Thus we have 16 lines in a 32bit word.
> +	 */
> +	reg = EDCR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
> +	val = drvdata->dsb->edge_ctrl[reg];
> +	val &= ~EDCR_TO_WORD_MASK(drvdata->dsb->edge_ctrl_idx);
> +	val |= EDCR_TO_WORD_VAL(edge_ctrl, drvdata->dsb->edge_ctrl_idx);
> +	drvdata->dsb->edge_ctrl[reg] = val;
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;
> +}
> +static DEVICE_ATTR_WO(ctrl_val);
> +
> +static ssize_t ctrl_mask_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf,
> +				  size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +	u32 set;
> +	int reg;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	/*
> +	 * There is 1 bit per DSB Edge Control Mark line.
> +	 * Thus we have 32 lines in a 32bit word.
> +	 */
> +	reg = EDCMR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
> +	set = drvdata->dsb->edge_ctrl_mask[reg];
> +	if (val)
> +		set |= BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
> +	else
> +		set &= ~BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
> +	drvdata->dsb->edge_ctrl_mask[reg] = set;
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;
> +}
> +static DEVICE_ATTR_WO(ctrl_mask);
> +
>   static ssize_t dsb_trig_type_show(struct device *dev,
>   		  struct device_attribute *attr, char *buf)
>   {
> @@ -367,6 +500,37 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(dsb_trig_ts);
>   
> +static struct attribute *tpdm_dsb_edge_attrs[] = {
> +	&dev_attr_ctrl_idx.attr,
> +	&dev_attr_ctrl_val.attr,
> +	&dev_attr_ctrl_mask.attr,
> +	DSB_EDGE_CTRL_ATTR(0),
> +	DSB_EDGE_CTRL_ATTR(1),
> +	DSB_EDGE_CTRL_ATTR(2),
> +	DSB_EDGE_CTRL_ATTR(3),
> +	DSB_EDGE_CTRL_ATTR(4),
> +	DSB_EDGE_CTRL_ATTR(5),
> +	DSB_EDGE_CTRL_ATTR(6),
> +	DSB_EDGE_CTRL_ATTR(7),
> +	DSB_EDGE_CTRL_ATTR(8),
> +	DSB_EDGE_CTRL_ATTR(9),
> +	DSB_EDGE_CTRL_ATTR(10),
> +	DSB_EDGE_CTRL_ATTR(11),
> +	DSB_EDGE_CTRL_ATTR(12),
> +	DSB_EDGE_CTRL_ATTR(13),
> +	DSB_EDGE_CTRL_ATTR(14),
> +	DSB_EDGE_CTRL_ATTR(15),
> +	DSB_EDGE_CTRL_MASK_ATTR(0),
> +	DSB_EDGE_CTRL_MASK_ATTR(1),
> +	DSB_EDGE_CTRL_MASK_ATTR(2),
> +	DSB_EDGE_CTRL_MASK_ATTR(3),
> +	DSB_EDGE_CTRL_MASK_ATTR(4),
> +	DSB_EDGE_CTRL_MASK_ATTR(5),
> +	DSB_EDGE_CTRL_MASK_ATTR(6),
> +	DSB_EDGE_CTRL_MASK_ATTR(7),
> +	NULL,
> +};
> +
>   static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_mode.attr,
>   	&dev_attr_dsb_trig_ts.attr,
> @@ -374,14 +538,21 @@ static struct attribute *tpdm_dsb_attrs[] = {
>   	NULL,
>   };
>   
> -static struct attribute_group tpdm_dsb_attr_grp = {
> +static struct attribute_group tpdm_dsb_attrs_grp = {
>   	.attrs = tpdm_dsb_attrs,
>   	.is_visible = tpdm_dsb_is_visible,
>   };
>   
> +static struct attribute_group tpdm_dsb_edge_grp = {
> +	.attrs = tpdm_dsb_edge_attrs,
> +	.is_visible = tpdm_dsb_is_visible,
> +	.name = "dsb_edge",
> +};
> +
>   static const struct attribute_group *tpdm_attr_grps[] = {
>   	&tpdm_attr_grp,
> -	&tpdm_dsb_attr_grp,
> +	&tpdm_dsb_attrs_grp,
> +	&tpdm_dsb_edge_grp,
>   	NULL,
>   };
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index f57c9fe..9736e00 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -12,6 +12,8 @@
>   /* DSB Subunit Registers */
>   #define TPDM_DSB_CR		(0x780)
>   #define TPDM_DSB_TIER		(0x784)
> +#define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
> +#define TPDM_DSB_EDCMR(n)	(0x848 + (n * 4))
>   
>   /* Enable bit for DSB subunit */
>   #define TPDM_DSB_CR_ENA		BIT(0)
> @@ -37,6 +39,16 @@
>   /* High performance mode */
>   #define TPDM_DSB_MODE_HPBYTESEL(val)	(val & GENMASK(8, 4))
>   
> +#define EDCRS_PER_WORD				16
> +#define EDCR_TO_WORD_IDX(r)			((r) / EDCRS_PER_WORD)
> +#define EDCR_TO_WORD_SHIFT(r)		((r % EDCRS_PER_WORD) * 2)
> +#define EDCR_TO_WORD_VAL(val, r)	(val << EDCR_TO_WORD_SHIFT(r))
> +#define EDCR_TO_WORD_MASK(r)		EDCR_TO_WORD_VAL(0x3, r)
> +
> +#define EDCMRS_PER_WORD				32
> +#define EDCMR_TO_WORD_IDX(r)		((r) / EDCMRS_PER_WORD)
> +#define EDCMR_TO_WORD_SHIFT(r)		((r) % EDCMRS_PER_WORD)
> +
>   /* TPDM integration test registers */
>   #define TPDM_ITATBCNTRL		(0xEF0)
>   #define TPDM_ITCNTRL		(0xF00)
> @@ -63,14 +75,44 @@
>   #define TPDM_PIDR0_DS_IMPDEF	BIT(0)
>   #define TPDM_PIDR0_DS_DSB	BIT(1)
>   
> +#define TPDM_DSB_MAX_LINES	256
> +/* MAX number of EDCR registers */
> +#define TPDM_DSB_MAX_EDCR	16
> +/* MAX number of EDCMR registers */
> +#define TPDM_DSB_MAX_EDCMR	8
> +
> +#define tpdm_simple_dataset_ro(name, mem, idx, max)			\

Do we need max ? The mem itself is enough to help you
choose the max ? See my comment above.

> +	(&((struct tpdm_dataset_attribute[]) {			\
> +	   {								\
> +		__ATTR(name, 0444, tpdm_simple_dataset_show, NULL),	\
> +		mem,							\
> +		idx,							\
> +		max,							\
> +	   }								\
> +	})[0].attr.attr)
> +
> +#define DSB_EDGE_CTRL_ATTR(nr)					\
> +		tpdm_simple_dataset_ro(edcr##nr,		\
> +		DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
> +
> +#define DSB_EDGE_CTRL_MASK_ATTR(nr)				\
> +		tpdm_simple_dataset_ro(edcmr##nr,		\
> +		DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
> +
>   /**
>    * struct dsb_dataset - specifics associated to dsb dataset
>    * @mode:             DSB programming mode
> + * @edge_ctrl_idx     Index number of the edge control
> + * @edge_ctrl:        Save value for edge control
> + * @edge_ctrl_mask:   Save value for edge control mask
>    * @trig_ts:          Enable/Disable trigger timestamp.
>    * @trig_type:        Enable/Disable trigger type.
>    */
>   struct dsb_dataset {
>   	u32				mode;
> +	u32				edge_ctrl_idx;
> +	u32				edge_ctrl[TPDM_DSB_MAX_EDCR] > +	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
>   	bool			trig_ts;
>   	bool			trig_type;
>   };
> @@ -96,4 +138,25 @@ struct tpdm_drvdata {
>   	struct dsb_dataset	*dsb;
>   };
>   
> +/* Enumerate members of various datasets */
> +enum dataset_mem {
> +	DSB_EDGE_CTRL,
> +	DSB_EDGE_CTRL_MASK,
> +};
> +
> +/**
> + * struct tpdm_dataset_attribute - Record the member variables and
> + * index number of datasets that need to be operated by sysfs file
> + * @attr:       The device attribute
> + * @mem:        The member in the dataset data structure
> + * @idx:        The index number of the array data
> + * @max:        The maximal number of the array data

As mentioned above, you could remove this.

Suzuki

> + */
> +struct tpdm_dataset_attribute {
> +	struct device_attribute attr;
> +	enum dataset_mem mem;
> +	u32 idx;
> +	u32 max;
> +};
> +
>   #endif  /* _CORESIGHT_CORESIGHT_TPDM_H */

Re: [PATCH v8 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

[Tao Zhang]

On 9/1/2023 6:43 PM, Suzuki K Poulose wrote:
> On 22/08/2023 06:26, Tao Zhang wrote:
>> The nodes are needed to set or show the trigger timestamp and
>> trigger type. This change is to add these nodes to achieve these
>> function.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 22 +++++
>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 95 
>> ++++++++++++++++++++++
>>   2 files changed, 117 insertions(+)
>>
>> diff --git 
>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index 2936226..9e26e30 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -21,3 +21,25 @@ Description:
>>             Accepts only one value -  1.
>>           1 : Reset the dataset of the tpdm
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_type
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> 6.7
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the trigger type of the DSB for tpdm.
>> +
>> +        Accepts only one of the 2 values -  0 or 1.
>> +        0 : Set the DSB trigger type to false
>> +        1 : Set the DSB trigger type to true
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> Same here
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the trigger timestamp of the DSB for tpdm.
>> +
>> +        Accepts only one of the 2 values -  0 or 1.
>> +        0 : Set the DSB trigger type to false
>> +        1 : Set the DSB trigger type to true
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index d6e7c8c..8e11c9b 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -25,6 +25,18 @@ static bool tpdm_has_dsb_dataset(struct 
>> tpdm_drvdata *drvdata)
>>       return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
>>   }
>>   +static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
>> +                       struct attribute *attr, int n)
>
> minor nit: please align.
>
> static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
>                    struct attribute *attr, int n)
>
> I don't know if you have a different setting for tabs in your editor.
> Please refer to the coding style document.

Tab size is set to 4 in my editor.

There are 5 tabs and 3 spaces at the beginning of this line.

I don't know if this is the same as what you see in this patch.

I see from the editor that the code meets the requirements of the coding 
style document.

If the gap in alignment is not resolved, such alignment problems may 
still occur.


Best,

Tao

>
>> +{
>> +    struct device *dev = kobj_to_dev(kobj);
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    if (drvdata && tpdm_has_dsb_dataset(drvdata))
>> +        return attr->mode;
>> +
>> +    return 0;
>> +}
>> +
>>   static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
>>   {
>>       if (tpdm_has_dsb_dataset(drvdata)) {
>> @@ -232,8 +244,91 @@ static struct attribute_group tpdm_attr_grp = {
>>       .attrs = tpdm_attrs,
>>   };
>>   +static ssize_t dsb_trig_type_show(struct device *dev,
>> +          struct device_attribute *attr, char *buf)
>
> same here.
>
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    return sysfs_emit(buf, "%u\n",
>> +             (unsigned int)drvdata->dsb->trig_type);
>> +}
>> +
>> +/*
>> + * Trigger type (boolean):
>> + * false - Disable trigger type.
>> + * true  - Enable trigger type.
>> + */
>> +static ssize_t dsb_trig_type_store(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      const char *buf,
>> +                      size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    if (val)
>> +        drvdata->dsb->trig_type = true;
>> +    else
>> +        drvdata->dsb->trig_type = false;
>> +    spin_unlock(&drvdata->spinlock);
>> +    return size;
>> +}
>> +static DEVICE_ATTR_RW(dsb_trig_type);
>> +
>> +static ssize_t dsb_trig_ts_show(struct device *dev,
>> +                    struct device_attribute *attr,
>> +                    char *buf)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    return sysfs_emit(buf, "%u\n",
>> +             (unsigned int)drvdata->dsb->trig_ts);
>> +}
>> +
>> +/*
>> + * Trigger timestamp (boolean):
>> + * false - Disable trigger timestamp.
>> + * true  - Enable trigger timestamp.
>> + */
>> +static ssize_t dsb_trig_ts_store(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   const char *buf,
>> +                   size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    if (val)
>> +        drvdata->dsb->trig_ts = true;
>> +    else
>> +        drvdata->dsb->trig_ts = false;
>> +    spin_unlock(&drvdata->spinlock);
>> +    return size;
>> +}
>> +static DEVICE_ATTR_RW(dsb_trig_ts);
>> +
>> +static struct attribute *tpdm_dsb_attrs[] = {
>> +    &dev_attr_dsb_trig_ts.attr,
>> +    &dev_attr_dsb_trig_type.attr,
>> +    NULL,
>> +};
>> +
>> +static struct attribute_group tpdm_dsb_attr_grp = {
>> +    .attrs = tpdm_dsb_attrs,
>> +    .is_visible = tpdm_dsb_is_visible,
>> +};
>> +
>>   static const struct attribute_group *tpdm_attr_grps[] = {
>>       &tpdm_attr_grp,
>> +    &tpdm_dsb_attr_grp,
>>       NULL,
>>   };
>
> Rest looks fine to me
>
> Suzuki
>

Re: [PATCH v8 09/13] coresight-tpdm: Add nodes for dsb edge control

[Tao Zhang]

On 9/1/2023 10:07 PM, Suzuki K Poulose wrote:
> On 22/08/2023 06:26, Tao Zhang wrote:
>> Add the nodes to set value for DSB edge control and DSB edge
>> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
>> resgisters to configure edge control. DSB edge detection control
>> 00: Rising edge detection
>> 01: Falling edge detection
>> 10: Rising and falling edge detection (toggle detection)
>> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
>> configure mask. Eight 32 bit registers providing DSB interface
>> edge detection mask control.
>>
>> Add the nodes to configure DSB edge control and DSB edge control
>> mask. Each DSB subunit TPDM maximum of 256 edge detections can be
>> configured. The index and value sysfs files need to be paired and
>> written to order. The index sysfs file is to set the index number
>> of the edge detection which needs to be configured. And the value
>> sysfs file is to set the control or mask for the edge detection.
>> DSB edge detection control should be set as the following values.
>> 00: Rising edge detection
>> 01: Falling edge detection
>> 10: Rising and falling edge detection (toggle detection)
>> And DSB edge mask should be set as 0 or 1.
>> Each DSB subunit TPDM has maximum of n(n<16) EDCR resgisters to
>> configure edge control. And each DSB subunit TPDM has maximum of
>> m(m<8) ECDMR registers to configure mask.
>>
>> Add the nodes to read a set of the edge control value and mask
>> of the DSB in TPDM.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  51 ++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 177 
>> ++++++++++++++++++++-
>>   drivers/hwtracing/coresight/coresight-tpdm.h       |  63 ++++++++
>>   3 files changed, 288 insertions(+), 3 deletions(-)
>>
>> diff --git 
>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index e17d1b4..097fdc4 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -57,3 +57,54 @@ Description:
>>           Bit[3] : Set to 0 for low performance mode.
>>                    Set to 1 for high performance mode.
>>           Bit[4:8] : Select byte lane for high performance mode.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_idx
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> s/6.5/6.7
Sure, I will update this in the next patch series.
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the index number of the edge detection for the DSB
>> +        subunit TPDM. Since there are at most 256 edge detections, this
>> +        value ranges from 0 to 255.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_val
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> same as above
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        Write a data to control the edge detection corresponding to
>> +        the index number. Before writing data to this sysfs file,
>> +        "ctrl_idx" should be written first to configure the index
>> +        number of the edge detection which needs to be controlled.
>> +
>> +        Accepts only one of the following values.
>> +        0 - Rising edge detection
>> +        1 - Falling edge detection
>> +        2 - Rising and falling edge detection (toggle detection)
>> +
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_mask
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        Write a data to mask the edge detection corresponding to the 
>> index
>> +        number. Before writing data to this sysfs file, "ctrl_idx" 
>> should
>> +        be written first to configure the index number of the edge 
>> detection
>> +        which needs to be masked.
>> +
>> +        Accepts only one of the 2 values -  0 or 1.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcr[0:15]
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        Read a set of the edge control value of the DSB in TPDM.
>
> Read edge control register n for edcr<n>.

In fact, we don't read the register directly through this sysfs file, 
but read the value

we set to be written to the registers. Do I still need to modify it here?

>
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcmr[0:7]
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        Read a set of the edge control mask of the DSB in TPDM.
>> \ No newline at end of file
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 2424eb7..ba61e6a 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -21,6 +21,29 @@
>>     DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>>   +/* Read dataset array member with the index number */
>> +static ssize_t tpdm_simple_dataset_show(struct device *dev,
>> +               struct device_attribute *attr, char *buf)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    struct tpdm_dataset_attribute *tpdm_attr =
>> +        container_of(attr, struct tpdm_dataset_attribute, attr);
>> +
>> +    if (tpdm_attr->idx >= tpdm_attr->max)
>
> minor nit: See my comment on max below. We could skip max.
I will update this in the next patch series.
>
>> +        return -EINVAL;
>> +
>> +    switch (tpdm_attr->mem) {
>> +    case DSB_EDGE_CTRL:
>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
>             break;
>
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->edge_ctrl[tpdm_attr->idx]);
>> +    case DSB_EDGE_CTRL_MASK:
>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
>             break;
>
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
>     }
>
>     return -EINVAL;
Why do we need to return this error code here?
>
>
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +}
>> +
>>   static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
>>   {
>>       return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
>> @@ -71,7 +94,14 @@ static void set_dsb_mode(struct tpdm_drvdata 
>> *drvdata, u32 *val)
>>     static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>>   {
>> -    u32 val;
>> +    u32 val, i;
>> +
>> +    for (i = 0; i < TPDM_DSB_MAX_EDCR; i++)
>> +        writel_relaxed(drvdata->dsb->edge_ctrl[i],
>> +               drvdata->base + TPDM_DSB_EDCR(i));
>> +    for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
>> +        writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
>> +               drvdata->base + TPDM_DSB_EDCMR(i));
>>         val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
>>       /* Set trigger timestamp */
>> @@ -296,6 +326,109 @@ static ssize_t dsb_mode_store(struct device *dev,
>>   }
>>   static DEVICE_ATTR_RW(dsb_mode);
>>   +static ssize_t ctrl_idx_show(struct device *dev,
>> +                struct device_attribute *attr,
>> +                char *buf)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    return sysfs_emit(buf, "%u\n",
>> +            (unsigned int)drvdata->dsb->edge_ctrl_idx);
>> +}
>> +
>> +/*
>> + * The EDCR registers can include up to 16 32-bit registers, and each
>> + * one can be configured to control up to 16 edge detections(2 bits
>> + * control one edge detection). So a total 256 edge detections can be
>> + * configured. This function provides a way to set the index number of
>> + * the edge detection which needs to be configured.
>> + */
>> +static ssize_t ctrl_idx_store(struct device *dev,
>> +                struct device_attribute *attr,
>> +                const char *buf,
>> +                size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val >= TPDM_DSB_MAX_LINES))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    drvdata->dsb->edge_ctrl_idx = val;
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    return size;
>> +}
>> +static DEVICE_ATTR_RW(ctrl_idx);
>> +
>> +/*
>> + * This function is used to control the edge detection according
>> + * to the index number that has been set.
>> + * "edge_ctrl" should be one of the following values.
>> + * 0 - Rising edge detection
>> + * 1 - Falling edge detection
>> + * 2 - Rising and falling edge detection (toggle detection)
>> + */
>> +static ssize_t ctrl_val_store(struct device *dev,
>> +                struct device_attribute *attr,
>> +                const char *buf,
>> +                size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val, edge_ctrl;
>> +    int reg;
>> +
>> +    if ((kstrtoul(buf, 0, &edge_ctrl)) || (edge_ctrl > 0x2))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    /*
>> +     * There are 2 bit per DSB Edge Control line.
>> +     * Thus we have 16 lines in a 32bit word.
>> +     */
>> +    reg = EDCR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
>> +    val = drvdata->dsb->edge_ctrl[reg];
>> +    val &= ~EDCR_TO_WORD_MASK(drvdata->dsb->edge_ctrl_idx);
>> +    val |= EDCR_TO_WORD_VAL(edge_ctrl, drvdata->dsb->edge_ctrl_idx);
>> +    drvdata->dsb->edge_ctrl[reg] = val;
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    return size;
>> +}
>> +static DEVICE_ATTR_WO(ctrl_val);
>> +
>> +static ssize_t ctrl_mask_store(struct device *dev,
>> +                  struct device_attribute *attr,
>> +                  const char *buf,
>> +                  size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +    u32 set;
>> +    int reg;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    /*
>> +     * There is 1 bit per DSB Edge Control Mark line.
>> +     * Thus we have 32 lines in a 32bit word.
>> +     */
>> +    reg = EDCMR_TO_WORD_IDX(drvdata->dsb->edge_ctrl_idx);
>> +    set = drvdata->dsb->edge_ctrl_mask[reg];
>> +    if (val)
>> +        set |= BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
>> +    else
>> +        set &= ~BIT(EDCMR_TO_WORD_SHIFT(drvdata->dsb->edge_ctrl_idx));
>> +    drvdata->dsb->edge_ctrl_mask[reg] = set;
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    return size;
>> +}
>> +static DEVICE_ATTR_WO(ctrl_mask);
>> +
>>   static ssize_t dsb_trig_type_show(struct device *dev,
>>             struct device_attribute *attr, char *buf)
>>   {
>> @@ -367,6 +500,37 @@ static ssize_t dsb_trig_ts_store(struct device 
>> *dev,
>>   }
>>   static DEVICE_ATTR_RW(dsb_trig_ts);
>>   +static struct attribute *tpdm_dsb_edge_attrs[] = {
>> +    &dev_attr_ctrl_idx.attr,
>> +    &dev_attr_ctrl_val.attr,
>> +    &dev_attr_ctrl_mask.attr,
>> +    DSB_EDGE_CTRL_ATTR(0),
>> +    DSB_EDGE_CTRL_ATTR(1),
>> +    DSB_EDGE_CTRL_ATTR(2),
>> +    DSB_EDGE_CTRL_ATTR(3),
>> +    DSB_EDGE_CTRL_ATTR(4),
>> +    DSB_EDGE_CTRL_ATTR(5),
>> +    DSB_EDGE_CTRL_ATTR(6),
>> +    DSB_EDGE_CTRL_ATTR(7),
>> +    DSB_EDGE_CTRL_ATTR(8),
>> +    DSB_EDGE_CTRL_ATTR(9),
>> +    DSB_EDGE_CTRL_ATTR(10),
>> +    DSB_EDGE_CTRL_ATTR(11),
>> +    DSB_EDGE_CTRL_ATTR(12),
>> +    DSB_EDGE_CTRL_ATTR(13),
>> +    DSB_EDGE_CTRL_ATTR(14),
>> +    DSB_EDGE_CTRL_ATTR(15),
>> +    DSB_EDGE_CTRL_MASK_ATTR(0),
>> +    DSB_EDGE_CTRL_MASK_ATTR(1),
>> +    DSB_EDGE_CTRL_MASK_ATTR(2),
>> +    DSB_EDGE_CTRL_MASK_ATTR(3),
>> +    DSB_EDGE_CTRL_MASK_ATTR(4),
>> +    DSB_EDGE_CTRL_MASK_ATTR(5),
>> +    DSB_EDGE_CTRL_MASK_ATTR(6),
>> +    DSB_EDGE_CTRL_MASK_ATTR(7),
>> +    NULL,
>> +};
>> +
>>   static struct attribute *tpdm_dsb_attrs[] = {
>>       &dev_attr_dsb_mode.attr,
>>       &dev_attr_dsb_trig_ts.attr,
>> @@ -374,14 +538,21 @@ static struct attribute *tpdm_dsb_attrs[] = {
>>       NULL,
>>   };
>>   -static struct attribute_group tpdm_dsb_attr_grp = {
>> +static struct attribute_group tpdm_dsb_attrs_grp = {
>>       .attrs = tpdm_dsb_attrs,
>>       .is_visible = tpdm_dsb_is_visible,
>>   };
>>   +static struct attribute_group tpdm_dsb_edge_grp = {
>> +    .attrs = tpdm_dsb_edge_attrs,
>> +    .is_visible = tpdm_dsb_is_visible,
>> +    .name = "dsb_edge",
>> +};
>> +
>>   static const struct attribute_group *tpdm_attr_grps[] = {
>>       &tpdm_attr_grp,
>> -    &tpdm_dsb_attr_grp,
>> +    &tpdm_dsb_attrs_grp,
>> +    &tpdm_dsb_edge_grp,
>>       NULL,
>>   };
>>   diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h 
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index f57c9fe..9736e00 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -12,6 +12,8 @@
>>   /* DSB Subunit Registers */
>>   #define TPDM_DSB_CR        (0x780)
>>   #define TPDM_DSB_TIER        (0x784)
>> +#define TPDM_DSB_EDCR(n)    (0x808 + (n * 4))
>> +#define TPDM_DSB_EDCMR(n)    (0x848 + (n * 4))
>>     /* Enable bit for DSB subunit */
>>   #define TPDM_DSB_CR_ENA        BIT(0)
>> @@ -37,6 +39,16 @@
>>   /* High performance mode */
>>   #define TPDM_DSB_MODE_HPBYTESEL(val)    (val & GENMASK(8, 4))
>>   +#define EDCRS_PER_WORD                16
>> +#define EDCR_TO_WORD_IDX(r)            ((r) / EDCRS_PER_WORD)
>> +#define EDCR_TO_WORD_SHIFT(r)        ((r % EDCRS_PER_WORD) * 2)
>> +#define EDCR_TO_WORD_VAL(val, r)    (val << EDCR_TO_WORD_SHIFT(r))
>> +#define EDCR_TO_WORD_MASK(r)        EDCR_TO_WORD_VAL(0x3, r)
>> +
>> +#define EDCMRS_PER_WORD                32
>> +#define EDCMR_TO_WORD_IDX(r)        ((r) / EDCMRS_PER_WORD)
>> +#define EDCMR_TO_WORD_SHIFT(r)        ((r) % EDCMRS_PER_WORD)
>> +
>>   /* TPDM integration test registers */
>>   #define TPDM_ITATBCNTRL        (0xEF0)
>>   #define TPDM_ITCNTRL        (0xF00)
>> @@ -63,14 +75,44 @@
>>   #define TPDM_PIDR0_DS_IMPDEF    BIT(0)
>>   #define TPDM_PIDR0_DS_DSB    BIT(1)
>>   +#define TPDM_DSB_MAX_LINES    256
>> +/* MAX number of EDCR registers */
>> +#define TPDM_DSB_MAX_EDCR    16
>> +/* MAX number of EDCMR registers */
>> +#define TPDM_DSB_MAX_EDCMR    8
>> +
>> +#define tpdm_simple_dataset_ro(name, mem, idx, max) \
>
> Do we need max ? The mem itself is enough to help you
> choose the max ? See my comment above.
I will update this in the next patch series.
>
>> +    (&((struct tpdm_dataset_attribute[]) {            \
>> +       {                                \
>> +        __ATTR(name, 0444, tpdm_simple_dataset_show, NULL), \
>> +        mem,                            \
>> +        idx,                            \
>> +        max,                            \
>> +       }                                \
>> +    })[0].attr.attr)
>> +
>> +#define DSB_EDGE_CTRL_ATTR(nr)                    \
>> +        tpdm_simple_dataset_ro(edcr##nr,        \
>> +        DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
>> +
>> +#define DSB_EDGE_CTRL_MASK_ATTR(nr)                \
>> +        tpdm_simple_dataset_ro(edcmr##nr,        \
>> +        DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
>> +
>>   /**
>>    * struct dsb_dataset - specifics associated to dsb dataset
>>    * @mode:             DSB programming mode
>> + * @edge_ctrl_idx     Index number of the edge control
>> + * @edge_ctrl:        Save value for edge control
>> + * @edge_ctrl_mask:   Save value for edge control mask
>>    * @trig_ts:          Enable/Disable trigger timestamp.
>>    * @trig_type:        Enable/Disable trigger type.
>>    */
>>   struct dsb_dataset {
>>       u32                mode;
>> +    u32                edge_ctrl_idx;
>> +    u32                edge_ctrl[TPDM_DSB_MAX_EDCR] > + 
>> u32                edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
>>       bool            trig_ts;
>>       bool            trig_type;
>>   };
>> @@ -96,4 +138,25 @@ struct tpdm_drvdata {
>>       struct dsb_dataset    *dsb;
>>   };
>>   +/* Enumerate members of various datasets */
>> +enum dataset_mem {
>> +    DSB_EDGE_CTRL,
>> +    DSB_EDGE_CTRL_MASK,
>> +};
>> +
>> +/**
>> + * struct tpdm_dataset_attribute - Record the member variables and
>> + * index number of datasets that need to be operated by sysfs file
>> + * @attr:       The device attribute
>> + * @mem:        The member in the dataset data structure
>> + * @idx:        The index number of the array data
>> + * @max:        The maximal number of the array data
>
> As mentioned above, you could remove this.

Sure, I will update this in the next patch series.


Best,

Tao

>
> Suzuki
>
>> + */
>> +struct tpdm_dataset_attribute {
>> +    struct device_attribute attr;
>> +    enum dataset_mem mem;
>> +    u32 idx;
>> +    u32 max;
>> +};
>> +
>>   #endif  /* _CORESIGHT_CORESIGHT_TPDM_H */
>

Re: [PATCH v8 09/13] coresight-tpdm: Add nodes for dsb edge control

[Suzuki K Poulose]

On 01/09/2023 17:01, Tao Zhang wrote:
> 
> On 9/1/2023 10:07 PM, Suzuki K Poulose wrote:
>> On 22/08/2023 06:26, Tao Zhang wrote:
>>> Add the nodes to set value for DSB edge control and DSB edge
>>> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
>>> resgisters to configure edge control. DSB edge detection control
>>> 00: Rising edge detection
>>> 01: Falling edge detection
>>> 10: Rising and falling edge detection (toggle detection)
>>> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
>>> configure mask. Eight 32 bit registers providing DSB interface
>>> edge detection mask control.
>>>
>>> Add the nodes to configure DSB edge control and DSB edge control
>>> mask. Each DSB subunit TPDM maximum of 256 edge detections can be
>>> configured. The index and value sysfs files need to be paired and
>>> written to order. The index sysfs file is to set the index number
>>> of the edge detection which needs to be configured. And the value
>>> sysfs file is to set the control or mask for the edge detection.
>>> DSB edge detection control should be set as the following values.
>>> 00: Rising edge detection
>>> 01: Falling edge detection
>>> 10: Rising and falling edge detection (toggle detection)
>>> And DSB edge mask should be set as 0 or 1.
>>> Each DSB subunit TPDM has maximum of n(n<16) EDCR resgisters to
>>> configure edge control. And each DSB subunit TPDM has maximum of
>>> m(m<8) ECDMR registers to configure mask.
>>>
>>> Add the nodes to read a set of the edge control value and mask
>>> of the DSB in TPDM.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> ---
>>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  51 ++++++
>>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 177 
>>> ++++++++++++++++++++-
>>>   drivers/hwtracing/coresight/coresight-tpdm.h       |  63 ++++++++
>>>   3 files changed, 288 insertions(+), 3 deletions(-)
>>>
>>> diff --git 
>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> index e17d1b4..097fdc4 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> @@ -57,3 +57,54 @@ Description:
>>>           Bit[3] : Set to 0 for low performance mode.
>>>                    Set to 1 for high performance mode.
>>>           Bit[4:8] : Select byte lane for high performance mode.
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_idx
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>
>> s/6.5/6.7
> Sure, I will update this in the next patch series.
>>
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        (RW) Set/Get the index number of the edge detection for the DSB
>>> +        subunit TPDM. Since there are at most 256 edge detections, this
>>> +        value ranges from 0 to 255.
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_val
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>
>> same as above
>>
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        Write a data to control the edge detection corresponding to
>>> +        the index number. Before writing data to this sysfs file,
>>> +        "ctrl_idx" should be written first to configure the index
>>> +        number of the edge detection which needs to be controlled.
>>> +
>>> +        Accepts only one of the following values.
>>> +        0 - Rising edge detection
>>> +        1 - Falling edge detection
>>> +        2 - Rising and falling edge detection (toggle detection)
>>> +
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_mask
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        Write a data to mask the edge detection corresponding to the 
>>> index
>>> +        number. Before writing data to this sysfs file, "ctrl_idx" 
>>> should
>>> +        be written first to configure the index number of the edge 
>>> detection
>>> +        which needs to be masked.
>>> +
>>> +        Accepts only one of the 2 values -  0 or 1.
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcr[0:15]
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        Read a set of the edge control value of the DSB in TPDM.
>>
>> Read edge control register n for edcr<n>.
> 
> In fact, we don't read the register directly through this sysfs file, 
> but read the value
> 
> we set to be written to the registers. Do I still need to modify it here?

thats fine.

> 
>>
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcmr[0:7]
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        Read a set of the edge control mask of the DSB in TPDM.
>>> \ No newline at end of file
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> index 2424eb7..ba61e6a 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> @@ -21,6 +21,29 @@
>>>     DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>>>   +/* Read dataset array member with the index number */
>>> +static ssize_t tpdm_simple_dataset_show(struct device *dev,
>>> +               struct device_attribute *attr, char *buf)
>>> +{
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +    struct tpdm_dataset_attribute *tpdm_attr =
>>> +        container_of(attr, struct tpdm_dataset_attribute, attr);
>>> +
>>> +    if (tpdm_attr->idx >= tpdm_attr->max)
>>
>> minor nit: See my comment on max below. We could skip max.
> I will update this in the next patch series.
>>
>>> +        return -EINVAL;
>>> +
>>> +    switch (tpdm_attr->mem) {
>>> +    case DSB_EDGE_CTRL:
>>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
>>             break;
>>
>>> +        return sysfs_emit(buf, "0x%x\n",
>>> + drvdata->dsb->edge_ctrl[tpdm_attr->idx]);
>>> +    case DSB_EDGE_CTRL_MASK:
>>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
>>             break;
>>
>>> +        return sysfs_emit(buf, "0x%x\n",
>>> + drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
>>     }
>>
>>     return -EINVAL;
> Why do we need to return this error code here?
>>


The whole block would look like :

	switch (tpdm->attr->mem) {
	case DSB_EDGE_CTRL:
		if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
			break;
		return sysfs_...
	case DSB_EDGE_CTRL_MASK:
		if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
			break;
		return sysfs_...
	}

	return -EINVAL;

Suzuki

Re: [PATCH v8 10/13] coresight-tpdm: Add nodes to configure pattern match output

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> Add nodes to configure trigger pattern and trigger pattern mask.
> Each DSB subunit TPDM has maximum of n(n<7) XPR registers to
> configure trigger pattern match output. Eight 32 bit registers
> providing DSB interface trigger output pattern match comparison.
> And each DSB subunit TPDM has maximum of m(m<7) XPMR registers to
> configure trigger pattern mask match output. Eight 32 bit
> registers providing DSB interface trigger output pattern match
> mask.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 18 +++++-
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 71 +++++++++++++++++++++-
>   drivers/hwtracing/coresight/coresight-tpdm.h       | 29 +++++++++
>   3 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 097fdc4..f5cd302 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -107,4 +107,20 @@ Date:		March 2023
>   KernelVersion	6.5
>   Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
>   Description:
> -		Read a set of the edge control mask of the DSB in TPDM.
> \ No newline at end of file
> +		Read a set of the edge control mask of the DSB in TPDM.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpr[0:7]
> +Date:		March 2023
> +KernelVersion	6.5

Same as previous patches

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the value of the trigger pattern for the DSB
> +		subunit TPDM.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpmr[0:7]
> +Date:		March 2023
> +KernelVersion	6.5

here too

> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the mask of the trigger pattern for the DSB
> +		subunit TPDM.
> \ No newline at end of file
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index ba61e6a..6521019 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -39,9 +39,46 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
>   	case DSB_EDGE_CTRL_MASK:
>   		return sysfs_emit(buf, "0x%x\n",
>   				drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
> +	case DSB_TRIG_PATT:
> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->trig_patt[tpdm_attr->idx]);
> +	case DSB_TRIG_PATT_MASK:
> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/* Write dataset array member with the index number */
> +static ssize_t tpdm_simple_dataset_store(struct device *dev,
> +					  struct device_attribute *attr,
> +					  const char *buf,
> +					  size_t size)
> +{
> +	unsigned long val;

	ssize_t ret = size;

> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	struct tpdm_dataset_attribute *tpdm_attr =
> +		container_of(attr, struct tpdm_dataset_attribute, attr);
> +
> +	if (kstrtoul(buf, 0, &val) || (tpdm_attr->idx >= tpdm_attr->max))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	switch (tpdm_attr->mem) {
> +	case DSB_TRIG_PATT:
> +		drvdata->dsb->trig_patt[tpdm_attr->idx] = val;
> +		break;
> +	case DSB_TRIG_PATT_MASK:
> +		drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
> +		break;
>   	default:
> +		spin_unlock(&drvdata->spinlock);

		ret = -EINVAL;

Please avoid releasing the lock in two different places where possible.
Instead use a variable like above.

>   		return -EINVAL;
>   	}
> +	spin_unlock(&drvdata->spinlock);
> +
> +	return size;

	return ret;

>   }
>   
>   static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
> @@ -102,7 +139,12 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   	for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
>   		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
>   			   drvdata->base + TPDM_DSB_EDCMR(i));
> -
> +	for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
> +		writel_relaxed(drvdata->dsb->trig_patt[i],
> +			    drvdata->base + TPDM_DSB_XPR(i));
> +		writel_relaxed(drvdata->dsb->trig_patt_mask[i],
> +			    drvdata->base + TPDM_DSB_XPMR(i));
> +	}
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
>   	/* Set trigger timestamp */
>   	if (drvdata->dsb->trig_ts)
> @@ -531,6 +573,26 @@ static struct attribute *tpdm_dsb_edge_attrs[] = {
>   	NULL,
>   };
>   
> +static struct attribute *tpdm_dsb_trig_patt_attrs[] = {
> +	DSB_TRIG_PATT_ATTR(0),
> +	DSB_TRIG_PATT_ATTR(1),
> +	DSB_TRIG_PATT_ATTR(2),
> +	DSB_TRIG_PATT_ATTR(3),
> +	DSB_TRIG_PATT_ATTR(4),
> +	DSB_TRIG_PATT_ATTR(5),
> +	DSB_TRIG_PATT_ATTR(6),
> +	DSB_TRIG_PATT_ATTR(7),
> +	DSB_TRIG_PATT_MASK_ATTR(0),
> +	DSB_TRIG_PATT_MASK_ATTR(1),
> +	DSB_TRIG_PATT_MASK_ATTR(2),
> +	DSB_TRIG_PATT_MASK_ATTR(3),
> +	DSB_TRIG_PATT_MASK_ATTR(4),
> +	DSB_TRIG_PATT_MASK_ATTR(5),
> +	DSB_TRIG_PATT_MASK_ATTR(6),
> +	DSB_TRIG_PATT_MASK_ATTR(7),
> +	NULL,
> +};
> +
>   static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_mode.attr,
>   	&dev_attr_dsb_trig_ts.attr,
> @@ -549,10 +611,17 @@ static struct attribute_group tpdm_dsb_edge_grp = {
>   	.name = "dsb_edge",
>   };
>   
> +static struct attribute_group tpdm_dsb_trig_patt_grp = {
> +	.attrs = tpdm_dsb_trig_patt_attrs,
> +	.is_visible = tpdm_dsb_is_visible,
> +	.name = "dsb_trig_patt",
> +};
> +
>   static const struct attribute_group *tpdm_attr_grps[] = {
>   	&tpdm_attr_grp,
>   	&tpdm_dsb_attrs_grp,
>   	&tpdm_dsb_edge_grp,
> +	&tpdm_dsb_trig_patt_grp,
>   	NULL,
>   };
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index 9736e00..9e1b0a4 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -12,6 +12,8 @@
>   /* DSB Subunit Registers */
>   #define TPDM_DSB_CR		(0x780)
>   #define TPDM_DSB_TIER		(0x784)
> +#define TPDM_DSB_XPR(n)		(0x7C8 + (n * 4))
> +#define TPDM_DSB_XPMR(n)	(0x7E8 + (n * 4))
>   #define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
>   #define TPDM_DSB_EDCMR(n)	(0x848 + (n * 4))
>   
> @@ -80,6 +82,8 @@
>   #define TPDM_DSB_MAX_EDCR	16
>   /* MAX number of EDCMR registers */
>   #define TPDM_DSB_MAX_EDCMR	8
> +/* MAX number of DSB pattern */
> +#define TPDM_DSB_MAX_PATT	8
>   
>   #define tpdm_simple_dataset_ro(name, mem, idx, max)			\
>   	(&((struct tpdm_dataset_attribute[]) {			\
> @@ -91,6 +95,17 @@
>   	   }								\
>   	})[0].attr.attr)
>   
> +#define tpdm_simple_dataset_rw(name, mem, idx, max)			\
> +	(&((struct tpdm_dataset_attribute[]) {			\
> +	   {								\
> +		__ATTR(name, 0644, tpdm_simple_dataset_show,		\
> +		tpdm_simple_dataset_store),		\
> +		mem,							\ > +		idx,							\
> +		max								\

Same as previous, you could get rid of max

> +	   }								\
> +	})[0].attr.attr)
> +
>   #define DSB_EDGE_CTRL_ATTR(nr)					\
>   		tpdm_simple_dataset_ro(edcr##nr,		\
>   		DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
> @@ -99,12 +114,22 @@
>   		tpdm_simple_dataset_ro(edcmr##nr,		\
>   		DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
>   
> +#define DSB_TRIG_PATT_ATTR(nr)					\
> +		tpdm_simple_dataset_rw(xpr##nr,			\
> +		DSB_TRIG_PATT, nr, TPDM_DSB_MAX_PATT)
> +
> +#define DSB_TRIG_PATT_MASK_ATTR(nr)				\
> +		tpdm_simple_dataset_rw(xpmr##nr,		\
> +		DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
> +
>   /**
>    * struct dsb_dataset - specifics associated to dsb dataset
>    * @mode:             DSB programming mode
>    * @edge_ctrl_idx     Index number of the edge control
>    * @edge_ctrl:        Save value for edge control
>    * @edge_ctrl_mask:   Save value for edge control mask
> + * @trig_patt:        Save value for trigger pattern
> + * @trig_patt_mask:   Save value for trigger pattern mask
>    * @trig_ts:          Enable/Disable trigger timestamp.
>    * @trig_type:        Enable/Disable trigger type.
>    */
> @@ -113,6 +138,8 @@ struct dsb_dataset {
>   	u32				edge_ctrl_idx;
>   	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
>   	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
> +	u32				trig_patt[TPDM_DSB_MAX_PATT];
> +	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
>   	bool			trig_ts;
>   	bool			trig_type;
>   };
> @@ -142,6 +169,8 @@ struct tpdm_drvdata {
>   enum dataset_mem {
>   	DSB_EDGE_CTRL,
>   	DSB_EDGE_CTRL_MASK,
> +	DSB_TRIG_PATT,
> +	DSB_TRIG_PATT_MASK,
>   };

Suzuki

Re: [PATCH v8 11/13] coresight-tpdm: Add nodes for timestamp request

[Suzuki K Poulose]

On 22/08/2023 06:26, Tao Zhang wrote:
> Add nodes to configure the timestamp request based on input
> pattern match. Each TPDM that support DSB subunit has maximum of
> n(n<7) TPR registers to configure value for timestamp request
> based on input pattern match. Eight 32 bit registers providing
> DSB interface timestamp request  pattern match comparison. And
> each TPDM that support DSB subunit has maximum of m(m<7) TPMR
> registers to configure pattern mask for timestamp request. Eight
> 32 bit registers providing DSB interface timestamp request
> pattern match mask generation. Add nodes to enable/disable
> pattern timestamp and set pattern timestamp type.
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  40 ++++++-
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 133 ++++++++++++++++++++-
>   drivers/hwtracing/coresight/coresight-tpdm.h       |  24 ++++
>   3 files changed, 191 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index f5cd302..46a5535 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -123,4 +123,42 @@ KernelVersion	6.5
>   Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
>   Description:
>   		(RW) Set/Get the mask of the trigger pattern for the DSB
> -		subunit TPDM.
> \ No newline at end of file
> +		subunit TPDM.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpr[0:7]
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the value of the pattern for the DSB subunit TPDM.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpmr[0:7]
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the mask of the pattern for the DSB subunit TPDM.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts

Given we have a dedicated "group" for dsb_patt, could we move this to 
dsb_patt and name this "enable_timestamp"

i.e.,

		tpdm-name/dsb_patt/enable_timestamp

> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(Write) Set the pattern timestamp of DSB tpdm. Read
> +		the pattern timestamp of DSB tpdm.
> +
> +		Accepts only one of the 2 values -  0 or 1.
> +		0 : Disable DSB pattern timestamp.
> +		1 : Enable DSB pattern timestamp.
> +
> +What:		/sys/bus/coresight/devices/<tpdm-name>/dsb_patt_type

Similarly here.

> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(Write) Set the pattern type of DSB tpdm. Read
> +		the pattern type of DSB tpdm.
> +
> +		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.
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 6521019..9b0e060 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -45,6 +45,12 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
>   	case DSB_TRIG_PATT_MASK:
>   		return sysfs_emit(buf, "0x%x\n",
>   				drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
> +	case DSB_PATT:
> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->patt_val[tpdm_attr->idx]);
> +	case DSB_PATT_MASK:
> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->patt_mask[tpdm_attr->idx]);
>   	default:
>   		return -EINVAL;
>   	}
> @@ -72,6 +78,12 @@ static ssize_t tpdm_simple_dataset_store(struct device *dev,
>   	case DSB_TRIG_PATT_MASK:
>   		drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
>   		break;
> +	case DSB_PATT:
> +		drvdata->dsb->patt_val[tpdm_attr->idx] = val;
> +		break;
> +	case DSB_PATT_MASK:
> +		drvdata->dsb->patt_mask[tpdm_attr->idx] = val;
> +		break;
>   	default:
>   		spin_unlock(&drvdata->spinlock);
>   		return -EINVAL;
> @@ -129,6 +141,27 @@ static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
>   		*val &= ~TPDM_DSB_CR_MODE;
>   }
>   
> +static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val)
> +{

Could we not Write to the DSB_TIER register from this function ?
There are no other users of this function and keeping the
read and write operations in the caller doesn't make much
sense.


> +	/* Set pattern timestamp type and enablement */
> +	if (drvdata->dsb->patt_ts) {
> +		*val |= TPDM_DSB_TIER_PATT_TSENAB;
> +		if (drvdata->dsb->patt_type)
> +			*val |= TPDM_DSB_TIER_PATT_TYPE;
> +		else
> +			*val &= ~TPDM_DSB_TIER_PATT_TYPE;
> +	} else {
> +		*val &= ~TPDM_DSB_TIER_PATT_TSENAB;
> +	}
> +
> +	/* Set trigger timestamp */
> +	if (drvdata->dsb->trig_ts)
> +		*val |= TPDM_DSB_TIER_XTRIG_TSENAB;
> +	else
> +		*val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;

minor nit:
Does it make sense to clear everything in one shot and set the
required fields based on the fields ? That makes it a bit more
reader friendly.

{
	u32 val = readl_relaxed(drvdata.., TPDM_DSB_TIER);

	/* Clear all relevant fields */
	val &= ~(TPDM_DSB_TIER_PATT_TSENAB | TPDM_DSB_TIER_PATT_TYPE |\
		 TPDM_DSB_TIER_XTRIG_TSENAB)

	/* Set the required fields */
	if (drvdata->dsb->patt_ts) {
		val |= TPDM_DSB_TIER_PATT_TSENAB;
		if (drvdata->dsb->patt_type)
			val |= TPDM_DSB_TIER_PATT_TYPE;
	}

	if (drvdata->dsb->trig_ts)
		val |= TPDM_DSB_TIER_XTRIG_TSENAB;

	writel_relaxed(val, ... TPDM_DSB_TIER);	
}

> +
> +}
> +
>   static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   {
>   	u32 val, i;
> @@ -140,17 +173,17 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   		writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
>   			   drvdata->base + TPDM_DSB_EDCMR(i));
>   	for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
> +		writel_relaxed(drvdata->dsb->patt_val[i],
> +			    drvdata->base + TPDM_DSB_TPR(i));
> +		writel_relaxed(drvdata->dsb->patt_mask[i],
> +			    drvdata->base + TPDM_DSB_TPMR(i));
>   		writel_relaxed(drvdata->dsb->trig_patt[i],
>   			    drvdata->base + TPDM_DSB_XPR(i));
>   		writel_relaxed(drvdata->dsb->trig_patt_mask[i],
>   			    drvdata->base + TPDM_DSB_XPMR(i));
>   	}
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
> -	/* Set trigger timestamp */
> -	if (drvdata->dsb->trig_ts)
> -		val |= TPDM_DSB_TIER_XTRIG_TSENAB;
> -	else
> -		val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
> +	set_dsb_tier(drvdata, &val);
>   	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);

See above

>   
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
> @@ -471,6 +504,67 @@ static ssize_t ctrl_mask_store(struct device *dev,
>   }
>   static DEVICE_ATTR_WO(ctrl_mask);
>   
> +static ssize_t dsb_patt_ts_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%u\n",
> +			 (unsigned int)drvdata->dsb->patt_ts);
> +}
> +
> +/*
> + * value 1: Enable/Disable DSB pattern timestamp
> + */
> +static ssize_t dsb_patt_ts_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf,
> +				   size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->dsb->patt_ts = !!val;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_patt_ts);
> +
> +static ssize_t dsb_patt_type_show(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	return sysfs_emit(buf, "%u\n",
> +			 (unsigned int)drvdata->dsb->patt_type);
> +}
> +
> +/*
> + * value 1: Set DSB pattern type
> + */
> +static ssize_t dsb_patt_type_store(struct device *dev,
> +					  struct device_attribute *attr,
> +					  const char *buf, size_t size)
> +{
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	unsigned long val;
> +
> +	if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
> +		return -EINVAL;
> +
> +	spin_lock(&drvdata->spinlock);
> +	drvdata->dsb->patt_type = val;
> +	spin_unlock(&drvdata->spinlock);
> +	return size;
> +}
> +static DEVICE_ATTR_RW(dsb_patt_type);
> +
>   static ssize_t dsb_trig_type_show(struct device *dev,
>   		  struct device_attribute *attr, char *buf)
>   {
> @@ -593,8 +687,30 @@ static struct attribute *tpdm_dsb_trig_patt_attrs[] = {
>   	NULL,
>   };
>   
> +static struct attribute *tpdm_dsb_patt_attrs[] = {
> +	DSB_PATT_ATTR(0),
> +	DSB_PATT_ATTR(1),
> +	DSB_PATT_ATTR(2),
> +	DSB_PATT_ATTR(3),
> +	DSB_PATT_ATTR(4),
> +	DSB_PATT_ATTR(5),
> +	DSB_PATT_ATTR(6),
> +	DSB_PATT_ATTR(7),
> +	DSB_PATT_MASK_ATTR(0),
> +	DSB_PATT_MASK_ATTR(1),
> +	DSB_PATT_MASK_ATTR(2),
> +	DSB_PATT_MASK_ATTR(3),
> +	DSB_PATT_MASK_ATTR(4),
> +	DSB_PATT_MASK_ATTR(5),
> +	DSB_PATT_MASK_ATTR(6),
> +	DSB_PATT_MASK_ATTR(7),
> +	NULL,
> +};
> +
>   static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_mode.attr,

> +	&dev_attr_dsb_patt_ts.attr,
> +	&dev_attr_dsb_patt_type.attr,

As mentioned above, could we move the above two to the dsb_patt_attrs ?

Suzuki

>   	&dev_attr_dsb_trig_ts.attr,
>   	&dev_attr_dsb_trig_type.attr,
>   	NULL,
> @@ -617,11 +733,18 @@ static struct attribute_group tpdm_dsb_trig_patt_grp = {
>   	.name = "dsb_trig_patt",
>   };
>   
> +static struct attribute_group tpdm_dsb_patt_grp = {
> +	.attrs = tpdm_dsb_patt_attrs,
> +	.is_visible = tpdm_dsb_is_visible,
> +	.name = "dsb_patt",
> +};
> +
>   static const struct attribute_group *tpdm_attr_grps[] = {
>   	&tpdm_attr_grp,
>   	&tpdm_dsb_attrs_grp,
>   	&tpdm_dsb_edge_grp,
>   	&tpdm_dsb_trig_patt_grp,
> +	&tpdm_dsb_patt_grp,
>   	NULL,
>   };
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index 9e1b0a4..9173e80 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -12,6 +12,8 @@
>   /* DSB Subunit Registers */
>   #define TPDM_DSB_CR		(0x780)
>   #define TPDM_DSB_TIER		(0x784)
> +#define TPDM_DSB_TPR(n)		(0x788 + (n * 4))
> +#define TPDM_DSB_TPMR(n)	(0x7A8 + (n * 4))
>   #define TPDM_DSB_XPR(n)		(0x7C8 + (n * 4))
>   #define TPDM_DSB_XPMR(n)	(0x7E8 + (n * 4))
>   #define TPDM_DSB_EDCR(n)	(0x808 + (n * 4))
> @@ -28,8 +30,12 @@
>   /* Data bits for DSB test mode */
>   #define TPDM_DSB_CR_TEST_MODE		GENMASK(10, 9)
>   
> +/* Enable bit for DSB subunit pattern timestamp */
> +#define TPDM_DSB_TIER_PATT_TSENAB		BIT(0)
>   /* Enable bit for DSB subunit trigger timestamp */
>   #define TPDM_DSB_TIER_XTRIG_TSENAB		BIT(1)
> +/* Bit for DSB subunit pattern type */
> +#define TPDM_DSB_TIER_PATT_TYPE		BIT(2)
>   
>   /* DSB programming modes */
>   /* DSB mode bits mask */
> @@ -122,14 +128,26 @@
>   		tpdm_simple_dataset_rw(xpmr##nr,		\
>   		DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
>   
> +#define DSB_PATT_ATTR(nr)						\
> +		tpdm_simple_dataset_rw(tpr##nr,			\
> +		DSB_PATT, nr, TPDM_DSB_MAX_PATT)
> +
> +#define DSB_PATT_MASK_ATTR(nr)					\
> +		tpdm_simple_dataset_rw(tpmr##nr,		\
> +		DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
> +
>   /**
>    * struct dsb_dataset - specifics associated to dsb dataset
>    * @mode:             DSB programming mode
>    * @edge_ctrl_idx     Index number of the edge control
>    * @edge_ctrl:        Save value for edge control
>    * @edge_ctrl_mask:   Save value for edge control mask
> + * @patt_val:         Save value for pattern
> + * @patt_mask:        Save value for pattern mask
>    * @trig_patt:        Save value for trigger pattern
>    * @trig_patt_mask:   Save value for trigger pattern mask
> + * @patt_ts:          Enable/Disable pattern timestamp
> + * @patt_type:        Set pattern type
>    * @trig_ts:          Enable/Disable trigger timestamp.
>    * @trig_type:        Enable/Disable trigger type.
>    */
> @@ -138,8 +156,12 @@ struct dsb_dataset {
>   	u32				edge_ctrl_idx;
>   	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
>   	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
> +	u32				patt_val[TPDM_DSB_MAX_PATT];
> +	u32				patt_mask[TPDM_DSB_MAX_PATT];
>   	u32				trig_patt[TPDM_DSB_MAX_PATT];
>   	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
> +	bool			patt_ts;
> +	bool			patt_type;
>   	bool			trig_ts;
>   	bool			trig_type;
>   };
> @@ -171,6 +193,8 @@ enum dataset_mem {
>   	DSB_EDGE_CTRL_MASK,
>   	DSB_TRIG_PATT,
>   	DSB_TRIG_PATT_MASK,
> +	DSB_PATT,
> +	DSB_PATT_MASK,
>   };
>   
>   /**

Re: [PATCH v8 13/13] coresight-tpdm: Add nodes for dsb msr support

[Suzuki K Poulose]

On 22/08/2023 06:26, 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   |  8 +++
>   drivers/hwtracing/coresight/coresight-tpdm.c       | 79 ++++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-tpdm.h       | 12 ++++
>   3 files changed, 99 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 46a5535..9b7e172 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -162,3 +162,11 @@ 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/msr[0:31]
> +Date:		March 2023
> +KernelVersion	6.5
> +Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com>
> +Description:
> +		(RW) Set/Get the MSR(mux select register) for the DSB subunit
> +		TPDM.
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 9b0e060..367c5e2 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -51,6 +51,9 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev,
>   	case DSB_PATT_MASK:
>   		return sysfs_emit(buf, "0x%x\n",
>   				drvdata->dsb->patt_mask[tpdm_attr->idx]);
> +	case DSB_MSR:
> +		return sysfs_emit(buf, "0x%x\n",
> +				drvdata->dsb->msr[tpdm_attr->idx]);
>   	default:
>   		return -EINVAL;
>   	}
> @@ -84,6 +87,9 @@ static ssize_t tpdm_simple_dataset_store(struct device *dev,
>   	case DSB_PATT_MASK:
>   		drvdata->dsb->patt_mask[tpdm_attr->idx] = val;
>   		break;
> +	case DSB_MSR:
> +		drvdata->dsb->msr[tpdm_attr->idx] = val;
> +		break;
>   	default:
>   		spin_unlock(&drvdata->spinlock);
>   		return -EINVAL;
> @@ -110,6 +116,22 @@ static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
>   	return 0;
>   }
>   
> +static umode_t tpdm_dsb_msr_is_visible(struct kobject *kobj,
> +					   struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> +	if (drvdata && tpdm_has_dsb_dataset(drvdata)) {
> +		of_property_read_u32(drvdata->dev->of_node,
> +			   "qcom,dsb_msr_num", &drvdata->dsb->msr_num);

Why don't we read this during probe and use that here ?

> +		if (drvdata->dsb->msr_num != 0)

Also, could we not check the index of the attribute and make sure it is
available on this TPDM ?

		attr->idx < drvdata->dsb->msr_num ??

> +			return attr->mode;
> +	}
> +
> +	return 0;
> +}
> +
>   static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
>   {
>   	if (tpdm_has_dsb_dataset(drvdata)) {
> @@ -162,6 +184,18 @@ static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val)
>   
>   }
>   
> +static void set_dsb_msr(struct tpdm_drvdata *drvdata)
> +{
> +	int i;
> +
> +	if (drvdata->dsb->msr_num == 0)
> +		return;

You don;t need this special case, the loop handles it
already

> +
> +	for (i = 0; i < drvdata->dsb->msr_num; i++)
> +		writel_relaxed(drvdata->dsb->msr[i],
> +			   drvdata->base + TPDM_DSB_MSR(i));
> +}
> +
>   static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   {
>   	u32 val, i;
> @@ -186,6 +220,8 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>   	set_dsb_tier(drvdata, &val);
>   	writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
>   
> +	set_dsb_msr(drvdata);
> +
>   	val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>   	/* Set the mode of DSB dataset */
>   	set_dsb_mode(drvdata, &val);
> @@ -707,6 +743,42 @@ static struct attribute *tpdm_dsb_patt_attrs[] = {
>   	NULL,
>   };
>   
> +static struct attribute *tpdm_dsb_msr_attrs[] = {
> +	DSB_MSR_ATTR(0),
> +	DSB_MSR_ATTR(1),
> +	DSB_MSR_ATTR(2),
> +	DSB_MSR_ATTR(3),
> +	DSB_MSR_ATTR(4),
> +	DSB_MSR_ATTR(5),
> +	DSB_MSR_ATTR(6),
> +	DSB_MSR_ATTR(7),
> +	DSB_MSR_ATTR(8),
> +	DSB_MSR_ATTR(9),
> +	DSB_MSR_ATTR(10),
> +	DSB_MSR_ATTR(11),
> +	DSB_MSR_ATTR(12),
> +	DSB_MSR_ATTR(13),
> +	DSB_MSR_ATTR(14),
> +	DSB_MSR_ATTR(15),
> +	DSB_MSR_ATTR(16),
> +	DSB_MSR_ATTR(17),
> +	DSB_MSR_ATTR(18),
> +	DSB_MSR_ATTR(19),
> +	DSB_MSR_ATTR(20),
> +	DSB_MSR_ATTR(21),
> +	DSB_MSR_ATTR(22),
> +	DSB_MSR_ATTR(23),
> +	DSB_MSR_ATTR(24),
> +	DSB_MSR_ATTR(25),
> +	DSB_MSR_ATTR(26),
> +	DSB_MSR_ATTR(27),
> +	DSB_MSR_ATTR(28),
> +	DSB_MSR_ATTR(29),
> +	DSB_MSR_ATTR(30),
> +	DSB_MSR_ATTR(31),
> +	NULL,
> +};
> +
>   static struct attribute *tpdm_dsb_attrs[] = {
>   	&dev_attr_dsb_mode.attr,
>   	&dev_attr_dsb_patt_ts.attr,
> @@ -739,12 +811,19 @@ static struct attribute_group tpdm_dsb_patt_grp = {
>   	.name = "dsb_patt",
>   };
>   
> +static struct attribute_group tpdm_dsb_msr_grp = {
> +	.attrs = tpdm_dsb_msr_attrs,
> +	.is_visible = tpdm_dsb_msr_is_visible,
> +	.name = "dsb_msr",
> +};
> +
>   static const struct attribute_group *tpdm_attr_grps[] = {
>   	&tpdm_attr_grp,
>   	&tpdm_dsb_attrs_grp,
>   	&tpdm_dsb_edge_grp,
>   	&tpdm_dsb_trig_patt_grp,
>   	&tpdm_dsb_patt_grp,
> +	&tpdm_dsb_msr_grp,
>   	NULL,
>   };
>   
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index 9173e80..270e946 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)
> @@ -90,6 +91,8 @@
>   #define TPDM_DSB_MAX_EDCMR	8
>   /* MAX number of DSB pattern */
>   #define TPDM_DSB_MAX_PATT	8
> +/* MAX number of DSB MSR */
> +#define TPDM_DSB_MAX_MSR 32
>   
>   #define tpdm_simple_dataset_ro(name, mem, idx, max)			\
>   	(&((struct tpdm_dataset_attribute[]) {			\
> @@ -136,16 +139,22 @@
>   		tpdm_simple_dataset_rw(tpmr##nr,		\
>   		DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
>   
> +#define DSB_MSR_ATTR(nr)						\
> +		tpdm_simple_dataset_rw(msr##nr,			\
> +		DSB_MSR, nr, TPDM_DSB_MAX_MSR)
> +
>   /**
>    * struct dsb_dataset - specifics associated to dsb dataset
>    * @mode:             DSB programming mode
>    * @edge_ctrl_idx     Index number of the edge control
> + * @msr_num           Number of MSR supported by DSB TPDM
>    * @edge_ctrl:        Save value for edge control
>    * @edge_ctrl_mask:   Save value for edge control mask
>    * @patt_val:         Save value for pattern
>    * @patt_mask:        Save value for pattern mask
>    * @trig_patt:        Save value for trigger pattern
>    * @trig_patt_mask:   Save value for trigger pattern mask
> + * @msr               Save value for MSR
>    * @patt_ts:          Enable/Disable pattern timestamp
>    * @patt_type:        Set pattern type
>    * @trig_ts:          Enable/Disable trigger timestamp.
> @@ -154,12 +163,14 @@
>   struct dsb_dataset {
>   	u32				mode;
>   	u32				edge_ctrl_idx;
> +	u32				msr_num;

Why should this be part of the dataset ? If I remember correctly,
this data structure is zeroed on reset via sysfs ? Shouldn't that
be part of the drvdata-> instead ? So that you read it once during
probe and don't mess with it.

Suzuki

>   	u32				edge_ctrl[TPDM_DSB_MAX_EDCR];
>   	u32				edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
>   	u32				patt_val[TPDM_DSB_MAX_PATT];
>   	u32				patt_mask[TPDM_DSB_MAX_PATT];
>   	u32				trig_patt[TPDM_DSB_MAX_PATT];
>   	u32				trig_patt_mask[TPDM_DSB_MAX_PATT];
> +	u32				msr[TPDM_DSB_MAX_MSR];
>   	bool			patt_ts;
>   	bool			patt_type;
>   	bool			trig_ts;
> @@ -195,6 +206,7 @@ enum dataset_mem {
>   	DSB_TRIG_PATT_MASK,
>   	DSB_PATT,
>   	DSB_PATT_MASK,
> +	DSB_MSR,
>   };
>   
>   /**

Re: [PATCH v8 10/13] coresight-tpdm: Add nodes to configure pattern match output

[Tao Zhang]

On 9/5/2023 5:41 PM, Suzuki K Poulose wrote:
> On 22/08/2023 06:26, Tao Zhang wrote:
>> Add nodes to configure trigger pattern and trigger pattern mask.
>> Each DSB subunit TPDM has maximum of n(n<7) XPR registers to
>> configure trigger pattern match output. Eight 32 bit registers
>> providing DSB interface trigger output pattern match comparison.
>> And each DSB subunit TPDM has maximum of m(m<7) XPMR registers to
>> configure trigger pattern mask match output. Eight 32 bit
>> registers providing DSB interface trigger output pattern match
>> mask.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 18 +++++-
>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 71 
>> +++++++++++++++++++++-
>>   drivers/hwtracing/coresight/coresight-tpdm.h       | 29 +++++++++
>>   3 files changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git 
>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index 097fdc4..f5cd302 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -107,4 +107,20 @@ Date:        March 2023
>>   KernelVersion    6.5
>>   Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>   Description:
>> -        Read a set of the edge control mask of the DSB in TPDM.
>> \ No newline at end of file
>> +        Read a set of the edge control mask of the DSB in TPDM.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpr[0:7]
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> Same as previous patches
Got it.
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the value of the trigger pattern for the DSB
>> +        subunit TPDM.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt/xpmr[0:7]
>> +Date:        March 2023
>> +KernelVersion    6.5
>
> here too
Got it.
>
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the mask of the trigger pattern for the DSB
>> +        subunit TPDM.
>> \ No newline at end of file
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index ba61e6a..6521019 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -39,9 +39,46 @@ static ssize_t tpdm_simple_dataset_show(struct 
>> device *dev,
>>       case DSB_EDGE_CTRL_MASK:
>>           return sysfs_emit(buf, "0x%x\n",
>> drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
>> +    case DSB_TRIG_PATT:
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->trig_patt[tpdm_attr->idx]);
>> +    case DSB_TRIG_PATT_MASK:
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +}
>> +
>> +/* Write dataset array member with the index number */
>> +static ssize_t tpdm_simple_dataset_store(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      const char *buf,
>> +                      size_t size)
>> +{
>> +    unsigned long val;
>
>     ssize_t ret = size;
Sure, I will update this in the next patch series.
>
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    struct tpdm_dataset_attribute *tpdm_attr =
>> +        container_of(attr, struct tpdm_dataset_attribute, attr);
>> +
>> +    if (kstrtoul(buf, 0, &val) || (tpdm_attr->idx >= tpdm_attr->max))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    switch (tpdm_attr->mem) {
>> +    case DSB_TRIG_PATT:
>> +        drvdata->dsb->trig_patt[tpdm_attr->idx] = val;
>> +        break;
>> +    case DSB_TRIG_PATT_MASK:
>> +        drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
>> +        break;
>>       default:
>> +        spin_unlock(&drvdata->spinlock);
>
>         ret = -EINVAL;
>
> Please avoid releasing the lock in two different places where possible.
> Instead use a variable like above.
>
Sure, I will update this in the next patch series.
>>           return -EINVAL;
>>       }
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    return size;
>
>     return ret;
Got it.
>
>>   }
>>     static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
>> @@ -102,7 +139,12 @@ static void tpdm_enable_dsb(struct tpdm_drvdata 
>> *drvdata)
>>       for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++)
>>           writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
>>                  drvdata->base + TPDM_DSB_EDCMR(i));
>> -
>> +    for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
>> +        writel_relaxed(drvdata->dsb->trig_patt[i],
>> +                drvdata->base + TPDM_DSB_XPR(i));
>> +        writel_relaxed(drvdata->dsb->trig_patt_mask[i],
>> +                drvdata->base + TPDM_DSB_XPMR(i));
>> +    }
>>       val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
>>       /* Set trigger timestamp */
>>       if (drvdata->dsb->trig_ts)
>> @@ -531,6 +573,26 @@ static struct attribute *tpdm_dsb_edge_attrs[] = {
>>       NULL,
>>   };
>>   +static struct attribute *tpdm_dsb_trig_patt_attrs[] = {
>> +    DSB_TRIG_PATT_ATTR(0),
>> +    DSB_TRIG_PATT_ATTR(1),
>> +    DSB_TRIG_PATT_ATTR(2),
>> +    DSB_TRIG_PATT_ATTR(3),
>> +    DSB_TRIG_PATT_ATTR(4),
>> +    DSB_TRIG_PATT_ATTR(5),
>> +    DSB_TRIG_PATT_ATTR(6),
>> +    DSB_TRIG_PATT_ATTR(7),
>> +    DSB_TRIG_PATT_MASK_ATTR(0),
>> +    DSB_TRIG_PATT_MASK_ATTR(1),
>> +    DSB_TRIG_PATT_MASK_ATTR(2),
>> +    DSB_TRIG_PATT_MASK_ATTR(3),
>> +    DSB_TRIG_PATT_MASK_ATTR(4),
>> +    DSB_TRIG_PATT_MASK_ATTR(5),
>> +    DSB_TRIG_PATT_MASK_ATTR(6),
>> +    DSB_TRIG_PATT_MASK_ATTR(7),
>> +    NULL,
>> +};
>> +
>>   static struct attribute *tpdm_dsb_attrs[] = {
>>       &dev_attr_dsb_mode.attr,
>>       &dev_attr_dsb_trig_ts.attr,
>> @@ -549,10 +611,17 @@ static struct attribute_group tpdm_dsb_edge_grp 
>> = {
>>       .name = "dsb_edge",
>>   };
>>   +static struct attribute_group tpdm_dsb_trig_patt_grp = {
>> +    .attrs = tpdm_dsb_trig_patt_attrs,
>> +    .is_visible = tpdm_dsb_is_visible,
>> +    .name = "dsb_trig_patt",
>> +};
>> +
>>   static const struct attribute_group *tpdm_attr_grps[] = {
>>       &tpdm_attr_grp,
>>       &tpdm_dsb_attrs_grp,
>>       &tpdm_dsb_edge_grp,
>> +    &tpdm_dsb_trig_patt_grp,
>>       NULL,
>>   };
>>   diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h 
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index 9736e00..9e1b0a4 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -12,6 +12,8 @@
>>   /* DSB Subunit Registers */
>>   #define TPDM_DSB_CR        (0x780)
>>   #define TPDM_DSB_TIER        (0x784)
>> +#define TPDM_DSB_XPR(n)        (0x7C8 + (n * 4))
>> +#define TPDM_DSB_XPMR(n)    (0x7E8 + (n * 4))
>>   #define TPDM_DSB_EDCR(n)    (0x808 + (n * 4))
>>   #define TPDM_DSB_EDCMR(n)    (0x848 + (n * 4))
>>   @@ -80,6 +82,8 @@
>>   #define TPDM_DSB_MAX_EDCR    16
>>   /* MAX number of EDCMR registers */
>>   #define TPDM_DSB_MAX_EDCMR    8
>> +/* MAX number of DSB pattern */
>> +#define TPDM_DSB_MAX_PATT    8
>>     #define tpdm_simple_dataset_ro(name, mem, idx, max)            \
>>       (&((struct tpdm_dataset_attribute[]) {            \
>> @@ -91,6 +95,17 @@
>>          }                                \
>>       })[0].attr.attr)
>>   +#define tpdm_simple_dataset_rw(name, mem, idx, max)            \
>> +    (&((struct tpdm_dataset_attribute[]) {            \
>> +       {                                \
>> +        __ATTR(name, 0644, tpdm_simple_dataset_show,        \
>> +        tpdm_simple_dataset_store),        \
>> +        mem,                            \ > + 
>> idx,                            \
>> +        max                                \
>
> Same as previous, you could get rid of max
Sure, I will update this in the next patch series.
>
>> +       }                                \
>> +    })[0].attr.attr)
>> +
>>   #define DSB_EDGE_CTRL_ATTR(nr)                    \
>>           tpdm_simple_dataset_ro(edcr##nr,        \
>>           DSB_EDGE_CTRL, nr, TPDM_DSB_MAX_EDCR)
>> @@ -99,12 +114,22 @@
>>           tpdm_simple_dataset_ro(edcmr##nr,        \
>>           DSB_EDGE_CTRL_MASK, nr, TPDM_DSB_MAX_EDCMR)
>>   +#define DSB_TRIG_PATT_ATTR(nr)                    \
>> +        tpdm_simple_dataset_rw(xpr##nr,            \
>> +        DSB_TRIG_PATT, nr, TPDM_DSB_MAX_PATT)
>> +
>> +#define DSB_TRIG_PATT_MASK_ATTR(nr)                \
>> +        tpdm_simple_dataset_rw(xpmr##nr,        \
>> +        DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
>> +
>>   /**
>>    * struct dsb_dataset - specifics associated to dsb dataset
>>    * @mode:             DSB programming mode
>>    * @edge_ctrl_idx     Index number of the edge control
>>    * @edge_ctrl:        Save value for edge control
>>    * @edge_ctrl_mask:   Save value for edge control mask
>> + * @trig_patt:        Save value for trigger pattern
>> + * @trig_patt_mask:   Save value for trigger pattern mask
>>    * @trig_ts:          Enable/Disable trigger timestamp.
>>    * @trig_type:        Enable/Disable trigger type.
>>    */
>> @@ -113,6 +138,8 @@ struct dsb_dataset {
>>       u32                edge_ctrl_idx;
>>       u32                edge_ctrl[TPDM_DSB_MAX_EDCR];
>>       u32                edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
>> +    u32                trig_patt[TPDM_DSB_MAX_PATT];
>> +    u32                trig_patt_mask[TPDM_DSB_MAX_PATT];
>>       bool            trig_ts;
>>       bool            trig_type;
>>   };
>> @@ -142,6 +169,8 @@ struct tpdm_drvdata {
>>   enum dataset_mem {
>>       DSB_EDGE_CTRL,
>>       DSB_EDGE_CTRL_MASK,
>> +    DSB_TRIG_PATT,
>> +    DSB_TRIG_PATT_MASK,
>>   };
>
> Suzuki
>
>

Re: [PATCH v8 09/13] coresight-tpdm: Add nodes for dsb edge control

[Tao Zhang]

On 9/5/2023 5:36 PM, Suzuki K Poulose wrote:
> On 01/09/2023 17:01, Tao Zhang wrote:
>>
>> On 9/1/2023 10:07 PM, Suzuki K Poulose wrote:
>>> On 22/08/2023 06:26, Tao Zhang wrote:
>>>> Add the nodes to set value for DSB edge control and DSB edge
>>>> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
>>>> resgisters to configure edge control. DSB edge detection control
>>>> 00: Rising edge detection
>>>> 01: Falling edge detection
>>>> 10: Rising and falling edge detection (toggle detection)
>>>> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
>>>> configure mask. Eight 32 bit registers providing DSB interface
>>>> edge detection mask control.
>>>>
>>>> Add the nodes to configure DSB edge control and DSB edge control
>>>> mask. Each DSB subunit TPDM maximum of 256 edge detections can be
>>>> configured. The index and value sysfs files need to be paired and
>>>> written to order. The index sysfs file is to set the index number
>>>> of the edge detection which needs to be configured. And the value
>>>> sysfs file is to set the control or mask for the edge detection.
>>>> DSB edge detection control should be set as the following values.
>>>> 00: Rising edge detection
>>>> 01: Falling edge detection
>>>> 10: Rising and falling edge detection (toggle detection)
>>>> And DSB edge mask should be set as 0 or 1.
>>>> Each DSB subunit TPDM has maximum of n(n<16) EDCR resgisters to
>>>> configure edge control. And each DSB subunit TPDM has maximum of
>>>> m(m<8) ECDMR registers to configure mask.
>>>>
>>>> Add the nodes to read a set of the edge control value and mask
>>>> of the DSB in TPDM.
>>>>
>>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>>> ---
>>>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  51 ++++++
>>>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 177 
>>>> ++++++++++++++++++++-
>>>>   drivers/hwtracing/coresight/coresight-tpdm.h       |  63 ++++++++
>>>>   3 files changed, 288 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git 
>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> index e17d1b4..097fdc4 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> @@ -57,3 +57,54 @@ Description:
>>>>           Bit[3] : Set to 0 for low performance mode.
>>>>                    Set to 1 for high performance mode.
>>>>           Bit[4:8] : Select byte lane for high performance mode.
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_idx
>>>> +Date:        March 2023
>>>> +KernelVersion    6.5
>>>
>>> s/6.5/6.7
>> Sure, I will update this in the next patch series.
>>>
>>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>>>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>>> +Description:
>>>> +        (RW) Set/Get the index number of the edge detection for 
>>>> the DSB
>>>> +        subunit TPDM. Since there are at most 256 edge detections, 
>>>> this
>>>> +        value ranges from 0 to 255.
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_val
>>>> +Date:        March 2023
>>>> +KernelVersion    6.5
>>>
>>> same as above
>>>
>>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>>>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>>> +Description:
>>>> +        Write a data to control the edge detection corresponding to
>>>> +        the index number. Before writing data to this sysfs file,
>>>> +        "ctrl_idx" should be written first to configure the index
>>>> +        number of the edge detection which needs to be controlled.
>>>> +
>>>> +        Accepts only one of the following values.
>>>> +        0 - Rising edge detection
>>>> +        1 - Falling edge detection
>>>> +        2 - Rising and falling edge detection (toggle detection)
>>>> +
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/ctrl_mask
>>>> +Date:        March 2023
>>>> +KernelVersion    6.5
>>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>>>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>>> +Description:
>>>> +        Write a data to mask the edge detection corresponding to 
>>>> the index
>>>> +        number. Before writing data to this sysfs file, "ctrl_idx" 
>>>> should
>>>> +        be written first to configure the index number of the edge 
>>>> detection
>>>> +        which needs to be masked.
>>>> +
>>>> +        Accepts only one of the 2 values -  0 or 1.
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcr[0:15]
>>>> +Date:        March 2023
>>>> +KernelVersion    6.5
>>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>>>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>>> +Description:
>>>> +        Read a set of the edge control value of the DSB in TPDM.
>>>
>>> Read edge control register n for edcr<n>.
>>
>> In fact, we don't read the register directly through this sysfs file, 
>> but read the value
>>
>> we set to be written to the registers. Do I still need to modify it 
>> here?
>
> thats fine.
>
>>
>>>
>>>> +
>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge/edcmr[0:7]
>>>> +Date:        March 2023
>>>> +KernelVersion    6.5
>>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>>>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>>> +Description:
>>>> +        Read a set of the edge control mask of the DSB in TPDM.
>>>> \ No newline at end of file
>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>>>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>>>> index 2424eb7..ba61e6a 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>>> @@ -21,6 +21,29 @@
>>>>     DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>>>>   +/* Read dataset array member with the index number */
>>>> +static ssize_t tpdm_simple_dataset_show(struct device *dev,
>>>> +               struct device_attribute *attr, char *buf)
>>>> +{
>>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>> +    struct tpdm_dataset_attribute *tpdm_attr =
>>>> +        container_of(attr, struct tpdm_dataset_attribute, attr);
>>>> +
>>>> +    if (tpdm_attr->idx >= tpdm_attr->max)
>>>
>>> minor nit: See my comment on max below. We could skip max.
>> I will update this in the next patch series.
>>>
>>>> +        return -EINVAL;
>>>> +
>>>> +    switch (tpdm_attr->mem) {
>>>> +    case DSB_EDGE_CTRL:
>>>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
>>>             break;
>>>
>>>> +        return sysfs_emit(buf, "0x%x\n",
>>>> + drvdata->dsb->edge_ctrl[tpdm_attr->idx]);
>>>> +    case DSB_EDGE_CTRL_MASK:
>>>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
>>>             break;
>>>
>>>> +        return sysfs_emit(buf, "0x%x\n",
>>>> + drvdata->dsb->edge_ctrl_mask[tpdm_attr->idx]);
>>>     }
>>>
>>>     return -EINVAL;
>> Why do we need to return this error code here?
>>>
>
>
> The whole block would look like :
>
>     switch (tpdm->attr->mem) {
>     case DSB_EDGE_CTRL:
>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCR)
>             break;
>         return sysfs_...
>     case DSB_EDGE_CTRL_MASK:
>         if (tmp_attr->idx > TPDM_DSB_MAX_EDCMR)
>             break;
>         return sysfs_...
>     }
>
>     return -EINVAL;

Sure, I will update this in the next patch series.


Best,

Tao

>
> Suzuki
>

Re: [PATCH v8 11/13] coresight-tpdm: Add nodes for timestamp request

[Tao Zhang]

On 9/6/2023 5:09 PM, Suzuki K Poulose wrote:
> On 22/08/2023 06:26, Tao Zhang wrote:
>> Add nodes to configure the timestamp request based on input
>> pattern match. Each TPDM that support DSB subunit has maximum of
>> n(n<7) TPR registers to configure value for timestamp request
>> based on input pattern match. Eight 32 bit registers providing
>> DSB interface timestamp request  pattern match comparison. And
>> each TPDM that support DSB subunit has maximum of m(m<7) TPMR
>> registers to configure pattern mask for timestamp request. Eight
>> 32 bit registers providing DSB interface timestamp request
>> pattern match mask generation. Add nodes to enable/disable
>> pattern timestamp and set pattern timestamp type.
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> ---
>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   |  40 ++++++-
>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 133 
>> ++++++++++++++++++++-
>>   drivers/hwtracing/coresight/coresight-tpdm.h       |  24 ++++
>>   3 files changed, 191 insertions(+), 6 deletions(-)
>>
>> diff --git 
>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index f5cd302..46a5535 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -123,4 +123,42 @@ KernelVersion    6.5
>>   Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao 
>> Zhang (QUIC) <quic_taozha@quicinc.com>
>>   Description:
>>           (RW) Set/Get the mask of the trigger pattern for the DSB
>> -        subunit TPDM.
>> \ No newline at end of file
>> +        subunit TPDM.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpr[0:7]
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the value of the pattern for the DSB subunit TPDM.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpmr[0:7]
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (RW) Set/Get the mask of the pattern for the DSB subunit TPDM.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts
>
> Given we have a dedicated "group" for dsb_patt, could we move this to 
> dsb_patt and name this "enable_timestamp"
>
> i.e.,
>
>         tpdm-name/dsb_patt/enable_timestamp
Sure, I will update this in the next patch series.
>
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (Write) Set the pattern timestamp of DSB tpdm. Read
>> +        the pattern timestamp of DSB tpdm.
>> +
>> +        Accepts only one of the 2 values -  0 or 1.
>> +        0 : Disable DSB pattern timestamp.
>> +        1 : Enable DSB pattern timestamp.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_type
>
> Similarly here.
Got it.
>
>> +Date:        March 2023
>> +KernelVersion    6.5
>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>> (QUIC) <quic_taozha@quicinc.com>
>> +Description:
>> +        (Write) Set the pattern type of DSB tpdm. Read
>> +        the pattern type of DSB tpdm.
>> +
>> +        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.
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index 6521019..9b0e060 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -45,6 +45,12 @@ static ssize_t tpdm_simple_dataset_show(struct 
>> device *dev,
>>       case DSB_TRIG_PATT_MASK:
>>           return sysfs_emit(buf, "0x%x\n",
>> drvdata->dsb->trig_patt_mask[tpdm_attr->idx]);
>> +    case DSB_PATT:
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->patt_val[tpdm_attr->idx]);
>> +    case DSB_PATT_MASK:
>> +        return sysfs_emit(buf, "0x%x\n",
>> + drvdata->dsb->patt_mask[tpdm_attr->idx]);
>>       default:
>>           return -EINVAL;
>>       }
>> @@ -72,6 +78,12 @@ static ssize_t tpdm_simple_dataset_store(struct 
>> device *dev,
>>       case DSB_TRIG_PATT_MASK:
>>           drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val;
>>           break;
>> +    case DSB_PATT:
>> +        drvdata->dsb->patt_val[tpdm_attr->idx] = val;
>> +        break;
>> +    case DSB_PATT_MASK:
>> +        drvdata->dsb->patt_mask[tpdm_attr->idx] = val;
>> +        break;
>>       default:
>>           spin_unlock(&drvdata->spinlock);
>>           return -EINVAL;
>> @@ -129,6 +141,27 @@ static void set_dsb_mode(struct tpdm_drvdata 
>> *drvdata, u32 *val)
>>           *val &= ~TPDM_DSB_CR_MODE;
>>   }
>>   +static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val)
>> +{
>
> Could we not Write to the DSB_TIER register from this function ?
> There are no other users of this function and keeping the
> read and write operations in the caller doesn't make much
> sense.
>
Sure, I will update this in the next patch series.
>
>> +    /* Set pattern timestamp type and enablement */
>> +    if (drvdata->dsb->patt_ts) {
>> +        *val |= TPDM_DSB_TIER_PATT_TSENAB;
>> +        if (drvdata->dsb->patt_type)
>> +            *val |= TPDM_DSB_TIER_PATT_TYPE;
>> +        else
>> +            *val &= ~TPDM_DSB_TIER_PATT_TYPE;
>> +    } else {
>> +        *val &= ~TPDM_DSB_TIER_PATT_TSENAB;
>> +    }
>> +
>> +    /* Set trigger timestamp */
>> +    if (drvdata->dsb->trig_ts)
>> +        *val |= TPDM_DSB_TIER_XTRIG_TSENAB;
>> +    else
>> +        *val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
>
> minor nit:
> Does it make sense to clear everything in one shot and set the
> required fields based on the fields ? That makes it a bit more
> reader friendly.
>
> {
>     u32 val = readl_relaxed(drvdata.., TPDM_DSB_TIER);
>
>     /* Clear all relevant fields */
>     val &= ~(TPDM_DSB_TIER_PATT_TSENAB | TPDM_DSB_TIER_PATT_TYPE |\
>          TPDM_DSB_TIER_XTRIG_TSENAB)
>
>     /* Set the required fields */
>     if (drvdata->dsb->patt_ts) {
>         val |= TPDM_DSB_TIER_PATT_TSENAB;
>         if (drvdata->dsb->patt_type)
>             val |= TPDM_DSB_TIER_PATT_TYPE;
>     }
>
>     if (drvdata->dsb->trig_ts)
>         val |= TPDM_DSB_TIER_XTRIG_TSENAB;
>
>     writel_relaxed(val, ... TPDM_DSB_TIER);
> }
Sure, I will update this in the next patch series.
>
>> +
>> +}
>> +
>>   static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
>>   {
>>       u32 val, i;
>> @@ -140,17 +173,17 @@ static void tpdm_enable_dsb(struct tpdm_drvdata 
>> *drvdata)
>>           writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
>>                  drvdata->base + TPDM_DSB_EDCMR(i));
>>       for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
>> +        writel_relaxed(drvdata->dsb->patt_val[i],
>> +                drvdata->base + TPDM_DSB_TPR(i));
>> +        writel_relaxed(drvdata->dsb->patt_mask[i],
>> +                drvdata->base + TPDM_DSB_TPMR(i));
>>           writel_relaxed(drvdata->dsb->trig_patt[i],
>>                   drvdata->base + TPDM_DSB_XPR(i));
>>           writel_relaxed(drvdata->dsb->trig_patt_mask[i],
>>                   drvdata->base + TPDM_DSB_XPMR(i));
>>       }
>>       val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
>> -    /* Set trigger timestamp */
>> -    if (drvdata->dsb->trig_ts)
>> -        val |= TPDM_DSB_TIER_XTRIG_TSENAB;
>> -    else
>> -        val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
>> +    set_dsb_tier(drvdata, &val);
>>       writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);
>
> See above
Got it.
>
>>         val = readl_relaxed(drvdata->base + TPDM_DSB_CR);
>> @@ -471,6 +504,67 @@ static ssize_t ctrl_mask_store(struct device *dev,
>>   }
>>   static DEVICE_ATTR_WO(ctrl_mask);
>>   +static ssize_t dsb_patt_ts_show(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   char *buf)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    return sysfs_emit(buf, "%u\n",
>> +             (unsigned int)drvdata->dsb->patt_ts);
>> +}
>> +
>> +/*
>> + * value 1: Enable/Disable DSB pattern timestamp
>> + */
>> +static ssize_t dsb_patt_ts_store(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   const char *buf,
>> +                   size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    drvdata->dsb->patt_ts = !!val;
>> +    spin_unlock(&drvdata->spinlock);
>> +    return size;
>> +}
>> +static DEVICE_ATTR_RW(dsb_patt_ts);
>> +
>> +static ssize_t dsb_patt_type_show(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      char *buf)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +    return sysfs_emit(buf, "%u\n",
>> +             (unsigned int)drvdata->dsb->patt_type);
>> +}
>> +
>> +/*
>> + * value 1: Set DSB pattern type
>> + */
>> +static ssize_t dsb_patt_type_store(struct device *dev,
>> +                      struct device_attribute *attr,
>> +                      const char *buf, size_t size)
>> +{
>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +    unsigned long val;
>> +
>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>> +        return -EINVAL;
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    drvdata->dsb->patt_type = val;
>> +    spin_unlock(&drvdata->spinlock);
>> +    return size;
>> +}
>> +static DEVICE_ATTR_RW(dsb_patt_type);
>> +
>>   static ssize_t dsb_trig_type_show(struct device *dev,
>>             struct device_attribute *attr, char *buf)
>>   {
>> @@ -593,8 +687,30 @@ static struct attribute 
>> *tpdm_dsb_trig_patt_attrs[] = {
>>       NULL,
>>   };
>>   +static struct attribute *tpdm_dsb_patt_attrs[] = {
>> +    DSB_PATT_ATTR(0),
>> +    DSB_PATT_ATTR(1),
>> +    DSB_PATT_ATTR(2),
>> +    DSB_PATT_ATTR(3),
>> +    DSB_PATT_ATTR(4),
>> +    DSB_PATT_ATTR(5),
>> +    DSB_PATT_ATTR(6),
>> +    DSB_PATT_ATTR(7),
>> +    DSB_PATT_MASK_ATTR(0),
>> +    DSB_PATT_MASK_ATTR(1),
>> +    DSB_PATT_MASK_ATTR(2),
>> +    DSB_PATT_MASK_ATTR(3),
>> +    DSB_PATT_MASK_ATTR(4),
>> +    DSB_PATT_MASK_ATTR(5),
>> +    DSB_PATT_MASK_ATTR(6),
>> +    DSB_PATT_MASK_ATTR(7),
>> +    NULL,
>> +};
>> +
>>   static struct attribute *tpdm_dsb_attrs[] = {
>>       &dev_attr_dsb_mode.attr,
>
>> +    &dev_attr_dsb_patt_ts.attr,
>> +    &dev_attr_dsb_patt_type.attr,
>
> As mentioned above, could we move the above two to the dsb_patt_attrs ?

Sure, I will update this in the next patch series.


Best,

Tao

>
> Suzuki
>
>>       &dev_attr_dsb_trig_ts.attr,
>>       &dev_attr_dsb_trig_type.attr,
>>       NULL,
>> @@ -617,11 +733,18 @@ static struct attribute_group 
>> tpdm_dsb_trig_patt_grp = {
>>       .name = "dsb_trig_patt",
>>   };
>>   +static struct attribute_group tpdm_dsb_patt_grp = {
>> +    .attrs = tpdm_dsb_patt_attrs,
>> +    .is_visible = tpdm_dsb_is_visible,
>> +    .name = "dsb_patt",
>> +};
>> +
>>   static const struct attribute_group *tpdm_attr_grps[] = {
>>       &tpdm_attr_grp,
>>       &tpdm_dsb_attrs_grp,
>>       &tpdm_dsb_edge_grp,
>>       &tpdm_dsb_trig_patt_grp,
>> +    &tpdm_dsb_patt_grp,
>>       NULL,
>>   };
>>   diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h 
>> b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index 9e1b0a4..9173e80 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -12,6 +12,8 @@
>>   /* DSB Subunit Registers */
>>   #define TPDM_DSB_CR        (0x780)
>>   #define TPDM_DSB_TIER        (0x784)
>> +#define TPDM_DSB_TPR(n)        (0x788 + (n * 4))
>> +#define TPDM_DSB_TPMR(n)    (0x7A8 + (n * 4))
>>   #define TPDM_DSB_XPR(n)        (0x7C8 + (n * 4))
>>   #define TPDM_DSB_XPMR(n)    (0x7E8 + (n * 4))
>>   #define TPDM_DSB_EDCR(n)    (0x808 + (n * 4))
>> @@ -28,8 +30,12 @@
>>   /* Data bits for DSB test mode */
>>   #define TPDM_DSB_CR_TEST_MODE        GENMASK(10, 9)
>>   +/* Enable bit for DSB subunit pattern timestamp */
>> +#define TPDM_DSB_TIER_PATT_TSENAB        BIT(0)
>>   /* Enable bit for DSB subunit trigger timestamp */
>>   #define TPDM_DSB_TIER_XTRIG_TSENAB        BIT(1)
>> +/* Bit for DSB subunit pattern type */
>> +#define TPDM_DSB_TIER_PATT_TYPE        BIT(2)
>>     /* DSB programming modes */
>>   /* DSB mode bits mask */
>> @@ -122,14 +128,26 @@
>>           tpdm_simple_dataset_rw(xpmr##nr,        \
>>           DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
>>   +#define DSB_PATT_ATTR(nr)                        \
>> +        tpdm_simple_dataset_rw(tpr##nr,            \
>> +        DSB_PATT, nr, TPDM_DSB_MAX_PATT)
>> +
>> +#define DSB_PATT_MASK_ATTR(nr)                    \
>> +        tpdm_simple_dataset_rw(tpmr##nr,        \
>> +        DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT)
>> +
>>   /**
>>    * struct dsb_dataset - specifics associated to dsb dataset
>>    * @mode:             DSB programming mode
>>    * @edge_ctrl_idx     Index number of the edge control
>>    * @edge_ctrl:        Save value for edge control
>>    * @edge_ctrl_mask:   Save value for edge control mask
>> + * @patt_val:         Save value for pattern
>> + * @patt_mask:        Save value for pattern mask
>>    * @trig_patt:        Save value for trigger pattern
>>    * @trig_patt_mask:   Save value for trigger pattern mask
>> + * @patt_ts:          Enable/Disable pattern timestamp
>> + * @patt_type:        Set pattern type
>>    * @trig_ts:          Enable/Disable trigger timestamp.
>>    * @trig_type:        Enable/Disable trigger type.
>>    */
>> @@ -138,8 +156,12 @@ struct dsb_dataset {
>>       u32                edge_ctrl_idx;
>>       u32                edge_ctrl[TPDM_DSB_MAX_EDCR];
>>       u32                edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
>> +    u32                patt_val[TPDM_DSB_MAX_PATT];
>> +    u32                patt_mask[TPDM_DSB_MAX_PATT];
>>       u32                trig_patt[TPDM_DSB_MAX_PATT];
>>       u32                trig_patt_mask[TPDM_DSB_MAX_PATT];
>> +    bool            patt_ts;
>> +    bool            patt_type;
>>       bool            trig_ts;
>>       bool            trig_type;
>>   };
>> @@ -171,6 +193,8 @@ enum dataset_mem {
>>       DSB_EDGE_CTRL_MASK,
>>       DSB_TRIG_PATT,
>>       DSB_TRIG_PATT_MASK,
>> +    DSB_PATT,
>> +    DSB_PATT_MASK,
>>   };
>>     /**
>

Re: [PATCH v8 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

[Suzuki K Poulose]

On 01/09/2023 15:41, Tao Zhang wrote:
> 
> On 9/1/2023 6:43 PM, Suzuki K Poulose wrote:
>> On 22/08/2023 06:26, Tao Zhang wrote:
>>> The nodes are needed to set or show the trigger timestamp and
>>> trigger type. This change is to add these nodes to achieve these
>>> function.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> ---
>>>   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 22 +++++
>>>   drivers/hwtracing/coresight/coresight-tpdm.c       | 95 
>>> ++++++++++++++++++++++
>>>   2 files changed, 117 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> index 2936226..9e26e30 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> @@ -21,3 +21,25 @@ Description:
>>>             Accepts only one value -  1.
>>>           1 : Reset the dataset of the tpdm
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_type
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>
>> 6.7
>>
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        (RW) Set/Get the trigger type of the DSB for tpdm.
>>> +
>>> +        Accepts only one of the 2 values -  0 or 1.
>>> +        0 : Set the DSB trigger type to false
>>> +        1 : Set the DSB trigger type to true
>>> +
>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
>>> +Date:        March 2023
>>> +KernelVersion    6.5
>>
>> Same here
>>
>>> +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang 
>>> (QUIC) <quic_taozha@quicinc.com>
>>> +Description:
>>> +        (RW) Set/Get the trigger timestamp of the DSB for tpdm.
>>> +
>>> +        Accepts only one of the 2 values -  0 or 1.
>>> +        0 : Set the DSB trigger type to false
>>> +        1 : Set the DSB trigger type to true
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> index d6e7c8c..8e11c9b 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> @@ -25,6 +25,18 @@ static bool tpdm_has_dsb_dataset(struct 
>>> tpdm_drvdata *drvdata)
>>>       return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
>>>   }
>>>   +static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
>>> +                       struct attribute *attr, int n)
>>
>> minor nit: please align.
>>
>> static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
>>                    struct attribute *attr, int n)
>>
>> I don't know if you have a different setting for tabs in your editor.
>> Please refer to the coding style document.
> 
> Tab size is set to 4 in my editor.

Please read :

Documentation/process/coding-style.rst, section 1.

Suzuki


> 
> There are 5 tabs and 3 spaces at the beginning of this line.
> 
> I don't know if this is the same as what you see in this patch.
> 
> I see from the editor that the code meets the requirements of the coding 
> style document.
> 
> If the gap in alignment is not resolved, such alignment problems may 
> still occur.
> 
> 
> Best,
> 
> Tao
> 
>>
>>> +{
>>> +    struct device *dev = kobj_to_dev(kobj);
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +    if (drvdata && tpdm_has_dsb_dataset(drvdata))
>>> +        return attr->mode;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
>>>   {
>>>       if (tpdm_has_dsb_dataset(drvdata)) {
>>> @@ -232,8 +244,91 @@ static struct attribute_group tpdm_attr_grp = {
>>>       .attrs = tpdm_attrs,
>>>   };
>>>   +static ssize_t dsb_trig_type_show(struct device *dev,
>>> +          struct device_attribute *attr, char *buf)
>>
>> same here.
>>
>>> +{
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +    return sysfs_emit(buf, "%u\n",
>>> +             (unsigned int)drvdata->dsb->trig_type);
>>> +}
>>> +
>>> +/*
>>> + * Trigger type (boolean):
>>> + * false - Disable trigger type.
>>> + * true  - Enable trigger type.
>>> + */
>>> +static ssize_t dsb_trig_type_store(struct device *dev,
>>> +                      struct device_attribute *attr,
>>> +                      const char *buf,
>>> +                      size_t size)
>>> +{
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +    unsigned long val;
>>> +
>>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>>> +        return -EINVAL;
>>> +
>>> +    spin_lock(&drvdata->spinlock);
>>> +    if (val)
>>> +        drvdata->dsb->trig_type = true;
>>> +    else
>>> +        drvdata->dsb->trig_type = false;
>>> +    spin_unlock(&drvdata->spinlock);
>>> +    return size;
>>> +}
>>> +static DEVICE_ATTR_RW(dsb_trig_type);
>>> +
>>> +static ssize_t dsb_trig_ts_show(struct device *dev,
>>> +                    struct device_attribute *attr,
>>> +                    char *buf)
>>> +{
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +    return sysfs_emit(buf, "%u\n",
>>> +             (unsigned int)drvdata->dsb->trig_ts);
>>> +}
>>> +
>>> +/*
>>> + * Trigger timestamp (boolean):
>>> + * false - Disable trigger timestamp.
>>> + * true  - Enable trigger timestamp.
>>> + */
>>> +static ssize_t dsb_trig_ts_store(struct device *dev,
>>> +                   struct device_attribute *attr,
>>> +                   const char *buf,
>>> +                   size_t size)
>>> +{
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +    unsigned long val;
>>> +
>>> +    if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
>>> +        return -EINVAL;
>>> +
>>> +    spin_lock(&drvdata->spinlock);
>>> +    if (val)
>>> +        drvdata->dsb->trig_ts = true;
>>> +    else
>>> +        drvdata->dsb->trig_ts = false;
>>> +    spin_unlock(&drvdata->spinlock);
>>> +    return size;
>>> +}
>>> +static DEVICE_ATTR_RW(dsb_trig_ts);
>>> +
>>> +static struct attribute *tpdm_dsb_attrs[] = {
>>> +    &dev_attr_dsb_trig_ts.attr,
>>> +    &dev_attr_dsb_trig_type.attr,
>>> +    NULL,
>>> +};
>>> +
>>> +static struct attribute_group tpdm_dsb_attr_grp = {
>>> +    .attrs = tpdm_dsb_attrs,
>>> +    .is_visible = tpdm_dsb_is_visible,
>>> +};
>>> +
>>>   static const struct attribute_group *tpdm_attr_grps[] = {
>>>       &tpdm_attr_grp,
>>> +    &tpdm_dsb_attr_grp,
>>>       NULL,
>>>   };
>>
>> Rest looks fine to me
>>
>> Suzuki
>>