[v2,2/2,RESEND] usb: dwc3: remove generic PHY calibrate() calls

Message ID 20190808094128.27213-3-m.szyprowski@samsung.com
State New
Headers show
Series
  • Fix USB3.0 DRD PHY calibration issues (DWC3/XHCI) on Exynos542x SoCs
Related show

Commit Message

Marek Szyprowski Aug. 8, 2019, 9:41 a.m.
Calls to USB2 generic PHY calibrate() method has been moved to HCD core,
which now successfully handles generic PHYs and their calibration after
every HCD reset. This fixes all the timing issues related to PHY
calibration done directly from DWC3 driver: incorrect operation after
system suspend/resume or USB3.0 detection failure when XHCI-plat driver
compiled as separate module.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Anand Moon <linux.amoon@gmail.com>

Tested-by: Jochen Sprickerhof <jochen@sprickerhof.de>

---
 drivers/usb/dwc3/core.c | 2 --
 1 file changed, 2 deletions(-)

-- 
2.17.1

Comments

Felipe Balbi Aug. 8, 2019, 9:51 a.m. | #1
Hi,

Marek Szyprowski <m.szyprowski@samsung.com> writes:

> Calls to USB2 generic PHY calibrate() method has been moved to HCD core,

> which now successfully handles generic PHYs and their calibration after

> every HCD reset. This fixes all the timing issues related to PHY

> calibration done directly from DWC3 driver: incorrect operation after

> system suspend/resume or USB3.0 detection failure when XHCI-plat driver

> compiled as separate module.

>

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Tested-by: Anand Moon <linux.amoon@gmail.com>

> Tested-by: Jochen Sprickerhof <jochen@sprickerhof.de>

> ---

>  drivers/usb/dwc3/core.c | 2 --

>  1 file changed, 2 deletions(-)

>

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c

> index c9bb93a2c81e..7dd6d419254d 100644

> --- a/drivers/usb/dwc3/core.c

> +++ b/drivers/usb/dwc3/core.c

> @@ -168,7 +168,6 @@ static void __dwc3_set_mode(struct work_struct *work)

>  				otg_set_vbus(dwc->usb2_phy->otg, true);

>  			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);

>  			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);

> -			phy_calibrate(dwc->usb2_generic_phy);


are you sure you're the only one using phy_calibrate()? I don't want any
regressions because of this :-p

-- 
balbi
Marek Szyprowski Aug. 8, 2019, 10:38 a.m. | #2
Hi Felipe,

On 2019-08-08 11:51, Felipe Balbi wrote:
> Marek Szyprowski <m.szyprowski@samsung.com> writes:

>> Calls to USB2 generic PHY calibrate() method has been moved to HCD core,

>> which now successfully handles generic PHYs and their calibration after

>> every HCD reset. This fixes all the timing issues related to PHY

>> calibration done directly from DWC3 driver: incorrect operation after

>> system suspend/resume or USB3.0 detection failure when XHCI-plat driver

>> compiled as separate module.

>>

>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

>> Tested-by: Anand Moon <linux.amoon@gmail.com>

>> Tested-by: Jochen Sprickerhof <jochen@sprickerhof.de>

>> ---

>>   drivers/usb/dwc3/core.c | 2 --

>>   1 file changed, 2 deletions(-)

>>

>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c

>> index c9bb93a2c81e..7dd6d419254d 100644

>> --- a/drivers/usb/dwc3/core.c

>> +++ b/drivers/usb/dwc3/core.c

>> @@ -168,7 +168,6 @@ static void __dwc3_set_mode(struct work_struct *work)

>>   				otg_set_vbus(dwc->usb2_phy->otg, true);

>>   			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);

>>   			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);

>> -			phy_calibrate(dwc->usb2_generic_phy);

> are you sure you're the only one using phy_calibrate()? I don't want any

> regressions because of this :-p


Yes I've checked. In case of USB PHYs, the .calibrate method is only 
implemented by Exynos DRDUSB3 PHY driver:

# git grep \\\.calibrate drivers/phy
drivers/phy/broadcom/phy-brcm-sata.c:   .calibrate      = 
brcm_sata_phy_calibrate,
drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c: .calibrate              = 
ufs_qcom_phy_qmp_14nm_phy_calibrate,
drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c: .calibrate              = 
ufs_qcom_phy_qmp_20nm_phy_calibrate,
drivers/phy/samsung/phy-exynos5-usbdrd.c:       .calibrate      = 
exynos5_usbdrd_phy_calibrate,

(the other PHY drivers are for SATA or UFS).

To avoid the regression on Exynos it is enough to apply the 2 patches 
from this patchset together.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Felipe Balbi Aug. 8, 2019, 12:37 p.m. | #3
Hi,

Marek Szyprowski <m.szyprowski@samsung.com> writes:
> On 2019-08-08 11:51, Felipe Balbi wrote:

>> Marek Szyprowski <m.szyprowski@samsung.com> writes:

>>> Calls to USB2 generic PHY calibrate() method has been moved to HCD core,

>>> which now successfully handles generic PHYs and their calibration after

>>> every HCD reset. This fixes all the timing issues related to PHY

>>> calibration done directly from DWC3 driver: incorrect operation after

>>> system suspend/resume or USB3.0 detection failure when XHCI-plat driver

>>> compiled as separate module.

>>>

>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

>>> Tested-by: Anand Moon <linux.amoon@gmail.com>

>>> Tested-by: Jochen Sprickerhof <jochen@sprickerhof.de>

>>> ---

>>>   drivers/usb/dwc3/core.c | 2 --

>>>   1 file changed, 2 deletions(-)

>>>

>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c

>>> index c9bb93a2c81e..7dd6d419254d 100644

>>> --- a/drivers/usb/dwc3/core.c

>>> +++ b/drivers/usb/dwc3/core.c

>>> @@ -168,7 +168,6 @@ static void __dwc3_set_mode(struct work_struct *work)

>>>   				otg_set_vbus(dwc->usb2_phy->otg, true);

>>>   			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);

>>>   			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);

>>> -			phy_calibrate(dwc->usb2_generic_phy);

>> are you sure you're the only one using phy_calibrate()? I don't want any

>> regressions because of this :-p

>

> Yes I've checked. In case of USB PHYs, the .calibrate method is only 

> implemented by Exynos DRDUSB3 PHY driver:

>

> # git grep \\\.calibrate drivers/phy

> drivers/phy/broadcom/phy-brcm-sata.c:   .calibrate      = 

> brcm_sata_phy_calibrate,

> drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c: .calibrate              = 

> ufs_qcom_phy_qmp_14nm_phy_calibrate,

> drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c: .calibrate              = 

> ufs_qcom_phy_qmp_20nm_phy_calibrate,

> drivers/phy/samsung/phy-exynos5-usbdrd.c:       .calibrate      = 

> exynos5_usbdrd_phy_calibrate,

>

> (the other PHY drivers are for SATA or UFS).

>

> To avoid the regression on Exynos it is enough to apply the 2 patches 

> from this patchset together.


We should, certainly, apply them together. But my concern is that
someone has been depending on this. If you've checked all other PHY
drivers, then I'm okay with removing the call.

As $subject should be applied together with the previous patch, here's
my Ack:

Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>


@Greg, if you prefer that I queue both patches and send you in a pull
request, let me know. Works either way for me. I can just rebase my
testing/next on top of your tree once you apply these patches.

cheers

-- 
balbi

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c9bb93a2c81e..7dd6d419254d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -168,7 +168,6 @@  static void __dwc3_set_mode(struct work_struct *work)
 				otg_set_vbus(dwc->usb2_phy->otg, true);
 			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
 			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
-			phy_calibrate(dwc->usb2_generic_phy);
 		}
 		break;
 	case DWC3_GCTL_PRTCAP_DEVICE:
@@ -1166,7 +1165,6 @@  static int dwc3_core_init_mode(struct dwc3 *dwc)
 				dev_err(dev, "failed to initialize host\n");
 			return ret;
 		}
-		phy_calibrate(dwc->usb2_generic_phy);
 		break;
 	case USB_DR_MODE_OTG:
 		INIT_WORK(&dwc->drd_work, __dwc3_set_mode);