All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-16 18:00 ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-16 18:00 UTC (permalink / raw)
  To: gregkh
  Cc: mathieu.poirier, zhang.chunyan, pratikp, linux-kernel,
	linux-arm-kernel, stable

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 0xffffa95fa000
 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 0xffff7e9e2000
 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 20800000.etr: TMC-ETR enabled
 [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
 [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
 [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
 [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
 [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
 [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
  # End of first tracing session
 [  146.351135] coresight-tmc 20800000.etr: TMC read start
 [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
 [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org # 4.7+
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Reported-by: Robert Walker <robert.walker@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---

Greg,

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

---
 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 e4c55c5..93fc26f 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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-16 18:00 ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-16 18:00 UTC (permalink / raw)
  To: linux-arm-kernel

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 0xffffa95fa000
 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 0xffff7e9e2000
 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 20800000.etr: TMC-ETR enabled
 [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
 [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
 [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
 [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
 [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
 [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
  # End of first tracing session
 [  146.351135] coresight-tmc 20800000.etr: TMC read start
 [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
 [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable at vger.kernel.org # 4.7+
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Reported-by: Robert Walker <robert.walker@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---

Greg,

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

---
 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 e4c55c5..93fc26f 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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-16 18:00 ` Suzuki K Poulose
@ 2017-01-19 11:40   ` Greg KH
  -1 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-01-19 11:40 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mathieu.poirier, zhang.chunyan, pratikp, linux-kernel,
	linux-arm-kernel, stable

On Mon, Jan 16, 2017 at 06:00:00PM +0000, 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 0xffffa95fa000
>  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 0xffff7e9e2000
>  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 20800000.etr: TMC-ETR enabled
>  [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
>  [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
>  [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>  [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>  [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>  [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>   # End of first tracing session
>  [  146.351135] coresight-tmc 20800000.etr: TMC read start
>  [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>  [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 4.7+
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> 
> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-19 11:40   ` Greg KH
  0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-01-19 11:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 16, 2017 at 06:00:00PM +0000, 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 0xffffa95fa000
>  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 0xffff7e9e2000
>  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 20800000.etr: TMC-ETR enabled
>  [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
>  [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
>  [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>  [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>  [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>  [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>   # End of first tracing session
>  [  146.351135] coresight-tmc 20800000.etr: TMC read start
>  [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>  [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable at vger.kernel.org # 4.7+
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> 
> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-19 11:40   ` Greg KH
@ 2017-01-19 13:55     ` Suzuki K Poulose
  -1 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-19 13:55 UTC (permalink / raw)
  To: Greg KH
  Cc: mathieu.poirier, zhang.chunyan, pratikp, linux-kernel,
	linux-arm-kernel, stable

On 19/01/17 11:40, Greg KH wrote:
>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM component")
>> Cc: Pratik Patel <pratikp@codeaurora.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable@vger.kernel.org # 4.7+
>> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>
>> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-19 13:55     ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-19 13:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 19/01/17 11:40, Greg KH wrote:
>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM component")
>> Cc: Pratik Patel <pratikp@codeaurora.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable at vger.kernel.org # 4.7+
>> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>
>> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-13 17:11     ` Suzuki K Poulose
  (?)
@ 2017-01-16 17:50       ` Mathieu Poirier
  -1 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-16 17:50 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Greg KH, Chunyan Zhang, Pratik Patel, linux-kernel,
	linux-arm-kernel, # 4 . 7

On 13 January 2017 at 10:11, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 13/01/17 16:48, Mathieu Poirier wrote:
>>
>> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com>
>> 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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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
>>>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-16 17:50       ` Mathieu Poirier
  0 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-16 17:50 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Greg KH, Chunyan Zhang, Pratik Patel, linux-kernel,
	linux-arm-kernel, # 4 . 7

On 13 January 2017 at 10:11, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 13/01/17 16:48, Mathieu Poirier wrote:
>>
>> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com>
>> 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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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
>>>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-16 17:50       ` Mathieu Poirier
  0 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-16 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 January 2017 at 10:11, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 13/01/17 16:48, Mathieu Poirier wrote:
>>
>> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com>
>> 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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable at vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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
>>>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-13 16:48   ` Mathieu Poirier
  (?)
@ 2017-01-13 17:11     ` Suzuki K Poulose
  -1 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-13 17:11 UTC (permalink / raw)
  To: Mathieu Poirier, Greg KH
  Cc: Chunyan Zhang, Pratik Patel, linux-kernel, linux-arm-kernel, # 4 . 7

On 13/01/17 16:48, Mathieu Poirier wrote:
> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13 17:11     ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-13 17:11 UTC (permalink / raw)
  To: Mathieu Poirier, Greg KH
  Cc: Chunyan Zhang, Pratik Patel, linux-kernel, linux-arm-kernel, # 4 . 7

On 13/01/17 16:48, Mathieu Poirier wrote:
> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13 17:11     ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-13 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 13/01/17 16:48, Mathieu Poirier wrote:
> On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable at vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-10 11:21 ` Suzuki K Poulose
  (?)
@ 2017-01-13 16:48   ` Mathieu Poirier
  -1 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-13 16:48 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Chunyan Zhang, Pratik Patel, linux-kernel, linux-arm-kernel,
	Greg KH, # 4 . 7

On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 0xffffa95fa000
> 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 0xffff7e9e2000
> 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 20800000.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 20800000.etr: TMC read start
> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13 16:48   ` Mathieu Poirier
  0 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-13 16:48 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Chunyan Zhang, Pratik Patel, linux-kernel, linux-arm-kernel,
	Greg KH, # 4 . 7

On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 0xffffa95fa000
> 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 0xffff7e9e2000
> 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 20800000.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 20800000.etr: TMC read start
> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13 16:48   ` Mathieu Poirier
  0 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-13 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 10 January 2017 at 04:21, Suzuki K Poulose <suzuki.poulose@arm.com> 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 0xffffa95fa000
> 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 0xffff7e9e2000
> 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 20800000.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
> [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 20800000.etr: TMC read start
> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable at vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-11 13:59       ` Suzuki K Poulose
  (?)
@ 2017-01-13  2:10         ` Chunyan Zhang
  -1 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-13  2:10 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Mathieu Poirier, Pratik Patel, linux-kernel, linux-arm-kernel,
	gregkh, stable

On 11 January 2017 at 21:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 11/01/17 11:41, Chunyan Zhang wrote:
>>
>> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org>
>> wrote:
>>>
>>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>>> 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 0xffff7e9e2000
>>>> 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 20800000.etr: TMC-ETR enabled
>>>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR
>>>> enabled
>>>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR
>>>> enabled
>>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0
>>>> enabled
>>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>>  # End of first tracing session
>>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: stable@vger.kernel.org # 4.7+
>>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>  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 <zhang.chunyan@linaro.org>

Thanks,
Chunyan

>
>
> Suzuki
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13  2:10         ` Chunyan Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-13  2:10 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Mathieu Poirier, Pratik Patel, linux-kernel, linux-arm-kernel,
	gregkh, stable

On 11 January 2017 at 21:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 11/01/17 11:41, Chunyan Zhang wrote:
>>
>> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org>
>> wrote:
>>>
>>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>>> 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 0xffff7e9e2000
>>>> 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 20800000.etr: TMC-ETR enabled
>>>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR
>>>> enabled
>>>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR
>>>> enabled
>>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0
>>>> enabled
>>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>>  # End of first tracing session
>>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: stable@vger.kernel.org # 4.7+
>>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>  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 <zhang.chunyan@linaro.org>

Thanks,
Chunyan

>
>
> Suzuki
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-13  2:10         ` Chunyan Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-13  2:10 UTC (permalink / raw)
  To: linux-arm-kernel

On 11 January 2017 at 21:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 11/01/17 11:41, Chunyan Zhang wrote:
>>
>> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org>
>> wrote:
>>>
>>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>>> 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 0xffff7e9e2000
>>>> 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 20800000.etr: TMC-ETR enabled
>>>> [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR
>>>> enabled
>>>> [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR
>>>> enabled
>>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0
>>>> enabled
>>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>>  # End of first tracing session
>>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: stable at vger.kernel.org # 4.7+
>>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>  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 <zhang.chunyan@linaro.org>

Thanks,
Chunyan

>
>
> Suzuki
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-11 11:41     ` Chunyan Zhang
  (?)
@ 2017-01-11 13:59       ` Suzuki K Poulose
  -1 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-11 13:59 UTC (permalink / raw)
  To: Chunyan Zhang, Mathieu Poirier
  Cc: Pratik Patel, linux-kernel, linux-arm-kernel, gregkh, stable

On 11/01/17 11:41, Chunyan Zhang wrote:
> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>> 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 0xffff7e9e2000
>>> 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 20800000.etr: TMC-ETR enabled
>>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
>>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>  # End of first tracing session
>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-11 13:59       ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-11 13:59 UTC (permalink / raw)
  To: Chunyan Zhang, Mathieu Poirier
  Cc: Pratik Patel, linux-kernel, linux-arm-kernel, gregkh, stable

On 11/01/17 11:41, Chunyan Zhang wrote:
> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>> 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 0xffff7e9e2000
>>> 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 20800000.etr: TMC-ETR enabled
>>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
>>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>  # End of first tracing session
>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable@vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-11 13:59       ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-11 13:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/01/17 11:41, Chunyan Zhang wrote:
> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>>> 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 0xffff7e9e2000
>>> 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 20800000.etr: TMC-ETR enabled
>>> [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
>>> [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
>>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>>  # End of first tracing session
>>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable at vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>  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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-10 17:36   ` Mathieu Poirier
  (?)
@ 2017-01-11 11:41     ` Chunyan Zhang
  -1 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-11 11:41 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Pratik Patel, linux-kernel, linux-arm-kernel,
	gregkh, stable

On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>> 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 0xffff7e9e2000
>> 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 20800000.etr: TMC-ETR enabled
>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>  # End of first tracing session
>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-11 11:41     ` Chunyan Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-11 11:41 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Pratik Patel, linux-kernel, linux-arm-kernel,
	gregkh, stable

On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>> 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 0xffff7e9e2000
>> 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 20800000.etr: TMC-ETR enabled
>> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
>> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>  # End of first tracing session
>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable@vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-11 11:41     ` Chunyan Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: Chunyan Zhang @ 2017-01-11 11:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
>> 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 0xffff7e9e2000
>> 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 20800000.etr: TMC-ETR enabled
>> [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
>> [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
>> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
>> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>  # End of first tracing session
>> [  146.351135] coresight-tmc 20800000.etr: TMC read start
>> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
>> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: stable at vger.kernel.org # 4.7+
>> Reported-by: Robert Walker <robert.walker@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] coresight: STM: Balance enable/disable
  2017-01-10 11:21 ` Suzuki K Poulose
@ 2017-01-10 17:36   ` Mathieu Poirier
  -1 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-10 17:36 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: zhang.chunyan, pratikp, linux-kernel, linux-arm-kernel, gregkh, stable

On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
> 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 0xffff7e9e2000
> 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 20800000.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
> [   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 20800000.etr: TMC read start
> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-10 17:36   ` Mathieu Poirier
  0 siblings, 0 replies; 28+ messages in thread
From: Mathieu Poirier @ 2017-01-10 17:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 10, 2017 at 11:21:55AM +0000, 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 0xffffa95fa000
> 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 0xffff7e9e2000
> 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 20800000.etr: TMC-ETR enabled
> [   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
> [   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
> [   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [   77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [  139.554252] coresight-stm 20860000.stm: STM tracing disabled
>  # End of first tracing session
> [  146.351135] coresight-tmc 20800000.etr: TMC read start
> [  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
> [  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable at vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-10 11:21 ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-10 11:21 UTC (permalink / raw)
  To: mathieu.poirier, zhang.chunyan
  Cc: pratikp, linux-kernel, linux-arm-kernel, gregkh, stable

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 0xffffa95fa000
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 0xffff7e9e2000
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 20800000.etr: TMC-ETR enabled
[   77.537097] coresight-replicator etr_replicator@20890000: REPLICATOR enabled
[   77.558828] coresight-replicator main_replicator@208a0000: REPLICATOR enabled
[   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
[   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
[   77.618422] coresight-stm 20860000.stm: STM tracing enabled
[  139.554252] coresight-stm 20860000.stm: STM tracing disabled
 # End of first tracing session
[  146.351135] coresight-tmc 20800000.etr: TMC read start
[  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
[  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org # 4.7+
Reported-by: Robert Walker <robert.walker@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH] coresight: STM: Balance enable/disable
@ 2017-01-10 11:21 ` Suzuki K Poulose
  0 siblings, 0 replies; 28+ messages in thread
From: Suzuki K Poulose @ 2017-01-10 11:21 UTC (permalink / raw)
  To: linux-arm-kernel

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 0xffffa95fa000
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 0xffff7e9e2000
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 20800000.etr: TMC-ETR enabled
[   77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
[   77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
[   77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
[   77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
[   77.618422] coresight-stm 20860000.stm: STM tracing enabled
[  139.554252] coresight-stm 20860000.stm: STM tracing disabled
 # End of first tracing session
[  146.351135] coresight-tmc 20800000.etr: TMC read start
[  146.514486] coresight-tmc 20800000.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 20800000.etr: TMC read start
[  152.542632] coresight-tmc 20800000.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 <pratikp@codeaurora.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable at vger.kernel.org # 4.7+
Reported-by: Robert Walker <robert.walker@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-01-19 14:17 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 18:00 [PATCH] coresight: STM: Balance enable/disable Suzuki K Poulose
2017-01-16 18:00 ` Suzuki K Poulose
2017-01-19 11:40 ` Greg KH
2017-01-19 11:40   ` Greg KH
2017-01-19 13:55   ` Suzuki K Poulose
2017-01-19 13:55     ` Suzuki K Poulose
  -- strict thread matches above, loose matches on Subject: below --
2017-01-10 11:21 Suzuki K Poulose
2017-01-10 11:21 ` Suzuki K Poulose
2017-01-10 17:36 ` Mathieu Poirier
2017-01-10 17:36   ` Mathieu Poirier
2017-01-11 11:41   ` Chunyan Zhang
2017-01-11 11:41     ` Chunyan Zhang
2017-01-11 11:41     ` Chunyan Zhang
2017-01-11 13:59     ` Suzuki K Poulose
2017-01-11 13:59       ` Suzuki K Poulose
2017-01-11 13:59       ` Suzuki K Poulose
2017-01-13  2:10       ` Chunyan Zhang
2017-01-13  2:10         ` Chunyan Zhang
2017-01-13  2:10         ` Chunyan Zhang
2017-01-13 16:48 ` Mathieu Poirier
2017-01-13 16:48   ` Mathieu Poirier
2017-01-13 16:48   ` Mathieu Poirier
2017-01-13 17:11   ` Suzuki K Poulose
2017-01-13 17:11     ` Suzuki K Poulose
2017-01-13 17:11     ` Suzuki K Poulose
2017-01-16 17:50     ` Mathieu Poirier
2017-01-16 17:50       ` Mathieu Poirier
2017-01-16 17:50       ` Mathieu Poirier

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.