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

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

[Sekhar Nori]

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(+)

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;
-- 
1.7.10.1

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

[Sekhar Nori]

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(+)

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;
-- 
1.7.10.1

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

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>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

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>

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140617/86f291a2/attachment.sig>

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

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.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

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.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140701/3e652d65/attachment.sig>

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* 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

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

[Tony Lindgren]

* 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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* 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

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

[Tony Lindgren]

* 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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* 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

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

[Tony Lindgren]

* 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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Sekhar Nori]

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

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

[Sekhar Nori]

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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* 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

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

[Tony Lindgren]

* 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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Russell King - ARM Linux]

On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
> * 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.

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@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
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@arm.linux.org.uk>
---
 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.

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

[Russell King - ARM Linux]

On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
> * 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.

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@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
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@arm.linux.org.uk>
---
 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.

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]

Hi,

On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
> > * 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.
> 
> 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@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> 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@arm.linux.org.uk>
> ---
>  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;

even with this change, l2c still tries to write to power control
register, at least on AM437x. Looking a little deeper here, AM437x
identifies itself as l2c PL310 r3p3, which should have power control
register, but aparentely there's no way to write that register. I'll
file a bug to our ROM team, but we will certainly need a way to
workaround this inside omap4-common.c

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

Hi,

On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
> > * 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.
> 
> 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@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> 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@arm.linux.org.uk>
> ---
>  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;

even with this change, l2c still tries to write to power control
register, at least on AM437x. Looking a little deeper here, AM437x
identifies itself as l2c PL310 r3p3, which should have power control
register, but aparentely there's no way to write that register. I'll
file a bug to our ROM team, but we will certainly need a way to
workaround this inside omap4-common.c

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140707/88ccd160/attachment.sig>

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Sekhar Nori]

On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
>>> * 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.
>>
>> 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@arm.linux.org.uk>
>> Cc: linux-arm-kernel@lists.infradead.org
>> 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@arm.linux.org.uk>
>> ---
>>  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;
> 
> even with this change, l2c still tries to write to power control
> register, at least on AM437x. Looking a little deeper here, AM437x
> identifies itself as l2c PL310 r3p3, which should have power control
> register, but aparentely there's no way to write that register. I'll
> file a bug to our ROM team, but we will certainly need a way to
> workaround this inside omap4-common.c

Looks like we need both my patch as well as Russell's patch. I can
respin my patch with the pr_info_once() dropped if it helps further
reduce boot noise.

Thanks,
Sekhar

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

[Sekhar Nori]

On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
>>> * 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.
>>
>> 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@arm.linux.org.uk>
>> Cc: linux-arm-kernel at lists.infradead.org
>> 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@arm.linux.org.uk>
>> ---
>>  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;
> 
> even with this change, l2c still tries to write to power control
> register, at least on AM437x. Looking a little deeper here, AM437x
> identifies itself as l2c PL310 r3p3, which should have power control
> register, but aparentely there's no way to write that register. I'll
> file a bug to our ROM team, but we will certainly need a way to
> workaround this inside omap4-common.c

Looks like we need both my patch as well as Russell's patch. I can
respin my patch with the pr_info_once() dropped if it helps further
reduce boot noise.

Thanks,
Sekhar

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >> --- 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;
> > 
> > even with this change, l2c still tries to write to power control
> > register, at least on AM437x. Looking a little deeper here, AM437x
> > identifies itself as l2c PL310 r3p3, which should have power control
> > register, but aparentely there's no way to write that register. I'll
> > file a bug to our ROM team, but we will certainly need a way to
> > workaround this inside omap4-common.c
> 
> Looks like we need both my patch as well as Russell's patch. I can
> respin my patch with the pr_info_once() dropped if it helps further
> reduce boot noise.

In that case I'm fine with the original patch in this series. Russell,
got any better ideas?

Regards,

Tony

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

[Tony Lindgren]

* Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >> --- 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;
> > 
> > even with this change, l2c still tries to write to power control
> > register, at least on AM437x. Looking a little deeper here, AM437x
> > identifies itself as l2c PL310 r3p3, which should have power control
> > register, but aparentely there's no way to write that register. I'll
> > file a bug to our ROM team, but we will certainly need a way to
> > workaround this inside omap4-common.c
> 
> Looks like we need both my patch as well as Russell's patch. I can
> respin my patch with the pr_info_once() dropped if it helps further
> reduce boot noise.

