Re: [PATCH] omap4: Fix ULPI PHY init for ES1.0 SDP (Re: 4430SDP boot failure)

Russell King - ARM Linux
2011-01-15T17:05:19Z

On Fri, Jan 14, 2011 at 04:37:34PM -0800, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [110114 16:24]:
> > On Fri, Jan 14, 2011 at 04:12:55PM -0800, Tony Lindgren wrote:
> > > * Russell King - ARM Linux <linux@arm.linux.org.uk> [110114 15:58]:
> > > >
> > > > # ARMv6k
> > > > config CPU_32v6K
> > > > bool "Support ARM V6K processor extensions" if !SMP
> > > > depends on CPU_V6 || CPU_V7
> > > > default y if SMP && !(ARCH_MX3 || ARCH_OMAP2)
> > > >
> > > > OMAP2 prevents the selection of armv6k support. This is probably a very
> > > > bad idea if you want to run the resulting kernel on SMP hardware as it
> > > > removes a barrier in the spinlock code and disables the SMP-safe bitops.
> > >
> > > I have some ideas to fix this. Unfortunately it will be inefficient
> > > as spinlock.h can be included from modules too :( I was thinking we can
> > > implement dsb_sev in the proc-*.S functions for the unoptimized multi-arm
> > > builds.
> >
> > For spinlocks, the important thing is the barrier. The wfe/sev are an
> > optimization. The barrier contained with the ifdef is a valid V6
> > instruction.
>
> OK, great it's just drain WB. Then we can do the ussual iffdeffery
> on it for multi-arm builds as it does not depend on the 6K extensions.
> I can do a patch for this on Monday, gotta run now.
>
> > > > The original patch which started turning this off was from the MX3 stuff,
> > > > but without explaination.
> > > >
> > > > However, OMAP extended this to disabling the select statement for CPU_32v6K
> > > > even if CPU_V7 is set:
> > > >
> > > > config CPU_V7
> > > > bool "Support ARM V7 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB |- select CPU_32v6K
> > > > + select CPU_32v6K if !ARCH_OMAP2
> > > >
> > > > Arguably, SMP _requires_ CPU_32v6K to be enabled for a safe kernel, and this
> > > > patch should not have been merged.
> > >
> > > The only way we can fix that is do smp_on_up style rewriting of the assembly
> > > during init between CPUv6 and v6K. Want me to do a patch for that?
> >
> > The bitops code is quite different between the two versions, and I doubt
> > the smp_on_up rewriting will look at all pretty. I think it needs an
> > alternative idea - like not using the 'byte' operations at all.
> >
> > Whether we have any code which passes non-word aligned pointers to bitops
> > isn't particularly known - in theory they should all be unsigned long *'s,
> > so should be word-aligned. Who knows what filesystems do though... and
> > such a change could be disasterous to peoples data if the block/inode
> > bitmaps get corrupted.
>
> Hmm, how about emulation of those instructions for non-v6K ARMv6 processors?
> I guess we could do some address checking in the bitops functions too for
> multi-arm builds..
>
> > IOW, such a change needs testing on a box where a range of filesystems are
> > used, and the filesystems can be thrown away if corrupted.
>
> Or we could patch in the address checking first and only disable it
> later if now warnings.

Right, this is what I'm going to do - and it's going to *intentionally*
break omap2plus_defconfig. Please see the commit comments for the
reason why. We need to address the V6 issue properly without risking
users data.

To Sascha: this replaces the previous patch which I asked for your ack.

8<-----------
Subject: [PATCH] ARM: Do not disable CPU_32v6K based on platform selection

CPU_32v6K controls whether we use the ARMv6K extension instructions in
the kernel, and in some places whether we use SMP-safe code sequences
(eg, bitops.)

Having this configuration option disabled on a SMP supporting kernel
results in a problem: the SMP-unsafe code sequences will be used, and
as such the resulting kernel is not SMP safe.

As the atomic bitops are used by filesystems (eg, ext2 - to manipulate
the inode and block bitmaps) not having the SMP safe code sequences is
fatal for filesystem data integrity. So running an SMP kernel without
CPU_32v6K set is dangerous.

MX3 prevents the selection of this option to ensure that it is not
enabled for their CPU, which is ARMv6 only. MX3 folk need to ensure
that their kernel is properly configured.

OMAP prevents the selection of this option in an attempt to produce a
kernel which runs on architectures from ARMv6 to ARMv7 MPCore.

Balancing between oopsing on boot and filesystem corruption, it is far
more preferable to oops on boot until the code sequences are sorted out
properly.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
arch/arm/mm/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 49db8b3..d61af9c 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -405,7 +405,7 @@ config CPU_V6
config CPU_32v6K
bool "Support ARM V6K processor extensions" if !SMP
depends on CPU_V6 || CPU_V7
- default y if SMP && !(ARCH_MX3 || ARCH_OMAP2)
+ default y if SMP
help
Say Y here if your ARMv6 processor supports the 'K' extension.
This enables the kernel to use some instructions not present
--
1.6.2.5