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

[Mike Leach]

Hi,

On Wed, 29 Apr 2020 at 15:48, Sai Prakash Ranjan
<saiprakash.ranjan@xxxxxxxxxxxxxx> wrote:
>
> Hi Mike,
>
> On 2020-04-29 19:57, Mike Leach wrote:
> > Hi,
> >
>
> [...]
>
> >> >> Looking more into replicator1(swao_replicator) values as 0x0 even
> >> >> after replicator_reset()
> >> >> in replicator probe, I added dynamic_replicator_reset in
> >> >> dynamic_replicator_enable()
> >> >> and am not seeing any hardlockup. Also I added some prints to check
> >> >> the idfilter
> >> >> values before and after reset and found that its not set to 0xff even
> >> >> after replicator_reset()
> >> >> in replicator probe, I don't see any other path setting it to 0x0.
> >> >>
> >> >> After probe:
> >> >>
> >> >> [    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
> >> >
> >> > AFAICS, after the reset both of them are set to 0xff.
> >>
> >> Yes I see this too as we call replicator_reset() in probe. What I
> >> wanted
> >> to highlight was the below part where it is set to 0x0 before enabling
> >> dynamic replicator.
> >>
> >> >
> >> >> [    8.502738] func replicator_probe before reset replicator
> >> >> replicator0 REPLICATOR_IDFILTER0=0x0 REPLICATOR_IDFILTER1=0x0
> >> >> [    8.515214] func replicator_probe after reset replicator
> >> >> replicator0 REPLICATOR_IDFILTER0=0xff REPLICATOR_IDFILTER1=0xff
> >> >
> >> >
> >> >
> >> >> localhost ~ #
> >> >> localhost ~ #
> >> >> localhost ~ # echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
> >> >> localhost ~ #
> >> >> localhost ~ # echo 1 > /sys/bus/coresight/devices/etm0/enable_source
> >> >> [   58.490485] func dynamic_replicator_enable before reset replicator
> >> >> replicator0 REPLICATOR_IDFILTER0=0xff REPLICATOR_IDFILTER1=0xff
> >> >> [   58.503246] func dynamic_replicator_enable after reset replicator
> >> >> replicator0 REPLICATOR_IDFILTER0=0xff REPLICATOR_IDFILTER1=0xff
> >> >> [   58.520902] func dynamic_replicator_enable before reset replicator
> >> >> replicator1 REPLICATOR_IDFILTER0=0x0 REPLICATOR_IDFILTER1=0x0
> >> >
> >> > 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.


> [    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.

Mike

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



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK