CoreSight

coresight@lists.linaro.org

6 participants
2564 discussions

Re: [PATCH 04/10] Coresight: Enable BC and GPR for TPDM driver

by Mathieu Poirier

On Thu, Oct 21, 2021 at 03:38:50PM +0800, Tao Zhang wrote: > Enable GPR and Basic Counts(BC) for TPDM. Add GPR interface and But what is GPR? What does it stand for? Where is this documented? > basic control sysFS interface for TPDM. The GPR interface has RW > and RO fields for controlling external logic and mapping core > signals to an APB accessible address in the TPDM address map. > > Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > --- > drivers/hwtracing/coresight/coresight-tpdm.c | 334 +++++++++++++++++++ > 1 file changed, 334 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 906776c859d6..c0a01979e42f 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -276,6 +276,93 @@ struct tpdm_drvdata { > > static void tpdm_init_default_data(struct tpdm_drvdata *drvdata); > > +static void __tpdm_enable_gpr(struct tpdm_drvdata *drvdata) There is no need for the double underscore at the beginning of the function name, please remove. The same goes for __tpdm_config_bc_msr() and __tpdm_enable_bc(). > +{ > + int i; > + > + for (i = 0; i < TPDM_GPR_REGS_MAX; i++) { > + if (!test_bit(i, drvdata->gpr->gpr_dirty)) > + continue; > + tpdm_writel(drvdata, drvdata->gpr->gp_regs[i], TPDM_GPR_CR(i)); > + } > +} > + > +static void __tpdm_config_bc_msr(struct tpdm_drvdata *drvdata) > +{ > + int i; > + > + if (!drvdata->msr_support) Shouldn't msr_support be part of bc_dataset? And why do we have a function for this when an if() condition would work just as well, as it is done in bc_trig_type below? > + return; > + > + for (i = 0; i < TPDM_BC_MAX_MSR; i++) > + tpdm_writel(drvdata, drvdata->bc->msr[i], TPDM_BC_MSR(i)); > +} > + > +static void __tpdm_enable_bc(struct tpdm_drvdata *drvdata) > +{ > + int i; > + uint32_t val; > + > + if (drvdata->bc->sat_mode) > + tpdm_writel(drvdata, drvdata->bc->sat_mode, > + TPDM_BC_SATROLL); > + else > + tpdm_writel(drvdata, 0x0, TPDM_BC_SATROLL); > + > + if (drvdata->bc->enable_counters) { > + tpdm_writel(drvdata, 0xFFFFFFFF, TPDM_BC_CNTENCLR); > + tpdm_writel(drvdata, drvdata->bc->enable_counters, > + TPDM_BC_CNTENSET); > + } > + if (drvdata->bc->clear_counters) > + tpdm_writel(drvdata, drvdata->bc->clear_counters, > + TPDM_BC_CNTENCLR); > + > + if (drvdata->bc->enable_irq) { > + tpdm_writel(drvdata, 0xFFFFFFFF, TPDM_BC_INTENCLR); > + tpdm_writel(drvdata, drvdata->bc->enable_irq, > + TPDM_BC_INTENSET); > + } > + if (drvdata->bc->clear_irq) > + tpdm_writel(drvdata, drvdata->bc->clear_irq, > + TPDM_BC_INTENCLR); > + > + if (drvdata->bc_trig_type == TPDM_SUPPORT_TYPE_FULL) { > + for (i = 0; i < drvdata->bc_counters_avail; i++) { Same here - shouldn't bc_trig_type and bc_counters_avail be part of bc_dataset? > + tpdm_writel(drvdata, drvdata->bc->trig_val_lo[i], > + TPDM_BC_TRIG_LO(i)); > + tpdm_writel(drvdata, drvdata->bc->trig_val_hi[i], > + TPDM_BC_TRIG_HI(i)); > + } > + } else if (drvdata->bc_trig_type == TPDM_SUPPORT_TYPE_PARTIAL) { > + tpdm_writel(drvdata, drvdata->bc->trig_val_lo[0], > + TPDM_BC_TRIG_LO(0)); > + tpdm_writel(drvdata, drvdata->bc->trig_val_hi[0], > + TPDM_BC_TRIG_HI(0)); > + } > + > + if (drvdata->bc->enable_ganging) > + tpdm_writel(drvdata, drvdata->bc->enable_ganging, TPDM_BC_GANG); > + > + for (i = 0; i < TPDM_BC_MAX_OVERFLOW; i++) > + tpdm_writel(drvdata, drvdata->bc->overflow_val[i], > + TPDM_BC_OVERFLOW(i)); > + > + __tpdm_config_bc_msr(drvdata); > + > + val = tpdm_readl(drvdata, TPDM_BC_CR); > + if (drvdata->bc->retrieval_mode == TPDM_MODE_APB) > + val = val | BIT(2); > + else > + val = val & ~BIT(2); > + tpdm_writel(drvdata, val, TPDM_BC_CR); > + > + val = tpdm_readl(drvdata, TPDM_BC_CR); > + /* Set the enable bit */ > + val = val | BIT(0); > + tpdm_writel(drvdata, val, TPDM_BC_CR); As a whole, this function is very hard to read and understand due to the lack of comments. > +} > + > static void __tpdm_enable(struct tpdm_drvdata *drvdata) > { > TPDM_UNLOCK(drvdata); > @@ -283,6 +370,12 @@ static void __tpdm_enable(struct tpdm_drvdata *drvdata) > if (drvdata->clk_enable) > tpdm_writel(drvdata, 0x1, TPDM_CLK_CTRL); > > + if (test_bit(TPDM_DS_GPR, drvdata->enable_ds)) > + __tpdm_enable_gpr(drvdata); > + > + if (test_bit(TPDM_DS_BC, drvdata->enable_ds)) > + __tpdm_enable_bc(drvdata); > + > TPDM_LOCK(drvdata); > } > > @@ -307,10 +400,22 @@ static int tpdm_enable(struct coresight_device *csdev, > return 0; > } > > +static void __tpdm_disable_bc(struct tpdm_drvdata *drvdata) > +{ > + uint32_t config; > + > + config = tpdm_readl(drvdata, TPDM_BC_CR); > + config = config & ~BIT(0); > + tpdm_writel(drvdata, config, TPDM_BC_CR); > +} > + > static void __tpdm_disable(struct tpdm_drvdata *drvdata) > { > TPDM_UNLOCK(drvdata); > > + if (test_bit(TPDM_DS_BC, drvdata->enable_ds)) > + __tpdm_disable_bc(drvdata); > + Shouldn't GPRs be disabled as well? If not why is it the case? A comment should be explaining what is going on. > if (drvdata->clk_enable) > tpdm_writel(drvdata, 0x0, TPDM_CLK_CTRL); > > @@ -352,6 +457,234 @@ static const struct coresight_ops tpdm_cs_ops = { > .source_ops = &tpdm_source_ops, > }; Everything related to sysfs should be in a patch on its own. > > +static ssize_t available_datasets_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) Indentation problem here and for the rest of the patch. > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); > + ssize_t size = 0; > + > + if (test_bit(TPDM_DS_IMPLDEF, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", > + "IMPLDEF"); > + > + if (test_bit(TPDM_DS_DSB, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "DSB"); > + > + if (test_bit(TPDM_DS_CMB, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "CMB"); > + > + if (test_bit(TPDM_DS_TC, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "TC"); > + > + if (test_bit(TPDM_DS_BC, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "BC"); > + > + if (test_bit(TPDM_DS_GPR, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "GPR"); > + > + if (test_bit(TPDM_DS_MCMB, drvdata->datasets)) > + size += scnprintf(buf + size, PAGE_SIZE - size, "%-8s", "MCMB"); > + > + size += scnprintf(buf + size, PAGE_SIZE - size, "\n"); > + return size; > +} > +static DEVICE_ATTR_RO(available_datasets); > + > +static ssize_t enable_datasets_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); > + ssize_t size; > + > + size = scnprintf(buf, PAGE_SIZE, "%*pb\n", TPDM_DATASETS, > + drvdata->enable_ds); > + > + if (PAGE_SIZE - size < 2) > + size = -EINVAL; TPDM_DATASETS is set to 32 - is this realistic? > + else > + size += scnprintf(buf + size, 2, "\n"); ...and what is going on here? > + return size; > +} > + > +static ssize_t enable_datasets_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; > + int i; > + > + if (kstrtoul(buf, 16, &val)) > + return -EINVAL; > + > + mutex_lock(&drvdata->lock); > + if (drvdata->enable) { > + mutex_unlock(&drvdata->lock); > + return -EPERM; > + } > + > + for (i = 0; i < TPDM_DATASETS; i++) { > + if (test_bit(i, drvdata->datasets) && (val & BIT(i))) > + __set_bit(i, drvdata->enable_ds); > + else > + __clear_bit(i, drvdata->enable_ds); > + } > + mutex_unlock(&drvdata->lock); > + return size; > +} > +static DEVICE_ATTR_RW(enable_datasets); > + > +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 mcmb_dataset *mcmb_temp = NULL; > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + ret = kstrtoul(buf, 10, &val); The coresight subsystem normally uses the hexadecimal base. > + if (ret) > + return ret; Shouldn't this be "if (!ret)" ? > + > + mutex_lock(&drvdata->lock); > + /* Reset all datasets to ZERO */ > + if (drvdata->gpr != NULL) > + memset(drvdata->gpr, 0, sizeof(struct gpr_dataset)); > + > + if (drvdata->bc != NULL) > + memset(drvdata->bc, 0, sizeof(struct bc_dataset)); > + > + if (drvdata->tc != NULL) > + memset(drvdata->tc, 0, sizeof(struct tc_dataset)); > + > + if (drvdata->dsb != NULL) > + memset(drvdata->dsb, 0, sizeof(struct dsb_dataset)); > + > + if (drvdata->cmb != NULL) { > + if (drvdata->cmb->mcmb != NULL) { > + mcmb_temp = drvdata->cmb->mcmb; > + memset(drvdata->cmb->mcmb, 0, > + sizeof(struct mcmb_dataset)); > + } > + > + memset(drvdata->cmb, 0, sizeof(struct cmb_dataset)); > + drvdata->cmb->mcmb = mcmb_temp; > + } > + /* Init the default data */ > + tpdm_init_default_data(drvdata); > + > + mutex_unlock(&drvdata->lock); > + > + /* Disable tpdm if enabled */ > + if (drvdata->enable) > + coresight_disable(drvdata->csdev); Why is this done out of the lock? > + > + return size; > +} > +static DEVICE_ATTR_WO(reset); > + > +static ssize_t integration_test_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + int i, ret = 0; > + unsigned long val; > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + ret = kstrtoul(buf, 10, &val); > + if (ret) > + return ret; > + > + if (val != 1 && val != 2) > + return -EINVAL; > + > + if (!drvdata->enable) > + return -EINVAL; > + > + if (val == 1) > + val = ATBCNTRL_VAL_64; > + else > + val = ATBCNTRL_VAL_32; > + TPDM_UNLOCK(drvdata); > + tpdm_writel(drvdata, 0x1, TPDM_ITCNTRL); > + > + for (i = 1; i < 5; i++) > + tpdm_writel(drvdata, val, TPDM_ITATBCNTRL); > + > + tpdm_writel(drvdata, 0, TPDM_ITCNTRL); > + TPDM_LOCK(drvdata); > + return size; > +} > +static DEVICE_ATTR_WO(integration_test); Integration test interface should be conditional to a compile time option. Have a look at what was done for CTIs. > + > +static ssize_t gp_regs_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 = 0; > + > + if (!test_bit(TPDM_DS_GPR, drvdata->datasets)) > + return -EPERM; return -EINVAL; > + > + mutex_lock(&drvdata->lock); > + for (i = 0; i < TPDM_GPR_REGS_MAX; i++) { > + if (!test_bit(i, drvdata->gpr->gpr_dirty)) > + continue; > + size += scnprintf(buf + size, PAGE_SIZE - size, > + "Index: 0x%x Value: 0x%x\n", i, > + drvdata->gpr->gp_regs[i]); This should not be - the sysfs interface requires outputs of a single line. > + } > + mutex_unlock(&drvdata->lock); > + return size; > +} > + > +static ssize_t gp_regs_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 (!test_bit(TPDM_DS_GPR, drvdata->datasets) || > + index >= TPDM_GPR_REGS_MAX) > + return -EPERM; > + > + mutex_lock(&drvdata->lock); > + drvdata->gpr->gp_regs[index] = val; > + __set_bit(index, drvdata->gpr->gpr_dirty); > + mutex_unlock(&drvdata->lock); > + return size; > +} > +static DEVICE_ATTR_RW(gp_regs); > + > +static struct attribute *tpdm_attrs[] = { > + &dev_attr_available_datasets.attr, > + &dev_attr_enable_datasets.attr, > + &dev_attr_reset.attr, > + &dev_attr_integration_test.attr, > + &dev_attr_gp_regs.attr, > + NULL, > +}; All new sysfs interface need to be documented. See here: Documentation/ABI/testing/sysfs-bus-coresight-devices-xyz More comments to come... Thanks, Mathieu > + > +static struct attribute_group tpdm_attr_grp = { > + .attrs = tpdm_attrs, > +}; > +static const struct attribute_group *tpdm_attr_grps[] = { > + &tpdm_attr_grp, > + NULL, > +}; > + > static int tpdm_datasets_alloc(struct tpdm_drvdata *drvdata) > { > if (test_bit(TPDM_DS_GPR, drvdata->datasets)) { > @@ -513,6 +846,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > desc.ops = &tpdm_cs_ops; > desc.pdata = adev->dev.platform_data; > desc.dev = &adev->dev; > + desc.groups = tpdm_attr_grps; > drvdata->csdev = coresight_register(&desc); > if (IS_ERR(drvdata->csdev)) > return PTR_ERR(drvdata->csdev); > -- > 2.17.1 >

4 years, 1 month

Re: [PATCH 10/10] ARM: dts: msm: Add TPDA and TPDM support to DTS for RB5

by Mathieu Poirier

On Thu, Oct 21, 2021 at 03:38:56PM +0800, Tao Zhang wrote: > Add TPDA and TPDM support to DTS for RB5 board. This change is a > sample for validating. After applying this patch, the new TPDM and > TPDA nodes can be observed at the coresight devices path. TPDM and > TPDA hardware can be operated by commands. > > List the commands for validating this series patches as below. > echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source > echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test > echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test > cat /dev/tmc_etf0 > /data/etf-tpdm0.bin > echo 0 > /sys/bus/coresight/devices/tpdm0/enable_source > echo 0 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > echo 1 > /sys/bus/coresight/devices/tpdm1/enable_source > echo 1 > /sys/bus/coresight/devices/tpdm1/integration_test > echo 2 > /sys/bus/coresight/devices/tpdm1/integration_test > cat /dev/tmc_etf0 > /data/etf-tpdm1.bin > echo 0 > /sys/bus/coresight/devices/tpdm1/enable_source > echo 0 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > echo 1 > /sys/bus/coresight/devices/tpdm2/enable_source > echo 1 > /sys/bus/coresight/devices/tpdm2/integration_test > echo 2 > /sys/bus/coresight/devices/tpdm2/integration_test > cat /dev/tmc_etf0 > /data/etf-tpdm2.bin > echo 0 > /sys/bus/coresight/devices/tpdm2/enable_source > echo 0 > /sys/bus/coresight/devices/tmc_etf0/enable_sink > > If the data from TPDMs can be obtained from the ETF, it means > that the TPDMs verification is successful. At the same time, > since TPDM0, TPDM1 and TPDM2 are all connected to the same > funnel "funnel@6c2d000" and output via different output ports, Where is "funnel@6c2d000" in this patch? > it also means that the following patches verification is > successful. > coresight: add support to enable more coresight paths > coresight: funnel: add support for multiple output ports > > Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > --- > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 439 +++++++++++++++++++++++ > 1 file changed, 439 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > index 8ac96f8e79d4..bcec8b181e11 100644 > --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > @@ -222,6 +222,445 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > }; > + > + stm@6002000 { > + compatible = "arm,coresight-stm", "arm,primecell"; > + reg = <0 0x06002000 0 0x1000>, > + <0 0x16280000 0 0x180000>; > + reg-names = "stm-base", "stm-stimulus-base"; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + stm_out: endpoint { > + remote-endpoint = > + <&funnel0_in7>; > + }; > + }; > + }; > + }; > + > + funnel@6041000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + reg = <0 0x06041000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + funnel0_out: endpoint { > + remote-endpoint = > + <&merge_funnel_in0>; > + }; > + }; > + }; > + > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@7 { > + reg = <7>; > + funnel0_in7: endpoint { > + remote-endpoint = <&stm_out>; > + }; > + }; > + }; > + }; > + > + funnel@6042000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + reg = <0 0x06042000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + funnel2_out: endpoint { > + remote-endpoint = > + <&merge_funnel_in2>; > + }; > + }; > + }; > + > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@4 { > + reg = <4>; > + funnel2_in5: endpoint { > + remote-endpoint = > + <&apss_merge_funnel_out>; > + }; > + }; > + }; > + }; > + > + funnel@6b04000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + arm,primecell-periphid = <0x000bb908>; > + > + reg = <0 0x6b04000 0 0x1000>; > + reg-names = "funnel-base"; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + merge_funnel_out: endpoint { > + remote-endpoint = > + <&etf_in>; > + }; > + }; > + }; > + > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@7 { > + reg = <7>; > + swao_funnel_in7: endpoint { > + slave-mode; > + remote-endpoint= > + <&merg_funnel_out>; > + }; > + }; > + }; > + > + }; > + > + funnel@6045000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + reg = <0 0x06045000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + merg_funnel_out: endpoint { > + remote-endpoint = <&swao_funnel_in7>; > + }; > + }; > + }; > + > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + merge_funnel_in0: endpoint { > + remote-endpoint = > + <&funnel0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + merge_funnel_in2: endpoint { > + remote-endpoint = > + <&funnel2_out>; > + }; > + }; > + }; > + }; > + > + etf@6b05000 { > + compatible = "arm,coresight-tmc", "arm,primecell"; > + reg = <0 0x06b05000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + in-ports { > + port { > + etf_in: endpoint { > + remote-endpoint = > + <&merge_funnel_out>; > + }; > + }; > + }; > + }; > + > + etm@7040000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07040000 0 0x1000>; > + > + cpu = <&CPU0>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm0_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in0>; > + }; > + }; > + }; > + }; > + > + etm@7140000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07140000 0 0x1000>; > + > + cpu = <&CPU1>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm1_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in1>; > + }; > + }; > + }; > + }; > + > + etm@7240000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07240000 0 0x1000>; > + > + cpu = <&CPU2>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm2_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in2>; > + }; > + }; > + }; > + }; > + > + etm@7340000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07340000 0 0x1000>; > + > + cpu = <&CPU3>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm3_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in3>; > + }; > + }; > + }; > + }; > + > + etm@7440000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07440000 0 0x1000>; > + > + cpu = <&CPU4>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm4_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in4>; > + }; > + }; > + }; > + }; > + > + etm@7540000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07540000 0 0x1000>; > + > + cpu = <&CPU5>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm5_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in5>; > + }; > + }; > + }; > + }; > + > + etm@7640000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07640000 0 0x1000>; > + > + cpu = <&CPU6>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm6_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in6>; > + }; > + }; > + }; > + }; > + > + etm@7740000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0 0x07740000 0 0x1000>; > + > + cpu = <&CPU7>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + arm,coresight-loses-context-with-cpu; > + > + out-ports { > + port { > + etm7_out: endpoint { > + remote-endpoint = > + <&apss_funnel_in7>; > + }; > + }; > + }; > + }; > + > + funnel@7800000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + reg = <0 0x07800000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + apss_funnel_out: endpoint { > + remote-endpoint = > + <&apss_merge_funnel_in>; > + }; > + }; > + }; > + > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + apss_funnel_in0: endpoint { > + remote-endpoint = > + <&etm0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + apss_funnel_in1: endpoint { > + remote-endpoint = > + <&etm1_out>; > + }; > + }; > + > + port@2 { > + reg = <2>; > + apss_funnel_in2: endpoint { > + remote-endpoint = > + <&etm2_out>; > + }; > + }; > + > + port@3 { > + reg = <3>; > + apss_funnel_in3: endpoint { > + remote-endpoint = > + <&etm3_out>; > + }; > + }; > + > + port@4 { > + reg = <4>; > + apss_funnel_in4: endpoint { > + remote-endpoint = > + <&etm4_out>; > + }; > + }; > + > + port@5 { > + reg = <5>; > + apss_funnel_in5: endpoint { > + remote-endpoint = > + <&etm5_out>; > + }; > + }; > + > + port@6 { > + reg = <6>; > + apss_funnel_in6: endpoint { > + remote-endpoint = > + <&etm6_out>; > + }; > + }; > + > + port@7 { > + reg = <7>; > + apss_funnel_in7: endpoint { > + remote-endpoint = > + <&etm7_out>; > + }; > + }; > + }; > + }; > + > + funnel@7810000 { > + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > + reg = <0 0x07810000 0 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + apss_merge_funnel_out: endpoint { > + remote-endpoint = > + <&funnel2_in5>; > + }; > + }; > + }; > + > + in-ports { > + port { > + apss_merge_funnel_in: endpoint { > + remote-endpoint = > + <&apss_funnel_out>; > + }; > + }; > + }; > + }; Which of the above introduces TPDM and TPDA devices? More comments to come tomorrow. Thanks, Mathieu > }; > > &adsp { > -- > 2.17.1 >

4 years, 1 month

Re: [PATCH 4/5] perf arm-spe: Implement find_snapshot callback

by German Gomez

Hi Leo, Would you be ok with the current patch the way it is? In case it's of any help, I'm sharing the testing steps that James and I went through when testing this internally, if you want to add to it - Test that only a portion of the buffer is saved until there is a wraparound $ ./perf record -vvv -e arm_spe/period=148576/u -S -- taskset --cpu-list 0 stress --cpu 1 & while true; do sleep 0.2; killall -s USR2 perf; done - Test snapshot mode in CPU mode $ sudo ./perf record -vvv -C 0 -e arm_spe/period=148576/u -S -- taskset --cpu-list 0 stress --cpu 1 & - Test that auxtrace buffers correspond to an aux record - Test snapshot default sizes in sudo and user modes - Test small snapshot size $ ./perf record -vvv -e arm_spe/period=148576/u -S1000 -m16,16 -- taskset --cpu-list 0 stress --cpu 1 & If there are any concerns with the patches, please let me know and I will try to address them. Thanks, German On 13/10/2021 08:51, Will Deacon wrote: > On Wed, Oct 13, 2021 at 08:39:16AM +0800, Leo Yan wrote: >> On Mon, Oct 11, 2021 at 04:55:37PM +0100, German Gomez wrote: >>> On 06/10/2021 10:51, Leo Yan wrote: >>>> On Wed, Oct 06, 2021 at 10:35:20AM +0100, German Gomez wrote: >>>> >>>> [...] >>>> >>>>>> So simply say, I think the head pointer monotonically increasing is >>>>>> the right thing to do in Arm SPE driver. >>>>> I will talk to James about how we can proceed on this. >>>> Thanks! >>> I took this offline with James and, though it looks possible to patch >>> the SPE driver to have a monotonically increasing head pointer in order >>> to simplify the handling in the perf tool, it could be a breaking change >>> for users of the perf_event_open syscall that currently rely on the way >>> it works now. >> Here I cannot create the connection between AUX head pointer and the >> breakage of calling perf_event_open(). >> >> Could you elaborate what's the reason the monotonical increasing head >> pointer will lead to the breakage for perf_event_open()? > It's a user-visible change in behaviour, isn't it? Therefore we risk > breaking applications that rely on the current behaviour if we change it > unconditionally. > > Given that the driver has always worked like this and it doesn't sound like > it's the end of the world to deal with it in userspace (after all, it's > aligned with intel-pt), then I don't think we should change it. > > Will

4 years, 1 month

[PATCH 0/6] coresight: syscfg: Extend configfs for config load.

by Mike Leach

This set extends the configfs support to allow loading and unloading of configurations as binary files via configfs. Additional attributes - load, unload and last_load_status are provided to implement the load functionality. Routines to generate binary configuration files are supplied in ./samples/coresight. Documentation is updated to describe feature usage. Set applies on top of dynamic load set here: https://lists.linaro.org/pipermail/coresight/2021-October/007254.html Mike Leach (6): coresight: configfs: Add in functionality for load via configfs coresight: configfs: Add in binary attributes to load files coresight: configfs: Modify config files to allow userspace use coresight: samples: Add an example config writer for configfs load coresight: samples: Add coresight file reader sample program Documentation: coresight: Update coresight configuration docs .../trace/coresight/coresight-config.rst | 65 ++- drivers/hwtracing/coresight/Makefile | 2 +- .../coresight/coresight-config-file.c | 472 ++++++++++++++++++ .../coresight/coresight-config-file.h | 158 ++++++ .../hwtracing/coresight/coresight-config.h | 38 ++ .../coresight/coresight-syscfg-configfs.c | 148 +++++- .../coresight/coresight-syscfg-configfs.h | 8 + .../hwtracing/coresight/coresight-syscfg.c | 36 ++ .../hwtracing/coresight/coresight-syscfg.h | 2 + samples/coresight/Makefile | 23 + samples/coresight/Makefile.host | 39 ++ samples/coresight/coresight-cfg-bufw.c | 302 +++++++++++ samples/coresight/coresight-cfg-bufw.h | 24 + samples/coresight/coresight-cfg-file-read.c | 191 +++++++ samples/coresight/coresight-cfg-filegen.c | 89 ++++ 15 files changed, 1583 insertions(+), 14 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-config-file.c create mode 100644 drivers/hwtracing/coresight/coresight-config-file.h create mode 100644 samples/coresight/Makefile.host create mode 100644 samples/coresight/coresight-cfg-bufw.c create mode 100644 samples/coresight/coresight-cfg-bufw.h create mode 100644 samples/coresight/coresight-cfg-file-read.c create mode 100644 samples/coresight/coresight-cfg-filegen.c -- 2.17.1

4 years, 1 month

Re: [PATCH 02/10] coresight: funnel: add support for multiple output ports

by Mathieu Poirier

Good morning, On Thu, Oct 21, 2021 at 03:38:48PM +0800, Tao Zhang wrote: > Funnel devices are now capable of supporting multiple-inputs and > multiple-outputs configuration with in built hardware filtering > for TPDM devices. Add software support to this function. Output > port is selected according to the source of the trace path. There isn't any modification in this patch that relates to funnel input ports. > > The source of the input port on funnels will be marked in the > device tree. > e.g. > tpdm_XXX: tpdm@xxxxxxx { > ... ... ... ... > }; > > funnel_XXX: funnel@xxxxxxx { > ... ... ... ... > out-ports { > ... ... ... ... > port@x { > ... ... ... ... > label = <&tpdm_XXX>; <-- To point out tpdm_XXX should > }; be outputed from port@x. And > ... ... ... ... this is a hardware static > }; connections. Here needs > ... ... ... ... to refer to hardware design. > }; There is no indication as to why the output port of a funnel needs to be labelled. On that topic, twice in the past we have been clear on the fact that labels should be introduced once the initial functionality has been merged. And yet this patch is centered entirely on that concept... > > Then driver will parse the source name marked in the device tree, and > save it to the coresight path. When the function needs to know the > source name, it could get the source name from coresight path parameter. > Finally, the output port knows which source it corresponds to, and it > also knows which input port it corresponds to. > > Signed-off-by: Tingwei Zhang <tingwei(a)codeaurora.org> > Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 75 +++++++++++++++---- > .../hwtracing/coresight/coresight-platform.c | 8 ++ > include/linux/coresight.h | 2 + > 3 files changed, 71 insertions(+), 14 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 1e621d61307a..490c9d8d43f9 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -56,6 +56,8 @@ static LIST_HEAD(cs_active_paths); > const u32 coresight_barrier_pkt[4] = {0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; > EXPORT_SYMBOL_GPL(coresight_barrier_pkt); > > +static struct coresight_device *coresight_get_source(struct list_head *path); > + > static const struct cti_assoc_op *cti_assoc_ops; > > void coresight_set_cti_ops(const struct cti_assoc_op *cti_op) > @@ -121,14 +123,46 @@ static int coresight_source_is_unique(struct coresight_device *csdev) > csdev, coresight_id_match); > } > > +/** > + * coresight_source_filter - checks whether the connection matches the source > + * of path if connection is binded to specific source. > + * @path: The list of devices > + * @conn: The connection of one outport > + * > + * Return zero if the connection doesn't have a source binded or source of the > + * path matches the source binds to connection. > + */ > +static int coresight_source_filter(struct list_head *path, > + struct coresight_connection *conn) > +{ > + int ret = 0; > + struct coresight_device *source = NULL; > + > + if (conn->source_name == NULL) > + return ret; > + > + source = coresight_get_source(path); > + if (source == NULL) > + return ret; > + > + if (is_of_node(source->dev.fwnode)) > + return strcmp(conn->source_name, > + of_node_full_name(to_of_node(source->dev.fwnode))); > + > + return ret; > +} > + > static int coresight_find_link_inport(struct coresight_device *csdev, > - struct coresight_device *parent) > + struct coresight_device *parent, > + struct list_head *path) > { > int i; > struct coresight_connection *conn; > > for (i = 0; i < parent->pdata->nr_outport; i++) { > conn = &parent->pdata->conns[i]; > + if (coresight_source_filter(path, conn)) > + continue; > if (conn->child_dev == csdev) > return conn->child_port; > } > @@ -140,13 +174,16 @@ static int coresight_find_link_inport(struct coresight_device *csdev, > } > > static int coresight_find_link_outport(struct coresight_device *csdev, > - struct coresight_device *child) > + struct coresight_device *child, > + struct list_head *path) > { > int i; > struct coresight_connection *conn; > > for (i = 0; i < csdev->pdata->nr_outport; i++) { > conn = &csdev->pdata->conns[i]; > + if (coresight_source_filter(path, conn)) > + continue; > if (conn->child_dev == child) > return conn->outport; > } > @@ -358,7 +395,8 @@ static void coresight_disable_sink(struct coresight_device *csdev) > > static int coresight_enable_link(struct coresight_device *csdev, > struct coresight_device *parent, > - struct coresight_device *child) > + struct coresight_device *child, > + struct list_head *path) > { > int ret = 0; > int link_subtype; > @@ -367,8 +405,8 @@ static int coresight_enable_link(struct coresight_device *csdev, > if (!parent || !child) > return -EINVAL; > > - inport = coresight_find_link_inport(csdev, parent); > - outport = coresight_find_link_outport(csdev, child); > + inport = coresight_find_link_inport(csdev, parent, path); > + outport = coresight_find_link_outport(csdev, child, path); > link_subtype = csdev->subtype.link_subtype; > > if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && inport < 0) > @@ -393,7 +431,8 @@ static int coresight_enable_link(struct coresight_device *csdev, > > static void coresight_disable_link(struct coresight_device *csdev, > struct coresight_device *parent, > - struct coresight_device *child) > + struct coresight_device *child, > + struct list_head *path) > { > int i, nr_conns; > int link_subtype; > @@ -402,8 +441,8 @@ static void coresight_disable_link(struct coresight_device *csdev, > if (!parent || !child) > return; > > - inport = coresight_find_link_inport(csdev, parent); > - outport = coresight_find_link_outport(csdev, child); > + inport = coresight_find_link_inport(csdev, parent, path); > + outport = coresight_find_link_outport(csdev, child, path); > link_subtype = csdev->subtype.link_subtype; > > if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG) { > @@ -518,7 +557,7 @@ static void coresight_disable_path_from(struct list_head *path, > case CORESIGHT_DEV_TYPE_LINK: > parent = list_prev_entry(nd, link)->csdev; > child = list_next_entry(nd, link)->csdev; > - coresight_disable_link(csdev, parent, child); > + coresight_disable_link(csdev, parent, child, path); > break; > default: > break; > @@ -573,7 +612,7 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data) > case CORESIGHT_DEV_TYPE_LINK: > parent = list_prev_entry(nd, link)->csdev; > child = list_next_entry(nd, link)->csdev; > - ret = coresight_enable_link(csdev, parent, child); > + ret = coresight_enable_link(csdev, parent, child, path); > if (ret) > goto err; > break; > @@ -801,7 +840,8 @@ static void coresight_drop_device(struct coresight_device *csdev) > */ > static int _coresight_build_path(struct coresight_device *csdev, > struct coresight_device *sink, > - struct list_head *path) > + struct list_head *path, > + struct coresight_device *source) > { > int i, ret; > bool found = false; > @@ -813,7 +853,7 @@ static int _coresight_build_path(struct coresight_device *csdev, > > if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) && > sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) { > - if (_coresight_build_path(sink, sink, path) == 0) { > + if (_coresight_build_path(sink, sink, path, source) == 0) { > found = true; > goto out; > } > @@ -824,8 +864,15 @@ static int _coresight_build_path(struct coresight_device *csdev, > struct coresight_device *child_dev; > > child_dev = csdev->pdata->conns[i].child_dev; > + > + if (csdev->pdata->conns[i].source_name && > + is_of_node(source->dev.fwnode) && > + strcmp(csdev->pdata->conns[i].source_name, > + of_node_full_name(to_of_node(source->dev.fwnode)))) > + continue; > + All this seems to be doing is optimize the search graph by not taking the paths that are not labelled. As far as I can tell the sink would be found regardless of this enhancement. We can think about optimizations for building paths when it is demonstrated that bottlenecks originate from that part of the code. > if (child_dev && > - _coresight_build_path(child_dev, sink, path) == 0) { > + _coresight_build_path(child_dev, sink, path, source) == 0) { > found = true; > break; > } > @@ -870,7 +917,7 @@ struct list_head *coresight_build_path(struct coresight_device *source, > > INIT_LIST_HEAD(path); > > - rc = _coresight_build_path(source, sink, path); > + rc = _coresight_build_path(source, sink, path, source); > if (rc) { > kfree(path); > return ERR_PTR(rc); > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index c594f45319fc..60c43ab149a5 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -222,6 +222,7 @@ static int of_coresight_parse_endpoint(struct device *dev, > struct of_endpoint endpoint, rendpoint; > struct device_node *rparent = NULL; > struct device_node *rep = NULL; > + struct device_node *sn = NULL; > struct device *rdev = NULL; > struct fwnode_handle *rdev_fwnode; > struct coresight_connection *conn; > @@ -269,6 +270,13 @@ static int of_coresight_parse_endpoint(struct device *dev, > */ > conn->child_fwnode = fwnode_handle_get(rdev_fwnode); > conn->child_port = rendpoint.port; > + conn->source_name = NULL; > + sn = of_parse_phandle(ep, "label", 0); > + if (sn) { > + conn->source_name = sn->full_name; > + of_node_put(sn); > + } See my comments above on this. Moreover neither of the yaml files for both TPDM and TPDA nor the DTS submitted with this patchset account for a "label", making me very confused about the relevance of this patch. More comments to come on Monday. Thanks, Mathieu > + > /* Connection record updated */ > } while (0); > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 93a2922b7653..7065b60a767b 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -170,6 +170,7 @@ struct coresight_desc { > * struct coresight_connection - representation of a single connection > * @outport: a connection's output port number. > * @child_port: remote component's port number @output is connected to. > + * @source_name:source component's name. > * @chid_fwnode: remote component's fwnode handle. > * @child_dev: a @coresight_device representation of the component > connected to @outport. > @@ -178,6 +179,7 @@ struct coresight_desc { > struct coresight_connection { > int outport; > int child_port; > + const char *source_name; > struct fwnode_handle *child_fwnode; > struct coresight_device *child_dev; > struct coresight_sysfs_link *link; > -- > 2.17.1 >

4 years, 1 month

Re: [PATCH v5 09/15] coresight: trbe: Add infrastructure for Errata handling

by Suzuki K Poulose

Hi Arnd Thanks for the report. On 29/10/2021 11:31, Arnd Bergmann wrote: > On Fri, Oct 15, 2021 at 12:31 AM Suzuki K Poulose > <suzuki.poulose(a)arm.com> wrote: >> >> +static void trbe_check_errata(struct trbe_cpudata *cpudata) >> +{ >> + int i; >> + >> + for (i = 0; i < TRBE_ERRATA_MAX; i++) { >> + int cap = trbe_errata_cpucaps[i]; >> + >> + if (WARN_ON_ONCE(cap < 0)) >> + return; >> + if (this_cpu_has_cap(cap)) >> + set_bit(i, cpudata->errata); >> + } >> +} > > this_cpu_has_cap() is private to arch/arm64 and not exported, so this causes > a build failure when used from a loadable module: > > ERROR: modpost: "this_cpu_has_cap" > [drivers/hwtracing/coresight/coresight-trbe.ko] undefined! > > Should this symbol be exported or do we need a different workaround? This should be exported. I can send in a patch. Suzuki

4 years, 1 month

Re: [PATCH 01/10] coresight: add support to enable more coresight paths

by Mathieu Poirier

On Thu, Oct 21, 2021 at 03:38:47PM +0800, Tao Zhang wrote: > Current coresight implementation only supports enabling source > ETMs or STM. This patch adds support to enable more kinds of > coresight source to sink paths. We build a path from source to > sink when any source is enabled and store it in a list. When the > source is disabled, we fetch the corresponding path from the list > and decrement the refcount on each device in the path. The device > is disabled if the refcount reaches zero. Don't store path to > coresight data structure of source to avoid unnecessary change to > ABI. > Since some targets may have coresight sources other than STM and > ETMs, we need to add this change to support these coresight > devices. > > Signed-off-by: Tingwei Zhang <tingwei(a)codeaurora.org> > Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 100 +++++++++++-------- > 1 file changed, 56 insertions(+), 44 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 8a18c71df37a..1e621d61307a 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -37,18 +37,16 @@ struct coresight_node { > }; > > /* > - * When operating Coresight drivers from the sysFS interface, only a single > - * path can exist from a tracer (associated to a CPU) to a sink. > + * struct coresight_path - path from source to sink > + * @path: Address of path list. > + * @link: hook to the list. > */ > -static DEFINE_PER_CPU(struct list_head *, tracer_path); > +struct coresight_path { > + struct list_head *path; > + struct list_head link; > +}; For sources associated with a CPU, like ETMs, having a per-cpu way of storing paths is a definite advantage and should be kept that way. > > -/* > - * As of this writing only a single STM can be found in CS topologies. Since > - * there is no way to know if we'll ever see more and what kind of > - * configuration they will enact, for the time being only define a single path > - * for STM. > - */ > -static struct list_head *stm_path; > +static LIST_HEAD(cs_active_paths); Then there are sources that aren't associated with a CPU like STMs and TPDMs. Perhaps using an IDR or the hash of the device name as a key to a hashing vector would be better than doing a sequential search, especially as the list of devices is bound to increase over time. > > /* > * When losing synchronisation a new barrier packet needs to be inserted at the > @@ -354,6 +352,7 @@ static void coresight_disable_sink(struct coresight_device *csdev) > if (ret) > return; > coresight_control_assoc_ectdev(csdev, false); > + csdev->activated = false; I don't see why this is needed and without proper documentation there is no way for me to guess the logic behind the change. The ->activated flag should be manipulated from the command line interface only. > csdev->enable = false; > } > > @@ -590,6 +589,20 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data) > goto out; > } > > +static struct coresight_device *coresight_get_source(struct list_head *path) > +{ > + struct coresight_device *csdev; > + > + if (!path) > + return NULL; > + > + csdev = list_first_entry(path, struct coresight_node, link)->csdev; > + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE) > + return NULL; > + > + return csdev; > +} > + > struct coresight_device *coresight_get_sink(struct list_head *path) > { > struct coresight_device *csdev; > @@ -1086,9 +1099,23 @@ static int coresight_validate_source(struct coresight_device *csdev, > return 0; > } > > +static int coresight_store_path(struct list_head *path) > +{ > + struct coresight_path *node; > + > + node = kzalloc(sizeof(struct coresight_path), GFP_KERNEL); > + if (!node) > + return -ENOMEM; > + > + node->path = path; > + list_add(&node->link, &cs_active_paths); > + > + return 0; > +} > + > int coresight_enable(struct coresight_device *csdev) > { > - int cpu, ret = 0; > + int ret = 0; > struct coresight_device *sink; > struct list_head *path; > enum coresight_dev_subtype_source subtype; > @@ -1133,25 +1160,9 @@ int coresight_enable(struct coresight_device *csdev) > if (ret) > goto err_source; > > - switch (subtype) { > - case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: > - /* > - * When working from sysFS it is important to keep track > - * of the paths that were created so that they can be > - * undone in 'coresight_disable()'. Since there can only > - * be a single session per tracer (when working from sysFS) > - * a per-cpu variable will do just fine. > - */ > - cpu = source_ops(csdev)->cpu_id(csdev); > - per_cpu(tracer_path, cpu) = path; > - break; > - case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE: > - stm_path = path; > - break; > - default: > - /* We can't be here */ > - break; > - } > + ret = coresight_store_path(path); > + if (ret) > + goto err_source; > > out: > mutex_unlock(&coresight_mutex); > @@ -1168,8 +1179,11 @@ EXPORT_SYMBOL_GPL(coresight_enable); > > void coresight_disable(struct coresight_device *csdev) > { > - int cpu, ret; > + int ret; > struct list_head *path = NULL; > + struct coresight_path *cspath = NULL; > + struct coresight_path *cspath_next = NULL; > + struct coresight_device *src_csdev = NULL; > > mutex_lock(&coresight_mutex); > > @@ -1180,20 +1194,18 @@ void coresight_disable(struct coresight_device *csdev) > if (!csdev->enable || !coresight_disable_source(csdev)) > goto out; > > - switch (csdev->subtype.source_subtype) { > - case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: > - cpu = source_ops(csdev)->cpu_id(csdev); > - path = per_cpu(tracer_path, cpu); > - per_cpu(tracer_path, cpu) = NULL; > - break; > - case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE: > - path = stm_path; > - stm_path = NULL; > - break; > - default: > - /* We can't be here */ > - break; > + list_for_each_entry_safe(cspath, cspath_next, &cs_active_paths, link) { > + src_csdev = coresight_get_source(cspath->path); > + if (!src_csdev) > + continue; > + if (src_csdev == csdev) { > + path = cspath->path; > + list_del(&cspath->link); > + kfree(cspath); See my comment above - I agree that sources _not_ associated with a CPU should be handled differently. CPU bound sources should be kept untouched. That is all the time I had for today, I will continue tomorrow. Thanks, Mathieu > + } > } > + if (path == NULL) > + goto out; > > coresight_disable_path(path); > coresight_release_path(path); > -- > 2.17.1 >

4 years, 1 month

Re: [PATCH 00/10] Add support for TPDM and TPDA

by Mathieu Poirier

I have started to review this set. It is substantial in size and as such will take me several days, most likely weeks. Did you run checkpatch before sending this set out? If so, any reason why there are still errors? Thanks, Mathieu On Thu, Oct 21, 2021 at 03:38:46PM +0800, Tao Zhang wrote: > This series adds support for the trace performance monitoring and > diagnostics hardware (TPDM and TPDA). It is composed of two major > elements. > a) Changes for original coresight framework to support for TPDM and TPDA. > b) Add driver code for TPDM and TPDA. > > Introduction of changes for original coresight framework > a) Support TPDM as new coresight source. > Since only STM and ETM are supported as coresight source originally. > TPDM is a newly added coresight source. We need to change > the original way of saving coresight path to support more types source > for coresight driver. > The following patch is to add support more coresight sources. > coresight: add support to enable more coresight paths > > b) To support multi-port input and multi-port output for funnels > In some cases, different TPDM hardware will be connected to the same > funnel, but eventually they need to be linked to different TPDAs or > funnels. This requires funnel to support multi-port input and multi-port > output, and can specify which input port corresponds to which output port. > Use property ?label? in the funnel?s configuration to point out from > which input port to which output port. > The following patch is to support multi-port input and multi-port output > for funnels. > coresight: funnel: add support for multiple output ports > > Introduction of TPDM and TPDA > TPDM - The trace performance monitoring and diagnostics monitor or TPDM in > short serves as data collection component for various dataset types > specified in the QPMDA(Qualcomm performance monitoring and diagnostics > architecture) spec. The primary use case of the TPDM is to collect data > from different data sources and send it to a TPDA for packetization, > timestamping and funneling. > The following patch is to add driver for TPDM. > Coresight: Add driver to support Coresight device TPDM > Coresight: Enable BC and GPR for TPDM driver > Coresight: Add interface for TPDM BC subunit > Coresight: Enable and add interface for TPDM TC subunit > Coresight: Enable DSB subunit for TPDM > Coresight: Enable CMB subunit for TPDM > > TPDA - The trace performance monitoring and diagnostics aggregator or > TPDA in short serves as an arbitration and packetization engine for the > performance monitoring and diagnostics network as specified in the QPMDA > (Qualcomm performance monitoring and diagnostics architecture) > specification. The primary use case of the TPDA is to provide > packetization, funneling and timestamping of Monitor data as specified > in the QPMDA specification. > The following patch is to add driver for TPDA. > coresight: Add driver to support Coresight device TPDA > > The last patch of this series is a device tree modification, which add > the TPDM and TPDA configuration to device tree for validating. > ARM: dts: msm: Add TPDA and TPDM configuration to device > > Once this series patches are applied properly, the tpdm and tpda nodes > should be observed at the coresight path /sys/bus/coresight/devices > e.g. > /sys/bus/coresight/devices # ls -l | grep tpd > tpda0 -> ../../../devices/platform/soc(a)0/6004000.tpda/tpda0 > tpdm0 -> ../../../devices/platform/soc(a)0/6844000.lpass.tpdm/tpdm0 > > We can use the commands are similar to the below to validate TPDMs. > Enable coresight sink first. > echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source > echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test > echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test > The test data will be collected in the coresight sink which is enabled. > > This series applies to coresight/next > https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git?h=next > > Tao Zhang (10): > coresight: add support to enable more coresight paths > coresight: funnel: add support for multiple output ports > Coresight: Add driver to support Coresight device TPDM > Coresight: Enable BC and GPR for TPDM driver > Coresight: Add interface for TPDM BC subunit > Coresight: Enable and add interface for TPDM TC subunit > Coresight: Enable DSB subunit for TPDM > Coresight: Enable CMB subunit for TPDM > coresight: Add driver to support Coresight device TPDA > ARM: dts: msm: Add TPDA and TPDM support to DTS for RB5 > > .../bindings/arm/coresight-tpda.yaml | 169 + > .../bindings/arm/coresight-tpdm.yaml | 86 + > MAINTAINERS | 6 + > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 439 ++ > drivers/hwtracing/coresight/Kconfig | 18 + > drivers/hwtracing/coresight/Makefile | 2 + > drivers/hwtracing/coresight/coresight-core.c | 175 +- > .../hwtracing/coresight/coresight-platform.c | 8 + > drivers/hwtracing/coresight/coresight-tpda.c | 828 ++++ > drivers/hwtracing/coresight/coresight-tpdm.c | 4253 +++++++++++++++++ > include/linux/coresight.h | 2 + > 11 files changed, 5928 insertions(+), 58 deletions(-) > create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml > create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml > create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c > create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c > > -- > 2.17.1 >

4 years, 1 month

Re: [PATCH] coresight: Use devm_bitmap_zalloc when applicable

by Mathieu Poirier

Good day, On Sat, Oct 23, 2021 at 10:09:14PM +0200, Christophe JAILLET wrote: > Le 23/10/2021 à 21:36, Joe Perches a écrit : > > On Sat, 2021-10-23 at 21:24 +0200, Christophe JAILLET wrote: > > > 'drvdata->chs.guaranteed' is a bitmap. So use 'devm_bitmap_kzalloc()' to > > > simplify code, improve the semantic and avoid some open-coded arithmetic > > > in allocator arguments. > > [] > > > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c > > [] > > > @@ -862,7 +862,6 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) > > > struct stm_drvdata *drvdata; > > > struct resource *res = &adev->res; > > > struct resource ch_res; > > > - size_t bitmap_size; > > > struct coresight_desc desc = { 0 }; > > > desc.name = coresight_alloc_device_name(&stm_devs, dev); > > > @@ -904,9 +903,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) > > > else > > > drvdata->numsp = stm_num_stimulus_port(drvdata); > > > - bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long); > > > - > > > - guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL); > > > + guaranteed = devm_bitmap_zalloc(dev, drvdata->numsp, GFP_KERNEL); > > > if (!guaranteed) > > > return -ENOMEM; > > > drvdata->chs.guaranteed = guaranteed; > > > > guaranteed is also pretty useless > > > > I agree, but removing it would make the line with devm_bitmap_zalloc() 86 > chars. This would not be consistent with the rest of the file and would > (IMHO) require splitting. > > Let see if the maintainer prefer saving one additional line of code, or > keeping the logic in place. I think we can get rid of @guaranteed and splitting is fine with me: drvdata->chs.guaranteed = devm_bitmap_zalloc(dev, drvdata->numsp, GFP_KERNEL); Thanks, Mathieu > > CJ >

4 years, 1 month

[GIT PULL v3] Coresight changes for v5.16

by Mathieu Poirier

The following changes since commit 5816b3e6577eaa676ceb00a848f0fd65fe2adc29: Linux 5.15-rc3 (2021-09-26 14:08:19 -0700) are available in the Git repository at: git@gitolite.kernel.org:pub/scm/linux/kernel/git/coresight/linux.git tags/coresight-next-v5.16.v3 for you to fetch changes up to 561ced0bb90a4be298b7db5fb54f29731d74a3f6: arm64: errata: Enable TRBE workaround for write to out-of-range address (2021-10-27 11:46:06 -0600) ---------------------------------------------------------------- Coresight changes for v5.16 - A new option to make coresight cpu-debug capabilities available as early as possible in the kernel boot process. - Make trace sessions more enduring by coping with scenarios where events are scheduled on CPUs that can't reach the selected sink. - A set of improvement to make the TMC-ETR driver more efficient. - Enhancements to the TRBE driver to correct several errata. - An enhancement to make the AXI burts size configurable for TMC devices that can't work with the default value. - A fix in the CTI module to use the correct device when calling pm_runtime_put() - The addition of the Kryo-5xx device to the list of support ETMs. Signed-off-by: Mathieu Poirier <mathieu.poirier(a)linaro.org> ---------------------------------------------------------------- Brian Norris (1): coresight: cpu-debug: Control default behavior via Kconfig James Clark (1): coresight: Don't immediately close events that are run on invalid CPU/sink combos Leo Yan (5): coresight: tmc-etr: Add barrier after updating AUX ring buffer coresight: tmc-etf: Add comment for store ordering coresight: tmc-etr: Use perf_output_handle::head for AUX ring buffer coresight: Update comments for removing cs_etm_find_snapshot() coresight: tmc-etr: Speed up for bounce buffer in flat mode Suzuki K Poulose (28): arm64: Add Neoverse-N2, Cortex-A710 CPU part definition arm64: errata: Add detection for TRBE overwrite in FILL mode arm64: errata: Add workaround for TSB flush failures arm64: errata: Add detection for TRBE write to out-of-range coresight: etm4x: Save restore TRFCR_EL1 coresight: etm4x: Use Trace Filtering controls dynamically coresight: etm-pmu: Ensure the AUX handle is valid coresight: trbe: Ensure the format flag is always set coresight: trbe: Drop duplicate TRUNCATE flags coresight: trbe: Unify the enabling sequence coresight: trbe: irq handler: Do not disable TRBE if no action is needed coresight: trbe: Fix handling of spurious interrupts coresight: trbe: Do not truncate buffer on IRQ coresight: trbe: End the AUX handle on truncation coresight: trbe: Prohibit trace before disabling TRBE coresight: trbe: Fix incorrect access of the sink specific data coresight: trbe: Defer the probe on offline CPUs coresight: trbe: Add a helper to calculate the trace generated coresight: trbe: Add a helper to pad a given buffer area coresight: trbe: Decouple buffer base from the hardware base coresight: trbe: Allow driver to choose a different alignment coresight: trbe: Add infrastructure for Errata handling coresight: trbe: Workaround TRBE errata overwrite in FILL mode coresight: trbe: Add a helper to determine the minimum buffer size coresight: trbe: Make sure we have enough space coresight: trbe: Work around write to out of range arm64: errata: Enable workaround for TRBE overwrite in FILL mode arm64: errata: Enable TRBE workaround for write to out-of-range address Tanmay Jagdale (2): dt-bindings: coresight: Add burst size for TMC coresight: tmc: Configure AXI write burst size Tao Zhang (2): coresight: cti: Correct the parameter for pm_runtime_put coresight: etm4x: Add ETM PID for Kryo-5XX Documentation/arm64/silicon-errata.rst | 12 + .../devicetree/bindings/arm/coresight.txt | 5 + arch/arm64/Kconfig | 111 +++++ arch/arm64/include/asm/barrier.h | 16 +- arch/arm64/include/asm/cputype.h | 4 + arch/arm64/kernel/cpu_errata.c | 64 +++ arch/arm64/tools/cpucaps | 3 + drivers/hwtracing/coresight/Kconfig | 13 + drivers/hwtracing/coresight/coresight-cpu-debug.c | 2 +- drivers/hwtracing/coresight/coresight-cti-core.c | 2 +- drivers/hwtracing/coresight/coresight-etb10.c | 5 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 56 ++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 101 +++- drivers/hwtracing/coresight/coresight-etm4x.h | 9 +- .../coresight/coresight-self-hosted-trace.h | 33 ++ drivers/hwtracing/coresight/coresight-tmc-core.c | 21 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 +- drivers/hwtracing/coresight/coresight-tmc-etr.c | 52 +- drivers/hwtracing/coresight/coresight-tmc.h | 6 +- drivers/hwtracing/coresight/coresight-trbe.c | 534 +++++++++++++++++---- 20 files changed, 905 insertions(+), 154 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-self-hosted-trace.h

4 years, 1 month

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight