On Tue, Aug 12, 2025 at 04:49:33PM +0100, Vadim Fedorenko wrote:

quoted

+struct mucse_dma_info {
+	void __iomem *dma_base_addr;
+	void __iomem *dma_ring_addr;
+	void *back;

it might be better to keep the type of back pointer and give it
a bit more meaningful name ...

quoted

+	u32 dma_version;
+};
+
+struct mucse_eth_info {
+	void __iomem *eth_base_addr;
+	void *back;

.. here ...

quoted

+};
+
+struct mucse_mac_info {
+	void __iomem *mac_addr;
+	void *back;

and here...

quoted

+};
+
+struct mucse_mbx_info {
+	/* fw <--> pf mbx */
+	u32 fw_pf_shm_base;
+	u32 pf2fw_mbox_ctrl;
+	u32 pf2fw_mbox_mask;
+	u32 fw_pf_mbox_mask;
+	u32 fw2pf_mbox_vec;
+};
+
+struct mucse_hw {
+	void *back;

you can also use container_of() as all these structures are embedded and
simple pointer math can give you proper result.

Got it, I will use container_of(), and remove the '*back' define.
Maybe eth to hw like this:
#define eth_to_hw(eth) container_of(eth, struct rnpgbe_hw, eth)
It is ok?

quoted

+	void __iomem *hw_addr;
+	void __iomem *ring_msix_base;
+	struct pci_dev *pdev;
+	enum rnpgbe_hw_type hw_type;
+	struct mucse_dma_info dma;
+	struct mucse_eth_info eth;
+	struct mucse_mac_info mac;
+	struct mucse_mbx_info mbx;
+	u32 driver_version;
+	u16 usecstocount;
+};
+
  struct mucse {
  	struct net_device *netdev;
  	struct pci_dev *pdev;
+	struct mucse_hw hw;
  	u16 bd_number;
  };

[...]

quoted

+/**
+ * rnpgbe_add_adapter - Add netdev for this pci_dev
+ * @pdev: PCI device information structure
+ * @info: chip info structure
+ *
+ * rnpgbe_add_adapter initializes a netdev for this pci_dev
+ * structure. Initializes Bar map, private structure, and a
+ * hardware reset occur.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int rnpgbe_add_adapter(struct pci_dev *pdev,
+			      const struct rnpgbe_info *info)
+{
+	struct net_device *netdev;
+	void __iomem *hw_addr;
+	static int bd_number;

it's not clear from the patchset why do you need this static variable...

Ok, bd_number seems no usefull, I will remove it.

quoted

+	struct mucse *mucse;
+	struct mucse_hw *hw;
+	u32 dma_version = 0;
+	u32 queues;
+	int err;
+
+	queues = info->total_queue_pair_cnts;
+	netdev = alloc_etherdev_mq(sizeof(struct mucse), queues);
+	if (!netdev)
+		return -ENOMEM;
+
+	SET_NETDEV_DEV(netdev, &pdev->dev);
+	mucse = netdev_priv(netdev);
+	mucse->netdev = netdev;
+	mucse->pdev = pdev;
+	mucse->bd_number = bd_number++;

... but this code is racy by design

quoted

+	pci_set_drvdata(pdev, mucse);
+
+	hw = &mucse->hw;
+	hw->back = mucse;
+	hw->hw_type = info->hw_type;
+	hw->pdev = pdev;
+

Thanks for your feedback.