Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Suzuki K Poulose 

On 25/04/16 15:38, Mathieu Poirier wrote:

On 25 April 2016 at 05:16, Suzuki K Poulose  wrote:

On 22/04/16 18:14, Mathieu Poirier wrote:


Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
   drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
   2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
*drvdata)
 CS_UNLOCK(drvdata->base);

 tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to
be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
 tmc_disable_hw(drvdata);

 CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
*drvdata)
 CS_UNLOCK(drvdata->base);

 tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to
be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
 tmc_disable_hw(drvdata);



Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
decide
whether to dump the hw data or not. That way, the callers can make the
decision,
leaving the disable_hw code unaware of the mode checks.


My goal in pushing the functionality to tmc_etX_disable_hw() was to
especially keep the the higher layers unaware of sink buffer
management specifics.  There is two ways to look at this and we simply
landed on different sides of the fence.  I'm not strongly opinionated
on this, I'll modify if you're really keen on this.


I am not, you can retain as it is.

Reviewed-by: Suzuki K Poulose 

Suzuki

Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Suzuki K Poulose 

On 25/04/16 15:38, Mathieu Poirier wrote:

On 25 April 2016 at 05:16, Suzuki K Poulose  wrote:

On 22/04/16 18:14, Mathieu Poirier wrote:


Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
   drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
   2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
*drvdata)
 CS_UNLOCK(drvdata->base);

 tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to
be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
 tmc_disable_hw(drvdata);

 CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
*drvdata)
 CS_UNLOCK(drvdata->base);

 tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to
be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
 tmc_disable_hw(drvdata);



Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
decide
whether to dump the hw data or not. That way, the callers can make the
decision,
leaving the disable_hw code unaware of the mode checks.


My goal in pushing the functionality to tmc_etX_disable_hw() was to
especially keep the the higher layers unaware of sink buffer
management specifics.  There is two ways to look at this and we simply
landed on different sides of the fence.  I'm not strongly opinionated
on this, I'll modify if you're really keen on this.


I am not, you can retain as it is.

Reviewed-by: Suzuki K Poulose 

Suzuki

Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Mathieu Poirier 
On 25 April 2016 at 05:16, Suzuki K Poulose  wrote:
> On 22/04/16 18:14, Mathieu Poirier wrote:
>>
>> Calling tmc_etf/etr_dump_hw() is required only when operating from
>> sysFS.  When working from Perf, the system memory is harvested
>> from the AUX trace API.
>>
>> Signed-off-by: Mathieu Poirier 
>> ---
>>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
>>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index d090a9745c73..25fad93b68d4 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
>> *drvdata)
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> -   tmc_etb_dump_hw(drvdata);
>> +   /*
>> +* When operating in sysFS mode the content of the buffer needs to
>> be
>> +* read before the TMC is disabled.
>> +*/
>> +   if (local_read(>mode) == CS_MODE_SYSFS)
>> +   tmc_etb_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>>
>> CS_LOCK(drvdata->base);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 8bbbf3ab1387..4b000f4003a2 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
>> *drvdata)
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> -   tmc_etr_dump_hw(drvdata);
>> +   /*
>> +* When operating in sysFS mode the content of the buffer needs to
>> be
>> +* read before the TMC is disabled.
>> +*/
>> +   if (local_read(>mode) == CS_MODE_SYSFS)
>> +   tmc_etr_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>
>
> Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
> decide
> whether to dump the hw data or not. That way, the callers can make the
> decision,
> leaving the disable_hw code unaware of the mode checks.

My goal in pushing the functionality to tmc_etX_disable_hw() was to
especially keep the the higher layers unaware of sink buffer
management specifics.  There is two ways to look at this and we simply
landed on different sides of the fence.  I'm not strongly opinionated
on this, I'll modify if you're really keen on this.

Thanks,
Mathieu

>
> Suzuki

Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Mathieu Poirier 
On 25 April 2016 at 05:16, Suzuki K Poulose  wrote:
> On 22/04/16 18:14, Mathieu Poirier wrote:
>>
>> Calling tmc_etf/etr_dump_hw() is required only when operating from
>> sysFS.  When working from Perf, the system memory is harvested
>> from the AUX trace API.
>>
>> Signed-off-by: Mathieu Poirier 
>> ---
>>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
>>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index d090a9745c73..25fad93b68d4 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata
>> *drvdata)
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> -   tmc_etb_dump_hw(drvdata);
>> +   /*
>> +* When operating in sysFS mode the content of the buffer needs to
>> be
>> +* read before the TMC is disabled.
>> +*/
>> +   if (local_read(>mode) == CS_MODE_SYSFS)
>> +   tmc_etb_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>>
>> CS_LOCK(drvdata->base);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 8bbbf3ab1387..4b000f4003a2 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata
>> *drvdata)
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> -   tmc_etr_dump_hw(drvdata);
>> +   /*
>> +* When operating in sysFS mode the content of the buffer needs to
>> be
>> +* read before the TMC is disabled.
>> +*/
>> +   if (local_read(>mode) == CS_MODE_SYSFS)
>> +   tmc_etr_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>
>
> Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to
> decide
> whether to dump the hw data or not. That way, the callers can make the
> decision,
> leaving the disable_hw code unaware of the mode checks.

My goal in pushing the functionality to tmc_etX_disable_hw() was to
especially keep the the higher layers unaware of sink buffer
management specifics.  There is two ways to look at this and we simply
landed on different sides of the fence.  I'm not strongly opinionated
on this, I'll modify if you're really keen on this.

Thanks,
Mathieu

>
> Suzuki

Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Suzuki K Poulose 

On 22/04/16 18:14, Mathieu Poirier wrote:

Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
  drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
  drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
  2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c 
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);

tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);

CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c 
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);

tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);


Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to 
decide
whether to dump the hw data or not. That way, the callers can make the decision,
leaving the disable_hw code unaware of the mode checks.

Suzuki

Re: [PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-25 Thread Suzuki K Poulose 

On 22/04/16 18:14, Mathieu Poirier wrote:

Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
  drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
  drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
  2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c 
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);

tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);

CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c 
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);

tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);


Nit: It would be cleaner if tmc_etX_disable_hw accepted a bool parameter to 
decide
whether to dump the hw data or not. That way, the callers can make the decision,
leaving the disable_hw code unaware of the mode checks.

Suzuki

[PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-22 Thread Mathieu Poirier 
Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c 
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);
 
tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);
 
CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c 
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);
 
tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);
 
CS_LOCK(drvdata->base);
-- 
2.5.0

[PATCH V3 12/18] coresight: tmc: dump system memory content only when needed

2016-04-22 Thread Mathieu Poirier 
Calling tmc_etf/etr_dump_hw() is required only when operating from
sysFS.  When working from Perf, the system memory is harvested
from the AUX trace API.

Signed-off-by: Mathieu Poirier 
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 ++-
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c 
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d090a9745c73..25fad93b68d4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -74,7 +74,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);
 
tmc_flush_and_stop(drvdata);
-   tmc_etb_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etb_dump_hw(drvdata);
tmc_disable_hw(drvdata);
 
CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c 
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 8bbbf3ab1387..4b000f4003a2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -76,7 +76,12 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);
 
tmc_flush_and_stop(drvdata);
-   tmc_etr_dump_hw(drvdata);
+   /*
+* When operating in sysFS mode the content of the buffer needs to be
+* read before the TMC is disabled.
+*/
+   if (local_read(>mode) == CS_MODE_SYSFS)
+   tmc_etr_dump_hw(drvdata);
tmc_disable_hw(drvdata);
 
CS_LOCK(drvdata->base);
-- 
2.5.0