From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 98CDAC4CEC9 for ; Tue, 17 Sep 2019 12:25:42 +0000 (UTC) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 69955214AF for ; Tue, 17 Sep 2019 12:25:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 69955214AF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 39FC21451; Tue, 17 Sep 2019 12:25:42 +0000 (UTC) Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 2ABB31450 for ; Tue, 17 Sep 2019 12:25:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [85.220.165.71]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id EE29B81A for ; Tue, 17 Sep 2019 12:25:39 +0000 (UTC) Received: from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5]) by metis.ext.pengutronix.de with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1iACY2-00073e-FZ; Tue, 17 Sep 2019 14:25:34 +0200 Received: from ukl by pty.hi.pengutronix.de with local (Exim 4.89) (envelope-from ) id 1iACXy-0007cg-QF; Tue, 17 Sep 2019 14:25:30 +0200 Date: Tue, 17 Sep 2019 14:25:30 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Peter Rosin Subject: Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell_count Message-ID: <20190917122530.3xy7sut3xdvzlomj@pengutronix.de> References: <20190824132846.8589-1-u.kleine-koenig@pengutronix.de> <20190824132846.8589-2-u.kleine-koenig@pengutronix.de> <20190913215809.GA11833@bogus> <20190917101303.t5otztdus7y3ayau@pengutronix.de> <489c90fb-a135-4fd8-ecb9-46404bd3c234@axentia.se> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <489c90fb-a135-4fd8-ecb9-46404bd3c234@axentia.se> User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c5 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: iommu@lists.linux-foundation.org Cc: Rob Herring , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Frank Rowand , Linux Kernel Mailing List , Linux-Renesas , Wolfram Sang , Linux IOMMU , Geert Uytterhoeven , Linux I2C , Sascha Hauer , Matthias Brugger , "linux-mediatek@lists.infradead.org" , Will Deacon , Linux ARM , Robin Murphy X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: iommu-bounces@lists.linux-foundation.org Errors-To: iommu-bounces@lists.linux-foundation.org On Tue, Sep 17, 2019 at 11:25:46AM +0000, Peter Rosin wrote: > On 2019-09-17 12:13, Uwe Kleine-K=F6nig 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, =3D?UTF-8?q?Uwe=3D20Kleine-K=3DC3= =3DB6nig?=3D wrote: > >>>> Referencing device tree nodes from a property allows to pass argumen= ts. > >>>> This is for example used for referencing gpios. This looks as follow= s: > >>>> > >>>> gpio_ctrl: gpio-controller { > >>>> #gpio-cells =3D <2> > >>>> ... > >>>> } > >>>> > >>>> someothernode { > >>>> gpios =3D <&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 u= se > >>>> 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=F6nig > >> > >> This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fall= back > >> 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=3D6fe/1/0x40000002 > >> softirq=3D29/29 fqs=3D1050 > >> (t=3D2100 jiffies g=3D-1131 q=3D0) > >> 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/0x1= 4) > >> [] (show_stack) from [] (dump_stack+0x7c/0x9c) > >> [] (dump_stack) from [] (nmi_cpu_backtrace+0xa0/0x= b8) > >> [] (nmi_cpu_backtrace) from [] (nmi_trigger_cpumas= k_backtrace+0x84/0x114) > >> [] (nmi_trigger_cpumask_backtrace) from [] (rcu_du= mp_cpu_stacks+0xac/0xc8) > >> [] (rcu_dump_cpu_stacks) from [] (rcu_sched_clock_= irq+0x2ac/0x6b4) > >> [] (rcu_sched_clock_irq) from [] (update_process_t= imes+0x30/0x5c) > >> [] (update_process_times) from [] (tick_nohz_handl= er+0xcc/0x120) > >> [] (tick_nohz_handler) from [] (arch_timer_handler= _virt+0x28/0x30) > >> [] (arch_timer_handler_virt) from [] (handle_percp= u_devid_irq+0xe8/0x21c) > >> [] (handle_percpu_devid_irq) from [] (generic_hand= le_irq+0x18/0x28) > >> [] (generic_handle_irq) from [] (__handle_domain_i= rq+0xa0/0xb4) > >> [] (__handle_domain_irq) from [] (gic_handle_irq+0= x58/0x90) > >> [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) > >> Exception stack(0xeb08dd30 to 0xeb08dd78) > >> dd20: c0cc7514 20000013 00000005 0= 0003b27 > >> dd40: eb7c4020 c0cc750c 00000051 00000051 20000013 c0c66b08 eb1cdc00 0= 0000018 > >> dd60: 00000000 eb08dd80 c05c1a38 c0756c00 20000013 ffffffff > >> [] (__irq_svc) from [] (_raw_spin_unlock_irqrestor= e+0x1c/0x20) > >> [] (_raw_spin_unlock_irqrestore) from [] (of_find_= node_by_phandle+0xcc/0xf0) > >> [] (of_find_node_by_phandle) from [] (of_phandle_i= terator_next+0x68/0x178) > >> [] (of_phandle_iterator_next) from [] (of_count_ph= andle_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+0x1f= 0/0x2b8) > >> [] (really_probe) from [] (driver_probe_device+0x1= 40/0x158) > >> [] (driver_probe_device) from [] (device_driver_at= tach+0x44/0x5c) > >> [] (device_driver_attach) from [] (__driver_attach= +0xac/0xb4) > >> [] (__driver_attach) from [] (bus_for_each_dev+0x6= 4/0xa0) > >> [] (bus_for_each_dev) from [] (bus_add_driver+0x14= 8/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: 00000000 00000000 00000000 0= 0000000 > >> dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 0= 0000000 > >> dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > >> > >> 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=3DNULL. 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 loop in of_count_phandle_with_args is > > indeed not terminating, e.g. with > = > The below indicated else-branch was not touched by e42ee61017f58cd9, > which ends up setting the count to -1 (aka 0xff...ff in this case). > No? > = > int of_phandle_iterator_next(struct of_phandle_iterator *it) > { > = > ... > = > if (it->cells_name) { > = > ... > = > } else { > count =3D it->cell_count; /* <---- SUSPECT!!! */ > } 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_itera= tor *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 =3D of_get_property(np, list_name, &size); @@ -1765,6 +1772,18 @@ int of_count_phandle_with_args(const struct device_n= ode *np, const char *list_na struct of_phandle_iterator it; int rc, cur_index =3D 0; = + /* If cells_name is NULL we assume an cell_count of 0 */ + if (cells_name =3D=3D NULL) { + const __be32 *list; + int size; + + list =3D of_get_property(np, list_name, &size); + if (!list) + return -ENOENT; + + return size / sizeof(*list); + } + rc =3D of_phandle_iterator_init(&it, np, list_name, cells_name, -1); if (rc) return rc; Best regards Uwe -- = Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu