[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Paul Gortmaker]

Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
Kconfig for PHYS_OFFSET but it did not specify a default value.

This has the undesired side effect of breaking an implictly used
trick for updating defconfigs on the fly for routine buildtesting
across all arch and all platforms, i.e.

  cp /path/to/somedefconfig .config ; yes "" | make oldconfig

In particular, the at91x40_defconfig will enter an infinite loop
with the above, since there is no default value, and conf will just
keep asking for one, and "yes" will keep feeding in a line return.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 44789ef..90a475e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -221,6 +221,7 @@ config NEED_MACH_MEMORY_H
 
 config PHYS_OFFSET
 	hex "Physical address of main memory"
+	default 0x00000000
 	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
 	help
 	  Please provide the physical address corresponding to the
-- 
1.7.7.2

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Sergei Shtylyov]

Hello.

On 01.12.2011 7:52, Paul Gortmaker wrote:

> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new

    Please also cite that commit's summary in parens.

> Kconfig for PHYS_OFFSET but it did not specify a default value.

> This has the undesired side effect of breaking an implictly used
> trick for updating defconfigs on the fly for routine buildtesting
> across all arch and all platforms, i.e.

>    cp /path/to/somedefconfig .config ; yes "" | make oldconfig

> In particular, the at91x40_defconfig will enter an infinite loop
> with the above, since there is no default value, and conf will just
> keep asking for one, and "yes" will keep feeding in a line return.

> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>

WBR, Sergei

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Arnd Bergmann]

On Thursday 01 December 2011, Paul Gortmaker wrote:
> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
> Kconfig for PHYS_OFFSET but it did not specify a default value.
> 
> This has the undesired side effect of breaking an implictly used
> trick for updating defconfigs on the fly for routine buildtesting
> across all arch and all platforms, i.e.
> 
>   cp /path/to/somedefconfig .config ; yes "" | make oldconfig
> 

FWIW, a better way to achieve the above is using 

make KCONFIG_ALLCONFIG=/path/to/somedefconfig listnewconfig alldefconfig

which does not have this problem.

Your patch looks still ok, but you might want to update your scripts
to be more robust.

	Arnd

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Paul Gortmaker]

On Thu, Dec 1, 2011 at 9:19 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 01 December 2011, Paul Gortmaker wrote:
>> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
>> Kconfig for PHYS_OFFSET but it did not specify a default value.
>>
>> This has the undesired side effect of breaking an implictly used
>> trick for updating defconfigs on the fly for routine buildtesting
>> across all arch and all platforms, i.e.
>>
>> ? cp /path/to/somedefconfig .config ; yes "" | make oldconfig
>>
>
> FWIW, a better way to achieve the above is using
>
> make KCONFIG_ALLCONFIG=/path/to/somedefconfig listnewconfig alldefconfig
>
> which does not have this problem.
>
> Your patch looks still ok, but you might want to update your scripts
> to be more robust.

Thanks -- but on the other hand, the above use case does actually
emulate exactly what happens when someone runs the
"make oldconfig" and simply holds down the Enter key. (yes it
happens probably way more than we want to know...)

So, I guess what I'm trying to say is that the above "script" is
intentionally fragile.  :)  As a use case, we shouldn't break it.

Thanks,
Paul.

>
> ? ? ? ?Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Nicolas Pitre]

On Thu, 1 Dec 2011, Paul Gortmaker wrote:

> On Thu, Dec 1, 2011 at 9:19 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 01 December 2011, Paul Gortmaker wrote:
> >> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
> >> Kconfig for PHYS_OFFSET but it did not specify a default value.
> >>
> >> This has the undesired side effect of breaking an implictly used
> >> trick for updating defconfigs on the fly for routine buildtesting
> >> across all arch and all platforms, i.e.
> >>
> >> ? cp /path/to/somedefconfig .config ; yes "" | make oldconfig
> >>
> >
> > FWIW, a better way to achieve the above is using
> >
> > make KCONFIG_ALLCONFIG=/path/to/somedefconfig listnewconfig alldefconfig
> >
> > which does not have this problem.
> >
> > Your patch looks still ok, but you might want to update your scripts
> > to be more robust.
> 
> Thanks -- but on the other hand, the above use case does actually
> emulate exactly what happens when someone runs the
> "make oldconfig" and simply holds down the Enter key. (yes it
> happens probably way more than we want to know...)
> 
> So, I guess what I'm trying to say is that the above "script" is
> intentionally fragile.  :)  As a use case, we shouldn't break it.

Well... I took a while before replying because I'm also modifying things 
around this, but those changes are good for a merge window not a -rc 
period.

Still, I don't think your patch is good, even as a temporary measure.  
The fact is that we can't just pick a random default value since it is 
likely to be wrong for the majority of the boards as the right answer 
for this option is quite varied.  So the fact that the config system 
insists on your entering a proper value is actually a good thing.

In the at91x40_defconfig case, it has CONFIG_EMBEDDED=y.  If it was not 
selected then you'd get CONFIG_ARM_PATCH_PHYS_VIRT=y by default and then 
no CONFIG_PHYS_OFFSET would be presented.

So the right fix here would actually consist of adding to 
at91x40_defconfig either CONFIG_ARM_PATCH_PHYS_VIRT=y, or a default 
value for CONFIG_PHYS_OFFSET which is the right value for that platform.


Nicolas

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Nicolas Pitre]

On Fri, 2 Dec 2011, Nicolas Pitre wrote:

> On Thu, 1 Dec 2011, Paul Gortmaker wrote:
> 
> > On Thu, Dec 1, 2011 at 9:19 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thursday 01 December 2011, Paul Gortmaker wrote:
> > >> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
> > >> Kconfig for PHYS_OFFSET but it did not specify a default value.
> > >>
> > >> This has the undesired side effect of breaking an implictly used
> > >> trick for updating defconfigs on the fly for routine buildtesting
> > >> across all arch and all platforms, i.e.
> > >>
> > >> ? cp /path/to/somedefconfig .config ; yes "" | make oldconfig
> > >>
> > >
> > > FWIW, a better way to achieve the above is using
> > >
> > > make KCONFIG_ALLCONFIG=/path/to/somedefconfig listnewconfig alldefconfig
> > >
> > > which does not have this problem.
> > >
> > > Your patch looks still ok, but you might want to update your scripts
> > > to be more robust.
> > 
> > Thanks -- but on the other hand, the above use case does actually
> > emulate exactly what happens when someone runs the
> > "make oldconfig" and simply holds down the Enter key. (yes it
> > happens probably way more than we want to know...)
> > 
> > So, I guess what I'm trying to say is that the above "script" is
> > intentionally fragile.  :)  As a use case, we shouldn't break it.
> 
> Well... I took a while before replying because I'm also modifying things 
> around this, but those changes are good for a merge window not a -rc 
> period.
> 
> Still, I don't think your patch is good, even as a temporary measure.  
> The fact is that we can't just pick a random default value since it is 
> likely to be wrong for the majority of the boards as the right answer 
> for this option is quite varied.  So the fact that the config system 
> insists on your entering a proper value is actually a good thing.
> 
> In the at91x40_defconfig case, it has CONFIG_EMBEDDED=y.  If it was not 
> selected then you'd get CONFIG_ARM_PATCH_PHYS_VIRT=y by default and then 
> no CONFIG_PHYS_OFFSET would be presented.
> 
> So the right fix here would actually consist of adding to 
> at91x40_defconfig either CONFIG_ARM_PATCH_PHYS_VIRT=y, or a default 
> value for CONFIG_PHYS_OFFSET which is the right value for that platform.

And it seems that at91x40 is the only defconfig with MMU=n, so the only 
option is really to provide a default CONFIG_PHYS_OFFSET.  So what about 
this:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e084b7e981..a24672924f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -222,6 +222,7 @@ config NEED_MACH_MEMORY_H
 config PHYS_OFFSET
 	hex "Physical address of main memory"
 	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
+	default DRAM_BASE if !MMU
 	help
 	  Please provide the physical address corresponding to the
 	  location of main memory in your system.

Eventually the DRAM_BASE config option could simply be replaced by 
PHYS_OFFSET directly, but that's a larger change.


Nicolas

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Uwe Kleine-König]

Hello Nicolas,

