[patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Cyrill Gorcunov]

This patch series removes code duplication for PAGE_SIZE
definition on various architectures.

Please review. Any comments are welcome. *UNTESTED*

Statistics:
 asm-avr32/page.h     |    8 +++-----
 asm-blackfin/page.h  |    8 +++-----
 asm-frv/mem-layout.h |   10 +++-------
 asm-sparc/page.h     |   11 ++++-------

4 files changed, 13 insertions(+), 24 deletions(-)

		- Cyrill -
-- 

Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Sam Ravnborg]

On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
> This patch series removes code duplication for PAGE_SIZE
> definition on various architectures.
> 
> Please review. Any comments are welcome. *UNTESTED*

Looks good.
Could you do a followup patch for each architecture introducing
use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
I think more than your 4 listed architectures could need this fix.

	Sam

Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Cyrill Gorcunov]

[Sam Ravnborg - Sun, Feb 03, 2008 at 01:22:39PM +0100]
| On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
| > This patch series removes code duplication for PAGE_SIZE
| > definition on various architectures.
| > 
| > Please review. Any comments are welcome. *UNTESTED*
| 
| Looks good.
| Could you do a followup patch for each architecture introducing
| use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
| I think more than your 4 listed architectures could need this fix.
| 
| 	Sam
| 

Hi, Sam,

i'll try but a bit lately, ok?

		- Cyrill -

Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Cyrill Gorcunov]

[Sam Ravnborg - Sun, Feb 03, 2008 at 01:22:39PM +0100]
| On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
| > This patch series removes code duplication for PAGE_SIZE
| > definition on various architectures.
| > 
| > Please review. Any comments are welcome. *UNTESTED*
| 
| Looks good.
| Could you do a followup patch for each architecture introducing
| use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
| I think more than your 4 listed architectures could need this fix.
| 
| 	Sam
| 

a bit *later* I mean (was a misprint ;)

		- Cyrill -

Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Mike Frysinger]

On Feb 3, 2008 7:22 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
> > This patch series removes code duplication for PAGE_SIZE
> > definition on various architectures.
> >
> > Please review. Any comments are welcome. *UNTESTED*
>
> Looks good.
> Could you do a followup patch for each architecture introducing
> use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
> I think more than your 4 listed architectures could need this fix.

what did you have in mind exactly ?  afaik, the Blackfin linker script
should already be friendly wrt using defines instead of hardcoded
constants ...
-mike

Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Sam Ravnborg]

On Mon, Feb 04, 2008 at 03:01:30PM -0500, Mike Frysinger wrote:
> On Feb 3, 2008 7:22 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> > On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
> > > This patch series removes code duplication for PAGE_SIZE
> > > definition on various architectures.
> > >
> > > Please review. Any comments are welcome. *UNTESTED*
> >
> > Looks good.
> > Could you do a followup patch for each architecture introducing
> > use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
> > I think more than your 4 listed architectures could need this fix.
> 
> what did you have in mind exactly ?  afaik, the Blackfin linker script
> should already be friendly wrt using defines instead of hardcoded
> constants ...
Blackfin look ok.
Lets take following code snippet from blackfin:
       .init.text :
        {
                . = ALIGN(PAGE_SIZE);
                __sinittext = .;
                INIT_TEXT
                __einittext = .;
        }


And a grep quicly reveals:

m32r/kernel/vmlinux.lds.S-  . = ALIGN(4096);            /* Init code and data */
m32r/kernel/vmlinux.lds.S-  __init_begin = .;
m32r/kernel/vmlinux.lds.S-  .init.text : {
m32r/kernel/vmlinux.lds.S-      _sinittext = .;
m32r/kernel/vmlinux.lds.S:      INIT_TEXT
m32r/kernel/vmlinux.lds.S-      _einittext = .;
m32r/kernel/vmlinux.lds.S-  }
m32r/kernel/vmlinux.lds.S-  .init.data : { INIT_DATA }
m32r/kernel/vmlinux.lds.S-  . = ALIGN(16);
m32r/kernel/vmlinux.lds.S-  __setup_start = .;
--
m68knommu/kernel/vmlinux.lds.S-
m68knommu/kernel/vmlinux.lds.S- .init : {
m68knommu/kernel/vmlinux.lds.S-         . = ALIGN(4096);
m68knommu/kernel/vmlinux.lds.S-         __init_begin = .;
m68knommu/kernel/vmlinux.lds.S-         _sinittext = .;
m68knommu/kernel/vmlinux.lds.S:         INIT_TEXT
m68knommu/kernel/vmlinux.lds.S-         _einittext = .;

It is maybe common knowledge that 4096 is equal to PAGE_SIZE
but it is more descriptive to use a constant.

As for sparc64 where we fixed this recently it was a bug since the
page size are configuration dependent.

So I just ask to use appropriate constants when they are
already available and visible from the vmlinux.lds.S files.

Consistent indenting in all vmlinux.lds.S files would also
be good - but I am still missing a third of them.
Blackfin looks good also in this respect.

	Sam