From: Ulf Hansson <ulf.hansson@linaro.org> To: Rob Herring <robh@kernel.org>, devicetree@vger.kernel.org Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>, Saravana Kannan <saravanak@google.com>, Stephen Boyd <sboyd@kernel.org>, Dmitry Osipenko <digetx@gmail.com>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set Date: Thu, 18 Nov 2021 15:02:06 +0100 [thread overview] Message-ID: <CAPDyKFqdjpT2fzpq7LGDotkzwruZ7OjKnkw=_O+2eV9bgdYvkQ@mail.gmail.com> (raw) In-Reply-To: <20210902090221.820254-1-ulf.hansson@linaro.org> On Thu, 2 Sept 2021 at 11:02, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > In the struct supplier_bindings the member 'node_not_dev' is described as > "The consumer node containing the property is never a device.", but that is > inconsistent with the behaviour of the code in of_link_property(), as it > calls of_get_compat_node() that starts parsing for a compatible property > from the node it gets passed to it. The proper behaviour is to start at the > node's parent, so let's do that. > > While at it, let's take the opportunity to update the description of the > 'node_not_dev' flag, as to clarify its purpose. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Rob, just a gentle reminder about this one. Should I perhaps resend? Kind regards Uffe > --- > drivers/of/property.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 6c028632f425..a94d007be416 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1075,6 +1075,17 @@ static struct device_node *of_get_compat_node(struct device_node *np) > return np; > } > > +static struct device_node *of_get_compat_node_parent(struct device_node *np) > +{ > + struct device_node *parent, *node; > + > + parent = of_get_parent(np); > + node = of_get_compat_node(parent); > + of_node_put(parent); > + > + return node; > +} > + > /** > * of_link_to_phandle - Add fwnode link to supplier from supplier phandle > * @con_np: consumer device tree node > @@ -1249,7 +1260,9 @@ static struct device_node *parse_##fname(struct device_node *np, \ > * @parse_prop.index: For properties holding a list of phandles, this is the > * index into the list > * @optional: Describes whether a supplier is mandatory or not > - * @node_not_dev: The consumer node containing the property is never a device. > + * @node_not_dev: The consumer node containing the property is never converted > + * to a struct device. Instead, parse ancestor nodes for the > + * compatible property to find a node corresponding to a device. > * > * Returns: > * parse_prop() return values are > @@ -1416,7 +1429,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name) > struct device_node *con_dev_np; > > con_dev_np = s->node_not_dev > - ? of_get_compat_node(con_np) > + ? of_get_compat_node_parent(con_np) > : of_node_get(con_np); > matched = true; > i++; > -- > 2.25.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Ulf Hansson <ulf.hansson@linaro.org> To: Rob Herring <robh@kernel.org>, devicetree@vger.kernel.org Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>, Saravana Kannan <saravanak@google.com>, Stephen Boyd <sboyd@kernel.org>, Dmitry Osipenko <digetx@gmail.com>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set Date: Thu, 18 Nov 2021 15:02:06 +0100 [thread overview] Message-ID: <CAPDyKFqdjpT2fzpq7LGDotkzwruZ7OjKnkw=_O+2eV9bgdYvkQ@mail.gmail.com> (raw) In-Reply-To: <20210902090221.820254-1-ulf.hansson@linaro.org> On Thu, 2 Sept 2021 at 11:02, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > In the struct supplier_bindings the member 'node_not_dev' is described as > "The consumer node containing the property is never a device.", but that is > inconsistent with the behaviour of the code in of_link_property(), as it > calls of_get_compat_node() that starts parsing for a compatible property > from the node it gets passed to it. The proper behaviour is to start at the > node's parent, so let's do that. > > While at it, let's take the opportunity to update the description of the > 'node_not_dev' flag, as to clarify its purpose. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Rob, just a gentle reminder about this one. Should I perhaps resend? Kind regards Uffe > --- > drivers/of/property.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 6c028632f425..a94d007be416 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1075,6 +1075,17 @@ static struct device_node *of_get_compat_node(struct device_node *np) > return np; > } > > +static struct device_node *of_get_compat_node_parent(struct device_node *np) > +{ > + struct device_node *parent, *node; > + > + parent = of_get_parent(np); > + node = of_get_compat_node(parent); > + of_node_put(parent); > + > + return node; > +} > + > /** > * of_link_to_phandle - Add fwnode link to supplier from supplier phandle > * @con_np: consumer device tree node > @@ -1249,7 +1260,9 @@ static struct device_node *parse_##fname(struct device_node *np, \ > * @parse_prop.index: For properties holding a list of phandles, this is the > * index into the list > * @optional: Describes whether a supplier is mandatory or not > - * @node_not_dev: The consumer node containing the property is never a device. > + * @node_not_dev: The consumer node containing the property is never converted > + * to a struct device. Instead, parse ancestor nodes for the > + * compatible property to find a node corresponding to a device. > * > * Returns: > * parse_prop() return values are > @@ -1416,7 +1429,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name) > struct device_node *con_dev_np; > > con_dev_np = s->node_not_dev > - ? of_get_compat_node(con_np) > + ? of_get_compat_node_parent(con_np) > : of_node_get(con_np); > matched = true; > i++; > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-18 14:02 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-02 9:02 [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set Ulf Hansson 2021-09-02 9:02 ` Ulf Hansson 2021-09-02 17:09 ` Saravana Kannan 2021-09-02 17:09 ` Saravana Kannan 2021-11-18 14:02 ` Ulf Hansson [this message] 2021-11-18 14:02 ` Ulf Hansson 2021-11-18 14:14 ` Rob Herring 2021-11-18 14:14 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAPDyKFqdjpT2fzpq7LGDotkzwruZ7OjKnkw=_O+2eV9bgdYvkQ@mail.gmail.com' \ --to=ulf.hansson@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=digetx@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=rjw@rjwysocki.net \ --cc=robh@kernel.org \ --cc=saravanak@google.com \ --cc=sboyd@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.