Hi,

On 27/10/18 3:28 PM, Yu Chen wrote:

This license text doesn't have to added explicitly. It should be already
governed by SPDX line.

+
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+
+#define PERI_CRG_CLK_EN4			(0x40)
+#define PERI_CRG_CLK_DIS4			(0x44)
+#define GT_CLK_USB3OTG_REF			BIT(0)
+#define GT_ACLK_USB3OTG				BIT(1)
+
+#define PERI_CRG_RSTEN4				(0x90)
+#define PERI_CRG_RSTDIS4			(0x94)
+#define IP_RST_USB3OTGPHY_POR			BIT(3)
+#define IP_RST_USB3OTG				BIT(5)
+
+#define PERI_CRG_ISODIS				(0x148)
+#define USB_REFCLK_ISO_EN			BIT(25)
+
+#define PCTRL_PERI_CTRL3			(0x10)
+#define PCTRL_PERI_CTRL3_MSK_START		(16)
+#define USB_TCXO_EN				BIT(1)
+
+#define PCTRL_PERI_CTRL24			(0x64)
+#define SC_CLK_USB3PHY_3MUX1_SEL		BIT(25)
+
+#define USBOTG3_CTRL0				(0x00)
+#define SC_USB3PHY_ABB_GT_EN			BIT(15)
+
+#define USBOTG3_CTRL2				(0x08)
+#define USBOTG3CTRL2_POWERDOWN_HSP		BIT(0)
+#define USBOTG3CTRL2_POWERDOWN_SSP		BIT(1)
+
+#define USBOTG3_CTRL3				(0x0C)
+#define USBOTG3_CTRL3_VBUSVLDEXT		BIT(6)
+#define USBOTG3_CTRL3_VBUSVLDEXTSEL		BIT(5)
+
+#define USBOTG3_CTRL4				(0x10)
+
+#define USBOTG3_CTRL7				(0x1c)
+#define REF_SSP_EN				BIT(16)
+
+#define HI3660_USB_DEFAULT_PHY_PARAM		(0x1c466e3)
+
+struct hi3660_priv {
+	struct device *dev;
+	struct regmap *peri_crg;
+	struct regmap *pctrl;
+	struct regmap *otg_bc;
+	u32 eye_diagram_param;
+
+	u32 peri_crg_offset;
+	u32 pctrl_offset;
+	u32 otg_bc_offset;
+};
+
+static int hi3660_phy_init(struct phy *phy)
+{
+	struct hi3660_priv *priv = phy_get_drvdata(phy);
+	u32 val, mask;
+	int ret;
+
+	/* usb refclk iso disable */
+	ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS, USB_REFCLK_ISO_EN);
+	if (ret)
+		goto out;
+
+	/* enable usb_tcxo_en */
+	val = USB_TCXO_EN | (USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
+	ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3, val);
+	if (ret)
+		goto out;
+
+	/* select usbphy clk from abb */
+	mask = SC_CLK_USB3PHY_3MUX1_SEL;

Shouldn't we use clock API's for enabling/selecting clocks?

+	ret = regmap_update_bits(priv->pctrl, PCTRL_PERI_CTRL24, mask, 0);
+	if (ret)
+		goto out;
+
+	/* assert phy */
+	val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+	ret = regmap_write(priv->peri_crg, PERI_CRG_RSTEN4, val);
+	if (ret)
+		goto out;
+
+	/* enable phy ref clk */
+	val = SC_USB3PHY_ABB_GT_EN;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL0, mask, val);
+	if (ret)
+		goto out;
+
+	val = REF_SSP_EN;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL7, mask, val);
+	if (ret)
+		goto out;
+
+	/* exit from IDDQ mode */
+	mask = USBOTG3CTRL2_POWERDOWN_HSP | USBOTG3CTRL2_POWERDOWN_SSP;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL2, mask, 0);
+	if (ret)
+		goto out;
+
+	udelay(100);

Please add a comment for this delay. Does the HW manual states that or it's a
result of experimentation.

Also according to timers/timers-howto.txt, it's preferable to use usleep_range
for 10us - 20ms.

+
+	/* deassert phy */
+	val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+	ret = regmap_write(priv->peri_crg, PERI_CRG_RSTDIS4, val);
+	if (ret)
+		goto out;
+
+	usleep_range(10000, 15000);

Add a comment for this delay.

+
+	/* fake vbus valid signal */
+	val = USBOTG3_CTRL3_VBUSVLDEXT | USBOTG3_CTRL3_VBUSVLDEXTSEL;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL3, mask, val);
+	if (ret)
+		goto out;
+
+	udelay(100);

Same comment here.

Thanks
Kishon