All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Wenyou Yang <wenyou.yang@atmel.com>
Cc: nicolas.ferre@atmel.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	alexandre.belloni@free-electrons.com,
	sylvain.rochet@finsecur.com, peda@axentia.se,
	Patrice.VILCHEZ@atmel.com
Subject: Re: [PATCH 4/7] ARM: at91: enable the L2 Cache controller
Date: Mon, 26 Jan 2015 12:45:31 +0000	[thread overview]
Message-ID: <20150126124531.GH26493@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1422266836-24592-1-git-send-email-wenyou.yang@atmel.com>

On Mon, Jan 26, 2015 at 06:07:16PM +0800, Wenyou Yang wrote:
> +#ifdef CONFIG_CACHE_L2X0
> +static void __init at91_init_l2cache(void)
> +{
> +	struct device_node *np;
> +	u32 reg;
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +	if (!np)
> +		return;
> +
> +	at91_l2cc_base = of_iomap(np, 0);
> +	if (!at91_l2cc_base)
> +		panic("unable to map l2cc cpu registers\n");
> +
> +	of_node_put(np);
> +
> +	/* Disable cache if it hasn't been done yet */
> +	if (readl_relaxed(at91_l2cc_base + L2X0_CTRL) & L2X0_CTRL_EN)
> +		writel_relaxed(~L2X0_CTRL_EN, at91_l2cc_base + L2X0_CTRL);
> +
> +	/* Prefetch Control */
> +	reg = readl_relaxed(at91_l2cc_base + L310_PREFETCH_CTRL);
> +	reg &= ~L310_PREFETCH_CTRL_OFFSET_MASK;
> +	reg |= 0x01;
> +	reg |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR;
> +	reg |= L310_PREFETCH_CTRL_PREFETCH_DROP;
> +	reg |= L310_PREFETCH_CTRL_DATA_PREFETCH;
> +	reg |= L310_PREFETCH_CTRL_INSTR_PREFETCH;
> +	reg |= L310_PREFETCH_CTRL_DBL_LINEFILL;
> +	writel_relaxed(reg, at91_l2cc_base + L310_PREFETCH_CTRL);
> +
> +	/* Power Control */
> +	reg = readl_relaxed(at91_l2cc_base + L310_POWER_CTRL);
> +	reg |= L310_STNDBY_MODE_EN;
> +	reg |= L310_DYNAMIC_CLK_GATING_EN;
> +	writel_relaxed(reg, at91_l2cc_base + L310_POWER_CTRL);
> +
> +	/* Disable interrupts */
> +	writel_relaxed(0x00, at91_l2cc_base + L2X0_INTR_MASK);
> +	writel_relaxed(0x01ff, at91_l2cc_base + L2X0_INTR_CLEAR);

Stop hacking around the core L2x0 code.  None of the above should be
necessary, and is in fact potentially dangerous if the cache was already
previously enabled.  Disabling an already enabled cache is a potential
data corrupting event.

Any of the above configuration should be performed by your boot loader
or board firmware, and if not, then we need DT properties for it.

The last thing we need is platforms buggering around in this way, so
consider this a firm NAK against this approach.

> +	l2x0_of_init(0, ~0UL);
> +}
> +#else
> +static inline void at91_init_l2cache(void) {}
> +#endif
> +
>  static void __init sama5_dt_device_init(void)
>  {
> +	at91_init_l2cache();
> +
>  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  	at91_sam9x5_pm_init();
>  }
> -- 
> 1.7.9.5

In fact, none of this code is necessary.  If you set l2c_aux_mask and
l2c_aux_val (preferably to ~0 and 0 respectively) then the generic ARM
code will initialise the L2 cache in the appropriate way for you.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/7] ARM: at91: enable the L2 Cache controller
Date: Mon, 26 Jan 2015 12:45:31 +0000	[thread overview]
Message-ID: <20150126124531.GH26493@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1422266836-24592-1-git-send-email-wenyou.yang@atmel.com>

On Mon, Jan 26, 2015 at 06:07:16PM +0800, Wenyou Yang wrote:
> +#ifdef CONFIG_CACHE_L2X0
> +static void __init at91_init_l2cache(void)
> +{
> +	struct device_node *np;
> +	u32 reg;
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +	if (!np)
> +		return;
> +
> +	at91_l2cc_base = of_iomap(np, 0);
> +	if (!at91_l2cc_base)
> +		panic("unable to map l2cc cpu registers\n");
> +
> +	of_node_put(np);
> +
> +	/* Disable cache if it hasn't been done yet */
> +	if (readl_relaxed(at91_l2cc_base + L2X0_CTRL) & L2X0_CTRL_EN)
> +		writel_relaxed(~L2X0_CTRL_EN, at91_l2cc_base + L2X0_CTRL);
> +
> +	/* Prefetch Control */
> +	reg = readl_relaxed(at91_l2cc_base + L310_PREFETCH_CTRL);
> +	reg &= ~L310_PREFETCH_CTRL_OFFSET_MASK;
> +	reg |= 0x01;
> +	reg |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR;
> +	reg |= L310_PREFETCH_CTRL_PREFETCH_DROP;
> +	reg |= L310_PREFETCH_CTRL_DATA_PREFETCH;
> +	reg |= L310_PREFETCH_CTRL_INSTR_PREFETCH;
> +	reg |= L310_PREFETCH_CTRL_DBL_LINEFILL;
> +	writel_relaxed(reg, at91_l2cc_base + L310_PREFETCH_CTRL);
> +
> +	/* Power Control */
> +	reg = readl_relaxed(at91_l2cc_base + L310_POWER_CTRL);
> +	reg |= L310_STNDBY_MODE_EN;
> +	reg |= L310_DYNAMIC_CLK_GATING_EN;
> +	writel_relaxed(reg, at91_l2cc_base + L310_POWER_CTRL);
> +
> +	/* Disable interrupts */
> +	writel_relaxed(0x00, at91_l2cc_base + L2X0_INTR_MASK);
> +	writel_relaxed(0x01ff, at91_l2cc_base + L2X0_INTR_CLEAR);

Stop hacking around the core L2x0 code.  None of the above should be
necessary, and is in fact potentially dangerous if the cache was already
previously enabled.  Disabling an already enabled cache is a potential
data corrupting event.

Any of the above configuration should be performed by your boot loader
or board firmware, and if not, then we need DT properties for it.

The last thing we need is platforms buggering around in this way, so
consider this a firm NAK against this approach.

> +	l2x0_of_init(0, ~0UL);
> +}
> +#else
> +static inline void at91_init_l2cache(void) {}
> +#endif
> +
>  static void __init sama5_dt_device_init(void)
>  {
> +	at91_init_l2cache();
> +
>  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  	at91_sam9x5_pm_init();
>  }
> -- 
> 1.7.9.5

In fact, none of this code is necessary.  If you set l2c_aux_mask and
l2c_aux_val (preferably to ~0 and 0 respectively) then the generic ARM
code will initialise the L2 cache in the appropriate way for you.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

  parent reply	other threads:[~2015-01-26 12:45 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 10:03 [PATCH 0/7] AT91 pm improvements for 3.20 Wenyou Yang
2015-01-26 10:03 ` Wenyou Yang
2015-01-26 10:04 ` [PATCH 1/7] pm: at91: achieve the memory controller's type from the dts file Wenyou Yang
2015-01-26 10:04   ` Wenyou Yang
2015-01-26 10:06 ` [PATCH 2/7] pm: at91: pm_suspend: add the WFI support for ARMv7 Wenyou Yang
2015-01-26 10:06   ` Wenyou Yang
2015-01-26 13:05   ` Sergei Shtylyov
2015-01-26 13:05     ` Sergei Shtylyov
2015-01-27  4:44     ` Yang, Wenyou
2015-01-27  4:44       ` Yang, Wenyou
2015-01-28 11:25   ` Lorenzo Pieralisi
2015-01-28 11:25     ` Lorenzo Pieralisi
2015-01-29  2:36     ` Yang, Wenyou
2015-01-29  2:36       ` Yang, Wenyou
2015-01-29 12:22       ` Lorenzo Pieralisi
2015-01-29 12:22         ` Lorenzo Pieralisi
2015-01-30  7:23         ` Yang, Wenyou
2015-01-30  7:23           ` Yang, Wenyou
2015-01-30 10:17           ` Lorenzo Pieralisi
2015-01-30 10:17             ` Lorenzo Pieralisi
2015-01-30 10:44           ` Lorenzo Pieralisi
2015-01-30 10:44             ` Lorenzo Pieralisi
2015-01-26 10:06 ` [PATCH 3/7] pm: at91: pm_suspend: MOR register KEY was missing Wenyou Yang
2015-01-26 10:06   ` Wenyou Yang
2015-01-26 10:07 ` [PATCH 4/7] ARM: at91: enable the L2 Cache controller Wenyou Yang
2015-01-26 10:07   ` Wenyou Yang
2015-01-26 11:46   ` Mark Rutland
2015-01-26 11:46     ` Mark Rutland
2015-01-26 12:45   ` Russell King - ARM Linux [this message]
2015-01-26 12:45     ` Russell King - ARM Linux
2015-01-26 22:36   ` Alexandre Belloni
2015-01-26 22:36     ` Alexandre Belloni
2015-01-27  5:11     ` Yang, Wenyou
2015-01-27  5:11       ` Yang, Wenyou
2015-01-26 10:07 ` [PATCH 5/7] pm: at91: add disable/enable the L1/L2 cache while suspend/resume Wenyou Yang
2015-01-26 10:07   ` Wenyou Yang
2015-01-26 10:08 ` [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the DDR clock ID support Wenyou Yang
2015-01-26 10:08   ` Wenyou Yang
2015-01-26 11:49   ` Mark Rutland
2015-01-26 11:49     ` Mark Rutland
2015-01-27  5:24     ` Yang, Wenyou
2015-01-27  5:24       ` Yang, Wenyou
2015-01-26 10:08 ` [PATCH 7/7] pm: at91: add disable/enable the mpddrc's clock and DDR clock support Wenyou Yang
2015-01-26 10:08   ` Wenyou Yang

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=20150126124531.GH26493@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=Patrice.VILCHEZ@atmel.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.ferre@atmel.com \
    --cc=peda@axentia.se \
    --cc=sylvain.rochet@finsecur.com \
    --cc=wenyou.yang@atmel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.