[PATCH v3 4/4] net: phy: aquantia: use generic firmware loader
Beiyan Yun
root at infi.wang
Wed Oct 29 10:54:46 CET 2025
Hi Weijie,
Sorry for the late reply.
> On 15 Oct 2025, at 8:43 AM, Weijie Gao <weijie.gao at mediatek.com> wrote:
>
> Hi Beiyan,
>
> On Tue, 2025-10-14 at 20:12 +0800, Beiyan Yun wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> Aquantia PHYs are being used w/o SPI flash in some routers recently.
>> Current firmware loader only attempts to load from FS on top of MMC,
>> limiting the use on many devices.
>>
>> Removed the old firmware loader, migrate to generic firmware loader
>> to
>> allow a wider range and runtime override of firmware source. (e.g.,
>> USB).
>>
>> Tested on Buffalo WXR18000BE10P with UBIFS.
>>
>> Tested-by: Beiyan Yun <root at infi.wang>
>> Signed-off-by: Beiyan Yun <root at infi.wang>
>>
>>
>> ---
>>
>> Changes in v3:
>> - Select FW_LOADER with PHY_AQUANTIA_UPLOAD_FW
>>
>> Changes in v2:
>> - Add support for script based loader
>>
>> drivers/net/phy/Kconfig | 24 ++++---
>> drivers/net/phy/aquantia.c | 133 ++++++++++++++++++++---------------
>> --
>> 2 files changed, 88 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index 185c6a3156e..68765290671 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -1,4 +1,3 @@
>> -
>> config BITBANGMII
>> bool "Bit-banged ethernet MII management channel support"
>>
>> @@ -90,23 +89,30 @@ menuconfig PHY_AQUANTIA
>> config PHY_AQUANTIA_UPLOAD_FW
>> bool "Aquantia firmware loading support"
>> depends on PHY_AQUANTIA
>> + select FS_LOADER
>> + select FW_LOADER
>> help
>> Aquantia PHYs use firmware which can be either loaded
>> automatically
>> from storage directly attached to the phy or loaded
>> by the boot loader
>> - via MDIO commands. The firmware is loaded from a
>> file, specified by
>> - the PHY_AQUANTIA_FW_PART and PHY_AQUANTIA_FW_NAME
>> options.
>> + via MDIO commands.
>> +
>> + This option enables loading the firmware using the
>> generic
>> + file system firmware loader framework.
>>
>> -config PHY_AQUANTIA_FW_PART
>> - string "Aquantia firmware partition"
>> +config PHY_AQUANTIA_FW_MAX_SIZE
>> + hex "Max firmware size"
>> depends on PHY_AQUANTIA_UPLOAD_FW
>> + default 0x80000
>> help
>> - Partition containing the firmware file.
>> + The maximum size of the Aquantia PHY firmware. This is used
>> to
>> + allocate a buffer to load the firmware into.
>>
>> -config PHY_AQUANTIA_FW_NAME
>> - string "Aquantia firmware filename"
>> +config PHY_AQUANTIA_FW_LOADER_SCRIPT
>> + string "Aquantia firmware loader script"
>> depends on PHY_AQUANTIA_UPLOAD_FW
>> + default "aqr_phy_load_firmware"
>> help
>> - Firmware filename.
>> + The firmware loading script variable name.
>>
>> config PHY_ATHEROS
>> bool "Atheros Ethernet PHYs support"
>> diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
>> index d9df0e23a45..98ee7ae10e7 100644
>> --- a/drivers/net/phy/aquantia.c
>> +++ b/drivers/net/phy/aquantia.c
>> @@ -17,6 +17,10 @@
>> #include <malloc.h>
>> #include <asm/byteorder.h>
>> #include <fs.h>
>> +#if (IS_ENABLED(CONFIG_PHY_AQUANTIA_UPLOAD_FW))
>> +#include <fs_loader.h>
>> +#include <fw_loader.h>
>> +#endif
>>
>> #define AQUNTIA_10G_CTL 0x20
>> #define AQUNTIA_VENDOR_P1 0xc400
>> @@ -127,53 +131,7 @@ struct fw_header {
>>
>> #pragma pack()
>>
>> -#if defined(CONFIG_PHY_AQUANTIA_UPLOAD_FW)
>> -static int aquantia_read_fw(u8 **fw_addr, size_t *fw_length)
>
> I think it's better to add a weak function, e.g.:
> int __weak aquantia_read_fw(u8 **fw_addr, size_t *fw_length);
>
> And put the default fw loading logic into this function.
> Overriding this function by board allows using firmware data embedded
> into u-boot by binman.
>
> It's useful in case that firmware data was damaged in filesystem.
>
Good point, I can see that’s how MTK U-Boot manages the firmware currently.
There’s a catch though: I’ve prototyped the change locally, realize it breaks current assumption of lifetime. Let's say we override `aquantia_upload_firmware` with a wrapper function to bin data array, in that case we can not free `fw_addr` in `aquantia_upload_firmware`, or a dumb memory copy is needed.
Maybe we can add a structure wrapper with additional info like Qcom's `struct rpmh_request`? That’s a bit too much for me, I’d like further opinion.
Cheers,
Yun
>> -{
>> - loff_t length, read;
>> - int ret;
>> - void *addr = NULL;
>> -
>> - *fw_addr = NULL;
>> - *fw_length = 0;
>> - debug("Loading Aquantia microcode from %s %s\n",
>> - CONFIG_PHY_AQUANTIA_FW_PART,
>> CONFIG_PHY_AQUANTIA_FW_NAME);
>> - ret = fs_set_blk_dev("mmc", CONFIG_PHY_AQUANTIA_FW_PART,
>> FS_TYPE_ANY);
>> - if (ret < 0)
>> - goto cleanup;
>> -
>> - ret = fs_size(CONFIG_PHY_AQUANTIA_FW_NAME, &length);
>> - if (ret < 0)
>> - goto cleanup;
>> -
>> - addr = malloc(length);
>> - if (!addr) {
>> - ret = -ENOMEM;
>> - goto cleanup;
>> - }
>> -
>> - ret = fs_set_blk_dev("mmc", CONFIG_PHY_AQUANTIA_FW_PART,
>> FS_TYPE_ANY);
>> - if (ret < 0)
>> - goto cleanup;
>> -
>> - ret = fs_read(CONFIG_PHY_AQUANTIA_FW_NAME, (ulong)addr, 0,
>> length,
>> - &read);
>> - if (ret < 0)
>> - goto cleanup;
>> -
>> - *fw_addr = addr;
>> - *fw_length = length;
>> - debug("Found Aquantia microcode.\n");
>> -
>> -cleanup:
>> - if (ret < 0) {
>> - printf("loading firmware file %s %s failed with error
>> %d\n",
>> - CONFIG_PHY_AQUANTIA_FW_PART,
>> - CONFIG_PHY_AQUANTIA_FW_NAME, ret);
>> - free(addr);
>> - }
>> - return ret;
>> -}
>> +#if (IS_ENABLED(CONFIG_PHY_AQUANTIA_UPLOAD_FW))
>>
>> /* load data into the phy's memory */
>> static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
>> @@ -218,27 +176,27 @@ static u32 unpack_u24(const u8 *data)
>> return (data[2] << 16) + (data[1] << 8) + data[0];
>> }
>>
>> -static int aquantia_upload_firmware(struct phy_device *phydev)
>> +/* Common firmware upload implementation */
>> +static int aquantia_do_upload_firmware(struct phy_device *phydev,
>> + const u8 *addr, size_t
>> fw_length)
>> {
>> int ret;
>> - u8 *addr = NULL;
>> - size_t fw_length = 0;
>> u16 calculated_crc, read_crc;
>> char version[VERSION_STRING_SIZE];
>> u32 primary_offset, iram_offset, iram_size, dram_offset,
>> dram_size;
>> const struct fw_header *header;
>>
>> - ret = aquantia_read_fw(&addr, &fw_length);
>> - if (ret != 0)
>> - return ret;
>> + if (!addr || !fw_length) {
>> + printf("%s: Invalid firmware data\n", phydev->dev-
>>> name);
>> + return -EINVAL;
>> + }
>>
>> - read_crc = (addr[fw_length - 2] << 8) | addr[fw_length - 1];
>> + read_crc = (addr[fw_length - 2] << 8) | addr[fw_length - 1];
>> calculated_crc = crc16_ccitt(0, addr, fw_length - 2);
>> if (read_crc != calculated_crc) {
>> printf("%s bad firmware crc: file 0x%04x calculated
>> 0x%04x\n",
>> phydev->dev->name, read_crc, calculated_crc);
>> - ret = -EINVAL;
>> - goto done;
>> + return -EINVAL;
>> }
>>
>> /* Find the DRAM and IRAM sections within the firmware file.
>> */
>> @@ -268,14 +226,14 @@ static int aquantia_upload_firmware(struct
>> phy_device *phydev)
>> ret = aquantia_load_memory(phydev, DRAM_BASE_ADDR,
>> &addr[dram_offset],
>> dram_size);
>> if (ret != 0)
>> - goto done;
>> + return ret;
>>
>> debug("loading iram 0x%08x from offset=%d size=%d\n",
>> IRAM_BASE_ADDR, iram_offset, iram_size);
>> ret = aquantia_load_memory(phydev, IRAM_BASE_ADDR,
>> &addr[iram_offset],
>> iram_size);
>> if (ret != 0)
>> - goto done;
>> + return ret;
>>
>> /* make sure soft reset and low power mode are clear */
>> phy_write(phydev, MDIO_MMD_VEND1, GLOBAL_STANDARD_CONTROL,
>> 0);
>> @@ -289,8 +247,63 @@ static int aquantia_upload_firmware(struct
>> phy_device *phydev)
>> phy_write(phydev, MDIO_MMD_VEND1, UP_CONTROL,
>> UP_RUN_STALL_OVERRIDE);
>>
>> printf("%s firmware loading done.\n", phydev->dev->name);
>> -done:
>> - free(addr);
>> + return 0;
>> +}
>> +
>> +static int aquantia_upload_firmware(struct phy_device *phydev)
>> +{
>> + int ret;
>> + ofnode node;
>> + struct udevice *loader_dev;
>> + const char *fw_name;
>> + u8 *fw_addr = NULL;
>> + size_t fw_length;
>> +
>> + fw_addr = malloc(CONFIG_PHY_AQUANTIA_FW_MAX_SIZE);
>> + if (!fw_addr) {
>> + printf("Failed to allocate memory for firmware\n");
>> + return -ENOMEM;
>> + }
>> +
>> + /* First, try to load firmware via script */
>> + ret = request_firmware_into_buf_via_script(fw_addr,
>> + CONFIG_PHY_AQUANTI
>> A_FW_MAX_SIZE,
>> + CONFIG_PHY_AQUANTI
>> A_FW_LOADER_SCRIPT,
>> + &fw_length);
>> + if (ret) {
>> + /* Fallback to DT specified firmware */
>> + node = phy_get_ofnode(phydev);
>> + if (!ofnode_valid(node)) {
>> + printf("Failed to get PHY node\n");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + fw_name = ofnode_read_string(node, "firmware-name");
>> + if (!fw_name) {
>> + printf("Failed to get firmware name\n");
>> + ret = -ENOENT;
>> + goto out;
>> + }
>> +
>> + ret = get_fs_loader(&loader_dev);
>> + if (ret) {
>> + printf("Failed to get fs_loader instance:
>> %d\n", ret);
>> + goto out;
>> + }
>> +
>> + ret = request_firmware_into_buf(loader_dev, fw_name,
>> fw_addr,
>> + CONFIG_PHY_AQUANTIA_F
>> W_MAX_SIZE, 0);
>> + if (ret < 0) {
>> + printf("Failed to load firmware %s: %d\n",
>> fw_name, ret);
>> + goto out;
>> + }
>> + fw_length = ret;
>> + }
>> +
>> + ret = aquantia_do_upload_firmware(phydev, fw_addr,
>> fw_length);
>> +out:
>> + free(fw_addr);
>> return ret;
>> }
>> #else
>> --
>> 2.47.3
More information about the U-Boot
mailing list