phy: rockchip-inno-usb2: fix some race conditions

There are some race conditions related to phy power on/off
and otg charger detection work, otg sm work. I can find at
least three race conditions at present.

Race condition[1]:
The first race condition involving phy power on/off which
may be caused by the following case.

Test on rk3399 evaluation board Type-C0, connect to PC usb
port with Type-C cable, then phy power on/off operation may
be done twice because of race condition between phy driver
and usb controller driver.

CPU 0:
- rockchip_usb2phy_bvalid_irq()
 - rockchip_usb2phy_otg_sm_work()
  - detect connect to PC usb, do phy power on
   - rockchip_usb2phy_power_on()

CPU 1:
- dwc3 driver do runtime resume process
 - dwc3_runtime_resume()
  - dwc3_core_init()
   - phy_power_on()
    - rockchip_usb2phy_power_on()

Although we use a suspended flag in rockchip_usb2phy_power_on()
to avoid doing the same things twice, but it's not enough to
prevent race condition if phy driver and usb controller driver
access the rockchip_usb2phy_power_on() at the same time. This
race condition may cause clk management unbalanced.

Race condition[2]:
The second race condition related to phy power on/off and otg
charger detection work. We need to keep the usb phy staying in
suspend mode when do usb charger detection. But now it don't
have any protection to prevent the other threads to operate phy
during charger detection.

The problem can also be easily reproduced on rk3399 evaluation
board Type-C0 when connect to PC usb port with Type-C cable.

CPU 0:
- rockchip_chg_detect_work()
 - power off phy and start to do charge detection work

CPU 1:
- dwc3 driver do runtime resume process
 - dwc3_runtime_resume()
  - dwc3_core_init()
   - phy_power_on()
    - power on phy again

This race condition may cause charger detection and later usb
enumeration abnormally.

Race condition[3]:
The third race condition involving otg sm work. The otg sm
work can be interrupted by bvalid irq, and the bvalid irq
handler rockchip_usb2phy_bvalid_irq() will do otg sm work,
which may cause unknown error.

This patch uses mutex lock to protect the phy operations,
otg charger detection work and otg sm work.

Change-Id: Ic6845a10b3e69fe9ae6cf0b2d4e2beb098232abd
Signed-off-by: William Wu <william.wu@rock-chips.com>

diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
index e005fa52d77ab4a545a8b7ba739a1621000fb814..8c930fcac0cd9bd56b65ce5ce18d0fb3c48d3d63 100644 (file)
--- a/drivers/phy/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/phy-rockchip-inno-usb2.c
@@ -524,22 +524,30 @@ static int rockchip_usb2phy_power_on(struct phy *phy)
 
        dev_dbg(&rport->phy->dev, "port power on\n");
 
-       if (!rport->suspended)
-               return 0;
+       mutex_lock(&rport->mutex);
+
+       if (!rport->suspended) {
+               ret = 0;
+               goto unlock;
+       }
 
        ret = clk_prepare_enable(rphy->clk480m);
        if (ret)
-               return ret;
+               goto unlock;
 
        ret = property_enable(rphy, &rport->port_cfg->phy_sus, false);
        if (ret)
-               return ret;
+               goto unlock;
 
        /* waiting for the utmi_clk to become stable */
        usleep_range(1500, 2000);
 
        rport->suspended = false;
-       return 0;
+
+unlock:
+       mutex_unlock(&rport->mutex);
+
+       return ret;
 }
 
 static int rockchip_usb2phy_power_off(struct phy *phy)
@@ -550,23 +558,32 @@ static int rockchip_usb2phy_power_off(struct phy *phy)
 
        dev_dbg(&rport->phy->dev, "port power off\n");
 
-       if (rport->suspended)
-               return 0;
+       mutex_lock(&rport->mutex);
+
+       if (rport->suspended) {
+               ret = 0;
+               goto unlock;
+       }
 
        ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
        if (ret)
-               return ret;
+               goto unlock;
 
        rport->suspended = true;
        clk_disable_unprepare(rphy->clk480m);
 
