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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C52B6C38142 for ; Tue, 31 Jan 2023 10:13:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229992AbjAaKNq (ORCPT ); Tue, 31 Jan 2023 05:13:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229716AbjAaKNq (ORCPT ); Tue, 31 Jan 2023 05:13:46 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2D58549029; Tue, 31 Jan 2023 02:13:45 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DD1CA2F4; Tue, 31 Jan 2023 02:14:26 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 770FD3F64C; Tue, 31 Jan 2023 02:13:39 -0800 (PST) Date: Tue, 31 Jan 2023 10:13:37 +0000 From: Sudeep Holla To: Saravana Kannan Cc: Andy Shevchenko , Sudeep Holla , Greg Kroah-Hartman , "Rafael J. Wysocki" , Cristian Marussi , Linus Walleij , Bartosz Golaszewski , Thomas Gleixner , Marc Zyngier , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Rob Herring , Frank Rowand , Geert Uytterhoeven , Magnus Damm , Len Brown , Daniel Scally , Heikki Krogerus , Sakari Ailus , Tony Lindgren , Linux Kernel Functional Testing , Naresh Kamboju , Abel Vesa , Alexander Stein , Geert Uytterhoeven , John Stultz , Doug Anderson , Guenter Roeck , Dmitry Baryshkov , Maxim Kiselev , Maxim Kochetkov , Miquel Raynal , Luca Weiss , Colin Foster , Martin Kepplinger , Jean-Philippe Brucker , kernel-team@android.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-acpi@vger.kernel.org Subject: Re: [PATCH v2 04/11] gpiolib: Clear the gpio_device's fwnode initialized flag before adding Message-ID: <20230131101337.376mnrvhltbsychd@bogus> References: <20230127001141.407071-1-saravanak@google.com> <20230127001141.407071-5-saravanak@google.com> <20230130143153.67dsxn4lugfetfwb@bogus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Mon, Jan 30, 2023 at 08:01:17PM -0800, Saravana Kannan wrote: > On Mon, Jan 30, 2023 at 7:14 AM Andy Shevchenko > wrote: > > > > On Mon, Jan 30, 2023 at 02:31:53PM +0000, Sudeep Holla wrote: > > > On Thu, Jan 26, 2023 at 04:11:31PM -0800, Saravana Kannan wrote: > > > > Registering an irqdomain sets the flag for the fwnode. But having the > > > > flag set when a device is added is interpreted by fw_devlink to mean the > > > > device has already been initialized and will never probe. This prevents > > > > fw_devlink from creating device links with the gpio_device as a > > > > supplier. So, clear the flag before adding the device. > > > > ... > > > > > > + /* > > > > + * If fwnode doesn't belong to another device, it's safe to clear its > > > > + * initialized flag. > > > > + */ > > > > + if (!gdev->dev.fwnode->dev) > > > > + fwnode_dev_initialized(gdev->dev.fwnode, false); > > > > > > This is the one causing the kernel crash during the boot on FVP which > > > Naresh has reported. Just reverted this and was able to boot, confirming > > > the issue with this patch. > > > > I'm wondering if > > > > if (!dev_fwnode(&gdev->dev)->dev) > > fwnode_dev_initialized(&dev_fwnode(gdev->dev), false); > > > > works. > > No, that won't help. The problem was that with arm32, we have gpio > devices created without any of_node or fwnode. So I can't assume > fwnode will always be present. > Correct, and this one is not even arm32. But it is just reusing a driver that needs to be supported even on arm32. Not sure on how to proceed. As a simple way to check, I added a NULL check for fwnode building on top of Andy's suggestion[1]. That works. Also the driver in question on arm64 FVP model is drivers/mfd/vexpress-sysreg.c mfd_add_device() in drivers/mfd/mfd-core.c allows addition of devices without of_node/fwnode. I am sure returning error like[2] will break many platforms but I just wanted to confirm the root cause and [2] fixes the boot without NULL check for fwnode in gpiochip_setup_dev(). Hope this helps. -- Regards, Sudeep [1] -->8 diff --git i/drivers/gpio/gpiolib.c w/drivers/gpio/gpiolib.c index b23140c6485f..e162f13aa2c9 100644 --- i/drivers/gpio/gpiolib.c +++ w/drivers/gpio/gpiolib.c @@ -577,13 +577,15 @@ static void gpiodevice_release(struct device *dev) static int gpiochip_setup_dev(struct gpio_device *gdev) { int ret; + struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev); /* * If fwnode doesn't belong to another device, it's safe to clear its * initialized flag. */ - if (!gdev->dev.fwnode->dev) - fwnode_dev_initialized(gdev->dev.fwnode, false); + if (fwnode && !fwnode->dev) + fwnode_dev_initialized(fwnode, false); + ret = gcdev_register(gdev, gpio_devt); if (ret) return ret; [2] -->8 diff --git i/drivers/mfd/mfd-core.c w/drivers/mfd/mfd-core.c index 16d1861e9682..3b2c4b0e9a2a 100644 --- i/drivers/mfd/mfd-core.c +++ w/drivers/mfd/mfd-core.c @@ -231,9 +231,11 @@ static int mfd_add_device(struct device *parent, int id, } } - if (!pdev->dev.of_node) + if (!pdev->dev.of_node) { pr_warn("%s: Failed to locate of_node [id: %d]\n", cell->name, platform_id); + goto fail_alias; + } } mfd_acpi_add_device(cell, pdev); 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 844E2C38142 for ; Tue, 31 Jan 2023 10:39:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RU1k8k9HM3a8zjtoILNQkMsNjqPynmniiWqFrV0W8x8=; b=XU9gnP/NiCYIXk Empev3aKxmMES44h6PPerf+LGOv1PBI68UFqKzVXuHa8G52/V8yLTUjQtJlBXzC6xL04Rg9LC9qE3 SCay0W8IwgNSZZGlf/nc2Td/5vYqAhDD03CdmlwPdf0s2IBJn5Rzr+3jTP08NST+Pth+5jWcOO/nF YgZ+23paL7CcXc8yY+TKOmWdRV14K42BomOJH3tOQKPrSOxBmgj0ux0CuP7H1BriIzqLhkdJDPa9R E4EG7eVz0OcoYOAY0JrqKh1N2xquYGoVreqNXqXm1wqneUi4bKR0Ac5sCxeUq6ILxj5F9zb8dXmMr ReKZZeojOO89TqDdgwhg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pMo1c-007Tbx-Bc; Tue, 31 Jan 2023 10:38:04 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pMneC-007J8v-EW for linux-arm-kernel@lists.infradead.org; Tue, 31 Jan 2023 10:13:55 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DD1CA2F4; Tue, 31 Jan 2023 02:14:26 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 770FD3F64C; Tue, 31 Jan 2023 02:13:39 -0800 (PST) Date: Tue, 31 Jan 2023 10:13:37 +0000 From: Sudeep Holla To: Saravana Kannan Cc: Andy Shevchenko , Sudeep Holla , Greg Kroah-Hartman , "Rafael J. Wysocki" , Cristian Marussi , Linus Walleij , Bartosz Golaszewski , Thomas Gleixner , Marc Zyngier , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Rob Herring , Frank Rowand , Geert Uytterhoeven , Magnus Damm , Len Brown , Daniel Scally , Heikki Krogerus , Sakari Ailus , Tony Lindgren , Linux Kernel Functional Testing , Naresh Kamboju , Abel Vesa , Alexander Stein , Geert Uytterhoeven , John Stultz , Doug Anderson , Guenter Roeck , Dmitry Baryshkov , Maxim Kiselev , Maxim Kochetkov , Miquel Raynal , Luca Weiss , Colin Foster , Martin Kepplinger , Jean-Philippe Brucker , kernel-team@android.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-acpi@vger.kernel.org Subject: Re: [PATCH v2 04/11] gpiolib: Clear the gpio_device's fwnode initialized flag before adding Message-ID: <20230131101337.376mnrvhltbsychd@bogus> References: <20230127001141.407071-1-saravanak@google.com> <20230127001141.407071-5-saravanak@google.com> <20230130143153.67dsxn4lugfetfwb@bogus> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230131_021352_619133_EB4B8B63 X-CRM114-Status: GOOD ( 31.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jan 30, 2023 at 08:01:17PM -0800, Saravana Kannan wrote: > On Mon, Jan 30, 2023 at 7:14 AM Andy Shevchenko > wrote: > > > > On Mon, Jan 30, 2023 at 02:31:53PM +0000, Sudeep Holla wrote: > > > On Thu, Jan 26, 2023 at 04:11:31PM -0800, Saravana Kannan wrote: > > > > Registering an irqdomain sets the flag for the fwnode. But having the > > > > flag set when a device is added is interpreted by fw_devlink to mean the > > > > device has already been initialized and will never probe. This prevents > > > > fw_devlink from creating device links with the gpio_device as a > > > > supplier. So, clear the flag before adding the device. > > > > ... > > > > > > + /* > > > > + * If fwnode doesn't belong to another device, it's safe to clear its > > > > + * initialized flag. > > > > + */ > > > > + if (!gdev->dev.fwnode->dev) > > > > + fwnode_dev_initialized(gdev->dev.fwnode, false); > > > > > > This is the one causing the kernel crash during the boot on FVP which > > > Naresh has reported. Just reverted this and was able to boot, confirming > > > the issue with this patch. > > > > I'm wondering if > > > > if (!dev_fwnode(&gdev->dev)->dev) > > fwnode_dev_initialized(&dev_fwnode(gdev->dev), false); > > > > works. > > No, that won't help. The problem was that with arm32, we have gpio > devices created without any of_node or fwnode. So I can't assume > fwnode will always be present. > Correct, and this one is not even arm32. But it is just reusing a driver that needs to be supported even on arm32. Not sure on how to proceed. As a simple way to check, I added a NULL check for fwnode building on top of Andy's suggestion[1]. That works. Also the driver in question on arm64 FVP model is drivers/mfd/vexpress-sysreg.c mfd_add_device() in drivers/mfd/mfd-core.c allows addition of devices without of_node/fwnode. I am sure returning error like[2] will break many platforms but I just wanted to confirm the root cause and [2] fixes the boot without NULL check for fwnode in gpiochip_setup_dev(). Hope this helps. -- Regards, Sudeep [1] -->8 diff --git i/drivers/gpio/gpiolib.c w/drivers/gpio/gpiolib.c index b23140c6485f..e162f13aa2c9 100644 --- i/drivers/gpio/gpiolib.c +++ w/drivers/gpio/gpiolib.c @@ -577,13 +577,15 @@ static void gpiodevice_release(struct device *dev) static int gpiochip_setup_dev(struct gpio_device *gdev) { int ret; + struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev); /* * If fwnode doesn't belong to another device, it's safe to clear its * initialized flag. */ - if (!gdev->dev.fwnode->dev) - fwnode_dev_initialized(gdev->dev.fwnode, false); + if (fwnode && !fwnode->dev) + fwnode_dev_initialized(fwnode, false); + ret = gcdev_register(gdev, gpio_devt); if (ret) return ret; [2] -->8 diff --git i/drivers/mfd/mfd-core.c w/drivers/mfd/mfd-core.c index 16d1861e9682..3b2c4b0e9a2a 100644 --- i/drivers/mfd/mfd-core.c +++ w/drivers/mfd/mfd-core.c @@ -231,9 +231,11 @@ static int mfd_add_device(struct device *parent, int id, } } - if (!pdev->dev.of_node) + if (!pdev->dev.of_node) { pr_warn("%s: Failed to locate of_node [id: %d]\n", cell->name, platform_id); + goto fail_alias; + } } mfd_acpi_add_device(cell, pdev); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel