[PATCH 3/5] mach-u300: patch physoffset by default

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Cc: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Linus Walleij <redacted>
---
 arch/arm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2c71a8f..14bfd8b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig

@@ -831,6 +831,7 @@ config ARCH_U300
 	select HAVE_SCHED_CLOCK
 	select HAVE_TCM
 	select ARM_AMBA
+	select ARM_PATCH_PHYS_VIRT
 	select ARM_VIC
 	select GENERIC_CLOCKEVENTS
 	select CLKDEV_LOOKUP

-- 
1.7.3.2

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Will

On Wed, Aug 10, 2011 at 10:22:06AM +0100, Will Deacon wrote:

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Maybe also making the option hidden depending on EXPERT, or even EMBEDDED
would be a good idea too.  I think it falls into at least the same class
as UID16, sysctl, hotplug, printk, etc. which are all EXPERT options.

On Wed, Aug 10, 2011 at 11:16 AM, Russell King - ARM Linux
[off-list ref] wrote:

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now. ?This
raises the question is whether we should now default it to 'y' - I
think we should. ?Anyone have any objections?

I agree, no objections. Works flawlessly on all my systems.

Thanks,
Linus Walleij

On Wed, Aug 10, 2011 at 10:29:55AM +0100, Russell King - ARM Linux wrote:

On Wed, Aug 10, 2011 at 10:22:06AM +0100, Will Deacon wrote:

quoted

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Maybe also making the option hidden depending on EXPERT, or even EMBEDDED
would be a good idea too.  I think it falls into at least the same class
as UID16, sysctl, hotplug, printk, etc. which are all EXPERT options.

Right, I'm now committing a patch to hide the option unless EMBEDDED
is enabled.  I think this means we should get rid of the 'select
ARM_PATCH_PHYS_VIRT' statements from the various platforms, so that
folk can optimize away that code if they know what they're doing.

Note: this patch will conflict with the removal of the 16-bit P2V
patching, so we need to sort that out.

8<---------
From: Russell King - ARM Linux <redacted>
ARM: enable ARM_PATCH_PHYS_VIRT by default

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>
---
 arch/arm/Kconfig |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5ebc5d9..6085a6c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig

@@ -195,7 +195,8 @@ config VECTORS_BASE
 	  The base address of exception vectors.
 
 config ARM_PATCH_PHYS_VIRT
-	bool "Patch physical to virtual translations at runtime"
+	bool "Patch physical to virtual translations at runtime" if EMBEDDED
+	default y
 	depends on !XIP_KERNEL && MMU
 	depends on !ARCH_REALVIEW || !SPARSEMEM
 	help

@@ -207,6 +208,10 @@ config ARM_PATCH_PHYS_VIRT
 	  of physical memory is at a 16MB boundary, or theoretically 64K
 	  for the MSM machine class.
 
+	  Only disable this option if you know what you do not require
+	  this feature (eg, building a kernel for a single machine) and
+	  you need to shrink the kernel to the minimal size.
+
 config ARM_PATCH_PHYS_VIRT_16BIT
 	def_bool y
 	depends on ARM_PATCH_PHYS_VIRT && ARCH_MSM

@@ -301,7 +306,6 @@ config ARCH_AT91
 	select ARCH_REQUIRE_GPIOLIB
 	select HAVE_CLK
 	select CLKDEV_LOOKUP
-	select ARM_PATCH_PHYS_VIRT if MMU
 	help
 	  This enables support for systems based on the Atmel AT91RM9200,
 	  AT91SAM9 and AT91CAP9 processors.

-- 
1.7.4.4

Hi Russell,

On Thu, Aug 11, 2011 at 09:24:13AM +0100, Russell King - ARM Linux wrote:

On Wed, Aug 10, 2011 at 10:29:55AM +0100, Russell King - ARM Linux wrote:

quoted

On Wed, Aug 10, 2011 at 10:22:06AM +0100, Will Deacon wrote:

quoted

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Maybe also making the option hidden depending on EXPERT, or even EMBEDDED
would be a good idea too.  I think it falls into at least the same class
as UID16, sysctl, hotplug, printk, etc. which are all EXPERT options.

Right, I'm now committing a patch to hide the option unless EMBEDDED
is enabled.  I think this means we should get rid of the 'select
ARM_PATCH_PHYS_VIRT' statements from the various platforms, so that
folk can optimize away that code if they know what they're doing.

Note: this patch will conflict with the removal of the 16-bit P2V
patching, so we need to sort that out.

8<---------
From: Russell King - ARM Linux <redacted>
ARM: enable ARM_PATCH_PHYS_VIRT by default

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>

[snip]

+	  Only disable this option if you know what you do not require
+	  this feature (eg, building a kernel for a single machine) and
+	  you need to shrink the kernel to the minimal size.

The word 'what' in this statement looks redundant.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

On Thu, Aug 11, 2011 at 11:32:44AM +0300, Baruch Siach wrote:

Hi Russell,

On Thu, Aug 11, 2011 at 09:24:13AM +0100, Russell King - ARM Linux wrote:

quoted

On Wed, Aug 10, 2011 at 10:29:55AM +0100, Russell King - ARM Linux wrote:

quoted

On Wed, Aug 10, 2011 at 10:22:06AM +0100, Will Deacon wrote:

quoted

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Maybe also making the option hidden depending on EXPERT, or even EMBEDDED
would be a good idea too.  I think it falls into at least the same class
as UID16, sysctl, hotplug, printk, etc. which are all EXPERT options.

Right, I'm now committing a patch to hide the option unless EMBEDDED
is enabled.  I think this means we should get rid of the 'select
ARM_PATCH_PHYS_VIRT' statements from the various platforms, so that
folk can optimize away that code if they know what they're doing.

Note: this patch will conflict with the removal of the 16-bit P2V
patching, so we need to sort that out.

8<---------
From: Russell King - ARM Linux <redacted>
ARM: enable ARM_PATCH_PHYS_VIRT by default

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>

[snip]

quoted

+	  Only disable this option if you know what you do not require
+	  this feature (eg, building a kernel for a single machine) and
+	  you need to shrink the kernel to the minimal size.

The word 'what' in this statement looks redundant.

Should be 'that'.

Hi Russell,

On Thu, Aug 11, 2011 at 09:24:13AM +0100, Russell King - ARM Linux wrote:

Right, I'm now committing a patch to hide the option unless EMBEDDED
is enabled.  I think this means we should get rid of the 'select
ARM_PATCH_PHYS_VIRT' statements from the various platforms, so that
folk can optimize away that code if they know what they're doing.

Note: this patch will conflict with the removal of the 16-bit P2V
patching, so we need to sort that out.

8<---------
From: Russell King - ARM Linux <redacted>
ARM: enable ARM_PATCH_PHYS_VIRT by default

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>
---
 arch/arm/Kconfig |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

With the typo fix, you can have my ack if you like:

Acked-by: Will Deacon <redacted>

Cheers,

Will

On 09:24 Thu 11 Aug     , Russell King - ARM Linux wrote:

On Wed, Aug 10, 2011 at 10:29:55AM +0100, Russell King - ARM Linux wrote:

quoted

On Wed, Aug 10, 2011 at 10:22:06AM +0100, Will Deacon wrote:

quoted

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Maybe also making the option hidden depending on EXPERT, or even EMBEDDED
would be a good idea too.  I think it falls into at least the same class
as UID16, sysctl, hotplug, printk, etc. which are all EXPERT options.

Right, I'm now committing a patch to hide the option unless EMBEDDED
is enabled.  I think this means we should get rid of the 'select
ARM_PATCH_PHYS_VIRT' statements from the various platforms, so that
folk can optimize away that code if they know what they're doing.

Note: this patch will conflict with the removal of the 16-bit P2V
patching, so we need to sort that out.

8<---------
From: Russell King - ARM Linux <redacted>
ARM: enable ARM_PATCH_PHYS_VIRT by default

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>

Best Regards,
J.

On Thu, 11 Aug 2011, Russell King - ARM Linux wrote:

Enable virtual to physical translation patching by default in all
kernels.  Hide the option behind EMBEDDED.

This can still be turned off if people desire, and they know what
they're doing, to shrink the size of the kernel to a minimum.

Signed-off-by: Russell King <redacted>

Acked-by: Nicolas Pitre <redacted>

Will Deacon wrote:

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

Hm, another u-boot problem was found[1].  On the affected machines it
prevents booting a compressed kernel (but uncompressed kernels are
fine) unless CONFIG_ARM_PATCH_PHYS_VIRT is disabled or u-boot is
patched.

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Looks like it worked. ;-)

So, what now?  Is there anything the kernel can do to work around
this, or should we just live with the regression until people patch
their u-boot?

Thanks,
Jonathan

[1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/124051/focus=124062
https://bugzilla.kernel.org/show_bug.cgi?id=42680

On Mon, 6 Feb 2012, Jonathan Nieder wrote:

Will Deacon wrote:

quoted

On Wed, Aug 10, 2011 at 10:16:35AM +0100, Russell King - ARM Linux wrote:

quoted

On Tue, Aug 09, 2011 at 09:41:38PM +0200, Linus Walleij wrote:

quoted

quoted

quoted

From: Linus Walleij <redacted>

This works like a charm so I'll just default-select it.

Well, we can remove the EXPERIMENTAL status of this option now.  This
raises the question is whether we should now default it to 'y' - I
think we should.  Anyone have any objections?

I've been running with this option enabled for the collection of ARM boards
I have and the only problem I have encountered was related to u-boot loading
at the wrong address.

Hm, another u-boot problem was found[1].  On the affected machines it
prevents booting a compressed kernel (but uncompressed kernels are
fine) unless CONFIG_ARM_PATCH_PHYS_VIRT is disabled or u-boot is
patched.

quoted

So I'm all for enabling it by default, especially since it will force out
any remaining issues for boards where this hasn't been used extensively.

Looks like it worked. ;-)

So, what now?  Is there anything the kernel can do to work around
this, or should we just live with the regression until people patch
their u-boot?

You really do want to have uboot patched.  Who knows what other latent 
issues are there that you don't know about.


Nicolas

Nicolas Pitre wrote:

You really do want to have uboot patched.  Who knows what other latent 
issues are there that you don't know about.

Ah, sorry for the lack of clarity.  I don't believe this u-boot patch[1]
has been committed anywhere, so even people with the latest u-boot are
affected.  It's just available from the mailing list archive.

"It's not worth working around this bootloader bug" may still be the
right answer.  After all, in a couple of months presumably most people
will have the patch applied.  And for all I know, it might not even be
possible to work around this bootloader bug, short of flipping the P2V
default again (which seems like a bad idea).  I just wanted to make
sure you were aware of it.

Cheers,
Jonathan

[1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/124051/focus=124062

On Mon, 2012-02-06 at 17:23 -0600, Jonathan Nieder wrote:

Nicolas Pitre wrote:

quoted

You really do want to have uboot patched.  Who knows what other latent 
issues are there that you don't know about.

Ah, sorry for the lack of clarity.  I don't believe this u-boot patch[1]
has been committed anywhere, so even people with the latest u-boot are
affected.  It's just available from the mailing list archive.

FYI it has now been applied to the Marvell u-boot custodian's tree:
git://git.denx.de/u-boot-marvell.git

Ian.

-- 
Ian Campbell


An evil mind is a great comfort.