-       return 0;
+unlock:
+       mutex_unlock(&rport->mutex);
+
+       return ret;
 }
 
 static int rockchip_usb2phy_exit(struct phy *phy)
 {
        struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
 
+       mutex_lock(&rport->mutex);
+
        if (rport->port_id == USB2PHY_PORT_OTG &&
            rport->mode != USB_DR_MODE_HOST &&
            rport->mode != USB_DR_MODE_UNKNOWN &&
@@ -575,6 +592,8 @@ static int rockchip_usb2phy_exit(struct phy *phy)
        else if (rport->port_id == USB2PHY_PORT_HOST)
                cancel_delayed_work_sync(&rport->sm_work);
 
+       mutex_unlock(&rport->mutex);
+
        return 0;
 }
 
@@ -633,6 +652,8 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
        unsigned long delay;
        bool sch_work;
 
+       mutex_lock(&rport->mutex);
+
        if (rport->utmi_avalid)
                rport->vbus_attached =
                        property_enabled(rphy, &rport->port_cfg->utmi_avalid);
@@ -649,8 +670,11 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
        switch (rport->state) {
        case OTG_STATE_UNDEFINED:
                rport->state = OTG_STATE_B_IDLE;
-               if (!rport->vbus_attached)
+               if (!rport->vbus_attached) {
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_off(rport->phy);
+                       mutex_lock(&rport->mutex);
+               }
                /* fall through */
        case OTG_STATE_B_IDLE:
                if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) > 0 ||
@@ -658,12 +682,14 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                                            EXTCON_USB_VBUS_EN) > 0) {
                        dev_dbg(&rport->phy->dev, "usb otg host connect\n");
                        rport->state = OTG_STATE_A_HOST;
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_on(rport->phy);
                        return;
                } else if (rport->vbus_attached) {
                        dev_dbg(&rport->phy->dev, "vbus_attach\n");
                        switch (rphy->chg_state) {
                        case USB_CHG_STATE_UNDEFINED:
+                               mutex_unlock(&rport->mutex);
                                schedule_delayed_work(&rport->chg_work, 0);
                                return;
                        case USB_CHG_STATE_DETECTED:
@@ -673,7 +699,9 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                                                "sdp cable is connecetd\n");
                                        wake_lock(&rport->wakelock);
                                        cable = EXTCON_CHG_USB_SDP;
+                                       mutex_unlock(&rport->mutex);
                                        rockchip_usb2phy_power_on(rport->phy);
+                                       mutex_lock(&rport->mutex);
                                        rport->state = OTG_STATE_B_PERIPHERAL;
                                        rport->perip_connected = true;
                                        sch_work = true;
@@ -682,7 +710,9 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                                        dev_dbg(&rport->phy->dev,
                                                "dcp cable is connecetd\n");
                                        cable = EXTCON_CHG_USB_DCP;
+                                       mutex_unlock(&rport->mutex);
                                        rockchip_usb2phy_power_off(rport->phy);
+                                       mutex_lock(&rport->mutex);
                                        sch_work = true;
                                        break;
                                case POWER_SUPPLY_TYPE_USB_CDP:
@@ -690,7 +720,9 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                                                "cdp cable is connecetd\n");
                                        wake_lock(&rport->wakelock);
                                        cable = EXTCON_CHG_USB_CDP;
+                                       mutex_unlock(&rport->mutex);
                                        rockchip_usb2phy_power_on(rport->phy);
+                                       mutex_lock(&rport->mutex);
                                        rport->state = OTG_STATE_B_PERIPHERAL;
                                        rport->perip_connected = true;
                                        sch_work = true;
@@ -699,7 +731,9 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                                        dev_dbg(&rport->phy->dev,
                                                "floating cable is connecetd\n");
                                        cable = EXTCON_CHG_USB_DCP;
+                                       mutex_unlock(&rport->mutex);
                                        rockchip_usb2phy_power_off(rport->phy);
+                                       mutex_lock(&rport->mutex);
                                        sch_work = true;
                                        break;
                                default:
@@ -722,7 +756,9 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                        rport->state = OTG_STATE_B_IDLE;
                        rport->perip_connected = false;
                        delay = 0;
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_off(rport->phy);
+                       mutex_lock(&rport->mutex);
                        wake_unlock(&rport->wakelock);
                } else {
                        sch_work = true;
@@ -732,10 +768,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
                if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) == 0) {
                        dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
                        rport->state = OTG_STATE_B_IDLE;
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_off(rport->phy);
+                       return;
                }
-               return;
        default:
+               mutex_unlock(&rport->mutex);
                return;
        }
 
@@ -752,6 +790,8 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 
        if (sch_work)
                schedule_delayed_work(&rport->otg_sm_work, delay);
+
+       mutex_unlock(&rport->mutex);
 }
 
 static const char *chg_to_string(enum power_supply_type chg_type)
@@ -805,10 +845,15 @@ static void rockchip_chg_detect_work(struct work_struct *work)
 
        dev_dbg(&rport->phy->dev, "chg detection work state = %d\n",
                rphy->chg_state);
+
        switch (rphy->chg_state) {
        case USB_CHG_STATE_UNDEFINED:
-               if (!rport->suspended)
+               mutex_lock(&rport->mutex);
+               if (!rport->suspended) {
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_off(rport->phy);
+                       mutex_lock(&rport->mutex);
+               }
                /* put the controller in non-driving mode */
                property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, false);
                /* Start DCD processing stage 1 */
@@ -886,14 +931,21 @@ static void rockchip_chg_detect_work(struct work_struct *work)
        case USB_CHG_STATE_DETECTED:
                /* put the controller in normal mode */
                property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, true);
+               mutex_unlock(&rport->mutex);
                rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
                dev_info(&rport->phy->dev, "charger = %s\n",
                         chg_to_string(rphy->chg_type));
                return;
        default:
+               mutex_unlock(&rport->mutex);
                return;
        }
 
+       /*
+        * Hold the mutex lock during the whole charger
+        * detection stage, and release it after detect
+        * the charger type.
+        */
        schedule_delayed_work(&rport->chg_work, delay);
 }
 
@@ -970,7 +1022,9 @@ static void rockchip_usb2phy_sm_work(struct work_struct *work)
        case PHY_STATE_CONNECT:
                if (rport->suspended) {
                        dev_dbg(&rport->phy->dev, "Connected\n");
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_on(rport->phy);
+                       mutex_lock(&rport->mutex);
                        rport->suspended = false;
                } else {
                        /* D+ line pull-up, D- line pull-down */
@@ -980,7 +1034,9 @@ static void rockchip_usb2phy_sm_work(struct work_struct *work)
        case PHY_STATE_DISCONNECT:
                if (!rport->suspended) {
                        dev_dbg(&rport->phy->dev, "Disconnected\n");
+                       mutex_unlock(&rport->mutex);
                        rockchip_usb2phy_power_off(rport->phy);
+                       mutex_lock(&rport->mutex);
                        rport->suspended = true;
                }