In that case I'm fine with the original patch in this series. Russell,
got any better ideas?

Regards,

Tony

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [140708 01:32]:
> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> > On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> > >> --- 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;
> > > 
> > > even with this change, l2c still tries to write to power control
> > > register, at least on AM437x. Looking a little deeper here, AM437x
> > > identifies itself as l2c PL310 r3p3, which should have power control
> > > register, but aparentely there's no way to write that register. I'll
> > > file a bug to our ROM team, but we will certainly need a way to
> > > workaround this inside omap4-common.c
> > 
> > Looks like we need both my patch as well as Russell's patch. I can
> > respin my patch with the pr_info_once() dropped if it helps further
> > reduce boot noise.
> 
> In that case I'm fine with the original patch in this series. Russell,
> got any better ideas?

I guess no more comments. Took a look at the patch again, Sekhar, can
you please update the description with what has been discovered in this
thread and repost?

Regards,

Tony

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

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [140708 01:32]:
> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> > On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> > >> --- 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;
> > > 
> > > even with this change, l2c still tries to write to power control
> > > register, at least on AM437x. Looking a little deeper here, AM437x
> > > identifies itself as l2c PL310 r3p3, which should have power control
> > > register, but aparentely there's no way to write that register. I'll
> > > file a bug to our ROM team, but we will certainly need a way to
> > > workaround this inside omap4-common.c
> > 
> > Looks like we need both my patch as well as Russell's patch. I can
> > respin my patch with the pr_info_once() dropped if it helps further
> > reduce boot noise.
> 
> In that case I'm fine with the original patch in this series. Russell,
> got any better ideas?

I guess no more comments. Took a look at the patch again, Sekhar, can
you please update the description with what has been discovered in this
thread and repost?

Regards,

Tony

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Sekhar Nori]

On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
>> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
>>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
>>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>>>>> --- 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;
>>>>
>>>> even with this change, l2c still tries to write to power control
>>>> register, at least on AM437x. Looking a little deeper here, AM437x
>>>> identifies itself as l2c PL310 r3p3, which should have power control
>>>> register, but aparentely there's no way to write that register. I'll
>>>> file a bug to our ROM team, but we will certainly need a way to
>>>> workaround this inside omap4-common.c
>>>
>>> Looks like we need both my patch as well as Russell's patch. I can
>>> respin my patch with the pr_info_once() dropped if it helps further
>>> reduce boot noise.
>>
>> In that case I'm fine with the original patch in this series. Russell,
>> got any better ideas?
> 
> I guess no more comments. Took a look at the patch again, Sekhar, can
> you please update the description with what has been discovered in this
> thread and repost?

How does the following sound:

---
AM437x has L2C-310 version r3p2 and ROM code on that device 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.

>From the public TRM available for OMAP4470, even on that device, ROM
does not support writing to this register even though it uses a version
of L2C-310 which has the register implemented. So this patch should take
care of all variants of existing OMAPs.
---

Two things that I have added are the version of L2C on AM437x and the
fact that OMAP4470 also has the same issue (at least from the public
TRM). Let me know if you would like to see anything else mentioned.

Thanks,
Sekhar

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

[Sekhar Nori]

On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
>> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
>>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
>>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>>>>> --- 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;
>>>>
>>>> even with this change, l2c still tries to write to power control
>>>> register, at least on AM437x. Looking a little deeper here, AM437x
>>>> identifies itself as l2c PL310 r3p3, which should have power control
>>>> register, but aparentely there's no way to write that register. I'll
>>>> file a bug to our ROM team, but we will certainly need a way to
>>>> workaround this inside omap4-common.c
>>>
>>> Looks like we need both my patch as well as Russell's patch. I can
>>> respin my patch with the pr_info_once() dropped if it helps further
>>> reduce boot noise.
>>
>> In that case I'm fine with the original patch in this series. Russell,
>> got any better ideas?
> 
> I guess no more comments. Took a look at the patch again, Sekhar, can
> you please update the description with what has been discovered in this
> thread and repost?

How does the following sound:

---
AM437x has L2C-310 version r3p2 and ROM code on that device 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.

>From the public TRM available for OMAP4470, even on that device, ROM
does not support writing to this register even though it uses a version
of L2C-310 which has the register implemented. So this patch should take
care of all variants of existing OMAPs.
---

Two things that I have added are the version of L2C on AM437x and the
fact that OMAP4470 also has the same issue (at least from the public
TRM). Let me know if you would like to see anything else mentioned.

