linux-kernel - Re: [PATCH v2] ARC: Add perf support for ARC700 cores

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20131030175853.GB2815@kartoffel>
Date:	Wed, 30 Oct 2013 10:58:53 -0700
From:	Mark Rutland <mark.rutland@....com>
To:	Mischa Jonker <Mischa.Jonker@...opsys.com>
Cc:	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH v2] ARC: Add perf support for ARC700 cores

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@...opsys.com>
> Cc: Vineet Gupta <vgupta@...opsys.com>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> ---
> 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@...r.kernel.org, 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@....com>

Thanks,
Mark.
--
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/