From patchwork Wed Apr 15 15:05:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Saenz Julienne X-Patchwork-Id: 1271237 Return-Path: X-Original-To: incoming-dt@patchwork.ozlabs.org Delivered-To: patchwork-incoming-dt@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=devicetree-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 492Qfk6xq7z9sSk for ; Thu, 16 Apr 2020 01:06:22 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394152AbgDOPGV (ORCPT ); Wed, 15 Apr 2020 11:06:21 -0400 Received: from mx2.suse.de ([195.135.220.15]:60680 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394090AbgDOPGT (ORCPT ); Wed, 15 Apr 2020 11:06:19 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C85A1AFBA; Wed, 15 Apr 2020 15:06:16 +0000 (UTC) From: Nicolas Saenz Julienne To: saravanak@google.com, Greg Kroah-Hartman , devicetree@vger.kernel.org, Rob Herring , Frank Rowand Cc: Nicolas Saenz Julienne , linux-kernel@vger.kernel.org Subject: [PATCH 1/4] of: property: Fix create device links for all child-supplier dependencies Date: Wed, 15 Apr 2020 17:05:46 +0200 Message-Id: <20200415150550.28156-2-nsaenzjulienne@suse.de> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20200415150550.28156-1-nsaenzjulienne@suse.de> References: <20200415150550.28156-1-nsaenzjulienne@suse.de> MIME-Version: 1.0 Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Upon adding a new platform device we scan its properties and its children's properties in order to create a consumer/supplier relationship between the device and the property supplier. That said, it's possible for some of the node's children to be disabled, which will create links that'll never be fulfilled. To get around this, use the for_each_available_child_of_node() function instead of for_each_available_node() when iterating over the node's children. Fixes: d4387cd11741 ("of: property: Create device links for all child-supplier depencencies") Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Saravana Kannan --- drivers/of/property.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/property.c b/drivers/of/property.c index b4916dcc9e725..a8c2b13521b27 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1296,7 +1296,7 @@ static int of_link_to_suppliers(struct device *dev, if (of_link_property(dev, con_np, p->name)) ret = -ENODEV; - for_each_child_of_node(con_np, child) + for_each_available_child_of_node(con_np, child) if (of_link_to_suppliers(dev, child) && !ret) ret = -EAGAIN; From patchwork Wed Apr 15 15:05:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Saenz Julienne X-Patchwork-Id: 1271238 Return-Path: X-Original-To: incoming-dt@patchwork.ozlabs.org Delivered-To: patchwork-incoming-dt@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=devicetree-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 492Qfl6Fczz9sSy for ; Thu, 16 Apr 2020 01:06:23 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394182AbgDOPGW (ORCPT ); Wed, 15 Apr 2020 11:06:22 -0400 Received: from mx2.suse.de ([195.135.220.15]:60700 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394091AbgDOPGU (ORCPT ); Wed, 15 Apr 2020 11:06:20 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E4142AFBE; Wed, 15 Apr 2020 15:06:17 +0000 (UTC) From: Nicolas Saenz Julienne To: saravanak@google.com, Greg Kroah-Hartman , devicetree@vger.kernel.org, Rob Herring , Frank Rowand Cc: Nicolas Saenz Julienne , linux-kernel@vger.kernel.org Subject: [PATCH 2/4] of: property: Do not link to disabled devices Date: Wed, 15 Apr 2020 17:05:47 +0200 Message-Id: <20200415150550.28156-3-nsaenzjulienne@suse.de> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20200415150550.28156-1-nsaenzjulienne@suse.de> References: <20200415150550.28156-1-nsaenzjulienne@suse.de> MIME-Version: 1.0 Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org When creating a consumer/supplier relationship between two devices, make sure the supplier node is actually active. Otherwise this will create a device link that will never be fulfilled. This, in the worst case scenario, will hang the system during boot. Note that, in practice, the fact that a device-tree represented consumer/supplier relationship isn't fulfilled will not prevent devices from successfully probing. Fixes: a3e1d1a7f5fc ("of: property: Add functional dependency link from DT bindings") Signed-off-by: Nicolas Saenz Julienne --- drivers/of/property.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/of/property.c b/drivers/of/property.c index a8c2b13521b27..487685ff8bb19 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1052,6 +1052,13 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, return -ENODEV; } + /* Don't allow linking a device node as consumer of a disabled node */ + if (!of_device_is_available(sup_np)) { + dev_dbg(dev, "Not linking to %pOFP - Not available\n", sup_np); + of_node_put(sup_np); + return -ENODEV; + } + /* * Don't allow linking a device node as a consumer of one of its * descendant nodes. By definition, a child node can't be a functional From patchwork Wed Apr 15 15:05:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Saenz Julienne X-Patchwork-Id: 1271242 Return-Path: X-Original-To: incoming-dt@patchwork.ozlabs.org Delivered-To: patchwork-incoming-dt@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=devicetree-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 492QgM0Fq1z9sT3 for ; Thu, 16 Apr 2020 01:06:55 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S371208AbgDOPGx (ORCPT ); Wed, 15 Apr 2020 11:06:53 -0400 Received: from mx2.suse.de ([195.135.220.15]:60720 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394095AbgDOPGV (ORCPT ); Wed, 15 Apr 2020 11:06:21 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C82B1AFBB; Wed, 15 Apr 2020 15:06:18 +0000 (UTC) From: Nicolas Saenz Julienne To: saravanak@google.com, Greg Kroah-Hartman , devicetree@vger.kernel.org, Rob Herring , Frank Rowand Cc: Nicolas Saenz Julienne , linux-kernel@vger.kernel.org Subject: [PATCH 3/4] of: property: Move of_link_to_phandle() Date: Wed, 15 Apr 2020 17:05:48 +0200 Message-Id: <20200415150550.28156-4-nsaenzjulienne@suse.de> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20200415150550.28156-1-nsaenzjulienne@suse.de> References: <20200415150550.28156-1-nsaenzjulienne@suse.de> MIME-Version: 1.0 Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org This is done in order to simplify further changes. Signed-off-by: Nicolas Saenz Julienne --- drivers/of/property.c | 145 +++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 72 deletions(-) diff --git a/drivers/of/property.c b/drivers/of/property.c index 487685ff8bb19..2c7978ef22be1 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1014,78 +1014,6 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor, return false; } -/** - * of_link_to_phandle - Add device link to supplier from supplier phandle - * @dev: consumer device - * @sup_np: phandle to supplier device tree node - * - * Given a phandle to a supplier device tree node (@sup_np), this function - * finds the device that owns the supplier device tree node and creates a - * device link from @dev consumer device to the supplier device. This function - * doesn't create device links for invalid scenarios such as trying to create a - * link with a parent device as the consumer of its child device. In such - * cases, it returns an error. - * - * Returns: - * - 0 if link successfully created to supplier - * - -EAGAIN if linking to the supplier should be reattempted - * - -EINVAL if the supplier link is invalid and should not be created - * - -ENODEV if there is no device that corresponds to the supplier phandle - */ -static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, - u32 dl_flags) -{ - struct device *sup_dev; - int ret = 0; - struct device_node *tmp_np = sup_np; - int is_populated; - - of_node_get(sup_np); - /* - * Find the device node that contains the supplier phandle. It may be - * @sup_np or it may be an ancestor of @sup_np. - */ - while (sup_np && !of_find_property(sup_np, "compatible", NULL)) - sup_np = of_get_next_parent(sup_np); - if (!sup_np) { - dev_dbg(dev, "Not linking to %pOFP - No device\n", tmp_np); - return -ENODEV; - } - - /* Don't allow linking a device node as consumer of a disabled node */ - if (!of_device_is_available(sup_np)) { - dev_dbg(dev, "Not linking to %pOFP - Not available\n", sup_np); - of_node_put(sup_np); - return -ENODEV; - } - - /* - * Don't allow linking a device node as a consumer of one of its - * descendant nodes. By definition, a child node can't be a functional - * dependency for the parent node. - */ - if (of_is_ancestor_of(dev->of_node, sup_np)) { - dev_dbg(dev, "Not linking to %pOFP - is descendant\n", sup_np); - of_node_put(sup_np); - return -EINVAL; - } - sup_dev = get_dev_from_fwnode(&sup_np->fwnode); - is_populated = of_node_check_flag(sup_np, OF_POPULATED); - of_node_put(sup_np); - if (!sup_dev && is_populated) { - /* Early device without struct device. */ - dev_dbg(dev, "Not linking to %pOFP - No struct device\n", - sup_np); - return -ENODEV; - } else if (!sup_dev) { - return -EAGAIN; - } - if (!device_link_add(dev, sup_dev, dl_flags)) - ret = -EAGAIN; - put_device(sup_dev); - return ret; -} - /** * parse_prop_cells - Property parsing function for suppliers * @@ -1243,6 +1171,79 @@ static const struct supplier_bindings of_supplier_bindings[] = { {} }; +/** + * of_link_to_phandle - Add device link to supplier from supplier phandle + * @dev: consumer device + * @sup_np: phandle to supplier device tree node + * + * Given a phandle to a supplier device tree node (@sup_np), this function + * finds the device that owns the supplier device tree node and creates a + * device link from @dev consumer device to the supplier device. This function + * doesn't create device links for invalid scenarios such as trying to create a + * link with a parent device as the consumer of its child device. In such + * cases, it returns an error. + * + * Returns: + * - 0 if link successfully created to supplier + * - -EAGAIN if linking to the supplier should be reattempted + * - -EINVAL if the supplier link is invalid and should not be created + * - -ENODEV if there is no device that corresponds to the supplier phandle + */ +static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, + u32 dl_flags) +{ + struct device *sup_dev; + int ret = 0; + struct device_node *tmp_np = sup_np; + int is_populated; + + of_node_get(sup_np); + /* + * Find the device node that contains the supplier phandle. It may be + * @sup_np or it may be an ancestor of @sup_np. + */ + while (sup_np && !of_find_property(sup_np, "compatible", NULL)) + sup_np = of_get_next_parent(sup_np); + if (!sup_np) { + dev_dbg(dev, "Not linking to %pOFP - No device\n", tmp_np); + return -ENODEV; + } + + /* Don't allow linking a device node as consumer of a disabled node */ + if (!of_device_is_available(sup_np)) { + dev_dbg(dev, "Not linking to %pOFP - Not available\n", sup_np); + of_node_put(sup_np); + return -ENODEV; + } + + /* + * Don't allow linking a device node as a consumer of one of its + * descendant nodes. By definition, a child node can't be a functional + * dependency for the parent node. + */ + if (of_is_ancestor_of(dev->of_node, sup_np)) { + dev_dbg(dev, "Not linking to %pOFP - is descendant\n", sup_np); + of_node_put(sup_np); + return -EINVAL; + } + sup_dev = get_dev_from_fwnode(&sup_np->fwnode); + is_populated = of_node_check_flag(sup_np, OF_POPULATED); + of_node_put(sup_np); + if (!sup_dev && is_populated) { + /* Early device without struct device. */ + dev_dbg(dev, "Not linking to %pOFP - No struct device\n", + sup_np); + return -ENODEV; + } else if (!sup_dev) { + return -EAGAIN; + } + + if (!device_link_add(dev, sup_dev, dl_flags)) + ret = -EAGAIN; + put_device(sup_dev); + return ret; +} + /** * of_link_property - Create device links to suppliers listed in a property * @dev: Consumer device From patchwork Wed Apr 15 15:05:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Saenz Julienne X-Patchwork-Id: 1271241 Return-Path: X-Original-To: incoming-dt@patchwork.ozlabs.org Delivered-To: patchwork-incoming-dt@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=devicetree-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 492QgK4SHgz9sT5 for ; Thu, 16 Apr 2020 01:06:53 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394188AbgDOPGX (ORCPT ); Wed, 15 Apr 2020 11:06:23 -0400 Received: from mx2.suse.de ([195.135.220.15]:60738 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394096AbgDOPGW (ORCPT ); Wed, 15 Apr 2020 11:06:22 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id BD32EAFBF; Wed, 15 Apr 2020 15:06:19 +0000 (UTC) From: Nicolas Saenz Julienne To: saravanak@google.com, Greg Kroah-Hartman , devicetree@vger.kernel.org, Rob Herring , Frank Rowand Cc: Nicolas Saenz Julienne , linux-kernel@vger.kernel.org Subject: [PATCH 4/4] of: property: Avoid linking devices with circular dependencies Date: Wed, 15 Apr 2020 17:05:49 +0200 Message-Id: <20200415150550.28156-5-nsaenzjulienne@suse.de> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20200415150550.28156-1-nsaenzjulienne@suse.de> References: <20200415150550.28156-1-nsaenzjulienne@suse.de> MIME-Version: 1.0 Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org When creating a consumer/supplier relationship between devices it's essential to make sure they aren't supplying each other creating a circular dependency. Introduce a new function to check if such circular dependency exists between two device nodes and use it in of_link_to_phandle(). Fixes: a3e1d1a7f5fc ("of: property: Add functional dependency link from DT bindings") Signed-off-by: Nicolas Saenz Julienne --- NOTE: I feel of_link_is_circular() is a little dense, and could benefit from some abstraction/refactoring. That said, I'd rather get some feedback, before spending time on it. drivers/of/property.c | 50 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/drivers/of/property.c b/drivers/of/property.c index 2c7978ef22be1..74a5190408c3b 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1171,6 +1171,44 @@ static const struct supplier_bindings of_supplier_bindings[] = { {} }; +/** + * of_link_is_circular - Make sure potential link isn't circular + * + * @sup_np: Supplier device + * @con_np: Consumer device + * + * This function checks if @sup_np's properties contain a reference to @con_np. + * + * Will return true if there's a circular dependency and false otherwise. + */ +static bool of_link_is_circular(struct device_node *sup_np, + struct device_node *con_np) +{ + const struct supplier_bindings *s = of_supplier_bindings; + struct device_node *tmp; + bool matched = false; + struct property *p; + int i = 0; + + for_each_property_of_node(sup_np, p) { + while (!matched && s->parse_prop) { + while ((tmp = s->parse_prop(sup_np, p->name, i))) { + matched = true; + i++; + + if (tmp == con_np) + return true; + } + i = 0; + s++; + } + s = of_supplier_bindings; + matched = false; + } + + return false; +} + /** * of_link_to_phandle - Add device link to supplier from supplier phandle * @dev: consumer device @@ -1216,6 +1254,18 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, return -ENODEV; } + /* + * It is possible for consumer device nodes to also supply the device + * node they are consuming from. Creating an unwarranted circular + * dependency. + */ + if (of_link_is_circular(sup_np, dev->of_node)) { + dev_dbg(dev, "Not linking to %pOFP - Circular dependency\n", + sup_np); + of_node_put(sup_np); + return -ENODEV; + } + /* * Don't allow linking a device node as a consumer of one of its * descendant nodes. By definition, a child node can't be a functional