Re: [PATCH v5 5/5] net: rnpgbe: Add register_netdev

From: Yibo Dong
Date: Wed Aug 20 2025 - 23:45:30 EST


On Wed, Aug 20, 2025 at 10:42:47PM +0200, Andrew Lunn wrote:
> > +static int rnpgbe_get_permanent_mac(struct mucse_hw *hw,
> > + u8 *mac_addr)
> > +{
> > + struct device *dev = &hw->pdev->dev;
> > +
> > + if (mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->port) ||
> > + !is_valid_ether_addr(mac_addr)) {
> > + dev_err(dev, "Failed to get valid MAC from FW\n");
> > + return -EINVAL;
>
> I _think_ mucse_fw_get_macaddr() can return -EINTR, because deep down,
> it has a call to mutex_lock_interruptible(). If that happens, you
> should not return iEINVAL, it is not an invalid value, its just an
> interrupted system call.
>
> This is what i'm talking about needing careful review...
>
> Andrew
>
> ---
> pw-bot: cr
>
>

Ok, Maybe like This?
Just return function return if mucse_fw_get_macaddr failed, and return
-EINVAL if not a valid mac.

static int rnpgbe_get_permanent_mac(struct mucse_hw *hw,
u8 *mac_addr)
{
struct device *dev = &hw->pdev->dev;
int ret;

ret = mucse_fw_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->port);
if (ret) {
dev_err(dev, "Failed to get MAC from FW\n");
return ret;
}

if (!is_valid_ether_addr(mac_addr)) {
dev_err(dev, "MAC from FW is not valid\n");
return -EINVAL;
}

return 0;
}

Thanks for your feedback.