On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> [140707 05:17]:
> > On Mon, Jul 07, 2014 at 05:20:27PM +0530, Sekhar Nori wrote:
> > > OMAP4430 had L2 cache controller version r2p0 (per the public TRM) which
> > > does not have this register. So unless there is a ROM API that was
> > > introduced after OMAP4430, this would not be there even for other
> > > OMAP4s. Public TRM of OMAP4470 does not indicate an API for this.
> > >
> > > Before creating the patch, I checked with ROM team handling AM437x and
> > > they denied an API to write to this register was present in AM437x ROM.
> >
> > Okay, so why are we trying to write to this register then...
> >
> > Ah, we have a bug in cache-l2x0.c:
> >
> > #define L2X0_CACHE_ID_PART_MASK (0xf << 6)
> > #define L2X0_CACHE_ID_RTL_MASK 0x3f
> > #define L310_CACHE_ID_RTL_R3P0 0x05
> >
> > unsigned rev = readl_relaxed(base + L2X0_CACHE_ID) & L2X0_CACHE_ID_PART_MASK;
> >
> > if (rev >= L310_CACHE_ID_RTL_R2P0) {
> > ...
> > if (rev >= L310_CACHE_ID_RTL_R3P0) {
> > l2c_write_sec(L310_DYNAMIC_CLK_GATING_EN | L310_STNDBY_MODE_EN,
> > base, L310_POWER_CTRL);
> >
> > So, because we're masking the wrong bits, we end up with these tests
> > always succeeding.
> >
> > So that's a NACK for the original patch, it's the wrong fix. The
> > right fix is to avoid writing this register by fixing the RTL masking.
>
> Okie dokie, dropping the omap specific fix.
Here's the revision mask fix - with the existing code, the revision checks
are all useless since they would all pass irrespective of the actual
revision. (Had the L2C series been better tested rather than being largely
ignored, this may have been noticed before it was merged...) Anyway, what
isn't clear from Sekhar's message is which revision L2C310 is in the AM437x.
From: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Subject: [PATCH] ARM: l2c: fix revision checking
The revision checking in l2c310_enable() was not correct; we were
masking the part number rather than the revision number. Fix this
to use the correct macro.
Fixes: 4374d64933b1 ("ARM: l2c: add automatic enable of early BRESP")
Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
---
arch/arm/mm/cache-l2x0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 948f12cf6180..0b5068256baf 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -732,7 +732,7 @@ static int l2c310_cpu_enable_flz(struct notifier_block *nb, unsigned long act, v
static void __init l2c310_enable(void __iomem *base, u32 aux, unsigned num_lock)
{
- unsigned rev = readl_relaxed(base + L2X0_CACHE_ID) & L2X0_CACHE_ID_PART_MASK;
+ unsigned rev = readl_relaxed(base + L2X0_CACHE_ID) & L2X0_CACHE_ID_RTL_MASK;
bool cortex_a9 = read_cpuid_part() == ARM_CPU_PART_CORTEX_A9;
if (rev >= L310_CACHE_ID_RTL_R2P0) {
--
1.8.3.1
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
