| Message ID | 20180905193738.19325-1-robh@kernel.org |
|---|---|
| Headers | show |
| Series | DT cpu node iterator | expand |
Quoting Rob Herring (2018-09-05 12:37:34) > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: linux-clk@vger.kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- Acked-by: Stephen Boyd <sboyd@kernel.org>
On Wed, 5 Sep 2018, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: x86@kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
On Wed, Sep 05, 2018 at 02:37:35PM -0500, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Borislav Petkov <bp@alien8.de> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: linux-edac@vger.kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Please ack and I will take via the DT tree. This is dependent on the > first 2 patches. Completely unknown territory for me so I'd trust your judgement. Staring at 1/21, the conversion looks ok except the removal of those prints that a cpu nodes are not present - I wonder if they even meant anything or were just there during driver development... > drivers/edac/cpc925_edac.c | 20 ++------------------ > 1 file changed, 2 insertions(+), 18 deletions(-) > > diff --git a/drivers/edac/cpc925_edac.c b/drivers/edac/cpc925_edac.c > index 2c98e020df05..3c0881ac9880 100644 > --- a/drivers/edac/cpc925_edac.c > +++ b/drivers/edac/cpc925_edac.c > @@ -593,8 +593,7 @@ static void cpc925_mc_check(struct mem_ctl_info *mci) > /******************** CPU err device********************************/ > static u32 cpc925_cpu_mask_disabled(void) > { > - struct device_node *cpus; > - struct device_node *cpunode = NULL; > + struct device_node *cpunode; > static u32 mask = 0; > > /* use cached value if available */ > @@ -603,20 +602,8 @@ static u32 cpc925_cpu_mask_disabled(void) > > mask = APIMASK_ADI0 | APIMASK_ADI1; > > - cpus = of_find_node_by_path("/cpus"); > - if (cpus == NULL) { > - cpc925_printk(KERN_DEBUG, "No /cpus node !\n"); This thing... > - return 0; > - } > - > - while ((cpunode = of_get_next_child(cpus, cpunode)) != NULL) { > + for_each_of_cpu_node(cpunode) { > const u32 *reg = of_get_property(cpunode, "reg", NULL); > - > - if (strcmp(cpunode->type, "cpu")) { > - cpc925_printk(KERN_ERR, "Not a cpu node in /cpus: %s\n", cpunode->name); ... and this thing. Thx.
On Wed, Sep 5, 2018 at 9:38 PM Rob Herring <robh@kernel.org> wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Simon Horman <horms@verge.net.au> > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-renesas-soc@vger.kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Wed, Sep 05, 2018 at 02:37:22PM -0500, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Simon Horman <horms@verge.net.au> > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-renesas-soc@vger.kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
On Wed, Sep 05, 2018 at 02:37:27PM -0500, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > This also fixes a leaked reference for cpus node. > > Cc: Jonas Bonn <jonas@southpole.se> > Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> > Cc: Stafford Horne <shorne@gmail.com> > Cc: openrisc@lists.librecores.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Please ack and I will take via the DT tree. This is dependent on the > first 2 patches. > > arch/openrisc/kernel/setup.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c > index 9d28ab14d139..e17fcd83120f 100644 > --- a/arch/openrisc/kernel/setup.c > +++ b/arch/openrisc/kernel/setup.c > @@ -158,9 +158,8 @@ static struct device_node *setup_find_cpu_node(int cpu) > { > u32 hwid; > struct device_node *cpun; > - struct device_node *cpus = of_find_node_by_path("/cpus"); > > - for_each_available_child_of_node(cpus, cpun) { > + for_each_of_cpu_node(cpun) { > if (of_property_read_u32(cpun, "reg", &hwid)) > continue; > if (hwid == cpu) This looks fine to me. Thanks Rob Acked-by: Stafford Horne <shorne@gmail.com>
On Wed, Sep 05, 2018 at 02:37:23PM -0500, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Please ack and I will take via the DT tree. This is dependent on the > first 2 patches. Acked-by: Will Deacon <will.deacon@arm.com> Will
On Thu, Sep 6, 2018 at 3:35 AM Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Sep 05, 2018 at 02:37:35PM -0500, Rob Herring wrote: > > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > > has the side effect of defaulting to iterating using "cpu" node names in > > preference to the deprecated (for FDT) device_type == "cpu". > > > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > > Cc: linux-edac@vger.kernel.org > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > Please ack and I will take via the DT tree. This is dependent on the > > first 2 patches. > > Completely unknown territory for me so I'd trust your judgement. Staring > at 1/21, the conversion looks ok except the removal of those prints that > a cpu nodes are not present - I wonder if they even meant anything or > were just there during driver development... I should have noted this. It's not the kernel's job to validate the DT and certainly not some driver's job to validate cpu nodes. It's bad enough that some random driver is parsing cpu nodes. If they are missing or are crap, you should get warnings or messages well before this point. Rob
On Thu, Sep 06, 2018 at 06:12:51AM -0500, Rob Herring wrote: > I should have noted this. It's not the kernel's job to validate the DT > and certainly not some driver's job to validate cpu nodes. It's bad > enough that some random driver is parsing cpu nodes. If they are > missing or are crap, you should get warnings or messages well before > this point. That is useful info for the commit message, I'd say. In any case: Acked-by: Borislav Petkov <bp@suse.de>
On Fri, Sep 7, 2018 at 7:54 AM Michal Simek <monstr@monstr.eu> wrote: > > Hi Rob, > > 2018-09-05 21:37 GMT+02:00 Rob Herring <robh@kernel.org>: >> >> This series adds an iterator for cpu nodes and converts users over to use >> it or of_get_cpu_node in some cases. This allows us to remove the >> dependency on device_type property for cpu nodes though removing that >> from DTS files will have to wait for some time. In some cases, this makes >> the DT search more strict by only looking in /cpus child nodes rather >> than any node with the device_type == cpu. The iterator also honors the >> status property which is often forgotten. >> >> I've only tested on ARM under QEMU and compiled powerpc. > > > > Do you have this somewhere in your tree not to apply 21 patches by hand? Yes, dt/cpu-type branch on my kernel.org tree. Rob
On Wed, Sep 05, 2018 at 02:37:31PM -0500, Rob Herring wrote: > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > has the side effect of defaulting to iterating using "cpu" node names in > preference to the deprecated (for FDT) device_type == "cpu". > > Cc: Palmer Dabbelt <palmer@sifive.com> > Cc: Albert Ou <aou@eecs.berkeley.edu> > Cc: linux-riscv@lists.infradead.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Please ack and I will take via the DT tree. This is dependent on the > first 2 patches. We have a few pending patches in this area that look like they might conflict. It might be worth to delay this one and apply it post -rc1 to avoid dependencies if that isn't a problem for you.
On Mon, Sep 10, 2018 at 8:38 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Wed, Sep 05, 2018 at 02:37:31PM -0500, Rob Herring wrote: > > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This > > has the side effect of defaulting to iterating using "cpu" node names in > > preference to the deprecated (for FDT) device_type == "cpu". > > > > Cc: Palmer Dabbelt <palmer@sifive.com> > > Cc: Albert Ou <aou@eecs.berkeley.edu> > > Cc: linux-riscv@lists.infradead.org > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > Please ack and I will take via the DT tree. This is dependent on the > > first 2 patches. > > We have a few pending patches in this area that look like they might > conflict. It might be worth to delay this one and apply it post -rc1 > to avoid dependencies if that isn't a problem for you. Sure. That's fine. Rob
On 5.9.2018 21:37, Rob Herring wrote: > "device_type" use is deprecated for FDT though it has continued to be used > for nodes like cpu nodes. Use of_get_cpu_node() instead which works using > node names by default. This will allow the eventually removal of cpu > device_type properties. > > Also, fix a leaked reference by adding a missing of_node_put. > > Cc: Michal Simek <monstr@monstr.eu> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Please ack and I will take via the DT tree. This is dependent on the > first 2 patches. I have tested it and it is align with the spec and all dtses generated before 2015 will work without any issue. In 2015 new device tree generator was introduced and it is not adding reg property to cpu node which is required by this change. This will be fixed but that means that all generated dtses from 2015 are affected. That's why will be great if you can also change that pr_err message to mentioned to also check reg property to give users a chance to fix it properly. Error log below. Anyway here is my Tested-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal [ 0.000000] Ramdisk addr 0x00000000, [ 0.000000] Compiled-in FDT at (ptrval) [ 0.000000] Linux version 4.19.0-rc2-00010-gf9a96b0ac503 (monstr@monstr-desktop2) (gcc version 4.9.2 (crosstool-NG 1.20.0)) #4 Mon Sep 10 16:31:14 CEST 2018 [ 0.000000] setup_memory: max_mapnr: 0x40000 [ 0.000000] setup_memory: min_low_pfn: 0x80000 [ 0.000000] setup_memory: max_low_pfn: 0xb0000 [ 0.000000] setup_memory: max_pfn: 0xc0000 [ 0.000000] Zone ranges: [ 0.000000] DMA [mem 0x0000000080000000-0x00000000afffffff] [ 0.000000] Normal empty [ 0.000000] HighMem [mem 0x00000000b0000000-0x00000000bfffffff] [ 0.000000] Movable zone start for each node [ 0.000000] Early memory node ranges [ 0.000000] node 0: [mem 0x0000000080000000-0x00000000bfffffff] [ 0.000000] Initmem setup node 0 [mem 0x0000000080000000-0x00000000bfffffff] [ 0.000000] earlycon: ns16550a0 at MMIO 0x44a01000 (options '') [ 0.000000] bootconsole [ns16550a0] enabled [ 0.000000] You don't have cpu!!! [ 0.000000] setup_cpuinfo: initialising [ 0.000000] setup_cpuinfo: Using full CPU PVR support [ 0.000000] ERROR: Microblaze BARREL, MSR, PCMP or DIV-different for PVR and DTS [ 0.000000] ERROR: Microblaze HW_MUL-different for PVR and DTS [ 0.000000] wt_msr_noirq [ 0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 [ 0.000000] pcpu-alloc: [0] 0 [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 260608 [ 0.000000] Kernel command line: console=ttyS0,115200 earlycon [ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) [ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes) [ 0.000000] Memory: 1027496K/1048576K available (4270K kernel code, 153K rwdata, 1256K rodata, 4989K init, 561K bss, 21080K reserved, 0K cma-reserved, 262144K highmem) [ 0.000000] Kernel virtual memory layout: [ 0.000000] * 0xfffea000..0xfffff000 : fixmap [ 0.000000] * 0xff800000..0xffc00000 : highmem PTEs [ 0.000000] * 0xff7ff000..0xff800000 : early ioremap [ 0.000000] * 0xf0000000..0xff7ff000 : vmalloc & ioremap [ 0.000000] NR_IRQS: 33 [ 0.000000] irq-xilinx: /amba_pl/interrupt-controller@41200000: num_irq=6, edge=0x0 [ 0.000000] Oops: kernel access of bad area, sig: 11 [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc2-00010-gf9a96b0ac503 #4 [ 0.000000] Registers dump: mode=80571E90 [ 0.000000] r1=C028BD54, r2=C056FAB6, r3=00000000, r4=00000010 [ 0.000000] r5=00000000, r6=C0495348, r7=C0495350, r8=00000000 [ 0.000000] r9=C0571F44, r10=EF002400, r11=00000030, r12=00000000 [ 0.000000] r13=410C2FC0, r14=C0496688, r15=C02831F0, r16=00000000 [ 0.000000] r17=C02831EC, r18=FFFFFFFF, r19=C05BABE0, r20=BFFEC168 [ 0.000000] r21=BFFEC168, r22=EF7F9A80, r23=00000000, r24=00000000 [ 0.000000] r25=BFE6B84C, r26=80000000, r27=00000001, r28=90000040 [ 0.000000] r29=01000000, r30=00000380, r31=C05782E8, rPC=C02831EC [ 0.000000] msr=000046A0, ear=0000000C, esr=00001A72, fsr=000065A0 [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! [ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
On Mon, Sep 10, 2018 at 9:56 AM Michal Simek <michal.simek@xilinx.com> wrote: > > On 5.9.2018 21:37, Rob Herring wrote: > > "device_type" use is deprecated for FDT though it has continued to be used > > for nodes like cpu nodes. Use of_get_cpu_node() instead which works using > > node names by default. This will allow the eventually removal of cpu > > device_type properties. > > > > Also, fix a leaked reference by adding a missing of_node_put. > > > > Cc: Michal Simek <monstr@monstr.eu> > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > Please ack and I will take via the DT tree. This is dependent on the > > first 2 patches. > > I have tested it and it is align with the spec and all dtses generated > before 2015 will work without any issue. > In 2015 new device tree generator was introduced and it is not adding > reg property to cpu node which is required by this change. > This will be fixed but that means that all generated dtses from 2015 are > affected. Patch 2 was supposed to handle that case. However, it does expect that there should be an #address-cells equal to 0 in that case. Is that not a valid assumption? > That's why will be great if you can also change that pr_err message to > mentioned to also check reg property to give users a chance to fix it > properly. Error log below. I don't think breaking users is good. I could make this a find by path (/cpus/cpu) instead or just drop it for microblaze. It doesn't really affect my plans for removing device_node.type ptr. Rob > Anyway here is my > Tested-by: Michal Simek <michal.simek@xilinx.com> > > Thanks, > Michal
On 9/5/18 2:37 PM, Rob Herring wrote: > #ifdef CONFIG_NOT_COHERENT_CACHE > - np = of_find_node_by_type(NULL, "cpu"); > + np = of_get_cpu_node(0, NULL); This #ifdef means that it's only compiled on an MPC5121, which is a very dead platform. of_get_cpu_node() looks okay to me, but I'm going to have to assume that you know what you're doing. Acked-by: Timur Tabi <timur@kernel.org>
On 10.9.2018 22:49, Rob Herring wrote: > On Mon, Sep 10, 2018 at 9:56 AM Michal Simek <michal.simek@xilinx.com> wrote: >> >> On 5.9.2018 21:37, Rob Herring wrote: >>> "device_type" use is deprecated for FDT though it has continued to be used >>> for nodes like cpu nodes. Use of_get_cpu_node() instead which works using >>> node names by default. This will allow the eventually removal of cpu >>> device_type properties. >>> >>> Also, fix a leaked reference by adding a missing of_node_put. >>> >>> Cc: Michal Simek <monstr@monstr.eu> >>> Signed-off-by: Rob Herring <robh@kernel.org> >>> --- >>> Please ack and I will take via the DT tree. This is dependent on the >>> first 2 patches. >> >> I have tested it and it is align with the spec and all dtses generated >> before 2015 will work without any issue. >> In 2015 new device tree generator was introduced and it is not adding >> reg property to cpu node which is required by this change. >> This will be fixed but that means that all generated dtses from 2015 are >> affected. > > Patch 2 was supposed to handle that case. However, it does expect that > there should be an #address-cells equal to 0 in that case. Is that not > a valid assumption? as you can expect we have #address-cells = <0x1>; #cpus = <0x1>; #size-cells = <0x0>; cpu@0 { /* no reg property */ }; That missing reg property was even reported by dtc but none has fixed that. This will be fixed for xilinx releases. >> That's why will be great if you can also change that pr_err message to >> mentioned to also check reg property to give users a chance to fix it >> properly. Error log below. > > I don't think breaking users is good. > > I could make this a find by path (/cpus/cpu) instead or just drop it > for microblaze. It doesn't really affect my plans for removing > device_node.type ptr. If this is fine for you that will be the best solution. Thanks, Michal