Re: [PATCH v2] drivers: CCI: add ARM CCI PMU support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Stephen,

Thanks for the helpful comments.

On 16/08/13 19:31, Stephen Warren wrote:
On 08/16/2013 11:19 AM, Punit Agrawal wrote:
The CCI PMU can profile bus transactions at the master and slave
interfaces of the CCI. The PMU can be used to observe an aggregated view
of the bus traffic between the various components connected to the CCI.

Extend the existing CCI driver to support the PMU by registering a perf
backend for it.

I think this binding addresses my comments, thanks. Just one comment below:

diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt

+		- reg:
+			Usage: required
+			Value type: <prop-encoded-array>

+		- interrupts:
+			Usage: required
+			Value type: <prop-encoded-array>

That makes it sound like the layout/content of those two properties is
the same. That's not true; one is an array of (base, size) cells, and
the other is of (phandle, args*) cells. The difference between the data
being phandles-vs-integers seems important.

Perhaps says:

Value type: Integer cells. Array of register entries, each expressed as
a pair of cells, containing base and size.

Value type: Integer cells. Array of interrupt specifier entries, as
defined in ../interrupt-controller/interupts.txt.


This is indeed better. I've updated the documentation for "interrupts" but am not sure about changing the "reg" property. The description used here is similar to other "reg" property description in the same file used for other CCI sub-nodes. Do you think this is sufficiently important clarification to change the other instances as well?

+			Definition: comma-separated list of counter overflow

Oh, and lists of cells aren't necessarily comma-separated; comma is used
between <> but not inside <>, and there's no requirement that each
individual interrupt specifier be in its own <>, vs. just aggregating
all of them into a single <>.


I wasn't aware of this. I've now updated the text to remove "comma-separated". Thanks.

I'll post an updated version (including some of the improvements suggested here and in the other email by Kumar) soon. Please let me know if you'd prefer to change the description of all instances of "reg" in the file or keep this one as is.

Cheers,
Punit

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux