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=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 41660C388F2 for ; Fri, 6 Nov 2020 08:30:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CF47B20724 for ; Fri, 6 Nov 2020 08:30:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="swGAdYh9" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726027AbgKFIaH (ORCPT ); Fri, 6 Nov 2020 03:30:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43818 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726598AbgKFIaF (ORCPT ); Fri, 6 Nov 2020 03:30:05 -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 25122C0613D2 for ; Fri, 6 Nov 2020 00:30:04 -0800 (PST) Received: by mail-yb1-xb41.google.com with SMTP id n142so453664ybf.7 for ; Fri, 06 Nov 2020 00:30:04 -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=rD9/ZK7971oPQsgTQDh0lNY/GvvQHJh4mKJIEvE3uW4=; b=swGAdYh9B4UcdKFMGOCYdOwSbZ937eS+baY29gHnJVzue1i6yqXwQF+5BKXpvKHr4Z RUmRrSbxIH9BqZJTbba/OvEVoirIt9d4jzAHqxguCotfDN1/W40/GMutFHguG2mUwzhi M4uq4gk+4QW2Ba7xpUV83eZSWr8OAGFvs+oJFFb6ym0AFvmVFkWocz+me2nV5VLsF+xA CaJMSqO+80UsA+xoFLE7I95pNMW3c4O9qWTgJBC5dOKZGc1+v9vPFzDgWjdMlQn1vHF0 cPyugi+aPoE2+5RhsKIIn37Ics5Kgj0P4Cerj20T/T4lcehFRB/jWr2wp0dbMGqKlhM3 Unxw== 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=rD9/ZK7971oPQsgTQDh0lNY/GvvQHJh4mKJIEvE3uW4=; b=i3mUkwldR3zIeMSTHokdqV7DcCmcwG79X6Holg5Z9HS9To1AKBgg/KwiHNIyeNKL14 1AClwLdTOAm1lSPbl6/Rg2bJyOz7G7MfMrItO7jWgO6C8zedp5bHBVJLzu3OMDb5a7d3 GYJThoNl8aZqYm5Kfl83fR9QkZOx5qSWLfescSQaKMPVI6crStH1lAPH1WAMysCNcyOI yz++lOM0X+Fu64fmE1o2Vnc6mOfuaN24VMFrkd7nvHwgdzdmHLKqvpVg4d1T+OgFiRVK cQeyMxj4mlTC6S6LDm5GsfJ+Gj1ASb+3WbkZ7kcezXjN+zzwZyQv03CexJPBFThdKYFz c6Xw== X-Gm-Message-State: AOAM5329EKHbXhP0Ru0u4vNoECCtiL8utUQc0sMuqSu800IvrEFg+gul dVqcJ9hHw0Ily91SFdMfzXRQHTmlVMrKUKlh1p1k4w== X-Google-Smtp-Source: ABdhPJwpd/q7RalIXkxlcZXBgsLbLVH8dlZ2vdkcPOrnPviYs7G3l0dFWmp/wsONIlbvLar8/0m7H/olvn4jVk0vsL4= X-Received: by 2002:a25:3:: with SMTP id 3mr1306315yba.412.1604651403053; Fri, 06 Nov 2020 00:30:03 -0800 (PST) MIME-Version: 1.0 References: <20201104232356.4038506-1-saravanak@google.com> <20201104232356.4038506-16-saravanak@google.com> <20201105094228.GE3439341@kroah.com> <20201106072247.GB2614221@kroah.com> <20201106075148.GA2619937@kroah.com> In-Reply-To: <20201106075148.GA2619937@kroah.com> From: Saravana Kannan Date: Fri, 6 Nov 2020 00:29:27 -0800 Message-ID: Subject: Re: [PATCH v1 15/18] of: property: Update implementation of add_links() to create fwnode links To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , "Rafael J. Wysocki" , Len Brown , Ard Biesheuvel , Rob Herring , Frank Rowand , Marc Zyngier , Thomas Gleixner , Tomi Valkeinen , Laurent Pinchart , Grygorii Strashko , Android Kernel Team , ACPI Devel Maling List , LKML , linux-efi , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Thu, Nov 5, 2020 at 11:51 PM Greg Kroah-Hartman wrote: > > On Thu, Nov 05, 2020 at 11:41:20PM -0800, Saravana Kannan wrote: > > On Thu, Nov 5, 2020 at 11:22 PM Greg Kroah-Hartman > > wrote: > > > > > > On Thu, Nov 05, 2020 at 03:25:56PM -0800, Saravana Kannan wrote: > > > > On Thu, Nov 5, 2020 at 1:41 AM Greg Kroah-Hartman > > > > wrote: > > > > > > > > > > On Wed, Nov 04, 2020 at 03:23:52PM -0800, Saravana Kannan wrote: > > > > > > The semantics of add_links() has changed from creating device link > > > > > > between devices to creating fwnode links between fwnodes. So, update the > > > > > > implementation of add_links() to match the new semantics. > > > > > > > > > > > > Signed-off-by: Saravana Kannan > > > > > > --- > > > > > > drivers/of/property.c | 150 ++++++++++++------------------------------ > > > > > > 1 file changed, 41 insertions(+), 109 deletions(-) > > > > > > > > > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > > > > > index 408a7b5f06a9..86303803f1b3 100644 > > > > > > --- a/drivers/of/property.c > > > > > > +++ b/drivers/of/property.c > > > > > > @@ -1038,33 +1038,9 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor, > > > > > > } > > > > > > > > > > > > /** > > > > > > - * of_get_next_parent_dev - Add device link to supplier from supplier phandle > > > > > > - * @np: device tree node > > > > > > - * > > > > > > - * Given a device tree node (@np), this function finds its closest ancestor > > > > > > - * device tree node that has a corresponding struct device. > > > > > > - * > > > > > > - * The caller of this function is expected to call put_device() on the returned > > > > > > - * device when they are done. > > > > > > - */ > > > > > > -static struct device *of_get_next_parent_dev(struct device_node *np) > > > > > > -{ > > > > > > - struct device *dev = NULL; > > > > > > - > > > > > > - of_node_get(np); > > > > > > - do { > > > > > > - np = of_get_next_parent(np); > > > > > > - if (np) > > > > > > - dev = get_dev_from_fwnode(&np->fwnode); > > > > > > - } while (np && !dev); > > > > > > - of_node_put(np); > > > > > > - return dev; > > > > > > -} > > > > > > - > > > > > > -/** > > > > > > - * of_link_to_phandle - Add device link to supplier from supplier phandle > > > > > > - * @dev: consumer device > > > > > > - * @sup_np: phandle to supplier device tree node > > > > > > + * of_link_to_phandle - Add fwnode link to supplier from supplier phandle > > > > > > + * @con_np: consumer device tree node > > > > > > + * @sup_np: 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 > > > > > > @@ -1074,16 +1050,14 @@ static struct device *of_get_next_parent_dev(struct device_node *np) > > > > > > * cases, it returns an error. > > > > > > * > > > > > > * Returns: > > > > > > - * - 0 if link successfully created to supplier > > > > > > - * - -EAGAIN if linking to the supplier should be reattempted > > > > > > + * - 0 if fwnode link successfully created to supplier > > > > > > * - -EINVAL if the supplier link is invalid and should not be created > > > > > > - * - -ENODEV if there is no device that corresponds to the supplier phandle > > > > > > + * - -ENODEV if struct device will never be create for supplier > > > > > > */ > > > > > > -static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, > > > > > > - u32 dl_flags) > > > > > > +static int of_link_to_phandle(struct device_node *con_np, > > > > > > + struct device_node *sup_np) > > > > > > { > > > > > > - struct device *sup_dev, *sup_par_dev; > > > > > > - int ret = 0; > > > > > > + struct device *sup_dev; > > > > > > struct device_node *tmp_np = sup_np; > > > > > > > > > > > > of_node_get(sup_np); > > > > > > @@ -1106,7 +1080,8 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, > > > > > > } > > > > > > > > > > > > if (!sup_np) { > > > > > > - dev_dbg(dev, "Not linking to %pOFP - No device\n", tmp_np); > > > > > > + pr_debug("Not linking %pOFP to %pOFP - No device\n", > > > > > > + con_np, tmp_np); > > > > > > > > > > Who is calling this function without a valid dev pointer? > > > > > > > > Sorry, I plan to delete the "dev" parameter as it's not really used > > > > anywhere. I'm trying to do that without causing build time errors and > > > > making the series into digestible small patches. > > > > > > > > I can do the deletion of the parameter as a Patch 19/19. Will that work? > > > > > > That's fine, but why get rid of dev? The driver core works on these > > > things, and we want errors/messages/warnings to spit out what device is > > > causing those issues. It is fine to drag around a struct device pointer > > > just for messages, that's to be expected, and is good. > > > > In general I agree. If the fwnode being parsed is related to the dev, > > it's nice to have the dev name in the logs. > > > > But in this instance I feel it's analogous to printing the PID that's > > parsing the fwnode -- kinda irrelevant. The device in question can > > appear very random and it'll just cause more confusion than be of help > > if it shows up in the logs. > > > > For example it can be something like: > > : linking to > > > > If the device was actually that of the wifi fwnode of the iommu > > fwnode, I agree it'll be useful to carry around the dev even if it's > > just for logs. > > > > Hope that makes sense. > > Not really, as the device here should be the one that is doing the > linking, so why doesn't that matter? No, the links being created here are fwnode links. Also, when a fwnode is parsed (because the device corresponding to it is added), the whole fwnode tree under it is also parsed. So the device in question won't be either the consumer or the supplier when the log is printed. So the device will just be some random ancestor up the parent chain. -Saravana