lkml.org 
[lkml]   [2021]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v2 1/2] modpost: file2alias: fixup mdio alias garbled code in modules.alias
From
Date


在 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.
>
> You are speculating that in Yinbo Zhu case, the MAC driver is built
> in, the PHY is a module. The initial request for the firmware fails.
> Yinbo Zhu would like udev to try again later when the modules are
> available.
>
>> What we _could_ do is review all device trees and PHY drivers to see
>> whether DT modaliases are ever used for module loading. If they aren't,
>> then we _could_ make the modalias published by the kernel conditional
>> on the type of mdio device - continue with the DT approach for non-PHY
>> devices, and switch to the mdio: scheme for PHY devices. I repeat, this
>> can only happen if no PHY drivers match using the DT scheme, otherwise
>> making this change _will_ cause a regression.
>

> Take a look at
> drivers/net/mdio/of_mdio.c:whitelist_phys[] and the comment above it.
>
> So there are some DT blobs out there with compatible strings for
> PHYs. I've no idea if they actually load that way, or the standard PHY
> mechanism is used.
>
> Andrew
>


> 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,
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:



- ret = request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT,
- MDIO_ID_ARGS(phy_id));
+ ret = request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT, phy_id);
/* We only check for failures in executing the usermode binary,
* not whether a PHY driver module exists for the PHY ID.
* Accept -ENOENT because this may occur in case no initramfs
exists,
diff --git a/include/linux/mod_devicetable.h
b/include/linux/mod_devicetable.h
index 7bd23bf..bc6ea0d 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -600,16 +600,7 @@ struct platform_device_id {
#define MDIO_NAME_SIZE 32
#define MDIO_MODULE_PREFIX "mdio:"

-#define MDIO_ID_FMT
"%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u%u"
-#define MDIO_ID_ARGS(_id) \
- ((_id)>>31) & 1, ((_id)>>30) & 1, ((_id)>>29) & 1, ((_id)>>28) &
1, \
- ((_id)>>27) & 1, ((_id)>>26) & 1, ((_id)>>25) & 1, ((_id)>>24) &
1, \
- ((_id)>>23) & 1, ((_id)>>22) & 1, ((_id)>>21) & 1, ((_id)>>20) &
1, \
- ((_id)>>19) & 1, ((_id)>>18) & 1, ((_id)>>17) & 1, ((_id)>>16) &
1, \
- ((_id)>>15) & 1, ((_id)>>14) & 1, ((_id)>>13) & 1, ((_id)>>12) &
1, \
- ((_id)>>11) & 1, ((_id)>>10) & 1, ((_id)>>9) & 1, ((_id)>>8) & 1, \
- ((_id)>>7) & 1, ((_id)>>6) & 1, ((_id)>>5) & 1, ((_id)>>4) & 1, \
- ((_id)>>3) & 1, ((_id)>>2) & 1, ((_id)>>1) & 1, (_id) & 1
+#define MDIO_ID_FMT "p%08x"


\
 
 \ /
  Last update: 2021-12-07 10:42    [W:0.107 / U:0.552 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site