linux-kernel - Re: [PATCH v4 5/5] net: rnpgbe: Add register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b669db06-83f8-447c-8081-7ef6ae9d2aba@lunn.ch>
Date: Fri, 15 Aug 2025 20:06:35 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Yibo Dong <dong100@...se.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com,
	mpe@...erman.id.au, danishanwar@...com, lee@...ger.us,
	gongfan1@...wei.com, lorenzo@...nel.org, geert+renesas@...der.be,
	Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
	alexanderduyck@...com, richardcochran@...il.com,
	netdev@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/5] net: rnpgbe: Add register_netdev

> > > +static int rnpgbe_reset_hw_ops(struct mucse_hw *hw)
> > > +{
> > > +	struct mucse_dma_info *dma = &hw->dma;
> > > +	int err;
> > > +
> > > +	dma_wr32(dma, RNPGBE_DMA_AXI_EN, 0);
> > > +	err = mucse_mbx_fw_reset_phy(hw);
> > > +	if (err)
> > > +		return err;
> > > +	/* Store the permanent mac address */
> > > +	if (!(hw->flags & M_FLAGS_INIT_MAC_ADDRESS))
> > 
> > What do this hw->flags add to the driver? Why is it here?
> > 
> 
> It is used to init 'permanent addr' only once.
> rnpgbe_reset_hw_ops maybe called when netdev down or hw hang, no need
> try to get 'permanent addr' more times.

It normally costs ~0 to ask the firmware something. So it is generally
simpler to just ask it. If the firmware is dead, you should not really
care, the RPC should timeout, ETIMEDOUT will get returned to user
space, and likely everything else will fail anyway.
 
> > >  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;
> > > +	if (netdev->reg_state == NETREG_REGISTERED)
> > > +		unregister_netdev(netdev);
> > 
> > Is that possible?
> > 
> 
> Maybe probe failed before register_netdev? Then rmmod the driver.

Functions like this come in pairs. There is some sort of setup
function, and a corresponding teardown function. probe/remove,
open/close. In Linux, if the first fails, the second is never called.

	Andrew