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 62E03C19F53 for ; Tue, 30 Apr 2024 11:58:08 +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=7t5a+g9QbO+SeyEdkzqdoXGuHQXDsaZCMhRbcE5zYpw=; b=3Pjwq8Hm2oihDP +NB8YM9rudrUCZ+zNXaMdYZO4ufLtPtrzempKPaS5ap1HLzKqxdys3Kdnqaes6GTlimQmyCxUM+3Q gCXMzHBZtpLIhEvTCo9y2+Yv+bUoDD44MjZOyb/ourIwd0ajv4OZlRM5g4sjQQ2p9DniDaPOMsvbr 9Gi/1DeL0G1vh3zuIjFfqw4/yP4O9bcvNQp6hMgURETRuqpZDq1mHvWdL3lGx5zTFKK4YXZ8iQbSp AC2AdXZqgF7J4Kp1mRqkq2bddoH4RtFizobWhCRWMTPaOC8uyw/09s/NPVPfRcCbAOAElwZwC2CmD 1gNLh+sm+D/rouCRnYog==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1m7P-00000006FWU-00NF; Tue, 30 Apr 2024 11:57:55 +0000 Received: from relay5-d.mail.gandi.net ([217.70.183.197]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1m7L-00000006FUg-17sW for linux-arm-kernel@lists.infradead.org; Tue, 30 Apr 2024 11:57:53 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 6D07E1C0009; Tue, 30 Apr 2024 11:57:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1714478262; 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=m2HIuXOxxXCgZkw0XsjUo+QE130qanWOjxQaANrRc0w=; b=T+IwUXyAzz36ab/71Ll7jL6I21BUNWFq10zkqhke9HmIV4LyY12sJXlI6U7gXnZuHg7M10 cnw5spz6UNDzR9T0Sm3esk2/IQXS1Pirem95mt1FB23J9/dU6A/jF0oukoKuR+qxAjwJWW fZ7nnvh+fzDNUKY5aLufDpHAHiZXnQASRWrIbTnXWNABCWqkjdxf7Myr3Bawv11L5DS63b bd86R9P83BaEPKenSP15ToLNFaSq2nqdKx3G++29N5Qzi8hckc0Fz28o/hJbCs3rz/NgZW NhhGDu8gnZfS+kbtyJznUuzfxtJ5etx9xEsqmTjbtNcHpVs+rtbI3CdSFrUdWQ== Date: Tue, 30 Apr 2024 13:57:34 +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: Don't conditionally compile the phy_link_topology creation Message-ID: <20240430135734.503f51a2@device-28.home> In-Reply-To: <1ed5b8cb-c79b-44b9-8dbe-f78d7505b3b4@gmail.com> References: <20240429131008.439231-1-maxime.chevallier@bootlin.com> <1ed5b8cb-c79b-44b9-8dbe-f78d7505b3b4@gmail.com> 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-20240430_045751_762452_C20D4027 X-CRM114-Status: GOOD ( 26.54 ) 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 Tue, 30 Apr 2024 10:17:31 +0200 Heiner Kallweit wrote: > On 29.04.2024 15:10, Maxime Chevallier wrote: > > The core of the phy_link_topology isn't directly tied to phylib, and at > > the moment it's initialized, phylib might not be loaded yet. Move the > > initialization of the topology to the phy_link_topology_core header, > > which contains the bare minimum so that we can initialize it at netdev > > creation. > > > > The change fixes the issue for me, but according to my personal taste > the code isn't intuitive and still error-prone. Also there's no good > reason to inline a function like phy_link_topo_create() and make it > publicly available. Do you expect it to be ever used outside net core? > In general it may make sense to add a config symbol for the topology > extension, there seem to be very few, specialized use cases for it. I think I'm missing the point here then. Do you mean adding a Kconfig option to explicitely turn phy_link_topology on ? or build it as a dedicated kernel module ? Or do you see something such as "if phylib is M or Y, then build the topology stuff and make sure it's allocated when a netdev gets created ?" Thanks, Maxime > > > 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/ > > --- > > drivers/net/phy/phy_link_topology.c | 23 -------------------- > > include/linux/phy_link_topology.h | 5 ----- > > include/linux/phy_link_topology_core.h | 30 +++++++++++++++++--------- > > 3 files changed, 20 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c > > index 985941c5c558..960aedd73308 100644 > > --- a/drivers/net/phy/phy_link_topology.c > > +++ b/drivers/net/phy/phy_link_topology.c > > @@ -12,29 +12,6 @@ > > #include > > #include > > > > -struct phy_link_topology *phy_link_topo_create(struct net_device *dev) > > -{ > > - struct phy_link_topology *topo; > > - > > - topo = kzalloc(sizeof(*topo), GFP_KERNEL); > > - if (!topo) > > - return ERR_PTR(-ENOMEM); > > - > > - xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1); > > - topo->next_phy_index = 1; > > - > > - return topo; > > -} > > - > > -void phy_link_topo_destroy(struct phy_link_topology *topo) > > -{ > > - if (!topo) > > - return; > > - > > - xa_destroy(&topo->phys); > > - kfree(topo); > > -} > > - > > int phy_link_topo_add_phy(struct phy_link_topology *topo, > > struct phy_device *phy, > > enum phy_upstream upt, void *upstream) > > diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h > > index 6b79feb607e7..ad72d7881257 100644 > > --- a/include/linux/phy_link_topology.h > > +++ b/include/linux/phy_link_topology.h > > @@ -32,11 +32,6 @@ struct phy_device_node { > > struct phy_device *phy; > > }; > > > > -struct phy_link_topology { > > - struct xarray phys; > > - u32 next_phy_index; > > -}; > > - > > static inline struct phy_device * > > phy_link_topo_get_phy(struct phy_link_topology *topo, u32 phyindex) > > { > > diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h > > index 0a6479055745..0116ec49cd1b 100644 > > --- a/include/linux/phy_link_topology_core.h > > +++ b/include/linux/phy_link_topology_core.h > > @@ -2,24 +2,34 @@ > > #ifndef __PHY_LINK_TOPOLOGY_CORE_H > > #define __PHY_LINK_TOPOLOGY_CORE_H > > > > -struct phy_link_topology; > > +#include > > > > -#if IS_REACHABLE(CONFIG_PHYLIB) > > - > > -struct phy_link_topology *phy_link_topo_create(struct net_device *dev); > > -void phy_link_topo_destroy(struct phy_link_topology *topo); > > - > > -#else > > +struct phy_link_topology { > > + struct xarray phys; > > + u32 next_phy_index; > > +}; > > > > static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev) > > { > > - return NULL; > > + struct phy_link_topology *topo; > > + > > + topo = kzalloc(sizeof(*topo), GFP_KERNEL); > > + if (!topo) > > + return ERR_PTR(-ENOMEM); > > + > > + xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1); > > + topo->next_phy_index = 1; > > + > > + return topo; > > } > > > > static inline void phy_link_topo_destroy(struct phy_link_topology *topo) > > { > > -} > > + if (!topo) > > + return; > > > > -#endif > > + xa_destroy(&topo->phys); > > + kfree(topo); > > +} > > > > #endif /* __PHY_LINK_TOPOLOGY_CORE_H */ > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel