Hi Leo,

On Mon, 2 Sep 2019 at 14:26, Leo Yan <leo.yan@linaro.org> wrote:
>
> On Mon, Aug 19, 2019 at 10:14:00PM +0100, Mike Leach wrote:
> > Add section in coresight.txt explaining operation and use of CTI devices
> > in CoreSight.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> >  Documentation/trace/coresight-ect.txt | 164 ++++++++++++++++++++++++++
> >  Documentation/trace/coresight.txt     |   7 ++
> >  MAINTAINERS                           |   1 +
> >  3 files changed, 172 insertions(+)
> >  create mode 100644 Documentation/trace/coresight-ect.txt
> >
> > diff --git a/Documentation/trace/coresight-ect.txt b/Documentation/trace/coresight-ect.txt
> > new file mode 100644
> > index 000000000000..298123963dd2
> > --- /dev/null
> > +++ b/Documentation/trace/coresight-ect.txt
> > @@ -0,0 +1,164 @@
> > +CoreSight Embedded Cross Trigger (CTI & CTM).
> > +---------------------------------------------
> > +
> > +The CoreSight Cross Trigger Interface (CTI) is a hardware device that takes
> > +individual input and output hardware signals known as triggers to and from
> > +devices and interconnects them via the Cross Trigger Matrix (CTM) to other
> > +devices via numbered channels, in order to propagate events between devices.
> > +
> > +e.g.
> > +
> > + 0000000  in_trigs  :::::::
> > + 0 C   0----------->:     :             +======>(other CTI channel IO)
> > + 0  P  0<-----------:     :             v
> > + 0   U 0  out_trigs :     : Channels  *****      :::::::
> > + 0000000            : CTI :<=========>*CTM*<====>: CTI :---+
> > + #######  in_trigs  :     : (id 0-3)  *****      :::::::   v
> > + # ETM #----------->:     :                         ^   #######
> > + #     #<-----------:     :                         +---# ETR #
> > + ####### out_trigs  :::::::                             #######
> > +
> > +The CTI driver enables the programming of the CTI to attach triggers to
> > +channels. When an input trigger becomes active, the attached channel will
> > +become active. Any output trigger attached to that channel will also
> > +become active. The active channel is propagated to other CTIs via the CTM,
> > +activiating connected output triggers there, unless filtered by the CTI
> > +channel gate.
> > +
> > +It is also possible to activate a channel using system software directly
> > +programming registers in the CTI.
> > +
> > +The CTIs are registered by the system to be associated with CPUs and/or other
> > +CoreSight devices on the trace data path. When these devices are enabled the
> > +attached CTIs will also be enabled. By default/on power up the CTIs have
> > +no programmed trigger/channel attachments, so will not affect the system
> > +until explicitly programmed.
> > +
> > +The hardware trigger connections between CTIs and devices is implementation
> > +defined, unless the CPU/ETM combination is a v8 architecture, in which case
> > +the connections have an architecturally defined standard layout.
> > +
> > +The hardware trigger signals can also be connected to non-CoreSight devices
> > +(e.g. UART), or be propagated off chip as hardware IO lines.
> > +
> > +All the CTI devices are associated with a CTM. On many systems there will be a
> > +single effective CTM (one CTM, or multiple CTMs all interconnected), but it is
> > +possible that systems can have nets of CTIs+CTM that are not interconnected by
> > +a CTM to each other. On these systems a CTM index is declared to associate
> > +CTI devices that are interconnected via a given CTM.
> > +
> > +The CTI devices appear on the existing CoreSight bus alongside the other
> > +CoreSight devices.
> > +
> > +>$ ls /sys/bus/coresight/devices
> > +cti_cpu0  cti_cpu2  cti_sys0  etm0  etm2  funnel0  replicator0  tmc_etr0
> > +cti_cpu1  cti_cpu3  cti_sys1  etm1  etm3  funnel1  tmc_etf0     tpiu0
> > +
> > +The cti_cpu<N> named CTIs are associated with a CPU, and any ETM used by that
> > +core. the cti_sys<N> CTIs are general system infrastructure CTIs that can be
> > +associated with other CoreSight devices, or other system hardware capable of
> > +generating or using trigger signals.
> > +
> > +>$ ls /sys/bus/coresight/devices/etm0/cti_cpu0
> > +channels  connections  ctmid  enable  mgmt  power  regs  subsystem  triggers0
> > +triggers1  uevent
> > +
> > +Key file items are
> > +     * enable: enables/disables the CTI.
> > +     * ctmid : associated CTM - only relevant if system has multiple CTI+CTM
> > +     clusters that are not interconnected.
> > +
> > +Sub-directories:-
> > +     *triggers<N>: contains list of triggers for an individual connection
> > +     *connections: contains general connection information for the CTI plus
> > +     sysfs links to other registered CoreSight components that connect to
> > +     this CTI.
> > +     *channels: Contains the channel API - CTI main programming interface.
> > +     *regs: Gives access to the raw programmable CTI regs.
> > +     *mgmt: the standard CoreSight management registers.
> > +
> > +** Connections: This contains a list of general data describing the devices
> > +connected to this CTI, and links to other CoreSight devices.
> > +
> > +>$ ls -l ./cti_cpu0/connections/
> > +nr_cons  trigout_filtered etm0 -> ../../../85c000.etm/etm0
> > +
> > +      * nr_cons - total number of connections - triggers<N> directories.
> > +      * trigout_filtered - trigger out signals that are prevented from being
> > +      set if filtering is enabled. Prevents accidental edbgreq signals
> > +      stopping a core.
> > +      * etm0 - link showing connected CoreSight device - this will be named
> > +      according to the connected device. Multiple links will appear when a
> > +      CTI is connected to multiple CoreSight devices
>
> Seems to me, the nodes 'inen', 'inout_sel' and 'outen' under
> /sys/bus/coresight/devices/cti_X/regs/ should be moved to 'connections'
> folder.  Since we can use these nodes to enable the connections
> between channel and triggers.
>

These are deliberately in the /regs sub-dir as they directly r/w registers rather than provide a higher level API.
/connections is used to describe the fixed hardware links between components, not the transient links programmed up using the /regs or /channels API.

> > +
> > +** triggers<N>: Individual trigger connections.
> > +
> > +Each triggers directory has a set of parameters describing the triggers for
> > +the connection.
> > +     * name : name of connection
> > +     * in_signals : input trigger signal indexes used in this connection.
> > +     * in_types : functional types for in signals.
> > +     * out_signals : output trigger signals for this connection.
> > +     * out_types : functional types for out signals.
> > +
> > +e.g.
> > +>$ ls ./cti_cpu0/triggers0/
> > +in_signals  in_types  name  out_signals  out_types
> > +>$ cat ./cti_cpu0/triggers0/name
> > +cpu0
> > +>$ cat ./cti_cpu0/triggers0/out_signals
> > +0-2
> > +>$ cat ./cti_cpu0/triggers0/out_types
> > +pe_edbgreq pe_dbgrestart pe_ctiirq
> > +>$ cat ./cti_cpu0/triggers0/in_signals
> > +0-1
> > +>$ cat ./cti_cpu0/triggers0/in_types
> > +pe_dbgtrigger pe_pmuirq
> > +
> > +If a connection has zero signals in either the in or out triggers then those
> > +parameters will be omitted.
> > +
> > +** Channels API : This provides an easy way to attach triggers to channels,
> > +without needing the multiple register operations that are required if
> > +manipluating the 'regs' sub-dir elements directly.
> > +
> > +>$ ls ./cti_sys0/channels/
> > +chan_clear  gate_disable  list_gate_enable  show_chan_sel       trigin_attach
> > +chan_pulse  gate_enable   list_inuse        show_chan_xtrigs    trigin_detach
> > +chan_set    list_free     reset_xtrigs      trig_filter_enable  trigout_attach
> > +trigout_detach
>
> Several comments for channels.
>
> The 'chan_clear'/'chan_pulse'/'chan_set' nodes are easily to introduce
> confusion when I use them.  Other nodes will directly use the input
> values, but these three nodes will take the input value as bit offset
> (e.g. if echo '0' > chan_set, actually it will set value (1 << 0)).
> So maybe we can use more directive naming to reflect this?  E.g.
> chan_idx_{set|clear|pulse}.
>

e,g, echo 0 1 > trigin_attach
attaches signal input idx 1 to channel 0.

These will always translate the indexes to appropriate register writes - selecting the register and shifting bits as required.
This means that the user does not need to know the bit positions for channels / indexes.

> 'list_free', 'list_gate_enable' and 'list_inuse' are not very clear for
> the meaning of "list".  IMHO, "chans" might be more friendly for users
> than "list".  So finally we can get 'chans_free', 'chans_gate_enable',
> 'chans_inuse'.
>

