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

[Christian Marangi <ansuelsmth@gmail.com>]

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