2018-10-25 00:18:35

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v3 0/3] of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()

Hi all,

Based on Rob's suggestion, this is a follow-up to the patch series
posted here previously that aims at cutting the ARM64 rebuild
time/number of objects. Rob indicated that he was adamant to just having
drivers/of/fdt.c contain the ARM64 specific behavior, so this patch
series does exactly that.

https://www.spinics.net/lists/devicetree/msg255512.html

Florian Fainelli (3):
of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()
arm64: Remove custom __early_init_dt_declare_initrd implenentation
of/fdt: Remove definition check for __early_init_dt_declare_initrd

arch/arm64/include/asm/memory.h | 8 --------
drivers/of/fdt.c | 7 +++++--
2 files changed, 5 insertions(+), 10 deletions(-)

--
2.17.1



2018-10-25 00:18:37

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v3 2/3] arm64: Remove custom __early_init_dt_declare_initrd implenentation

Now that drivers/of/fdt.c includes the proper implementation for ARM64,
we can get rid of our custom __early_init_dt_declare_initrd()
implementation.

Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm64/include/asm/memory.h | 8 --------
1 file changed, 8 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
#define IOREMAP_MAX_ORDER (PMD_SHIFT)
#endif

-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end) \
- do { \
- initrd_start = (__start); \
- initrd_end = (__end); \
- } while (0)
-#endif
-
#ifndef __ASSEMBLY__

#include <linux/bitops.h>
--
2.17.1


2018-10-25 00:18:39

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v3 3/3] of/fdt: Remove definition check for __early_init_dt_declare_initrd

With ARM64 no longer providing a custom __early_init_dt_declare_initrd()
in its headers, and no other architecture doing something similar,
remove the check for __early_init_dt_declare_initrd being already
defined since we now have the one and only definition for it.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/of/fdt.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 7d316f008f22..2e962d75dbce 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -892,7 +892,6 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
}

#ifdef CONFIG_BLK_DEV_INITRD
-#ifndef __early_init_dt_declare_initrd
static void __early_init_dt_declare_initrd(unsigned long start,
unsigned long end)
{
@@ -905,7 +904,6 @@ static void __early_init_dt_declare_initrd(unsigned long start,
initrd_below_start_ok = 1;
#endif
}
-#endif

/**
* early_init_dt_check_for_initrd - Decode initrd location from flat tree
--
2.17.1


2018-10-25 00:20:00

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v3 1/3] of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()

ARM64 is the only architecture that requires a re-definition of
__early_init_dt_declare_initrd(), absorb its custom implemention in
drivers/of/fdt.c.

Suggested-by: Rob Herring <[email protected].
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/of/fdt.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252cf9c..7d316f008f22 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -896,9 +896,14 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
static void __early_init_dt_declare_initrd(unsigned long start,
unsigned long end)
{
+#if IS_ENABLED(CONFIG_ARM64)
+ initrd_start = start;
+ initrd_end = end;
+#else
initrd_start = (unsigned long)__va(start);
initrd_end = (unsigned long)__va(end);
initrd_below_start_ok = 1;
+#endif
}
#endif

--
2.17.1


2018-10-25 13:29:10

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()

On Wed, Oct 24, 2018 at 7:17 PM Florian Fainelli <[email protected]> wrote:
>
> ARM64 is the only architecture that requires a re-definition of
> __early_init_dt_declare_initrd(), absorb its custom implemention in
> drivers/of/fdt.c.
>
> Suggested-by: Rob Herring <[email protected].

You forgot a shift key. :)

> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/of/fdt.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 800ad252cf9c..7d316f008f22 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -896,9 +896,14 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
> static void __early_init_dt_declare_initrd(unsigned long start,
> unsigned long end)
> {
> +#if IS_ENABLED(CONFIG_ARM64)

C code, not preprocessor please.

> + initrd_start = start;
> + initrd_end = end;

Thinking some more about this, perhaps it is better to just add the
*_phys variants now along side the VA variants and set them here. Then
the arm64 code can override the initrd_start, initrd_end, and
initrd_below_start_ok values.

> +#else
> initrd_start = (unsigned long)__va(start);
> initrd_end = (unsigned long)__va(end);
> initrd_below_start_ok = 1;
> +#endif
> }
> #endif
>
> --
> 2.17.1
>

2018-10-25 14:08:13

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()

On Thu, Oct 25, 2018 at 08:25:04AM -0500, Rob Herring wrote:
> On Wed, Oct 24, 2018 at 7:17 PM Florian Fainelli <[email protected]> wrote:
> >
> > ARM64 is the only architecture that requires a re-definition of
> > __early_init_dt_declare_initrd(), absorb its custom implemention in
> > drivers/of/fdt.c.
> >
> > Suggested-by: Rob Herring <[email protected].
>
> You forgot a shift key. :)
>
> > Signed-off-by: Florian Fainelli <[email protected]>
> > ---
> > drivers/of/fdt.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 800ad252cf9c..7d316f008f22 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -896,9 +896,14 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
> > static void __early_init_dt_declare_initrd(unsigned long start,
> > unsigned long end)
> > {
> > +#if IS_ENABLED(CONFIG_ARM64)
>
> C code, not preprocessor please.
>
> > + initrd_start = start;
> > + initrd_end = end;
>
> Thinking some more about this, perhaps it is better to just add the
> *_phys variants now along side the VA variants and set them here. Then
> the arm64 code can override the initrd_start, initrd_end, and
> initrd_below_start_ok values.

Please, let's not make the age old mistake of inventing new symbols
for stuff that already exists:

$ grep phys_initrd_start arch/ -rl
arch/arm/mm/init.c
arch/nds32/mm/init.c
arch/unicore32/mm/init.c
$ grep initrd_start_phys arch/ -rl
$

Please use the "phys_initrd_start" and "phys_initrd_end" naming,
which already exist on some architectures rather than inventing a new
set of symbols for the same thing and then forcing arches to change.
We could then get rid of:

/* FDT scan will populate initrd_start */
if (initrd_start && !phys_initrd_size) {
phys_initrd_start = __virt_to_phys(initrd_start);
phys_initrd_size = initrd_end - initrd_start;
}

initrd_start = initrd_end = 0;

in ARM, which exists purely to cope with DT.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

2018-10-25 14:34:27

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] of/fdt: Absorb ARM64's __early_init_dt_declare_initrd()

On Thu, Oct 25, 2018 at 9:06 AM Russell King - ARM Linux
<[email protected]> wrote:
>
> On Thu, Oct 25, 2018 at 08:25:04AM -0500, Rob Herring wrote:
> > On Wed, Oct 24, 2018 at 7:17 PM Florian Fainelli <[email protected]> wrote:
> > >
> > > ARM64 is the only architecture that requires a re-definition of
> > > __early_init_dt_declare_initrd(), absorb its custom implemention in
> > > drivers/of/fdt.c.
> > >
> > > Suggested-by: Rob Herring <[email protected].
> >
> > You forgot a shift key. :)
> >
> > > Signed-off-by: Florian Fainelli <[email protected]>
> > > ---
> > > drivers/of/fdt.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index 800ad252cf9c..7d316f008f22 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -896,9 +896,14 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
> > > static void __early_init_dt_declare_initrd(unsigned long start,
> > > unsigned long end)
> > > {
> > > +#if IS_ENABLED(CONFIG_ARM64)
> >
> > C code, not preprocessor please.
> >
> > > + initrd_start = start;
> > > + initrd_end = end;
> >
> > Thinking some more about this, perhaps it is better to just add the
> > *_phys variants now along side the VA variants and set them here. Then
> > the arm64 code can override the initrd_start, initrd_end, and
> > initrd_below_start_ok values.
>
> Please, let's not make the age old mistake of inventing new symbols
> for stuff that already exists:
>
> $ grep phys_initrd_start arch/ -rl
> arch/arm/mm/init.c
> arch/nds32/mm/init.c
> arch/unicore32/mm/init.c
> $ grep initrd_start_phys arch/ -rl
> $
>
> Please use the "phys_initrd_start" and "phys_initrd_end" naming,

Yes, of course. That is what I intended. I assumed Florian would be
aware of it from your other email.

> which already exist on some architectures rather than inventing a new
> set of symbols for the same thing and then forcing arches to change.
> We could then get rid of:
>
> /* FDT scan will populate initrd_start */
> if (initrd_start && !phys_initrd_size) {
> phys_initrd_start = __virt_to_phys(initrd_start);
> phys_initrd_size = initrd_end - initrd_start;
> }
>
> initrd_start = initrd_end = 0;
>
> in ARM, which exists purely to cope with DT.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up