Initialize get mac from hw, register the netdev.

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

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 4d2cca59fb23..33ee6f05e9b8 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,27 @@ struct mucse_mbx_info {
 	u32 fw2pf_mbox_vec;
 };
 
+struct mucse_hw;
+
+struct mucse_hw_operations {
+	int (*reset_hw)(struct mucse_hw *hw);
+	void (*driver_status)(struct mucse_hw *hw, bool enable, int mode);
+};
+
+enum {
+	mucse_driver_insmod,
+};
+
 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 +92,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..f25dbc0dab5a 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,11 +1,87 @@
 // 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
+ * @mac_addr: pointer to store mac
+ *
+ * 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,
+				    u8 *mac_addr)
+{
+	struct device *dev = &hw->pdev->dev;
+	int err;
+
+	err = mucse_fw_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;
+	int err;
+
+	rnpgbe_dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
+	err = mucse_mbx_fw_reset_phy(hw);
+	if (err)
+		return err;
+	return rnpgbe_get_permanent_mac(hw, hw->perm_addr);
+}
+
+/**
+ * rnpgbe_driver_status_hw_ops - Echo driver status to hw
+ * @hw: hw information structure
+ * @enable: true or false status
+ * @mode: status mode
+ **/
+static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
+					bool enable,
+					int mode)
+{
+	switch (mode) {
+	case mucse_driver_insmod:
+		mucse_mbx_ifinsmod(hw, enable);
+		break;
+	}
+}
+
+static const struct mucse_hw_operations rnpgbe_hw_ops = {
+	.reset_hw = &rnpgbe_reset_hw_ops,
+	.driver_status = &rnpgbe_driver_status_hw_ops,
+};
 
 /**
  * rnpgbe_init_common - Setup common attribute
@@ -13,10 +89,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..82805b61cd07 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,27 @@ static int rnpgbe_add_adapter(struct pci_dev *pdev,
 
 	hw->hw_addr = hw_addr;
 	info->init(hw);
+	mucse_init_mbx_params_pf(hw);
+	/* echo fw driver insmod to control hw */
+	hw->ops->driver_status(hw, true, mucse_driver_insmod);
+	err = mucse_mbx_get_capability(hw);
+	if (err) {
+		dev_err(&pdev->dev,
+			"mucse_mbx_get_capability 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;
+	}
+	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 +217,15 @@ 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;
 
 	if (!mucse)
 		return;
 	netdev = mucse->netdev;
+	unregister_netdev(netdev);
 	mucse->netdev = NULL;
+	hw->ops->driver_status(hw, false, mucse_driver_insmod);
 	free_netdev(netdev);
 }
 
-- 
2.25.1

> +/**
> + * 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;
> +	int err;
> +
> +	rnpgbe_dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
> +	err = mucse_mbx_fw_reset_phy(hw);
> +	if (err)
> +		return err;
> +	return rnpgbe_get_permanent_mac(hw, hw->perm_addr);

Why is a function named rnpgbe_reset_hw_ops() getting the permanent
MAC address? Reset should not have anything to do with MAC address.

If the MAC address is not valid, you normally use a random MAC
address. But you cannot easily differentiate between the reset failed,
the reset got ^C, and the MAC address was not valid.

> +/**
> + * rnpgbe_driver_status_hw_ops - Echo driver status to hw
> + * @hw: hw information structure
> + * @enable: true or false status
> + * @mode: status mode
> + **/
> +static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
> +					bool enable,
> +					int mode)
> +{
> +	switch (mode) {
> +	case mucse_driver_insmod:
> +		mucse_mbx_ifinsmod(hw, enable);

Back to the ^C handling. This could be interrupted before the firmware
is told the driver is loaded. That EINTR is thrown away here, so the
driver thinks everything is O.K, but the firmware still thinks there
is no MAC driver. What happens then?

And this is the same problem i pointed out before, you ignore EINTR in
a void function. Rather than fix one instance, you should of reviewed
the whole driver and fixed them all. You cannot expect the Reviewers
to do this for you.

    Andrew

---
pw-bot: cr

On Thu, Aug 28, 2025 at 03:20:04PM +0200, Andrew Lunn wrote:
> > +/**
> > + * 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;
> > +	int err;
> > +
> > +	rnpgbe_dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
> > +	err = mucse_mbx_fw_reset_phy(hw);
> > +	if (err)
> > +		return err;
> > +	return rnpgbe_get_permanent_mac(hw, hw->perm_addr);
> 
> Why is a function named rnpgbe_reset_hw_ops() getting the permanent
> MAC address? Reset should not have anything to do with MAC address.
> 
> If the MAC address is not valid, you normally use a random MAC
> address. But you cannot easily differentiate between the reset failed,
> the reset got ^C, and the MAC address was not valid.
> 

Ok, I will remove call 'rnpgbe_get_permanent_mac' in function
'rnpgbe_reset_hw_ops'. And call it in probe.

> > +/**
> > + * rnpgbe_driver_status_hw_ops - Echo driver status to hw
> > + * @hw: hw information structure
> > + * @enable: true or false status
> > + * @mode: status mode
> > + **/
> > +static void rnpgbe_driver_status_hw_ops(struct mucse_hw *hw,
> > +					bool enable,
> > +					int mode)
> > +{
> > +	switch (mode) {
> > +	case mucse_driver_insmod:
> > +		mucse_mbx_ifinsmod(hw, enable);
> 
> Back to the ^C handling. This could be interrupted before the firmware
> is told the driver is loaded. That EINTR is thrown away here, so the
> driver thinks everything is O.K, but the firmware still thinks there
> is no MAC driver. What happens then?
> 

The performance will be very poor since low working frequency,
that is not we want.

> And this is the same problem i pointed out before, you ignore EINTR in
> a void function. Rather than fix one instance, you should of reviewed
> the whole driver and fixed them all. You cannot expect the Reviewers
> to do this for you.

I see, I will change 'void' to 'int' in order to handle err, and try to check
other functions.

> 
>     Andrew
> 
> ---
> pw-bot: cr
> 

Thanks for your feedback.

> > Back to the ^C handling. This could be interrupted before the firmware
> > is told the driver is loaded. That EINTR is thrown away here, so the
> > driver thinks everything is O.K, but the firmware still thinks there
> > is no MAC driver. What happens then?
> > 
> 
> The performance will be very poor since low working frequency,
> that is not we want.
> 
> > And this is the same problem i pointed out before, you ignore EINTR in
> > a void function. Rather than fix one instance, you should of reviewed
> > the whole driver and fixed them all. You cannot expect the Reviewers
> > to do this for you.
> 
> I see, I will change 'void' to 'int' in order to handle err, and try to check
> other functions.

Also, consider, do you really want ^C handling? How many other drivers
do this? How much time and effort is it going to take you to fix up
all the calls which might return -EINTR and your code is currently
broken?

	Andrew

On Fri, Aug 29, 2025 at 09:51:57PM +0200, Andrew Lunn wrote:
> > > Back to the ^C handling. This could be interrupted before the firmware
> > > is told the driver is loaded. That EINTR is thrown away here, so the
> > > driver thinks everything is O.K, but the firmware still thinks there
> > > is no MAC driver. What happens then?
> > > 
> > 
> > The performance will be very poor since low working frequency,
> > that is not we want.
> > 
> > > And this is the same problem i pointed out before, you ignore EINTR in
> > > a void function. Rather than fix one instance, you should of reviewed
> > > the whole driver and fixed them all. You cannot expect the Reviewers
> > > to do this for you.
> > 
> > I see, I will change 'void' to 'int' in order to handle err, and try to check
> > other functions.
> 
> Also, consider, do you really want ^C handling? How many other drivers
> do this? How much time and effort is it going to take you to fix up
> all the calls which might return -EINTR and your code is currently
> broken?
> 
> 	Andrew
> 

Originally ^C is designed to handle '/sys/xxx’, and as you said before,
'It is pretty unusual for ethernet drivers to export data in /sys'.
I should use 'metex_lock', not 'mutex_lock_interruptible'.

Thanks for your feedback.