Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
Hi Uwe, On Tue, Sep 17, 2019 at 2:25 PM Uwe Kleine-König wrote: > On Tue, Sep 17, 2019 at 11:25:46AM +, Peter Rosin wrote: > > On 2019-09-17 12:13, Uwe Kleine-König wrote: > > > On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote: > > >> On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: > > >>> On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= > > >>> wrote: > > Referencing device tree nodes from a property allows to pass arguments. > > This is for example used for referencing gpios. This looks as follows: > > > > gpio_ctrl: gpio-controller { > > #gpio-cells = <2> > > ... > > } > > > > someothernode { > > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > > ... > > } > > > > To know the number of arguments this must be either fixed, or the > > referenced node is checked for a $cells_name (here: "#gpio-cells") > > property and with this information the start of the second reference > > can > > be determined. > > > > Currently regulators are referenced with no additional arguments. To > > allow some optional arguments without having to change all referenced > > nodes this change introduces a way to specify a default cell_count. So > > when a phandle is parsed we check for the $cells_name property and use > > it as before if present. If it is not present we fall back to > > cells_count if non-negative and only fail if cells_count is smaller > > than > > zero. > > > > Signed-off-by: Uwe Kleine-König > > >> > > >> This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle > > >> fallback > > >> to non-negative cell_count") in robh/for-next, which causes a lock-up > > >> when > > >> booting a shmobile_defconfig kernel on r8a7791/koelsch: > > >> > > >> rcu: INFO: rcu_sched self-detected stall on CPU > Oh yeah, you're right. I'm a bit disappointed that I didn't spot this > myself :-| > > Untested patch to fix this problem: > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 2f25d2dfecfa..26f7a21d7187 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -1284,6 +1284,13 @@ int of_phandle_iterator_init(struct > of_phandle_iterator *it, > const __be32 *list; > int size; > > + /* > +* one of cell_count or cells_name must be provided to determine the > +* argument length. > +*/ > + if (cell_count < 0 && !cells_name) > + return -EINVAL; > + > memset(it, 0, sizeof(*it)); > > list = of_get_property(np, list_name, &size); > @@ -1765,6 +1772,18 @@ int of_count_phandle_with_args(const struct > device_node *np, const char *list_na > struct of_phandle_iterator it; > int rc, cur_index = 0; > > + /* If cells_name is NULL we assume an cell_count of 0 */ a cell count > + if (cells_name == NULL) { > + const __be32 *list; > + int size; > + > + list = of_get_property(np, list_name, &size); > + if (!list) > + return -ENOENT; > + > + return size / sizeof(*list); > + } > + > rc = of_phandle_iterator_init(&it, np, list_name, cells_name, -1); > if (rc) > return rc; Thanks, that fixes the boot for me! Tested-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
Hi Uwe, On 17.09.2019 14:25, Uwe Kleine-König wrote: > On Tue, Sep 17, 2019 at 11:25:46AM +, Peter Rosin wrote: >> On 2019-09-17 12:13, Uwe Kleine-König wrote: >>> Hello Geert, >>> >>> On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote: Hi Rob, Uwe, On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: > On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= >wrote: >> Referencing device tree nodes from a property allows to pass arguments. >> This is for example used for referencing gpios. This looks as follows: >> >>gpio_ctrl: gpio-controller { >>#gpio-cells = <2> >>... >>} >> >>someothernode { >>gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; >>... >>} >> >> To know the number of arguments this must be either fixed, or the >> referenced node is checked for a $cells_name (here: "#gpio-cells") >> property and with this information the start of the second reference can >> be determined. >> >> Currently regulators are referenced with no additional arguments. To >> allow some optional arguments without having to change all referenced >> nodes this change introduces a way to specify a default cell_count. So >> when a phandle is parsed we check for the $cells_name property and use >> it as before if present. If it is not present we fall back to >> cells_count if non-negative and only fail if cells_count is smaller than >> zero. >> >> Signed-off-by: Uwe Kleine-König This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback to non-negative cell_count") in robh/for-next, which causes a lock-up when booting a shmobile_defconfig kernel on r8a7791/koelsch: rcu: INFO: rcu_sched self-detected stall on CPU rcu: 0-: (2099 ticks this GP) idle=6fe/1/0x4002 softirq=29/29 fqs=1050 (t=2100 jiffies g=-1131 q=0) NMI backtrace for cpu 0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc2-shmobile-00050-ge42ee61017f58cd9 #376 Hardware name: Generic R-Car Gen2 (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x7c/0x9c) [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0xb8) [] (nmi_cpu_backtrace) from [] (nmi_trigger_cpumask_backtrace+0x84/0x114) [] (nmi_trigger_cpumask_backtrace) from [] (rcu_dump_cpu_stacks+0xac/0xc8) [] (rcu_dump_cpu_stacks) from [] (rcu_sched_clock_irq+0x2ac/0x6b4) [] (rcu_sched_clock_irq) from [] (update_process_times+0x30/0x5c) [] (update_process_times) from [] (tick_nohz_handler+0xcc/0x120) [] (tick_nohz_handler) from [] (arch_timer_handler_virt+0x28/0x30) [] (arch_timer_handler_virt) from [] (handle_percpu_devid_irq+0xe8/0x21c) [] (handle_percpu_devid_irq) from [] (generic_handle_irq+0x18/0x28) [] (generic_handle_irq) from [] (__handle_domain_irq+0xa0/0xb4) [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x90) [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) Exception stack(0xeb08dd30 to 0xeb08dd78) dd20: c0cc7514 2013 0005 3b27 dd40: eb7c4020 c0cc750c 0051 0051 2013 c0c66b08 eb1cdc00 0018 dd60: eb08dd80 c05c1a38 c0756c00 2013 [] (__irq_svc) from [] (_raw_spin_unlock_irqrestore+0x1c/0x20) [] (_raw_spin_unlock_irqrestore) from [] (of_find_node_by_phandle+0xcc/0xf0) [] (of_find_node_by_phandle) from [] (of_phandle_iterator_next+0x68/0x178) [] (of_phandle_iterator_next) from [] (of_count_phandle_with_args+0x5c/0x7c) [] (of_count_phandle_with_args) from [] (i2c_demux_pinctrl_probe+0x24/0x1fc) [] (i2c_demux_pinctrl_probe) from [] (platform_drv_probe+0x48/0x94) [] (platform_drv_probe) from [] (really_probe+0x1f0/0x2b8) [] (really_probe) from [] (driver_probe_device+0x140/0x158) [] (driver_probe_device) from [] (device_driver_attach+0x44/0x5c) [] (device_driver_attach) from [] (__driver_attach+0xac/0xb4) [] (__driver_attach) from [] (bus_for_each_dev+0x64/0xa0) [] (bus_for_each_dev) from [] (bus_add_driver+0x148/0x1a8) [] (bus_add_driver) from [] (driver_register+0xac/0xf0) [] (driver_register) from [] (do_one_initcall+0xa8/0x1d4) [] (do_one_initcall) from [] (kernel_init_freeable+0x26c/0x2c8) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) Exception stack(0xeb08dfb0 to 0xeb08dff8) dfa0: dfc0: 00
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
On Tue, Sep 17, 2019 at 11:25:46AM +, Peter Rosin wrote: > On 2019-09-17 12:13, Uwe Kleine-König wrote: > > Hello Geert, > > > > On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote: > >> Hi Rob, Uwe, > >> > >> On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: > >>> On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= > >>>wrote: > Referencing device tree nodes from a property allows to pass arguments. > This is for example used for referencing gpios. This looks as follows: > > gpio_ctrl: gpio-controller { > #gpio-cells = <2> > ... > } > > someothernode { > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > ... > } > > To know the number of arguments this must be either fixed, or the > referenced node is checked for a $cells_name (here: "#gpio-cells") > property and with this information the start of the second reference can > be determined. > > Currently regulators are referenced with no additional arguments. To > allow some optional arguments without having to change all referenced > nodes this change introduces a way to specify a default cell_count. So > when a phandle is parsed we check for the $cells_name property and use > it as before if present. If it is not present we fall back to > cells_count if non-negative and only fail if cells_count is smaller than > zero. > > Signed-off-by: Uwe Kleine-König > >> > >> This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback > >> to non-negative cell_count") in robh/for-next, which causes a lock-up when > >> booting a shmobile_defconfig kernel on r8a7791/koelsch: > >> > >> rcu: INFO: rcu_sched self-detected stall on CPU > >> rcu: 0-: (2099 ticks this GP) idle=6fe/1/0x4002 > >> softirq=29/29 fqs=1050 > >> (t=2100 jiffies g=-1131 q=0) > >> NMI backtrace for cpu 0 > >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted > >> 5.3.0-rc2-shmobile-00050-ge42ee61017f58cd9 #376 > >> Hardware name: Generic R-Car Gen2 (Flattened Device Tree) > >> [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > >> [] (show_stack) from [] (dump_stack+0x7c/0x9c) > >> [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0xb8) > >> [] (nmi_cpu_backtrace) from [] > >> (nmi_trigger_cpumask_backtrace+0x84/0x114) > >> [] (nmi_trigger_cpumask_backtrace) from [] > >> (rcu_dump_cpu_stacks+0xac/0xc8) > >> [] (rcu_dump_cpu_stacks) from [] > >> (rcu_sched_clock_irq+0x2ac/0x6b4) > >> [] (rcu_sched_clock_irq) from [] > >> (update_process_times+0x30/0x5c) > >> [] (update_process_times) from [] > >> (tick_nohz_handler+0xcc/0x120) > >> [] (tick_nohz_handler) from [] > >> (arch_timer_handler_virt+0x28/0x30) > >> [] (arch_timer_handler_virt) from [] > >> (handle_percpu_devid_irq+0xe8/0x21c) > >> [] (handle_percpu_devid_irq) from [] > >> (generic_handle_irq+0x18/0x28) > >> [] (generic_handle_irq) from [] > >> (__handle_domain_irq+0xa0/0xb4) > >> [] (__handle_domain_irq) from [] > >> (gic_handle_irq+0x58/0x90) > >> [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) > >> Exception stack(0xeb08dd30 to 0xeb08dd78) > >> dd20: c0cc7514 2013 0005 > >> 3b27 > >> dd40: eb7c4020 c0cc750c 0051 0051 2013 c0c66b08 eb1cdc00 > >> 0018 > >> dd60: eb08dd80 c05c1a38 c0756c00 2013 > >> [] (__irq_svc) from [] > >> (_raw_spin_unlock_irqrestore+0x1c/0x20) > >> [] (_raw_spin_unlock_irqrestore) from [] > >> (of_find_node_by_phandle+0xcc/0xf0) > >> [] (of_find_node_by_phandle) from [] > >> (of_phandle_iterator_next+0x68/0x178) > >> [] (of_phandle_iterator_next) from [] > >> (of_count_phandle_with_args+0x5c/0x7c) > >> [] (of_count_phandle_with_args) from [] > >> (i2c_demux_pinctrl_probe+0x24/0x1fc) > >> [] (i2c_demux_pinctrl_probe) from [] > >> (platform_drv_probe+0x48/0x94) > >> [] (platform_drv_probe) from [] > >> (really_probe+0x1f0/0x2b8) > >> [] (really_probe) from [] > >> (driver_probe_device+0x140/0x158) > >> [] (driver_probe_device) from [] > >> (device_driver_attach+0x44/0x5c) > >> [] (device_driver_attach) from [] > >> (__driver_attach+0xac/0xb4) > >> [] (__driver_attach) from [] > >> (bus_for_each_dev+0x64/0xa0) > >> [] (bus_for_each_dev) from [] > >> (bus_add_driver+0x148/0x1a8) > >> [] (bus_add_driver) from [] (driver_register+0xac/0xf0) > >> [] (driver_register) from [] > >> (do_one_initcall+0xa8/0x1d4) > >> [] (do_one_initcall) from [] > >> (kernel_init_freeable+0x26c/0x2c8) > >> [] (kernel_init_freeable) from [] > >> (kernel_init+0x8/0x10c) > >> [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) > >> Exception stack(0xeb08dfb0 to 0xeb08dff8) > >> dfa0: > >> > >> dfc0: > >> > >> dfe0:
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
On 2019-09-17 12:13, Uwe Kleine-König wrote: > Hello Geert, > > On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote: >> Hi Rob, Uwe, >> >> On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: >>> On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= >>> wrote: Referencing device tree nodes from a property allows to pass arguments. This is for example used for referencing gpios. This looks as follows: gpio_ctrl: gpio-controller { #gpio-cells = <2> ... } someothernode { gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; ... } To know the number of arguments this must be either fixed, or the referenced node is checked for a $cells_name (here: "#gpio-cells") property and with this information the start of the second reference can be determined. Currently regulators are referenced with no additional arguments. To allow some optional arguments without having to change all referenced nodes this change introduces a way to specify a default cell_count. So when a phandle is parsed we check for the $cells_name property and use it as before if present. If it is not present we fall back to cells_count if non-negative and only fail if cells_count is smaller than zero. Signed-off-by: Uwe Kleine-König >> >> This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback >> to non-negative cell_count") in robh/for-next, which causes a lock-up when >> booting a shmobile_defconfig kernel on r8a7791/koelsch: >> >> rcu: INFO: rcu_sched self-detected stall on CPU >> rcu: 0-: (2099 ticks this GP) idle=6fe/1/0x4002 >> softirq=29/29 fqs=1050 >> (t=2100 jiffies g=-1131 q=0) >> NMI backtrace for cpu 0 >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted >> 5.3.0-rc2-shmobile-00050-ge42ee61017f58cd9 #376 >> Hardware name: Generic R-Car Gen2 (Flattened Device Tree) >> [] (unwind_backtrace) from [] (show_stack+0x10/0x14) >> [] (show_stack) from [] (dump_stack+0x7c/0x9c) >> [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0xb8) >> [] (nmi_cpu_backtrace) from [] >> (nmi_trigger_cpumask_backtrace+0x84/0x114) >> [] (nmi_trigger_cpumask_backtrace) from [] >> (rcu_dump_cpu_stacks+0xac/0xc8) >> [] (rcu_dump_cpu_stacks) from [] >> (rcu_sched_clock_irq+0x2ac/0x6b4) >> [] (rcu_sched_clock_irq) from [] >> (update_process_times+0x30/0x5c) >> [] (update_process_times) from [] >> (tick_nohz_handler+0xcc/0x120) >> [] (tick_nohz_handler) from [] >> (arch_timer_handler_virt+0x28/0x30) >> [] (arch_timer_handler_virt) from [] >> (handle_percpu_devid_irq+0xe8/0x21c) >> [] (handle_percpu_devid_irq) from [] >> (generic_handle_irq+0x18/0x28) >> [] (generic_handle_irq) from [] >> (__handle_domain_irq+0xa0/0xb4) >> [] (__handle_domain_irq) from [] >> (gic_handle_irq+0x58/0x90) >> [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) >> Exception stack(0xeb08dd30 to 0xeb08dd78) >> dd20: c0cc7514 2013 0005 3b27 >> dd40: eb7c4020 c0cc750c 0051 0051 2013 c0c66b08 eb1cdc00 0018 >> dd60: eb08dd80 c05c1a38 c0756c00 2013 >> [] (__irq_svc) from [] >> (_raw_spin_unlock_irqrestore+0x1c/0x20) >> [] (_raw_spin_unlock_irqrestore) from [] >> (of_find_node_by_phandle+0xcc/0xf0) >> [] (of_find_node_by_phandle) from [] >> (of_phandle_iterator_next+0x68/0x178) >> [] (of_phandle_iterator_next) from [] >> (of_count_phandle_with_args+0x5c/0x7c) >> [] (of_count_phandle_with_args) from [] >> (i2c_demux_pinctrl_probe+0x24/0x1fc) >> [] (i2c_demux_pinctrl_probe) from [] >> (platform_drv_probe+0x48/0x94) >> [] (platform_drv_probe) from [] >> (really_probe+0x1f0/0x2b8) >> [] (really_probe) from [] >> (driver_probe_device+0x140/0x158) >> [] (driver_probe_device) from [] >> (device_driver_attach+0x44/0x5c) >> [] (device_driver_attach) from [] >> (__driver_attach+0xac/0xb4) >> [] (__driver_attach) from [] (bus_for_each_dev+0x64/0xa0) >> [] (bus_for_each_dev) from [] >> (bus_add_driver+0x148/0x1a8) >> [] (bus_add_driver) from [] (driver_register+0xac/0xf0) >> [] (driver_register) from [] (do_one_initcall+0xa8/0x1d4) >> [] (do_one_initcall) from [] >> (kernel_init_freeable+0x26c/0x2c8) >> [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) >> [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) >> Exception stack(0xeb08dfb0 to 0xeb08dff8) >> dfa0: >> dfc0: >> dfe0: 0013 >> >> Presumably it loops forever, due to a conversion of -1 to unsigned >> somewhere? > > Hmm, I fail to see the culprit. i2c_demux_pinctrl_probe calls > of_count_phandle_with_args with cells_name=NULL. With that I don't see > how my patch changes anything as the only ch
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
Hello Geert, On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote: > Hi Rob, Uwe, > > On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: > > On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= > > wrote: > > > Referencing device tree nodes from a property allows to pass arguments. > > > This is for example used for referencing gpios. This looks as follows: > > > > > > gpio_ctrl: gpio-controller { > > > #gpio-cells = <2> > > > ... > > > } > > > > > > someothernode { > > > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > > > ... > > > } > > > > > > To know the number of arguments this must be either fixed, or the > > > referenced node is checked for a $cells_name (here: "#gpio-cells") > > > property and with this information the start of the second reference can > > > be determined. > > > > > > Currently regulators are referenced with no additional arguments. To > > > allow some optional arguments without having to change all referenced > > > nodes this change introduces a way to specify a default cell_count. So > > > when a phandle is parsed we check for the $cells_name property and use > > > it as before if present. If it is not present we fall back to > > > cells_count if non-negative and only fail if cells_count is smaller than > > > zero. > > > > > > Signed-off-by: Uwe Kleine-König > > This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback > to non-negative cell_count") in robh/for-next, which causes a lock-up when > booting a shmobile_defconfig kernel on r8a7791/koelsch: > > rcu: INFO: rcu_sched self-detected stall on CPU > rcu: 0-: (2099 ticks this GP) idle=6fe/1/0x4002 > softirq=29/29 fqs=1050 > (t=2100 jiffies g=-1131 q=0) > NMI backtrace for cpu 0 > CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 5.3.0-rc2-shmobile-00050-ge42ee61017f58cd9 #376 > Hardware name: Generic R-Car Gen2 (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x7c/0x9c) > [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0xb8) > [] (nmi_cpu_backtrace) from [] > (nmi_trigger_cpumask_backtrace+0x84/0x114) > [] (nmi_trigger_cpumask_backtrace) from [] > (rcu_dump_cpu_stacks+0xac/0xc8) > [] (rcu_dump_cpu_stacks) from [] > (rcu_sched_clock_irq+0x2ac/0x6b4) > [] (rcu_sched_clock_irq) from [] > (update_process_times+0x30/0x5c) > [] (update_process_times) from [] > (tick_nohz_handler+0xcc/0x120) > [] (tick_nohz_handler) from [] > (arch_timer_handler_virt+0x28/0x30) > [] (arch_timer_handler_virt) from [] > (handle_percpu_devid_irq+0xe8/0x21c) > [] (handle_percpu_devid_irq) from [] > (generic_handle_irq+0x18/0x28) > [] (generic_handle_irq) from [] > (__handle_domain_irq+0xa0/0xb4) > [] (__handle_domain_irq) from [] > (gic_handle_irq+0x58/0x90) > [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) > Exception stack(0xeb08dd30 to 0xeb08dd78) > dd20: c0cc7514 2013 0005 3b27 > dd40: eb7c4020 c0cc750c 0051 0051 2013 c0c66b08 eb1cdc00 0018 > dd60: eb08dd80 c05c1a38 c0756c00 2013 > [] (__irq_svc) from [] > (_raw_spin_unlock_irqrestore+0x1c/0x20) > [] (_raw_spin_unlock_irqrestore) from [] > (of_find_node_by_phandle+0xcc/0xf0) > [] (of_find_node_by_phandle) from [] > (of_phandle_iterator_next+0x68/0x178) > [] (of_phandle_iterator_next) from [] > (of_count_phandle_with_args+0x5c/0x7c) > [] (of_count_phandle_with_args) from [] > (i2c_demux_pinctrl_probe+0x24/0x1fc) > [] (i2c_demux_pinctrl_probe) from [] > (platform_drv_probe+0x48/0x94) > [] (platform_drv_probe) from [] (really_probe+0x1f0/0x2b8) > [] (really_probe) from [] > (driver_probe_device+0x140/0x158) > [] (driver_probe_device) from [] > (device_driver_attach+0x44/0x5c) > [] (device_driver_attach) from [] > (__driver_attach+0xac/0xb4) > [] (__driver_attach) from [] (bus_for_each_dev+0x64/0xa0) > [] (bus_for_each_dev) from [] (bus_add_driver+0x148/0x1a8) > [] (bus_add_driver) from [] (driver_register+0xac/0xf0) > [] (driver_register) from [] (do_one_initcall+0xa8/0x1d4) > [] (do_one_initcall) from [] > (kernel_init_freeable+0x26c/0x2c8) > [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) > [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) > Exception stack(0xeb08dfb0 to 0xeb08dff8) > dfa0: > dfc0: > dfe0: 0013 > > Presumably it loops forever, due to a conversion of -1 to unsigned > somewhere? Hmm, I fail to see the culprit. i2c_demux_pinctrl_probe calls of_count_phandle_with_args with cells_name=NULL. With that I don't see how my patch changes anything as the only change is in an if (it->cells_name) block that shouldn't be relevant in your case. Can you please verify that the l
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
Hi Rob, Uwe, On Fri, Sep 13, 2019 at 11:58 PM Rob Herring wrote: > On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= >wrote: > > Referencing device tree nodes from a property allows to pass arguments. > > This is for example used for referencing gpios. This looks as follows: > > > > gpio_ctrl: gpio-controller { > > #gpio-cells = <2> > > ... > > } > > > > someothernode { > > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > > ... > > } > > > > To know the number of arguments this must be either fixed, or the > > referenced node is checked for a $cells_name (here: "#gpio-cells") > > property and with this information the start of the second reference can > > be determined. > > > > Currently regulators are referenced with no additional arguments. To > > allow some optional arguments without having to change all referenced > > nodes this change introduces a way to specify a default cell_count. So > > when a phandle is parsed we check for the $cells_name property and use > > it as before if present. If it is not present we fall back to > > cells_count if non-negative and only fail if cells_count is smaller than > > zero. > > > > Signed-off-by: Uwe Kleine-König This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback to non-negative cell_count") in robh/for-next, which causes a lock-up when booting a shmobile_defconfig kernel on r8a7791/koelsch: rcu: INFO: rcu_sched self-detected stall on CPU rcu: 0-: (2099 ticks this GP) idle=6fe/1/0x4002 softirq=29/29 fqs=1050 (t=2100 jiffies g=-1131 q=0) NMI backtrace for cpu 0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc2-shmobile-00050-ge42ee61017f58cd9 #376 Hardware name: Generic R-Car Gen2 (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x7c/0x9c) [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0xb8) [] (nmi_cpu_backtrace) from [] (nmi_trigger_cpumask_backtrace+0x84/0x114) [] (nmi_trigger_cpumask_backtrace) from [] (rcu_dump_cpu_stacks+0xac/0xc8) [] (rcu_dump_cpu_stacks) from [] (rcu_sched_clock_irq+0x2ac/0x6b4) [] (rcu_sched_clock_irq) from [] (update_process_times+0x30/0x5c) [] (update_process_times) from [] (tick_nohz_handler+0xcc/0x120) [] (tick_nohz_handler) from [] (arch_timer_handler_virt+0x28/0x30) [] (arch_timer_handler_virt) from [] (handle_percpu_devid_irq+0xe8/0x21c) [] (handle_percpu_devid_irq) from [] (generic_handle_irq+0x18/0x28) [] (generic_handle_irq) from [] (__handle_domain_irq+0xa0/0xb4) [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x90) [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) Exception stack(0xeb08dd30 to 0xeb08dd78) dd20: c0cc7514 2013 0005 3b27 dd40: eb7c4020 c0cc750c 0051 0051 2013 c0c66b08 eb1cdc00 0018 dd60: eb08dd80 c05c1a38 c0756c00 2013 [] (__irq_svc) from [] (_raw_spin_unlock_irqrestore+0x1c/0x20) [] (_raw_spin_unlock_irqrestore) from [] (of_find_node_by_phandle+0xcc/0xf0) [] (of_find_node_by_phandle) from [] (of_phandle_iterator_next+0x68/0x178) [] (of_phandle_iterator_next) from [] (of_count_phandle_with_args+0x5c/0x7c) [] (of_count_phandle_with_args) from [] (i2c_demux_pinctrl_probe+0x24/0x1fc) [] (i2c_demux_pinctrl_probe) from [] (platform_drv_probe+0x48/0x94) [] (platform_drv_probe) from [] (really_probe+0x1f0/0x2b8) [] (really_probe) from [] (driver_probe_device+0x140/0x158) [] (driver_probe_device) from [] (device_driver_attach+0x44/0x5c) [] (device_driver_attach) from [] (__driver_attach+0xac/0xb4) [] (__driver_attach) from [] (bus_for_each_dev+0x64/0xa0) [] (bus_for_each_dev) from [] (bus_add_driver+0x148/0x1a8) [] (bus_add_driver) from [] (driver_register+0xac/0xf0) [] (driver_register) from [] (do_one_initcall+0xa8/0x1d4) [] (do_one_initcall) from [] (kernel_init_freeable+0x26c/0x2c8) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) Exception stack(0xeb08dfb0 to 0xeb08dff8) dfa0: dfc0: dfe0: 0013 Presumably it loops forever, due to a conversion of -1 to unsigned somewhere? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= wrote: > Referencing device tree nodes from a property allows to pass arguments. > This is for example used for referencing gpios. This looks as follows: > > gpio_ctrl: gpio-controller { > #gpio-cells = <2> > ... > } > > someothernode { > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > ... > } > > To know the number of arguments this must be either fixed, or the > referenced node is checked for a $cells_name (here: "#gpio-cells") > property and with this information the start of the second reference can > be determined. > > Currently regulators are referenced with no additional arguments. To > allow some optional arguments without having to change all referenced > nodes this change introduces a way to specify a default cell_count. So > when a phandle is parsed we check for the $cells_name property and use > it as before if present. If it is not present we fall back to > cells_count if non-negative and only fail if cells_count is smaller than > zero. > > Signed-off-by: Uwe Kleine-König > --- > drivers/of/base.c | 25 + > 1 file changed, 17 insertions(+), 8 deletions(-) > Applied both patches, thanks. Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
On Sat, Aug 24, 2019 at 03:28:46PM +0200, Uwe Kleine-König wrote: > Referencing device tree nodes from a property allows to pass arguments. > This is for example used for referencing gpios. This looks as follows: > > gpio_ctrl: gpio-controller { > #gpio-cells = <2> > ... > } > > someothernode { > gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; > ... > } > > To know the number of arguments this must be either fixed, or the > referenced node is checked for a $cells_name (here: "#gpio-cells") > property and with this information the start of the second reference can > be determined. > > Currently regulators are referenced with no additional arguments. To > allow some optional arguments without having to change all referenced > nodes this change introduces a way to specify a default cell_count. So > when a phandle is parsed we check for the $cells_name property and use > it as before if present. If it is not present we fall back to > cells_count if non-negative and only fail if cells_count is smaller than > zero. > > Signed-off-by: Uwe Kleine-König > --- > drivers/of/base.c | 25 + > 1 file changed, 17 insertions(+), 8 deletions(-) Looks fine to me. I can apply with an ack from the iommu folks on patch 1. Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count
Referencing device tree nodes from a property allows to pass arguments. This is for example used for referencing gpios. This looks as follows: gpio_ctrl: gpio-controller { #gpio-cells = <2> ... } someothernode { gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>; ... } To know the number of arguments this must be either fixed, or the referenced node is checked for a $cells_name (here: "#gpio-cells") property and with this information the start of the second reference can be determined. Currently regulators are referenced with no additional arguments. To allow some optional arguments without having to change all referenced nodes this change introduces a way to specify a default cell_count. So when a phandle is parsed we check for the $cells_name property and use it as before if present. If it is not present we fall back to cells_count if non-negative and only fail if cells_count is smaller than zero. Signed-off-by: Uwe Kleine-König --- drivers/of/base.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 55e7f5bb0549..2f25d2dfecfa 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1335,11 +1335,20 @@ int of_phandle_iterator_next(struct of_phandle_iterator *it) if (of_property_read_u32(it->node, it->cells_name, &count)) { - pr_err("%pOF: could not get %s for %pOF\n", - it->parent, - it->cells_name, - it->node); - goto err; + /* +* If both cell_count and cells_name is given, +* fall back to cell_count in absence +* of the cells_name property +*/ + if (it->cell_count >= 0) { + count = it->cell_count; + } else { + pr_err("%pOF: could not get %s for %pOF\n", + it->parent, + it->cells_name, + it->node); + goto err; + } } } else { count = it->cell_count; @@ -1505,7 +1514,7 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na { if (index < 0) return -EINVAL; - return __of_parse_phandle_with_args(np, list_name, cells_name, 0, + return __of_parse_phandle_with_args(np, list_name, cells_name, -1, index, out_args); } EXPORT_SYMBOL(of_parse_phandle_with_args); @@ -1588,7 +1597,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np, if (!pass_name) goto free; - ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index, + ret = __of_parse_phandle_with_args(np, list_name, cells_name, -1, index, out_args); if (ret) goto free; @@ -1756,7 +1765,7 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na struct of_phandle_iterator it; int rc, cur_index = 0; - rc = of_phandle_iterator_init(&it, np, list_name, cells_name, 0); + rc = of_phandle_iterator_init(&it, np, list_name, cells_name, -1); if (rc) return rc; -- 2.20.1