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

[Andrew Lunn <andrew@lunn.ch>]

> > > +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