2014-10-31 13:26:43

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v7 0/2] Fine tune USB 3.0 PHY on exynos5420

This series is tested with V3 of Heikki's patches for simpliefied phy lookup table:
[PATCHv3 0/6] phy: simplified phy lookup [1] on 'usb-next' branch.
V4 of this series is giving some issue, which i have already pointed out in the
patch: [PATCHv4 2/6] phy: improved lookup method

Changes since v6:
- Dropped the changes for adding additional phy_calibrate() callback.
- Added phy_init() and phy_power_on() sequence in xhci-plat driver;
NOTE: both phy_init() and phy_power_on() will now require PHY's
'init_count' and 'power_count' to be reset to '0' so that
we can actually re-initialize the phy. Though this has already been
pointed out in discussion for the previous patch-series. [2]
- Refactored return codes and error handling in cr_port functions as pointed
out by Felipe.

Changes since v5:
- Assigned NULL to hcd->gen_phy in error path in xhci-plat.c, so that
we don't need to check for IS_ERR() while calibrating the PHYs in
core/hcd.c
- Removed extra empty lines in register definitions in exynos5-usbdrd
phy driver.
- Added write access for EXYNOS5_DRD_PHYREG0 register before any
crport_handshake() call as suggested by Jingoo Han.
- Renamed member 'calibrate' to 'phy_exynos_calibrate' of
struct exynos5_usbdrd_phy_drvdata.

Changes since v4:
- Rebased on latest patches by Heikki.
- Took care of handling -EPROBE_DEFER error number while getting PHY in
xhci plat.

Changes from v3:
- Modified error message as per review comments from Julius.

Changes since v2:
- Removed any check for DWC3 in xhci-plat for getting usb2-phy and usb3-phy,
in order to make it more generic.
- Moved the phy_calibration calls to core/hcd.c to enable a more generic
solution for issues of calibrating the PHYs.

Changes since v1:
- Using 'gen_phy' member of 'hcd' instead of declaring more variables
to hold phys.
- Added a check for compatible match for 'Synopsys-dwc3' controller,
since the 'gen_phy' member of 'hcd' already gets the 'usb' PHY
in core/hcd.c; but XHCI on Synopsys-dwc3 doesn't need that,
instead two separate PHYs for UTMI+ and PIPE3 for the two HCDs
(main hcd and shared hcd).
- Restructured the code in 'xhci_plat_setup()' and 'xhci_plat_resume()'
to use hcd->gen_phy directly. Also added the check for Synopsys's DWC3
controller while trying to calibrate the PHY.

Explanation for the need of this patch-series:
"The DWC3-exynos eXtensible host controller present on Exynos5420/5800
SoCs is quirky. The PHY serving this controller operates at High-Speed
by default, so it detects even Super-speed devices as high-speed ones.
Certain PHY parameters like Tx LOS levels and Boost levels need to be
calibrated further post initialization of xHCI controller, to get
SuperSpeed operations working."

[1] https://www.mail-archive.com/[email protected]/msg710094.html
[2] https://lkml.org/lkml/2014/9/2/170; (to be specific https://lkml.org/lkml/2014/9/10/132)

Vivek Gautam (2):
usb: host: xhci-plat: Get PHYs for xhci's hcds
phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

drivers/phy/phy-exynos5-usbdrd.c | 219 +++++++++++++++++++++++++++++++++++---
drivers/usb/host/xhci-plat.c | 74 +++++++++++++
2 files changed, 277 insertions(+), 16 deletions(-)

--
1.7.10.4


2014-10-31 13:26:50

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

The host controller by itself may sometimes need to handle PHY
and re-initialize it to re-configure some of the PHY parameters
to get full support out of the PHY controller.
Therefore, facilitate getting the two possible PHYs, viz.
USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.

Signed-off-by: Vivek Gautam <[email protected]>
---
drivers/usb/host/xhci-plat.c | 74 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..5207d5b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
#include <linux/slab.h>
#include <linux/usb/xhci_pdriver.h>

@@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto put_hcd;
}

+ /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
+ hcd->phy = devm_phy_get(&pdev->dev, "usb2-phy");
+ if (IS_ERR(hcd->phy)) {
+ ret = PTR_ERR(hcd->phy);
+ if (ret == -EPROBE_DEFER) {
+ goto disable_clk;
+ } else if (ret != -ENOSYS && ret != -ENODEV) {
+ hcd->phy = NULL;
+ dev_warn(&pdev->dev,
+ "Error retrieving usb2 phy: %d\n", ret);
+ }
+ }
+
ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (ret)
goto disable_clk;

+ /*
+ * Initialize and power-on USB 2.0 PHY
+ * FIXME: Isn't this a hacky way of initializing the PHY again ?
+ * xhci's parent would have already initialized the PHY, but we
+ * wanna do it again.
+ */
+ hcd->phy->init_count = 0;
+ ret = phy_init(hcd->phy);
+ if (ret)
+ goto dealloc_usb2_hcd;
+
+ hcd->phy->power_count = 0;
+ ret = phy_power_on(hcd->phy);
+ if (ret) {
+ phy_exit(hcd->phy);
+ goto dealloc_usb2_hcd;
+ }
+
device_wakeup_enable(hcd->self.controller);

/* USB 2.0 roothub is stored in the platform_device now. */
@@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
xhci->shared_hcd->can_do_streams = 1;

+ /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
+ xhci->shared_hcd->phy = devm_phy_get(&pdev->dev, "usb3-phy");
+ if (IS_ERR(xhci->shared_hcd->phy)) {
+ ret = PTR_ERR(xhci->shared_hcd->phy);
+ if (ret == -EPROBE_DEFER) {
+ goto put_usb3_hcd;
+ } else if (ret != -ENOSYS && ret != -ENODEV) {
+ xhci->shared_hcd->phy = NULL;
+ dev_warn(&pdev->dev,
+ "Error retrieving usb3 phy: %d\n", ret);
+ }
+ }
+
ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
if (ret)
goto put_usb3_hcd;

+ /* 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;
+ }
+
return 0;

+dealloc_usb3_hcd:
+ usb_remove_hcd(xhci->shared_hcd);
+
put_usb3_hcd:
usb_put_hcd(xhci->shared_hcd);

@@ -186,9 +247,15 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;

+ phy_power_off(xhci->shared_hcd->phy);
+ phy_exit(xhci->shared_hcd->phy);
+
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);

+ phy_power_off(hcd->phy);
+ phy_exit(hcd->phy);
+
usb_remove_hcd(hcd);
if (!IS_ERR(clk))
clk_disable_unprepare(clk);
@@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);

+ phy_exit(hcd->phy);
+
return xhci_suspend(xhci);
}

@@ -211,6 +280,11 @@ static int xhci_plat_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ int ret;
+
+ ret = phy_init(hcd->phy);
+ if (ret)
+ return ret;

return xhci_resume(xhci, 0);
}
--
1.7.10.4

2014-10-31 13:26:56

by Vivek Gautam

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

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 <[email protected]>
---
drivers/phy/phy-exynos5-usbdrd.c | 219 +++++++++++++++++++++++++++++++++++---
1 file changed, 203 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index b3ca3bc..1a63634 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
-
#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;
+ int (*phy_exynos_calibrate)(struct exynos5_usbdrd_phy *phy_drd);
};

/**
@@ -399,9 +415,21 @@ static int exynos5_usbdrd_phy_init(struct phy *phy)
reg &= ~PHYCLKRST_PORTRESET;
writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST);

+ /*
+ * Calibrate some of the PHY parameters, using cr_port control
+ * register, which are internal to PHY and are not exposed
+ * directly to the outside world for configuring.
+ */
+ if (inst->phy_cfg->phy_calibrate) {
+ ret = inst->phy_cfg->phy_calibrate(inst);
+ if (ret)
+ dev_err(phy_drd->dev,
+ "Exiting init: Failed to calibrate PHY\n");
+ }
+
clk_disable_unprepare(phy_drd->clk);

- return 0;
+ return ret;
}

static int exynos5_usbdrd_phy_exit(struct phy *phy)
@@ -487,6 +515,163 @@ 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 handshake timeout2 (0x%08x)\n", val);
+ return -ETIME;
+ }
+
+ return 0;
+}
+
+static int crport_ctrl_write(struct exynos5_usbdrd_phy *phy_drd,
+ u32 addr, u32 data)
+{
+ int ret;
+
+ /* Write Address */
+ writel(PHYREG0_CR_DATA_IN(addr),
+ phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
+ ret = crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(addr),
+ PHYREG0_CR_CAP_ADDR);
+ if (ret)
+ return ret;
+
+ /* Write Data */
+ writel(PHYREG0_CR_DATA_IN(data),
+ phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
+ ret = crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(data),
+ PHYREG0_CR_CAP_DATA);
+ if (ret)
+ return ret;
+
+ ret = crport_handshake(phy_drd, PHYREG0_CR_DATA_IN(data),
+ PHYREG0_CR_WRITE);
+
+ return ret;
+}
+
+/*
+ * Calibrate few PHY parameters using CR_PORT register to 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)
+{
+ unsigned int temp;
+ int ret = 0;
+
+ /*
+ * 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;
+ ret = crport_ctrl_write(phy_drd,
+ EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN,
+ temp);
+ if (ret) {
+ dev_err(phy_drd->dev,
+ "Failed setting Loss-of-Signal level for SuperSpeed\n");
+ return ret;
+ }
+
+ /*
+ * 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;
+ ret = crport_ctrl_write(phy_drd,
+ EXYNOS5_DRD_PHYSS_TX_VBOOSTLEVEL_OVRD_IN,
+ temp);
+ if (ret) {
+ dev_err(phy_drd->dev,
+ "Failed setting Tx-Vboost-Level for SuperSpeed\n");
+ return ret;
+ }
+
+ /*
+ * Set proper time to wait for RxDetect measurement, for
+ * desired reference clock of PHY, by tuning the CR_PORT
+ * 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;
+ }
+
+ ret = crport_ctrl_write(phy_drd,
+ EXYNOS5_DRD_PHYSS_LANE0_TX_DEBUG,
+ temp);
+ if (ret)
+ dev_err(phy_drd->dev,
+ "Failed setting RxDetect measurement time for SuperSpeed\n");
+
+ return ret;
+}
+
+/* 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->phy_exynos_calibrate)
+ return phy_drd->drv_data->phy_exynos_calibrate(phy_drd);
+
+ return -ENODEV;
+}
+
static struct phy *exynos5_usbdrd_phy_xlate(struct device *dev,
struct of_phandle_args *args)
{
@@ -518,6 +703,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 +711,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,
+ .phy_exynos_calibrate = exynos5420_usbdrd_phy_calibrate,
};

static const struct exynos5_usbdrd_phy_drvdata exynos5250_usbdrd_phy = {
--
1.7.10.4

2014-10-31 13:51:15

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

Hello.

On 10/31/2014 4:26 PM, Vivek Gautam wrote:

> The host controller by itself may sometimes need to handle PHY
> and re-initialize it to re-configure some of the PHY parameters
> to get full support out of the PHY controller.
> Therefore, facilitate getting the two possible PHYs, viz.
> USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.

> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> drivers/usb/host/xhci-plat.c | 74 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)

> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3d78b0c..5207d5b 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
[...]
> @@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
> goto put_hcd;
> }
>
> + /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
> + hcd->phy = devm_phy_get(&pdev->dev, "usb2-phy");
> + if (IS_ERR(hcd->phy)) {
> + ret = PTR_ERR(hcd->phy);
> + if (ret == -EPROBE_DEFER) {
> + goto disable_clk;
> + } else if (ret != -ENOSYS && ret != -ENODEV) {

Asking to be a *switch* statement instead...

> + hcd->phy = NULL;
> + dev_warn(&pdev->dev,
> + "Error retrieving usb2 phy: %d\n", ret);
> + }
> + }
> +
[...]
> @@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> xhci->shared_hcd->can_do_streams = 1;
>
> + /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
> + xhci->shared_hcd->phy = devm_phy_get(&pdev->dev, "usb3-phy");
> + if (IS_ERR(xhci->shared_hcd->phy)) {
> + ret = PTR_ERR(xhci->shared_hcd->phy);
> + if (ret == -EPROBE_DEFER) {
> + goto put_usb3_hcd;
> + } else if (ret != -ENOSYS && ret != -ENODEV) {

Likewise...

> + xhci->shared_hcd->phy = NULL;
> + dev_warn(&pdev->dev,
> + "Error retrieving usb3 phy: %d\n", ret);
> + }
> + }
> +
[...]
> @@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>
> + phy_exit(hcd->phy);

Hm, in the suspend() method?

> +
> return xhci_suspend(xhci);
> }
>
[...]

WBR, Sergei

2014-11-17 06:36:34

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

Hi,


On Fri, Oct 31, 2014 at 7:21 PM, Sergei Shtylyov
<[email protected]> wrote:
> Hello.
>
> On 10/31/2014 4:26 PM, Vivek Gautam wrote:
>
>> The host controller by itself may sometimes need to handle PHY
>> and re-initialize it to re-configure some of the PHY parameters
>> to get full support out of the PHY controller.
>> Therefore, facilitate getting the two possible PHYs, viz.
>> USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.
>
>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> ---
>> drivers/usb/host/xhci-plat.c | 74
>> ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 74 insertions(+)
>
>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 3d78b0c..5207d5b 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>
> [...]
>>
>> @@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device
>> *pdev)
>> goto put_hcd;
>> }
>>
>> + /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
>> + hcd->phy = devm_phy_get(&pdev->dev, "usb2-phy");
>> + if (IS_ERR(hcd->phy)) {
>> + ret = PTR_ERR(hcd->phy);
>> + if (ret == -EPROBE_DEFER) {
>> + goto disable_clk;
>> + } else if (ret != -ENOSYS && ret != -ENODEV) {
>
>
> Asking to be a *switch* statement instead...

Sure, will change this to *switch* statement. That will improve the
readability.

>
>> + hcd->phy = NULL;
>> + dev_warn(&pdev->dev,
>> + "Error retrieving usb2 phy: %d\n", ret);
>> + }
>> + }
>> +
>
> [...]
>>
>> @@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device
>> *pdev)
>> if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>> xhci->shared_hcd->can_do_streams = 1;
>>
>> + /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
>> + xhci->shared_hcd->phy = devm_phy_get(&pdev->dev, "usb3-phy");
>> + if (IS_ERR(xhci->shared_hcd->phy)) {
>> + ret = PTR_ERR(xhci->shared_hcd->phy);
>> + if (ret == -EPROBE_DEFER) {
>> + goto put_usb3_hcd;
>> + } else if (ret != -ENOSYS && ret != -ENODEV) {
>
>
> Likewise...

ok

>
>> + xhci->shared_hcd->phy = NULL;
>> + dev_warn(&pdev->dev,
>> + "Error retrieving usb3 phy: %d\n", ret);
>> + }
>> + }
>> +
>
> [...]
>>
>> @@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>
>> + phy_exit(hcd->phy);
>
>
> Hm, in the suspend() method?

phy_exit() should eventually be suspending the PHY and put it to
low power state.
phy_init() in resume() will then take up the task of activating the
PHY again.

phy_power_on() and phy_power_off() are called at xhci_probe() and remove() time.

Does this makes sense ?



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-11-17 06:38:56

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

Hi Felipe,



On Fri, Oct 31, 2014 at 6:56 PM, Vivek Gautam <[email protected]> wrote:
> The host controller by itself may sometimes need to handle PHY
> and re-initialize it to re-configure some of the PHY parameters
> to get full support out of the PHY controller.
> Therefore, facilitate getting the two possible PHYs, viz.
> USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> drivers/usb/host/xhci-plat.c | 74 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3d78b0c..5207d5b 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> #include <linux/slab.h>
> #include <linux/usb/xhci_pdriver.h>
>
> @@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
> goto put_hcd;
> }
>
> + /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
> + hcd->phy = devm_phy_get(&pdev->dev, "usb2-phy");
> + if (IS_ERR(hcd->phy)) {
> + ret = PTR_ERR(hcd->phy);
> + if (ret == -EPROBE_DEFER) {
> + goto disable_clk;
> + } else if (ret != -ENOSYS && ret != -ENODEV) {
> + hcd->phy = NULL;
> + dev_warn(&pdev->dev,
> + "Error retrieving usb2 phy: %d\n", ret);
> + }
> + }
> +
> ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (ret)
> goto disable_clk;
>
> + /*
> + * Initialize and power-on USB 2.0 PHY
> + * FIXME: Isn't this a hacky way of initializing the PHY again ?
> + * xhci's parent would have already initialized the PHY, but we
> + * wanna do it again.
> + */

