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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jingoo,


On Wed, Aug 27, 2014 at 7:28 AM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
> On Thursday, August 21, 2014 11:55 PM, Vivek Gautam wrote:
>>
>> Adding phy calibrate callback, which facilitates setting certain
>> PHY settings post initialization of the PHY controller.
>> Exynos5420 and Exynos5800 have 28nm USB 3.0 DRD PHY for which
>> the Loss-of-Signal (LOS) Detector Threshold Level as well as
>> Tx-Vboost-Level should be controlled for Super-Speed operations.
>>
>> Additionally 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
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@xxxxxxxxxxx>
>> ---
>>  drivers/phy/phy-exynos5-usbdrd.c |  169 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 169 insertions(+)
>>
>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
>> index 47f47fe..fa13784 100644
>> --- a/drivers/phy/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/phy-exynos5-usbdrd.c
>> @@ -89,8 +89,20 @@
>>  #define PHYCLKRST_COMMONONN                  BIT(0)
>>
>>  #define EXYNOS5_DRD_PHYREG0                  0x14
>> +
>> +#define EXYNOS5_DRD_PHYREG0_SSC_REF_CLK_SEL  BIT(21)
>> +#define EXYNOS5_DRD_PHYREG0_SSC_RANGE                BIT(20)
>> +#define EXYNOS5_DRD_PHYREG0_CR_WRITE         BIT(19)
>> +#define EXYNOS5_DRD_PHYREG0_CR_READ          BIT(18)
>> +#define EXYNOS5_DRD_PHYREG0_CR_DATA_IN(_x)   ((_x) << 2)
>> +#define EXYNOS5_DRD_PHYREG0_CR_CAP_DATA              BIT(1)
>> +#define EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR              BIT(0)
>> +
>>  #define EXYNOS5_DRD_PHYREG1                  0x18
>>
>> +#define EXYNOS5_DRD_PHYREG1_CR_DATA_OUT(_x)  ((_x) << 1)
>> +#define EXYNOS5_DRD_PHYREG1_CR_ACK           BIT(0)
>> +
>>  #define EXYNOS5_DRD_PHYPARAM0                        0x1c
>>
>>  #define PHYPARAM0_REF_USE_PAD                        BIT(31)
>> @@ -118,6 +130,26 @@
>>  #define EXYNOS5_DRD_PHYRESUME                        0x34
>>  #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)
>> +
>
> Please remove unnecessary line.

Sure will remove extra lines.

[snip]

>> +static void crport_ctrl_write(struct exynos5_usbdrd_phy *phy_drd,
>> +                                             u32 addr, u32 data)
>> +{
>> +     /* Write Address */
>> +     crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
>> +                      EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR);
>
> According to the guidance from H/W team, before calling crport_handshake(),
> write access for EXYNOS5_DRD_PHYREG0 register is necessary.
>
> Please, add the write access as follows.
>
> +       /* Write Address */
> +       writel(EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
> +               phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
> +       crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
> +                        EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR);

Sure will add this, thanks for getting the information from H/W team
and suggesting.

