[PATCH RESEND 0/4] net: phy: aquantia: Switch to generic firmware loader

[PATCH RESEND 0/4] net: phy: aquantia: Switch to generic firmware loader

[Beiyan Yun]

Hi,

This patch series refactors the Aquantia PHY firmware loader to use
the generic fsloader framework.

The existing loader is limited to loading firmware from an MMC device,
which restricts its use on many devices (e.g., routers) that may use
other storage like USB or have a UBIFS root filesystem. Migrating to the
generic firmware loader allows firmware to be sourced from any backend
supported by the fsloader framework.

The series is structured as follows:
- The first patch is a minor cleanup, switching to the phy_get_ofnode()
  helper.
- The second patch fixes a small typo in the binding document.
- The third and main patch replaces the custom loader with generic one.
- The final patch updates the device tree binding documentation to
  introduce the firmware-name property and provide usage examples.

This change has been tested on a Buffalo WXR18000BE10P router, loading
firmware from a UBIFS volume for Marvell CUX3410.


Beiyan Yun (4):
  net: phy: aquantia: switch to use phy_get_ofnode()
  doc: bindings: fix aquantia-phy.txt typo
  net: phy: aquantia: use generic firmware loader
  doc: bindings: add Aquantia PHY node's "firmware-name" binding

 doc/device-tree-bindings/net/aquantia-phy.txt |  32 ++++-
 drivers/net/phy/Kconfig                       |  19 ++-
 drivers/net/phy/aquantia.c                    | 124 +++++++++---------
 3 files changed, 103 insertions(+), 72 deletions(-)

-- 
2.47.3

base-commit: d33b21b7e261691e8d6613a24cc9b0ececba3b01
branch: aquantia-fsloader

[PATCH RESEND 1/4] net: phy: aquantia: switch to use phy_get_ofnode()

[Beiyan Yun]

Use PHY API phy_get_ofnode() helper to get PHY DT node.

Signed-off-by: Beiyan Yun <root@infi.wang>
---

 drivers/net/phy/aquantia.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index f63a13824ca..903fcd667f6 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -338,7 +338,7 @@ static int aquantia_set_proto(struct phy_device *phydev,
 
 static int aquantia_dts_config(struct phy_device *phydev)
 {
-	ofnode node = phydev->node;
+	ofnode node = phy_get_ofnode(phydev);
 	u32 prop;
 	u16 reg;
 
-- 
2.47.3

[PATCH RESEND 2/4] doc: bindings: fix aquantia-phy.txt typo

[Beiyan Yun]

Fix typo: "weays" -> "ways"

Signed-off-by: Beiyan Yun <root@infi.wang>
---

 doc/device-tree-bindings/net/aquantia-phy.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
index 89ce61e05bb..7dd3d45df12 100644
--- a/doc/device-tree-bindings/net/aquantia-phy.txt
+++ b/doc/device-tree-bindings/net/aquantia-phy.txt
@@ -4,7 +4,7 @@ This text describes properties that are applicable to Aquantia PHY nodes in
 addition to the bindings in phy.txt.
 
 Aquantia PHYs allow some flexibility in the way they are wired in a system,
-they allow MDI pins to be reversed, LEDs linked up in different weays, have an
+they allow MDI pins to be reversed, LEDs linked up in different ways, have an
 I2C slave interface that can be used for debug.  Normally the configuration
 corresponding to these is driven by the PHY firmware with the downside that
 a custom firmware is needed for each integration of a PHY.
-- 
2.47.3

[PATCH RESEND 3/4] net: phy: aquantia: use generic firmware loader

[Beiyan Yun]

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 of firmware source (e.g., USB, UBIFS).

Tested on Buffalo WXR18000BE10P with UBIFS.

Tested-by: Beiyan Yun <root@infi.wang>
Signed-off-by: Beiyan Yun <root@infi.wang>
---

 drivers/net/phy/Kconfig    |  19 +++---
 drivers/net/phy/aquantia.c | 122 +++++++++++++++++++------------------
 2 files changed, 71 insertions(+), 70 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 21bf983056a..16a853793c5 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -90,23 +90,22 @@ menuconfig PHY_AQUANTIA
 config PHY_AQUANTIA_UPLOAD_FW
 	bool "Aquantia firmware loading support"
 	depends on PHY_AQUANTIA
+	select FS_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.
 
-config PHY_AQUANTIA_FW_PART
-	string "Aquantia firmware partition"
-	depends on PHY_AQUANTIA_UPLOAD_FW
-	help
-		Partition containing the firmware file.
+		This option enables loading the firmware using the generic
+		file system firmware loader framework.
 
-config PHY_AQUANTIA_FW_NAME
-	string "Aquantia firmware filename"
+config PHY_AQUANTIA_FW_MAX_SIZE
+	hex "Max firmware size"
 	depends on PHY_AQUANTIA_UPLOAD_FW
+	default 0x80000
 	help
-		Firmware filename.
+	  The maximum size of the Aquantia PHY firmware. This is used to
+	  allocate a buffer to load the firmware into.
 
 config PHY_ATHEROS
 	bool "Atheros Ethernet PHYs support"
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 903fcd667f6..dc5f30a9e53 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -17,6 +17,9 @@
 #include <malloc.h>
 #include <asm/byteorder.h>
 #include <fs.h>
+#if (IS_ENABLED(CONFIG_PHY_AQUANTIA_UPLOAD_FW))
+#include <fs_loader.h>
+#endif
 
 #define AQUNTIA_10G_CTL		0x20
 #define AQUNTIA_VENDOR_P1	0xc400
@@ -127,53 +130,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)
-{
-	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 +175,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 +225,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 +246,53 @@ 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;
+
+	node = phy_get_ofnode(phydev);
+	if (!ofnode_valid(node)) {
+		printf("Failed to get PHY node\n");
+		return -EINVAL;
+	}
+
+	fw_name = ofnode_read_string(node, "firmware-name");
+	if (!fw_name) {
+		printf("Failed to get firmware name\n");
+		return -ENOENT;
+	}
+
+	ret = get_fs_loader(&loader_dev);
+	if (ret) {
+		printf("Failed to get fs_loader instance: %d\n", ret);
+		return ret;
+	}
+
+	fw_addr = malloc(CONFIG_PHY_AQUANTIA_FW_MAX_SIZE);
+	if (!fw_addr) {
+		printf("Failed to allocate memory for firmware\n");
+		return -ENOMEM;
+	}
+
+	ret = request_firmware_into_buf(loader_dev, fw_name, fw_addr,
+					CONFIG_PHY_AQUANTIA_FW_MAX_SIZE, 0);
+	if (ret < 0) {
+		printf("Failed to load firmware %s: %d\n", fw_name, ret);
+		free(fw_addr);
+		return ret;
+	}
+	fw_length = ret;
+
+	ret = aquantia_do_upload_firmware(phydev, fw_addr, fw_length);
+	free(fw_addr);
 	return ret;
 }
 #else
-- 
2.47.3

[PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Beiyan Yun]

With the switch to generic firmware loader, "firmware-name" binding
was introduced to define the firmware filename.
Provide the document and usage examples.

Signed-off-by: Beiyan Yun <root@infi.wang>

---

 doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
index 7dd3d45df12..1227c04d04f 100644
--- a/doc/device-tree-bindings/net/aquantia-phy.txt
+++ b/doc/device-tree-bindings/net/aquantia-phy.txt
@@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
 Several optional bindings are defined that allow these configuration points to
 be driven by the PHY driver and reduce dependency on specific FW versions.
 
+Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
+or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
+The driver uploads only when the PHY reports missing firmware or a fault.
+
 Optional properties:
 mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
               Firmware default is used if the property is missing.
 smb-addr:     I2C/SMBus address to use, firmware default is used if the property
               is missing.
+firmware-name: String containing the filename of the PHY firmware to load
+               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).
 
 Example node:
 phy@00 {
 	reg = <0x00>;
 	mdi-reversal = <1>;
 	smb-addr = <0x25>;
+	firmware-name = "aqr-firmware.cld";
+};
+
+Example using the generic firmware loader:
+/	{
+	chosen {
+		/* Select default firmware loader instance */
+		firmware-loader = &fs_loader0;
+	};
+
+	fs_loader0: fs-loader@0 {
+		bootph-all;
+		compatible = "u-boot,fs-loader";
+		/* Load from MMC0, partition 1 */
+		phandlepart = <&mmc_0 1>;
+	};
+
+	mdio {
+		phy@0 {
+			reg = <0>;
+			/* Load this file via the selected fs-loader */
+			firmware-name = "aqr-firmware.cld";
+		};
+	};
 };
-- 
2.47.3

Re: [PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Yao Zi]

