linux-kernel - Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67f7a119.050a0220.b15d0.3df3@mx.google.com>
Date: Thu, 10 Apr 2025 12:44:36 +0200
From: Christian Marangi <ansuelsmth@...il.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Daniel Golle <daniel@...rotopia.org>,
	Qingfang Deng <dqfext@...il.com>,
	SkyLake Huang <SkyLake.Huang@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Simon Horman <horms@...nel.org>, Netdev <netdev@...r.kernel.org>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile
 test GE SOC PHY driver

On Thu, Apr 10, 2025 at 12:40:15PM +0200, Christian Marangi wrote:
> On Thu, Apr 10, 2025 at 12:31:05PM +0200, Arnd Bergmann wrote:
> > On Thu, Apr 10, 2025, at 12:04, Christian Marangi wrote:
> > > When commit 462a3daad679 ("net: phy: mediatek: fix compile-test
> > > dependencies") fixed the dependency, it should have also introduced
> > > an or on COMPILE_TEST to permit this driver to be compile-tested even if
> > > NVMEM_MTK_EFUSE wasn't selected
> > 
> > Why does this matter? NVMEM_MTK_EFUSE can be enabled for both
> > allmodconfig and randconfig builds on any architecture, so you
> > get build coverage either way, it's just a little less likely
> > to be enabled in randconfig I guess?
> >
> 
> If we base stuff on the fact that everything is selected or that a
> random config by luck selects it, then COMPILE_TEST doesn't make sense
> at all.
> 
> For my personal test, I wanted to test the driver on a simple x86 build
> without having to depend on ARCH or having to cross compile. Won't
> happen on real world scenario? Totally. I should be able to compile it?
> Yes.
> 
> > > diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
> > > index 2a8ac5aed0f8..6a4c2b328c41 100644
> > > --- a/drivers/net/phy/mediatek/Kconfig
> > > +++ b/drivers/net/phy/mediatek/Kconfig
> > > @@ -15,8 +15,7 @@ config MEDIATEK_GE_PHY
> > > 
> > >  config MEDIATEK_GE_SOC_PHY
> > >  	tristate "MediaTek SoC Ethernet PHYs"
> > > -	depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
> > > -	depends on NVMEM_MTK_EFUSE
> > > +	depends on (ARM64 && ARCH_MEDIATEK && NVMEM_MTK_EFUSE) || COMPILE_TEST
> > >  	select MTK_NET_PHYLIB
> > >  	help
> > >  	  Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
> > > -- 
> > 
> > I would expect this to break the build with CONFIG_NVMEM=m
> > and MEDIATEK_GE_SOC_PHY=y.
> > 
> > The normal thing here would be to have a dependency on
> > CONFIG_NVMEM in place of the NVMEM_MTK_EFUSE dependency,
> > or possible on 'NVMEM || !NVMEM' if you want to make it
> > more likely to be enabled in randconfig builds.
> > 
> 
> The big idea of these dependency is that... In MTK the internal PHY of
> the switch needs calibration or it won't work hence it doesn't make
> sense to select the PHY as it won't ever work without the NVMEM driver.
> 
> But from a compile test view where we only evaluate if the driver have
> compilation error or other kind of warning, we should not care...
>

Also 99% I could be wrong but from what I can see in NVMEM kconfig,
NVMEM is not tristate but only bool? So NVMEM=m is not a thing?

-- 
	Ansuel