* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-13 12:20 Venkatesh Vivekanandan
2016-09-13 15:41 ` Mathieu Poirier
0 siblings, 1 reply; 4+ messages in thread
From: Venkatesh Vivekanandan @ 2016-09-13 12:20 UTC (permalink / raw)
To: linux-arm-kernel
tmc_etb_dump_hw is never called in sysFS mode to collect trace from
hardware, because drvdata->mode is set to CS_MODE_DISABLED at
tmc_disable_etf/etr_sink
static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
.
.
if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
tmc_etb_dump_hw(drvdata);
.
.
}
static void tmc_disable_etf_sink(struct coresight_device *csdev)
{
.
.
val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
/* Disable the TMC only if it needs to */
if (val != CS_MODE_DISABLED)
tmc_etb_disable_hw(drvdata);
.
.
}
Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
---
drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++----
drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 +++++----
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 466af86..c7fb7f7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -61,6 +61,8 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
+ long val;
+
CS_UNLOCK(drvdata->base);
tmc_flush_and_stop(drvdata);
@@ -68,7 +70,8 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
*/
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
+ val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
+ if (val == CS_MODE_SYSFS)
tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);
@@ -225,7 +228,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
static void tmc_disable_etf_sink(struct coresight_device *csdev)
{
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -235,9 +237,8 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
return;
}
- val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
/* Disable the TMC only if it needs to */
- if (val != CS_MODE_DISABLED)
+ if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
tmc_etb_disable_hw(drvdata);
spin_unlock_irqrestore(&drvdata->spinlock, flags);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 688be9e..480794b 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -73,6 +73,8 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
+ long val;
+
CS_UNLOCK(drvdata->base);
tmc_flush_and_stop(drvdata);
@@ -80,7 +82,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
*/
- if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
+ val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
+ if (val == CS_MODE_SYSFS)
tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);
@@ -215,7 +218,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
static void tmc_disable_etr_sink(struct coresight_device *csdev)
{
- long val;
unsigned long flags;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -225,9 +227,8 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
return;
}
- val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
/* Disable the TMC only if it needs to */
- if (val != CS_MODE_DISABLED)
+ if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
tmc_etr_disable_hw(drvdata);
spin_unlock_irqrestore(&drvdata->spinlock, flags);
--
2.1.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
2016-09-13 12:20 [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode Venkatesh Vivekanandan
@ 2016-09-13 15:41 ` Mathieu Poirier
2016-09-14 9:56 ` Suzuki K Poulose
0 siblings, 1 reply; 4+ messages in thread
From: Mathieu Poirier @ 2016-09-13 15:41 UTC (permalink / raw)
To: linux-arm-kernel
On 13 September 2016 at 06:20, Venkatesh Vivekanandan
<venkatesh.vivekanandan@broadcom.com> wrote:
> tmc_etb_dump_hw is never called in sysFS mode to collect trace from
> hardware, because drvdata->mode is set to CS_MODE_DISABLED at
> tmc_disable_etf/etr_sink
>
> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> {
> .
> .
> if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> tmc_etb_dump_hw(drvdata);
> .
> .
> }
>
> static void tmc_disable_etf_sink(struct coresight_device *csdev)
> {
> .
> .
> val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> if (val != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata);
You are correct.
> .
> .
> }
>
> Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
> ---
> drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++----
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 +++++----
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 466af86..c7fb7f7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -61,6 +61,8 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
>
> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> {
> + long val;
> +
> CS_UNLOCK(drvdata->base);
>
> tmc_flush_and_stop(drvdata);
> @@ -68,7 +70,8 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> + if (val == CS_MODE_SYSFS)
> tmc_etb_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -225,7 +228,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etf_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -235,9 +237,8 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata);
This would work but tmc_enable_etf_sink() and tmc_disable_etf_sink()
are no longer balanced. Another approach would be to add a "mode"
parameter to tmc_etb_disable_hw() and so something like:
if (val != CS_MODE_DISABLED)
tmc_etb_disable_hw(drvdata, val);
In tmc_etb_disable_hw(), if mode == CS_MODE_SYSFS then we can move
ahead with the dump operation. The same apply for ETR.
Thanks,
Mathieu
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 688be9e..480794b 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -73,6 +73,8 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
>
> static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> {
> + long val;
> +
> CS_UNLOCK(drvdata->base);
>
> tmc_flush_and_stop(drvdata);
> @@ -80,7 +82,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> + if (val == CS_MODE_SYSFS)
> tmc_etr_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -215,7 +218,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etr_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -225,9 +227,8 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
> tmc_etr_disable_hw(drvdata);
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
2016-09-13 15:41 ` Mathieu Poirier
@ 2016-09-14 9:56 ` Suzuki K Poulose
[not found] ` <CAGhh56GQmKnk5UaYYW-X1YpsPdb7SwCy1b5ANd3tmdAXBTBbZA@mail.gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Suzuki K Poulose @ 2016-09-14 9:56 UTC (permalink / raw)
To: linux-arm-kernel
On 13/09/16 16:41, Mathieu Poirier wrote:
> On 13 September 2016 at 06:20, Venkatesh Vivekanandan
> <venkatesh.vivekanandan@broadcom.com> wrote:
>> tmc_etb_dump_hw is never called in sysFS mode to collect trace from
>> hardware, because drvdata->mode is set to CS_MODE_DISABLED at
>> tmc_disable_etf/etr_sink
>>
>> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> {
>> .
>> .
>> if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> tmc_etb_dump_hw(drvdata);
>> .
>> .
>> }
>>
>> static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> {
>> .
>> .
>> val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> /* Disable the TMC only if it needs to */
>> if (val != CS_MODE_DISABLED)
>> tmc_etb_disable_hw(drvdata);
>
> You are correct.
>
>> .
>> .
>> }
>>
>> Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++----
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 +++++----
>> 2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index 466af86..c7fb7f7 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -61,6 +61,8 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
>>
>> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> {
>> + long val;
>> +
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> @@ -68,7 +70,8 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> * When operating in sysFS mode the content of the buffer needs to be
>> * read before the TMC is disabled.
>> */
>> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> + if (val == CS_MODE_SYSFS)
>> tmc_etb_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>>
>> @@ -225,7 +228,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>>
>> static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> {
>> - long val;
>> unsigned long flags;
>> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>
>> @@ -235,9 +237,8 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> return;
>> }
>>
>> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> /* Disable the TMC only if it needs to */
>> - if (val != CS_MODE_DISABLED)
>> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
>> tmc_etb_disable_hw(drvdata);
>
> This would work but tmc_enable_etf_sink() and tmc_disable_etf_sink()
> are no longer balanced. Another approach would be to add a "mode"
> parameter to tmc_etb_disable_hw() and so something like:
>
> if (val != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata, val);
>
> In tmc_etb_disable_hw(), if mode == CS_MODE_SYSFS then we can move
> ahead with the dump operation. The same apply for ETR
I think we should :
1) First switch the drvdata->mode to a normal type from local_t. Using an
atomic type for mode is completely unnecessary and comes with the overhead
of barriers/synchronisation instructions, while all accesses, including read/write
are performed under the drvdata->spinlock. I have a patch already for this, which
I plan to send it soon.
and
2) Do something like :
void tmc_disable_etX_sink()
{
if (drvdata->mode != CS_MODE_DISABLED) {
tmc_etX_disable_hw(drvdata);
drvdata->mode = CS_MODE_DISABLED;
}
}
Leaving the tmc_etX_disable_hw() untouched.
Suzuki
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-14 12:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-13 12:20 [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode Venkatesh Vivekanandan
2016-09-13 15:41 ` Mathieu Poirier
2016-09-14 9:56 ` Suzuki K Poulose
[not found] ` <CAGhh56GQmKnk5UaYYW-X1YpsPdb7SwCy1b5ANd3tmdAXBTBbZA@mail.gmail.com>
2016-09-14 12:26 ` Suzuki K Poulose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox