From: Rob Herring <robh@kernel.org> To: Michael Ellerman <mpe@ellerman.id.au> Cc: John Crispin <john@phrozen.org>, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Frank Rowand <frowand.list@gmail.com>, "open list:MIPS" <linux-mips@vger.kernel.org>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, devicetree@vger.kernel.org Subject: Re: [PATCH v2] of/fdt: Rework early_init_dt_scan_memory() to call directly Date: Tue, 14 Dec 2021 08:40:02 -0600 [thread overview] Message-ID: <CAL_JsqKbaRgivZMxEj6Mjdny2LNeSA1GQyDW-nQe7E2irPc-Fw@mail.gmail.com> (raw) In-Reply-To: <877dc7mo3o.fsf@mpe.ellerman.id.au> On Tue, Dec 14, 2021 at 5:18 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Rob Herring <robh@kernel.org> writes: > > On Mon, Dec 13, 2021 at 6:47 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > >> Rob Herring <robh@kernel.org> writes: > >> > Use of the of_scan_flat_dt() function predates libfdt and is discouraged > >> > as libfdt provides a nicer set of APIs. Rework > >> > early_init_dt_scan_memory() to be called directly and use libfdt. > >> ... > >> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > >> > index 6e1a106f02eb..63762a3b75e8 100644 > >> > --- a/arch/powerpc/kernel/prom.c > >> > +++ b/arch/powerpc/kernel/prom.c > >> > @@ -532,19 +532,19 @@ static int __init early_init_drmem_lmb(struct drmem_lmb *lmb, > >> > } > >> > #endif /* CONFIG_PPC_PSERIES */ > >> > > >> > -static int __init early_init_dt_scan_memory_ppc(unsigned long node, > >> > - const char *uname, > >> > - int depth, void *data) > >> > +static int __init early_init_dt_scan_memory_ppc(void) > >> > { > >> > #ifdef CONFIG_PPC_PSERIES > >> > - if (depth == 1 && > >> > - strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) { > >> > + const void *fdt = initial_boot_params; > >> > + int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory"); > >> > + > >> > + if (node > 0) { > >> > walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb); > >> > return 0; > >> > } > > It's that return that is the problem. > > Now that early_init_dt_scan_memory_ppc() is only called once, that > return causes us to skip scanning regular memory nodes if there is an > "ibm,dynamic-reconfiguration-memory" property present. > > So the fix is just: > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 1098de3b172f..125661e5fcf3 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -538,10 +538,8 @@ static int __init early_init_dt_scan_memory_ppc(void) > const void *fdt = initial_boot_params; > int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory"); > > - if (node > 0) { > + if (node > 0) > walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb); > - return 0; > - } > #endif > > return early_init_dt_scan_memory(); Thanks! I've rolled that in. > > The only thing I see is now there is an assumption that 'memory' nodes > > are off the root node only. Before they could be anywhere. > > I don't know of any machines where that would be a problem. But given > all the wild and wonderful device trees out there, who really knows :) > > Maybe we should continue to allow memory nodes to be anywhere, and print > a warning for any that aren't at the root. Then if no one reports any > hits for the warning we could switch to only allowing them at the root? I really doubt there's any case. I just have the least visibility into what IBM DTs look like. I checked some old DT files I have and also u-boot only supports off the root node. Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org> To: Michael Ellerman <mpe@ellerman.id.au> Cc: devicetree@vger.kernel.org, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, "open list:MIPS" <linux-mips@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Paul Mackerras <paulus@samba.org>, John Crispin <john@phrozen.org>, Frank Rowand <frowand.list@gmail.com> Subject: Re: [PATCH v2] of/fdt: Rework early_init_dt_scan_memory() to call directly Date: Tue, 14 Dec 2021 08:40:02 -0600 [thread overview] Message-ID: <CAL_JsqKbaRgivZMxEj6Mjdny2LNeSA1GQyDW-nQe7E2irPc-Fw@mail.gmail.com> (raw) In-Reply-To: <877dc7mo3o.fsf@mpe.ellerman.id.au> On Tue, Dec 14, 2021 at 5:18 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Rob Herring <robh@kernel.org> writes: > > On Mon, Dec 13, 2021 at 6:47 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > >> Rob Herring <robh@kernel.org> writes: > >> > Use of the of_scan_flat_dt() function predates libfdt and is discouraged > >> > as libfdt provides a nicer set of APIs. Rework > >> > early_init_dt_scan_memory() to be called directly and use libfdt. > >> ... > >> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > >> > index 6e1a106f02eb..63762a3b75e8 100644 > >> > --- a/arch/powerpc/kernel/prom.c > >> > +++ b/arch/powerpc/kernel/prom.c > >> > @@ -532,19 +532,19 @@ static int __init early_init_drmem_lmb(struct drmem_lmb *lmb, > >> > } > >> > #endif /* CONFIG_PPC_PSERIES */ > >> > > >> > -static int __init early_init_dt_scan_memory_ppc(unsigned long node, > >> > - const char *uname, > >> > - int depth, void *data) > >> > +static int __init early_init_dt_scan_memory_ppc(void) > >> > { > >> > #ifdef CONFIG_PPC_PSERIES > >> > - if (depth == 1 && > >> > - strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) { > >> > + const void *fdt = initial_boot_params; > >> > + int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory"); > >> > + > >> > + if (node > 0) { > >> > walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb); > >> > return 0; > >> > } > > It's that return that is the problem. > > Now that early_init_dt_scan_memory_ppc() is only called once, that > return causes us to skip scanning regular memory nodes if there is an > "ibm,dynamic-reconfiguration-memory" property present. > > So the fix is just: > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 1098de3b172f..125661e5fcf3 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -538,10 +538,8 @@ static int __init early_init_dt_scan_memory_ppc(void) > const void *fdt = initial_boot_params; > int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory"); > > - if (node > 0) { > + if (node > 0) > walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb); > - return 0; > - } > #endif > > return early_init_dt_scan_memory(); Thanks! I've rolled that in. > > The only thing I see is now there is an assumption that 'memory' nodes > > are off the root node only. Before they could be anywhere. > > I don't know of any machines where that would be a problem. But given > all the wild and wonderful device trees out there, who really knows :) > > Maybe we should continue to allow memory nodes to be anywhere, and print > a warning for any that aren't at the root. Then if no one reports any > hits for the warning we could switch to only allowing them at the root? I really doubt there's any case. I just have the least visibility into what IBM DTs look like. I checked some old DT files I have and also u-boot only supports off the root node. Rob
next prev parent reply other threads:[~2021-12-14 14:40 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-08 15:58 [PATCH v2] of/fdt: Rework early_init_dt_scan_memory() to call directly Rob Herring 2021-12-08 15:58 ` Rob Herring 2021-12-11 0:18 ` Frank Rowand 2021-12-11 0:18 ` Frank Rowand 2021-12-13 12:46 ` Michael Ellerman 2021-12-13 12:46 ` Michael Ellerman 2021-12-13 18:18 ` Rob Herring 2021-12-13 18:18 ` Rob Herring 2021-12-14 11:18 ` Michael Ellerman 2021-12-14 11:18 ` Michael Ellerman 2021-12-14 14:40 ` Rob Herring [this message] 2021-12-14 14:40 ` Rob Herring 2021-12-15 10:25 ` Michael Ellerman 2021-12-15 10:25 ` Michael Ellerman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAL_JsqKbaRgivZMxEj6Mjdny2LNeSA1GQyDW-nQe7E2irPc-Fw@mail.gmail.com \ --to=robh@kernel.org \ --cc=benh@kernel.crashing.org \ --cc=devicetree@vger.kernel.org \ --cc=frowand.list@gmail.com \ --cc=john@phrozen.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ --cc=tsbogend@alpha.franken.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.