[PATCH 2/2] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

[Andrzej Pietrasiewicz]

Hi,

W dniu 18.09.2017 o 13:27, Andrzej Pietrasiewicz pisze:
> W dniu 18.09.2017 o 13:06, Kishon Vijay Abraham I pisze:
>> Hi,
>>
>> On Monday 18 September 2017 04:08 PM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Andrzej Pietrasiewicz <andrzej.p at samsung.com> writes:
>>>> From: Vivek Gautam <gautam.vivek at samsung.com>
>>>>
>>>> Adding phy calibration sequence for USB 3.0 DRD PHY present on
>>>> Exynos5420/5800 systems.
> 
> <snip>
> 
>>>>    
>>>> +static int exynos5_usbdrd_phy_reset(struct phy *phy)
>>>> +{
>>>> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
>>>> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
>>>> +
>>>> +	return exynos5420_usbdrd_phy_calibrate(phy_drd);
>>>> +}
>>>> +
>>>>    static const struct phy_ops exynos5_usbdrd_phy_ops = {
>>>>    	.init		= exynos5_usbdrd_phy_init,
>>>>    	.exit		= exynos5_usbdrd_phy_exit,
>>>>    	.power_on	= exynos5_usbdrd_phy_power_on,
>>>>    	.power_off	= exynos5_usbdrd_phy_power_off,
>>>> +	.reset		= exynos5_usbdrd_phy_reset,
>>>>    	.owner		= THIS_MODULE,
>>>>    };
>>>>    
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 03474d3..1d5836e 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -156,9 +156,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>>>>    		} else {
>>>>    			if (dwc->usb2_phy)
>>>>    				otg_set_vbus(dwc->usb2_phy->otg, true);
>>>> -			if (dwc->usb2_generic_phy)
>>>> +			if (dwc->usb2_generic_phy) {
>>>>    				phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>> -
>>>> +				phy_reset(dwc->usb2_generic_phy);
>>>
>>> it doesn't look like this is the best place to reset the phy. Also,
>>
>> right, phy_reset is done during initialization before phy_power_on/phy_init or
>> in error cases.
>>
>>> ->reset() doesn't seem to match correctly with a calibration. That seems
>>> to be more fitting to a ->power_on() or ->init() implementation.
>>
>> yeah, the initial patch seems to calibrate in phy_init(). Not sure why it's
>> modified.
> 
> The original patch used a hack like below, in xhci_plat_probe():
> 
> +       /* Initialize and power-on USB 3.0 PHY */
> +       xhci->shared_hcd->phy->init_count = 0;
> +       ret = phy_init(xhci->shared_hcd->phy);
> +       if (ret)
> +               goto dealloc_usb3_hcd;
> +
> +       xhci->shared_hcd->phy->power_count = 0;
> +       ret = phy_power_on(xhci->shared_hcd->phy);
> +       if (ret) {
> +               phy_exit(xhci->shared_hcd->phy);
> +               goto dealloc_usb3_hcd;
> +       }
> +
> 
> Manually setting init_count to 0 in order for the subsequent phy_init() to
> happen probably does not look good.
> 
> The calibration is clearly needed. However, I don't have any strong opinions
> on from which place exactly to trigger the calibration process.
> The original patch did not make it upstream, but if that patch is ok,
> it is perfectly fine with me to drop my version and take that one instead.

Me bad, I did not write about an important issue.
The calibration must happen after usb_add_hcd(), otherwise
usb_add_hcd() indirectly triggers overwriting the effects of calibration.

Andrzej