Re: [PATCH] net: usb: lan78xx: add weak dependency with micrel phy module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 24, 2024 at 09:42:48PM GMT, Florian Fainelli wrote:


On 7/24/2024 9:25 PM, Lucas De Marchi wrote:
On Thu, Jul 25, 2024 at 12:57:05AM GMT, Andrew Lunn wrote:
For the commented case, I have included only one phy because it is the hardware that I have, but other phy devices (modules) are possible and they can be some.

So this the whole whacker a mole problem. It works for you but fails
for 99% of users. How is this helping us?

no, this is the first instance that was found/added.

if you declare a softdep what happens is that the dep is loaded first
(needed or not) and your module is loaded after that

if you declare a weakdep, you are just instructing the tools that the
module may or may not be needed.  Any module today that does a
request_module("foo") could be a candidate to migrate from
MODULE_SOFTDEP("pre: foo") to the new weakdep, as long as it handles
properly the module being loaded ondemand as opposed to using
request_module() to just synchronize the module being loaded.


Maybe a better solution is to first build an initramfs with
everything, plus the kitchen sink. Boot it, and then look at what has
been loaded in order to get the rootfs mounted. Then update the
initramfs with just what is needed? That should be pretty generic,
with throw out networking ig NFS root is not used, just load JFFS2 and
a NAND driver if it was used for the rootfs, etc.

that works for development systems or if you are fine tuning it for each
system you have. It doesn't work for a generic distro with the kitchen
sink of modules and still trying to minimize the initrd without end user
intervention. So it works for 99% of users.

OK, but 'config USB_LAN78XX' does have a number of 'select' meaning those are hard functional dependencies, and so those should be more than a hint that these modules are necessary. Why should we encode that information twice: once in Kconfig and another time within the

selecting a config currently has nothing to do with how module
dependency is calculated. You can select whatever in kconfig for
whatever reason. And a selecting a kconfig is often used to select
things other than modules.

"hard" dependencies are calculated by depmod based purely on the symbols
the module uses. So if module A calls (a exported) symbol from module
B, there is a "hard" dependency. modules.dep will have a line like

kernel/drivers/gpu/drm/xe/xe.ko.zst: kernel/drivers/gpu/drm/drm_gpuvm.ko.zst kernel/drivers/gpu/drm/drm_exec.ko.zst kernel/drivers/gpu/drm/scheduler/gpu-sched.ko.zst kernel/drivers/gpu/drm/drm_buddy.ko.zst kernel/drivers/gpu/drm/drm_suballoc_helper.ko.zst kernel/drivers/gpu/drm/drm_ttm_helper.ko.zst kernel/drivers/gpu/drm/ttm/ttm.ko.zst kernel/drivers/gpu/drm/display/drm_display_helper.ko.zst kernel/drivers/media/cec/core/cec.ko.zst kernel/drivers/media/rc/rc-core.ko.zst kernel/drivers/i2c/algos/i2c-algo-bit.ko.zst kernel/drivers/acpi/video.ko.zst kernel/drivers/platform/x86/wmi.ko.zst

that means the xe module uses symbols from (some of) these modules and
these modules uses symbols from the other ones in this line. This is the
expanded dependency chain.

On the other hand, there are certain situations in which you don't use
a symbol directly from the other module, but having it around provides
additional functionality by other means (apparently the situation here
in this patch). It's common to record that dependency with
MODULE_SOFTDEP("pre: ...")

$ grep lan78xx /lib/modules/$(uname -r)/modules.dep
kernel/drivers/net/usb/lan78xx.ko.zst:

So you don't use any symbol from other modules (the above doesn't list
things that are builtin in my kernel config)...

$ grep -e 'CONFIG_USB_LAN78XX[= ]' \
	-e 'CONFIG_MII[= ]' \
	-e 'CONFIG_PHYLIB[= ]' \
	-e 'CONFIG_MICROCHIP_PHY[= ]' \
	-e 'CONFIG_FIXED_PHY[= ]' \
	-e 'CONFIG_CRC32[= ]' \
	/boot/config-$(uname -r)
CONFIG_MII=m
CONFIG_PHYLIB=y
CONFIG_FIXED_PHY=y
CONFIG_MICROCHIP_PHY=m
CONFIG_USB_LAN78XX=m
CONFIG_CRC32=y

module .c file itself? Cannot we have better tooling to help build an initramfs which does include everything that has been selected?

if you are saying that the build system should automatically convert
this:

	config USB_LAN78XX
		tristate "Microchip LAN78XX Based USB Ethernet Adapters"
		select MII
		select PHYLIB
		select MICROCHIP_PHY
		select FIXED_PHY
		select CRC32

into (for my config):

	MODULE_WEAKDEP("mii");
	MODULE_WEAKDEP("microchip");

then humn... why is CONFIG_MICREL (being added in this patch) not there?
It seems even if we automatically derive that information it wouldn't
fix the problem Jose is trying to solve.

Note that the MODULE_WEAKDEP() should be added only if that's selecting
a module and if if there isn't already a hard dependency. Then I think
there would be quite some work to do. It was not how MODULE_SOFTDEP()
was handled in the past. Cc'ing Masahiro and linux-kbuild to know if
they have an idea how feasible that would be to add in modpost.

Lucas De Marchi

--
Florian




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux