Re: [PATCH v15, 2/2] net: Add dm9051 driver

[Jakub Kicinski]

On Fri, 28 Jan 2022 14:45:32 +0800 Joseph CHAMG wrote:
> +/* Open network device
> + * Called when the network device is marked active, such as a user executing
> + * 'ifconfig up' on the device
> + */
> +static int dm9051_open(struct net_device *ndev)
> +{
> +	struct board_info *db = to_dm9051_board(ndev);
> +	struct spi_device *spi = db->spidev;
> +	int ret;
> +
> +	db->imr_all = IMR_PAR | IMR_PRM;
> +	db->rcr_all = RCR_DIS_LONG | RCR_DIS_CRC | RCR_RXEN;
> +	db->lcr_all = LMCR_MODE1;
> +
> +	netif_wake_queue(ndev);

This should be last, after the device is actually ready to transmit.

> +	ndev->irq = spi->irq; /* by dts */
> +	ret = request_threaded_irq(spi->irq, NULL, dm9051_rx_threaded_irq,
> +				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +				   ndev->name, db);
> +	if (ret < 0) {
> +		netdev_err(ndev, "failed to get irq\n");
> +		return ret;
> +	}
> +
> +	phy_support_sym_pause(db->phydev); /* Enable support of sym pause */
> +	phy_start(db->phydev); /* it enclose with mutex_lock/mutex_unlock */
> +
> +	ret = dm9051_all_start(db);
> +	if (ret) {
> +		phy_stop(db->phydev);
> +		free_irq(spi->irq, db);
> +		netif_stop_queue(ndev);
> +		return ret;
> +	}
> +
> +	/* init pause param FlowCtrl */
> +	db->eth_pause.rx_pause = true;
> +	db->eth_pause.tx_pause = true;
> +	db->eth_pause.autoneg = AUTONEG_DISABLE;
> +
> +	if (db->phydev->autoneg)
> +		db->eth_pause.autoneg = AUTONEG_ENABLE;
> +
> +	return 0;
> +}
> +
> +/* Close network device
> + * Called to close down a network device which has been active. Cancel any
> + * work, shutdown the RX and TX process and then place the chip into a low
> + * power state while it is not being used
> + */
> +static int dm9051_stop(struct net_device *ndev)
> +{
> +	struct board_info *db = to_dm9051_board(ndev);
> +
> +	phy_stop(db->phydev);
> +	free_irq(db->spidev->irq, db);
> +	netif_stop_queue(ndev);

I don't see anything draining &db->txq when the worker is stopped.
It should probably be drained here, after the queue is stopped.

> +	return dm9051_all_stop(db);
> +}