diff mbox

ARM: OMAP2+: l2c: squelch warning dump on power control setting

Message ID 3c3a7a4861df01d0163787a9c18f7b7ac821c5b9.1403000372.git.nsekhar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sekhar Nori June 17, 2014, 10:34 a.m. UTC
ROM code on AM437x does not support writing to L2C-310 power control
register. The L2C driver, however, tries writing to this register for
all revisions >= r3p0.

This leads to a warning dump on boot which leads most users to believe
that L2 cache is non-functional.

Since the problem is understood, and cannot be addressed through software,
replace the warning with a pr_info() while maintaining the WARN_ON() for
other truly unexpected scenarios.

Reported-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-omap2/omap4-common.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Felipe Balbi June 17, 2014, 1:19 p.m. UTC | #1
On Tue, Jun 17, 2014 at 04:04:51PM +0530, Sekhar Nori wrote:
> ROM code on AM437x does not support writing to L2C-310 power control
> register. The L2C driver, however, tries writing to this register for
> all revisions >= r3p0.
> 
> This leads to a warning dump on boot which leads most users to believe
> that L2 cache is non-functional.
> 
> Since the problem is understood, and cannot be addressed through software,
> replace the warning with a pr_info() while maintaining the WARN_ON() for
> other truly unexpected scenarios.
> 
> Reported-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>

Tested with today's linux-next
(5f295cdf5c5dbbb0c40f10f2ddae02ff46bbf773) with am437x-sk, I can see
that the WARNING goes away.

Tested-by: Felipe Balbi <balbi@ti.com>
Felipe Balbi July 1, 2014, 7:47 p.m. UTC | #2
On Tue, Jun 17, 2014 at 08:19:35AM -0500, Felipe Balbi wrote:
> On Tue, Jun 17, 2014 at 04:04:51PM +0530, Sekhar Nori wrote:
> > ROM code on AM437x does not support writing to L2C-310 power control
> > register. The L2C driver, however, tries writing to this register for
> > all revisions >= r3p0.
> > 
> > This leads to a warning dump on boot which leads most users to believe
> > that L2 cache is non-functional.
> > 
> > Since the problem is understood, and cannot be addressed through software,
> > replace the warning with a pr_info() while maintaining the WARN_ON() for
> > other truly unexpected scenarios.
> > 
> > Reported-by: Nishanth Menon <nm@ti.com>
> > Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> 
> Tested with today's linux-next
> (5f295cdf5c5dbbb0c40f10f2ddae02ff46bbf773) with am437x-sk, I can see
> that the WARNING goes away.
> 
> Tested-by: Felipe Balbi <balbi@ti.com>

ping here, I can't see this patch onl linus/master or next/master yet.
Tony Lindgren July 2, 2014, 8:11 a.m. UTC | #3
* Felipe Balbi <balbi@ti.com> [140701 12:49]:
> On Tue, Jun 17, 2014 at 08:19:35AM -0500, Felipe Balbi wrote:
> > On Tue, Jun 17, 2014 at 04:04:51PM +0530, Sekhar Nori wrote:
> > > ROM code on AM437x does not support writing to L2C-310 power control
> > > register. The L2C driver, however, tries writing to this register for
> > > all revisions >= r3p0.
> > > 
> > > This leads to a warning dump on boot which leads most users to believe
> > > that L2 cache is non-functional.
> > > 
> > > Since the problem is understood, and cannot be addressed through software,
> > > replace the warning with a pr_info() while maintaining the WARN_ON() for
> > > other truly unexpected scenarios.
> > > 
> > > Reported-by: Nishanth Menon <nm@ti.com>
> > > Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> > 
> > Tested with today's linux-next
> > (5f295cdf5c5dbbb0c40f10f2ddae02ff46bbf773) with am437x-sk, I can see
> > that the WARNING goes away.
> > 
> > Tested-by: Felipe Balbi <balbi@ti.com>
> 
> ping here, I can't see this patch onl linus/master or next/master yet.

