| Message ID | 20200707074941.28078-2-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: some PM changes for cdns3 and xhci-plat | expand |
> >Since we have both USB2 and USB3 PHYs for cdns3 controller, it is >better we have unity APIs to handle both USB2 and USB3's power, it >could simplify code for error handling and further power management >implementation. > >Reviewed-by: Jun Li <jun.li@nxp.com> >Signed-off-by: Peter Chen <peter.chen@nxp.com> >--- > drivers/usb/cdns3/core.c | 43 ++++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 15 deletions(-) > >diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >index 19bbb5b7e6b6..8818935d157b 100644 >--- a/drivers/usb/cdns3/core.c >+++ b/drivers/usb/cdns3/core.c >@@ -384,6 +384,27 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role) > return ret; > } > >+static int set_phy_power_on(struct cdns3 *cdns) >+{ >+ int ret; >+ >+ ret = phy_power_on(cdns->usb2_phy); >+ if (ret) >+ return ret; >+ >+ ret = phy_power_on(cdns->usb3_phy); >+ if (ret) >+ phy_power_off(cdns->usb2_phy); >+ >+ return ret; >+} >+ >+static void set_phy_power_off(struct cdns3 *cdns) >+{ >+ phy_power_off(cdns->usb3_phy); >+ phy_power_off(cdns->usb2_phy); >+} >+ > /** > * cdns3_probe - probe for cdns3 core device > * @pdev: Pointer to cdns3 core platform device >@@ -477,14 +498,10 @@ static int cdns3_probe(struct platform_device *pdev) > if (ret) > goto err1; > >- ret = phy_power_on(cdns->usb2_phy); >+ ret = set_phy_power_on(cdns); > if (ret) > goto err2; > >- ret = phy_power_on(cdns->usb3_phy); >- if (ret) >- goto err3; >- > sw_desc.set = cdns3_role_set; > sw_desc.get = cdns3_role_get; > sw_desc.allow_userspace_control = true; >@@ -496,16 +513,16 @@ static int cdns3_probe(struct platform_device *pdev) > if (IS_ERR(cdns->role_sw)) { > ret = PTR_ERR(cdns->role_sw); > dev_warn(dev, "Unable to register Role Switch\n"); >- goto err4; >+ goto err3; > } > > ret = cdns3_drd_init(cdns); > if (ret) >- goto err5; >+ goto err4; > > ret = cdns3_core_init_role(cdns); > if (ret) >- goto err5; >+ goto err4; > > device_set_wakeup_capable(dev, true); > pm_runtime_set_active(dev); >@@ -522,14 +539,11 @@ static int cdns3_probe(struct platform_device *pdev) > dev_dbg(dev, "Cadence USB3 core: probe succeed\n"); > > return 0; >-err5: >+err4: > cdns3_drd_exit(cdns); > usb_role_switch_unregister(cdns->role_sw); >-err4: >- phy_power_off(cdns->usb3_phy); >- > err3: >- phy_power_off(cdns->usb2_phy); >+ set_phy_power_off(cdns); > err2: > phy_exit(cdns->usb3_phy); > err1: Dan Carpenter suggested me to use more meaningful labels instead err1 .. err5. Reviewed-by: Pawel Laszczak<pawell@cadence.com> Pawel >@@ -553,8 +567,7 @@ static int cdns3_remove(struct platform_device *pdev) > pm_runtime_put_noidle(&pdev->dev); > cdns3_exit_roles(cdns); > usb_role_switch_unregister(cdns->role_sw); >- phy_power_off(cdns->usb2_phy); >- phy_power_off(cdns->usb3_phy); >+ set_phy_power_off(cdns); > phy_exit(cdns->usb2_phy); > phy_exit(cdns->usb3_phy); > return 0; >-- >2.17.1
On 20-08-13 12:09:58, Pawel Laszczak wrote: > > > > >Since we have both USB2 and USB3 PHYs for cdns3 controller, it is > >better we have unity APIs to handle both USB2 and USB3's power, it > >could simplify code for error handling and further power management > >implementation. > > > >Reviewed-by: Jun Li <jun.li@nxp.com> > >Signed-off-by: Peter Chen <peter.chen@nxp.com> > >--- > > drivers/usb/cdns3/core.c | 43 ++++++++++++++++++++++++++-------------- > > 1 file changed, 28 insertions(+), 15 deletions(-) > > > >diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > >index 19bbb5b7e6b6..8818935d157b 100644 > >--- a/drivers/usb/cdns3/core.c > >+++ b/drivers/usb/cdns3/core.c > >@@ -384,6 +384,27 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role) > > return ret; > > } > > > >+static int set_phy_power_on(struct cdns3 *cdns) > >+{ > >+ int ret; > >+ > >+ ret = phy_power_on(cdns->usb2_phy); > >+ if (ret) > >+ return ret; > >+ > >+ ret = phy_power_on(cdns->usb3_phy); > >+ if (ret) > >+ phy_power_off(cdns->usb2_phy); > >+ > >+ return ret; > >+} > >+ > >+static void set_phy_power_off(struct cdns3 *cdns) > >+{ > >+ phy_power_off(cdns->usb3_phy); > >+ phy_power_off(cdns->usb2_phy); > >+} > >+ > > /** > > * cdns3_probe - probe for cdns3 core device > > * @pdev: Pointer to cdns3 core platform device > >@@ -477,14 +498,10 @@ static int cdns3_probe(struct platform_device *pdev) > > if (ret) > > goto err1; > > > >- ret = phy_power_on(cdns->usb2_phy); > >+ ret = set_phy_power_on(cdns); > > if (ret) > > goto err2; > > > >- ret = phy_power_on(cdns->usb3_phy); > >- if (ret) > >- goto err3; > >- > > sw_desc.set = cdns3_role_set; > > sw_desc.get = cdns3_role_get; > > sw_desc.allow_userspace_control = true; > >@@ -496,16 +513,16 @@ static int cdns3_probe(struct platform_device *pdev) > > if (IS_ERR(cdns->role_sw)) { > > ret = PTR_ERR(cdns->role_sw); > > dev_warn(dev, "Unable to register Role Switch\n"); > >- goto err4; > >+ goto err3; > > } > > > > ret = cdns3_drd_init(cdns); > > if (ret) > >- goto err5; > >+ goto err4; > > > > ret = cdns3_core_init_role(cdns); > > if (ret) > >- goto err5; > >+ goto err4; > > > > device_set_wakeup_capable(dev, true); > > pm_runtime_set_active(dev); > >@@ -522,14 +539,11 @@ static int cdns3_probe(struct platform_device *pdev) > > dev_dbg(dev, "Cadence USB3 core: probe succeed\n"); > > > > return 0; > >-err5: > >+err4: > > cdns3_drd_exit(cdns); > > usb_role_switch_unregister(cdns->role_sw); > >-err4: > >- phy_power_off(cdns->usb3_phy); > >- > > err3: > >- phy_power_off(cdns->usb2_phy); > >+ set_phy_power_off(cdns); > > err2: > > phy_exit(cdns->usb3_phy); > > err1: > > Dan Carpenter suggested me to use more meaningful labels instead err1 .. err5. Yes, it is reasonable, we could follow this rule when design the new function. Peter > > Reviewed-by: Pawel Laszczak<pawell@cadence.com> > > Pawel > > >@@ -553,8 +567,7 @@ static int cdns3_remove(struct platform_device *pdev) > > pm_runtime_put_noidle(&pdev->dev); > > cdns3_exit_roles(cdns); > > usb_role_switch_unregister(cdns->role_sw); > >- phy_power_off(cdns->usb2_phy); > >- phy_power_off(cdns->usb3_phy); > >+ set_phy_power_off(cdns); > > phy_exit(cdns->usb2_phy); > > phy_exit(cdns->usb3_phy); > > return 0; > >-- > >2.17.1 >
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index 19bbb5b7e6b6..8818935d157b 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -384,6 +384,27 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role) return ret; } +static int set_phy_power_on(struct cdns3 *cdns) +{ + int ret; + + ret = phy_power_on(cdns->usb2_phy); + if (ret) + return ret; + + ret = phy_power_on(cdns->usb3_phy); + if (ret) + phy_power_off(cdns->usb2_phy); + + return ret; +} + +static void set_phy_power_off(struct cdns3 *cdns) +{ + phy_power_off(cdns->usb3_phy); + phy_power_off(cdns->usb2_phy); +} + /** * cdns3_probe - probe for cdns3 core device * @pdev: Pointer to cdns3 core platform device @@ -477,14 +498,10 @@ static int cdns3_probe(struct platform_device *pdev) if (ret) goto err1; - ret = phy_power_on(cdns->usb2_phy); + ret = set_phy_power_on(cdns); if (ret) goto err2; - ret = phy_power_on(cdns->usb3_phy); - if (ret) - goto err3; - sw_desc.set = cdns3_role_set; sw_desc.get = cdns3_role_get; sw_desc.allow_userspace_control = true; @@ -496,16 +513,16 @@ static int cdns3_probe(struct platform_device *pdev) if (IS_ERR(cdns->role_sw)) { ret = PTR_ERR(cdns->role_sw); dev_warn(dev, "Unable to register Role Switch\n"); - goto err4; + goto err3; } ret = cdns3_drd_init(cdns); if (ret) - goto err5; + goto err4; ret = cdns3_core_init_role(cdns); if (ret) - goto err5; + goto err4; device_set_wakeup_capable(dev, true); pm_runtime_set_active(dev); @@ -522,14 +539,11 @@ static int cdns3_probe(struct platform_device *pdev) dev_dbg(dev, "Cadence USB3 core: probe succeed\n"); return 0; -err5: +err4: cdns3_drd_exit(cdns); usb_role_switch_unregister(cdns->role_sw); -err4: - phy_power_off(cdns->usb3_phy); - err3: - phy_power_off(cdns->usb2_phy); + set_phy_power_off(cdns); err2: phy_exit(cdns->usb3_phy); err1: @@ -553,8 +567,7 @@ static int cdns3_remove(struct platform_device *pdev) pm_runtime_put_noidle(&pdev->dev); cdns3_exit_roles(cdns); usb_role_switch_unregister(cdns->role_sw); - phy_power_off(cdns->usb2_phy); - phy_power_off(cdns->usb3_phy); + set_phy_power_off(cdns); phy_exit(cdns->usb2_phy); phy_exit(cdns->usb3_phy); return 0;