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 D3AB6C64E69 for ; Sat, 21 Nov 2020 02:01:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AFB7B223B0 for ; Sat, 21 Nov 2020 02:01:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="kPJh46MI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726985AbgKUCBU (ORCPT ); Fri, 20 Nov 2020 21:01:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726913AbgKUCBU (ORCPT ); Fri, 20 Nov 2020 21:01:20 -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 EAB82C061A48 for ; Fri, 20 Nov 2020 18:01:19 -0800 (PST) Received: by mail-yb1-xb41.google.com with SMTP id o71so10355002ybc.2 for ; Fri, 20 Nov 2020 18:01:19 -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=lptBBbGushlHJUmT2AIfJv9BrkLAXhn95EN3DEBkcQw=; b=kPJh46MIin1X5MmomtGmy+IMegd1fnFrRN5XCBU0BDeFY/SPYlUaHWi2XwcDhT8zpH llJb0JTUYtli9u94w48Nt/YENr3SUlo3KoMOOezh2njap0ClbfsHpIJZwSGYeY5lUegd K/6UvEMs3rCivA+uscms3XDbhVJzYlUPLwXdky9RO/Q3oL/IbcDhdhNN7NrDxRf6C2Ar t5f0tY/S4VosrrEn5PqrH/JLQntmambrJ1huRNGnAzroEDAf7rzsbyS65Pg5QH5AdaPX HjLiChlxtcBtXIDF+rm14mSpZozeinDkEqIpzhD1viKtwqrTe0HDrNLe/9CHIcvjJiuE 2Osw== 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=lptBBbGushlHJUmT2AIfJv9BrkLAXhn95EN3DEBkcQw=; b=Az2WywxG2SM/xGa5RQmDVvAuydoIECoCUFVfLUux2A+Mjql229qJtNDyhNZc27gYfZ z2xu8yvKW2AMzLGZnIE5py1VzyJm2cn6brpaeqDIDDRK6HvaiYykiYqv8qHY/Z1TYC0Z KM9lRJEkK3Q0pO18NfJXduylQz9jlmDSkPLuj0gF0/+VdNC5L6/k5bsfiMJOGTo9xzGB eOC2rEk3CXdeApnQ2pREEbg++UnyAXLw3UWGGYCyaHbS2rztrYNqukUtlVsVFSvko8Z1 DAzB7rqF4bImGW8bv1ZVFd+NgCG1Z1afwHANrdGHmH5bNvOfFEAac7W0xvDnnHJ+6mIX PokA== X-Gm-Message-State: AOAM533friPGtf1f9qG7Hq7hutVS/JQxXWPvBY7mJcCFRT1621xo1KIu cXZVuwdIFR4IqHkb1lQyVKywx6M+bVcZKcpwYUrfGQ== X-Google-Smtp-Source: ABdhPJwL+RWOtK1NK29JcTWnyDUn955B8Vc2hZVf1e8KN5TtA1N4vftbkBqKUHcWaybv95OBkNNlmFjIuyCz4UCouZk= X-Received: by 2002:a25:c483:: with SMTP id u125mr23456504ybf.310.1605924079027; Fri, 20 Nov 2020 18:01:19 -0800 (PST) MIME-Version: 1.0 References: <20201104232356.4038506-1-saravanak@google.com> <20201104232356.4038506-18-saravanak@google.com> In-Reply-To: From: Saravana Kannan Date: Fri, 20 Nov 2020 18:00:43 -0800 Message-ID: Subject: Re: [PATCH v1 17/18] driver core: Add helper functions to convert fwnode links to device links 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:57 AM Rafael J. Wysocki wrote: > > On Thu, Nov 5, 2020 at 12:24 AM Saravana Kannan wrote: > > > > Add helper functions __fw_devlink_link_to_consumers() and > > __fw_devlink_link_to_suppliers() that convert fwnode links to device > > links. > > > > __fw_devlink_link_to_consumers() is for creating: > > - Device links between a newly added device and all its consumer devices > > that have been added to driver core. > > - Proxy SYNC_STATE_ONLY device links between the newly added device and > > the parent devices of all its consumers that have not been added to > > driver core yet. > > > > __fw_devlink_link_to_suppliers() is for creating: > > - Device links between a newly added device and all its supplier devices > > - Proxy SYNC_STATE_ONLY device links between the newly added device and > > all the supplier devices of its child device nodes. > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/base/core.c | 228 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 228 insertions(+) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index d51dd564add1..0c87ff949d81 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -1585,6 +1585,234 @@ static struct device *fwnode_get_next_parent_dev(struct fwnode_handle *fwnode) > > return dev; > > } > > > > +/** > > + * fw_devlink_create_devlink - Create a device link from a consumer to fwnode > > Have you considered renaming "fw_devlink" to "fwnode_link"? I avoided using fwnode_link prefix because it's not related to the fwnode link APIs. This is a fw_devlink feature related code and hence the fw_devlink prefix. I could just call it fw_devlink_create() or fw_devlink_create_links() or fwnode_to_dev_link() if that's less confusing. Any preferences? > That would be much less confusing IMO and the name of this function > would be clearer. > > > + * @con - Consumer device for the device link > > + * @sup - fwnode handle of supplier > > + * > > + * This function will try to create a device link between the consumer and the > > + * supplier devices. > > "... between consumer device @con and the supplier device represented > by @sup" (and see below). > > > + * > > + * The supplier has to be provided as a fwnode because incorrect cycles in > > + * fwnode links can sometimes cause the supplier device to never be created. > > + * This function detects such cases and returns an error if a device link being > > + * created in invalid. > > "... returns an error if it cannot create a device link from the > consumer to a missing supplier." > > > + * > > + * Returns, > > + * 0 on successfully creating a device link > > + * -EINVAL if the device link being attempted is invalid > > "if the device link cannot be created as expected" > > > + * -EAGAIN if the device link needs to be attempted again in the future > > "if the device link cannot be created right now, but it may be > possible to do that in the future." Ack to all the documentation comments above. > > > + */ > > +static int fw_devlink_create_devlink(struct device *con, > > + struct fwnode_handle *sup, u32 flags) > > I'd call the second arg sup_handle to indicate that it is not a struct device. Ack > > > +{ > > + struct device *sup_dev, *sup_par_dev; > > + int ret = 0; > > + > > + sup_dev = get_dev_from_fwnode(sup); > > + /* > > + * If we can't find the supplier device from its fwnode, it might be > > + * due to a cyclic dependcy between fwnodes. Some of these cycles can > > dependency > Ack > > + * be broken by applying logic. Check for these types of cycles and > > + * break them so that devices in the cycle probe properly. > > + */ > > I would do > > if (sup_dev) { > if (!device_link_add(con, sup_dev, flags)) > ret = -EINVAL; > > put_device(sup_dev); > return ret; > } > > here and the cycle detection (error code path) below, possibly using > the same dev pointer. Nice suggestion, thanks! -Saravana