Sorry I've been waiting for my pull request against -rc1 to get
merged first, no idea why that is still pending.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren July 7, 2014, 10:47 a.m. UTC | #4
* Tony Lindgren <tony@atomide.com> [140702 01:13]:
> * Felipe Balbi <balbi@ti.com> [140701 12:49]:
> > On Tue, Jun 17, 2014 at 08:19:35AM -0500, Felipe Balbi wrote:
> > > On Tue, Jun 17, 2014 at 04:04:51PM +0530, Sekhar Nori wrote:
> > > > ROM code on AM437x does not support writing to L2C-310 power control
> > > > register. The L2C driver, however, tries writing to this register for
> > > > all revisions >= r3p0.
> > > > 
> > > > This leads to a warning dump on boot which leads most users to believe
> > > > that L2 cache is non-functional.
> > > > 
> > > > Since the problem is understood, and cannot be addressed through software,
> > > > replace the warning with a pr_info() while maintaining the WARN_ON() for
> > > > other truly unexpected scenarios.
> > > > 
> > > > Reported-by: Nishanth Menon <nm@ti.com>
> > > > Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> > > 
> > > Tested with today's linux-next
> > > (5f295cdf5c5dbbb0c40f10f2ddae02ff46bbf773) with am437x-sk, I can see
> > > that the WARNING goes away.
> > > 
> > > Tested-by: Felipe Balbi <balbi@ti.com>
> > 
> > ping here, I can't see this patch onl linus/master or next/master yet.
> 
> Sorry I've been waiting for my pull request against -rc1 to get
> merged first, no idea why that is still pending.

That's now merged into v3.16-rc4, so applying this into
omap-for-v3.16/fixes.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux July 7, 2014, 10:49 a.m. UTC | #5
On Mon, Jul 07, 2014 at 03:47:27AM -0700, Tony Lindgren wrote:
> That's now merged into v3.16-rc4, so applying this into
> omap-for-v3.16/fixes.

I intentionally left the warning in the hope that someone would update
it to write to the register.

From the comments in the patch, it seems that firmware doesn't provide
a method to do that, which is a tad annoying.
Tony Lindgren July 7, 2014, 11:02 a.m. UTC | #6
* Russell King - ARM Linux <linux@arm.linux.org.uk> [140707 03:51]:
> On Mon, Jul 07, 2014 at 03:47:27AM -0700, Tony Lindgren wrote:
> > That's now merged into v3.16-rc4, so applying this into
> > omap-for-v3.16/fixes.
> 
> I intentionally left the warning in the hope that someone would update
> it to write to the register.
> 
> From the comments in the patch, it seems that firmware doesn't provide
> a method to do that, which is a tad annoying.

So it seems.. Santosh, might be worth checking if this is set
up the same way for all omaps? Or do some have extra commands
for OMAP4_MON_L2X0_*  for ROM code calls?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 7, 2014, 11:50 a.m. UTC | #7
On Monday 07 July 2014 04:32 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [140707 03:51]:
>> On Mon, Jul 07, 2014 at 03:47:27AM -0700, Tony Lindgren wrote:
>>> That's now merged into v3.16-rc4, so applying this into
>>> omap-for-v3.16/fixes.
>>
>> I intentionally left the warning in the hope that someone would update
>> it to write to the register.
>>
>> From the comments in the patch, it seems that firmware doesn't provide
>> a method to do that, which is a tad annoying.
> 
> So it seems.. Santosh, might be worth checking if this is set
> up the same way for all omaps? Or do some have extra commands
> for OMAP4_MON_L2X0_*  for ROM code calls?

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.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux July 7, 2014, 12:15 p.m. UTC | #8
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.
Tony Lindgren July 7, 2014, 12:39 p.m. UTC | #9
* Russell King - ARM Linux <linux@arm.linux.org.uk> [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.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c
index 326cd98..9139729 100644
--- a/arch/arm/mach-omap2/omap4-common.c
+++ b/arch/arm/mach-omap2/omap4-common.c
@@ -188,6 +188,10 @@  static void omap4_l2c310_write_sec(unsigned long val, unsigned reg)
 		smc_op = OMAP4_MON_L2X0_PREFETCH_INDEX;
 		break;
 
+	case L310_POWER_CTRL:
+		pr_info_once("OMAP L2C310: ROM does not support power control setting\n");
+		return;
+
 	default:
 		WARN_ONCE(1, "OMAP L2C310: ignoring write to reg 0x%x\n", reg);
 		return;