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

2017-09-21 Thread Kishon Vijay Abraham I
Hi,

On Monday 18 September 2017 07:50 PM, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 18.09.2017 o 14:43, Felipe Balbi pisze:
>>
>> Hi,
>>
>> Andrzej Pietrasiewicz  writes:
>>> +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.
>>
>> in that case, you should do that from xhci-plat indeed. I think the
>> whole idea with init_count is just to make sure you don't initialize it
>> twice.
> 
> As far as I understand the code in question the desired result is exactly the
> opposite:
> to make sure it _does_ initialize twice, otherwise after the first
> initialization the
> calibration results were lost. In other words, in the code snippet above,
> in xhci_plat_probe() the phy_init() was creatively (ab)used in order to force
> the calibration at a desired moment, while in the original invocation of
> phy_init()
> the calibration result was merely a short-term side effect discarded soon
> afterwards.
> 
>>
>> One thing's for sure, ->reset() doesn't seem to be the matching callback
>> for you to use and, given your explanation above, dwc3 doesn't seem to
>> be the right place to fiddle with that.
>>
>> Seems like we need an extension of the generic PHY framework to cope
>> with your requirement.
>>
> 
> Here are old patches from Vivek:
> 
> https://lkml.org/lkml/2014/9/2/166
> 
> In particular:
> 
> https://lkml.org/lkml/2014/9/2/170
> 
> Please see the discussion that follows the latter.
> 
> All in all, is adding the calibrate() method to phy_ops the way to go or not?

Adding calibrate is fine but doing init() and power_on() in one driver and
calibrate() in another doesn't look correct. Why not let xhci do init() and
power_on() of phy instead of dwc3?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-09-18 Thread Andrzej Pietrasiewicz

Hi,

W dniu 18.09.2017 o 14:43, Felipe Balbi pisze:


Hi,

Andrzej Pietrasiewicz  writes:

+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.


in that case, you should do that from xhci-plat indeed. I think the
whole idea with init_count is just to make sure you don't initialize it
twice.


As far as I understand the code in question the desired result is exactly the 
opposite:
to make sure it _does_ initialize twice, otherwise after the first 
initialization the
calibration results were lost. In other words, in the code snippet above,
in xhci_plat_probe() the phy_init() was creatively (ab)used in order to force
the calibration at a desired moment, while in the original invocation of 
phy_init()
the calibration result was merely a short-term side effect discarded soon 
afterwards.



One thing's for sure, ->reset() doesn't seem to be the matching callback
for you to use and, given your explanation above, dwc3 doesn't seem to
be the right place to fiddle with that.

Seems like we need an extension of the generic PHY framework to cope
with your requirement.



Here are old patches from Vivek:

https://lkml.org/lkml/2014/9/2/166

In particular:

https://lkml.org/lkml/2014/9/2/170

Please see the discussion that follows the latter.

All in all, is adding the calibrate() method to phy_ops the way to go or not?

Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-09-18 Thread Felipe Balbi

Hi,

Andrzej Pietrasiewicz  writes:
> +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.

in that case, you should do that from xhci-plat indeed. I think the
whole idea with init_count is just to make sure you don't initialize it
twice.

One thing's for sure, ->reset() doesn't seem to be the matching callback
for you to use and, given your explanation above, dwc3 doesn't seem to
be the right place to fiddle with that.

Seems like we need an extension of the generic PHY framework to cope
with your requirement.

-- 
balbi


signature.asc
Description: PGP signature


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

2017-09-18 Thread 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  writes:

From: Vivek Gautam 

Adding phy calibration sequence for USB 3.0 DRD PHY present on
Exynos5420/5800 systems.




   
+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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-09-18 Thread Andrzej Pietrasiewicz

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  writes:

From: Vivek Gautam 

Adding phy calibration sequence for USB 3.0 DRD PHY present on
Exynos5420/5800 systems.




  
+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.

Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2017-09-18 Thread Kishon Vijay Abraham I
Hi,

