Aw: [U-Boot, v3, 1/1]net: phy: Add the Airoha EN8811H PHY driver

[frank-w at public-files.de]

Hi

> Gesendet: Sonntag, 20. Juli 2025 um 14:28
> Von: "Lucien.Jheng" <lucienzx159 at gmail.com>
> Betreff: [U-Boot, v3, 1/1]net: phy: Add the Airoha EN8811H PHY driver
>
> Add the driver for the Airoha EN8811H 2.5 Gigabit PHY. The PHY supports
> 100/1000/2500 Mbps with auto negotiation only.
> 
> The driver uses two firmware files, for which updated versions are added to
> linux-firmware already.
> 
> Locating the AIROHA FW within the filesystem at the designated partition
> and path will trigger its automatic loading and writing to the PHY via MDIO.
> If need board specific loading override,
> please override the en8811h_read_fw function on board or architecture level.
> 
> Linux upstream AIROHA EN8811H driver commit:
> 71e79430117d56c409c5ea485a263bc0d8083390
> 
> Based on the Linux upstream AIROHA EN8811H driver code(air_en8811h.c),
> I have modified the relevant process to align with the U-Boot boot sequence.
> and have validated this on Banana Pi BPI-R3 Mini.
> 
> Signed-off-by: Lucien.Jheng <lucienzx159 at gmail.com>
> ---
> Change in PATCH v3:
> air_en8811h.c:
>  * Add U-Boot environment variable(en8811h_fw_part, en8811h_fw_dm_dir, en8811h_fw_dsp_dir)
>    support for firmware loading.

Thank you lucien for taking care of this driver...

>  drivers/net/phy/Kconfig       |  25 +
>  drivers/net/phy/Makefile      |   1 +
>  drivers/net/phy/air_en8811h.c | 880 ++++++++++++++++++++++++++++++++++
>  3 files changed, 906 insertions(+)
>  create mode 100644 drivers/net/phy/air_en8811h.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 8d88c142900..9f12238e1ea 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -79,6 +79,31 @@ config PHY_ADIN
>  	help
>  		Add support for configuring RGMII on Analog Devices ADIN PHYs.
> 
> +menuconfig PHY_AIROHA
> +	bool "Airoha Ethernet PHYs support"
> +
> +config PHY_AIROHA_EN8811H
> +	bool "Airoha Ethernet EN8811H support"
> +	depends on PHY_AIROHA
> +	help
> +	  AIROHA EN8811H supported.
> +
> +choice
> +	prompt "Location of the Airoha PHY firmware"
> +	default PHY_AIROHA_FW_IN_MMC
> +	depends on PHY_AIROHA_EN8811H
> +
> +config PHY_AIROHA_FW_IN_MMC
> +	bool "Airoha firmware in MMC partition"
> +	help
> +	  Airoha PHYs use firmware which can be loaded automatically
> +	  from storage directly attached to the PHY, and then loaded
> +	  via MDIO commands by the boot loader. The firmware is loaded
> +	  from a file specified by the U-Boot environment variables
> +	  en8811h_fw_part, en8811h_fw_dm_dir, and en8811h_fw_dsp_dir.

maybe something like PHY_AIROHA_FW_IN_SUBSYSTEM = (MMC/USB/NVME/...) as choice? or better PHY_AIROHA_FW_BY_ENV and then check env-var with default "mmc" for maximum on flexibility without recompile.

> +endchoice
> +
>  menuconfig PHY_AQUANTIA
>  	bool "Aquantia Ethernet PHYs support"
>  	select PHY_GIGE
> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> new file mode 100644
> index 00000000000..1020b6d75b2
> --- /dev/null
> +++ b/drivers/net/phy/air_en8811h.c
...
> +__weak int en8811h_read_fw(void **addr)
> +{
> +	const char *fw_dm, *fw_dsp, *fw_part;
> +	u32 ca_crc32;
> +	void *buffer;
> +	loff_t read;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_PHY_AIROHA_FW_IN_MMC))
> +		return -EOPNOTSUPP;
> +
> +	/* Allocate memory to store both DM and DSP firmware */
> +	buffer = malloc(EN8811H_MD32_DM_SIZE + EN8811H_MD32_DSP_SIZE);
> +	if (!buffer) {
> +		printf("Failed to allocate memory for firmware\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* Get the partition name where the firmware is stored */
> +	fw_part = env_get("en8811h_fw_part");
> +	if (!fw_part) {
> +		printf("Error: env var en8811h_fw_part not set.\n");
> +		return -EINVAL;
> +	}

this could include non mmc partitions, e.g. usb,nvme to be more generic (maybe separate var).
CONFIG-option of course needs to have different name.

> +	/* Get the DM firmware file path */
> +	fw_dm = env_get("en8811h_fw_dm_dir");
> +	if (!fw_dm) {
> +		printf("Error: env var en8811h_fw_dm_dir not set.\n");
> +		return -EINVAL;
> +	}

imho path could be separated from filename. Filename and Directory (which is mostly same for both files) could be hardcoded,
maybe as fallback if vars are not set, to same value as on linux. So users have only point to device and partition.

so default for dir: /lib/firmware/airoha
filenames: EthMD32.dm.bin and EthMD32.DSP.bin

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/airoha

linux driver only defines subpath, but imho on most systems this is below /lib/firmware in rootfs

https://elixir.bootlin.com/linux/v6.16-rc3/source/drivers/net/phy/air_en8811h.c#L23

if i define as dir only /lib/firmware and the airoha-path with filename gets expanded by driver, it will be ok for me, 
but imho it makes it harder to verify path is set or not and if airoha needs to be added or not. So i would use full
path, but default to linux default.

> +	/* Get the DSP firmware file path */
> +	fw_dsp = env_get("en8811h_fw_dsp_dir");
> +	if (!fw_dsp) {
> +		printf("Error: env var en8811h_fw_dsp_dir not set.\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Load DM firmware */
> +	ret = fs_set_blk_dev("mmc", fw_part, FS_TYPE_ANY);

this could use a subsystem env-var (with default mmc maybe) to support usb/nvme too.

> +	if (ret < 0)
> +		return ret;
> +
> +	/* Read DM firmware file into the start of buffer */
> +	ret = fs_read(fw_dm, (ulong)buffer, 0, EN8811H_MD32_DM_SIZE, &read);
> +	if (ret < 0) {
> +		printf("Failed to read DM firmware: %s\n", fw_dm);
> +		return ret;
> +	}
> +
> +	/* Calculate CRC32 of DM firmware for verification */
> +	ca_crc32 = crc32(0, (unsigned char *)buffer, EN8811H_MD32_DM_SIZE);
> +	debug("DM crc32: 0x%x\n", ca_crc32);
> +
> +	/* Load DSP firmware */
> +	ret = fs_set_blk_dev("mmc", fw_part, FS_TYPE_ANY);
> +	if (ret < 0)
> +		return ret;

same here for subsystem and part

> +	/* Read DSP firmware file into buffer after DM section */
> +	ret = fs_read(fw_dsp, (ulong)buffer + EN8811H_MD32_DM_SIZE, 0,
> +		EN8811H_MD32_DSP_SIZE, &read);
> +	if (ret < 0) {
> +		printf("Failed to read DSP firmware: %s\n", fw_dsp);
> +		return ret;
> +	}
> +
> +	/* Calculate CRC32 of DSP firmware for verification */
> +	ca_crc32 = crc32(0, (unsigned char *)buffer + EN8811H_MD32_DM_SIZE,
> +		EN8811H_MD32_DSP_SIZE);
> +	debug("DSP crc32: 0x%x\n", ca_crc32);
> +
> +	*addr = buffer;
> +	debug("Found Airoha Firmware.\n");
> +
> +	return 0;
> +}
> +
> +static int en8811h_load_firmware(struct phy_device *phydev)
> +{
> +	struct en8811h_priv *priv = phydev->priv;
> +	void *buffer;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_PHY_AIROHA_FW_IN_MMC)) {
> +		printf("Airoha EN8811H firmware loading not implemented\n");
> +		return -EOPNOTSUPP;
> +	}

this check is redundant in en8811h_read_fw which call follows, so imho this could be dropped here, maybe put printf in en8811h_read_fw.

> +	ret = en8811h_read_fw(&buffer);
> +	if (ret < 0)
> +		goto en8811h_load_firmware_out;
> +
> +	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> +				     EN8811H_FW_CTRL_1_START);
> +	if (ret < 0)
> +		goto en8811h_load_firmware_out;
> +

regards Frank