Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1559960 > unrolled thread

[PATCH] coresight: STM: Balance enable/disable

Back to article view | Back to linux.kernel

Contents

  [PATCH] coresight: STM: Balance enable/disable Suzuki K Poulose <suzuki.poulose@arm.com> - 2017-01-16 19:10 +0100
    Re: [PATCH] coresight: STM: Balance enable/disable Greg KH <gregkh@linuxfoundation.org> - 2017-01-19 12:50 +0100
      Re: [PATCH] coresight: STM: Balance enable/disable Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2017-01-19 15:20 +0100

#1559960 — [PATCH] coresight: STM: Balance enable/disable

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

[toc] | [next] | [standalone]

#1562647

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

[toc] | [prev] | [next] | [standalone]

#1562761

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web