On 4/20/25 3:43 AM, Daniel Golle wrote:
Hi Lucien,
Hi Marek,
Hi,
On Fri, Apr 18, 2025 at 11:57:19PM +0800, Lucien.Jheng wrote:
Hi Marek Vasut
Thanks for the review.
The next commit will change the loading of the PHY firmware to the
filesystem on a block device.
Please keep in mind that most systems using this PHY are (like the
OpenWrt One, for example) simple embedded devices with only a small
amount of NAND or NOR flash. Loading the firmware from a filesystem
means overhead and complexity, while loading it from a simple
MTD partition, raw UBI volume or one of the EMMC boot hw partitions
is a better match to the reality of the devices it is being used on,
making the process less complex and more deterministic, and hence
more robust.
That works until someone else needs to load the firmware from somewhere
else. U-Boot is not a one-board-special-hack but a generic bootloader,
which is why special board-specific Kconfig options and other such
ad-hoc board-specific hacks in drivers are frowned upon. Having a PHY
driver depend on loading firmware in a non-generic manner is not helpful.
Whatever board specific firmware loading should not be in the driver, it
should be either in board code, or in some generic firmware loader (if
that is something someone wants to write, but that is more involved).
I am not buying the "reality of devices" argument, sorry, this is a
generic ethernet PHY, users can use it on whatever device they populate
it on, and they can very well chose to place the firmware into
/lib/firmware to share it with Linux expectations or in some EEPROM .
[...]
Marek Vasut 於 2025/4/6 下午 09:30 寫道:
[...]
Can this do the same thing as Aquatia PHY and load the PHY firmware from
filesystem on block device instead , using the blk* or fs* commands ?
Then the hard-coded filesize and co. won't be necessary.
Note that the size is afaik given by the hardware in this case, it won't
every change. Hence any handling of a more dynamic approach of loading
the firmware will also have to come with a size-check against the (again)
hard-coded size anyway.
If the firmware size is fixed, it shouldn't be exposed via Kconfig in
the first place. Linux doesn't do that either, why is it done here ?
If you need board specific loading override, implement __weak default
loading from file on filesystem by default (see what Aquatia does) and
then override the loading function on board or architecture level.
That can work, but in practice it will lead to code duplication, as all
platforms using those PHYs are kinda similar (ie. MT7981, MT7986 and
probably Airoha/EcoNet router SoCs as well)
Surely there is a way to factor common loader code parts into dedicated
loader-implementation.c to avoid duplication.
