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=-18.3 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,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 516F0C4320A for ; Mon, 30 Aug 2021 19:03:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 29D5060C40 for ; Mon, 30 Aug 2021 19:03:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232744AbhH3TEr (ORCPT ); Mon, 30 Aug 2021 15:04:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56936 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231715AbhH3TEq (ORCPT ); Mon, 30 Aug 2021 15:04:46 -0400 Received: from mail-yb1-xb2b.google.com (mail-yb1-xb2b.google.com [IPv6:2607:f8b0:4864:20::b2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C55BDC061760 for ; Mon, 30 Aug 2021 12:03:52 -0700 (PDT) Received: by mail-yb1-xb2b.google.com with SMTP id k78so26954462ybf.10 for ; Mon, 30 Aug 2021 12:03:52 -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=z2bxnmb0ihpOea9bKFBJwN2QiE7nKKp/Kf8m8WRCLmQ=; b=CNXv9uQY/btGpuGZCGIeiwDkbadXPvVPJGX8i+eMZ94MOA1GM1y93ZATn5Ei0ThY+i JTjfMEaijan7LRBtEtM5It2Yg9iQ5piQQycZRmmHt+mIfxP8D9saNFiGT+F6FaI2mnN1 LnCl45iXefqt9y4KBuDEFRCmohy3sNSy1hsaRqjNAcY3XBhkC/aCTPy7kun43JxxXbUF ZiZhgvGhYNK+g9yIztL2E04zfwdkMgBpMRTBngU0KkgHBmqMJtqvDX4lbzZvz4iLVO3Y Lu1pEj2hQ4LRYAsuo6jGb617+9fd0pcLfd/EWU8aZvpFFKvcMZrYsVkpo5VpUeG5F5gK zKUw== 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=z2bxnmb0ihpOea9bKFBJwN2QiE7nKKp/Kf8m8WRCLmQ=; b=ZGAWkMqBHqKTflR60QH30WKsVKcDNp5IiJbqXi4oMU3BMIFqjRYtqxUfXWRR37z+gN +bSSewEmIHP7QaUv/Fwb+4zIKvjgy2yGkdP5PybV/DzKe6xFgqyjZsMp3fxOBTKAkljZ m5uDMzcQ1JfRks9q/2GKCLYW0410uwGIFQgMVVXgvZGk2F0RMqRAJiVUnU15yC0wxQqG 2onC9UUPzNPEOK7K2JZY3y9NVHYyM7w6qu/DfKKv2322D55TmxLRZUajHWADdyameGxh PL7LQJlDd4A600evTJuH00XEOoqMmglgmRfbIbOObuRe/4M75Rafi6jdt4MiB+WSHDUH rDHg== X-Gm-Message-State: AOAM530s96jeA+Xx/arP6dr+7d3QlyK8tg/T97usOGP45ipAhtBWhlJJ mcrokN6/S9kNYZU04i6J3CrCmNsYko90k8C7DZ+OqQ== X-Google-Smtp-Source: ABdhPJz9aD6FzTGpzWANYNoSRWYbWlVX4WNltzALVRrIK1we8xo3YfH38PlE1sT1b+6yUjbcDmqzWkoGLExhUMGBhIE= X-Received: by 2002:a25:804:: with SMTP id 4mr24050887ybi.346.1630350231696; Mon, 30 Aug 2021 12:03:51 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Saravana Kannan Date: Mon, 30 Aug 2021 12:03:15 -0700 Message-ID: Subject: Re: [PATCH v1 1/2] driver core: fw_devlink: Add support for FWNODE_FLAG_BROKEN_PARENT To: Andrew Lunn Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Linus Walleij , Vivien Didelot , Florian Fainelli , Vladimir Oltean , "David S. Miller" , Jakub Kicinski , Len Brown , Alvin Sipraga , kernel-team@android.com, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-acpi@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Sat, Aug 28, 2021 at 10:01 AM Andrew Lunn wrote: > > On Fri, Aug 27, 2021 at 02:33:02PM -0700, Saravana Kannan wrote: > > On Fri, Aug 27, 2021 at 1:11 PM Andrew Lunn wrote: > > > > > > > > I've not yet looked at plain Ethernet drivers. This pattern could also > > > > > exist there. And i wonder about other complex structures, i2c bus > > > > > multiplexors, you can have interrupt controllers as i2c devices, > > > > > etc. So the general case could exist in other places. > > > > > > > > I haven't seen any generic issues like this reported so far. It's only > > > > after adding phy-handle that we are hitting these issues with DSA > > > > switches. > > > > > > Can you run your parser over the 2250 DTB blobs and see how many > > > children have dependencies on a parent? That could give us an idea how > > > many moles need whacking. And maybe, where in the tree they are > > > hiding? > > > > You are only responding to part of my email. As I said in my previous > > email: "There are plenty of cases where it's better to delay the child > > device's probe until the parent finishes. You even gave an example[7] > > where it would help avoid unnecessary deferred probes." Can you please > > give your thoughts on the rest of the points I made too? > > I must admit, my main problem at the moment is -rc1 in two weeks > time. It seems like a number of board with Ethernet switches will be > broken, that worked before. phy-handle is not limited to switch > drivers, it is also used for Ethernet drivers. So it could be, a > number of Ethernet drivers are also going to be broken in -rc1? Again, in those cases, based on your FEC example, fw_devlink=on actually improves things. > But the issues sounds not to be specific to phy-handle, but any > phandle that points back to a parent. I feel like I'm going in circles here. This statement is not true. Please read my previous explanations. > So it could be drivers outside > of networking are also going to be broken with -rc1? > You have been very focused on one or two drivers. I would much rather > see you getting an idea of how wide spread this problem is, and what > should we do for -rc1? Again, it's not a widespread problem as I explained before. fw_devlink=on has been the default for 2 kernel versions now. With no unfixed reported issues. > Even if modifying DSA drivers to component drivers is possible, while > not breaking backwards compatibility with DT, It should be possible without needing any DT changes. > it is not going to > happen over night. That is something for the next merge window, not > this merge window. Right, I wasn't suggesting the component driver option be implemented right away. We were talking about what the longer term proper fix would be for DSA (and Ethernet if we actually find issues there) and who would do it. That's what I hope this discussion could be. Also, if we replace Patch 2/2 in this series with the patch below, it will work as a generic quick fix for DSA that we could use for -rc1. And if we still have issues reported on the phy-handle patch by -rc5 or so, we could revert the phy-handle patch then so that v5.15 isn't broken. -Saravana --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -1286,6 +1286,17 @@ static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn) { int err; + /* A lot of switch devices have their PHYs as child devices and have + * the PHYs depend on the switch as a supplier (Eg: interrupt + * controller). With fw_devlink=on, that means the PHYs will defer + * probe until the probe() of the switch completes. However, the way + * the DSA framework is designed, the PHYs are expected to be probed + * successfully before the probe() of the switch completes. + * + * So, mark the switch devices as a "broken parent" so that fw_devlink + * knows not to create device links between PHYs and the parent switch. + */ + np->fwnode.flags |= FWNODE_FLAG_BROKEN_PARENT; err = dsa_switch_parse_member_of(ds, dn); if (err) return err;