on 2024/9/3 19:59, Paolo Abeni wrote:

On 8/30/24 14:15, Jijie Shao wrote:
[...]

quoted

+static int hbg_mdio_wait_ready(struct hbg_mac *mac)
+{
+#define HBG_MDIO_OP_TIMEOUT_US        (1 * 1000 * 1000)
+#define HBG_MDIO_OP_INTERVAL_US        (5 * 1000)

Minor nit: I find the define inside the function body less readable 
than placing them just before the function itself.

These two macros are only used in this function.
Is it necessary to move them to the header file?

quoted

+
+    struct hbg_priv *priv = HBG_MAC_GET_PRIV(mac);
+    u32 cmd;
+
+    return readl_poll_timeout(priv->io_base + 
HBG_REG_MDIO_COMMAND_ADDR, cmd,
+                  !FIELD_GET(HBG_REG_MDIO_COMMAND_START_B, cmd),
+                  HBG_MDIO_OP_INTERVAL_US,
+                  HBG_MDIO_OP_TIMEOUT_US);
+}

[...]> +static void hbg_phy_adjust_link(struct net_device *netdev)

quoted

+{
+    struct hbg_priv *priv = netdev_priv(netdev);
+    struct phy_device *phydev = priv->mac.phydev;

Minor nit: please respect the reverse x-mas tree order

Here, I need to get the *priv first, so I'm not following the reverse x-mas tree order here.
I respect the reverse x-mas tree order everywhere else.

	Thanks,
	Jijie Shao