devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Ilia Lin <ilialin@codeaurora.org>
Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, sboyd@codeaurora.org,
	devicetree@vger.kernel.org, will.deacon@arm.com,
	rnayak@codeaurora.org, qualcomm-lt@lists.linaro.org,
	celster@codeaurora.org, tfinkel@codeaurora.org
Subject: Re: [PATCH 01/10] soc: qcom: Separate kryo l2 accessors from PMU driver
Date: Tue, 12 Dec 2017 14:03:03 +0000	[thread overview]
Message-ID: <20171212140302.au4wart4uhwm7lfq@lakrids.cambridge.arm.com> (raw)
In-Reply-To: <1513081897-31612-2-git-send-email-ilialin@codeaurora.org>

Hi,

On Tue, Dec 12, 2017 at 02:31:28PM +0200, Ilia Lin wrote:
> The driver provides kernel level API for other drivers
> to access the MSM8996 L2 cache registers.
> Separating the L2 access code from the PMU driver and
> making it public to allow other drivers use it.
> The accesses must be separated with a single spinlock,
> maintained in this driver.

> -static void set_l2_indirect_reg(u64 reg, u64 val)
> -{
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&l2_access_lock, flags);
> -	write_sysreg_s(reg, L2CPUSRSELR_EL1);
> -	isb();
> -	write_sysreg_s(val, L2CPUSRDR_EL1);
> -	isb();
> -	raw_spin_unlock_irqrestore(&l2_access_lock, flags);
> -}

> +/**
> + * set_l2_indirect_reg: write value to an L2 register
> + * @reg: Address of L2 register.
> + * @value: Value to be written to register.
> + *
> + * Use architecturally required barriers for ordering between system register
> + * accesses, and system registers with respect to device memory
> + */
> +void set_l2_indirect_reg(u64 reg, u64 val)
> +{
> +	unsigned long flags;
> +	mb();

We didn't need this for the PMU driver, so it's unfortuante that it now
has to pay the cost.

Can we please factor this mb() into the callers that need it?

> +	raw_spin_lock_irqsave(&l2_access_lock, flags);
> +	write_sysreg_s(reg, L2CPUSRSELR_EL1);
> +	isb();
> +	write_sysreg_s(val, L2CPUSRDR_EL1);
> +	isb();
> +	raw_spin_unlock_irqrestore(&l2_access_lock, flags);
> +}
> +EXPORT_SYMBOL(set_l2_indirect_reg);

[...]

> +#ifdef CONFIG_ARCH_QCOM
> +void set_l2_indirect_reg(u64 reg_addr, u64 val);
> +u64 get_l2_indirect_reg(u64 reg_addr);
> +#else
> +static inline void set_l2_indirect_reg(u32 reg_addr, u32 val) {}
> +static inline u32 get_l2_indirect_reg(u32 reg_addr)
> +{
> +	return 0;
> +}
> +#endif
> +#endif

Are there any drivers that will bne built for !CONFIG_ARCH_QCOM that
reference this?

It might be better to not have the stub versions, so that we get a
build-error if they are erroneously used.

Thannks,
Mark.

  reply	other threads:[~2017-12-12 14:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-12 12:31 [PATCH 00/10] clk: qcom: CPU clock driver for msm8996 Ilia Lin
2017-12-12 12:31 ` [PATCH 01/10] soc: qcom: Separate kryo l2 accessors from PMU driver Ilia Lin
2017-12-12 14:03   ` Mark Rutland [this message]
2017-12-22  2:06     ` Stephen Boyd
2018-01-04 11:15       ` ilialin
2018-01-04 11:13     ` ilialin
2017-12-12 12:31 ` [PATCH 02/10] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update Ilia Lin
     [not found]   ` <1513081897-31612-3-git-send-email-ilialin-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-12-12 15:05     ` Julien Thierry
2018-01-04 11:14       ` ilialin
2017-12-12 12:31 ` [PATCH 03/10] clk: qcom: Make clk_alpha_pll_configure available to modules Ilia Lin
2017-12-12 12:31 ` [PATCH 04/10] clk: qcom: Add CPU clock driver for msm8996 Ilia Lin
2017-12-15 22:35   ` Rob Herring
2018-01-04 11:15     ` ilialin
     [not found] ` <1513081897-31612-1-git-send-email-ilialin-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-12-12 12:31   ` [PATCH 05/10] clk: qcom: cpu-8996: Add support to switch to alternate PLL Ilia Lin
2017-12-12 12:31   ` [PATCH 06/10] clk: qcom: cpu-8996: Add support to switch below 600Mhz Ilia Lin
2017-12-12 12:31   ` [PATCH 08/10] clk: qcom: Add ACD path to CPU clock driver for msm8996 Ilia Lin
2017-12-12 12:31 ` [PATCH 07/10] clk: qcom: clk-cpu-8996: Prepare PLLs on probe Ilia Lin
2017-12-12 12:31 ` [PATCH 09/10] DT: QCOM: Add cpufreq-dt to msm8996 Ilia Lin
2017-12-12 12:31 ` [PATCH 10/10] DT: QCOM: Add thermal mitigation " Ilia Lin

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=20171212140302.au4wart4uhwm7lfq@lakrids.cambridge.arm.com \
    --to=mark.rutland@arm.com \
    --cc=celster@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=ilialin@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=qualcomm-lt@lists.linaro.org \
    --cc=rnayak@codeaurora.org \
    --cc=sboyd@codeaurora.org \
    --cc=tfinkel@codeaurora.org \
    --cc=will.deacon@arm.com \
    /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).