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.
/sys/devices/platform/soc@0/69d0000.tpdm/tpdm0#ls -l | grep dsb
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_edge_ctrl
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_edge_ctrl_mask
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_mode
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_mask
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_ts
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_type
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_val
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_patt_mask
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_patt_val
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_ts
-rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_type
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/tpdm0/reset
echo 0x3 0x3 0x1 > /sys/bus/coresight/devices/tpdm0/dsb_edge_ctrl_mask
echo 0x6d 0x6d 0 > /sys/bus/coresight/devices/tpdm0/dsb_edge_ctrl
echo 1 > /sys/bus/coresight/devices/tpdm0/dsb_patt_ts
echo 1 > /sys/bus/coresight/devices/tpdm0/dsb_patt_type
echo 0 > /sys/bus/coresight/devices/tpdm0/dsb_trig_ts
echo 0 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm0/dsb_patt_mask
echo 0 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm0/dsb_trig_patt_val
This patch series depends on patch series "[PATCH v6 0/9] coresight:
Fix CTI module refcount leak by making it a helper device"
https://patchwork.kernel.org/project/linux-arm-kernel/patch/[email protected]/
TPDM_DSB commit tree:
https://git.codelinaro.org/clo/linux-kernel/coresight/-/tree/tpdm-dsb-v6
https://git.codelinaro.org/clo/linux-kernel/coresight/-/commits/tpdm-dsb-v6
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
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 | 169 +++++
.../bindings/arm/qcom,coresight-tpdm.yaml | 20 +
drivers/hwtracing/coresight/coresight-core.c | 1 +
drivers/hwtracing/coresight/coresight-tpda.c | 96 ++-
drivers/hwtracing/coresight/coresight-tpda.h | 4 +
drivers/hwtracing/coresight/coresight-tpdm.c | 721 ++++++++++++++++++++-
drivers/hwtracing/coresight/coresight-tpdm.h | 83 +++
include/linux/coresight.h | 1 +
8 files changed, 1077 insertions(+), 18 deletions(-)
--
2.7.4
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
.../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..dbc2fbd0 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
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Reset the dataset of the tpdm, and disable 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 52aa48a6..acc3eea 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -159,6 +159,27 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
return 0;
}
+static ssize_t reset_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, 10, &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);
+
/*
* value 1: 64 bits test data
* value 2: 32 bits test data
@@ -199,6 +220,7 @@ static ssize_t integration_test_store(struct device *dev,
static DEVICE_ATTR_WO(integration_test);
static struct attribute *tpdm_attrs[] = {
+ &dev_attr_reset.attr,
&dev_attr_integration_test.attr,
NULL,
};
--
2.7.4
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 <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 24 ++++++
drivers/hwtracing/coresight/coresight-tpdm.c | 94 ++++++++++++++++++++++
2 files changed, 118 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index dbc2fbd0..0b7b4ad 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -21,3 +21,27 @@ 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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the trigger type of DSB tpdm. Read the trigger
+ type of DSB 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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the trigger timestamp of DSB tpdm. Read the
+ trigger timestamp of DSB 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 acc3eea..62efc18 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -20,6 +20,18 @@
DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
+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 && (drvdata->datasets & TPDM_PIDR0_DS_DSB))
+ return attr->mode;
+
+ return 0;
+}
+
static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
{
if (drvdata->datasets & TPDM_PIDR0_DS_DSB) {
@@ -229,8 +241,90 @@ 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
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 <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 28 +++++++
drivers/hwtracing/coresight/coresight-tpdm.c | 93 ++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 12 ++-
3 files changed, 131 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 34189e4a..55ec81d 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -92,3 +92,31 @@ Description:
<integer1> : Start EDCMR register number
<integer2> : End EDCMR register number
<integer3> : The value need to be written
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt_val
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the trigger pattern value of DSB tpdm.
+ Read the trigger pattern value of DSB tpdm.
+
+ Expected format is the following:
+ <integer1> <integer2>
+
+ Where:
+ <integer1> : Index number of XPR register, the range is 0 to 7
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt_mask
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the trigger pattern mask of DSB tpdm.
+ Read the trigger pattern mask of DSB tpdm.
+
+ Expected format is the following:
+ <integer1> <integer2>
+
+ Where:
+ <integer1> : Index number of XPMR register, the range is 0 to 7
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index fc92900..974e63f 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -80,6 +80,13 @@ 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->trig_patt_val[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)
@@ -441,6 +448,90 @@ static ssize_t dsb_edge_ctrl_mask_store(struct device *dev,
}
static DEVICE_ATTR_RW(dsb_edge_ctrl_mask);
+static ssize_t dsb_trig_patt_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i = 0;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index: 0x%x Value: 0x%x\n", i,
+ drvdata->dsb->trig_patt_val[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+static ssize_t dsb_trig_patt_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 index, val;
+
+ if (sscanf(buf, "%lx %lx", &index, &val) != 2)
+ return -EINVAL;
+ if (index >= TPDM_DSB_MAX_PATT)
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->dsb->trig_patt_val[index] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_patt_val);
+
+static ssize_t dsb_trig_patt_mask_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i = 0;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index: 0x%x Value: 0x%x\n", i,
+ drvdata->dsb->trig_patt_mask[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+static ssize_t dsb_trig_patt_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 index, val;
+
+ if (sscanf(buf, "%lx %lx", &index, &val) != 2)
+ return -EINVAL;
+ if (index >= TPDM_DSB_MAX_PATT)
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->dsb->trig_patt_mask[index] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_patt_mask);
+
static ssize_t dsb_trig_type_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -515,6 +606,8 @@ static struct attribute *tpdm_dsb_attrs[] = {
&dev_attr_dsb_mode.attr,
&dev_attr_dsb_edge_ctrl.attr,
&dev_attr_dsb_edge_ctrl_mask.attr,
+ &dev_attr_dsb_trig_patt_val.attr,
+ &dev_attr_dsb_trig_patt_mask.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 f81bfe8..87d946e 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))
@@ -77,19 +79,25 @@
#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
/**
* struct dsb_dataset - specifics associated to dsb dataset
* @mode: DSB programming mode
* @edge_ctrl: Save value for edge control
* @edge_ctrl_mask: Save value for edge control mask
+ * @trig_patt_val: 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.
*/
struct dsb_dataset {
u32 mode;
- u32 edge_ctrl[TPDM_DSB_MAX_EDCR];
- u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
+ u32 edge_ctrl[TPDM_DSB_MAX_EDCR];
+ u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
+ u32 trig_patt_val[TPDM_DSB_MAX_PATT];
+ u32 trig_patt_mask[TPDM_DSB_MAX_PATT];
bool trig_ts;
bool trig_type;
};
--
2.7.4
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 <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 14 +++++
drivers/hwtracing/coresight/coresight-tpdm.c | 71 ++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 5 ++
3 files changed, 90 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 6e1b246..aaf02a6 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -166,3 +166,17 @@ Description:
Accepts only one of the 2 values - 0 or 1.
0 : Set the DSB pattern type to value.
1 : Set the DSB pattern type to toggle.
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_msr
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the MSR(mux select register) of DSB tpdm. Read
+ the MSR(mux select register) of DSB tpdm.
+
+ Expected format is the following:
+ <integer1> <integer2>
+
+ Where:
+ <integer1> : Index number of MSR register, the range is 0 to 31
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 1aff244..9c5782f 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -90,6 +90,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;
@@ -116,6 +128,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 test accurate mode */
set_dsb_test_mode(drvdata, &val);
@@ -234,6 +248,14 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
if (!drvdata->dsb)
return -ENOMEM;
}
+ if (!of_property_read_u32(drvdata->dev->of_node,
+ "qcom,dsb_msr_num", &drvdata->dsb->msr_num)) {
+ drvdata->dsb->msr = devm_kzalloc(drvdata->dev,
+ (drvdata->dsb->msr_num * sizeof(*drvdata->dsb->msr)),
+ GFP_KERNEL);
+ if (!drvdata->dsb->msr)
+ return -ENOMEM;
+ }
}
return 0;
@@ -775,6 +797,54 @@ static ssize_t dsb_trig_ts_store(struct device *dev,
}
static DEVICE_ATTR_RW(dsb_trig_ts);
+static ssize_t dsb_msr_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ unsigned int i;
+ unsigned long bytes;
+ ssize_t size = 0;
+
+ if (drvdata->dsb->msr_num == 0)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < drvdata->dsb->msr_num; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "%u 0x%x\n", i, drvdata->dsb->msr[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+
+ return size;
+}
+
+static ssize_t dsb_msr_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t size)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ unsigned int num, val;
+ int nval;
+
+ if (drvdata->dsb->msr_num == 0)
+ return -EINVAL;
+
+ nval = sscanf(buf, "%u %x", &num, &val);
+ if ((nval != 2) || (num >= drvdata->dsb->msr_num))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->dsb->msr[num] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_msr);
+
static struct attribute *tpdm_dsb_attrs[] = {
&dev_attr_dsb_mode.attr,
&dev_attr_dsb_edge_ctrl.attr,
@@ -787,6 +857,7 @@ static struct attribute *tpdm_dsb_attrs[] = {
&dev_attr_dsb_trig_patt_mask.attr,
&dev_attr_dsb_trig_ts.attr,
&dev_attr_dsb_trig_type.attr,
+ &dev_attr_dsb_msr.attr,
NULL,
};
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 3169fb5..5372093 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)
@@ -97,6 +98,8 @@
* @patt_mask: Save value for pattern mask
* @trig_patt_val: Save value for trigger pattern
* @trig_patt_mask: Save value for trigger pattern mask
+ * @msr_num Number of MSR supported by DSB TPDM
+ * @msr Save value for MSR
* @patt_ts: Enable/Disable pattern timestamp
* @patt_type: Set pattern type
* @trig_ts: Enable/Disable trigger timestamp.
@@ -110,6 +113,8 @@ struct dsb_dataset {
u32 patt_mask[TPDM_DSB_MAX_PATT];
u32 trig_patt_val[TPDM_DSB_MAX_PATT];
u32 trig_patt_mask[TPDM_DSB_MAX_PATT];
+ u32 msr_num;
+ u32 *msr;
bool patt_ts;
bool patt_type;
bool trig_ts;
--
2.7.4
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 <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 15 ++++++
drivers/hwtracing/coresight/coresight-tpdm.c | 62 ++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tpdm.h | 16 ++++++
3 files changed, 93 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 0b7b4ad..2a82cd0 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -45,3 +45,18 @@ 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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the mode of DSB tpdm. Read the mode of DSB
+ 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 62efc18..c38760b 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>
@@ -42,6 +43,32 @@ static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
}
}
+static void set_dsb_test_mode(struct tpdm_drvdata *drvdata, u32 *val)
+{
+ u32 mode;
+
+ mode = TPDM_DSB_MODE_TEST(drvdata->dsb->mode);
+ *val &= ~TPDM_DSB_TEST_MODE;
+ *val |= FIELD_PREP(TPDM_DSB_TEST_MODE, mode);
+}
+
+static void set_dsb_hpsel_mode(struct tpdm_drvdata *drvdata, u32 *val)
+{
+ u32 mode;
+
+ mode = TPDM_DSB_MODE_HPBYTESEL(drvdata->dsb->mode);
+ *val &= ~TPDM_DSB_HPSEL;
+ *val |= FIELD_PREP(TPDM_DSB_HPSEL, mode);
+}
+
+static void set_dsb_perf_mode(struct tpdm_drvdata *drvdata, u32 *val)
+{
+ 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;
@@ -55,6 +82,12 @@ 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 test accurate mode */
+ set_dsb_test_mode(drvdata, &val);
+ /* Set the byte lane for high-performance mode */
+ set_dsb_hpsel_mode(drvdata, &val);
+ /* Set the performance mode */
+ set_dsb_perf_mode(drvdata, &val);
/* Set trigger type */
if (drvdata->dsb->trig_type)
val |= TPDM_DSB_CR_TRIG_TYPE;
@@ -241,6 +274,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, "%lx\n",
+ (unsigned long)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)
+ 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)
{
@@ -312,6 +373,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 92c34cd..49fffb1 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.h
+++ b/drivers/hwtracing/coresight/coresight-tpdm.h
@@ -15,11 +15,25 @@
/* 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)
+
/* Enable bit for DSB subunit trigger timestamp */
#define TPDM_DSB_TIER_XTRIG_TSENAB BIT(1)
+/* DSB programming modes */
+/* 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))
+#define TPDM_DSB_MODE_MASK GENMASK(8, 0)
+#define TPDM_DSB_TEST_MODE GENMASK(10, 9)
+#define TPDM_DSB_HPSEL GENMASK(6, 2)
+
/* TPDM integration test registers */
#define TPDM_ITATBCNTRL (0xEF0)
#define TPDM_ITCNTRL (0xF00)
@@ -48,10 +62,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
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 <[email protected]>
---
drivers/hwtracing/coresight/coresight-tpda.c | 96 +++++++++++++++++++++++++---
drivers/hwtracing/coresight/coresight-tpda.h | 4 ++
2 files changed, 90 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 8d2b9d2..7c71342 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -21,6 +21,58 @@
DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
+/* 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 0 in the recursize call.
+ * Parameter "parent" is used to pass in the original call.
+ */
+static int tpda_set_element_size(struct tpda_drvdata *drvdata,
+ struct coresight_device *csdev, int inport, bool match_inport)
+{
+ static int nr_inport;
+ int i;
+ static bool tpdm_found;
+ struct coresight_device *in_csdev;
+
+ if (inport > (TPDA_MAX_INPORTS - 1))
+ return -EINVAL;
+
+ if (match_inport) {
+ nr_inport = inport;
+ tpdm_found = false;
+ }
+
+ for (i = 0; i < csdev->pdata->nr_inconns; i++) {
+ in_csdev = csdev->pdata->in_conns[i]->src_dev;
+ if (!in_csdev)
+ break;
+
+ if (match_inport)
+ if (csdev->pdata->in_conns[i]->dest_port != inport)
+ continue;
+
+ if ((in_csdev->type == CORESIGHT_DEV_TYPE_SOURCE) &&
+ (in_csdev->subtype.source_subtype
+ == CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM)) {
+ of_property_read_u8(in_csdev->dev.parent->of_node,
+ "qcom,dsb-element-size", &drvdata->dsb_esize[nr_inport]);
+ if (!tpdm_found)
+ tpdm_found = true;
+ else
+ dev_warn(drvdata->dev,
+ "More than one TPDM is mapped to the TPDA input port %d.\n",
+ nr_inport);
+ continue;
+ }
+ tpda_set_element_size(drvdata, in_csdev, 0, false);
+ }
+
+ return 0;
+}
+
/* Settings pre enabling port control register */
static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
{
@@ -32,26 +84,43 @@ 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;
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
+ */
+ if (drvdata->dsb_esize[port] == 32)
+ val &= ~TPDA_Pn_CR_DSBSIZE;
+ else if (drvdata->dsb_esize[port] == 64)
+ val |= TPDA_Pn_CR_DSBSIZE;
+ else
+ 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 +128,23 @@ static int tpda_enable(struct coresight_device *csdev,
struct coresight_connection *out)
{
struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ int ret;
+
+ ret = tpda_set_element_size(drvdata, csdev, in->dest_port, true);
+ if (ret)
+ return 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..12a1472 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
@@ -23,6 +25,7 @@
* @csdev: component vitals needed by the framework.
* @spinlock: lock for the drvdata value.
* @enable: enable status of the component.
+ * @dsb_esize: DSB element size for each inport, it must be 32 or 64.
*/
struct tpda_drvdata {
void __iomem *base;
@@ -30,6 +33,7 @@ struct tpda_drvdata {
struct coresight_device *csdev;
spinlock_t spinlock;
u8 atid;
+ u8 dsb_esize[TPDA_MAX_INPORTS];
};
#endif /* _CORESIGHT_CORESIGHT_TPDA_H */
--
2.7.4
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.
Signed-off-by: Tao Zhang <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
3 files changed, 196 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 2a82cd0..34189e4a 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -60,3 +60,35 @@ 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
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ Read/Write a set of the edge control registers of the DSB
+ in TPDM.
+
+ Expected format is the following:
+ <integer1> <integer2> <integer3>
+
+ Where:
+ <integer1> : Start EDCR register number
+ <integer2> : End EDCR register number
+ <integer3> : The value need to be written
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge_ctrl_mask
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ Read/Write a set of the edge control mask registers of the
+ DSB in TPDM.
+
+ Expected format is the following:
+ <integer1> <integer2> <integer3>
+
+ Where:
+ <integer1> : Start EDCMR register number
+ <integer2> : End EDCMR register number
+ <integer3> : The value need to be written
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index c38760b..fc92900 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -71,7 +71,14 @@ static void set_dsb_perf_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 */
@@ -302,6 +309,138 @@ static ssize_t dsb_mode_store(struct device *dev,
}
static DEVICE_ATTR_RW(dsb_mode);
+static ssize_t dsb_edge_ctrl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index:0x%x Val:0x%x\n", i,
+ drvdata->dsb->edge_ctrl[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+/*
+ * value 1: Start EDCR register number
+ * value 2: End EDCR register number
+ * value 3: The value need to be written
+ * 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. So the starting number(value 1) and ending number(value 2)
+ * cannot be greater than 256, and value 1 should be less than value 2.
+ * The following values are the rage of value 3.
+ * 0 - Rising edge detection
+ * 1 - Falling edge detection
+ * 2 - Rising and falling edge detection (toggle detection)
+ */
+static ssize_t dsb_edge_ctrl_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, mask, start, end, edge_ctrl;
+ int i, reg;
+
+ if (sscanf(buf, "%lx %lx %lx", &start, &end, &edge_ctrl) != 3)
+ return -EINVAL;
+ if ((start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES) ||
+ (start > end) || (edge_ctrl > 0x2))
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = start; i <= end; i++) {
+ /*
+ * There are 2 bit per DSB Edge Control line.
+ * Thus we have 16 lines in a 32bit word.
+ */
+ reg = EDCR_TO_WORD_IDX(i);
+ mask = EDCR_TO_WORD_MASK(i);
+ val = drvdata->dsb->edge_ctrl[reg];
+ val &= ~EDCR_TO_WORD_MASK(i);
+ val |= EDCR_TO_WORD_VAL(edge_ctrl, i);
+ drvdata->dsb->edge_ctrl[reg] = val;
+ }
+ spin_unlock(&drvdata->spinlock);
+
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_edge_ctrl);
+
+static ssize_t dsb_edge_ctrl_mask_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_EDCMR; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index:0x%x Val:0x%x\n", i,
+ drvdata->dsb->edge_ctrl_mask[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+/*
+ * value 1: Start EDCMR register number
+ * value 2: End EDCMR register number
+ * value 3: The value need to be written
+ */
+static ssize_t dsb_edge_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 start, end, val;
+ u32 set;
+ int i, reg;
+
+ if (sscanf(buf, "%lx %lx %lx", &start, &end, &val) != 3)
+ return -EINVAL;
+ if ((start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES)
+ || (start > end) || (val & ~1UL))
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = start; i <= end; i++) {
+ /*
+ * There is 1 bit per DSB Edge Control Mark line.
+ * Thus we have 32 lines in a 32bit word.
+ */
+ reg = EDCMR_TO_WORD_IDX(i);
+ set = drvdata->dsb->edge_ctrl_mask[reg];
+ if (val)
+ set |= BIT(EDCR_TO_WORD_SHIFT(i));
+ else
+ set &= ~BIT(EDCR_TO_WORD_SHIFT(i));
+ drvdata->dsb->edge_ctrl_mask[reg] = set;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_edge_ctrl_mask);
+
static ssize_t dsb_trig_type_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -374,6 +513,8 @@ static DEVICE_ATTR_RW(dsb_trig_ts);
static struct attribute *tpdm_dsb_attrs[] = {
&dev_attr_dsb_mode.attr,
+ &dev_attr_dsb_edge_ctrl.attr,
+ &dev_attr_dsb_edge_ctrl_mask.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 49fffb1..f81bfe8 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)
@@ -34,6 +36,16 @@
#define TPDM_DSB_TEST_MODE GENMASK(10, 9)
#define TPDM_DSB_HPSEL GENMASK(6, 2)
+#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)
@@ -60,14 +72,24 @@
#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
+
/**
* struct dsb_dataset - specifics associated to dsb dataset
* @mode: DSB programming mode
+ * @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[TPDM_DSB_MAX_EDCR];
+ u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCMR];
bool trig_ts;
bool trig_type;
};
--
2.7.4
Introduce the new subtype of "CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM"
for TPDM components in driver.
Signed-off-by: Tao Zhang <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 1 +
drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
include/linux/coresight.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 118fcf2..a8c52aa 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;
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 949aa24..29cd6d8 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -61,6 +61,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
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 <[email protected]>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 46 +++++
drivers/hwtracing/coresight/coresight-tpdm.c | 187 ++++++++++++++++++++-
drivers/hwtracing/coresight/coresight-tpdm.h | 14 ++
3 files changed, 242 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index 55ec81d..6e1b246 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -120,3 +120,49 @@ Description:
Where:
<integer1> : Index number of XPMR register, the range is 0 to 7
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_val
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the pattern value of DSB tpdm. Read
+ the pattern value of DSB tpdm.
+
+ Accepts the following two values.
+ value 1: Index number of TPR register, the range is 0 to 7
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_mask
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the pattern mask of DSB tpdm. Read
+ the pattern mask of DSB tpdm.
+
+ Accepts the following two values.
+ value 1: Index number of TPMR register, the range is 0 to 7
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+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 974e63f..1aff244 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -69,6 +69,27 @@ static void set_dsb_perf_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;
@@ -81,6 +102,10 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
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_val[i],
drvdata->base + TPDM_DSB_XPR(i));
writel_relaxed(drvdata->dsb->trig_patt_mask[i],
@@ -88,11 +113,7 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
}
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);
@@ -448,6 +469,158 @@ static ssize_t dsb_edge_ctrl_mask_store(struct device *dev,
}
static DEVICE_ATTR_RW(dsb_edge_ctrl_mask);
+static ssize_t dsb_patt_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i = 0;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index: 0x%x Value: 0x%x\n", i,
+ drvdata->dsb->patt_val[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+/*
+ * value 1: Index of TPR register
+ * value 2: Value need to be written
+ */
+static ssize_t dsb_patt_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 index, val;
+
+ if (sscanf(buf, "%lx %lx", &index, &val) != 2)
+ return -EINVAL;
+ if (index >= TPDM_DSB_MAX_PATT)
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->dsb->patt_val[index] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_patt_val);
+
+static ssize_t dsb_patt_mask_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ ssize_t size = 0;
+ unsigned long bytes;
+ int i = 0;
+
+ spin_lock(&drvdata->spinlock);
+ for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+ bytes = sysfs_emit_at(buf, size,
+ "Index: 0x%x Value: 0x%x\n", i,
+ drvdata->dsb->patt_mask[i]);
+ if (bytes <= 0)
+ break;
+ size += bytes;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+
+/*
+ * value 1: Index of TPMR register
+ * value 2: Value need to be written
+ */
+static ssize_t dsb_patt_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 index, val;
+
+ if (sscanf(buf, "%lx %lx", &index, &val) != 2)
+ return -EINVAL;
+ if (index >= TPDM_DSB_MAX_PATT)
+ return -EPERM;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->dsb->patt_mask[index] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_patt_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_patt_val_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -606,6 +779,10 @@ static struct attribute *tpdm_dsb_attrs[] = {
&dev_attr_dsb_mode.attr,
&dev_attr_dsb_edge_ctrl.attr,
&dev_attr_dsb_edge_ctrl_mask.attr,
+ &dev_attr_dsb_patt_val.attr,
+ &dev_attr_dsb_patt_mask.attr,
+ &dev_attr_dsb_patt_ts.attr,
+ &dev_attr_dsb_patt_type.attr,
&dev_attr_dsb_trig_patt_val.attr,
&dev_attr_dsb_trig_patt_mask.attr,
&dev_attr_dsb_trig_ts.attr,
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
index 87d946e..3169fb5 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))
@@ -24,8 +26,12 @@
/* Enable bit for DSB subunit trigger type */
#define TPDM_DSB_CR_TRIG_TYPE BIT(12)
+/* 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 */
/* Test mode control bit*/
@@ -87,8 +93,12 @@
* @mode: DSB programming mode
* @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_val: 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.
*/
@@ -96,8 +106,12 @@ struct dsb_dataset {
u32 mode;
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_val[TPDM_DSB_MAX_PATT];
u32 trig_patt_mask[TPDM_DSB_MAX_PATT];
+ bool patt_ts;
+ bool patt_type;
bool trig_ts;
bool trig_type;
};
--
2.7.4
On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>
> Signed-off-by: Tao Zhang <[email protected]>
> ---
> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
> drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
> 3 files changed, 196 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index 2a82cd0..34189e4a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -60,3 +60,35 @@ 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
> +Date: March 2023
> +KernelVersion 6.5
> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
> +Description:
> + Read/Write a set of the edge control registers of the DSB
> + in TPDM.
> +
> + Expected format is the following:
> + <integer1> <integer2> <integer3>
sysfs is "one value", not 3. Please never have to parse a sysfs file.
> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + ssize_t size = 0;
> + unsigned long bytes;
> + int i;
> +
> + spin_lock(&drvdata->spinlock);
> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
> + bytes = sysfs_emit_at(buf, size,
> + "Index:0x%x Val:0x%x\n", i,
Again, no, one value, no "string" needed to parse anything.
greg k-h
On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>
>> Signed-off-by: Tao Zhang <[email protected]>
>> ---
>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>> drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index 2a82cd0..34189e4a 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -60,3 +60,35 @@ 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
>> +Date: March 2023
>> +KernelVersion 6.5
>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
>> +Description:
>> + Read/Write a set of the edge control registers of the DSB
>> + in TPDM.
>> +
>> + Expected format is the following:
>> + <integer1> <integer2> <integer3>
> sysfs is "one value", not 3. Please never have to parse a sysfs file.
Do you mean sysfs file can only accept "one value"?
I see that more than one value are written to the sysfs file
"trigout_attach".
>
>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + ssize_t size = 0;
>> + unsigned long bytes;
>> + int i;
>> +
>> + spin_lock(&drvdata->spinlock);
>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>> + bytes = sysfs_emit_at(buf, size,
>> + "Index:0x%x Val:0x%x\n", i,
> Again, no, one value, no "string" needed to parse anything.
I also see other sysfs files can be read more than one value in other
drivers.
Is this "one value" limitation the usage rule of Linux sysfs system?
Or am I misunderstanding what you mean?
Best,
Tao
>
> greg k-h
On Tue, Jun 20, 2023 at 04:31:59PM +0800, Tao Zhang wrote:
>
> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
> > On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
> > >
> > > Signed-off-by: Tao Zhang <[email protected]>
> > > ---
> > > .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
> > > drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
> > > drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
> > > 3 files changed, 196 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > index 2a82cd0..34189e4a 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > @@ -60,3 +60,35 @@ 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
> > > +Date: March 2023
> > > +KernelVersion 6.5
> > > +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
> > > +Description:
> > > + Read/Write a set of the edge control registers of the DSB
> > > + in TPDM.
> > > +
> > > + Expected format is the following:
> > > + <integer1> <integer2> <integer3>
> > sysfs is "one value", not 3. Please never have to parse a sysfs file.
>
> Do you mean sysfs file can only accept "one value"?
Yes.
> I see that more than one value are written to the sysfs file
> "trigout_attach".
Then someone missed that and it needs to be fixed.
> > > +static ssize_t dsb_edge_ctrl_show(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > > + ssize_t size = 0;
> > > + unsigned long bytes;
> > > + int i;
> > > +
> > > + spin_lock(&drvdata->spinlock);
> > > + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
> > > + bytes = sysfs_emit_at(buf, size,
> > > + "Index:0x%x Val:0x%x\n", i,
> > Again, no, one value, no "string" needed to parse anything.
>
> I also see other sysfs files can be read more than one value in other
> drivers.
Again, they are broken, please send patches to fix them.
> Is this "one value" limitation the usage rule of Linux sysfs system?
Yes.
thanks,
greg k-h
On 20/06/2023 09:31, Tao Zhang wrote:
>
> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>
>>> Signed-off-by: Tao Zhang <[email protected]>
>>> ---
>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>> ++++++++++++++++++++-
>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> index 2a82cd0..34189e4a 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>> @@ -60,3 +60,35 @@ 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
>>> +Date: March 2023
>>> +KernelVersion 6.5
>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang
>>> (QUIC) <[email protected]>
>>> +Description:
>>> + Read/Write a set of the edge control registers of the DSB
>>> + in TPDM.
>>> +
>>> + Expected format is the following:
>>> + <integer1> <integer2> <integer3>
>> sysfs is "one value", not 3. Please never have to parse a sysfs file.
>
> Do you mean sysfs file can only accept "one value"?
>
> I see that more than one value are written to the sysfs file
> "trigout_attach".
>
>>
>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> + ssize_t size = 0;
>>> + unsigned long bytes;
>>> + int i;
>>> +
>>> + spin_lock(&drvdata->spinlock);
>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>> + bytes = sysfs_emit_at(buf, size,
>>> + "Index:0x%x Val:0x%x\n", i,
>> Again, no, one value, no "string" needed to parse anything.
>
> I also see other sysfs files can be read more than one value in other
> drivers.
>
> Is this "one value" limitation the usage rule of Linux sysfs system?
>
> Or am I misunderstanding what you mean?
Please fix the other sysfs tunables in the following patches.
Kind regards
Suzuki
On Tue, 20 Jun 2023 15:32:40 +0800, Tao Zhang wrote:
> 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 <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Wed, Jun 28, 2023 at 11:11:29AM +0800, Tao Zhang wrote:
>
> On 6/20/2023 4:49 PM, Greg Kroah-Hartman wrote:
> > On Tue, Jun 20, 2023 at 04:31:59PM +0800, Tao Zhang wrote:
> > > On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
> > > > On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
> > > > >
> > > > > Signed-off-by: Tao Zhang<[email protected]>
> > > > > ---
> > > > > .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
> > > > > drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
> > > > > drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
> > > > > 3 files changed, 196 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > > > index 2a82cd0..34189e4a 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > > > > @@ -60,3 +60,35 @@ 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
> > > > > +Date: March 2023
> > > > > +KernelVersion 6.5
> > > > > +Contact: Jinlong Mao (QUIC)<[email protected]>, Tao Zhang (QUIC)<[email protected]>
> > > > > +Description:
> > > > > + Read/Write a set of the edge control registers of the DSB
> > > > > + in TPDM.
> > > > > +
> > > > > + Expected format is the following:
> > > > > + <integer1> <integer2> <integer3>
> > > > sysfs is "one value", not 3. Please never have to parse a sysfs file.
> > > Do you mean sysfs file can only accept "one value"?
> > Yes.
>
> Hi Greg,
>
>
> I‘d like to clarify the usage of this sysfs file again.
>
> In the current design, three integers will be written to "dsb_edge_ctrl" to
> configure DSB edge detection.
>
> Integer #1: The start number of edge detection which needs to be configured.
>
> Integer #2: The end number of edge detection which needs to be configured.
>
> Integer #3: The type of the edge detection needs to be configured.
All of this is wrong. Again, sysfs is "one value per file"
> Below is an example.
>
> echo 0x3 0x25 0x1 > dsb_edge_ctrl
>
> It will configure edge detection #3 to #37 as "falling edge detection".
>
> Since these three integers are interrelated and written to achieve the same
> function, can we use these three integers as "one tuple" here?
Nope!
sorry,
greg k-h
On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
> On 20/06/2023 09:31, Tao Zhang wrote:
>>
>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>
>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>> ---
>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>> ++++++++++++++++++++-
>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git
>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> index 2a82cd0..34189e4a 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>> @@ -60,3 +60,35 @@ 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
>>>> +Date: March 2023
>>>> +KernelVersion 6.5
>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>> Zhang (QUIC) <[email protected]>
>>>> +Description:
>>>> + Read/Write a set of the edge control registers of the DSB
>>>> + in TPDM.
>>>> +
>>>> + Expected format is the following:
>>>> + <integer1> <integer2> <integer3>
>>> sysfs is "one value", not 3. Please never have to parse a sysfs file.
>>
>> Do you mean sysfs file can only accept "one value"?
>>
>> I see that more than one value are written to the sysfs file
>> "trigout_attach".
>>
>>>
>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>> + ssize_t size = 0;
>>>> + unsigned long bytes;
>>>> + int i;
>>>> +
>>>> + spin_lock(&drvdata->spinlock);
>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>> + bytes = sysfs_emit_at(buf, size,
>>>> + "Index:0x%x Val:0x%x\n", i,
>>> Again, no, one value, no "string" needed to parse anything.
>>
>> I also see other sysfs files can be read more than one value in other
>> drivers.
>>
>> Is this "one value" limitation the usage rule of Linux sysfs system?
>>
>> Or am I misunderstanding what you mean?
>
> Please fix the other sysfs tunables in the following patches.
List a new solution for the similar cases below, please see if this
design is reasonable?
1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val") will be
created in this case.
2. First write to the node "dsb_edge_ctrl_idx" to set the index number
of the edge detection.
3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
edge detection.
For example, if we need need to set "Falling edge detection" to the edge
detection #220-#222, we can issue the following commands.
echo 0xdc > tpdm1/dsb_edge_ctrl_idx
echo 0x1 > tpdm1/dsb_edge_ctrl_val
echo 0xdd > tpdm1/dsb_edge_ctrl_idx
echo 0x1 > tpdm1/dsb_edge_ctrl_val
echo 0xde > tpdm1/dsb_edge_ctrl_idx
echo 0x1 > tpdm1/dsb_edge_ctrl_val
If this design is acceptable, we will rewrite other similar nodes based
on this solution.
Let me know if you have any concerns or good suggestions for this solution.
Best,
Tao
>
> Kind regards
> Suzuki
>
>
HI Tao,
On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]> wrote:
>
>
> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
> > On 20/06/2023 09:31, Tao Zhang wrote:
> >>
> >> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
> >>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
> >>>>
> >>>> Signed-off-by: Tao Zhang <[email protected]>
> >>>> ---
> >>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
> >>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
> >>>> ++++++++++++++++++++-
> >>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
> >>>> 3 files changed, 196 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git
> >>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >>>> index 2a82cd0..34189e4a 100644
> >>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >>>> @@ -60,3 +60,35 @@ 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
> >>>> +Date: March 2023
> >>>> +KernelVersion 6.5
> >>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
> >>>> Zhang (QUIC) <[email protected]>
> >>>> +Description:
> >>>> + Read/Write a set of the edge control registers of the DSB
> >>>> + in TPDM.
> >>>> +
> >>>> + Expected format is the following:
> >>>> + <integer1> <integer2> <integer3>
> >>> sysfs is "one value", not 3. Please never have to parse a sysfs file.
> >>
> >> Do you mean sysfs file can only accept "one value"?
> >>
> >> I see that more than one value are written to the sysfs file
> >> "trigout_attach".
> >>
> >>>
> >>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
> >>>> + struct device_attribute *attr,
> >>>> + char *buf)
> >>>> +{
> >>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> >>>> + ssize_t size = 0;
> >>>> + unsigned long bytes;
> >>>> + int i;
> >>>> +
> >>>> + spin_lock(&drvdata->spinlock);
> >>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
> >>>> + bytes = sysfs_emit_at(buf, size,
> >>>> + "Index:0x%x Val:0x%x\n", i,
> >>> Again, no, one value, no "string" needed to parse anything.
> >>
> >> I also see other sysfs files can be read more than one value in other
> >> drivers.
> >>
> >> Is this "one value" limitation the usage rule of Linux sysfs system?
> >>
> >> Or am I misunderstanding what you mean?
> >
> > Please fix the other sysfs tunables in the following patches.
>
> List a new solution for the similar cases below, please see if this
> design is reasonable?
>
> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val") will be
> created in this case.
>
> 2. First write to the node "dsb_edge_ctrl_idx" to set the index number
> of the edge detection.
>
> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
> edge detection.
>
> For example, if we need need to set "Falling edge detection" to the edge
> detection #220-#222, we can issue the following commands.
>
> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>
> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>
> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>
> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>
> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>
> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>
> If this design is acceptable, we will rewrite other similar nodes based
> on this solution.
>
This index / value model is used in the coresight drivers so should be
OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
index selects the counter, and the other val registers are applied to
that counter.
Mike
> Let me know if you have any concerns or good suggestions for this solution.
>
>
> Best,
>
> Tao
>
> >
> > Kind regards
> > Suzuki
> >
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
On 13/07/2023 09:54, Mike Leach wrote:
> HI Tao,
>
> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]> wrote:
>>
>>
>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>
>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>
>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>> ---
>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>> ++++++++++++++++++++-
>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>> index 2a82cd0..34189e4a 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>> +Date: March 2023
>>>>>> +KernelVersion 6.5
>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>> Zhang (QUIC) <[email protected]>
>>>>>> +Description:
>>>>>> + Read/Write a set of the edge control registers of the DSB
>>>>>> + in TPDM.
>>>>>> +
>>>>>> + Expected format is the following:
>>>>>> + <integer1> <integer2> <integer3>
>>>>> sysfs is "one value", not 3. Please never have to parse a sysfs file.
>>>>
>>>> Do you mean sysfs file can only accept "one value"?
>>>>
>>>> I see that more than one value are written to the sysfs file
>>>> "trigout_attach".
>>>>
>>>>>
>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>> + struct device_attribute *attr,
>>>>>> + char *buf)
>>>>>> +{
>>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>> + ssize_t size = 0;
>>>>>> + unsigned long bytes;
>>>>>> + int i;
>>>>>> +
>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>
>>>> I also see other sysfs files can be read more than one value in other
>>>> drivers.
>>>>
>>>> Is this "one value" limitation the usage rule of Linux sysfs system?
>>>>
>>>> Or am I misunderstanding what you mean?
>>>
>>> Please fix the other sysfs tunables in the following patches.
>>
>> List a new solution for the similar cases below, please see if this
>> design is reasonable?
>>
>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val") will be
>> created in this case.
>>
>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index number
>> of the edge detection.
>>
>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
>> edge detection.
>>
>> For example, if we need need to set "Falling edge detection" to the edge
>> detection #220-#222, we can issue the following commands.
>>
>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>
>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>
>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>
>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>
>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>
>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>
>> If this design is acceptable, we will rewrite other similar nodes based
>> on this solution.
>>
>
> This index / value model is used in the coresight drivers so should be
> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
> index selects the counter, and the other val registers are applied to
> that counter.
True. That model is useful when there are variable number of "counters".
I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
EDCR.
e.g, edcr0...edcr15
Given there are only 16 of them, it is fine to keep a file for each.
This may be grouped under "mgmt" similar to what we have for other
components. That way, it can be easily hidden by checking for the
presence of DSB.
Suzuki
On 7/13/2023 5:34 PM, Suzuki K Poulose wrote:
> On 13/07/2023 09:54, Mike Leach wrote:
>> HI Tao,
>>
>> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]> wrote:
>>>
>>>
>>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>>
>>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>>
>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>> ---
>>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>>> ++++++++++++++++++++-
>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>> index 2a82cd0..34189e4a 100644
>>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>>> +Date: March 2023
>>>>>>> +KernelVersion 6.5
>>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>>> Zhang (QUIC) <[email protected]>
>>>>>>> +Description:
>>>>>>> + Read/Write a set of the edge control registers of the DSB
>>>>>>> + in TPDM.
>>>>>>> +
>>>>>>> + Expected format is the following:
>>>>>>> + <integer1> <integer2> <integer3>
>>>>>> sysfs is "one value", not 3. Please never have to parse a sysfs
>>>>>> file.
>>>>>
>>>>> Do you mean sysfs file can only accept "one value"?
>>>>>
>>>>> I see that more than one value are written to the sysfs file
>>>>> "trigout_attach".
>>>>>
>>>>>>
>>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>>> + struct device_attribute *attr,
>>>>>>> + char *buf)
>>>>>>> +{
>>>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>>> + ssize_t size = 0;
>>>>>>> + unsigned long bytes;
>>>>>>> + int i;
>>>>>>> +
>>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>>
>>>>> I also see other sysfs files can be read more than one value in other
>>>>> drivers.
>>>>>
>>>>> Is this "one value" limitation the usage rule of Linux sysfs system?
>>>>>
>>>>> Or am I misunderstanding what you mean?
>>>>
>>>> Please fix the other sysfs tunables in the following patches.
>>>
>>> List a new solution for the similar cases below, please see if this
>>> design is reasonable?
>>>
>>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val") will be
>>> created in this case.
>>>
>>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index number
>>> of the edge detection.
>>>
>>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
>>> edge detection.
>>>
>>> For example, if we need need to set "Falling edge detection" to the
>>> edge
>>> detection #220-#222, we can issue the following commands.
>>>
>>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>>
>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>
>>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>>
>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>
>>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>>
>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>
>>> If this design is acceptable, we will rewrite other similar nodes based
>>> on this solution.
>>>
>>
>> This index / value model is used in the coresight drivers so should be
>> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
>> index selects the counter, and the other val registers are applied to
>> that counter.
>
> True. That model is useful when there are variable number of "counters".
> I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
> EDCR.
>
> e.g, edcr0...edcr15
>
> Given there are only 16 of them, it is fine to keep a file for each.
> This may be grouped under "mgmt" similar to what we have for other
> components. That way, it can be easily hidden by checking for the
> presence of DSB.
The number of EDCR registers is not fixed. The maximum range is [0:15].
But the address of the maximum number of the registers will be reserved
first,
and can be accessed safely even if the TPDM doesn't have the maximum number
of EDCR registers.
And we are not able to dynamically know the number of EDCR registers per DSB
TPDM.
Can we use our proposal in this case?
Best,
Tao
>
> Suzuki
>
On 13/07/2023 17:13, Tao Zhang wrote:
>
> On 7/13/2023 5:34 PM, Suzuki K Poulose wrote:
>> On 13/07/2023 09:54, Mike Leach wrote:
>>> HI Tao,
>>>
>>> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]> wrote:
>>>>
>>>>
>>>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>>>
>>>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>>>
>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>> ---
>>>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>>>> ++++++++++++++++++++-
>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>> index 2a82cd0..34189e4a 100644
>>>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>>>> +Date: March 2023
>>>>>>>> +KernelVersion 6.5
>>>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>>>> Zhang (QUIC) <[email protected]>
>>>>>>>> +Description:
>>>>>>>> + Read/Write a set of the edge control registers of the DSB
>>>>>>>> + in TPDM.
>>>>>>>> +
>>>>>>>> + Expected format is the following:
>>>>>>>> + <integer1> <integer2> <integer3>
>>>>>>> sysfs is "one value", not 3. Please never have to parse a sysfs
>>>>>>> file.
>>>>>>
>>>>>> Do you mean sysfs file can only accept "one value"?
>>>>>>
>>>>>> I see that more than one value are written to the sysfs file
>>>>>> "trigout_attach".
>>>>>>
>>>>>>>
>>>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>>>> + struct device_attribute *attr,
>>>>>>>> + char *buf)
>>>>>>>> +{
>>>>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>>>> + ssize_t size = 0;
>>>>>>>> + unsigned long bytes;
>>>>>>>> + int i;
>>>>>>>> +
>>>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>>>
>>>>>> I also see other sysfs files can be read more than one value in other
>>>>>> drivers.
>>>>>>
>>>>>> Is this "one value" limitation the usage rule of Linux sysfs system?
>>>>>>
>>>>>> Or am I misunderstanding what you mean?
>>>>>
>>>>> Please fix the other sysfs tunables in the following patches.
>>>>
>>>> List a new solution for the similar cases below, please see if this
>>>> design is reasonable?
>>>>
>>>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val") will be
>>>> created in this case.
>>>>
>>>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index number
>>>> of the edge detection.
>>>>
>>>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
>>>> edge detection.
>>>>
>>>> For example, if we need need to set "Falling edge detection" to the
>>>> edge
>>>> detection #220-#222, we can issue the following commands.
>>>>
>>>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>>>
>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>
>>>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>>>
>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>
>>>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>>>
>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>
>>>> If this design is acceptable, we will rewrite other similar nodes based
>>>> on this solution.
>>>>
>>>
>>> This index / value model is used in the coresight drivers so should be
>>> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
>>> index selects the counter, and the other val registers are applied to
>>> that counter.
>>
>> True. That model is useful when there are variable number of "counters".
>> I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
>> EDCR.
>>
>> e.g, edcr0...edcr15
>>
>> Given there are only 16 of them, it is fine to keep a file for each.
>> This may be grouped under "mgmt" similar to what we have for other
>> components. That way, it can be easily hidden by checking for the
>> presence of DSB.
>
> The number of EDCR registers is not fixed. The maximum range is [0:15].
>
> But the address of the maximum number of the registers will be reserved
> first,
>
> and can be accessed safely even if the TPDM doesn't have the maximum number
>
> of EDCR registers.
>
> And we are not able to dynamically know the number of EDCR registers per
> DSB
>
> TPDM.
>
> Can we use our proposal in this case?
Please provide a file edcrN for each of the 0 <= N < 16. That way it is
easier to avoid locking the index. It doesn't matter how many EDCRs are
supported, there is a maximum limit and it is always guaranteed to be
write safe, if some are not implemented. Thus it is much easier from a
programming perspective too.
Suzuki
>
>
> Best,
>
> Tao
>
>>
>> Suzuki
>>
On 7/14/2023 12:37 AM, Suzuki K Poulose wrote:
> On 13/07/2023 17:13, Tao Zhang wrote:
>>
>> On 7/13/2023 5:34 PM, Suzuki K Poulose wrote:
>>> On 13/07/2023 09:54, Mike Leach wrote:
>>>> HI Tao,
>>>>
>>>> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>>>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>>>>
>>>>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>> ---
>>>>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>>>>> ++++++++++++++++++++-
>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>> index 2a82cd0..34189e4a 100644
>>>>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>>>>> +Date: March 2023
>>>>>>>>> +KernelVersion 6.5
>>>>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>>>>> Zhang (QUIC) <[email protected]>
>>>>>>>>> +Description:
>>>>>>>>> + Read/Write a set of the edge control registers of the
>>>>>>>>> DSB
>>>>>>>>> + in TPDM.
>>>>>>>>> +
>>>>>>>>> + Expected format is the following:
>>>>>>>>> + <integer1> <integer2> <integer3>
>>>>>>>> sysfs is "one value", not 3. Please never have to parse a
>>>>>>>> sysfs file.
>>>>>>>
>>>>>>> Do you mean sysfs file can only accept "one value"?
>>>>>>>
>>>>>>> I see that more than one value are written to the sysfs file
>>>>>>> "trigout_attach".
>>>>>>>
>>>>>>>>
>>>>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>>>>> + struct device_attribute *attr,
>>>>>>>>> + char *buf)
>>>>>>>>> +{
>>>>>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>>>>> + ssize_t size = 0;
>>>>>>>>> + unsigned long bytes;
>>>>>>>>> + int i;
>>>>>>>>> +
>>>>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>>>>
>>>>>>> I also see other sysfs files can be read more than one value in
>>>>>>> other
>>>>>>> drivers.
>>>>>>>
>>>>>>> Is this "one value" limitation the usage rule of Linux sysfs
>>>>>>> system?
>>>>>>>
>>>>>>> Or am I misunderstanding what you mean?
>>>>>>
>>>>>> Please fix the other sysfs tunables in the following patches.
>>>>>
>>>>> List a new solution for the similar cases below, please see if this
>>>>> design is reasonable?
>>>>>
>>>>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val")
>>>>> will be
>>>>> created in this case.
>>>>>
>>>>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index
>>>>> number
>>>>> of the edge detection.
>>>>>
>>>>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
>>>>> edge detection.
>>>>>
>>>>> For example, if we need need to set "Falling edge detection" to
>>>>> the edge
>>>>> detection #220-#222, we can issue the following commands.
>>>>>
>>>>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>>>>
>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>
>>>>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>>>>
>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>
>>>>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>>>>
>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>
>>>>> If this design is acceptable, we will rewrite other similar nodes
>>>>> based
>>>>> on this solution.
>>>>>
>>>>
>>>> This index / value model is used in the coresight drivers so should be
>>>> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
>>>> index selects the counter, and the other val registers are applied to
>>>> that counter.
>>>
>>> True. That model is useful when there are variable number of
>>> "counters".
>>> I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
>>> EDCR.
>>>
>>> e.g, edcr0...edcr15
>>>
>>> Given there are only 16 of them, it is fine to keep a file for each.
>>> This may be grouped under "mgmt" similar to what we have for other
>>> components. That way, it can be easily hidden by checking for the
>>> presence of DSB.
>>
>> The number of EDCR registers is not fixed. The maximum range is [0:15].
>>
>> But the address of the maximum number of the registers will be
>> reserved first,
>>
>> and can be accessed safely even if the TPDM doesn't have the maximum
>> number
>>
>> of EDCR registers.
>>
>> And we are not able to dynamically know the number of EDCR registers
>> per DSB
>>
>> TPDM.
>>
>> Can we use our proposal in this case?
>
> Please provide a file edcrN for each of the 0 <= N < 16. That way it is
> easier to avoid locking the index. It doesn't matter how many EDCRs are
> supported, there is a maximum limit and it is always guaranteed to be
> write safe, if some are not implemented. Thus it is much easier from a
> programming perspective too.
Hi Suzuki,
Thanks for the suggestion.
I'd like to further clarify our proposal below in case I didn't express
it clearly before.
1. In our design, the users don't need to know the mapping between the
number of the edge detection
and the control bits in EDCRN registers. They only need to focus on the
edge detection they need, don't
need to care about the design of the HW.
2. For example, if there are two users configure in the same test. One
needs to configure edge detection #7
as "Falling edge detection". The other one needs to configure edge
detection #8 as "toggle detection". They will
issue the following commands to implement it.
echo 0x7 > tpdm1/dsb_edge_ctrl_idx
echo 0x1 > tpdm1/dsb_edge_ctrl_val
echo 0x8 > tpdm1/dsb_edge_ctrl_idx
echo 0x2 > tpdm1/dsb_edge_ctrl_val
The value written to edcr0 will be 0x24000 in our proposal.
But in the solution of "tpdm1/dsb_edge_ctrl/edcrN 0 <= N < 16".
One user calculates that he needs to write 0x4000 to edcr0.
echo 0x4000 > tpdm1/dsb_edge_ctrl/edcr0
The other one calculates that he needs to write 0x20000 to edcr0.
echo 0x20000 > tpdm1/dsb_edge_ctrl/edcr0
The last write will overwrite the previous value in this case and 0x20000
will be written to the edcr0 finally.
3. Some DSB TPDMs may not have 16 EDCR registers. For example, TPDM2
may only have 7 EDCR registers. If we still create 16 edcr file at
tpdm2/dsb_edge_ctrl,
this may confuse users.
Based on the above points, is it possible to re-evaluate our proposal?
Best,
Tao
>
> Suzuki
>
>
>
>>
>>
>> Best,
>>
>> Tao
>>
>>>
>>> Suzuki
>>>
>
On 14/07/2023 06:50, Tao Zhang wrote:
>
> On 7/14/2023 12:37 AM, Suzuki K Poulose wrote:
>> On 13/07/2023 17:13, Tao Zhang wrote:
>>>
>>> On 7/13/2023 5:34 PM, Suzuki K Poulose wrote:
>>>> On 13/07/2023 09:54, Mike Leach wrote:
>>>>> HI Tao,
>>>>>
>>>>> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>>>>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>>>>>
>>>>>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>>> ---
>>>>>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>>>>>> ++++++++++++++++++++-
>>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> diff --git
>>>>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>> index 2a82cd0..34189e4a 100644
>>>>>>>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>>>>>> +Date: March 2023
>>>>>>>>>> +KernelVersion 6.5
>>>>>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>>>>>> Zhang (QUIC) <[email protected]>
>>>>>>>>>> +Description:
>>>>>>>>>> + Read/Write a set of the edge control registers of the
>>>>>>>>>> DSB
>>>>>>>>>> + in TPDM.
>>>>>>>>>> +
>>>>>>>>>> + Expected format is the following:
>>>>>>>>>> + <integer1> <integer2> <integer3>
>>>>>>>>> sysfs is "one value", not 3. Please never have to parse a
>>>>>>>>> sysfs file.
>>>>>>>>
>>>>>>>> Do you mean sysfs file can only accept "one value"?
>>>>>>>>
>>>>>>>> I see that more than one value are written to the sysfs file
>>>>>>>> "trigout_attach".
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>>>>>> + struct device_attribute *attr,
>>>>>>>>>> + char *buf)
>>>>>>>>>> +{
>>>>>>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>>>>>>>> + ssize_t size = 0;
>>>>>>>>>> + unsigned long bytes;
>>>>>>>>>> + int i;
>>>>>>>>>> +
>>>>>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>>>>>
>>>>>>>> I also see other sysfs files can be read more than one value in
>>>>>>>> other
>>>>>>>> drivers.
>>>>>>>>
>>>>>>>> Is this "one value" limitation the usage rule of Linux sysfs
>>>>>>>> system?
>>>>>>>>
>>>>>>>> Or am I misunderstanding what you mean?
>>>>>>>
>>>>>>> Please fix the other sysfs tunables in the following patches.
>>>>>>
>>>>>> List a new solution for the similar cases below, please see if this
>>>>>> design is reasonable?
>>>>>>
>>>>>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val")
>>>>>> will be
>>>>>> created in this case.
>>>>>>
>>>>>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index
>>>>>> number
>>>>>> of the edge detection.
>>>>>>
>>>>>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value of the
>>>>>> edge detection.
>>>>>>
>>>>>> For example, if we need need to set "Falling edge detection" to
>>>>>> the edge
>>>>>> detection #220-#222, we can issue the following commands.
>>>>>>
>>>>>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>>>>>
>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>
>>>>>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>>>>>
>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>
>>>>>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>>>>>
>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>
>>>>>> If this design is acceptable, we will rewrite other similar nodes
>>>>>> based
>>>>>> on this solution.
>>>>>>
>>>>>
>>>>> This index / value model is used in the coresight drivers so should be
>>>>> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
>>>>> index selects the counter, and the other val registers are applied to
>>>>> that counter.
>>>>
>>>> True. That model is useful when there are variable number of
>>>> "counters".
>>>> I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
>>>> EDCR.
>>>>
>>>> e.g, edcr0...edcr15
>>>>
>>>> Given there are only 16 of them, it is fine to keep a file for each.
>>>> This may be grouped under "mgmt" similar to what we have for other
>>>> components. That way, it can be easily hidden by checking for the
>>>> presence of DSB.
>>>
>>> The number of EDCR registers is not fixed. The maximum range is [0:15].
>>>
>>> But the address of the maximum number of the registers will be
>>> reserved first,
>>>
>>> and can be accessed safely even if the TPDM doesn't have the maximum
>>> number
>>>
>>> of EDCR registers.
>>>
>>> And we are not able to dynamically know the number of EDCR registers
>>> per DSB
>>>
>>> TPDM.
>>>
>>> Can we use our proposal in this case?
>>
>> Please provide a file edcrN for each of the 0 <= N < 16. That way it is
>> easier to avoid locking the index. It doesn't matter how many EDCRs are
>> supported, there is a maximum limit and it is always guaranteed to be
>> write safe, if some are not implemented. Thus it is much easier from a
>> programming perspective too.
>
> Hi Suzuki,
>
>
> Thanks for the suggestion.
>
> I'd like to further clarify our proposal below in case I didn't express
> it clearly before.
>
> 1. In our design, the users don't need to know the mapping between the
> number of the edge detection
>
> and the control bits in EDCRN registers. They only need to focus on the
> edge detection they need, don't
>
> need to care about the design of the HW.
Agreed
>
> 2. For example, if there are two users configure in the same test. One
> needs to configure edge detection #7
>
> as "Falling edge detection". The other one needs to configure edge
> detection #8 as "toggle detection". They will
>
> issue the following commands to implement it.
>
> echo 0x7 > tpdm1/dsb_edge_ctrl_idx
>
> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>
> echo 0x8 > tpdm1/dsb_edge_ctrl_idx
>
> echo 0x2 > tpdm1/dsb_edge_ctrl_val
>
> The value written to edcr0 will be 0x24000 in our proposal.
>
> But in the solution of "tpdm1/dsb_edge_ctrl/edcrN 0 <= N < 16".
>
> One user calculates that he needs to write 0x4000 to edcr0.
>
> echo 0x4000 > tpdm1/dsb_edge_ctrl/edcr0
>
> The other one calculates that he needs to write 0x20000 to edcr0.
>
> echo 0x20000 > tpdm1/dsb_edge_ctrl/edcr0
>
> The last write will overwrite the previous value in this case and 0x20000
>
> will be written to the edcr0 finally.
The solution of edcrN expects the users follow a Read-Modify-Write.
But given you want to control individual lines separately (which are 256
in number), I am fine with the _idx/value solution.
>
> 3. Some DSB TPDMs may not have 16 EDCR registers. For example, TPDM2
>
> may only have 7 EDCR registers. If we still create 16 edcr file at
> tpdm2/dsb_edge_ctrl,
>
> this may confuse users.
This is not relevant. The user can't know the maximum number anyway. If
the user knows TPDM2 has only 7 EDCR, then don't bother about the other
files.
Please go ahead with the _idx /_value
Suzuki
On 7/14/2023 6:24 PM, Suzuki K Poulose wrote:
> On 14/07/2023 06:50, Tao Zhang wrote:
>>
>> On 7/14/2023 12:37 AM, Suzuki K Poulose wrote:
>>> On 13/07/2023 17:13, Tao Zhang wrote:
>>>>
>>>> On 7/13/2023 5:34 PM, Suzuki K Poulose wrote:
>>>>> On 13/07/2023 09:54, Mike Leach wrote:
>>>>>> HI Tao,
>>>>>>
>>>>>> On Wed, 12 Jul 2023 at 14:53, Tao Zhang <[email protected]>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 6/20/2023 9:41 PM, Suzuki K Poulose wrote:
>>>>>>>> On 20/06/2023 09:31, Tao Zhang wrote:
>>>>>>>>>
>>>>>>>>> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
>>>>>>>>>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, 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.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Tao Zhang <[email protected]>
>>>>>>>>>>> ---
>>>>>>>>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>>>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143
>>>>>>>>>>> ++++++++++++++++++++-
>>>>>>>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>>>>>>>>>>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git
>>>>>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>>> index 2a82cd0..34189e4a 100644
>>>>>>>>>>> ---
>>>>>>>>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>>> +++
>>>>>>>>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>>>>>>>>>>> @@ -60,3 +60,35 @@ 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
>>>>>>>>>>> +Date: March 2023
>>>>>>>>>>> +KernelVersion 6.5
>>>>>>>>>>> +Contact: Jinlong Mao (QUIC) <[email protected]>, Tao
>>>>>>>>>>> Zhang (QUIC) <[email protected]>
>>>>>>>>>>> +Description:
>>>>>>>>>>> + Read/Write a set of the edge control registers of
>>>>>>>>>>> the DSB
>>>>>>>>>>> + in TPDM.
>>>>>>>>>>> +
>>>>>>>>>>> + Expected format is the following:
>>>>>>>>>>> + <integer1> <integer2> <integer3>
>>>>>>>>>> sysfs is "one value", not 3. Please never have to parse a
>>>>>>>>>> sysfs file.
>>>>>>>>>
>>>>>>>>> Do you mean sysfs file can only accept "one value"?
>>>>>>>>>
>>>>>>>>> I see that more than one value are written to the sysfs file
>>>>>>>>> "trigout_attach".
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>>>>>>>>>>> + struct device_attribute *attr,
>>>>>>>>>>> + char *buf)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tpdm_drvdata *drvdata =
>>>>>>>>>>> dev_get_drvdata(dev->parent);
>>>>>>>>>>> + ssize_t size = 0;
>>>>>>>>>>> + unsigned long bytes;
>>>>>>>>>>> + int i;
>>>>>>>>>>> +
>>>>>>>>>>> + spin_lock(&drvdata->spinlock);
>>>>>>>>>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>>>>>>>>>>> + bytes = sysfs_emit_at(buf, size,
>>>>>>>>>>> + "Index:0x%x Val:0x%x\n", i,
>>>>>>>>>> Again, no, one value, no "string" needed to parse anything.
>>>>>>>>>
>>>>>>>>> I also see other sysfs files can be read more than one value
>>>>>>>>> in other
>>>>>>>>> drivers.
>>>>>>>>>
>>>>>>>>> Is this "one value" limitation the usage rule of Linux sysfs
>>>>>>>>> system?
>>>>>>>>>
>>>>>>>>> Or am I misunderstanding what you mean?
>>>>>>>>
>>>>>>>> Please fix the other sysfs tunables in the following patches.
>>>>>>>
>>>>>>> List a new solution for the similar cases below, please see if this
>>>>>>> design is reasonable?
>>>>>>>
>>>>>>> 1. Two SysFS files("dsb_edge_ctrl_idx" and "dsb_edge_ctrl_val")
>>>>>>> will be
>>>>>>> created in this case.
>>>>>>>
>>>>>>> 2. First write to the node "dsb_edge_ctrl_idx" to set the index
>>>>>>> number
>>>>>>> of the edge detection.
>>>>>>>
>>>>>>> 3. Then write to the node "dsb_edge_ctrl_val" to set the value
>>>>>>> of the
>>>>>>> edge detection.
>>>>>>>
>>>>>>> For example, if we need need to set "Falling edge detection" to
>>>>>>> the edge
>>>>>>> detection #220-#222, we can issue the following commands.
>>>>>>>
>>>>>>> echo 0xdc > tpdm1/dsb_edge_ctrl_idx
>>>>>>>
>>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>>
>>>>>>> echo 0xdd > tpdm1/dsb_edge_ctrl_idx
>>>>>>>
>>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>>
>>>>>>> echo 0xde > tpdm1/dsb_edge_ctrl_idx
>>>>>>>
>>>>>>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>>>>>>
>>>>>>> If this design is acceptable, we will rewrite other similar
>>>>>>> nodes based
>>>>>>> on this solution.
>>>>>>>
>>>>>>
>>>>>> This index / value model is used in the coresight drivers so
>>>>>> should be
>>>>>> OK - eg etm4 has cntr_idx / cntrldvr / cntr_val / cntr_ctrl, where
>>>>>> index selects the counter, and the other val registers are
>>>>>> applied to
>>>>>> that counter.
>>>>>
>>>>> True. That model is useful when there are variable number of
>>>>> "counters".
>>>>> I guess it doesn't hurt to have a 64bit (or even 32bit) file for each
>>>>> EDCR.
>>>>>
>>>>> e.g, edcr0...edcr15
>>>>>
>>>>> Given there are only 16 of them, it is fine to keep a file for each.
>>>>> This may be grouped under "mgmt" similar to what we have for other
>>>>> components. That way, it can be easily hidden by checking for the
>>>>> presence of DSB.
>>>>
>>>> The number of EDCR registers is not fixed. The maximum range is
>>>> [0:15].
>>>>
>>>> But the address of the maximum number of the registers will be
>>>> reserved first,
>>>>
>>>> and can be accessed safely even if the TPDM doesn't have the
>>>> maximum number
>>>>
>>>> of EDCR registers.
>>>>
>>>> And we are not able to dynamically know the number of EDCR
>>>> registers per DSB
>>>>
>>>> TPDM.
>>>>
>>>> Can we use our proposal in this case?
>>>
>>> Please provide a file edcrN for each of the 0 <= N < 16. That way it is
>>> easier to avoid locking the index. It doesn't matter how many EDCRs are
>>> supported, there is a maximum limit and it is always guaranteed to be
>>> write safe, if some are not implemented. Thus it is much easier from
>>> a programming perspective too.
>>
>> Hi Suzuki,
>>
>>
>> Thanks for the suggestion.
>>
>> I'd like to further clarify our proposal below in case I didn't
>> express it clearly before.
>>
>> 1. In our design, the users don't need to know the mapping between
>> the number of the edge detection
>>
>> and the control bits in EDCRN registers. They only need to focus on
>> the edge detection they need, don't
>>
>> need to care about the design of the HW.
>
> Agreed
>
>>
>> 2. For example, if there are two users configure in the same test.
>> One needs to configure edge detection #7
>>
>> as "Falling edge detection". The other one needs to configure edge
>> detection #8 as "toggle detection". They will
>>
>> issue the following commands to implement it.
>>
>> echo 0x7 > tpdm1/dsb_edge_ctrl_idx
>>
>> echo 0x1 > tpdm1/dsb_edge_ctrl_val
>>
>> echo 0x8 > tpdm1/dsb_edge_ctrl_idx
>>
>> echo 0x2 > tpdm1/dsb_edge_ctrl_val
>>
>> The value written to edcr0 will be 0x24000 in our proposal.
>>
>> But in the solution of "tpdm1/dsb_edge_ctrl/edcrN 0 <= N < 16".
>>
>> One user calculates that he needs to write 0x4000 to edcr0.
>>
>> echo 0x4000 > tpdm1/dsb_edge_ctrl/edcr0
>>
>> The other one calculates that he needs to write 0x20000 to edcr0.
>>
>> echo 0x20000 > tpdm1/dsb_edge_ctrl/edcr0
>>
>> The last write will overwrite the previous value in this case and
>> 0x20000
>>
>> will be written to the edcr0 finally.
>
> The solution of edcrN expects the users follow a Read-Modify-Write.
> But given you want to control individual lines separately (which are 256
> in number), I am fine with the _idx/value solution.
>
>>
>> 3. Some DSB TPDMs may not have 16 EDCR registers. For example, TPDM2
>>
>> may only have 7 EDCR registers. If we still create 16 edcr file at
>> tpdm2/dsb_edge_ctrl,
>>
>> this may confuse users.
>
> This is not relevant. The user can't know the maximum number anyway.
> If the user knows TPDM2 has only 7 EDCR, then don't bother about the
> other
> files.
>
> Please go ahead with the _idx /_value
Thanks. I will update in the next patch series.
Best,
Tao
>
> Suzuki
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]