* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-13 12:20 ` Venkatesh Vivekanandan
0 siblings, 0 replies; 8+ messages in thread
From: Venkatesh Vivekanandan @ 2016-09-13 12:20 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel; +Cc: mathieu.poirier, Venkatesh Vivekanandan
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] 8+ messages in thread
* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-13 12:20 ` Venkatesh Vivekanandan
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [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
-1 siblings, 0 replies; 8+ messages in thread
From: Mathieu Poirier @ 2016-09-13 15:41 UTC (permalink / raw)
To: Venkatesh Vivekanandan; +Cc: linux-arm-kernel, linux-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] 8+ messages in thread
* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-13 15:41 ` Mathieu Poirier
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [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
-1 siblings, 0 replies; 8+ messages in thread
From: Suzuki K Poulose @ 2016-09-14 9:56 UTC (permalink / raw)
To: Mathieu Poirier, Venkatesh Vivekanandan; +Cc: linux-arm-kernel, linux-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] 8+ messages in thread
* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-14 9:56 ` Suzuki K Poulose
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
[not found] ` <CAGhh56GQmKnk5UaYYW-X1YpsPdb7SwCy1b5ANd3tmdAXBTBbZA@mail.gmail.com>
@ 2016-09-14 12:26 ` Suzuki K Poulose
0 siblings, 0 replies; 8+ messages in thread
From: Suzuki K Poulose @ 2016-09-14 12:26 UTC (permalink / raw)
To: Venkatesh Vivekanandan; +Cc: Mathieu Poirier, linux-arm-kernel, linux-kernel
On 14/09/16 12:30, Venkatesh Vivekanandan wrote:
>
>
> On Wed, Sep 14, 2016 at 3:26 PM, Suzuki K Poulose <Suzuki.Poulose@arm.com <mailto:Suzuki.Poulose@arm.com>> wrote:
>
> On 13/09/16 16:41, Mathieu Poirier wrote:
>
> On 13 September 2016 at 06:20, Venkatesh Vivekanandan
> <venkatesh.vivekanandan@broadcom.com <mailto: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.
>
> .
> .
> }
>
> 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;
> }
> }
>
> You will fix this along with above changes?
Yes.
nit: Please fix your mail client. Do not use HTML formatted emails on mailing list
Cheers
Suzuki
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
@ 2016-09-14 12:26 ` Suzuki K Poulose
0 siblings, 0 replies; 8+ messages in thread
From: Suzuki K Poulose @ 2016-09-14 12:26 UTC (permalink / raw)
To: linux-arm-kernel
On 14/09/16 12:30, Venkatesh Vivekanandan wrote:
>
>
> On Wed, Sep 14, 2016 at 3:26 PM, Suzuki K Poulose <Suzuki.Poulose at arm.com <mailto:Suzuki.Poulose@arm.com>> wrote:
>
> On 13/09/16 16:41, Mathieu Poirier wrote:
>
> On 13 September 2016 at 06:20, Venkatesh Vivekanandan
> <venkatesh.vivekanandan at broadcom.com <mailto: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.
>
> .
> .
> }
>
> 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;
> }
> }
>
> You will fix this along with above changes?
Yes.
nit: Please fix your mail client. Do not use HTML formatted emails on mailing list
Cheers
Suzuki
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-14 12:26 UTC | newest]
Thread overview: 8+ 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 12:20 ` Venkatesh Vivekanandan
2016-09-13 15:41 ` Mathieu Poirier
2016-09-13 15:41 ` Mathieu Poirier
2016-09-14 9:56 ` Suzuki K Poulose
2016-09-14 9:56 ` Suzuki K Poulose
[not found] ` <CAGhh56GQmKnk5UaYYW-X1YpsPdb7SwCy1b5ANd3tmdAXBTBbZA@mail.gmail.com>
2016-09-14 12:26 ` Suzuki K Poulose
2016-09-14 12:26 ` Suzuki K Poulose
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.