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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable 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 08039C4320E for ; Wed, 1 Sep 2021 20:57:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E38A561027 for ; Wed, 1 Sep 2021 20:57:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231866AbhIAU55 (ORCPT ); Wed, 1 Sep 2021 16:57:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35542 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238818AbhIAU5y (ORCPT ); Wed, 1 Sep 2021 16:57:54 -0400 Received: from mail-yb1-xb2f.google.com (mail-yb1-xb2f.google.com [IPv6:2607:f8b0:4864:20::b2f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1FCC3C0613C1 for ; Wed, 1 Sep 2021 13:56:57 -0700 (PDT) Received: by mail-yb1-xb2f.google.com with SMTP id r4so1271206ybp.4 for ; Wed, 01 Sep 2021 13:56:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L0G6R62N1gO7DOjfEZAPNKqynBW5XZ5XNt6xZIMqank=; b=u4SEUsyX5hLbDNCYJrkK/Dq3ED5N6gzAoGqvPapHzprtM++i9ndf4lKuBAebuB3Xif VZH3nWhY/dR+Zs4fJg4ZZ6PoJ+tnml+weetYIwbOsyjJNbfwVCwsxvQaeXOrN7paZlo/ vo5HRCSvtQ+mZgOd7pOwxXVXqGjVEbZNfbSow13HA2/Eq+58R82NmEKWnd4uArDPHtWw z0H1hH0zf19jG8Cs5DHbQR06BHcK+vrSD6lfIL8T7TmlXILI9qVAmsOP2nIQIlSXVDfG lTwOR4y7cyiLt4m8gld7ZI9Kj+iZS1kSOOKp4b6cyN567suVGHJLacrMqiRuxIevaT4o O3oA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L0G6R62N1gO7DOjfEZAPNKqynBW5XZ5XNt6xZIMqank=; b=YYVUbrP7IyL+2xqOV30N8ti0EZrTgnz6xXVtBSjvbo3UwvG3cgxYc7ZMtCNn0zgnIM /36yQkAK7eEWoIUdUp4DZUMSkUaDHBRE3PKPtrLuCmEIzrf6VwnsYQAnXE0ldo+Zu+m3 CSn8+z8ZXwFuvTNa2fVbb5tXLottkTXJD2GNGr8zRbs802ZN3BhYCGjcV3h3r28qm03h 5m6tV0kLop69H0Qs9S9B7T4ex9WRguLn31qrulQlqDnrjWvM1wGRDZTkXulydlpvhioc 4U8Smuzh4dBCRKg00qxs7ynCuFZpAV/VM2wfnTQLb+5GvmbWgtALSG662TKhK1dBrrz/ p5Sg== X-Gm-Message-State: AOAM531s1uWAIj1petjZxUKKOXrlkTJqBnFCCQQpZlWhh/0l2bZVkz+X KpRDy1iuwUz7yrtrXnRe2cR4/UWrsRGojcdfHqAoig== X-Google-Smtp-Source: ABdhPJzF7Zt72N0YmUtW1wz8l2WyJWxKJcMNBmyngHxWM/3Tu2UpcRUGekYR0bcdcfaQBUAjDifDWgmEvC0bo9QmANc= X-Received: by 2002:a25:6746:: with SMTP id b67mr2325216ybc.96.1630529816125; Wed, 01 Sep 2021 13:56:56 -0700 (PDT) MIME-Version: 1.0 References: <20210831102125.624661-1-ulf.hansson@linaro.org> In-Reply-To: From: Saravana Kannan Date: Wed, 1 Sep 2021 13:56:20 -0700 Message-ID: Subject: Re: [PATCH 1/2] of: property: fw_devlink: Rename 'node_not_dev' to 'optional_con_dev' To: Ulf Hansson Cc: Rob Herring , DTML , "Rafael J . Wysocki" , Stephen Boyd , Dmitry Osipenko , Linux PM , Linux Kernel Mailing List , Linux ARM Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 1, 2021 at 12:45 AM Ulf Hansson wrote: > > On Tue, 31 Aug 2021 at 19:31, Saravana Kannan wrote: > > > > On Tue, Aug 31, 2021 at 3:21 AM Ulf Hansson 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 > > > doesn't match the behaviour of the code in of_link_property(). > > > > > > To make the behaviour consistent with the description, let's rename the > > > member to "optional_con_dev" and clarify the corresponding comment. > > > > > > Signed-off-by: Ulf Hansson > > > --- > > > drivers/of/property.c | 9 +++++---- > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > > index 6c028632f425..2babb1807228 100644 > > > --- a/drivers/of/property.c > > > +++ b/drivers/of/property.c > > > @@ -1249,7 +1249,8 @@ 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. > > > + * @optional_con_dev: The consumer node containing the property may not be a > > > + * device, then try finding one from an ancestor node. > > > > Nak. This flag is not about "may not be". This is explicitly for > > "never a device". It has to do with stuff like remote-endpoint which > > is never listed under the root node of the device node. Your > > documentation change is changing the meaning of the flag. > > Okay, fair enough. > > Although, as stated in the commit message this isn't the way code > behaves. Shouldn't we at least make the behaviour consistent with the > description of the 'node_not_dev' flag? I know what you mean, but if you use the flag correctly (where the phandle pointed to will never be a device with compatible property), the existing code would work correctly. And since the flag is relevant only in this file, it's easy to keep it correct. I'd just leave it as is. -Saravana > > Along the lines of the below patch then? > > From: Ulf Hansson > Date: Wed, 1 Sep 2021 09:28:03 +0200 > Subject: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' > is set > > 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, > starting from the node it gets passed to it. > > Make the behaviour consistent with the description of the 'node_not_dev' > flag, by passing the parent node to of_get_compat_node() instead. > > Signed-off-by: Ulf Hansson > --- > drivers/of/property.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 6c028632f425..16ee017884b8 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 > @@ -1416,7 +1427,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 > > [...] > > Kind regards > Uffe