On Wed, Oct 30, 2013 at 02:45:36PM +0000, Mischa Jonker wrote:
> This adds basic perf support for ARC700 cores. Most PERF_COUNT_HW* events
> are supported now.
>
> Signed-off-by: Mischa Jonker <mjonker@xxxxxxxxxxxx>
> Cc: Vineet Gupta <vgupta@xxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
> v2:
> updated comments to kernel coding standards
> renamed devicetree binding to snps,arc700-pmu
> added devicetree binding doc
>
> Documentation/devicetree/bindings/arc/pmu.txt | 15 ++
> arch/arc/boot/dts/angel4.dts | 4 +
> arch/arc/include/asm/perf_event.h | 204 +++++++++++++++-
> arch/arc/kernel/Makefile | 1 +
> arch/arc/kernel/perf_event.c | 322 +++++++++++++++++++++++++
> 5 files changed, 545 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/arc/pmu.txt
> create mode 100644 arch/arc/kernel/perf_event.c
As a general note, for our own selfish reasons the device tree maintainers
would prefer that binding document changes were in a separate patch from any
code (though the whole series can be Cc'd to devicetree@xxxxxxxxxxxxxxx, it's
oftern useful context). It makes it far easier to find the information relevant
to us, manage review and so on.
>
> diff --git a/Documentation/devicetree/bindings/arc/pmu.txt b/Documentation/devicetree/bindings/arc/pmu.txt
> new file mode 100644
> index 0000000..d252d3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arc/pmu.txt
> @@ -0,0 +1,15 @@
> +* ARC Performance Monitor Unit
> +
> +The ARC 700 can be configured with a pipeline performance monitor for counting
> +CPU and cache events like cache misses and hits.
> +
> +Required properties:
> +
> +- compatible : should be
> + "snps,arc700-pmu"
s/should be/should contain/ -- there might be a future variant that's
compatible with the software-visible interface.
Is that ARC 700 a family or a specific implementation? If it's a family it may
make sense to define the strings you expect for particular implementations,
while still expecting "snps,arc700-pmu" to be in the compatible string list.
Are there any interrupts? Most PMUs I've seen have an interrupt for
overflow/saturation.
> +
> +Example:
> +
> +pmu {
> + compatible = "snps,arc700-pmu";
> +};
Regardless, given it's so simple and can be extended easily in future, this
looks fine to me. With the minor change to the compatible description:
Acked-by Mark Rutland <mark.rutland@xxxxxxx>
Thanks,
Mark.
--
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
