Re: [PATCH net-next v3 1/3] dinghai: add firmware version check and? RISC-V readiness polling

[han.junyang]

On Mon, Sep 21, 2026 at 02:56:44PM +0800, han.junyang@xxxxxxxxxx wrote:
> From: Junyang Han <han.junyang@xxxxxxxxxx>
>
> The DingHai firmware publishes a version compatibility block and a
> RISC-V health buffer at fixed offsets within BAR 0.
>
> After the PCI capabilities are mapped, poll the compatibility block
> until the firmware populates it (the region reads as all ones until
> then) and verify the driver/firmware version contract. Then wait for
> the RISC-V management core to set its power-on flag in the health
> buffer before the rest of the probe continues.
>
> Firmware images predating the health buffer protocol (health version
> other than 1 and patch level below ZXDH_HPIRQ_PATCH) skip the
> readiness wait.
>
> Signed-off-by: Junyang Han <han.junyang@xxxxxxxxxx>
> ---
> drivers/net/ethernet/zte/dinghai/en_pf.c | 113 +++++++++++++++++++++++
> drivers/net/ethernet/zte/dinghai/en_pf.h | 46 +++++++++
> 2 files changed, 159 insertions(+)
>
> diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.c b/drivers/net/ethernet/zte/dinghai/en_pf.c
> index 86d437408820..7c991e0951a8 100644
> --- a/drivers/net/ethernet/zte/dinghai/en_pf.c
> +++ b/drivers/net/ethernet/zte/dinghai/en_pf.c
> @@ -6,6 +6,8 @@
>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <net/devlink.h>
> #include <linux/dma-mapping.h>
> #include "en_pf.h"
> @@ -369,6 +371,103 @@ int zxdh_pf_modern_cfg_init(struct zxdh_core_dev *zxdh_dev)
> return ret;
> }
>
> +/* Read the firmware version block and verify the driver/firmware
> + * version contract.
> + */
> +static int zxdh_pf_fw_compat_check(struct zxdh_core_dev *zxdh_dev)
> +{
> + struct zxdh_pf_dev *pf_dev = zxdh_dev->priv;
> + struct zxdh_fw_compat __iomem *compat;
> + struct zxdh_fw_compat *fw_compat;
> + u32 erased;
> +
> + fw_compat = &pf_dev->fw_compat;
> + compat = pf_dev->pci_ioremap_addr[0] + ZXDH_FW_COMPAT_OFFSET;
> +
> + /* The region reads as all ones until the firmware populates it at
> + * the end of its boot; allow up to 200 s for a cold boot.
> + */
> + readx_poll_timeout(ioread32, compat, erased, erased != 0xffffffffU,
> + USEC_PER_SEC,
> + ZXDH_FW_COMPAT_TIMEOUT_SEC * USEC_PER_SEC);

> Hi,

> There is an AI-generated review of this patch available at
> https://sashiko.dev/#/patchset/202609211451400236_aZ55Ox3y7NW8MQnYImM%40zte.com.cn

> In my view the critical point made there, which I'd appreciate you looking
> into, is:

> Is the timeout error intentionally ignored here? If legacy firmware never
> populates this region, wouldn't the 200 second stall exceed the default
> udev timeout (180s), causing the worker to be killed and completely
> breaking legacy hardware support?

The ignored return value is intentional: distinguishing "firmware has not
populated the region yet" from "firmware never will" is only possible by waiting,
so the timeout itself is the legacy-firmware detection. On timeout the module id
check fails and the driver defers the decision to the readiness wait as
described in the commit message.

The udev concern is addressed in v4 by budget: the firmware publishes
the block within 10 s of boot, and the wait is now bounded at 20 s, an
order of magnitude below the 180 s event window, so even the
module-load path no longer risks the worker timeout. v4 also logs
the "assuming legacy firmware" case when the wait gives up.

While at it, the wait now checks every field of the block instead of
only the first dword: the firmware fills the fields one by one, so a
dword-granular check could observe a half-populated block.