On 02/06/2023 15:38, Tao Zhang wrote:
On 6/2/2023 4:45 PM, Suzuki K Poulose wrote:On 02/06/2023 09:21, Tao Zhang wrote:On 6/1/2023 8:14 PM, Suzuki K Poulose wrote:You are right, not all DSB TPDMs have MAX_EDCR registers. In our design, the inexistent register addressesOn 27/04/2023 10:00, 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 <quic_taozha@xxxxxxxxxxx> --- .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++drivers/hwtracing/coresight/coresight-tpdm.c | 135 ++++++++++++++++++++-drivers/hwtracing/coresight/coresight-tpdm.h | 21 ++++ 3 files changed, 187 insertions(+), 1 deletion(-)diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdmindex 348e167..a57f000 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.3+Contact: Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang (QUIC) <quic_taozha@xxxxxxxxxxx>+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.3+Contact: Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang (QUIC) <quic_taozha@xxxxxxxxxxx>+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 writtendiff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.cindex 1bacaa5..a40e458 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c@@ -80,7 +80,14 @@ static void set_trigger_type(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));Do all TPDM DSBs have MAX_EDCR registers ? Or some have less than that ?If it is latter, do we need special care to avoid writing to inexistent registers ?are not occupied and safe for accessing.Currently we don't have a good way to know the quantity of EDCR/EDCMR registers for DSB TPDMs.The only way we can think of is to set it in device tree manually. Do you have other suggestion for this?val = readl_relaxed(drvdata->base + TPDM_DSB_TIER); /* Set trigger timestamp */@@ -313,6 +320,130 @@ 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; + int i; + + spin_lock(&drvdata->spinlock); + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) { + size += sysfs_emit_at(buf, size, + "Index:0x%x Val:0x%x\n", i, + drvdata->dsb->edge_ctrl[i]);It may be safe, but please add a check to make sure that we don't overflow. At least bail out when we hit a return of 0, indicating reached the end of buffer.Can I add the following check to replace the current code?? int ret = 0; for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {ret = sysfs_emit_at(buf, size, "Index:0x%x Val:0x%x\n", i, drvdata->dsb->edge_ctrl[i]);if (!ret) { dev_warn(drvdata->dev, "The buffer has been overflowed\n");You don't need this, it already triggers a WARN() in sysfs_emit_at(). So you could do: for (....) { unsigned long bytes = sysfs_emit_at(buf, size, ....); if (bytes <= 0) break; size += bytes; }Sure, I will update in the next patch series.spin_unlock(&drvdata->spinlock); return size; } else size += ret; }Perhaps "FIELD_PREP" cannot be used here since "mask" must be constant in this macro.+ } + 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, edge_ctrl_shift; + 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];+ edge_ctrl_shift = EDCR_TO_WORD_VAL(edge_ctrl, i); + bitmap_replace(&val, &val, &edge_ctrl_shift, &mask, 32);Could we simply do : reg &= ~mask; reg |= FIELD_PREP(mask, edge_ctrl);Ah, you are right. Sorry about that.But in our code, the variable "mask" is not constant.Still I think using the bitmap_replace is an overkill. We could simply do: val &= ~mask; val |= EDCR_TO_WORD_VAL(edge_ctrl, i);
Since we don't need mask any longer we could even do : val &= ~EDCR_TO_WORD_MASK(i); val |= EDCR_TO_WORD_VAL(edge_ctrl, i); Suzuki