On Fri, Dec 02, 2011 at 12:33:26AM -0500, Nicolas Pitre wrote:
> On Fri, 2 Dec 2011, Nicolas Pitre wrote:
> 
> > On Thu, 1 Dec 2011, Paul Gortmaker wrote:
> > 
> > > On Thu, Dec 1, 2011 at 9:19 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Thursday 01 December 2011, Paul Gortmaker wrote:
> > > >> Commit 1b9f95f8ade9efc2bd49f0e7b9dc61a038ac3eef introduced the new
> > > >> Kconfig for PHYS_OFFSET but it did not specify a default value.
> > > >>
> > > >> This has the undesired side effect of breaking an implictly used
> > > >> trick for updating defconfigs on the fly for routine buildtesting
> > > >> across all arch and all platforms, i.e.
> > > >>
> > > >> ? cp /path/to/somedefconfig .config ; yes "" | make oldconfig
> > > >>
> > > >
> > > > FWIW, a better way to achieve the above is using
> > > >
> > > > make KCONFIG_ALLCONFIG=/path/to/somedefconfig listnewconfig alldefconfig
> > > >
> > > > which does not have this problem.
> > > >
> > > > Your patch looks still ok, but you might want to update your scripts
> > > > to be more robust.
> > > 
> > > Thanks -- but on the other hand, the above use case does actually
> > > emulate exactly what happens when someone runs the
> > > "make oldconfig" and simply holds down the Enter key. (yes it
> > > happens probably way more than we want to know...)
> > > 
> > > So, I guess what I'm trying to say is that the above "script" is
> > > intentionally fragile.  :)  As a use case, we shouldn't break it.
> > 
> > Well... I took a while before replying because I'm also modifying things 
> > around this, but those changes are good for a merge window not a -rc 
> > period.
> > 
> > Still, I don't think your patch is good, even as a temporary measure.  
> > The fact is that we can't just pick a random default value since it is 
> > likely to be wrong for the majority of the boards as the right answer 
> > for this option is quite varied.  So the fact that the config system 
> > insists on your entering a proper value is actually a good thing.
> > 
> > In the at91x40_defconfig case, it has CONFIG_EMBEDDED=y.  If it was not 
> > selected then you'd get CONFIG_ARM_PATCH_PHYS_VIRT=y by default and then 
> > no CONFIG_PHYS_OFFSET would be presented.
> > 
> > So the right fix here would actually consist of adding to 
> > at91x40_defconfig either CONFIG_ARM_PATCH_PHYS_VIRT=y, or a default 
> > value for CONFIG_PHYS_OFFSET which is the right value for that platform.
> 
> And it seems that at91x40 is the only defconfig with MMU=n, so the only 
> option is really to provide a default CONFIG_PHYS_OFFSET.  So what about 
> this:
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e084b7e981..a24672924f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -222,6 +222,7 @@ config NEED_MACH_MEMORY_H
>  config PHYS_OFFSET
>  	hex "Physical address of main memory"
>  	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
> +	default DRAM_BASE if !MMU
>  	help
>  	  Please provide the physical address corresponding to the
>  	  location of main memory in your system.
> 
> Eventually the DRAM_BASE config option could simply be replaced by 
> PHYS_OFFSET directly, but that's a larger change.
I'm working on a MMU-less machine, too and had that change already in my
working copy. I was about to commit and it send out later today.

So: Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Paul Gortmaker]

On 11-12-02 04:21 AM, Uwe Kleine-K?nig wrote:
> Hello Nicolas,
> 
> On Fri, Dec 02, 2011 at 12:33:26AM -0500, Nicolas Pitre wrote:

[...]

>> And it seems that at91x40 is the only defconfig with MMU=n, so the only 
>> option is really to provide a default CONFIG_PHYS_OFFSET.  So what about 
>> this:
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index e084b7e981..a24672924f 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -222,6 +222,7 @@ config NEED_MACH_MEMORY_H
>>  config PHYS_OFFSET
>>  	hex "Physical address of main memory"
>>  	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
>> +	default DRAM_BASE if !MMU
>>  	help
>>  	  Please provide the physical address corresponding to the
>>  	  location of main memory in your system.
>>
>> Eventually the DRAM_BASE config option could simply be replaced by 
>> PHYS_OFFSET directly, but that's a larger change.
> I'm working on a MMU-less machine, too and had that change already in my
> working copy. I was about to commit and it send out later today.
> 
> So: Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Looks fine to me as well; I had no particular attachment to the
original patch, and you guys have a much better understanding of
the specifics of the system.

Thanks,
Paul.

> 
> Best regards
> Uwe
> 

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Uwe Kleine-König]

Hello,

On Fri, Dec 02, 2011 at 11:31:50AM -0500, Paul Gortmaker wrote:
> On 11-12-02 04:21 AM, Uwe Kleine-K?nig wrote:
> > Hello Nicolas,
> > 
> > On Fri, Dec 02, 2011 at 12:33:26AM -0500, Nicolas Pitre wrote:
> 
> [...]
> 
> >> And it seems that at91x40 is the only defconfig with MMU=n, so the only 
> >> option is really to provide a default CONFIG_PHYS_OFFSET.  So what about 
> >> this:
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index e084b7e981..a24672924f 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -222,6 +222,7 @@ config NEED_MACH_MEMORY_H
> >>  config PHYS_OFFSET
> >>  	hex "Physical address of main memory"
> >>  	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
> >> +	default DRAM_BASE if !MMU
> >>  	help
> >>  	  Please provide the physical address corresponding to the
> >>  	  location of main memory in your system.
> >>
> >> Eventually the DRAM_BASE config option could simply be replaced by 
> >> PHYS_OFFSET directly, but that's a larger change.
> > I'm working on a MMU-less machine, too and had that change already in my
> > working copy. I was about to commit and it send out later today.
> > 
> > So: Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> Looks fine to me as well; I had no particular attachment to the
> original patch, and you guys have a much better understanding of
> the specifics of the system.
A post-ack comment: would it make sense to make this:

	config PHYS_OFFSET
		hex "Physical address of main memory" if MMU
	...

such that !MMU build don't get a chance to misconfigure it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm: add default value for PHYS_OFFSET Kconfig setting

[Nicolas Pitre]

On Fri, 2 Dec 2011, Uwe Kleine-K?nig wrote:

> Hello,
> 
> On Fri, Dec 02, 2011 at 11:31:50AM -0500, Paul Gortmaker wrote:
> > On 11-12-02 04:21 AM, Uwe Kleine-K?nig wrote:
> > > Hello Nicolas,
> > > 
> > > On Fri, Dec 02, 2011 at 12:33:26AM -0500, Nicolas Pitre wrote:
> > 
> > [...]
> > 
> > >> And it seems that at91x40 is the only defconfig with MMU=n, so the only 
> > >> option is really to provide a default CONFIG_PHYS_OFFSET.  So what about 
> > >> this:
> > >>
> > >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > >> index e084b7e981..a24672924f 100644
> > >> --- a/arch/arm/Kconfig
> > >> +++ b/arch/arm/Kconfig
> > >> @@ -222,6 +222,7 @@ config NEED_MACH_MEMORY_H
> > >>  config PHYS_OFFSET
> > >>  	hex "Physical address of main memory"
> > >>  	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
> > >> +	default DRAM_BASE if !MMU
> > >>  	help
> > >>  	  Please provide the physical address corresponding to the
> > >>  	  location of main memory in your system.
> > >>
> > >> Eventually the DRAM_BASE config option could simply be replaced by 
> > >> PHYS_OFFSET directly, but that's a larger change.
> > > I'm working on a MMU-less machine, too and had that change already in my
> > > working copy. I was about to commit and it send out later today.
> > > 
> > > So: Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > 
> > Looks fine to me as well; I had no particular attachment to the
> > original patch, and you guys have a much better understanding of
> > the specifics of the system.
> A post-ack comment: would it make sense to make this:
> 
> 	config PHYS_OFFSET
> 		hex "Physical address of main memory" if MMU
> 	...
> 
> such that !MMU build don't get a chance to misconfigure it?

Sure.


Nicolas

[PATCH] ARM: determine PHYS_OFFSET automatically for no-mmu configs

[Uwe Kleine-König]

From: Nicolas Pitre <nicolas.pitre@linaro.org>

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/Kconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 44789ef..763de24 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -220,8 +220,9 @@ config NEED_MACH_MEMORY_H
 	  be avoided when possible.
 
 config PHYS_OFFSET
-	hex "Physical address of main memory"
+	hex "Physical address of main memory" if MMU
 	depends on !ARM_PATCH_PHYS_VIRT && !NEED_MACH_MEMORY_H
+	default DRAM_BASE if !MMU
 	help
 	  Please provide the physical address corresponding to the
 	  location of main memory in your system.
-- 
1.7.7.3

[PATCH] ARM: determine PHYS_OFFSET automatically for no-mmu configs

[Nicolas Pitre]

On Fri, 2 Dec 2011, Uwe Kleine-K?nig wrote:

> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

I submitted this to the patch system:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7186/1


Nicolas