This seems reasonable. I think I might make chan_gate_enable rw - read will list enabled channels, and drop the list_ variant completely.

> Change 'show_chan_sel' to 'chan_sel'?  This node is RW but not read-only
> property.  The naming 'show_chan_sel' gives impression like a RO node.
>



> > +
> > +Most access to these elements take the form:
> > +echo <chan> [<trigger>] > /<device_path>/<operation>
> > +where the optional <trigger> is only needed for trigXX_at|detach operations.
> > +
> > +e.g.
> > +>$ echo 0 1 > ./cti_sys0/channels/trigin_attach
> > +attach trig_in(1) to channel(0).
> > +echo 0 > ./cti_sys0/channels/chan_set
> > +activate channel(0)
> > +
> > +      *gate_enable, gate_disable operations set the CTI gate to propagate
> > +      (enable) to other devices. These operation can take a channel number
> > +      or 'all' to operate on all channels. CTI gate is enabled by default
> > +      at power up.
> > +      *list_gate_enable: show the current channels enabled through the gate.
> > +      *list_inuse: show the current channels attached to any signal
> > +      *list_free: show channels with no attached signals.
> > +      *show_chan_sel: select the channel for the show_chan_xtrigs operation.
> > +      *show_chan_xtrigs: show the cross triggers programmed for the selected
> > +      channel.
> > +
> > +.../cti_sys0/channels# echo 0 1 > trigin_attach
> > +.../cti_sys0/channels# cat list_free
> > +1 2 3
> > +.../cti_sys0/channels# cat list_inuse
> > +0
> > +.../cti_sys0/channels# echo 0 > show_chan_sel
> > +.../cti_sys0/channels# cat show_chan_xtrigs
> > +IN: 1  OUT:
> > +
> > +     *trig_filter_enable: defaults to enabled, disable to allow potentially
> > +     dangerous output signals to be set.
> > +     *reset_xtrigs: clear all channel / trigger programming - reset device
> > +     hardware to default state.
>
> Suggest we can give a more complete example, e.g. we can use below
> steps to create connection between channel and triggers, and finally
> we can readout status register for triggering (below is an example I
> tested to send event from channel 0 to trigger 4, and finally we can
> read out the triggerout status to show the signal is asserted):
>

I’ll add a larger example.

Thanks and Regards

Mike

> .../cti_cpu0# echo 1 > enable
> .../cti_cpu0/channels# echo 0 4 > trigout_attach
> .../cti_sys0/channels# echo 0 > show_chan_sel
> .../cti_cpu0/channels# cat show_chan_xtrigs
> IN: OUT: 4
> .../cti_cpu0/channels# cat list_inuse
> 0
> .../cti_cpu0/regs# echo 4 > inout_sel
> .../cti_cpu0/regs# echo 1 > outen
> .../cti_cpu0/channels# echo 0 > chan_set
> .../cti_cpu0/regs# cat trigoutstatus
> 0x10
>
> Thanks,
> Leo Yan
>
> > diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
> > index b027d61b27a6..f37d927a368c 100644
> > --- a/Documentation/trace/coresight.txt
> > +++ b/Documentation/trace/coresight.txt
> > @@ -477,6 +477,13 @@ root@genericarmv8:~#
> >
> >  Details on how to use the generic STM API can be found here [2].
> >
> > +The CTI Module
> > +--------------
> > +
> > +A separate documentation file is provided to explain the use of CTI devices.
> > +(Documentation/trace/coresight-ect.txt)
> > +
> > +
> >  [1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
> >  [2]. Documentation/trace/stm.rst
> >  [3]. https://github.com/Linaro/perf-opencsd
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a81f3bd8340a..45cfe624cb8c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1585,6 +1585,7 @@ F:      drivers/hwtracing/coresight/*
> >  F:   include/dt-bindings/arm/coresight-cti-dt.h
> >  F:   Documentation/trace/coresight.txt
> >  F:   Documentation/trace/coresight-cpu-debug.txt
> > +F:   Documentation/trace/coresight-ect.txt
> >  F:   Documentation/devicetree/bindings/arm/coresight.txt
> >  F:   Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
> >  F:   Documentation/devicetree/bindings/arm/coresight-ect-cti.txt
> > --
> > 2.17.1
> >
> > _______________________________________________
> > CoreSight mailing list
> > CoreSight@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/coresight



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