Paul,
Working through my back-log...
On Mon, May 26, 2014 at 11:01:11AM +0200, Paul Bolle wrote:
> ARM_ERRATA_753970 was renamed to PL310_ERRATA_753970 in v3.2, through
> commit fa0ce4035d48 ("ARM: 7162/1: errata: tidy up Kconfig options for
> PL310 errata workarounds"). Two selects were added in v3.15-rc1 that
> still use the previous name. Rename these.
>
> Make these statements depend on CACHE_PL310, like all other selects of
> PL310_ERRATA_753970. That way it will only be selected if its dependency
> is met.
>
> Signed-off-by: Paul Bolle <[email protected]>
> ---
> I reported this before v3.15-rc1. I don't know whether any fixes are
> pending. None are in linux-next.
It looks like rmk just added a patch to fixing the selection of the
erratas. Could you please take a look and either rebase or drop this
patch?
thx,
Jason.
> And ignoring an errata were one
> apparently could be needed sounds, well, scary. Perhaps it is not.
> Anyhow, to make sure this gets fixed, hopefully before v3.15, I'm
> submitting this (untested!) patch.
>
> A related observation. There are three PL310 errata options: one depends
> on CACHE_PL310, three depend on CACHE_L2X0. The one depending on
> CACHE_PL310 is selected only if CACHE_PL310 is set.
>
> But the three depending on CACHE_L2X0 are selected a few times if
> CACHE_L2X0 is set, in other cases if CACHE_PL310 is set, and in some
> cases always. There may be good reasons for this, but it looks odd. I
> know nothing about the PL310 cache and its erratas, so I haven't looked
> into this any further.
>
> arch/arm/mach-mvebu/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index 3f73eecbcfb0..501d0f42e7b2 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -35,7 +35,7 @@ config MACH_ARMADA_370
> config MACH_ARMADA_375
> bool "Marvell Armada 375 boards" if ARCH_MULTI_V7
> select ARM_ERRATA_720789
> - select ARM_ERRATA_753970
> + select PL310_ERRATA_753970 if CACHE_PL310
> select ARM_GIC
> select ARMADA_375_CLK
> select CPU_V7
> @@ -48,7 +48,7 @@ config MACH_ARMADA_375
> config MACH_ARMADA_38X
> bool "Marvell Armada 380/385 boards" if ARCH_MULTI_V7
> select ARM_ERRATA_720789
> - select ARM_ERRATA_753970
> + select PL310_ERRATA_753970 if CACHE_PL310
> select ARM_GIC
> select ARMADA_38X_CLK
> select CPU_V7
> --
> 1.9.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Jason Cooper schreef op vr 20-06-2014 om 16:21 [-0400]:
> It looks like rmk just added a patch to fixing the selection of the
> erratas. Could you please take a look and either rebase or drop this
> patch?
Sure, no problem. What tree should I check?
Paul Bolle
On Fri, Jun 20, 2014 at 04:21:00PM -0400, Jason Cooper wrote:
> It looks like rmk just added a patch to fixing the selection of the
> erratas. Could you please take a look and either rebase or drop this
> patch?
Yes please. If it's needed, just update it to be:
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 3f73eecbcfb0..501d0f42e7b2 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -35,7 +35,7 @@ config MACH_ARMADA_370
> > config MACH_ARMADA_375
> > bool "Marvell Armada 375 boards" if ARCH_MULTI_V7
> > select ARM_ERRATA_720789
> > - select ARM_ERRATA_753970
> > + select PL310_ERRATA_753970 if CACHE_PL310
select PL310_ERRATA_753970 if CACHE_L2X0
for both platforms. Although there's no harm in using CACHE_PL310 at
the moment, my longer term plan is to eventually kill CACHE_PL310 as
it's entirely redundant for ARM versions of the L2 cache (it's only
used by the old L2x0 code which I've been unable to eliminate entirely.)
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
On Fri, Jun 20, 2014 at 10:42:57PM +0200, Paul Bolle wrote:
> Jason Cooper schreef op vr 20-06-2014 om 16:21 [-0400]:
> > It looks like rmk just added a patch to fixing the selection of the
> > erratas. Could you please take a look and either rebase or drop this
> > patch?
>
> Sure, no problem. What tree should I check?
It's
a641f3a6abce ARM: l2c: fix dependencies on PL310 errata symbols
in Russell's fixes branch:
http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/linux-arm.git fixes
thx,
Jason.
On Fri, 2014-06-20 at 17:10 -0400, Jason Cooper wrote:
> On Fri, Jun 20, 2014 at 10:42:57PM +0200, Paul Bolle wrote:
> > Jason Cooper schreef op vr 20-06-2014 om 16:21 [-0400]:
> > > It looks like rmk just added a patch to fixing the selection of the
> > > erratas. Could you please take a look and either rebase or drop this
> > > patch?
> >
> > Sure, no problem. What tree should I check?
>
> It's
>
> a641f3a6abce ARM: l2c: fix dependencies on PL310 errata symbols
>
> in Russell's fixes branch:
>
> http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/linux-arm.git fixes
Thanks. That commit landed in linux-next in next-20140620. It addresses
all comments I made, below the --- marker, in
https://lkml.org/lkml/2014/5/26/103 . It makes it a bit easier to reason
about the PL310_ERRATA_* symbols. I hope to submit a v2 soon.
Paul Bolle
ARM_ERRATA_753970 was renamed to PL310_ERRATA_753970 in v3.2, through
commit fa0ce4035d48 ("ARM: 7162/1: errata: tidy up Kconfig options for
PL310 errata workarounds"). Two selects were added in v3.15 that still
use the previous name. Rename these.
Signed-off-by: Paul Bolle <[email protected]>
---
v2: redone on top of next-21040623, and therefor on top of commit
a641f3a6abce ("ARM: l2c: fix dependencies on PL310 errata symbols").
Dropped the "if CACHE_PL310" fragments used in v1. No need to use "if
CACHE_L2X0" instead. Both CACHE_PL310 and CACHE_L2X0 will be
(indirectly) selected if MACH_ARMADA_375 or MACH_ARMADA_38X are set.
Tested only with "make ARCH=arm menuconfig". So not compile tested! If
that's not acceptable please let me know. (I have no access to these
machines so runtime testing is not possible for me.)
Perhaps we want something similar for v3.15+ stable. But that would
require a different patch (v1 could probably be used for stable).
arch/arm/mach-mvebu/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 955d4a3afabd..8d4cf4fa118a 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -33,7 +33,7 @@ config MACH_ARMADA_370
config MACH_ARMADA_375
bool "Marvell Armada 375 boards" if ARCH_MULTI_V7
select ARM_ERRATA_720789
- select ARM_ERRATA_753970
+ select PL310_ERRATA_753970
select ARM_GIC
select ARMADA_375_CLK
select HAVE_ARM_SCU
@@ -48,7 +48,7 @@ config MACH_ARMADA_375
config MACH_ARMADA_38X
bool "Marvell Armada 380/385 boards" if ARCH_MULTI_V7
select ARM_ERRATA_720789
- select ARM_ERRATA_753970
+ select PL310_ERRATA_753970
select ARM_GIC
select ARMADA_38X_CLK
select HAVE_ARM_SCU
--
1.9.3
It's nice to be ignored...
On Mon, Jun 23, 2014 at 11:01:44AM +0200, Paul Bolle wrote:
> ARM_ERRATA_753970 was renamed to PL310_ERRATA_753970 in v3.2, through
> commit fa0ce4035d48 ("ARM: 7162/1: errata: tidy up Kconfig options for
> PL310 errata workarounds"). Two selects were added in v3.15 that still
> use the previous name. Rename these.
>
> Signed-off-by: Paul Bolle <[email protected]>
> ---
> v2: redone on top of next-21040623, and therefor on top of commit
> a641f3a6abce ("ARM: l2c: fix dependencies on PL310 errata symbols").
And you obviously didn't read the commit or understand it.
> Dropped the "if CACHE_PL310" fragments used in v1. No need to use "if
> CACHE_L2X0" instead. Both CACHE_PL310 and CACHE_L2X0 will be
> (indirectly) selected if MACH_ARMADA_375 or MACH_ARMADA_38X are set.
Yes there is. We've recently seen the selection of CACHE_L2X0 be removed
from platforms, while leaving the selection of the errata behind. The
result is Kconfig warnings. My commit above ensures that this doesn't
happen in the future by adding the proper dependencies onto those errata
selects.
Please do the same.
NAK.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
On Mon, 2014-06-23 at 10:07 +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 23, 2014 at 11:01:44AM +0200, Paul Bolle wrote:
> > Dropped the "if CACHE_PL310" fragments used in v1. No need to use "if
> > CACHE_L2X0" instead. Both CACHE_PL310 and CACHE_L2X0 will be
> > (indirectly) selected if MACH_ARMADA_375 or MACH_ARMADA_38X are set.
>
> Yes there is. We've recently seen the selection of CACHE_L2X0 be removed
> from platforms, while leaving the selection of the errata behind. The
> result is Kconfig warnings. My commit above ensures that this doesn't
> happen in the future by adding the proper dependencies onto those errata
> selects.
The entries I touched read (in summary):
config MACH_MVEBU_V7
bool
select CACHE_L2X0
config MACH_ARMADA_370
bool "Marvell Armada 370 boards" if ARCH_MULTI_V7
select MACH_MVEBU_V7
help
[...].
config MACH_ARMADA_375
bool "Marvell Armada 375 boards" if ARCH_MULTI_V7
select MACH_MVEBU_V7
help
[...].
So the choice I faced was between:
- using "select PL310_ERRATA_753970 if CACHE_L2X0": that matches all
current occurrences of "select PL310_ERRATA_*" but adds a superfluous
dependency on CACHE_L2X0
- using just "select PL310_ERRATA_753970": sufficient, but will break
if CACHE_L2X0 isn't selected through MACH_MVEBU_V7 anymore.
I chose to just "select PL310_ERRATA_753970" but adding "if CACHE_L2X0"
is fine with me too.
> Please do the same.
Will do later today.
Thanks,
Paul Bolle