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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 399CCC25B10 for ; Mon, 29 Apr 2024 11:56:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UiTMAE3m6+jpn3/P7ZijMTYYMypZ4m65gEVw4aWP0Bo=; b=h6BeGWYEEWHlT9 M/pG++LbYtaofwhaWpSi08ySmZX3nSRadVT23pD448gVUOPGj3UdoESRsMbP9mYqOMkW0VJKkf5Dt UJf11x0SFDlttbGsfTSri7fVizVAK56reR76Sdr3Or3yYucsiMvS5cSIIefuaJAWTjesl48XCNRbK EwPseozk1U5bXnSmwVUACv/ZDgIJsvAsE4TF7fry/6ec1hTunF6r0fpXPGiTIAGo0zAPgZeM1G45x SCfu2FHeetRvvge3yITmdGT9Z8DFrG0zyJg++gqAUhZvmW1uIptZZHtgg44FSB6YrKrJ+w/+pLSVq eiUCQAPmWj2/fTMvuiVw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1PcD-00000002ZwI-0oRr; Mon, 29 Apr 2024 11:56:13 +0000 Received: from relay7-d.mail.gandi.net ([217.70.183.200]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1Pbt-00000002ZjH-3VsA for linux-arm-kernel@lists.infradead.org; Mon, 29 Apr 2024 11:56:11 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 4353920007; Mon, 29 Apr 2024 11:55:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1714391747; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zVjiQz4+6eouGFpCfe3kaMY1C/HczOpHYg45IACIDrE=; b=T1FzTGcpXo6qFjZOOhPVXy51vNt64OAwvwBr0PYKHGBWH7+aCu4WN5aH/6bf7tfJ72zdAA zqBFcdqxnt2xdUWXoVtN5fzfduBZ8eNAuAmFdFy4kewweHnMkczztNtQEmgNQwKqRCA4xm RI71/BN4bgcmRZhBiuRCYhmkALNS3VnPiSkGzbUf/aJEgz0kXOmTWVt7wgrhJgnnQ2fAGD GFonoGeqyB7Ob+L7VDSI0BTXQSTSxyHCCmXzgjpGwOtBmBbVGOZ7Nw80ODv4VxxdFQdNC3 fGS5En/VIH0nhmpLBdzYG1YywAPhU4o0YMjn1QBqujNIU3jDbnyG0EuEhtLJJw== Date: Mon, 29 Apr 2024 13:55:42 +0200 From: Maxime Chevallier To: Heiner Kallweit Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com, Andrew Lunn , Jakub Kicinski , Eric Dumazet , Paolo Abeni , Russell King , linux-arm-kernel@lists.infradead.org, Christophe Leroy , Herve Codina , Florian Fainelli , Vladimir Oltean , =?UTF-8?B?S8O2cnk=?= Maincent , Jesse Brandeburg , Marek =?UTF-8?B?QmVow7pu?= , Piergiorgio Beruto , Oleksij Rempel , =?UTF-8?B?Tmljb2zDsg==?= Veronese , Simon Horman , mwojtas@chromium.org, Nathan Chancellor , Antoine Tenart Subject: Re: [PATCH net-next] net: phy: phy_link_topology: Handle NULL topologies Message-ID: <20240429135542.73d039b2@device-28.home> In-Reply-To: References: <20240412104615.3779632-1-maxime.chevallier@bootlin.com> <20240412152335.751a8dbb@device-28.home> Organization: Bootlin X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-GND-Sasl: maxime.chevallier@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240429_045554_528907_FE14997A X-CRM114-Status: GOOD ( 33.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hello Heiner, On Sat, 27 Apr 2024 21:34:21 +0200 Heiner Kallweit wrote: > On 12.04.2024 15:23, Maxime Chevallier wrote: > > Hello Heiner, > > > > On Fri, 12 Apr 2024 15:07:46 +0200 > > Heiner Kallweit wrote: > > > >> On 12.04.2024 12:46, Maxime Chevallier wrote: > >>> In situations where phylib is a module, the topology can be NULL as it's > >>> not initialized at netdev creation. > >>> > >> > >> What we see here is a bigger drawback of IS_REACHABLE(). For phylib it's > >> false from net core, but true from r8169 driver. So topo_create is a stub, > >> but topo_add is not. IS_REACHABLE() hides dependencies. > >> > >> topo_create et al don't really use something from phylib. > >> Therefore, could/should it be moved to net core? > > > > That's a valid point, and a better solution indeed. > > > >> At least for topo_create this would resolve the dependency. > >> > >> We could also add a config symbol and the PHY topology an optional > >> extension of net core. > > > > That could be a thing indeed. It could be selected by phylib then, I > > don't see it being a user-controlled option, as this would make it very > > confusing for users to only be able to see when there are mutiple PHYs > > on the link when the relevant option is enabled (but I might be wrong). > > > AFAIK the issue still exists on net-next. Are you going to submit > an updated version? It still is indeed. I've been very busy last week unfortunately, I'll followup today though. Thanks for the patience, Maxime > > > Maxime > > > >> > >>> Allow passing a NULL topology pointer to phy_link_topo helpers. > >>> > >>> Signed-off-by: Maxime Chevallier > >>> Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/ > >>> Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/ > >>> --- > >>> > >>> Hi, > >>> > >>> This patch fixes a commit that is in net-next, hence the net-next tag and the > >>> lack of "Fixes" tag. > >>> > >>> Nathan, Heiner, can you confirm this solves what you're seeing ? > >>> > >>> I think we can improve on this solution by moving the topology init at > >>> the first PHY insertion and clearing it at netdev destruction. > >>> > >>> Maxime > >>> > >>> drivers/net/phy/phy_link_topology.c | 10 +++++++++- > >>> include/linux/phy_link_topology.h | 7 ++++++- > >>> 2 files changed, 15 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c > >>> index 985941c5c558..0f3973f07fac 100644 > >>> --- a/drivers/net/phy/phy_link_topology.c > >>> +++ b/drivers/net/phy/phy_link_topology.c > >>> @@ -42,6 +42,9 @@ int phy_link_topo_add_phy(struct phy_link_topology *topo, > >>> struct phy_device_node *pdn; > >>> int ret; > >>> > >>> + if (!topo) > >>> + return 0; > >>> + > >>> pdn = kzalloc(sizeof(*pdn), GFP_KERNEL); > >>> if (!pdn) > >>> return -ENOMEM; > >>> @@ -93,7 +96,12 @@ EXPORT_SYMBOL_GPL(phy_link_topo_add_phy); > >>> void phy_link_topo_del_phy(struct phy_link_topology *topo, > >>> struct phy_device *phy) > >>> { > >>> - struct phy_device_node *pdn = xa_erase(&topo->phys, phy->phyindex); > >>> + struct phy_device_node *pdn; > >>> + > >>> + if (!topo) > >>> + return; > >>> + > >>> + pdn = xa_erase(&topo->phys, phy->phyindex); > >>> > >>> /* We delete the PHY from the topology, however we don't re-set the > >>> * phy->phyindex field. If the PHY isn't gone, we can re-assign it the > >>> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h > >>> index 6b79feb607e7..21ca78127d0f 100644 > >>> --- a/include/linux/phy_link_topology.h > >>> +++ b/include/linux/phy_link_topology.h > >>> @@ -40,7 +40,12 @@ struct phy_link_topology { > >>> static inline struct phy_device * > >>> phy_link_topo_get_phy(struct phy_link_topology *topo, u32 phyindex) > >>> { > >>> - struct phy_device_node *pdn = xa_load(&topo->phys, phyindex); > >>> + struct phy_device_node *pdn; > >>> + > >>> + if (!topo) > >>> + return NULL; > >>> + > >>> + pdn = xa_load(&topo->phys, phyindex); > >>> > >>> if (pdn) > >>> return pdn->phy; > >> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel