Hi Marek
Thank you for reviewing my patch.
Marek Vasut 於 2025/9/15 上午 01:49 寫道:
On 9/14/25 6:41 AM, Lucien.Jheng wrote:
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 <lucienzx...@gmail.com>
---
Change in PATCH v4:
air_en8811.c:
* Create the airoha folder
directory
to store all future airoha phy drivers.
* Use `request_firmware_into_buf_via_script` for firmware loading.
[...]
+++ b/drivers/net/phy/airoha/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menuconfig PHY_AIROHA
+ bool "Airoha Ethernet PHYs support"
+
+config PHY_AIROHA_EN8811
+ bool "Airoha Ethernet EN8811H support"
+ depends on PHY_AIROHA
+ select FS_LOADER
It seems tabs/spaces are mixed here, please fix.
+ help
+ AIROHA EN8811H supported.
[...]
+#define EN8811H_MD32_DM_SIZE 0x4000
+#define EN8811H_MD32_DSP_SIZE 0x20000
+
+ #define EN8811H_FW_CTRL_1 0x0f0018
+ #define EN8811H_FW_CTRL_1_START 0x0
+ #define EN8811H_FW_CTRL_1_FINISH 0x1
+ #define EN8811H_FW_CTRL_2 0x800000
+ #define EN8811H_FW_CTRL_2_LOADING BIT(11)
Same here, avoid leading space.
[...]
+__weak int en8811h_read_fw(void **addr)
Does this still have to be a __weak function, now that it uses generic
fw loader ?
+{
+ void *buffer;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_FS_LOADER))
+ return -EOPNOTSUPP;
+
+ buffer = malloc(EN8811H_MD32_DM_SIZE + EN8811H_MD32_DSP_SIZE);
+ if (!buffer) {
+ printf("Failed to allocate memory for firmware\n");
+ return -ENOMEM;
+ }
+
+ ret = request_firmware_into_buf_via_script(&buffer,
I think 'buffer' without the & should be used here.
Regarding buffer,
I've reviewed the function definition for
request_firmware_into_buf_via_script.
The first parameter is void **buf, which is a pointer to a pointer.
I think passing &buffer is the correct way to call this function.
If we were to pass just buffer (a void *), the function would receive a
copy of the pointer's value, and any memory allocation inside the
function would not be reflected in our original buffer variable.
Please let me know if my understanding is incorrect, as I want to ensure
I'm using the function as intended.
+ EN8811H_MD32_DM_SIZE + EN8811H_MD32_DSP_SIZE,
+ "en8811h_load_firmware");
[...]
