Initialize get mac from hw, register the netdev.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    | 24 +++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   | 86 ++++++++++++++++++
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |  2 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 89 +++++++++++++++++++
 4 files changed, 201 insertions(+)

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 4d2cca59fb23..92bd3ba76c72 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -6,6 +6,7 @@
 
 #include <linux/types.h>
 #include <linux/mutex.h>
+#include <linux/netdevice.h>
 
 extern const struct rnpgbe_info rnpgbe_n500_info;
 extern const struct rnpgbe_info rnpgbe_n210_info;
@@ -24,6 +25,10 @@ enum rnpgbe_hw_type {
 	rnpgbe_hw_unknown
 };
 
+struct mucse_dma_info {
+	void __iomem *dma_base_addr;
+};
+
 struct mucse_mbx_stats {
 	u32 msgs_tx;
 	u32 msgs_rx;
@@ -47,12 +52,28 @@ struct mucse_mbx_info {
 	u32 fw2pf_mbox_vec;
 };
 
+struct mucse_hw;
+
+struct mucse_hw_operations {
+	int (*reset_hw)(struct mucse_hw *hw);
+	int (*get_perm_mac)(struct mucse_hw *hw);
+	int (*echo_fw_status)(struct mucse_hw *hw, bool enable, int mode);
+};
+
+enum {
+	mucse_fw_powerup,
+};
+
 struct mucse_hw {
 	void __iomem *hw_addr;
 	struct pci_dev *pdev;
 	enum rnpgbe_hw_type hw_type;
 	u8 pfvfnum;
+	const struct mucse_hw_operations *ops;
+	struct mucse_dma_info dma;
 	struct mucse_mbx_info mbx;
+	int port;
+	u8 perm_addr[ETH_ALEN];
 };
 
 struct mucse {
@@ -72,4 +93,7 @@ struct rnpgbe_info {
 #define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
 #define PCI_DEVICE_ID_N210 0x8208
 #define PCI_DEVICE_ID_N210L 0x820a
+
+#define rnpgbe_dma_wr32(dma, reg, val) \
+	writel((val), (dma)->dma_base_addr + (reg))
 #endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index f38daef752a3..1c70653545f2 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,11 +1,91 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright(c) 2020 - 2025 Mucse Corporation. */
 
+#include <linux/pci.h>
 #include <linux/string.h>
+#include <linux/etherdevice.h>
 
 #include "rnpgbe.h"
 #include "rnpgbe_hw.h"
 #include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
+
+/**
+ * rnpgbe_get_permanent_mac - Get permanent mac
+ * @hw: hw information structure
+ *
+ * rnpgbe_get_permanent_mac tries to get mac from hw.
+ * It use eth_random_addr if failed.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int rnpgbe_get_permanent_mac(struct mucse_hw *hw)
+{
+	struct device *dev = &hw->pdev->dev;
+	u8 *mac_addr = hw->perm_addr;
+	int err;
+
+	err = mucse_mbx_get_macaddr(hw, hw->pfvfnum, mac_addr, hw->port);
+	if (err) {
+		dev_err(dev, "Failed to get MAC from FW %d\n", err);
+		return err;
+	}
+
+	if (!is_valid_ether_addr(mac_addr)) {
+		dev_err(dev, "Failed to get valid MAC from FW\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * rnpgbe_reset_hw_ops - Do a hardware reset
+ * @hw: hw information structure
+ *
+ * rnpgbe_reset_hw_ops calls fw to do a hardware
+ * reset, and cleans some regs to default.
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
+{
+	struct mucse_dma_info *dma = &hw->dma;
+
+	rnpgbe_dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
+	return mucse_mbx_reset_hw(hw);
+}
+
+/**
+ * rnpgbe_echo_fw_status_hw_ops - Echo fw status
+ * @hw: hw information structure
+ * @enable: true or false status
+ * @mode: status mode
+ *
+ * Return: 0 on success, negative errno on failure
+ **/
+static int rnpgbe_echo_fw_status_hw_ops(struct mucse_hw *hw,
+					bool enable,
+					int mode)
+{
+	int err;
+
+	switch (mode) {
+	case mucse_fw_powerup:
+		err = mucse_mbx_powerup(hw, enable);
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static const struct mucse_hw_operations rnpgbe_hw_ops = {
+	.reset_hw = &rnpgbe_reset_hw_ops,
+	.get_perm_mac = &rnpgbe_get_permanent_mac,
+	.echo_fw_status = &rnpgbe_echo_fw_status_hw_ops,
+};
 
 /**
  * rnpgbe_init_common - Setup common attribute
@@ -13,10 +93,16 @@
  **/
 static void rnpgbe_init_common(struct mucse_hw *hw)
 {
+	struct mucse_dma_info *dma = &hw->dma;
 	struct mucse_mbx_info *mbx = &hw->mbx;
 
+	dma->dma_base_addr = hw->hw_addr;
+
 	mbx->pf2fw_mbox_ctrl = GBE_PF2FW_MBX_MASK_OFFSET;
 	mbx->fw_pf_mbox_mask = GBE_FWPF_MBX_MASK;
+
+	hw->ops = &rnpgbe_hw_ops;
+	hw->port = 0;
 }
 
 /**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index 746dca78f1df..0ab2c328c9e9 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -11,6 +11,8 @@
 #define GBE_FWPF_MBX_MASK 0x5700
 #define N210_FW2PF_MBX_VEC_OFFSET 0x29400
 #define N210_FWPF_SHM_BASE_OFFSET 0x2d900
+/**************** DMA Registers ****************************/
+#define RNPGBE_DMA_AXI_EN 0x0010
 /**************** CHIP Resource ****************************/
 #define RNPGBE_MAX_QUEUES 8
 #endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 25b7119d6ecb..4562aeba4a24 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -9,6 +9,8 @@
 
 #include "rnpgbe.h"
 #include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_mbx_fw.h"
 
 static const char rnpgbe_driver_name[] = "rnpgbe";
 static const struct rnpgbe_info *rnpgbe_info_tbl[] = {
@@ -35,6 +37,55 @@ static struct pci_device_id rnpgbe_pci_tbl[] = {
 	{0, },
 };
 
+/**
+ * rnpgbe_open - Called when a network interface is made active
+ * @netdev: network interface device structure
+ *
+ * The open entry point is called when a network interface is made
+ * active by the system (IFF_UP).
+ *
+ * Return: 0 on success, negative value on failure
+ **/
+static int rnpgbe_open(struct net_device *netdev)
+{
+	return 0;
+}
+
+/**
+ * rnpgbe_close - Disables a network interface
+ * @netdev: network interface device structure
+ *
+ * The close entry point is called when an interface is de-activated
+ * by the OS.
+ *
+ * Return: 0, this is not allowed to fail
+ **/
+static int rnpgbe_close(struct net_device *netdev)
+{
+	return 0;
+}
+
+/**
+ * rnpgbe_xmit_frame - Send a skb to driver
+ * @skb: skb structure to be sent
+ * @netdev: network interface device structure
+ *
+ * Return: NETDEV_TX_OK or NETDEV_TX_BUSY
+ **/
+static netdev_tx_t rnpgbe_xmit_frame(struct sk_buff *skb,
+				     struct net_device *netdev)
+{
+	dev_kfree_skb_any(skb);
+	netdev->stats.tx_dropped++;
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops rnpgbe_netdev_ops = {
+	.ndo_open = rnpgbe_open,
+	.ndo_stop = rnpgbe_close,
+	.ndo_start_xmit = rnpgbe_xmit_frame,
+};
+
 /**
  * rnpgbe_add_adapter - Add netdev for this pci_dev
  * @pdev: PCI device information structure
@@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
 
 	hw->hw_addr = hw_addr;
 	info->init(hw);
+	mucse_init_mbx_params_pf(hw);
+	err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
+	if (err) {
+		dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
+		dev_warn(&pdev->dev, "Maybe low performance\n");
+	}
+
+	err = mucse_mbx_sync_fw(hw);
+	if (err) {
+		dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
+		goto err_free_net;
+	}
+	netdev->netdev_ops = &rnpgbe_netdev_ops;
+	netdev->watchdog_timeo = 5 * HZ;
+	err = hw->ops->reset_hw(hw);
+	if (err) {
+		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
+		goto err_free_net;
+	}
+	err = hw->ops->get_perm_mac(hw);
+	if (err == -EINVAL) {
+		dev_warn(&pdev->dev, "Try to use random MAC\n");
+		eth_random_addr(hw->perm_addr);
+	} else if (err) {
+		dev_err(&pdev->dev, "get perm_addr failed %d\n", err);
+		goto err_free_net;
+	}
+	eth_hw_addr_set(netdev, hw->perm_addr);
+	err = register_netdev(netdev);
+	if (err)
+		goto err_free_net;
+
 	return 0;
 
 err_free_net:
@@ -145,12 +228,18 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 {
 	struct mucse *mucse = pci_get_drvdata(pdev);
+	struct mucse_hw *hw = &mucse->hw;
 	struct net_device *netdev;
+	int err;
 
 	if (!mucse)
 		return;
 	netdev = mucse->netdev;
+	unregister_netdev(netdev);
 	mucse->netdev = NULL;
+	err = hw->ops->echo_fw_status(hw, false, mucse_fw_powerup);
+	if (err)
+		dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
 	free_netdev(netdev);
 }
 
-- 
2.25.1

>   * rnpgbe_add_adapter - Add netdev for this pci_dev
>   * @pdev: PCI device information structure
> @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
>  
>  	hw->hw_addr = hw_addr;
>  	info->init(hw);
> +	mucse_init_mbx_params_pf(hw);
> +	err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> +	if (err) {
> +		dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> +		dev_warn(&pdev->dev, "Maybe low performance\n");
> +	}
> +
> +	err = mucse_mbx_sync_fw(hw);
> +	if (err) {
> +		dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> +		goto err_free_net;
> +	}

The order here seems odd. Don't you want to synchronise the mbox
before you power up? If your are out of sync, the power up could fail,
and you keep in lower power mode? 

> +	netdev->netdev_ops = &rnpgbe_netdev_ops;
> +	netdev->watchdog_timeo = 5 * HZ;
> +	err = hw->ops->reset_hw(hw);
> +	if (err) {
> +		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
> +		goto err_free_net;
> +	}
> +	err = hw->ops->get_perm_mac(hw);
> +	if (err == -EINVAL) {
> +		dev_warn(&pdev->dev, "Try to use random MAC\n");
> +		eth_random_addr(hw->perm_addr);

eth_random_addr() cannot fail. So you don't try to use a random MAC
address, you are using a random MAC address/

	Andrew

On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> >   * rnpgbe_add_adapter - Add netdev for this pci_dev
> >   * @pdev: PCI device information structure
> > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> >  
> >  	hw->hw_addr = hw_addr;
> >  	info->init(hw);
> > +	mucse_init_mbx_params_pf(hw);
> > +	err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > +	if (err) {
> > +		dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > +		dev_warn(&pdev->dev, "Maybe low performance\n");
> > +	}
> > +
> > +	err = mucse_mbx_sync_fw(hw);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > +		goto err_free_net;
> > +	}
> 
> The order here seems odd. Don't you want to synchronise the mbox
> before you power up? If your are out of sync, the power up could fail,
> and you keep in lower power mode? 
> 

As I explained before, powerup sends mbx and wait fw read out, but
without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
the corect response from fw, after mucse_mbx_sync_fw, driver->fw
request and fw->driver response will be both ok.

> > +	netdev->netdev_ops = &rnpgbe_netdev_ops;
> > +	netdev->watchdog_timeo = 5 * HZ;
> > +	err = hw->ops->reset_hw(hw);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Hw reset failed %d\n", err);
> > +		goto err_free_net;
> > +	}
> > +	err = hw->ops->get_perm_mac(hw);
> > +	if (err == -EINVAL) {
> > +		dev_warn(&pdev->dev, "Try to use random MAC\n");
> > +		eth_random_addr(hw->perm_addr);
> 
> eth_random_addr() cannot fail. So you don't try to use a random MAC
> address, you are using a random MAC address/
> 
> 	Andrew
> 

Maybe update it like this?
if (err == -EINVAL) {
	dev_warn(&pdev->dev, "Using a random MAC\n");
....

Thanks for your feedback.

On Thu, Sep 04, 2025 at 11:06:21AM +0800, Yibo Dong wrote:
> On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> > >   * rnpgbe_add_adapter - Add netdev for this pci_dev
> > >   * @pdev: PCI device information structure
> > > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > >  
> > >  	hw->hw_addr = hw_addr;
> > >  	info->init(hw);
> > > +	mucse_init_mbx_params_pf(hw);
> > > +	err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > > +	if (err) {
> > > +		dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > > +		dev_warn(&pdev->dev, "Maybe low performance\n");
> > > +	}
> > > +
> > > +	err = mucse_mbx_sync_fw(hw);
> > > +	if (err) {
> > > +		dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > > +		goto err_free_net;
> > > +	}
> > 
> > The order here seems odd. Don't you want to synchronise the mbox
> > before you power up? If your are out of sync, the power up could fail,
> > and you keep in lower power mode? 
> > 
> 
> As I explained before, powerup sends mbx and wait fw read out, but
> without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
> the corect response from fw, after mucse_mbx_sync_fw, driver->fw
> request and fw->driver response will be both ok.

Because this is logically the wrong order, this deserves a comment.

You choice of function names for the lower level functions also does
not help. It is not so easy to look at the function used to know if it
is a request/response to the firmware, or just a request without a
response.

	Andrew

On Thu, Sep 04, 2025 at 02:35:06PM +0200, Andrew Lunn wrote:
> On Thu, Sep 04, 2025 at 11:06:21AM +0800, Yibo Dong wrote:
> > On Thu, Sep 04, 2025 at 12:53:27AM +0200, Andrew Lunn wrote:
> > > >   * rnpgbe_add_adapter - Add netdev for this pci_dev
> > > >   * @pdev: PCI device information structure
> > > > @@ -78,6 +129,38 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
> > > >  
> > > >  	hw->hw_addr = hw_addr;
> > > >  	info->init(hw);
> > > > +	mucse_init_mbx_params_pf(hw);
> > > > +	err = hw->ops->echo_fw_status(hw, true, mucse_fw_powerup);
> > > > +	if (err) {
> > > > +		dev_warn(&pdev->dev, "Send powerup to hw failed %d\n", err);
> > > > +		dev_warn(&pdev->dev, "Maybe low performance\n");
> > > > +	}
> > > > +
> > > > +	err = mucse_mbx_sync_fw(hw);
> > > > +	if (err) {
> > > > +		dev_err(&pdev->dev, "Sync fw failed! %d\n", err);
> > > > +		goto err_free_net;
> > > > +	}
> > > 
> > > The order here seems odd. Don't you want to synchronise the mbox
> > > before you power up? If your are out of sync, the power up could fail,
> > > and you keep in lower power mode? 
> > > 
> > 
> > As I explained before, powerup sends mbx and wait fw read out, but
> > without response data from fw. mucse_mbx_sync_fw sends mbx and wait for
> > the corect response from fw, after mucse_mbx_sync_fw, driver->fw
> > request and fw->driver response will be both ok.
> 
> Because this is logically the wrong order, this deserves a comment.
> 
> You choice of function names for the lower level functions also does
> not help. It is not so easy to look at the function used to know if it
> is a request/response to the firmware, or just a request without a
> response.
> 
> 	Andrew
> 

Got it, I will add comment.

...
    /*
     * Step 1: Send power-up notification to firmware (no response expected)
     * This informs firmware to initialize hardware power state, but firmware
     * only acknowledges receipt without returning data. Must be done before
     * synchronization as firmware may be in low-power idle state initially.
     */
    mucse_init_mbx_params_pf(hw);
    err = hw->ops->mbx_send_notify(hw, true, mucse_fw_powerup);
    if (err) {
        dev_warn(&pdev->dev, "Failed to send power-up notification to firmware: %d\n", err);
        dev_warn(&pdev->dev, "Performance may be limited\n");
    }

    /*
     * Step 2: Synchronize mailbox communication with firmware (requires response)
     * After power-up, confirm firmware is ready to process requests with responses.
     * This ensures subsequent request/response interactions work reliably.
     */
    err = mucse_mbx_sync_fw(hw);
...
And lower lever function names will be consided to be renamed.

Thanks for your feedback.