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, 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 BC11DC56202 for ; Sat, 21 Nov 2020 02:01:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7CBFD2240B for ; Sat, 21 Nov 2020 02:01:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Hn9e9Va7" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726644AbgKUCAs (ORCPT ); Fri, 20 Nov 2020 21:00:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54970 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726636AbgKUCAs (ORCPT ); Fri, 20 Nov 2020 21:00:48 -0500 Received: from mail-yb1-xb41.google.com (mail-yb1-xb41.google.com [IPv6:2607:f8b0:4864:20::b41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E6865C061A48 for ; Fri, 20 Nov 2020 18:00:47 -0800 (PST) Received: by mail-yb1-xb41.google.com with SMTP id v92so10358826ybi.4 for ; Fri, 20 Nov 2020 18:00:47 -0800 (PST) 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=j2wWUWZqYKnQ8zyB+N3c82zv7RQJgeeS1mhMIBzqBbs=; b=Hn9e9Va7VU3kQCVvYW94zaiI0ssP/bahrUDyg+we8Uf5fpBld+4Zb9pVNeSB94QuCR ueyC+rhR/83c0G0FHJWCSQT6wubY/9Fh7mnt/yDEuOL1q7j+tq2SkU3q47D5DVsW8fbm JIzgYzKyCbTSqr7PAsZWGOgFV52SY0BaoievmbUe0sVrZ+ESavXqBdhdyHx8PiTEA+tD howQO/65LTMCkAS/v6MfIJSicSq4gOyzQHiox526RKrP4EYzjCDyB6bdGKZjhtQHYj05 d2/nO3fNjoDZLIy5qkrlRinpX3f097uRM5QxNVHRsRNdte/0ZGOYSOZvHmwPU3JaH/29 2iNg== 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=j2wWUWZqYKnQ8zyB+N3c82zv7RQJgeeS1mhMIBzqBbs=; b=DpLlSBx28cT15jD+7odcVU5awAQ5nriRGny78s7KFxPwXUxmbhBwC9cPTNAE4BtcTI DVwTC+3rhP20c6oGGbnA5zhI89zN/+LC17pzF4NU+jwVgQabymL9PsqQ8fuEuAinhs73 HYu9jvmjfvFes1cm35qpgV7iXFOfV1P0nApSDxcCn/lGRiH8Ls6Oh4tMfqkOea3AwC2g bSgUZ5FqlXFL19T+xhk227O0LikTtjisSHeA74XNv+Ax6l5dkBUjlyMbKPh2d568uwQG Bu7Lb9NcGZfTvhyRbJ/xPlVY2FBIAlNgnD8UPisFsTSL8t6rcO0NFpF8xFPyIYD6r2mb c6kQ== X-Gm-Message-State: AOAM532nP/vYI99DdKCjxrN3v8hzFw740bmS8ZSizP0g212sl+Q5JWn8 gW0uVQJAM19u3mWsknecPfbaOXr2HFXjzpzaiAXzOQ== X-Google-Smtp-Source: ABdhPJyvwTxSbO8UktNx9w1dInoBL/DB/rklOZuS30YiPBNmF94x5zjdboDH7Drkq+bARi73mxzx9vI1xKTs/rMjwaQ= X-Received: by 2002:a25:df05:: with SMTP id w5mr36113642ybg.20.1605924046981; Fri, 20 Nov 2020 18:00:46 -0800 (PST) MIME-Version: 1.0 References: <20201104232356.4038506-1-saravanak@google.com> <20201104232356.4038506-13-saravanak@google.com> In-Reply-To: From: Saravana Kannan Date: Fri, 20 Nov 2020 18:00:11 -0800 Message-ID: Subject: Re: [PATCH v1 12/18] driver core: Add fw_devlink_parse_fwtree() To: "Rafael J. Wysocki" Cc: "Rafael J. Wysocki" , Len Brown , Greg Kroah-Hartman , Ard Biesheuvel , Rob Herring , Frank Rowand , Marc Zyngier , Thomas Gleixner , Tomi Valkeinen , Laurent Pinchart , Grygorii Strashko , "Cc: Android Kernel" , ACPI Devel Maling List , Linux Kernel Mailing List , linux-efi , "devicetree@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Mon, Nov 16, 2020 at 8:25 AM Rafael J. Wysocki wrote: > > On Thu, Nov 5, 2020 at 12:24 AM Saravana Kannan wrote: > > > > This function is a wrapper around fwnode_operations.add_links(). > > > > This function parses each node in a fwnode tree and create fwnode links > > for each of those nodes. The information for creating the fwnode links > > (the supplier and consumer fwnode) is obtained by parsing the properties > > in each of the fwnodes. > > > > This function also ensures that no fwnode is parsed more than once by > > marking the fwnodes as parsed. > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/base/core.c | 19 +++++++++++++++++++ > > include/linux/fwnode.h | 3 +++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index 4a0907574646..ee28d8c7ee85 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -1543,6 +1543,25 @@ static bool fw_devlink_is_permissive(void) > > return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY; > > } > > > > +static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode) > > +{ > > + if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED) > > + return; > > Why is the flag needed? > > Duplicate links won't be created anyway and it doesn't cause the tree > walk to be terminated. To avoid parsing a fwnode more than once. The cumulative impact of the repeated parsing is actually quite high. And I intentionally didn't do this check at the tree walk level because DT overlay can add/remove/change individual fwnodes and I want to reparse those when they are added while avoiding parsing other nodes that have already been parsed and not changed by DT overlay. > > > + > > + fwnode_call_int_op(fwnode, add_links, NULL); > > + fwnode->flags |= FWNODE_FLAG_LINKS_ADDED; > > +} > > + > > +static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode) > > +{ > > + struct fwnode_handle *child = NULL; > > + > > + fw_devlink_parse_fwnode(fwnode); > > + > > + while ((child = fwnode_get_next_available_child_node(fwnode, child))) > > I'd prefer > > for (child = NULL; child; child = > fwnode_get_next_available_child_node(fwnode, child)) I was about to change to this and then realized it won't work. It would have to be for (child = fwnode_get_next_available_child_node(fwnode, NULL)); child; child = fwnode_get_next_available_child_node(fwnode, child)) Is that really better? The while() seems a lot more readable to me. I don't have a strong opinion, so I'll go with whatever you say after reading this. > > > + fw_devlink_parse_fwtree(child); > > +} > > + > > static void fw_devlink_link_device(struct device *dev) > > { > > int fw_ret; > > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > > index ec02e1e939cc..9aaf9e4f3994 100644 > > --- a/include/linux/fwnode.h > > +++ b/include/linux/fwnode.h > > @@ -15,12 +15,15 @@ > > struct fwnode_operations; > > struct device; > > > > Description here, please. Ack > > > +#define FWNODE_FLAG_LINKS_ADDED BIT(0) > > + > > struct fwnode_handle { > > struct fwnode_handle *secondary; > > const struct fwnode_operations *ops; > > struct device *dev; > > struct list_head suppliers; > > struct list_head consumers; > > + u32 flags; > > That's a bit wasteful. Maybe u8 would suffice for the time being? Ack. -Saravana