[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANLsYkxnJKB=uFwQDW4Ec3nMfOtnh3f1QQGKf=adH1jUxfsrng@mail.gmail.com>
Date: Fri, 30 May 2014 12:02:40 -0600
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: Rob Herring <robherring2@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Will Deacon <will.deacon@....com>,
Daniel Thompson <daniel.thompson@...aro.org>,
Robert Marklund <robbelibobban@...il.com>,
Al Grant <Al.Grant@....com>,
Linaro Patches <patches@...aro.org>,
Marcin Jabrzyk <marcin.jabrzyk@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Arnd Bergmann <arnd@...aro.org>,
Panchaxari Prasannamurthy Tumkur
<panchaxari.prasannamurthy@...aro.org>, r.sengupta@...sung.com,
Arve Hjønnevåg <arve@...roid.com>,
John Stultz <john.stultz@...aro.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
James King <james.king@...aro.org>,
Russell King - ARM Linux <linux@....linux.org.uk>,
Pratik Patel <pratikp@...eaurora.org>,
Vikas Varshney <varshney@...com>,
Jonas Svennebring <jonas.svennebring@...gotech.com>
Subject: Re: [RFC PATCH 01/11] coresight: add CoreSight core layer framework
On 30 May 2014 11:25, Rob Herring <robherring2@...il.com> wrote:
> On Fri, May 30, 2014 at 8:43 AM, <mathieu.poirier@...aro.org> wrote:
>> From: Pratik Patel <pratikp@...eaurora.org>
>>
>> CoreSight components are compliant with the ARM CoreSight
>> architecture specification and can be connected in various
>> topologies to suite a particular SoCs tracing needs. These trace
>> components can generally be classified as sources, links and
>> sinks. Trace data produced by one or more sources flows through
>> the intermediate links connecting the source to the currently
>> selected sink.
>>
>> CoreSight framework provides an interface for the CoreSight trace
>> drivers to register themselves with. It's intended to build up a
>> topological view of the CoreSight components and configure the
>> right series of components on user input via debugfs.
>>
>> For eg., when enabling a source, framework builds up a path
>> consisting of all the components connecting the source to the
>> currently selected sink and enables all of them.
>>
>> Framework also supports switching between available sinks and
>> also provides status information to user space applications
>> through sysfs interface.
>>
>> Signed-off-by: Pratik Patel <pratikp@...eaurora.org>
>> Signed-off-by: Panchaxari Prasannamurthy <panchaxari.prasannamurthy@...aro.org>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@...aro.org>
>> ---
>> .../devicetree/bindings/arm/coresight.txt | 138 ++++
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 1 +
>> drivers/coresight/Kconfig | 9 +
>> drivers/coresight/Makefile | 5 +
>> drivers/coresight/coresight-priv.h | 46 ++
>> drivers/coresight/coresight.c | 739 +++++++++++++++++++++
>> drivers/coresight/of_coresight.c | 124 ++++
>> include/linux/coresight.h | 181 +++++
>> include/linux/of_coresight.h | 27 +
>> 10 files changed, 1272 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/coresight.txt
>> create mode 100644 drivers/coresight/Kconfig
>> create mode 100644 drivers/coresight/Makefile
>> create mode 100644 drivers/coresight/coresight-priv.h
>> create mode 100644 drivers/coresight/coresight.c
>> create mode 100644 drivers/coresight/of_coresight.c
>> create mode 100644 include/linux/coresight.h
>> create mode 100644 include/linux/of_coresight.h
>>
>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
>> new file mode 100644
>> index 0000000..3e21665
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> @@ -0,0 +1,138 @@
>> +* CoreSight Components
>> +
>> +CoreSight components are compliant with the ARM CoreSight architecture
>> +specification and can be connected in various topologies to suite a particular
>> +SoCs tracing needs. These trace components can generally be classified as sinks,
>> +links and sources. Trace data produced by one or more sources flows through the
>> +intermediate links connecting the source to the currently selected sink. Each
>> +CoreSight component device should use these properties to describe its hardware
>> +characteristcs.
>> +
>> +Required properties:
>> +
>> +- compatible : name of the component used for driver matching
>
> Where are the specific strings for components?
"Coresight" is made of multiple components with each component needing
a different driver.
>
>> +- reg : physical base address and length of the register set(s) of the component
>> +- coresight-id : unique integer identifier for the component
>
> compatible + reg is a unique id.
Some coresight IP blocks like the replicator don't have a reg property
- the "coresight-id" and "coresight-name" are the only way to identify
them properly.
>
>> +- coresight-name : unique descriptive name of the component
>
> compatible is a descriptive name.
That is what ends up under /sys/kernel/debug/coresight/. Since there
can be multiple entity of the same type it is easier and more
convivial for users. For instance: "coresight-etb-replicator" and
"coresight-tpiu-replicator" are easier to identify than
"coresight-replicator0" and "coresight-replicator1".
We need to keep "coresight-id" or "coresight-name" - I prefer the
former. What is your take on this?
>
>> +- coresight-nr-inports : number of input ports on the component
>> +
>> +coresight-outports, coresight-child-list and coresight-child-ports lists will
>> +be of the same length and will have a one to one correspondence among the
>> +elements at the same list index.
>> +
>> +coresight-default-sink must be specified for one of the sink devices that is
>> +intended to be made the default sink. Other sink devices must not have this
>> +specified. Not specifying this property on any of the sinks is invalid.
>
> Why do you need the default-sink in DT? Isn't the configuration going
> to depend on the user?
Users can change this at will via debugfs. It's a default boot time
configuration and can be removed if you're absolutely keen on it.
Please advise.
>
>> +
>> +Optional properties:
>> +
>> +- coresight-outports : list of output port numbers of this component
>> +- coresight-child-list : list of phandles pointing to the children of this
>> + component
>> +- coresight-child-ports : list of input port numbers of the children
>> +- coresight-default-sink : represents the default compile time CoreSight sink
>
> Seems like the OF graph helpers being defined for V4L2 could be useful
> here to describe the connections. We should not invent something new
> to describe arbitrary connections.
Very well, I'll take a look.
>
>> +- arm,buffer-size : size of contiguous buffer space for TMC ETR
>
> TMC ETR?
Trace Memory Controller in Embedded Trace Router configuration.
>
>> +- arm,cp14 : cp14 access to ETM registers is implemented and should be used
>> +- clocks : the clock associated to the coresight entity.
>> +- cpu: only valid for ETM/PTMs - the cpu this ETM/PTM is affined to.
>
> I think you need some separation of bindings by component rather than
> saying which components certain properties apply to.
Ok.
>
>> +
>> +Example:
>> +
>> +1. Bus declaration:
>> + coresight {
>> + compatible = "arm,coresight";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + ...
>> + ...
>> + ...
>> + };
>> +
>> + coresight {
>> + compatible = "arm,coresight";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x20010000 0 0x20010000 0x1000
>> + 0x20030000 0 0x20030000 0x1000
>> + 0x20040000 0 0x20040000 0x1000
>> + 0x2201c000 0 0x2201c000 0x1000
>> + 0x2201d000 0 0x2201d000 0x1000
>> + 0x2203c000 0 0x2203c000 0x1000
>> + 0x2203d000 0 0x2203d000 0x1000
>> + 0x2203e000 0 0x2203e000 0x1000>;
>
> This is not how ranges generally work. For no translation, use just
> "ranges;". Otherwise you should have base address and reg properties
> are just offsets from the base. Anyway, it's not really pertinent to
> your example.
In the case of that specific board doing a one-to-one mapping in
"ranges" was mandatory. Otherwise the mapping wasn't done at all.
>
>> + ...
>> + ...
>> + ...
>> + };
>> +
>> +2. Sinks
>> + etb: etb@...10000 {
>> + compatible = "arm,coresight-etb";
>> + reg = <0x20010000 0x1000>;
>> +
>> + coresight-id = <0>;
>> + coresight-name = "coresight-etb";
>> + coresight-nr-inports = <1>;
>> + coresight-default-sink;
>> + };
>> +
>> + tpiu: tpiu@...30000 {
>> + compatible = "arm,coresight-tpiu";
>> + reg = <0x20030000 0x1000>;
>> +
>> + coresight-id = <1>;
>> + coresight-name = "coresight-tpiu";
>> + coresight-nr-inports = <1>;
>> + };
>> +
>> +3. Links
>> + replicator: replicator {
>> + compatible = "arm,coresight-replicator";
>> +
>> + coresight-id = <2>;
>> + coresight-name = "coresight-replicator";
>> + coresight-nr-inports = <1>;
>> + coresight-outports = <0 1>;
>> + coresight-child-list = <&etb &tpiu>;
>> + coresight-child-ports = <0 0>;
>
> This could be one property instead of 2: <&etb 0>, <&tpiu 0>;
>
But then we'd have to do the parsing of said property in the code -
isn't that less desirable?
> Then add a #coresight-cells
>
>> + };
>> +
>> + funnel: funnel@...40000 {
>> + compatible = "arm,coresight-funnel";
>> + reg = <0x20040000 0x1000>;
>> +
>> + coresight-id = <3>;
>> + coresight-name = "coresight-funnel";
>> + coresight-nr-inports = <8>;
>> + coresight-outports = <0>;
>> + coresight-child-list = <&replicator>;
>> + coresight-child-ports = <0>;
>> + };
>> +
>> +4. Sources
>> + ptm0: ptm@...1c000 {
>> + compatible = "arm,coresight-etm";
>> + reg = <0x2201c000 0x1000>;
>> +
>> + coresight-id = <9>;
>> + coresight-name = "coresight-ptm0";
>> + cpu = <&cpu0>;
>> + coresight-nr-inports = <0>;
>
> A source can be implied by no inports property. Likewise for a sink.
Sources have a compatible property to identify them. A sink has a
single input port. Did I get your comment right?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists