linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Christian Marangi <ansuelsmth@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Qingfang Deng <dqfext@gmail.com>,
	SkyLake Huang <SkyLake.Huang@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Simon Horman <horms@kernel.org>, Netdev <netdev@vger.kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver
Date: Thu, 10 Apr 2025 12:40:15 +0200	[thread overview]
Message-ID: <67f7a015.df0a0220.287b40.53b2@mx.google.com> (raw)
In-Reply-To: <c108aee9-f668-4cd7-b276-d5e0a266eaa4@app.fastmail.com>

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...

-- 
	Ansuel


  reply	other threads:[~2025-04-10 11:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-10 10:04 [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Christian Marangi
2025-04-10 10:04 ` [net-next PATCH v2 2/2] net: phy: mediatek: add Airoha PHY ID to SoC driver Christian Marangi
2025-04-10 10:33   ` Arnd Bergmann
2025-04-10 19:07   ` Simon Horman
2025-04-14 10:30     ` Christian Marangi
2025-04-10 10:31 ` [net-next PATCH v2 1/2] net: phy: mediatek: permit to compile test GE SOC PHY driver Arnd Bergmann
2025-04-10 10:40   ` Christian Marangi [this message]
2025-04-10 10:44     ` Christian Marangi
2025-04-10 12:01       ` Arnd Bergmann
2025-04-10 12:46     ` Arnd Bergmann
2025-04-12  3:30 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67f7a015.df0a0220.287b40.53b2@mx.google.com \
    --to=ansuelsmth@gmail.com \
    --cc=SkyLake.Huang@mediatek.com \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=arnd@arndb.de \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).