On Tue, Sep 23, 2025 at 03:13:01PM +0800, Beiyan Yun wrote:
> With the switch to generic firmware loader, "firmware-name" binding
> was introduced to define the firmware filename.
> Provide the document and usage examples.
> 
> Signed-off-by: Beiyan Yun <root@infi.wang>

IMO this patch should go before the driver change.

> ---
> 
>  doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
> index 7dd3d45df12..1227c04d04f 100644
> --- a/doc/device-tree-bindings/net/aquantia-phy.txt
> +++ b/doc/device-tree-bindings/net/aquantia-phy.txt
> @@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
>  Several optional bindings are defined that allow these configuration points to
>  be driven by the PHY driver and reduce dependency on specific FW versions.
>  
> +Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
> +or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
> +The driver uploads only when the PHY reports missing firmware or a fault.
> +
>  Optional properties:
>  mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
>                Firmware default is used if the property is missing.
>  smb-addr:     I2C/SMBus address to use, firmware default is used if the property
>                is missing.
> +firmware-name: String containing the filename of the PHY firmware to load
> +               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).

This looks good to me, but I have a question: should we switch to the
upstream binding for aquantia phys? It's already documented as
marvell,aquantia.yaml, and we could avoid the burden of maintaining a
separate binding file.

The "firmware-name" property is already described in the upstream
marvell,aquantia.yaml, and it only misses the smb-addr property. The
only U-Boot boards making use of this property are fsl-sch-30841 and
fsl-sch-30842, thus such conversion shouldn't be a big job.

Best regards,
Yao Zi

>  Example node:
>  phy@00 {
>  	reg = <0x00>;
>  	mdi-reversal = <1>;
>  	smb-addr = <0x25>;
> +	firmware-name = "aqr-firmware.cld";
> +};
> +
> +Example using the generic firmware loader:
> +/	{
> +	chosen {
> +		/* Select default firmware loader instance */
> +		firmware-loader = &fs_loader0;
> +	};
> +
> +	fs_loader0: fs-loader@0 {
> +		bootph-all;
> +		compatible = "u-boot,fs-loader";
> +		/* Load from MMC0, partition 1 */
> +		phandlepart = <&mmc_0 1>;
> +	};
> +
> +	mdio {
> +		phy@0 {
> +			reg = <0>;
> +			/* Load this file via the selected fs-loader */
> +			firmware-name = "aqr-firmware.cld";
> +		};
> +	};
>  };
> -- 
> 2.47.3
> 

Re: [PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Beiyan Yun]

> On 23 Sep 2025, at 8:44 PM, Yao Zi <ziyao@disroot.org> wrote:
> 
> On Tue, Sep 23, 2025 at 03:13:01PM +0800, Beiyan Yun wrote:
>> With the switch to generic firmware loader, "firmware-name" binding
>> was introduced to define the firmware filename.
>> Provide the document and usage examples.
>> 
>> Signed-off-by: Beiyan Yun <root@infi.wang <mailto:root@infi.wang>>
> 
> IMO this patch should go before the driver change.
> 
>> ---
>> 
>> doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
>> 1 file changed, 30 insertions(+)
>> 
>> diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
>> index 7dd3d45df12..1227c04d04f 100644
>> --- a/doc/device-tree-bindings/net/aquantia-phy.txt
>> +++ b/doc/device-tree-bindings/net/aquantia-phy.txt
>> @@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
>> Several optional bindings are defined that allow these configuration points to
>> be driven by the PHY driver and reduce dependency on specific FW versions.
>> 
>> +Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
>> +or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
>> +The driver uploads only when the PHY reports missing firmware or a fault.
>> +
>> Optional properties:
>> mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
>>               Firmware default is used if the property is missing.
>> smb-addr:     I2C/SMBus address to use, firmware default is used if the property
>>               is missing.
>> +firmware-name: String containing the filename of the PHY firmware to load
>> +               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).
> 
> This looks good to me, but I have a question: should we switch to the
> upstream binding for aquantia phys? It's already documented as
> marvell,aquantia.yaml, and we could avoid the burden of maintaining a
> separate binding file.
> 
> The "firmware-name" property is already described in the upstream
> marvell,aquantia.yaml, and it only misses the smb-addr property. The
> only U-Boot boards making use of this property are fsl-sch-30841 and
> fsl-sch-30842, thus such conversion shouldn't be a big job.

Good point. I’ll add a bit more info in related Kconfig options and drop the doc.

> 
> Best regards,
> Yao Zi
> 
>> Example node:
>> phy@00 {
>> 	reg = <0x00>;
>> 	mdi-reversal = <1>;
>> 	smb-addr = <0x25>;
>> +	firmware-name = "aqr-firmware.cld";
>> +};
>> +
>> +Example using the generic firmware loader:
>> +/	{
>> +	chosen {
>> +		/* Select default firmware loader instance */
>> +		firmware-loader = &fs_loader0;
>> +	};
>> +
>> +	fs_loader0: fs-loader@0 {
>> +		bootph-all;
>> +		compatible = "u-boot,fs-loader";
>> +		/* Load from MMC0, partition 1 */
>> +		phandlepart = <&mmc_0 1>;
>> +	};
>> +
>> +	mdio {
>> +		phy@0 {
>> +			reg = <0>;
>> +			/* Load this file via the selected fs-loader */
>> +			firmware-name = "aqr-firmware.cld";
>> +		};
>> +	};
>> };
>> -- 
>> 2.47.3

Cheers,
Beiyan Yun

Re: [PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Beiyan Yun]

> On 26 Sep 2025, at 4:22 PM, Beiyan Yun <root@infi.wang> wrote:
> 
> 
> 
>> On 23 Sep 2025, at 8:44 PM, Yao Zi <ziyao@disroot.org <mailto:ziyao@disroot.org>> wrote:
>> 
>> On Tue, Sep 23, 2025 at 03:13:01PM +0800, Beiyan Yun wrote:
>>> With the switch to generic firmware loader, "firmware-name" binding
>>> was introduced to define the firmware filename.
>>> Provide the document and usage examples.
>>> 
>>> Signed-off-by: Beiyan Yun <root@infi.wang <mailto:root@infi.wang> <mailto:root@infi.wang>>
>> 
>> IMO this patch should go before the driver change.
>> 
>>> ---
>>> 
>>> doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
>>> 1 file changed, 30 insertions(+)
>>> 
>>> diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
>>> index 7dd3d45df12..1227c04d04f 100644
>>> --- a/doc/device-tree-bindings/net/aquantia-phy.txt
>>> +++ b/doc/device-tree-bindings/net/aquantia-phy.txt
>>> @@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
>>> Several optional bindings are defined that allow these configuration points to
>>> be driven by the PHY driver and reduce dependency on specific FW versions.
>>> 
>>> +Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
>>> +or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
>>> +The driver uploads only when the PHY reports missing firmware or a fault.
>>> +
>>> Optional properties:
>>> mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
>>>              Firmware default is used if the property is missing.
>>> smb-addr:     I2C/SMBus address to use, firmware default is used if the property
>>>              is missing.
>>> +firmware-name: String containing the filename of the PHY firmware to load
>>> +               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).
>> 
>> This looks good to me, but I have a question: should we switch to the
>> upstream binding for aquantia phys? It's already documented as
>> marvell,aquantia.yaml, and we could avoid the burden of maintaining a
>> separate binding file.
>> 
>> The "firmware-name" property is already described in the upstream
>> marvell,aquantia.yaml, and it only misses the smb-addr property. The
>> only U-Boot boards making use of this property are fsl-sch-30841 and
>> fsl-sch-30842, thus such conversion shouldn't be a big job.
> 
> Good point. I’ll add a bit more info in related Kconfig options and drop the doc.

Hmm, I’m a bit lost. Here’s some of my concerns:

- Upstream binding has an optional nvmem cell as firmware source, such mechanism
   simply doesn’t exist in U-Boot. This could be misleading.
- About “smb-addr”, what’s the best move? Do we upstream them to Linux?

Any idea on how to fix these issues?

Thanks,
Beiyan Yun

> 
>> 
>> Best regards,
>> Yao Zi
>> 
>>> Example node:
>>> phy@00 {
>>> 	reg = <0x00>;
>>> 	mdi-reversal = <1>;
>>> 	smb-addr = <0x25>;
>>> +	firmware-name = "aqr-firmware.cld";
>>> +};
>>> +
>>> +Example using the generic firmware loader:
>>> +/	{
>>> +	chosen {
>>> +		/* Select default firmware loader instance */
>>> +		firmware-loader = &fs_loader0;
>>> +	};
>>> +
>>> +	fs_loader0: fs-loader@0 {
>>> +		bootph-all;
>>> +		compatible = "u-boot,fs-loader";
>>> +		/* Load from MMC0, partition 1 */
>>> +		phandlepart = <&mmc_0 1>;
>>> +	};
>>> +
>>> +	mdio {
>>> +		phy@0 {
>>> +			reg = <0>;
>>> +			/* Load this file via the selected fs-loader */
>>> +			firmware-name = "aqr-firmware.cld";
>>> +		};
>>> +	};
>>> };
>>> -- 
>>> 2.47.3
> 
> Cheers,
> Beiyan Yun

Re: [PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Yao Zi]

On Fri, Sep 26, 2025 at 05:30:03PM +0800, Beiyan Yun wrote:
> 
> 
> > On 26 Sep 2025, at 4:22 PM, Beiyan Yun <root@infi.wang> wrote:
> > 
> > 
> > 
> >> On 23 Sep 2025, at 8:44 PM, Yao Zi <ziyao@disroot.org <mailto:ziyao@disroot.org>> wrote:
> >> 
> >> On Tue, Sep 23, 2025 at 03:13:01PM +0800, Beiyan Yun wrote:
> >>> With the switch to generic firmware loader, "firmware-name" binding
> >>> was introduced to define the firmware filename.
> >>> Provide the document and usage examples.
> >>> 
> >>> Signed-off-by: Beiyan Yun <root@infi.wang <mailto:root@infi.wang> <mailto:root@infi.wang>>
> >> 
> >> IMO this patch should go before the driver change.
> >> 
> >>> ---
> >>> 
> >>> doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
> >>> 1 file changed, 30 insertions(+)
> >>> 
> >>> diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
> >>> index 7dd3d45df12..1227c04d04f 100644
> >>> --- a/doc/device-tree-bindings/net/aquantia-phy.txt
> >>> +++ b/doc/device-tree-bindings/net/aquantia-phy.txt
> >>> @@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
> >>> Several optional bindings are defined that allow these configuration points to
> >>> be driven by the PHY driver and reduce dependency on specific FW versions.
> >>> 
> >>> +Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
> >>> +or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
> >>> +The driver uploads only when the PHY reports missing firmware or a fault.
> >>> +
> >>> Optional properties:
> >>> mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
> >>>              Firmware default is used if the property is missing.
> >>> smb-addr:     I2C/SMBus address to use, firmware default is used if the property
> >>>              is missing.
> >>> +firmware-name: String containing the filename of the PHY firmware to load
> >>> +               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).
> >> 
> >> This looks good to me, but I have a question: should we switch to the
> >> upstream binding for aquantia phys? It's already documented as
> >> marvell,aquantia.yaml, and we could avoid the burden of maintaining a
> >> separate binding file.
> >> 
> >> The "firmware-name" property is already described in the upstream
> >> marvell,aquantia.yaml, and it only misses the smb-addr property. The
> >> only U-Boot boards making use of this property are fsl-sch-30841 and
> >> fsl-sch-30842, thus such conversion shouldn't be a big job.
> > 
> > Good point. I’ll add a bit more info in related Kconfig options and drop the doc.
> 
> Hmm, I’m a bit lost. Here’s some of my concerns:
> 
> - Upstream binding has an optional nvmem cell as firmware source, such mechanism
>    simply doesn’t exist in U-Boot. This could be misleading.

I think it's okay to support part of the binding in U-Boot: the goal is
to reuse upstream binding, avoiding the burden of maintaining a
duplication, and making it possible to share devicetree between kernel
and firmware (same binding means same ABI).

I did a brief search in the upstream dts tree, and found no consumer
of this nvmem-cells property, thus I don't think it's a problem, at
least for now.

> - About “smb-addr”, what’s the best move? Do we upstream them to Linux?

Yes, if it's really necesary for the phy to work. If you decide to
adapt the upstream binding in the series, then I suggest upstreaming the
property to dt-bindings first.

Anyway I think your original patch looks good to me as well, it isn't a
must (at least to me) to switch to the upstream binding.

> Any idea on how to fix these issues?
> 
> Thanks,
> Beiyan Yun

Best regards,
Yao Zi

Re: [PATCH RESEND 4/4] doc: bindings: add Aquantia PHY node's "firmware-name" binding

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4216 bytes --]

