| Message ID | 4a4ee61ce8ef402615a4528b21a051de3444fb7b.1677540079.git.scclevenger@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight-etm4: Fix for() loop drvdata->nr_addr_cmp range bug | expand |
On 27/02/2023 23:54, Steve Clevenger wrote: > In etm4_enable_hw, fix for() loop range to represent address comparator pairs. > > Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 1ea8f173cca0..104333c2c8a3 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > if (etm4x_sspcicrn_present(drvdata, i)) > etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); > } > - for (i = 0; i < drvdata->nr_addr_cmp; i++) { > + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { > etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); > etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); > } Reviewed-by: James Clark <james.clark@arm.com> Are there other ones in coresight-etm3x-sysfs.c:reset_store() and coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different?
Hi On Tue, 28 Feb 2023 at 11:27, James Clark <james.clark@arm.com> wrote: > > > > On 27/02/2023 23:54, Steve Clevenger wrote: > > In etm4_enable_hw, fix for() loop range to represent address comparator pairs. > > > > Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> > > --- > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > > index 1ea8f173cca0..104333c2c8a3 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > > @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > > if (etm4x_sspcicrn_present(drvdata, i)) > > etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); > > } > > - for (i = 0; i < drvdata->nr_addr_cmp; i++) { > > + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { > > etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); > > etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); > > } > > Reviewed-by: James Clark <james.clark@arm.com> > > Are there other ones in coresight-etm3x-sysfs.c:reset_store() and > coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different? For reasons passing understanding - the etm3x code does the 2 * multiplier at the point that config->nr_addr_cmp is initialised, so it reflects the number of comparators rather than the number of pairs. That said the comments and docs in ./Documents/ABI/testing/sysfs-bus-coresight-devices-etm3x claim that the value is still the number of pairs. Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
On 28/02/2023 11:27, James Clark wrote: > > > On 27/02/2023 23:54, Steve Clevenger wrote: >> In etm4_enable_hw, fix for() loop range to represent address comparator pairs. >> >> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> >> --- >> drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index 1ea8f173cca0..104333c2c8a3 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) >> if (etm4x_sspcicrn_present(drvdata, i)) >> etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); >> } >> - for (i = 0; i < drvdata->nr_addr_cmp; i++) { >> + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { >> etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); >> etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); >> } > > Reviewed-by: James Clark <james.clark@arm.com> > > Are there other ones in coresight-etm3x-sysfs.c:reset_store() and > coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different? Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver") I have queued this as fixes with the above tag and will send it out for v6.3 Thanks Suzuki
Hi Mike and James, I did see the etm3x initialization code. etm4x seems to not do it that way. Steve On 2/28/2023 4:32 AM, Mike Leach wrote: > Hi > > On Tue, 28 Feb 2023 at 11:27, James Clark <james.clark@arm.com> wrote: >> >> >> >> On 27/02/2023 23:54, Steve Clevenger wrote: >>> In etm4_enable_hw, fix for() loop range to represent address comparator pairs. >>> >>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> >>> --- >>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> index 1ea8f173cca0..104333c2c8a3 100644 >>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) >>> if (etm4x_sspcicrn_present(drvdata, i)) >>> etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); >>> } >>> - for (i = 0; i < drvdata->nr_addr_cmp; i++) { >>> + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { >>> etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); >>> etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); >>> } >> >> Reviewed-by: James Clark <james.clark@arm.com> >> >> Are there other ones in coresight-etm3x-sysfs.c:reset_store() and >> coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different? > > For reasons passing understanding - the etm3x code does the 2 * > multiplier at the point that config->nr_addr_cmp is initialised, so it > reflects the number of comparators rather than the number of pairs. > That said the comments and docs in > ./Documents/ABI/testing/sysfs-bus-coresight-devices-etm3x claim that > the value is still the number of pairs. > > Mike > > > > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 1ea8f173cca0..104333c2c8a3 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) if (etm4x_sspcicrn_present(drvdata, i)) etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); } - for (i = 0; i < drvdata->nr_addr_cmp; i++) { + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); }
In etm4_enable_hw, fix for() loop range to represent address comparator pairs. Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)