>
> Best regards,
> Jingoo Han
>
>> +
>> +     /* Write Data */
>> +     crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(data),
>> +                      EXYNOS5_DRD_PHYREG0_CR_CAP_DATA);
>> +     crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(data),
>> +                      EXYNOS5_DRD_PHYREG0_CR_WRITE);
>> +}
>> +
>> +/*
>> + * Override PHY paramaeters using CR_PORT register to calibrate settings
>> + * to meet meet SuperSpeed requirements, on Exynos5420 and Exynos5800 systems,
>> + * which have 28nm USB 3.0 DRD PHY.
>> + */
>> +static void exynos5420_usbdrd_phy_calibrate(struct exynos5_usbdrd_phy *phy_drd)
>> +{
>> +     u32 temp;
>> +
>> +     /*
>> +      * Change los_bias to (0x5) for 28nm PHY from a
>> +      * default value (0x0); los_level is set as default
>> +      * (0x9) as also reflected in los_level[30:26] bits
>> +      * of PHYPARAM0 register.
>> +      */
>> +     temp = LOSLEVEL_OVRD_IN_LOS_BIAS_5420 |
>> +             LOSLEVEL_OVRD_IN_EN |
>> +             LOSLEVEL_OVRD_IN_LOS_LEVEL_DEFAULT;
>> +     crport_ctrl_write(phy_drd,
>> +                       EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN,
>> +                       temp);
>> +
>> +     /*
>> +      * Set tx_vboost_lvl to (0x5) for 28nm PHY Tuning,
>> +      * to raise Tx signal level from its default value of (0x4)
>> +      */
>> +     temp = TX_VBOOSTLEVEL_OVRD_IN_VBOOST_5420;
>> +     crport_ctrl_write(phy_drd,
>> +                       EXYNOS5_DRD_PHYSS_TX_VBOOSTLEVEL_OVRD_IN,
>> +                       temp);
>> +
>> +     /*
>> +      * Set proper time to wait for RxDetect measurement, for
>> +      * desired reference clock of PHY, by tuning the CRPORT
>> +      * register LANE0.TX_DEBUG which is internal to PHY.
>> +      * This fixes issue with few USB 3.0 devices, which are
>> +      * not detected (not even generate interrupts on the bus
>> +      * on insertion) without this change.
>> +      * e.g. Samsung SUM-TSB16S 3.0 USB drive.
>> +      */
>> +     switch (phy_drd->extrefclk) {
>> +     case EXYNOS5_FSEL_50MHZ:
>> +             temp = LANE0_TX_DEBUG_RXDET_MEAS_TIME_48M_50M_52M;
>> +             break;
>> +     case EXYNOS5_FSEL_20MHZ:
>> +     case EXYNOS5_FSEL_19MHZ2:
>> +             temp = LANE0_TX_DEBUG_RXDET_MEAS_TIME_19M2_20M;
>> +             break;
>> +     case EXYNOS5_FSEL_24MHZ:
>> +     default:
>> +             temp = LANE0_TX_DEBUG_RXDET_MEAS_TIME_24M;
>> +             break;
>> +     }
>> +
>> +     crport_ctrl_write(phy_drd,
>> +                       EXYNOS5_DRD_PHYSS_LANE0_TX_DEBUG,
>> +                       temp);
>> +}
>> +
>> +/* Calibrate PIPE3 PHY settings, if any */
>> +static int exynos5_usbdrd_pipe3_calibrate(struct phy_usb_instance *inst)
>> +{
>> +     struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
>> +
>> +     /* Call respective phy_calibrate given by certain platform */
>> +     if (phy_drd->drv_data->calibrate)
>> +             phy_drd->drv_data->calibrate(phy_drd);
>> +
>> +     return 0;
>> +}
>> +
>> +static int exynos5_usbdrd_phy_calibrate(struct phy *phy)
>> +{
>> +     struct phy_usb_instance *inst = phy_get_drvdata(phy);
>> +
>> +     if (inst->phy_cfg->phy_calibrate)
>> +             inst->phy_cfg->phy_calibrate(inst);
>> +
>> +     return 0;
>> +}
>> +
>>  static struct phy *exynos5_usbdrd_phy_xlate(struct device *dev,
>>                                       struct of_phandle_args *args)
>>  {
>> @@ -503,6 +669,7 @@ static struct phy_ops exynos5_usbdrd_phy_ops = {
>>       .exit           = exynos5_usbdrd_phy_exit,
>>       .power_on       = exynos5_usbdrd_phy_power_on,
>>       .power_off      = exynos5_usbdrd_phy_power_off,
>> +     .calibrate      = exynos5_usbdrd_phy_calibrate,
>>       .owner          = THIS_MODULE,
>>  };
>>
>> @@ -518,6 +685,7 @@ static const struct exynos5_usbdrd_phy_config phy_cfg_exynos5[] = {
>>               .phy_isol       = exynos5_usbdrd_phy_isol,
>>               .phy_init       = exynos5_usbdrd_pipe3_init,
>>               .set_refclk     = exynos5_usbdrd_pipe3_set_refclk,
>> +             .phy_calibrate  = exynos5_usbdrd_pipe3_calibrate,
>>       },
>>  };
>>
>> @@ -525,6 +693,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = {
>>       .phy_cfg                = phy_cfg_exynos5,
>>       .pmu_offset_usbdrd0_phy = EXYNOS5_USBDRD_PHY_CONTROL,
>>       .pmu_offset_usbdrd1_phy = EXYNOS5420_USBDRD1_PHY_CONTROL,
>> +     .calibrate              = exynos5420_usbdrd_phy_calibrate,
>
> Hmm, how about adding just flag for this, instead of function callback?
> It looks a little bit confusing.

Yeah, i understand the function pointer name looks a bit confusing
since we are using same name in struct phy_ops too.
But i think keeping a function here for each platform may help in
adding any such support in future SoCs too.

May be we can rename it to phy_exynos_calibrate, if you think
that's fine.
Please let me know your opinion about this.




-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux