From mboxrd@z Thu Jan 1 00:00:00 1970 From: balbi@ti.com (Felipe Balbi) Date: Wed, 3 Sep 2014 09:42:25 -0500 Subject: [PATCH v6 4/4] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800 In-Reply-To: References: <1409656338-20337-1-git-send-email-gautam.vivek@samsung.com> <1409656338-20337-5-git-send-email-gautam.vivek@samsung.com> <20140902143438.GB16872@saruman.home> Message-ID: <20140903144225.GD31855@saruman.home> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On Wed, Sep 03, 2014 at 12:59:27PM +0530, Vivek Gautam wrote: > > On Tue, Sep 02, 2014 at 04:42:18PM +0530, 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 > >> > >> [Also removed unnecessary extra lines in the register macro definitions] > >> > >> Signed-off-by: Vivek Gautam > >> --- > >> drivers/phy/phy-exynos5-usbdrd.c | 185 ++++++++++++++++++++++++++++++++++---- > >> 1 file changed, 170 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c > >> index 47f47fe..85742b0 100644 > >> --- a/drivers/phy/phy-exynos5-usbdrd.c > >> +++ b/drivers/phy/phy-exynos5-usbdrd.c > >> @@ -37,13 +37,11 @@ > >> > >> /* EXYNOS5: USB 3.0 DRD PHY registers */ > >> #define EXYNOS5_DRD_LINKSYSTEM 0x04 > >> - > >> #define LINKSYSTEM_FLADJ_MASK (0x3f << 1) > >> #define LINKSYSTEM_FLADJ(_x) ((_x) << 1) > >> #define LINKSYSTEM_XHCI_VERSION_CONTROL BIT(27) > >> > >> #define EXYNOS5_DRD_PHYUTMI 0x08 > >> - > >> #define PHYUTMI_OTGDISABLE BIT(6) > >> #define PHYUTMI_FORCESUSPEND BIT(1) > >> #define PHYUTMI_FORCESLEEP BIT(0) > >> @@ -51,26 +49,20 @@ > >> #define EXYNOS5_DRD_PHYPIPE 0x0c > >> > >> #define EXYNOS5_DRD_PHYCLKRST 0x10 > >> - > >> #define PHYCLKRST_EN_UTMISUSPEND BIT(31) > >> - > >> #define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23) > >> #define PHYCLKRST_SSC_REFCLKSEL(_x) ((_x) << 23) > >> - > >> #define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21) > >> #define PHYCLKRST_SSC_RANGE(_x) ((_x) << 21) > >> - > >> #define PHYCLKRST_SSC_EN BIT(20) > >> #define PHYCLKRST_REF_SSP_EN BIT(19) > >> #define PHYCLKRST_REF_CLKDIV2 BIT(18) > >> - > >> #define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f << 11) > >> #define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11) > >> #define PHYCLKRST_MPLL_MULTIPLIER_50M_REF (0x32 << 11) > >> #define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF (0x68 << 11) > >> #define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF (0x7d << 11) > >> #define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF (0x02 << 11) > >> - > >> #define PHYCLKRST_FSEL_UTMI_MASK (0x7 << 5) > >> #define PHYCLKRST_FSEL_PIPE_MASK (0x7 << 8) > >> #define PHYCLKRST_FSEL(_x) ((_x) << 5) > >> @@ -78,46 +70,68 @@ > >> #define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5) > >> #define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5) > >> #define PHYCLKRST_FSEL_PAD_19_2MHZ (0x38 << 5) > >> - > >> #define PHYCLKRST_RETENABLEN BIT(4) > >> - > >> #define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2) > >> #define PHYCLKRST_REFCLKSEL_PAD_REFCLK (0x2 << 2) > >> #define PHYCLKRST_REFCLKSEL_EXT_REFCLK (0x3 << 2) > >> - > >> #define PHYCLKRST_PORTRESET BIT(1) > >> #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_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_PHYPARAM0 0x1c > >> - > >> #define PHYPARAM0_REF_USE_PAD BIT(31) > >> #define PHYPARAM0_REF_LOSLEVEL_MASK (0x1f << 26) > >> #define PHYPARAM0_REF_LOSLEVEL (0x9 << 26) > >> > >> #define EXYNOS5_DRD_PHYPARAM1 0x20 > >> - > >> #define PHYPARAM1_PCS_TXDEEMPH_MASK (0x1f << 0) > >> #define PHYPARAM1_PCS_TXDEEMPH (0x1c) > >> > >> #define EXYNOS5_DRD_PHYTERM 0x24 > >> > >> #define EXYNOS5_DRD_PHYTEST 0x28 > >> - > >> #define PHYTEST_POWERDOWN_SSP BIT(3) > >> #define PHYTEST_POWERDOWN_HSP BIT(2) > >> > >> #define EXYNOS5_DRD_PHYADP 0x2c > >> > >> #define EXYNOS5_DRD_PHYUTMICLKSEL 0x30 > >> - > > > > this and all above are unrelated to $subject. > > True, but i have mentioned about this change in the commit message. > It looked so trivial to me that i included it here in this patch. > > Will separate it out in a separate patch if you like that. :-) probably not necessary. Kishon's got the final word for drivers/phy, though. > > > > >> #define PHYUTMICLKSEL_UTMI_CLKSEL BIT(2) > >> > >> #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) > >> +#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) > >> > >> @@ -135,12 +149,14 @@ struct exynos5_usbdrd_phy_config { > >> void (*phy_isol)(struct phy_usb_instance *inst, u32 on); > >> void (*phy_init)(struct exynos5_usbdrd_phy *phy_drd); > >> unsigned int (*set_refclk)(struct phy_usb_instance *inst); > >> + int (*phy_calibrate)(struct phy_usb_instance *inst); > >> }; > >> > >> struct exynos5_usbdrd_phy_drvdata { > >> const struct exynos5_usbdrd_phy_config *phy_cfg; > >> u32 pmu_offset_usbdrd0_phy; > >> u32 pmu_offset_usbdrd1_phy; > >> + void (*phy_exynos_calibrate)(struct exynos5_usbdrd_phy *phy_drd); > >> }; > >> > >> /** > >> @@ -487,6 +503,142 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy) > >> return 0; > >> } > >> > >> +static void crport_handshake(struct exynos5_usbdrd_phy *phy_drd, > >> + u32 val, u32 cmd) > >> +{ > >> + u32 usec = 100; > >> + u32 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); > > > > if handshake timed out, shouldn't you error out here ? IOW, make this > > function return an int and return an error code from this point ? Why > > would you even continue to the next step ? > > Point taken, we should have error'ed out here once timeout occured. > Will update it accordingly. > > > > >> + > >> + 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 handshake timeout2 (0x%08x)\n", val); > > > > return an error ? > sure > > > > >> +} > >> + > >> +static void crport_ctrl_write(struct exynos5_usbdrd_phy *phy_drd, > >> + u32 addr, u32 data) > >> +{ > >> + /* Write Address */ > >> + writel(PHYREG0_CR_DATA_IN(addr), > >> + phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0); > >> + crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(addr), > >> + PHYREG0_CR_CAP_ADDR); > >> + > >> + /* Write Data */ > >> + writel(PHYREG0_CR_DATA_IN(data), > >> + phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0); > >> + crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(data), > >> + PHYREG0_CR_CAP_DATA); > >> + crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(data), > >> + 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: > > > > why can't this be done during probe() ? Are you going to change external > > reference clock frequency during runtime ? And even if you do, > > wouldn't it be easier to subscribe to clock rate change notifiers > > instead of adding this ->calibrate() ? > > Wait, did this patch give an impression that we are changing the reference > clock settings ? not, it gives the impression that you need to change RX Detect timeout value based on external reference clock frequency :-) > We are actually trying to override some of the PHY parameters, which > is possible only after the controller has been reset. > These PHY parameter overriding (on PHY registers which are not exposed > outside) is done post initialization, only through > EXYNOS5_DRD_PHYREG0. so it _has_ to be done after dwc3's reset ? Don't we have phy_power_on() for that ? It looks like you could just as well do this from phy_power_on() ? > > It still looks, to me, that this ->calibrate() thing is unnecessary. We > > have a single user for it and this user could very well get this sorted > > out during probe() or by subscribing to clock rate notifiers. > > We can't program these parameters during PHY probe(), since they reset to > their default value after the controller reset. > So we needed a way to do this post initialization, once the controller > has been reset. right, and we already have a call to phy_power_on() after core has been reset. Can't you use that ? -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: