On Fri, Jun 01, 2018 at 02:15:59PM +0100, Suzuki K Poulose wrote:
> Coresight uses DT graph bindings to describe the connections of the
> components. However we have some undocumented usage of the bindings
> to describe some of the properties of the connections.
> 
> The coresight driver needs to know the hardware ports invovled
> in the connection and the direction of data flow to effectively
> manage the trace sessions. So far we have relied on the "port"
> address (as described by the generic graph bindings) to represent
> the hardware port of the component for a connection.
> 
> The hardware uses separate numbering scheme for input and output
> ports, which implies, we could have two different (input and output)
> ports with the same port number. This could create problems in the
> graph bindings where the label of the port wouldn't match the address.
> 
> e.g, with the existing bindings we get :
> 
>       port@0{                         // Output port 0
>               reg = <0>;
>               ...
>       };
> 
>       port@1{
>               reg = <0>;              // Input port 0
>               endpoint {
>                       slave-mode;
>                       ...
>               };
>       };
> 
> With the new enforcement in the DT rules, mismatches in label and address
> are not allowed (as see in the case for port@1). So, we need a new mechanism
> to describe the hardware port number reliably.
> 
> Also, we relied on an undocumented "slave-mode" property (see the above
> example) to indicate if the port is an input port. Let us formalise and
> switch to a new property to describe the direction of data flow.
> 
> There were three options considered for the hardware port number scheme:
> 
>  1) Use natural ordering in the DT to infer the hardware port number.
>   i.e, Mandate that the all ports are listed in the DT and in the ascending
>   order for each class (input and output respectively).
>    Pros :
>       - We don't need new properties and if the existing DTS list them in
>         order (which most of them do), they work out of the box.
>    Cons :
>       - We must list all the ports even if the system cannot/shouldn't use
>         it.
>       - It is prone to human errors (if the order is not kept).
> 
>  2) Use an explicit property to list both the direction and the hw port
>     number and direction. Define "coresight,hwid" as 2 member array of u32,
>     where the members are port number and the direction respectively.
>       e.g
> 
>       port@0{
>               reg = <0>;
>               endpoint {
>                       coresight,hwid = <0 1>; // Port # 0, Output
>               }
>       };
> 
>       port@1{
>               reg = <1>;
>               endpoint {
>                       coresight,hwid = <0 0>; // Port # 0, Input
>               };
>       };
> 
>       Pros:
>         - The bindings are formal but not so reader friendly and could 
> potentially
>           lead to human errors.
>       Cons:
>         - Backward compatiblity is lost.
>  3) Use explicit properties (implemented in the series) for the hardware
>     port id and direction. We define a new property "coresight,hwid" for
>     each endpoint in coresight devices to specify the hardware port number
>     explicitly. Also use a separate property "direction" to specify the
>     direction of the data flow.
> 
>       e.g,
> 
>       port@0{
>               reg = <0>;
>               endpoint {
>                       direction = <1>;        // Output
>                       coresight,hwid = <0>;   // Port # 0
>               }
>       };
> 
>       port@1{
>               reg = <1>;
>               endpoint {
>                       direction = <0>;        // Input
>                       coresight,hwid = <0>;   // Port # 0
>               };
>       };
> 
>     Pros:
>        - The bindings are formal and reader friendly, and less prone to 
> errors.
>     Cons:
>        - Backward compatibility is lost.
> 
> 
> This series achieves implements Option (3) listed above while still retaining
> the backward compatibility. The driver now issues a warning (once) when it
> encounters the old bindings.
> It also cleans up the platform parsing code to reduce the memory usage by
> reusing the platform description. The series also includes the
> changes for Juno platform as an example. If there are no objections
> to the approach, I could post the series, converting all the
> in-kernel DTS to the new binding.
> 
> Suzuki K Poulose (8):
>   dts: binding: coresight: Document graph bindings
>   coresight: Fix remote endpoint parsing
>   coresight: Cleanup platform description data
>   coresight: platform: Cleanup coresight connection handling
>   coresight: Handle errors in finding input/output ports
>   dts: coresight: Clean up the device tree graph bindings
>   dts: coresight: Define new bindings for direction of data flow
>   dts: juno: Update coresight bindings for hw port
> 
>  .../devicetree/bindings/arm/coresight.txt          |  52 ++++++++--
>  arch/arm64/boot/dts/arm/juno-base.dtsi             |  82 +++++++++++----
>  arch/arm64/boot/dts/arm/juno.dts                   |   5 +-
>  drivers/hwtracing/coresight/coresight.c            |  28 ++----
>  drivers/hwtracing/coresight/of_coresight.c         | 111 
> ++++++++++++---------
>  include/linux/coresight.h                          |  11 +-
>  6 files changed, 181 insertions(+), 108 deletions(-)

Aside from the comments I've already posted I'm pretty much good with this set.
Please rebase the next revision on my "next" branch and run checkpatch.pl on the
set.  Patch 6/8 and 7/8 are generating warnings.

Thanks,
Mathieu

> 
> -- 
> 2.7.4
> 

Reply via email to