From: Mark Rutland <mark.rutland@arm.com>
To: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: morten.rasmussen@arm.com, souvik.chakravarty@arm.com,
catalin.marinas@arm.com, sudeep.holla@arm.com,
linux-kernel@vger.kernel.org, dietmar.eggemann@arm.com,
viresh.kumar@linaro.org, will@kernel.org,
valentin.schneider@arm.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 1/3] arm64: wrap and generalise counter read functions
Date: Thu, 5 Nov 2020 13:27:05 +0000 [thread overview]
Message-ID: <20201105132705.GF82102@C02TD0UTHF1T.local> (raw)
In-Reply-To: <20201105122702.13916-2-ionela.voinescu@arm.com>
Hi Ionela,
On Thu, Nov 05, 2020 at 12:27:00PM +0000, Ionela Voinescu wrote:
> -#ifdef CONFIG_ARM64_AMU_EXTN
> +#define COUNTER_READ_STORE(NAME, VAL) \
> +static inline u64 read_##NAME(void) \
> +{ \
> + return VAL; \
> +} \
> +static inline void store_##NAME(void *val) \
> +{ \
> + *(u64 *)val = read_##NAME(); \
> +}
>
> -#undef pr_fmt
> -#define pr_fmt(fmt) "AMU: " fmt
> +#ifdef CONFIG_ARM64_AMU_EXTN
> +COUNTER_READ_STORE(corecnt, read_sysreg_s(SYS_AMEVCNTR0_CORE_EL0));
> +COUNTER_READ_STORE(constcnt, read_sysreg_s(SYS_AMEVCNTR0_CONST_EL0));
> +#else
> +COUNTER_READ_STORE(corecnt, 0);
> +COUNTER_READ_STORE(constcnt, 0);
> +#endif
I think this level of indirection is confusing (e.g. since you can't
grep for `read_constcnt` to find the implementation). I also don't think
we need to store_* functions here -- they're only needed by patch 3, and
they can be wrapped in helper functions there (which should also solve
the warning you mention in the cover letter).
I think if we need these wrappers, it'd be better to have:
#ifdef CONFIG_ARM64_AMU_EXTN
#define read_corecnt() read_sysreg_s(SYS_AMEVCNTR0_CORE_EL0)
#define read_constcnt() read_sysreg_s(SYS_AMEVCNTR0_CONST_EL0)
#else
#define read_corecnt() (0UL)
#define read_constcnt() (0UL)
#endif
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-05 13:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-05 12:26 [PATCH v3 0/3] arm64: cppc: add FFH support using AMUs Ionela Voinescu
2020-11-05 12:27 ` [PATCH v3 1/3] arm64: wrap and generalise counter read functions Ionela Voinescu
2020-11-05 13:27 ` Mark Rutland [this message]
2020-11-05 12:27 ` [PATCH v3 2/3] arm64: split counter validation function Ionela Voinescu
2020-11-05 12:27 ` [PATCH v3 3/3] arm64: implement CPPC FFH support using AMUs Ionela Voinescu
2020-11-05 13:28 ` Mark Rutland
2020-11-05 14:10 ` Ionela Voinescu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201105132705.GF82102@C02TD0UTHF1T.local \
--to=mark.rutland@arm.com \
--cc=catalin.marinas@arm.com \
--cc=dietmar.eggemann@arm.com \
--cc=ionela.voinescu@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=morten.rasmussen@arm.com \
--cc=souvik.chakravarty@arm.com \
--cc=sudeep.holla@arm.com \
--cc=valentin.schneider@arm.com \
--cc=viresh.kumar@linaro.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).