Message-ID: <Ya9DGWhJBJA1zRZ5@shell.armlinux.org.uk>
Date:   Tue, 7 Dec 2021 11:18:49 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     zhuyinbo <zhuyinbo@...ngson.cn>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH v2 1/2] modpost: file2alias: fixup mdio alias garbled
 code in modules.alias

On Tue, Dec 07, 2021 at 05:41:51PM +0800, zhuyinbo wrote:
> 在 2021/12/1 上午8:38, Andrew Lunn 写道:
> > > However, this won't work for PHY devices created _before_ the kernel
> > > has mounted the rootfs, whether or not they end up being used. So,
> > > every PHY mentioned in DT will be created before the rootfs is mounted,
> > > and none of these PHYs will have their modules loaded.
> > 
> > Hi Russell
> > 
> > I think what you are saying here is, if the MAC or MDIO bus driver is
> > built in, the PHY driver also needs to be built in?
> > 
> > If the MAC or MDIO bus driver is a module, it means the rootfs has
> > already been mounted in order to get these modules. And so the PHY
> > driver as a module will also work.
> > 
> > > I believe this is the root cause of Yinbo Zhu's issue.
> 
> I think you should be right and I had did lots of test but use rquest_module
> it doesn't load marvell module, and dts does't include any phy node. even
> though I was use "marvell" as input's args of request_module.

Please can you report the contents of /proc/sys/kernel/modprobe, and
the kernel configuration of CONFIG_MODPROBE_PATH. I wonder if your
userspace has that module loading mechanism disabled, or your kernel
has CONFIG_MODPROBE_PATH as an empty string.

If the module is not present by the time this call is made, then
even if you load the appropriate driver module later, that module
will not be used - the PHY will end up being driven by the generic
clause 22 driver.

> > That is not true universally for all MDIO though - as
> > xilinx_gmii2rgmii.c clearly shows. That is a MDIO driver which uses DT
> > the compatible string to do the module load. So, we have proof there
> > that Yinbo Zhu's change will definitely cause a regression which we
> > can not allow.
> 
> I don't understand that what you said about regression.  My patch doesn't
> cause  xilinx_gmii2rgmii.c driver load fail, in this time that do_of_table
> and platform_uevent will be responsible "of" type driver auto load and my
> patch was responsible for "mdio" type driver auto load,

xilinx_gmii2rgmii is not a platform driver. It is a mdio driver:

static struct mdio_driver xgmiitorgmii_driver = {
              ^^^^^^^^^^^

Therefore, platform_uevent() is irrelevant since this will never match
a platform device. It will only match mdio devices, and the uevent
generation for that is via mdio_uevent() which is the function you
are changing.

> In default code. There are request_module to load phy driver, but as Russell
> King said that request_module doesn't garantee auto load will always work
> well, but udev mechanism can garantee it. and udev mechaism is more
> mainstream, otherwise mdio_uevent is useless. if use udev mechanism that my
> patch was needed. and if apply my patch it doesn't cause request_module
> mechaism work bad because I will add following change:

Please report back what the following command produces on your
problem system:

/sbin/modprobe -vn mdio:00000001010000010000110111010001

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists