Commit 35b5f6b1 authored by Nate Case's avatar Nate Case Committed by David S. Miller
Browse files

PHYLIB: Locking fixes for PHY I/O potentially sleeping 



PHY read/write functions can potentially sleep (e.g., a PHY accessed
via I2C).  The following changes were made to account for this:

    * Change spin locks to mutex locks
    * Add a BUG_ON() to phy_read() phy_write() to warn against
      calling them from an interrupt context.
    * Use work queue for PHY state machine handling since
      it can potentially sleep
    * Change phydev lock from spinlock to mutex

Signed-off-by: default avatarNate Case <ncase@xes-inc.com>
Acked-by: default avatarAndy Fleming <afleming@freescale.com>
Signed-off-by: default avatarJeff Garzik <jeff@garzik.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2b912130

drivers/net/phy/mdio_bus.c

+1 −1
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ int mdiobus_register(struct mii_bus *bus) 
	int i;

	int err = 0;



	spin_lock_init(&bus->mdio_lock);

	mutex_init(&bus->mdio_lock);



	if (NULL == bus || NULL == bus->name ||

			NULL == bus->read ||

drivers/net/phy/phy.c

+46 −22
Original line number Diff line number Diff line
@@ -26,7 +26,6 @@ 
#include <linux/netdevice.h>

#include <linux/etherdevice.h>

#include <linux/skbuff.h>

#include <linux/spinlock.h>

#include <linux/mm.h>

#include <linux/module.h>

#include <linux/mii.h>
@@ -72,9 +71,11 @@ int phy_read(struct phy_device *phydev, u16 regnum) 
	int retval;

	struct mii_bus *bus = phydev->bus;



	spin_lock_bh(&bus->mdio_lock);

	BUG_ON(in_interrupt());



	mutex_lock(&bus->mdio_lock);

	retval = bus->read(bus, phydev->addr, regnum);

	spin_unlock_bh(&bus->mdio_lock);

	mutex_unlock(&bus->mdio_lock);



	return retval;

}
@@ -95,9 +96,11 @@ int phy_write(struct phy_device *phydev, u16 regnum, u16 val) 
	int err;

	struct mii_bus *bus = phydev->bus;



	spin_lock_bh(&bus->mdio_lock);

	BUG_ON(in_interrupt());



	mutex_lock(&bus->mdio_lock);

	err = bus->write(bus, phydev->addr, regnum, val);

	spin_unlock_bh(&bus->mdio_lock);

	mutex_unlock(&bus->mdio_lock);



	return err;

}
@@ -428,7 +431,7 @@ int phy_start_aneg(struct phy_device *phydev) 
{

	int err;



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);



	if (AUTONEG_DISABLE == phydev->autoneg)

		phy_sanitize_settings(phydev);
@@ -449,13 +452,14 @@ int phy_start_aneg(struct phy_device *phydev) 
	}



out_unlock:

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);

	return err;

}

EXPORT_SYMBOL(phy_start_aneg);





static void phy_change(struct work_struct *work);

static void phy_state_machine(struct work_struct *work);

static void phy_timer(unsigned long data);



/**
@@ -476,6 +480,7 @@ void phy_start_machine(struct phy_device *phydev, 
{

	phydev->adjust_state = handler;



	INIT_WORK(&phydev->state_queue, phy_state_machine);

	init_timer(&phydev->phy_timer);

	phydev->phy_timer.function = &phy_timer;

	phydev->phy_timer.data = (unsigned long) phydev;
@@ -493,11 +498,12 @@ void phy_start_machine(struct phy_device *phydev, 
void phy_stop_machine(struct phy_device *phydev)

{

	del_timer_sync(&phydev->phy_timer);

	cancel_work_sync(&phydev->state_queue);



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);

	if (phydev->state > PHY_UP)

		phydev->state = PHY_UP;

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	phydev->adjust_state = NULL;

}
@@ -541,9 +547,9 @@ static void phy_force_reduction(struct phy_device *phydev) 
 */

void phy_error(struct phy_device *phydev)

{

	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);

	phydev->state = PHY_HALTED;

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);

}



/**
@@ -705,10 +711,10 @@ static void phy_change(struct work_struct *work) 
	if (err)

		goto phy_err;



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);

	if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))

		phydev->state = PHY_CHANGELINK;

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	atomic_dec(&phydev->irq_disable);

	enable_irq(phydev->irq);
@@ -735,7 +741,7 @@ phy_err: 
 */

void phy_stop(struct phy_device *phydev)

{

	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);



	if (PHY_HALTED == phydev->state)

		goto out_unlock;
@@ -751,7 +757,7 @@ void phy_stop(struct phy_device *phydev) 
	phydev->state = PHY_HALTED;



out_unlock:

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	/*

	 * Cannot call flush_scheduled_work() here as desired because
@@ -773,7 +779,7 @@ out_unlock: 
 */

void phy_start(struct phy_device *phydev)

{

	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);



	switch (phydev->state) {

		case PHY_STARTING:
@@ -787,19 +793,26 @@ void phy_start(struct phy_device *phydev) 
		default:

			break;

	}

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);

}

EXPORT_SYMBOL(phy_stop);

EXPORT_SYMBOL(phy_start);



/* PHY timer which handles the state machine */

static void phy_timer(unsigned long data)

/**

 * phy_state_machine - Handle the state machine

 * @work: work_struct that describes the work to be done

 *

 * Description: Scheduled by the state_queue workqueue each time

 *   phy_timer is triggered.

 */

static void phy_state_machine(struct work_struct *work)

{

	struct phy_device *phydev = (struct phy_device *)data;

	struct phy_device *phydev =

			container_of(work, struct phy_device, state_queue);

	int needs_aneg = 0;

	int err = 0;



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);



	if (phydev->adjust_state)

		phydev->adjust_state(phydev->attached_dev);
@@ -965,7 +978,7 @@ static void phy_timer(unsigned long data) 
			break;

	}



	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	if (needs_aneg)

		err = phy_start_aneg(phydev);
@@ -976,3 +989,14 @@ static void phy_timer(unsigned long data) 
	mod_timer(&phydev->phy_timer, jiffies + PHY_STATE_TIME * HZ);

}



/* PHY timer which schedules the state machine work */

static void phy_timer(unsigned long data)

{

	struct phy_device *phydev = (struct phy_device *)data;



	/*

	 * PHY I/O operations can potentially sleep so we ensure that

	 * it's done from a process context

	 */

	schedule_work(&phydev->state_queue);

}

drivers/net/phy/phy_device.c

+5 −6
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ 
#include <linux/netdevice.h>

#include <linux/etherdevice.h>

#include <linux/skbuff.h>

#include <linux/spinlock.h>

#include <linux/mm.h>

#include <linux/module.h>

#include <linux/mii.h>
@@ -80,7 +79,7 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id) 


	dev->state = PHY_DOWN;



	spin_lock_init(&dev->lock);

	mutex_init(&dev->lock);



	return dev;

}
@@ -656,7 +655,7 @@ static int phy_probe(struct device *dev) 
	if (!(phydrv->flags & PHY_HAS_INTERRUPT))

		phydev->irq = PHY_POLL;



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);



	/* Start out supporting everything. Eventually,

	 * a controller will attach, and may modify one
@@ -670,7 +669,7 @@ static int phy_probe(struct device *dev) 
	if (phydev->drv->probe)

		err = phydev->drv->probe(phydev);



	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	return err;
@@ -682,9 +681,9 @@ static int phy_remove(struct device *dev) 


	phydev = to_phy_device(dev);



	spin_lock_bh(&phydev->lock);

	mutex_lock(&phydev->lock);

	phydev->state = PHY_DOWN;

	spin_unlock_bh(&phydev->lock);

	mutex_unlock(&phydev->lock);



	if (phydev->drv->remove)

		phydev->drv->remove(phydev);

include/linux/phy.h

+3 −2
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ struct mii_bus { 


	/* A lock to ensure that only one thing can read/write

	 * the MDIO bus at a time */

	spinlock_t mdio_lock;

	struct mutex mdio_lock;



	struct device *dev;
@@ -284,10 +284,11 @@ struct phy_device { 


	/* Interrupt and Polling infrastructure */

	struct work_struct phy_queue;

	struct work_struct state_queue;

	struct timer_list phy_timer;

	atomic_t irq_disable;



	spinlock_t lock;

	struct mutex lock;



	struct net_device *attached_dev;

CRA Git | Maintained and supported by SUSTech CRA and CCSE