Thanks,
Sekhar

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Tony Lindgren]

* Sekhar Nori <nsekhar@ti.com> [140709 05:28]:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> >> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> >>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> >>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >>>>> --- 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;
> >>>>
> >>>> even with this change, l2c still tries to write to power control
> >>>> register, at least on AM437x. Looking a little deeper here, AM437x
> >>>> identifies itself as l2c PL310 r3p3, which should have power control
> >>>> register, but aparentely there's no way to write that register. I'll
> >>>> file a bug to our ROM team, but we will certainly need a way to
> >>>> workaround this inside omap4-common.c
> >>>
> >>> Looks like we need both my patch as well as Russell's patch. I can
> >>> respin my patch with the pr_info_once() dropped if it helps further
> >>> reduce boot noise.
> >>
> >> In that case I'm fine with the original patch in this series. Russell,
> >> got any better ideas?
> > 
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
> 
> From the public TRM available for OMAP4470, even on that device, ROM
> does not support writing to this register even though it uses a version
> of L2C-310 which has the register implemented. So this patch should take
> care of all variants of existing OMAPs.
> ---
> 
> Two things that I have added are the version of L2C on AM437x and the
> fact that OMAP4470 also has the same issue (at least from the public
> TRM). Let me know if you would like to see anything else mentioned.

Looks good to me thanks.

Tony

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

[Tony Lindgren]

* Sekhar Nori <nsekhar@ti.com> [140709 05:28]:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> >> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> >>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> >>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >>>>> --- 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;
> >>>>
> >>>> even with this change, l2c still tries to write to power control
> >>>> register, at least on AM437x. Looking a little deeper here, AM437x
> >>>> identifies itself as l2c PL310 r3p3, which should have power control
> >>>> register, but aparentely there's no way to write that register. I'll
> >>>> file a bug to our ROM team, but we will certainly need a way to
> >>>> workaround this inside omap4-common.c
> >>>
> >>> Looks like we need both my patch as well as Russell's patch. I can
> >>> respin my patch with the pr_info_once() dropped if it helps further
> >>> reduce boot noise.
> >>
> >> In that case I'm fine with the original patch in this series. Russell,
> >> got any better ideas?
> > 
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
> 
> From the public TRM available for OMAP4470, even on that device, ROM
> does not support writing to this register even though it uses a version
> of L2C-310 which has the register implemented. So this patch should take
> care of all variants of existing OMAPs.
> ---
> 
> Two things that I have added are the version of L2C on AM437x and the
> fact that OMAP4470 also has the same issue (at least from the public
> TRM). Let me know if you would like to see anything else mentioned.

Looks good to me thanks.

Tony

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Russell King - ARM Linux]

On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
> 
> >From the public TRM available for OMAP4470, even on that device, ROM
> does not support writing to this register even though it uses a version
> of L2C-310 which has the register implemented. So this patch should take
> care of all variants of existing OMAPs.
> ---

That sounds perfect, and explains why the change has to exist, and why
it can't be fixed elsewhere.  Thanks for providing the full reasoning in
the commit message.

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

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

[Russell King - ARM Linux]

On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
> 
> >From the public TRM available for OMAP4470, even on that device, ROM
> does not support writing to this register even though it uses a version
> of L2C-310 which has the register implemented. So this patch should take
> care of all variants of existing OMAPs.
> ---

That sounds perfect, and explains why the change has to exist, and why
it can't be fixed elsewhere.  Thanks for providing the full reasoning in
the commit message.

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

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

On Wed, Jul 09, 2014 at 02:25:31AM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> > * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> > > On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > > > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> > > >> --- 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;
> > > > 
> > > > even with this change, l2c still tries to write to power control
> > > > register, at least on AM437x. Looking a little deeper here, AM437x
> > > > identifies itself as l2c PL310 r3p3, which should have power control
> > > > register, but aparentely there's no way to write that register. I'll
> > > > file a bug to our ROM team, but we will certainly need a way to
> > > > workaround this inside omap4-common.c
> > > 
> > > Looks like we need both my patch as well as Russell's patch. I can
> > > respin my patch with the pr_info_once() dropped if it helps further
> > > reduce boot noise.
> > 
> > In that case I'm fine with the original patch in this series. Russell,
> > got any better ideas?
> 
> I guess no more comments. Took a look at the patch again, Sekhar, can
> you please update the description with what has been discovered in this
> thread and repost?

another thing to note is that as of today, there is no known way of
accessing power control register in *any* TI parts. That's quite
unfortunate :-(

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

On Wed, Jul 09, 2014 at 02:25:31AM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> > * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> > > On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> > > > On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> > > >> --- 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;
> > > > 
> > > > even with this change, l2c still tries to write to power control
> > > > register, at least on AM437x. Looking a little deeper here, AM437x
> > > > identifies itself as l2c PL310 r3p3, which should have power control
> > > > register, but aparentely there's no way to write that register. I'll
> > > > file a bug to our ROM team, but we will certainly need a way to
> > > > workaround this inside omap4-common.c
> > > 
> > > Looks like we need both my patch as well as Russell's patch. I can
> > > respin my patch with the pr_info_once() dropped if it helps further
> > > reduce boot noise.
> > 
> > In that case I'm fine with the original patch in this series. Russell,
> > got any better ideas?
> 
> I guess no more comments. Took a look at the patch again, Sekhar, can
> you please update the description with what has been discovered in this
> thread and repost?

another thing to note is that as of today, there is no known way of
accessing power control register in *any* TI parts. That's quite
unfortunate :-(

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140709/06d18d0f/attachment.sig>

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]

On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> >> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> >>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> >>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >>>>> --- 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;
> >>>>
> >>>> even with this change, l2c still tries to write to power control
> >>>> register, at least on AM437x. Looking a little deeper here, AM437x
> >>>> identifies itself as l2c PL310 r3p3, which should have power control
> >>>> register, but aparentely there's no way to write that register. I'll
> >>>> file a bug to our ROM team, but we will certainly need a way to
> >>>> workaround this inside omap4-common.c
> >>>
> >>> Looks like we need both my patch as well as Russell's patch. I can
> >>> respin my patch with the pr_info_once() dropped if it helps further
> >>> reduce boot noise.
> >>
> >> In that case I'm fine with the original patch in this series. Russell,
> >> got any better ideas?
> > 
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device does not

at least my SoC has r3p3 (cache ID 0x9), other than that, commit log
looks good to my eyes.


-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [140708 01:32]:
> >> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
> >>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
> >>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
> >>>>> --- 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;
> >>>>
> >>>> even with this change, l2c still tries to write to power control
> >>>> register, at least on AM437x. Looking a little deeper here, AM437x
> >>>> identifies itself as l2c PL310 r3p3, which should have power control
> >>>> register, but aparentely there's no way to write that register. I'll
> >>>> file a bug to our ROM team, but we will certainly need a way to
> >>>> workaround this inside omap4-common.c
> >>>
> >>> Looks like we need both my patch as well as Russell's patch. I can
> >>> respin my patch with the pr_info_once() dropped if it helps further
> >>> reduce boot noise.
> >>
> >> In that case I'm fine with the original patch in this series. Russell,
> >> got any better ideas?
> > 
> > I guess no more comments. Took a look at the patch again, Sekhar, can
> > you please update the description with what has been discovered in this
> > thread and repost?
> 
> How does the following sound:
> 
> ---
> AM437x has L2C-310 version r3p2 and ROM code on that device does not

at least my SoC has r3p3 (cache ID 0x9), other than that, commit log
looks good to my eyes.


-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140709/ba8147d0/attachment-0001.sig>

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Santosh Shilimkar]

On Monday 07 July 2014 09:40 AM, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
>> * 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.
> 
> 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.
> 
Sorry for joining late on the thread. Yes the power control register API
isn't provided and write should be avoiding. 

> From: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> 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@arm.linux.org.uk>
> ---
Right. Feel free add my ack if you need one.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

>  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) {
> 

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

[Santosh Shilimkar]

On Monday 07 July 2014 09:40 AM, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 05:39:26AM -0700, Tony Lindgren wrote:
>> * 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.
> 
> 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.
> 
Sorry for joining late on the thread. Yes the power control register API
isn't provided and write should be avoiding. 

> From: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> 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@arm.linux.org.uk>
> ---
Right. Feel free add my ack if you need one.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

>  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) {
> 

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Santosh Shilimkar]

On Wednesday 09 July 2014 08:39 AM, Russell King - ARM Linux wrote:
> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>> you please update the description with what has been discovered in this
>>> thread and repost?
>>
>> How does the following sound:
>>
>> ---
>> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
>>
Power controller register setting doesn't make cache controller
functional but it is for really clock gating and standby.
So please reword, the above statement accordingly.

>> 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.
>>
Instead of being vague here and below, I will just make it very simple as
below.

On OMAP SOCs using PL310 controllers, Power_ctrl register is not
accessible from non-secure software on PL310 versions which supports
it. The secure code takes care of setting it up correctly and the
power transitions are proven on these devices.

So lets add the ignore write check for PL310 Power_ctrl register
write.

>> >From the public TRM available for OMAP4470, even on that device, ROM
>> does not support writing to this register even though it uses a version
>> of L2C-310 which has the register implemented. So this patch should take
>> care of all variants of existing OMAPs.
>> ---
> 
> That sounds perfect, and explains why the change has to exist, and why
> it can't be fixed elsewhere.  Thanks for providing the full reasoning in
> the commit message.
> 

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

[Santosh Shilimkar]

On Wednesday 09 July 2014 08:39 AM, Russell King - ARM Linux wrote:
> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>> you please update the description with what has been discovered in this
>>> thread and repost?
>>
>> How does the following sound:
>>
>> ---
>> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
>>
Power controller register setting doesn't make cache controller
functional but it is for really clock gating and standby.
So please reword, the above statement accordingly.

>> 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.
>>
Instead of being vague here and below, I will just make it very simple as
below.

On OMAP SOCs using PL310 controllers, Power_ctrl register is not
accessible from non-secure software on PL310 versions which supports
it. The secure code takes care of setting it up correctly and the
power transitions are proven on these devices.

So lets add the ignore write check for PL310 Power_ctrl register
write.

>> >From the public TRM available for OMAP4470, even on that device, ROM
>> does not support writing to this register even though it uses a version
>> of L2C-310 which has the register implemented. So this patch should take
>> care of all variants of existing OMAPs.
>> ---
> 
> That sounds perfect, and explains why the change has to exist, and why
> it can't be fixed elsewhere.  Thanks for providing the full reasoning in
> the commit message.
> 

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Sekhar Nori]

On Wednesday 09 July 2014 07:25 PM, Felipe Balbi wrote:
> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
>>>> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
>>>>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
>>>>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>>>>>>> --- 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;
>>>>>>
>>>>>> even with this change, l2c still tries to write to power control
>>>>>> register, at least on AM437x. Looking a little deeper here, AM437x
>>>>>> identifies itself as l2c PL310 r3p3, which should have power control
>>>>>> register, but aparentely there's no way to write that register. I'll
>>>>>> file a bug to our ROM team, but we will certainly need a way to
>>>>>> workaround this inside omap4-common.c
>>>>>
>>>>> Looks like we need both my patch as well as Russell's patch. I can
>>>>> respin my patch with the pr_info_once() dropped if it helps further
>>>>> reduce boot noise.
>>>>
>>>> In that case I'm fine with the original patch in this series. Russell,
>>>> got any better ideas?
>>>
>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>> you please update the description with what has been discovered in this
>>> thread and repost?
>>
>> How does the following sound:
>>
>> ---
>> AM437x has L2C-310 version r3p2 and ROM code on that device does not
> 
> at least my SoC has r3p3 (cache ID 0x9), other than that, commit log
> looks good to my eyes.

Heh, the TRM documents it as r3p2 and I was relying on it to be correct.
I will change it to r3p3.

Thanks,
Sekhar

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

[Sekhar Nori]

On Wednesday 09 July 2014 07:25 PM, Felipe Balbi wrote:
> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>> * Tony Lindgren <tony@atomide.com> [140708 01:32]:
>>>> * Sekhar Nori <nsekhar@ti.com> [140707 21:56]:
>>>>> On Monday 07 July 2014 08:40 PM, Felipe Balbi wrote:
>>>>>> On Mon, Jul 07, 2014 at 02:40:08PM +0100, Russell King - ARM Linux wrote:
>>>>>>> --- 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;
>>>>>>
>>>>>> even with this change, l2c still tries to write to power control
>>>>>> register, at least on AM437x. Looking a little deeper here, AM437x
>>>>>> identifies itself as l2c PL310 r3p3, which should have power control
>>>>>> register, but aparentely there's no way to write that register. I'll
>>>>>> file a bug to our ROM team, but we will certainly need a way to
>>>>>> workaround this inside omap4-common.c
>>>>>
>>>>> Looks like we need both my patch as well as Russell's patch. I can
>>>>> respin my patch with the pr_info_once() dropped if it helps further
>>>>> reduce boot noise.
>>>>
>>>> In that case I'm fine with the original patch in this series. Russell,
>>>> got any better ideas?
>>>
>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>> you please update the description with what has been discovered in this
>>> thread and repost?
>>
>> How does the following sound:
>>
>> ---
>> AM437x has L2C-310 version r3p2 and ROM code on that device does not
> 
> at least my SoC has r3p3 (cache ID 0x9), other than that, commit log
> looks good to my eyes.

Heh, the TRM documents it as r3p2 and I was relying on it to be correct.
I will change it to r3p3.

Thanks,
Sekhar

Re: [PATCH] ARM: OMAP2+: l2c: squelch warning dump on power control setting

[Sekhar Nori]

On Wednesday 09 July 2014 07:45 PM, Santosh Shilimkar wrote:
> On Wednesday 09 July 2014 08:39 AM, Russell King - ARM Linux wrote:
>> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>>> you please update the description with what has been discovered in this
>>>> thread and repost?
>>>
>>> How does the following sound:
>>>
>>> ---
>>> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
>>>
> Power controller register setting doesn't make cache controller
> functional but it is for really clock gating and standby.
> So please reword, the above statement accordingly.

I think you misunderstood the context here. I am not claiming that power
controller setting (or the absence of it) is  making the L2 cache
non-functional. The line addresses the "Why?" part of why I am creating
the patch - because the warning misleads people into thinking something
went wrong with cache initialization when nothing is wrong really.

> 
>>> 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.
>>>
> Instead of being vague here and below, I will just make it very simple as
> below.
> 
> On OMAP SOCs using PL310 controllers, Power_ctrl register is not
> accessible from non-secure software on PL310 versions which supports
> it. The secure code takes care of setting it up correctly and the
> power transitions are proven on these devices.
> 
> So lets add the ignore write check for PL310 Power_ctrl register
> write.

Since the description is already okayed by others, instead of changing
it completely and starting a re-review, I just prepended your input into
an opening paragraph and removed the last para on OMAP4. I will send out
a v2 shortly with this description.

---
On OMAP SOCs using PL310 controllers, power_ctrl register is not
accessible from non-secure software on PL310 versions which supports
it. The secure code takes care of setting it up correctly and the
power transitions are proven on these devices.

For example, AM437x has L2C-310 version r3p3 and ROM code on that device
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.
---

Thanks,
Sekhar

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

[Sekhar Nori]

On Wednesday 09 July 2014 07:45 PM, Santosh Shilimkar wrote:
> On Wednesday 09 July 2014 08:39 AM, Russell King - ARM Linux wrote:
>> On Wed, Jul 09, 2014 at 05:56:37PM +0530, Sekhar Nori wrote:
>>> On Wednesday 09 July 2014 02:55 PM, Tony Lindgren wrote:
>>>> I guess no more comments. Took a look at the patch again, Sekhar, can
>>>> you please update the description with what has been discovered in this
>>>> thread and repost?
>>>
>>> How does the following sound:
>>>
>>> ---
>>> AM437x has L2C-310 version r3p2 and ROM code on that device 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.
>>>
> Power controller register setting doesn't make cache controller
> functional but it is for really clock gating and standby.
> So please reword, the above statement accordingly.

I think you misunderstood the context here. I am not claiming that power
controller setting (or the absence of it) is  making the L2 cache
non-functional. The line addresses the "Why?" part of why I am creating
the patch - because the warning misleads people into thinking something
went wrong with cache initialization when nothing is wrong really.

> 
>>> 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.
>>>
> Instead of being vague here and below, I will just make it very simple as
> below.
> 
> On OMAP SOCs using PL310 controllers, Power_ctrl register is not
> accessible from non-secure software on PL310 versions which supports
> it. The secure code takes care of setting it up correctly and the
> power transitions are proven on these devices.
> 
> So lets add the ignore write check for PL310 Power_ctrl register
> write.

Since the description is already okayed by others, instead of changing
it completely and starting a re-review, I just prepended your input into
an opening paragraph and removed the last para on OMAP4. I will send out
a v2 shortly with this description.

---
On OMAP SOCs using PL310 controllers, power_ctrl register is not
accessible from non-secure software on PL310 versions which supports
it. The secure code takes care of setting it up correctly and the
power transitions are proven on these devices.

For example, AM437x has L2C-310 version r3p3 and ROM code on that device
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.
---

Thanks,
Sekhar