On Monday 18 September 2017 04:08 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> Andrzej Pietrasiewicz  writes:
>> From: Vivek Gautam 
>>
>> Adding phy calibration sequence for USB 3.0 DRD PHY present on
>> Exynos5420/5800 systems.
>> This calibration facilitates setting certain PHY parameters viz.
>> the Loss-of-Signal (LOS) Detector Threshold Level, as well as
>> Tx-Vboost-Level for Super-Speed operations.
>> Additionally we also set proper time to wait for RxDetect measurement,
>> for desired PHY reference clock, so as to solve issue with enumeration
>> of few USB 3.0 devices, like Samsung SUM-TSB16S 3.0 USB drive
>> on the controller.
>>
>> We are using CR_port for this purpose to send required data
>> to override the LOS values.
>>
>> On testing with USB 3.0 devices on USB 3.0 port present on
>> SMDK5420, and peach-pit boards should see following message:
>> usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
>>
>> and without this patch, should see below shown message:
>> usb 1-1: new high-speed USB device number 2 using xhci-hcd
>>
>> [Also removed unnecessary extra lines in the register macro definitions]
>>
>> Signed-off-by: Vivek Gautam 
>> [adapted to use phy_reset as entry point]
>> Signed-off-by: Andrzej Pietrasiewicz 
>> ---
>>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 
>> +++
>>  drivers/usb/dwc3/core.c  |   8 +-
>>  2 files changed, 189 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
>> b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> index 7c41daa..f7de067 100644
>> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
>> @@ -89,7 +89,17 @@
>>  #define PHYCLKRST_COMMONONN BIT(0)
>>  
>>  #define EXYNOS5_DRD_PHYREG0 0x14
>> +#define PHYREG0_SSC_REF_CLK_SEL BIT(21)
>> +#define PHYREG0_SSC_RANGE   BIT(20)
>> +#define PHYREG0_CR_WRITEBIT(19)
>> +#define PHYREG0_CR_READ BIT(18)
>> +#define PHYREG0_CR_DATA_IN(_x)  ((_x) << 2)
>> +#define PHYREG0_CR_CAP_DATA BIT(1)
>> +#define PHYREG0_CR_CAP_ADDR BIT(0)
>> +
>>  #define EXYNOS5_DRD_PHYREG1 0x18
>> +#define PHYREG1_CR_DATA_OUT(_x) ((_x) << 1)
>> +#define PHYREG1_CR_ACK  BIT(0)
>>  
>>  #define EXYNOS5_DRD_PHYPARAM0   0x1c
>>  
>> @@ -118,6 +128,25 @@
>>  #define EXYNOS5_DRD_PHYRESUME   0x34
>>  #define EXYNOS5_DRD_LINKPORT0x44
>>  
>> +/* USB 3.0 DRD PHY SS Function Control Reg; accessed by CR_PORT */
>> +#define EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN  (0x15)
>> +#define LOSLEVEL_OVRD_IN_LOS_BIAS_5420  (0x5 << 13)
>> +#define LOSLEVEL_OVRD_IN_LOS_BIAS_DEFAULT   (0x0 << 13)
>> +#define LOSLEVEL_OVRD_IN_EN (0x1 << 10)
>> +#define LOSLEVEL_OVRD_IN_LOS_LEVEL_DEFAULT  (0x9 << 0)
>> +
>> +#define EXYNOS5_DRD_PHYSS_TX_VBOOSTLEVEL_OVRD_IN(0x12)
>> +#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_5420  (0x5 << 13)
>> +#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_DEFAULT   (0x4 << 13)
>> +
>> +#define EXYNOS5_DRD_PHYSS_LANE0_TX_DEBUG(0x1010)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_19M2_20M (0x4 << 4)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_24M  (0x8 << 4)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_25M_26M  (0x8 << 4)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_48M_50M_52M  (0x20 << 4)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_62M5 (0x20 << 4)
>> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_96M_100M (0x40 << 4)
>> +
>>  #define KHZ 1000
>>  #define MHZ (KHZ * KHZ)
>>  
>> @@ -526,6 +555,151 @@ static int exynos5_usbdrd_phy_power_off(struct phy 
>> *phy)
>>  return 0;
>>  }
>>  
>> +static int crport_handshake(struct exynos5_usbdrd_phy *phy_drd,
>> +u32 val, u32 cmd)
>> +{
>> +u32 usec = 100;
>> +unsigned int result;
>> +
>> +writel(val | cmd, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
>> +
>> +do {
>> +result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
>> +if (result & PHYREG1_CR_ACK)
>> +break;
>> +
>> +udelay(1);
>> +} while (usec-- > 0);
>> +
>> +if (!usec) {
>> +dev_err(phy_drd->dev,
>> +"CRPORT handshake timeout1 (0x%08x)\n", val);
>> +return -ETIME;
>> +}
>> +
>> +usec = 100;
>> +
>> +writel(val, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
>> +
>> +do {
>> +result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
>> +if (!(result & PHYREG1_CR_ACK))
>> +break;
>> +
>> +

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

2017-09-18 Thread Felipe Balbi

Hi,

Andrzej Pietrasiewicz  writes:
> From: Vivek Gautam 
>
> Adding phy calibration sequence for USB 3.0 DRD PHY present on
> Exynos5420/5800 systems.
> This calibration facilitates setting certain PHY parameters viz.
> the Loss-of-Signal (LOS) Detector Threshold Level, as well as
> Tx-Vboost-Level for Super-Speed operations.
> Additionally we also set proper time to wait for RxDetect measurement,
> for desired PHY reference clock, so as to solve issue with enumeration
> of few USB 3.0 devices, like Samsung SUM-TSB16S 3.0 USB drive
> on the controller.
>
> We are using CR_port for this purpose to send required data
> to override the LOS values.
>
> On testing with USB 3.0 devices on USB 3.0 port present on
> SMDK5420, and peach-pit boards should see following message:
> usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
>
> and without this patch, should see below shown message:
> usb 1-1: new high-speed USB device number 2 using xhci-hcd
>
> [Also removed unnecessary extra lines in the register macro definitions]
>
> Signed-off-by: Vivek Gautam 
> [adapted to use phy_reset as entry point]
> Signed-off-by: Andrzej Pietrasiewicz 
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 
> +++
>  drivers/usb/dwc3/core.c  |   8 +-
>  2 files changed, 189 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
> b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 7c41daa..f7de067 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -89,7 +89,17 @@
>  #define PHYCLKRST_COMMONONN  BIT(0)
>  
>  #define EXYNOS5_DRD_PHYREG0  0x14
> +#define PHYREG0_SSC_REF_CLK_SEL  BIT(21)
> +#define PHYREG0_SSC_RANGEBIT(20)
> +#define PHYREG0_CR_WRITE BIT(19)
> +#define PHYREG0_CR_READ  BIT(18)
> +#define PHYREG0_CR_DATA_IN(_x)   ((_x) << 2)
> +#define PHYREG0_CR_CAP_DATA  BIT(1)
> +#define PHYREG0_CR_CAP_ADDR  BIT(0)
> +
>  #define EXYNOS5_DRD_PHYREG1  0x18
> +#define PHYREG1_CR_DATA_OUT(_x)  ((_x) << 1)
> +#define PHYREG1_CR_ACK   BIT(0)
>  
>  #define EXYNOS5_DRD_PHYPARAM00x1c
>  
> @@ -118,6 +128,25 @@
>  #define EXYNOS5_DRD_PHYRESUME0x34
>  #define EXYNOS5_DRD_LINKPORT 0x44
>  
> +/* USB 3.0 DRD PHY SS Function Control Reg; accessed by CR_PORT */
> +#define EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN   (0x15)
> +#define LOSLEVEL_OVRD_IN_LOS_BIAS_5420   (0x5 << 13)
> +#define LOSLEVEL_OVRD_IN_LOS_BIAS_DEFAULT(0x0 << 13)
> +#define LOSLEVEL_OVRD_IN_EN  (0x1 << 10)
> +#define LOSLEVEL_OVRD_IN_LOS_LEVEL_DEFAULT   (0x9 << 0)
> +
> +#define EXYNOS5_DRD_PHYSS_TX_VBOOSTLEVEL_OVRD_IN (0x12)
> +#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_5420   (0x5 << 13)
> +#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_DEFAULT(0x4 << 13)
> +
> +#define EXYNOS5_DRD_PHYSS_LANE0_TX_DEBUG (0x1010)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_19M2_20M  (0x4 << 4)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_24M   (0x8 << 4)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_25M_26M   (0x8 << 4)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_48M_50M_52M   (0x20 << 4)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_62M5  (0x20 << 4)
> +#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_96M_100M  (0x40 << 4)
> +
>  #define KHZ  1000
>  #define MHZ  (KHZ * KHZ)
>  
> @@ -526,6 +555,151 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
>   return 0;
>  }
>  
> +static int crport_handshake(struct exynos5_usbdrd_phy *phy_drd,
> + u32 val, u32 cmd)
> +{
> + u32 usec = 100;
> + unsigned int result;
> +
> + writel(val | cmd, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
> +
> + do {
> + result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
> + if (result & PHYREG1_CR_ACK)
> + break;
> +
> + udelay(1);
> + } while (usec-- > 0);
> +
> + if (!usec) {
> + dev_err(phy_drd->dev,
> + "CRPORT handshake timeout1 (0x%08x)\n", val);
> + return -ETIME;
> + }
> +
> + usec = 100;
> +
> + writel(val, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
> +
> + do {
> + result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
> + if (!(result & PHYREG1_CR_ACK))
> + break;
> +
> + udelay(1);
> + } while (usec-- > 0);
> +
> + if (!usec) {
> + dev_err(phy_drd->dev,
> + "CRPORT