Does this change looks anywhere close to what you suggested to
re-initialize PHYs in XHCI even after DWC3 has initialized them once,
in order to avoid adding phy_calibration() callback ? ;-)

> + hcd->phy->init_count = 0;
> + ret = phy_init(hcd->phy);
> + if (ret)
> + goto dealloc_usb2_hcd;
> +
> + hcd->phy->power_count = 0;
> + ret = phy_power_on(hcd->phy);
> + if (ret) {
> + phy_exit(hcd->phy);
> + goto dealloc_usb2_hcd;
> + }
> +
> device_wakeup_enable(hcd->self.controller);
>
> /* USB 2.0 roothub is stored in the platform_device now. */
> @@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> xhci->shared_hcd->can_do_streams = 1;
>
> + /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
> + xhci->shared_hcd->phy = devm_phy_get(&pdev->dev, "usb3-phy");
> + if (IS_ERR(xhci->shared_hcd->phy)) {
> + ret = PTR_ERR(xhci->shared_hcd->phy);
> + if (ret == -EPROBE_DEFER) {
> + goto put_usb3_hcd;
> + } else if (ret != -ENOSYS && ret != -ENODEV) {
> + xhci->shared_hcd->phy = NULL;
> + dev_warn(&pdev->dev,
> + "Error retrieving usb3 phy: %d\n", ret);
> + }
> + }
> +
> ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> if (ret)
> goto put_usb3_hcd;
>
> + /* 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;
> + }
> +
> return 0;
>
> +dealloc_usb3_hcd:
> + usb_remove_hcd(xhci->shared_hcd);
> +
> put_usb3_hcd:
> usb_put_hcd(xhci->shared_hcd);
>
> @@ -186,9 +247,15 @@ static int xhci_plat_remove(struct platform_device *dev)
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> struct clk *clk = xhci->clk;
>
> + phy_power_off(xhci->shared_hcd->phy);
> + phy_exit(xhci->shared_hcd->phy);
> +
> usb_remove_hcd(xhci->shared_hcd);
> usb_put_hcd(xhci->shared_hcd);
>
> + phy_power_off(hcd->phy);
> + phy_exit(hcd->phy);
> +
> usb_remove_hcd(hcd);
> if (!IS_ERR(clk))
> clk_disable_unprepare(clk);
> @@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>
> + phy_exit(hcd->phy);
> +
> return xhci_suspend(xhci);
> }
>
> @@ -211,6 +280,11 @@ static int xhci_plat_resume(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + int ret;
> +
> + ret = phy_init(hcd->phy);
> + if (ret)
> + return ret;
>
> return xhci_resume(xhci, 0);
> }
> --
> 1.7.10.4
>



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-11-17 16:16:42

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

Hello.

On 11/17/2014 9:36 AM, Vivek Gautam wrote:

>>> The host controller by itself may sometimes need to handle PHY
>>> and re-initialize it to re-configure some of the PHY parameters
>>> to get full support out of the PHY controller.
>>> Therefore, facilitate getting the two possible PHYs, viz.
>>> USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.

>>> Signed-off-by: Vivek Gautam <[email protected]>
>>> ---
>>> drivers/usb/host/xhci-plat.c | 74
>>> ++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 74 insertions(+)

>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 3d78b0c..5207d5b 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
[...]
>>> @@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
>>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>>> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>
>>> + phy_exit(hcd->phy);

>> Hm, in the suspend() method?

> phy_exit() should eventually be suspending the PHY and put it to
> low power state.

I thought it's a role that the power_off() mothod should play, considering
that the power_on() method gets called after the init() method....

> phy_init() in resume() will then take up the task of activating the
> PHY again.

> phy_power_on() and phy_power_off() are called at xhci_probe() and remove() time.

Of course.

> Does this makes sense ?

Not much, really.

WBR, Sergei

2014-11-18 12:20:22

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds

On Mon, Nov 17, 2014 at 9:46 PM, Sergei Shtylyov
<[email protected]> wrote:
> Hello.
>
> On 11/17/2014 9:36 AM, Vivek Gautam wrote:
>
>>>> The host controller by itself may sometimes need to handle PHY
>>>> and re-initialize it to re-configure some of the PHY parameters
>>>> to get full support out of the PHY controller.
>>>> Therefore, facilitate getting the two possible PHYs, viz.
>>>> USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.
>
>
>>>> Signed-off-by: Vivek Gautam <[email protected]>
>>>> ---
>>>> drivers/usb/host/xhci-plat.c | 74
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 74 insertions(+)
>
>
>>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>>> index 3d78b0c..5207d5b 100644
>>>> --- a/drivers/usb/host/xhci-plat.c
>>>> +++ b/drivers/usb/host/xhci-plat.c
>
> [...]
>>>>
>>>> @@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
>>>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>>>> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>>
>>>> + phy_exit(hcd->phy);
>
>
>>> Hm, in the suspend() method?
>
>
>> phy_exit() should eventually be suspending the PHY and put it to
>> low power state.
>
>
> I thought it's a role that the power_off() mothod should play,
> considering that the power_on() method gets called after the init()
> method....

phy_power_off() should be cutting the clocks and power from PHY
completely, no ?
In that case one may not be able to wakeup the system from USB.

So phy_exit() gets the responsibility to put the PHY into low power state.

Ccing Kishon also to get his opinion on actual role of the two callbacks -
phy_init/exit() and phy_power_on/off().

>
>> phy_init() in resume() will then take up the task of activating the
>> PHY again.
>
>
>> phy_power_on() and phy_power_off() are called at xhci_probe() and remove()
>> time.
>
>
> Of course.
>
>> Does this makes sense ?
>
>
> Not much, really.
>
> WBR, Sergei
>



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India