Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-19 Thread Suzuki K Poulose 

On 19/01/17 11:40, Greg KH wrote:

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Acked-by: Mathieu Poirier 
Reviewed-by: Chunyan Zhang 
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---

Greg,

Without this patch, the coresight STM IP can only be used for one tracing
session per boot, seriously limiting its usability.


When you resend a patch, please tell me what is different from the
previous version you sent.  I figured it out here, but please do this
next time.


Hi Greg,

Sure, will keep that in mind. For this patch nothing has changed, except for
the addition of Review/Ack. It was resent (with to: you) just to make sure it
gets through the rc series. Thanks for queuing it.

Suzuki

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-19 Thread Suzuki K Poulose 

On 19/01/17 11:40, Greg KH wrote:

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Acked-by: Mathieu Poirier 
Reviewed-by: Chunyan Zhang 
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---

Greg,

Without this patch, the coresight STM IP can only be used for one tracing
session per boot, seriously limiting its usability.


When you resend a patch, please tell me what is different from the
previous version you sent.  I figured it out here, but please do this
next time.


Hi Greg,

Sure, will keep that in mind. For this patch nothing has changed, except for
the addition of Review/Ack. It was resent (with to: you) just to make sure it
gets through the rc series. Thanks for queuing it.

Suzuki

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-19 Thread Greg KH 
On Mon, Jan 16, 2017 at 06:00:00PM +, Suzuki K Poulose wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't enable the hardware when required. Even manually
> enabling the STM via sysfs can't really enable the hw.
> 
> e.g,
> 
>  $ echo 1 > $CS_DEVS/$ETR/enable_sink
>  $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
>  $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
>  $ echo 64 > $CS_DEVS/$source/traceid
>  $ ./stm_app
>  Sending 64000 byte blocks of pattern 0 at 0us intervals
>  Success to map channel(32768~32783) to 0xa95fa000
>  Sending on channel 32768
>  $ dd if=/dev/$ETR of=~/trace.bin.1
>  597+1 records in
>  597+1 records out
>  305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
>  $ ./stm_app
>  Sending 64000 byte blocks of pattern 0 at 0us intervals
>  Success to map channel(32768~32783) to 0x7e9e2000
>  Sending on channel 32768
>  $ dd if=/dev/$ETR of=~/trace.bin.2
>  0+0 records in
>  0+0 records out
>  0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
> 
>  Note that we don't get any data from the ETR for the second session.
> 
>  Also dmesg shows :
> 
>  [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
>  [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
>  [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
>  [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
>  [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
>  [   77.618422] coresight-stm 2086.stm: STM tracing enabled
>  [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>   # End of first tracing session
>  [  146.351135] coresight-tmc 2080.etr: TMC read start
>  [  146.514486] coresight-tmc 2080.etr: TMC read end
>   # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>   # and hence none of the components are turned on.
>  [  152.479080] coresight-tmc 2080.etr: TMC read start
>  [  152.542632] coresight-tmc 2080.etr: TMC read end
> 
> This patch fixes the problem by balancing the unlink operation by using
> the coresight_disable(), keeping the coresight layer in sync with the
> hardware state and thus allowing normal usage of the STM component.
> 
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Acked-by: Mathieu Poirier 
> Reviewed-by: Chunyan Zhang 
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
> 
> Greg,
> 
> Without this patch, the coresight STM IP can only be used for one tracing
> session per boot, seriously limiting its usability.

When you resend a patch, please tell me what is different from the
previous version you sent.  I figured it out here, but please do this
next time.

thanks,

greg k-h

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-19 Thread Greg KH 
On Mon, Jan 16, 2017 at 06:00:00PM +, Suzuki K Poulose wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't enable the hardware when required. Even manually
> enabling the STM via sysfs can't really enable the hw.
> 
> e.g,
> 
>  $ echo 1 > $CS_DEVS/$ETR/enable_sink
>  $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
>  $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
>  $ echo 64 > $CS_DEVS/$source/traceid
>  $ ./stm_app
>  Sending 64000 byte blocks of pattern 0 at 0us intervals
>  Success to map channel(32768~32783) to 0xa95fa000
>  Sending on channel 32768
>  $ dd if=/dev/$ETR of=~/trace.bin.1
>  597+1 records in
>  597+1 records out
>  305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
>  $ ./stm_app
>  Sending 64000 byte blocks of pattern 0 at 0us intervals
>  Success to map channel(32768~32783) to 0x7e9e2000
>  Sending on channel 32768
>  $ dd if=/dev/$ETR of=~/trace.bin.2
>  0+0 records in
>  0+0 records out
>  0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
> 
>  Note that we don't get any data from the ETR for the second session.
> 
>  Also dmesg shows :
> 
>  [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
>  [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
>  [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
>  [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
>  [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
>  [   77.618422] coresight-stm 2086.stm: STM tracing enabled
>  [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>   # End of first tracing session
>  [  146.351135] coresight-tmc 2080.etr: TMC read start
>  [  146.514486] coresight-tmc 2080.etr: TMC read end
>   # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>   # and hence none of the components are turned on.
>  [  152.479080] coresight-tmc 2080.etr: TMC read start
>  [  152.542632] coresight-tmc 2080.etr: TMC read end
> 
> This patch fixes the problem by balancing the unlink operation by using
> the coresight_disable(), keeping the coresight layer in sync with the
> hardware state and thus allowing normal usage of the STM component.
> 
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Acked-by: Mathieu Poirier 
> Reviewed-by: Chunyan Zhang 
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
> 
> Greg,
> 
> Without this patch, the coresight STM IP can only be used for one tracing
> session per boot, seriously limiting its usability.

When you resend a patch, please tell me what is different from the
previous version you sent.  I figured it out here, but please do this
next time.

thanks,

greg k-h

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-16 Thread Mathieu Poirier 
On 13 January 2017 at 10:11, Suzuki K Poulose  wrote:
> On 13/01/17 16:48, Mathieu Poirier wrote:
>>
>> On 10 January 2017 at 04:21, Suzuki K Poulose 
>> wrote:
>>>
>>> The stm is automatically enabled when an application sets the policy
>>> via ->link() call back by using coresight_enable(), which keeps the
>>> refcount of the current users of the STM. However, the unlink() callback
>>> issues stm_disable() directly, which leaves the STM turned off, without
>>> the coresight layer knowing about it. This prevents any further uses
>>> of the STM hardware as the coresight layer still thinks the STM is
>>> turned on and doesn't issue an stm_enable(). Even manually enabling
>>> the STM via sysfs can't really enable the hw.
>>>
> ...
>>>
>>>
>>> This patch balances the unlink operation by using the
>>> coresight_disable(),
>>> keeping the coresight layer in sync with the hardware state.
>>>
>>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight
>>> STM component")
>>> Cc: Pratik Patel 
>>> Cc: Mathieu Poirier 
>>> Cc: Chunyan Zhang 
>>> Cc: Greg Kroah-Hartman 
>>> Cc: sta...@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker 
>>> Signed-off-by: Suzuki K Poulose 
>>> ---
>>>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c
>>> b/drivers/hwtracing/coresight/coresight-stm.c
>>> index 3524452..57b7330 100644
>>> --- a/drivers/hwtracing/coresight/coresight-stm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>>> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data
>>> *stm_data,
>>> if (!drvdata || !drvdata->csdev)
>>> return;
>>>
>>> -   stm_disable(drvdata->csdev, NULL);
>>> +   coresight_disable(drvdata->csdev);
>>>  }
>>>
>>>  static phys_addr_t
>>
>>
>> Applied - thanks,
>
>
> Mathieu, Greg,
>
> I think this should go into 4.10 (either way, as fix in this cycle or via
> stable after the release). I think
> it would be easier if it goes in as fix during one of these rc cycle.
>
> Please let me know your thoughts.

I'm good with squeezing this patch in the 4.10 cycle.  From here I
suppose the easiest for Greg is for you to send another patch with
Chunyan's Reviewed-by and my ack.

>
> Suzuki
>
>> Mathieu
>>
>>> --
>>> 2.7.4
>>>
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-16 Thread Mathieu Poirier 
On 13 January 2017 at 10:11, Suzuki K Poulose  wrote:
> On 13/01/17 16:48, Mathieu Poirier wrote:
>>
>> On 10 January 2017 at 04:21, Suzuki K Poulose 
>> wrote:
>>>
>>> The stm is automatically enabled when an application sets the policy
>>> via ->link() call back by using coresight_enable(), which keeps the
>>> refcount of the current users of the STM. However, the unlink() callback
>>> issues stm_disable() directly, which leaves the STM turned off, without
>>> the coresight layer knowing about it. This prevents any further uses
>>> of the STM hardware as the coresight layer still thinks the STM is
>>> turned on and doesn't issue an stm_enable(). Even manually enabling
>>> the STM via sysfs can't really enable the hw.
>>>
> ...
>>>
>>>
>>> This patch balances the unlink operation by using the
>>> coresight_disable(),
>>> keeping the coresight layer in sync with the hardware state.
>>>
>>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight
>>> STM component")
>>> Cc: Pratik Patel 
>>> Cc: Mathieu Poirier 
>>> Cc: Chunyan Zhang 
>>> Cc: Greg Kroah-Hartman 
>>> Cc: sta...@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker 
>>> Signed-off-by: Suzuki K Poulose 
>>> ---
>>>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c
>>> b/drivers/hwtracing/coresight/coresight-stm.c
>>> index 3524452..57b7330 100644
>>> --- a/drivers/hwtracing/coresight/coresight-stm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>>> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data
>>> *stm_data,
>>> if (!drvdata || !drvdata->csdev)
>>> return;
>>>
>>> -   stm_disable(drvdata->csdev, NULL);
>>> +   coresight_disable(drvdata->csdev);
>>>  }
>>>
>>>  static phys_addr_t
>>
>>
>> Applied - thanks,
>
>
> Mathieu, Greg,
>
> I think this should go into 4.10 (either way, as fix in this cycle or via
> stable after the release). I think
> it would be easier if it goes in as fix during one of these rc cycle.
>
> Please let me know your thoughts.

I'm good with squeezing this patch in the 4.10 cycle.  From here I
suppose the easiest for Greg is for you to send another patch with
Chunyan's Reviewed-by and my ack.

>
> Suzuki
>
>> Mathieu
>>
>>> --
>>> 2.7.4
>>>
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-13 Thread Suzuki K Poulose 

On 13/01/17 16:48, Mathieu Poirier wrote:

On 10 January 2017 at 04:21, Suzuki K Poulose  wrote:

The stm is automatically enabled when an application sets the policy
via ->link() call back by using coresight_enable(), which keeps the
refcount of the current users of the STM. However, the unlink() callback
issues stm_disable() directly, which leaves the STM turned off, without
the coresight layer knowing about it. This prevents any further uses
of the STM hardware as the coresight layer still thinks the STM is
turned on and doesn't issue an stm_enable(). Even manually enabling
the STM via sysfs can't really enable the hw.


...


This patch balances the unlink operation by using the coresight_disable(),
keeping the coresight layer in sync with the hardware state.

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Mathieu Poirier 
Cc: Chunyan Zhang 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---
 drivers/hwtracing/coresight/coresight-stm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
b/drivers/hwtracing/coresight/coresight-stm.c
index 3524452..57b7330 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
if (!drvdata || !drvdata->csdev)
return;

-   stm_disable(drvdata->csdev, NULL);
+   coresight_disable(drvdata->csdev);
 }

 static phys_addr_t


Applied - thanks,


Mathieu, Greg,

I think this should go into 4.10 (either way, as fix in this cycle or via 
stable after the release). I think
it would be easier if it goes in as fix during one of these rc cycle.

Please let me know your thoughts.

Suzuki


Mathieu


--
2.7.4

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-13 Thread Suzuki K Poulose 

On 13/01/17 16:48, Mathieu Poirier wrote:

On 10 January 2017 at 04:21, Suzuki K Poulose  wrote:

The stm is automatically enabled when an application sets the policy
via ->link() call back by using coresight_enable(), which keeps the
refcount of the current users of the STM. However, the unlink() callback
issues stm_disable() directly, which leaves the STM turned off, without
the coresight layer knowing about it. This prevents any further uses
of the STM hardware as the coresight layer still thinks the STM is
turned on and doesn't issue an stm_enable(). Even manually enabling
the STM via sysfs can't really enable the hw.


...


This patch balances the unlink operation by using the coresight_disable(),
keeping the coresight layer in sync with the hardware state.

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Mathieu Poirier 
Cc: Chunyan Zhang 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---
 drivers/hwtracing/coresight/coresight-stm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
b/drivers/hwtracing/coresight/coresight-stm.c
index 3524452..57b7330 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
if (!drvdata || !drvdata->csdev)
return;

-   stm_disable(drvdata->csdev, NULL);
+   coresight_disable(drvdata->csdev);
 }

 static phys_addr_t


Applied - thanks,


Mathieu, Greg,

I think this should go into 4.10 (either way, as fix in this cycle or via 
stable after the release). I think
it would be easier if it goes in as fix during one of these rc cycle.

Please let me know your thoughts.

Suzuki


Mathieu


--
2.7.4

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-13 Thread Mathieu Poirier 
On 10 January 2017 at 04:21, Suzuki K Poulose  wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't issue an stm_enable(). Even manually enabling
> the STM via sysfs can't really enable the hw.
>
> e.g,
>
> $ echo 1 > $CS_DEVS/$ETR/enable_sink
> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
> $ echo 64 > $CS_DEVS/$source/traceid
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xa95fa000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.1
> 597+1 records in
> 597+1 records out
> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0x7e9e2000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.2
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>
> Note that we don't get any data from the ETR for the second session.
>
> Also dmesg shows :
>
> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 2080.etr: TMC read start
> [  146.514486] coresight-tmc 2080.etr: TMC read end
>  # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>  # and hence none of the components are turned on.
> [  152.479080] coresight-tmc 2080.etr: TMC read start
> [  152.542632] coresight-tmc 2080.etr: TMC read end
>
> This patch balances the unlink operation by using the coresight_disable(),
> keeping the coresight layer in sync with the hardware state.
>
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Mathieu Poirier 
> Cc: Chunyan Zhang 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
> b/drivers/hwtracing/coresight/coresight-stm.c
> index 3524452..57b7330 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
> if (!drvdata || !drvdata->csdev)
> return;
>
> -   stm_disable(drvdata->csdev, NULL);
> +   coresight_disable(drvdata->csdev);
>  }
>
>  static phys_addr_t

Applied - thanks,
Mathieu

> --
> 2.7.4
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-13 Thread Mathieu Poirier 
On 10 January 2017 at 04:21, Suzuki K Poulose  wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't issue an stm_enable(). Even manually enabling
> the STM via sysfs can't really enable the hw.
>
> e.g,
>
> $ echo 1 > $CS_DEVS/$ETR/enable_sink
> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
> $ echo 64 > $CS_DEVS/$source/traceid
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xa95fa000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.1
> 597+1 records in
> 597+1 records out
> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0x7e9e2000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.2
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>
> Note that we don't get any data from the ETR for the second session.
>
> Also dmesg shows :
>
> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 2080.etr: TMC read start
> [  146.514486] coresight-tmc 2080.etr: TMC read end
>  # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>  # and hence none of the components are turned on.
> [  152.479080] coresight-tmc 2080.etr: TMC read start
> [  152.542632] coresight-tmc 2080.etr: TMC read end
>
> This patch balances the unlink operation by using the coresight_disable(),
> keeping the coresight layer in sync with the hardware state.
>
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Mathieu Poirier 
> Cc: Chunyan Zhang 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
> b/drivers/hwtracing/coresight/coresight-stm.c
> index 3524452..57b7330 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
> if (!drvdata || !drvdata->csdev)
> return;
>
> -   stm_disable(drvdata->csdev, NULL);
> +   coresight_disable(drvdata->csdev);
>  }
>
>  static phys_addr_t

Applied - thanks,
Mathieu

> --
> 2.7.4
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-12 Thread Chunyan Zhang 
On 11 January 2017 at 21:59, Suzuki K Poulose  wrote:
> On 11/01/17 11:41, Chunyan Zhang wrote:
>>
>> On 11 January 2017 at 01:36, Mathieu Poirier 
>> wrote:
>>>
>>> On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:

 The stm is automatically enabled when an application sets the policy
 via ->link() call back by using coresight_enable(), which keeps the
 refcount of the current users of the STM. However, the unlink() callback
 issues stm_disable() directly, which leaves the STM turned off, without
 the coresight layer knowing about it. This prevents any further uses
 of the STM hardware as the coresight layer still thinks the STM is
 turned on and doesn't issue an stm_enable(). Even manually enabling
 the STM via sysfs can't really enable the hw.

 e.g,

 $ echo 1 > $CS_DEVS/$ETR/enable_sink
 $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
 $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
 $ echo 64 > $CS_DEVS/$source/traceid
 $ ./stm_app
 Sending 64000 byte blocks of pattern 0 at 0us intervals
 Success to map channel(32768~32783) to 0xa95fa000
 Sending on channel 32768
 $ dd if=/dev/$ETR of=~/trace.bin.1
 597+1 records in
 597+1 records out
 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
 $ ./stm_app
 Sending 64000 byte blocks of pattern 0 at 0us intervals
 Success to map channel(32768~32783) to 0x7e9e2000
 Sending on channel 32768
 $ dd if=/dev/$ETR of=~/trace.bin.2
 0+0 records in
 0+0 records out
 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s

 Note that we don't get any data from the ETR for the second session.

 Also dmesg shows :

 [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
 [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR
 enabled
 [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR
 enabled
 [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0
 enabled
 [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
 [   77.618422] coresight-stm 2086.stm: STM tracing enabled
 [  139.554252] coresight-stm 2086.stm: STM tracing disabled
  # End of first tracing session
 [  146.351135] coresight-tmc 2080.etr: TMC read start
 [  146.514486] coresight-tmc 2080.etr: TMC read end
  # Note that the STM is not turned on via
 stm_generic_link()->coresight_enable()
  # and hence none of the components are turned on.
 [  152.479080] coresight-tmc 2080.etr: TMC read start
 [  152.542632] coresight-tmc 2080.etr: TMC read end

 This patch balances the unlink operation by using the
 coresight_disable(),
 keeping the coresight layer in sync with the hardware state.

 Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for
 CoreSight STM component")
 Cc: Pratik Patel 
 Cc: Mathieu Poirier 
 Cc: Chunyan Zhang 
 Cc: Greg Kroah-Hartman 
 Cc: sta...@vger.kernel.org # 4.7+
 Reported-by: Robert Walker 
 Signed-off-by: Suzuki K Poulose 
 ---
  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/hwtracing/coresight/coresight-stm.c
 b/drivers/hwtracing/coresight/coresight-stm.c
 index 3524452..57b7330 100644
 --- a/drivers/hwtracing/coresight/coresight-stm.c
 +++ b/drivers/hwtracing/coresight/coresight-stm.c
 @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data
 *stm_data,
   if (!drvdata || !drvdata->csdev)
   return;

 - stm_disable(drvdata->csdev, NULL);
 + coresight_disable(drvdata->csdev);
>>>
>>>
>>> This looks valid to me.
>>>
>>> Chunyan, any reason to use stm_disable() directly rather than calling it
>>> as part
>>> of the device OPS in coresight_disable()?
>>
>>
>> I don't think there's some special reason for this. I simply hadn't
>> noticed that these two operations didn't use two balanced functions.
>
>
> Please can I have an Ack/Reviewed -by on it, so that we can push it
> as a fix.

Sure, I've had a run with this patch, it works well, so,
Reviewed-by: Chunyan Zhang 

Thanks,
Chunyan

>
>
> Suzuki
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-12 Thread Chunyan Zhang 
On 11 January 2017 at 21:59, Suzuki K Poulose  wrote:
> On 11/01/17 11:41, Chunyan Zhang wrote:
>>
>> On 11 January 2017 at 01:36, Mathieu Poirier 
>> wrote:
>>>
>>> On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:

 The stm is automatically enabled when an application sets the policy
 via ->link() call back by using coresight_enable(), which keeps the
 refcount of the current users of the STM. However, the unlink() callback
 issues stm_disable() directly, which leaves the STM turned off, without
 the coresight layer knowing about it. This prevents any further uses
 of the STM hardware as the coresight layer still thinks the STM is
 turned on and doesn't issue an stm_enable(). Even manually enabling
 the STM via sysfs can't really enable the hw.

 e.g,

 $ echo 1 > $CS_DEVS/$ETR/enable_sink
 $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
 $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
 $ echo 64 > $CS_DEVS/$source/traceid
 $ ./stm_app
 Sending 64000 byte blocks of pattern 0 at 0us intervals
 Success to map channel(32768~32783) to 0xa95fa000
 Sending on channel 32768
 $ dd if=/dev/$ETR of=~/trace.bin.1
 597+1 records in
 597+1 records out
 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
 $ ./stm_app
 Sending 64000 byte blocks of pattern 0 at 0us intervals
 Success to map channel(32768~32783) to 0x7e9e2000
 Sending on channel 32768
 $ dd if=/dev/$ETR of=~/trace.bin.2
 0+0 records in
 0+0 records out
 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s

 Note that we don't get any data from the ETR for the second session.

 Also dmesg shows :

 [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
 [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR
 enabled
 [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR
 enabled
 [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0
 enabled
 [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
 [   77.618422] coresight-stm 2086.stm: STM tracing enabled
 [  139.554252] coresight-stm 2086.stm: STM tracing disabled
  # End of first tracing session
 [  146.351135] coresight-tmc 2080.etr: TMC read start
 [  146.514486] coresight-tmc 2080.etr: TMC read end
  # Note that the STM is not turned on via
 stm_generic_link()->coresight_enable()
  # and hence none of the components are turned on.
 [  152.479080] coresight-tmc 2080.etr: TMC read start
 [  152.542632] coresight-tmc 2080.etr: TMC read end

 This patch balances the unlink operation by using the
 coresight_disable(),
 keeping the coresight layer in sync with the hardware state.

 Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for
 CoreSight STM component")
 Cc: Pratik Patel 
 Cc: Mathieu Poirier 
 Cc: Chunyan Zhang 
 Cc: Greg Kroah-Hartman 
 Cc: sta...@vger.kernel.org # 4.7+
 Reported-by: Robert Walker 
 Signed-off-by: Suzuki K Poulose 
 ---
  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/hwtracing/coresight/coresight-stm.c
 b/drivers/hwtracing/coresight/coresight-stm.c
 index 3524452..57b7330 100644
 --- a/drivers/hwtracing/coresight/coresight-stm.c
 +++ b/drivers/hwtracing/coresight/coresight-stm.c
 @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data
 *stm_data,
   if (!drvdata || !drvdata->csdev)
   return;

 - stm_disable(drvdata->csdev, NULL);
 + coresight_disable(drvdata->csdev);
>>>
>>>
>>> This looks valid to me.
>>>
>>> Chunyan, any reason to use stm_disable() directly rather than calling it
>>> as part
>>> of the device OPS in coresight_disable()?
>>
>>
>> I don't think there's some special reason for this. I simply hadn't
>> noticed that these two operations didn't use two balanced functions.
>
>
> Please can I have an Ack/Reviewed -by on it, so that we can push it
> as a fix.

Sure, I've had a run with this patch, it works well, so,
Reviewed-by: Chunyan Zhang 

Thanks,
Chunyan

>
>
> Suzuki
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-11 Thread Suzuki K Poulose 

On 11/01/17 11:41, Chunyan Zhang wrote:

On 11 January 2017 at 01:36, Mathieu Poirier  wrote:

On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:

The stm is automatically enabled when an application sets the policy
via ->link() call back by using coresight_enable(), which keeps the
refcount of the current users of the STM. However, the unlink() callback
issues stm_disable() directly, which leaves the STM turned off, without
the coresight layer knowing about it. This prevents any further uses
of the STM hardware as the coresight layer still thinks the STM is
turned on and doesn't issue an stm_enable(). Even manually enabling
the STM via sysfs can't really enable the hw.

e.g,

$ echo 1 > $CS_DEVS/$ETR/enable_sink
$ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
$ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
$ echo 64 > $CS_DEVS/$source/traceid
$ ./stm_app
Sending 64000 byte blocks of pattern 0 at 0us intervals
Success to map channel(32768~32783) to 0xa95fa000
Sending on channel 32768
$ dd if=/dev/$ETR of=~/trace.bin.1
597+1 records in
597+1 records out
305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
$ ./stm_app
Sending 64000 byte blocks of pattern 0 at 0us intervals
Success to map channel(32768~32783) to 0x7e9e2000
Sending on channel 32768
$ dd if=/dev/$ETR of=~/trace.bin.2
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s

Note that we don't get any data from the ETR for the second session.

Also dmesg shows :

[   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
[   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR enabled
[   77.558828] coresight-replicator main_replicator@208a: REPLICATOR enabled
[   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
[   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
[   77.618422] coresight-stm 2086.stm: STM tracing enabled
[  139.554252] coresight-stm 2086.stm: STM tracing disabled
 # End of first tracing session
[  146.351135] coresight-tmc 2080.etr: TMC read start
[  146.514486] coresight-tmc 2080.etr: TMC read end
 # Note that the STM is not turned on via stm_generic_link()->coresight_enable()
 # and hence none of the components are turned on.
[  152.479080] coresight-tmc 2080.etr: TMC read start
[  152.542632] coresight-tmc 2080.etr: TMC read end

This patch balances the unlink operation by using the coresight_disable(),
keeping the coresight layer in sync with the hardware state.

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Mathieu Poirier 
Cc: Chunyan Zhang 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---
 drivers/hwtracing/coresight/coresight-stm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
b/drivers/hwtracing/coresight/coresight-stm.c
index 3524452..57b7330 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
  if (!drvdata || !drvdata->csdev)
  return;

- stm_disable(drvdata->csdev, NULL);
+ coresight_disable(drvdata->csdev);


This looks valid to me.

Chunyan, any reason to use stm_disable() directly rather than calling it as part
of the device OPS in coresight_disable()?


I don't think there's some special reason for this. I simply hadn't
noticed that these two operations didn't use two balanced functions.


Please can I have an Ack/Reviewed -by on it, so that we can push it
as a fix.


Suzuki

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-11 Thread Suzuki K Poulose 

On 11/01/17 11:41, Chunyan Zhang wrote:

On 11 January 2017 at 01:36, Mathieu Poirier  wrote:

On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:

The stm is automatically enabled when an application sets the policy
via ->link() call back by using coresight_enable(), which keeps the
refcount of the current users of the STM. However, the unlink() callback
issues stm_disable() directly, which leaves the STM turned off, without
the coresight layer knowing about it. This prevents any further uses
of the STM hardware as the coresight layer still thinks the STM is
turned on and doesn't issue an stm_enable(). Even manually enabling
the STM via sysfs can't really enable the hw.

e.g,

$ echo 1 > $CS_DEVS/$ETR/enable_sink
$ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
$ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
$ echo 64 > $CS_DEVS/$source/traceid
$ ./stm_app
Sending 64000 byte blocks of pattern 0 at 0us intervals
Success to map channel(32768~32783) to 0xa95fa000
Sending on channel 32768
$ dd if=/dev/$ETR of=~/trace.bin.1
597+1 records in
597+1 records out
305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
$ ./stm_app
Sending 64000 byte blocks of pattern 0 at 0us intervals
Success to map channel(32768~32783) to 0x7e9e2000
Sending on channel 32768
$ dd if=/dev/$ETR of=~/trace.bin.2
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s

Note that we don't get any data from the ETR for the second session.

Also dmesg shows :

[   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
[   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR enabled
[   77.558828] coresight-replicator main_replicator@208a: REPLICATOR enabled
[   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
[   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
[   77.618422] coresight-stm 2086.stm: STM tracing enabled
[  139.554252] coresight-stm 2086.stm: STM tracing disabled
 # End of first tracing session
[  146.351135] coresight-tmc 2080.etr: TMC read start
[  146.514486] coresight-tmc 2080.etr: TMC read end
 # Note that the STM is not turned on via stm_generic_link()->coresight_enable()
 # and hence none of the components are turned on.
[  152.479080] coresight-tmc 2080.etr: TMC read start
[  152.542632] coresight-tmc 2080.etr: TMC read end

This patch balances the unlink operation by using the coresight_disable(),
keeping the coresight layer in sync with the hardware state.

Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
component")
Cc: Pratik Patel 
Cc: Mathieu Poirier 
Cc: Chunyan Zhang 
Cc: Greg Kroah-Hartman 
Cc: sta...@vger.kernel.org # 4.7+
Reported-by: Robert Walker 
Signed-off-by: Suzuki K Poulose 
---
 drivers/hwtracing/coresight/coresight-stm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
b/drivers/hwtracing/coresight/coresight-stm.c
index 3524452..57b7330 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
  if (!drvdata || !drvdata->csdev)
  return;

- stm_disable(drvdata->csdev, NULL);
+ coresight_disable(drvdata->csdev);


This looks valid to me.

Chunyan, any reason to use stm_disable() directly rather than calling it as part
of the device OPS in coresight_disable()?


I don't think there's some special reason for this. I simply hadn't
noticed that these two operations didn't use two balanced functions.


Please can I have an Ack/Reviewed -by on it, so that we can push it
as a fix.


Suzuki

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-11 Thread Chunyan Zhang 
On 11 January 2017 at 01:36, Mathieu Poirier  wrote:
> On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:
>> The stm is automatically enabled when an application sets the policy
>> via ->link() call back by using coresight_enable(), which keeps the
>> refcount of the current users of the STM. However, the unlink() callback
>> issues stm_disable() directly, which leaves the STM turned off, without
>> the coresight layer knowing about it. This prevents any further uses
>> of the STM hardware as the coresight layer still thinks the STM is
>> turned on and doesn't issue an stm_enable(). Even manually enabling
>> the STM via sysfs can't really enable the hw.
>>
>> e.g,
>>
>> $ echo 1 > $CS_DEVS/$ETR/enable_sink
>> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
>> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
>> $ echo 64 > $CS_DEVS/$source/traceid
>> $ ./stm_app
>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>> Success to map channel(32768~32783) to 0xa95fa000
>> Sending on channel 32768
>> $ dd if=/dev/$ETR of=~/trace.bin.1
>> 597+1 records in
>> 597+1 records out
>> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
>> $ ./stm_app
>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>> Success to map channel(32768~32783) to 0x7e9e2000
>> Sending on channel 32768
>> $ dd if=/dev/$ETR of=~/trace.bin.2
>> 0+0 records in
>> 0+0 records out
>> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>>
>> Note that we don't get any data from the ETR for the second session.
>>
>> Also dmesg shows :
>>
>> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
>> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
>> enabled
>> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
>> enabled
>> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
>> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
>> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
>> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>>  # End of first tracing session
>> [  146.351135] coresight-tmc 2080.etr: TMC read start
>> [  146.514486] coresight-tmc 2080.etr: TMC read end
>>  # Note that the STM is not turned on via 
>> stm_generic_link()->coresight_enable()
>>  # and hence none of the components are turned on.
>> [  152.479080] coresight-tmc 2080.etr: TMC read start
>> [  152.542632] coresight-tmc 2080.etr: TMC read end
>>
>> This patch balances the unlink operation by using the coresight_disable(),
>> keeping the coresight layer in sync with the hardware state.
>>
>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight 
>> STM component")
>> Cc: Pratik Patel 
>> Cc: Mathieu Poirier 
>> Cc: Chunyan Zhang 
>> Cc: Greg Kroah-Hartman 
>> Cc: sta...@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker 
>> Signed-off-by: Suzuki K Poulose 
>> ---
>>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
>> b/drivers/hwtracing/coresight/coresight-stm.c
>> index 3524452..57b7330 100644
>> --- a/drivers/hwtracing/coresight/coresight-stm.c
>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>>   if (!drvdata || !drvdata->csdev)
>>   return;
>>
>> - stm_disable(drvdata->csdev, NULL);
>> + coresight_disable(drvdata->csdev);
>
> This looks valid to me.
>
> Chunyan, any reason to use stm_disable() directly rather than calling it as 
> part
> of the device OPS in coresight_disable()?

I don't think there's some special reason for this. I simply hadn't
noticed that these two operations didn't use two balanced functions.

Thanks,
Chunyan

>
> Thanks,
> Mathieu
>
>>  }
>>
>>  static phys_addr_t
>> --
>> 2.7.4
>>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-11 Thread Chunyan Zhang 
On 11 January 2017 at 01:36, Mathieu Poirier  wrote:
> On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:
>> The stm is automatically enabled when an application sets the policy
>> via ->link() call back by using coresight_enable(), which keeps the
>> refcount of the current users of the STM. However, the unlink() callback
>> issues stm_disable() directly, which leaves the STM turned off, without
>> the coresight layer knowing about it. This prevents any further uses
>> of the STM hardware as the coresight layer still thinks the STM is
>> turned on and doesn't issue an stm_enable(). Even manually enabling
>> the STM via sysfs can't really enable the hw.
>>
>> e.g,
>>
>> $ echo 1 > $CS_DEVS/$ETR/enable_sink
>> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
>> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
>> $ echo 64 > $CS_DEVS/$source/traceid
>> $ ./stm_app
>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>> Success to map channel(32768~32783) to 0xa95fa000
>> Sending on channel 32768
>> $ dd if=/dev/$ETR of=~/trace.bin.1
>> 597+1 records in
>> 597+1 records out
>> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
>> $ ./stm_app
>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>> Success to map channel(32768~32783) to 0x7e9e2000
>> Sending on channel 32768
>> $ dd if=/dev/$ETR of=~/trace.bin.2
>> 0+0 records in
>> 0+0 records out
>> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>>
>> Note that we don't get any data from the ETR for the second session.
>>
>> Also dmesg shows :
>>
>> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
>> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
>> enabled
>> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
>> enabled
>> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
>> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
>> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
>> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>>  # End of first tracing session
>> [  146.351135] coresight-tmc 2080.etr: TMC read start
>> [  146.514486] coresight-tmc 2080.etr: TMC read end
>>  # Note that the STM is not turned on via 
>> stm_generic_link()->coresight_enable()
>>  # and hence none of the components are turned on.
>> [  152.479080] coresight-tmc 2080.etr: TMC read start
>> [  152.542632] coresight-tmc 2080.etr: TMC read end
>>
>> This patch balances the unlink operation by using the coresight_disable(),
>> keeping the coresight layer in sync with the hardware state.
>>
>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight 
>> STM component")
>> Cc: Pratik Patel 
>> Cc: Mathieu Poirier 
>> Cc: Chunyan Zhang 
>> Cc: Greg Kroah-Hartman 
>> Cc: sta...@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker 
>> Signed-off-by: Suzuki K Poulose 
>> ---
>>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
>> b/drivers/hwtracing/coresight/coresight-stm.c
>> index 3524452..57b7330 100644
>> --- a/drivers/hwtracing/coresight/coresight-stm.c
>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>>   if (!drvdata || !drvdata->csdev)
>>   return;
>>
>> - stm_disable(drvdata->csdev, NULL);
>> + coresight_disable(drvdata->csdev);
>
> This looks valid to me.
>
> Chunyan, any reason to use stm_disable() directly rather than calling it as 
> part
> of the device OPS in coresight_disable()?

I don't think there's some special reason for this. I simply hadn't
noticed that these two operations didn't use two balanced functions.

Thanks,
Chunyan

>
> Thanks,
> Mathieu
>
>>  }
>>
>>  static phys_addr_t
>> --
>> 2.7.4
>>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-10 Thread Mathieu Poirier 
On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't issue an stm_enable(). Even manually enabling
> the STM via sysfs can't really enable the hw.
> 
> e.g,
> 
> $ echo 1 > $CS_DEVS/$ETR/enable_sink
> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
> $ echo 64 > $CS_DEVS/$source/traceid
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xa95fa000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.1
> 597+1 records in
> 597+1 records out
> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0x7e9e2000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.2
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
> 
> Note that we don't get any data from the ETR for the second session.
> 
> Also dmesg shows :
> 
> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 2080.etr: TMC read start
> [  146.514486] coresight-tmc 2080.etr: TMC read end
>  # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>  # and hence none of the components are turned on.
> [  152.479080] coresight-tmc 2080.etr: TMC read start
> [  152.542632] coresight-tmc 2080.etr: TMC read end
> 
> This patch balances the unlink operation by using the coresight_disable(),
> keeping the coresight layer in sync with the hardware state.
> 
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Mathieu Poirier 
> Cc: Chunyan Zhang 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
> b/drivers/hwtracing/coresight/coresight-stm.c
> index 3524452..57b7330 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>   if (!drvdata || !drvdata->csdev)
>   return;
>  
> - stm_disable(drvdata->csdev, NULL);
> + coresight_disable(drvdata->csdev);

This looks valid to me.

Chunyan, any reason to use stm_disable() directly rather than calling it as part
of the device OPS in coresight_disable()?

Thanks,
Mathieu

>  }
>  
>  static phys_addr_t
> -- 
> 2.7.4
>

Re: [PATCH] coresight: STM: Balance enable/disable

2017-01-10 Thread Mathieu Poirier 
On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't issue an stm_enable(). Even manually enabling
> the STM via sysfs can't really enable the hw.
> 
> e.g,
> 
> $ echo 1 > $CS_DEVS/$ETR/enable_sink
> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
> $ echo 64 > $CS_DEVS/$source/traceid
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xa95fa000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.1
> 597+1 records in
> 597+1 records out
> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0x7e9e2000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.2
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
> 
> Note that we don't get any data from the ETR for the second session.
> 
> Also dmesg shows :
> 
> [   77.520458] coresight-tmc 2080.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@2089: REPLICATOR 
> enabled
> [   77.558828] coresight-replicator main_replicator@208a: REPLICATOR 
> enabled
> [   77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 2084.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 2086.stm: STM tracing enabled
> [  139.554252] coresight-stm 2086.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 2080.etr: TMC read start
> [  146.514486] coresight-tmc 2080.etr: TMC read end
>  # Note that the STM is not turned on via 
> stm_generic_link()->coresight_enable()
>  # and hence none of the components are turned on.
> [  152.479080] coresight-tmc 2080.etr: TMC read start
> [  152.542632] coresight-tmc 2080.etr: TMC read end
> 
> This patch balances the unlink operation by using the coresight_disable(),
> keeping the coresight layer in sync with the hardware state.
> 
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM 
> component")
> Cc: Pratik Patel 
> Cc: Mathieu Poirier 
> Cc: Chunyan Zhang 
> Cc: Greg Kroah-Hartman 
> Cc: sta...@vger.kernel.org # 4.7+
> Reported-by: Robert Walker 
> Signed-off-by: Suzuki K Poulose 
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c 
> b/drivers/hwtracing/coresight/coresight-stm.c
> index 3524452..57b7330 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>   if (!drvdata || !drvdata->csdev)
>   return;
>  
> - stm_disable(drvdata->csdev, NULL);
> + coresight_disable(drvdata->csdev);

This looks valid to me.

Chunyan, any reason to use stm_disable() directly rather than calling it as part
of the device OPS in coresight_disable()?

Thanks,
Mathieu

>  }
>  
>  static phys_addr_t
> -- 
> 2.7.4
>