On Sun, Sep 28, 2025 at 01:04:16PM +0000, Yao Zi wrote:
> On Fri, Sep 26, 2025 at 05:30:03PM +0800, Beiyan Yun wrote:
> > 
> > 
> > > On 26 Sep 2025, at 4:22 PM, Beiyan Yun <root@infi.wang> wrote:
> > > 
> > > 
> > > 
> > >> On 23 Sep 2025, at 8:44 PM, Yao Zi <ziyao@disroot.org <mailto:ziyao@disroot.org>> wrote:
> > >> 
> > >> On Tue, Sep 23, 2025 at 03:13:01PM +0800, Beiyan Yun wrote:
> > >>> With the switch to generic firmware loader, "firmware-name" binding
> > >>> was introduced to define the firmware filename.
> > >>> Provide the document and usage examples.
> > >>> 
> > >>> Signed-off-by: Beiyan Yun <root@infi.wang <mailto:root@infi.wang> <mailto:root@infi.wang>>
> > >> 
> > >> IMO this patch should go before the driver change.
> > >> 
> > >>> ---
> > >>> 
> > >>> doc/device-tree-bindings/net/aquantia-phy.txt | 30 +++++++++++++++++++
> > >>> 1 file changed, 30 insertions(+)
> > >>> 
> > >>> diff --git a/doc/device-tree-bindings/net/aquantia-phy.txt b/doc/device-tree-bindings/net/aquantia-phy.txt
> > >>> index 7dd3d45df12..1227c04d04f 100644
> > >>> --- a/doc/device-tree-bindings/net/aquantia-phy.txt
> > >>> +++ b/doc/device-tree-bindings/net/aquantia-phy.txt
> > >>> @@ -11,15 +11,45 @@ a custom firmware is needed for each integration of a PHY.
> > >>> Several optional bindings are defined that allow these configuration points to
> > >>> be driven by the PHY driver and reduce dependency on specific FW versions.
> > >>> 
> > >>> +Aquantia PHY's firmware is often provided by PHY-resident SPI flash; if absent
> > >>> +or outdated, U-Boot can upload firmware over MDIO during PHY initialization.
> > >>> +The driver uploads only when the PHY reports missing firmware or a fault.
> > >>> +
> > >>> Optional properties:
> > >>> mdi-reversal: 0 or 1 indicating that reversal must be disabled/enabled.
> > >>>              Firmware default is used if the property is missing.
> > >>> smb-addr:     I2C/SMBus address to use, firmware default is used if the property
> > >>>              is missing.
> > >>> +firmware-name: String containing the filename of the PHY firmware to load
> > >>> +               (only when CONFIG_PHY_AQUANTIA_UPLOAD_FW is enabled).
> > >> 
> > >> This looks good to me, but I have a question: should we switch to the
> > >> upstream binding for aquantia phys? It's already documented as
> > >> marvell,aquantia.yaml, and we could avoid the burden of maintaining a
> > >> separate binding file.
> > >> 
> > >> The "firmware-name" property is already described in the upstream
> > >> marvell,aquantia.yaml, and it only misses the smb-addr property. The
> > >> only U-Boot boards making use of this property are fsl-sch-30841 and
> > >> fsl-sch-30842, thus such conversion shouldn't be a big job.
> > > 
> > > Good point. I’ll add a bit more info in related Kconfig options and drop the doc.
> > 
> > Hmm, I’m a bit lost. Here’s some of my concerns:
> > 
> > - Upstream binding has an optional nvmem cell as firmware source, such mechanism
> >    simply doesn’t exist in U-Boot. This could be misleading.
> 
> I think it's okay to support part of the binding in U-Boot: the goal is
> to reuse upstream binding, avoiding the burden of maintaining a
> duplication, and making it possible to share devicetree between kernel
> and firmware (same binding means same ABI).
> 
> I did a brief search in the upstream dts tree, and found no consumer
> of this nvmem-cells property, thus I don't think it's a problem, at
> least for now.
> 
> > - About “smb-addr”, what’s the best move? Do we upstream them to Linux?
> 
> Yes, if it's really necesary for the phy to work. If you decide to
> adapt the upstream binding in the series, then I suggest upstreaming the
> property to dt-bindings first.
> 
> Anyway I think your original patch looks good to me as well, it isn't a
> must (at least to me) to switch to the upstream binding.

Figuring out how to use the upstream binding, and addressing issues /
short-comings there is the best path forward. Maybe U-Boot will need
some method for being able to load firmware blobs from nvme cells in the
future for example.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]