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

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: <20220128133551.55c09fca@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 28 Jan 2022 13:35:51 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Joseph CHAMG <josright123@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>, joseph_chang@...icom.com.tw,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, andy.shevchenko@...il.com,
        andrew@...n.ch, leon@...nel.org
Subject: Re: [PATCH v15, 2/2] net: Add dm9051 driver

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);
> +}