| Message ID | 20200420120101.31405-3-nsaenzjulienne@suse.de |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | of: property: fw_devlink misc fixes | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 21 lines checked" |
On Mon, Apr 20, 2020 at 5:02 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > When creating a consumer/supplier relationship between two devices, > make sure the supplier node is actually active. Otherwise this will > create a link relationship 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 <nsaenzjulienne@suse.de> > > --- > > Changes since v2: > - Correct code comment > - Use already available return handling code > > Changes since v1: > - Move availability check into the compatible search code and stop if > node disabled > > drivers/of/property.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index dc034eb45defd..7bcf31ba717d8 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1045,8 +1045,20 @@ static int of_link_to_phandle(struct device *dev, struct device_node *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)) > + while (sup_np) { > + > + /* Don't allow linking to a disabled supplier */ > + if (!of_device_is_available(sup_np)) { > + of_node_put(sup_np); > + sup_np = NULL; > + } > + > + if (of_find_property(sup_np, "compatible", NULL)) > + break; > + > 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; Thanks for the fix! Reviewed-by: Saravana Kannan <saravanak@google.com>
On Mon, 20 Apr 2020 14:01:02 +0200, Nicolas Saenz Julienne wrote: > When creating a consumer/supplier relationship between two devices, > make sure the supplier node is actually active. Otherwise this will > create a link relationship 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 <nsaenzjulienne@suse.de> > > --- > > Changes since v2: > - Correct code comment > - Use already available return handling code > > Changes since v1: > - Move availability check into the compatible search code and stop if > node disabled > > drivers/of/property.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > Applied, thanks. Rob
diff --git a/drivers/of/property.c b/drivers/of/property.c index dc034eb45defd..7bcf31ba717d8 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1045,8 +1045,20 @@ static int of_link_to_phandle(struct device *dev, struct device_node *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)) + while (sup_np) { + + /* Don't allow linking to a disabled supplier */ + if (!of_device_is_available(sup_np)) { + of_node_put(sup_np); + sup_np = NULL; + } + + if (of_find_property(sup_np, "compatible", NULL)) + break; + 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;
When creating a consumer/supplier relationship between two devices, make sure the supplier node is actually active. Otherwise this will create a link relationship 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 <nsaenzjulienne@suse.de> --- Changes since v2: - Correct code comment - Use already available return handling code Changes since v1: - Move availability check into the compatible search code and stop if node disabled drivers/of/property.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)