It was <2021-10-01 pią 15:39>, when Jakub Kicinski wrote:
On Wed, 29 Sep 2021 16:08:54 +0200 Łukasz Stelmach wrote:
quoted
+static char *no_regs_list = "80018001,e1918001,8001a001,fc0d0000";
static const char ...
quoted
+static int
+ax88796c_close(struct net_device *ndev)
+{
+ struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
+
+ netif_stop_queue(ndev);
This can run concurrently with the work which restarts the queue.
You should take the mutex and purge the queue here, so that there
is no chance queue will get restarted by the work right after.
quoted
+ phy_stop(ndev->phydev);
+
+ mutex_lock(&ax_local->spi_lock);
This was a bit tricky. The function now looks like this (details in the
comments). In theory there is a chance of dropping a packet if the
ax88796c_work() runs between phy_stop() and mutex_lock(), but I'd say
the overall risk is negligible.
--8<---------------cut here---------------start------------->8---
static int
ax88796c_close(struct net_device *ndev)
{
struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
phy_stop(ndev->phydev);
/* We lock the mutex early not only to protect the device
* against concurrent access, but also avoid waking up the
* queue in ax88796c_work(). phy_stop() needs to be called
* before because it locks the mutex to access SPI.
*/
mutex_lock(&ax_local->spi_lock);
netif_stop_queue(ndev);
/* No more work can be scheduled now. Make any pending work,
* including one already waiting for the mutex to be unlocked,
* NOP.
*/
clear_bit(EVENT_SET_MULTI, &ax_local->flags);
clear_bit(EVENT_INTR, &ax_local->flags);
clear_bit(EVENT_TX, &ax_local->flags);
/* Disable MAC interrupts */
AX_WRITE(&ax_local->ax_spi, IMR_MASKALL, P0_IMR);
__skb_queue_purge(&ax_local->tx_wait_q);
ax88796c_soft_reset(ax_local);
mutex_unlock(&ax_local->spi_lock);
cancel_work_sync(&ax_local->ax_work);
free_irq(ndev->irq, ndev);
return 0;
}
--8<---------------cut here---------------end--------------->8---
quoted
+MODULE_AUTHOR("ASIX");
You can drop this, author should be human.
I looked at diff --stat and decided it is OK to put my name there (-;
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics