Re: [PATCH] coresight: dynamic-replicator: Fix handling of multiple connections

From: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] coresight: dynamic-replicator: Fix handling of multiple connections
Date: Wed, 29 Apr 2020 22:41:06 +0530	[thread overview]
Message-ID: <ae0fe2050be01cc1403c7d53a0da8cb8@codeaurora.org> (raw)
In-Reply-To: <CAJ9a7Vj4eyv1n=RxuqfV=pdBN3SDG+ShYS5J4s40KJtqOnR7vw@mail.gmail.com>

Hi Mike,

On 2020-04-29 22:28, Mike Leach wrote:
> Hi,
> 

[...]

>> >> > You need to find what is resetting the IDFILTERs to 0 for replicator1.
>> >> >
>> >>
>> >> That is right.
>> >>
>> >
>> > By default all replicators have the IDFILTER registers set to 0 out of
>> > hardware reset. This ensures that programmable replicators behave in
>> > the same way as non-programmable replicators out of reset.
>> >
>> > The  dynamic_replicator_reset() is of course a driver state reset -
>> > which filters out all trace on the output ports. The trace is then
>> > enabled when we set the trace path from source to sink.
>> >
>> 
>> Thanks for these explanations.
>> 
>> > It seems to me that you have 2 problems that need solving here:
>> >
>> > 1) Why does the reset_replicator() called from probe() _not_ work
>> > correctly on replicator 1? It seems to work later if you introduce a
>> > reset after more of the system has powered and booted. This is
>> > startiing to look a little like a PM / clocking issue.
>> 
>> reset_replicator() does work in probe correctly for both replicators,
>> below logs is collected before and after reset in probe. It is later
>> that it's set back to 0x0 and hence the suggestion to look at firmware
>> using this replicator1.
>> 
> OK - sorry I read your statement saying that replicator1 was 0 after
> the reset in probe(), rather than look at the logs.
> 
> From the logs it is working at the time probe() occurs, but by the
> time we come to enable the replicator later, something has reset these
> registers / hardware outside the control of the replicator driver.
> 

Yes, I will try to get some more information from the firmware side if 
there is anything messing up.

> 
>> [    8.477669] func replicator_probe before reset replicator 
>> replicator1
>> REPLICATOR_IDFILTER0=0x0 REPLICATOR_IDFILTER1=0x0
>> [    8.489470] func replicator_probe after reset replicator 
>> replicator1
>> REPLICATOR_IDFILTER0=0xff REPLICATOR_IDFILTER1=0xff
>> 
>> >
>> > This failure is causing the state when we are trying to set an output
>> > port that both branches of this replicator are enabled for output.
>> > In effect for this replicator, setting the output port has no effect
>> > as it is already enabled.
>> >
>> > 2) Why does having both ports of this repilicator enabled cause a hard
>> > lockup? This is a separate hardware  / system issue.
>> >
>> > The worst that should happen if both branches of a replicator are
>> > enabled is that you get undesirable back pressure. (e.g. there is a
>> > system we have seen - I think it is Juno - where there is a static
>> > replicator feeding the TPIU and ETR - we need to disable the TPIU to
>> > prevent undesired back pressure).
>> >
>> 
>> Ok so hardlockup is not expected because of this backpressure.
>> 
> 
> Hardlockup is not expected, but this is not related to any possible
> backpressure.
> 
> Ordinarily having both legs of a replicator enabled should not cause
> system failure.
> 

Ok got it, thanks.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation