Hi Stan,

On Mon, 2013-10-07 at 12:22 +0300, Stanimir Varbanov wrote: 
> Hi Ivan,
> 
> Few comments below.
> 
> On 10/07/2013 10:44 AM, Ivan T. Ivanov wrote:
> > From: "Ivan T. Ivanov" <iiva...@mm-sol.com>
> > 
> > These drivers handles control and configuration of the HS
> > and SS USB PHY transceivers. They are part of the driver
> > which manage Synopsys DesignWare USB3 controller stack
> > inside Qualcomm SoC's.
> > 
> > Signed-off-by: Ivan T. Ivanov <iiva...@mm-sol.com>
> > ---
> >  drivers/usb/phy/Kconfig         |   11 ++
> >  drivers/usb/phy/Makefile        |    2 +
> >  drivers/usb/phy/phy-msm-dw-hs.c |  329 ++++++++++++++++++++++++++++++++++
> >  drivers/usb/phy/phy-msm-dw-ss.c |  375 
> > +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 717 insertions(+)
> >  create mode 100644 drivers/usb/phy/phy-msm-dw-hs.c
> >  create mode 100644 drivers/usb/phy/phy-msm-dw-ss.c
> > 
> > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> > index d5589f9..bbb2d0e 100644
> > --- a/drivers/usb/phy/Kconfig
> > +++ b/drivers/usb/phy/Kconfig
> > @@ -214,6 +214,17 @@ config USB_RCAR_PHY
> >       To compile this driver as a module, choose M here: the
> >       module will be called phy-rcar-usb.
> >  
> > +config USB_MSM_DW_PHYS
> > +   tristate "Qualcomm USB controller DW PHY's wrappers support"
> > +   depends on (USB || USB_GADGET) && ARCH_MSM
> > +   select USB_PHY
> > +   help
> > +     Enable this to support the DW USB PHY transceivers on MSM chips
> > +     with DWC3 USB core. It handles PHY initialization, clock
> > +     management required after resetting the hardware and power
> > +     management. This driver is required even for peripheral only or
> > +     host only mode configurations.
> > +
> >  config USB_ULPI
> >     bool "Generic ULPI Transceiver Driver"
> >     depends on ARM
> > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> > index 2135e85..4813eb5 100644
> > --- a/drivers/usb/phy/Makefile
> > +++ b/drivers/usb/phy/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_USB_EHCI_TEGRA)              += 
> > phy-tegra-usb.o
> >  obj-$(CONFIG_USB_GPIO_VBUS)                += phy-gpio-vbus-usb.o
> >  obj-$(CONFIG_USB_ISP1301)          += phy-isp1301.o
> >  obj-$(CONFIG_USB_MSM_OTG)          += phy-msm-usb.o
> > +obj-$(CONFIG_USB_MSM_DW_PHYS)              += phy-msm-dw-hs.o
> > +obj-$(CONFIG_USB_MSM_DW_PHYS)              += phy-msm-dw-ss.o
> >  obj-$(CONFIG_USB_MV_OTG)           += phy-mv-usb.o
> >  obj-$(CONFIG_USB_MXS_PHY)          += phy-mxs-usb.o
> >  obj-$(CONFIG_USB_RCAR_PHY)         += phy-rcar-usb.o
> > diff --git a/drivers/usb/phy/phy-msm-dw-hs.c 
> > b/drivers/usb/phy/phy-msm-dw-hs.c
> > new file mode 100644
> > index 0000000..d29c1f1
> > --- /dev/null
> > +++ b/drivers/usb/phy/phy-msm-dw-hs.c
> > @@ -0,0 +1,329 @@
> > +/* Copyright (c) 2013, Code Aurora Forum. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/usb/phy.h>
> > +
> > +/**
> > + *  USB QSCRATCH Hardware registers
> > + */
> > +#define QSCRATCH_CTRL_REG          (0x04)
> > +#define QSCRATCH_GENERAL_CFG               (0x08)
> > +#define PHY_CTRL_REG                       (0x10)
> > +#define PARAMETER_OVERRIDE_X_REG   (0x14)
> > +#define CHARGING_DET_CTRL_REG              (0x18)
> > +#define CHARGING_DET_OUTPUT_REG            (0x1c)
> > +#define ALT_INTERRUPT_EN_REG               (0x20)
> > +#define PHY_IRQ_STAT_REG           (0x24)
> > +#define CGCTL_REG                  (0x28)
> > +
> 
> Please remove braces above and below.

ok

> 
> > +#define PHY_3P3_VOL_MIN                    3050000 /* uV */
> > +#define PHY_3P3_VOL_MAX                    3300000 /* uV */
> > +#define PHY_3P3_HPM_LOAD           16000   /* uA */
> > +
> > +#define PHY_1P8_VOL_MIN                    1800000 /* uV */
> > +#define PHY_1P8_VOL_MAX                    1800000 /* uV */
> > +#define PHY_1P8_HPM_LOAD           19000   /* uA */
> > +
> > +/* TODO: these are suspicious */
> > +#define USB_VDDCX_NO                       1       /* index */
> > +#define USB_VDDCX_MIN                      5       /* index */
> > +#define USB_VDDCX_MAX                      7       /* index */
> > +
> > +struct msm_dw_hs_phy {
> > +   struct usb_phy          phy;
> > +   void __iomem            *base;
> > +   struct device           *dev;
> > +
> > +   struct clk              *xo_clk;
> > +   struct clk              *sleep_a_clk;
> > +
> > +   struct regulator        *v3p3;
> > +   struct regulator        *v1p8;
> > +   struct regulator        *vddcx;
> > +   struct regulator        *vbus;
> > +};
> > +
> > +#define    phy_to_dw_phy(x)        container_of((x), struct msm_dw_hs_phy, 
> > phy)
> 
> I think that using tab after #define is uncommon, isn't it?

Not in this case. I see both patterns used.

> 
> > +
> > +
> > +/**
> > + * Write register.
> > + *
> > + * @base - MSM DWC3 PHY base virtual address.
> > + * @offset - register offset.
> > + * @val - value to write.
> > + */
> > +static inline void msm_dw_hs_write(void __iomem *base, u32 offset, u32 val)
> 
> Why inline? here and below
> 

Hm, because I will like function to be inlined??

> > +{
> > +   iowrite32(val, base + offset);
> > +}
> > +
> > +/**
> > + * Write register and read back masked value to confirm it is written
> > + *
> > + * @base - MSM DWC3 PHY base virtual address.
> > + * @offset - register offset.
> > + * @mask - register bitmask specifying what should be updated
> > + * @val - value to write.
> > + */
> > +static inline void msm_dw_hs_write_readback(void __iomem *base, u32 offset,
> > +                                       const u32 mask, u32 val)
> > +{
> > +   u32 write_val, tmp = ioread32(base + offset);
> > +
> > +   tmp &= ~mask;           /* retain other bits */
> > +   write_val = tmp | val;
> > +
> > +   iowrite32(write_val, base + offset);
> > +
> > +   /* Read back to see if val was written */
> > +   tmp = ioread32(base + offset);
> > +   tmp &= mask;            /* clear other bits */
> > +
> > +   if (tmp != val)
> > +           pr_err("write: %x to QSCRATCH: %x FAILED\n", val, offset);
> 
> You should use dev_err() here. Or better remove the error printing and
> make the function to return int.

I change it to dev_err(). This could happen only if clocks are
not properly setup.

> 
> > +}
> > +
> > +static void msm_dw_hs_phy_shutdown(struct usb_phy *x)
> > +{
> > +   struct msm_dw_hs_phy *phy = phy_to_dw_phy(x);
> > +   int ret;
> > +
> > +   ret = regulator_set_voltage(phy->v3p3, 0, PHY_3P3_VOL_MAX);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot set voltage for v3p3\n");
> > +
> > +   ret = regulator_set_voltage(phy->v1p8, 0, PHY_1P8_VOL_MAX);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot set voltage for v1p8\n");
> > +
> > +   ret = regulator_disable(phy->v1p8);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot disable v1p8\n");
> > +
> > +   ret = regulator_disable(phy->v3p3);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot disable v3p3\n");
> > +
> > +   ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_NO, USB_VDDCX_MAX);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot set voltage for vddcx\n");
> > +
> > +   ret = regulator_disable(phy->vddcx);
> > +   if (ret)
> > +           dev_err(phy->dev, "cannot enable vddcx\n");
> > +
> > +   clk_disable_unprepare(phy->sleep_a_clk);
> > +}
> > +
> > +static int msm_dw_hs_phy_init(struct usb_phy *x)
> > +{
> > +   struct msm_dw_hs_phy    *phy = phy_to_dw_phy(x);
> 
> Using tab after struct name is uncommon IMO. 

Take a look at OMAP's USB PHY driver, on which thes driver are based.

> 
> > +   int ret;

Important point is to consistent, and in this case I am not :-)
Will remove variables indentation.

> > +
> > +   clk_prepare_enable(phy->sleep_a_clk);
> > +
> > +   ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_MIN, USB_VDDCX_MAX);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot set voltage for vddcx\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_enable(phy->vddcx);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot enable vddcx\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_set_voltage(phy->v3p3, PHY_3P3_VOL_MIN,
> > +                               PHY_3P3_VOL_MAX);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot set voltage for v3p3\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_set_voltage(phy->v1p8, PHY_1P8_VOL_MIN,
> > +                               PHY_1P8_VOL_MAX);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot set voltage for v1p8\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_set_optimum_mode(phy->v1p8, PHY_1P8_HPM_LOAD);
> > +   if (ret < 0) {
> > +           dev_err(phy->dev, "cannot set HPM of regulator v1p8\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_enable(phy->v1p8);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot enable v1p8\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_set_optimum_mode(phy->v3p3, PHY_3P3_HPM_LOAD);
> > +   if (ret < 0) {
> > +           dev_err(phy->dev, "cannot set HPM of regulator v3p3\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_enable(phy->v3p3);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot enable v3p3\n");
> > +           return ret;
> > +   }
> > +
> > +   /*
> > +    * HSPHY Initialization: Enable UTMI clock and clamp enable HVINTs,
> > +    * and disable RETENTION (power-on default is ENABLED)
> > +    */
> > +   msm_dw_hs_write(phy->base, PHY_CTRL_REG, 0x5220bb2);
> > +   usleep_range(2000, 2200);
> > +
> > +   /*
> > +    * write HSPHY init value to QSCRATCH reg to set HSPHY parameters like
> > +    * VBUS valid threshold, disconnect valid threshold, DC voltage level,
> > +    * preempasis and rise/fall time.
> > +    */
> > +   msm_dw_hs_write_readback(phy->base, PARAMETER_OVERRIDE_X_REG,
> > +                           0x03ffffff, 0x00d191a4);
> > +
> > +   /* Disable (bypass) VBUS and ID filters */
> > +   msm_dw_hs_write(phy->base, QSCRATCH_GENERAL_CFG, 0x78);
> > +
> > +   return 0;
> > +}
> > +
> > +static int msm_dw_hs_phy_set_vbus(struct usb_phy *x, int on)
> > +{
> > +   struct msm_dw_hs_phy    *phy = phy_to_dw_phy(x);
> > +   int ret;
> > +
> > +   if (IS_ERR(phy->vbus))
> > +           return on ? PTR_ERR(phy->vbus) : 0;
> > +
> > +   if (on)
> > +           ret = regulator_enable(phy->vbus);
> > +   else
> > +           ret = regulator_disable(phy->vbus);
> > +
> > +   if (ret)
> > +           dev_err(x->dev, "Cannot %s Vbus\n", on ? "set" : "off");
> 
> This looks like debug code. Should the error being handled by the upper
> layers? I'd remove this.

I could lower this to debug, but will prefer to keep message here.

> 
> > +   return ret;
> > +}
> > +
> > +static int msm_dw_hs_probe(struct platform_device *pdev)
> > +{
> > +   struct msm_dw_hs_phy    *phy;
> > +   struct resource         *res;
> > +   void __iomem            *base;
> > +
> > +   phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> > +   if (!phy)
> > +           return -ENOMEM;
> > +
> > +   platform_set_drvdata(pdev, phy);
> > +
> > +   phy->dev = &pdev->dev;
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   base = devm_ioremap_resource(phy->dev, res);
> > +   if (IS_ERR(base))
> > +           return PTR_ERR(base);
> > +
> > +   phy->vddcx = devm_regulator_get(phy->dev, "vddcx");
> > +   if (IS_ERR(phy->vddcx)) {
> > +           dev_dbg(phy->dev, "cannot get vddcx\n");
> > +           return  PTR_ERR(phy->vddcx);
> > +   }
> > +
> > +   phy->v3p3 = devm_regulator_get(phy->dev, "v3p3");
> > +   if (IS_ERR(phy->v3p3)) {
> > +           dev_dbg(phy->dev, "cannot get v3p3\n");
> > +           return PTR_ERR(phy->v3p3);
> > +   }
> > +
> > +   phy->v1p8 = devm_regulator_get(phy->dev, "v1p8");
> > +   if (IS_ERR(phy->v1p8)) {
> > +           dev_dbg(phy->dev, "cannot get v1p8\n");
> > +           return PTR_ERR(phy->v1p8);
> > +   }
> > +
> > +   phy->vbus = devm_regulator_get(phy->dev, "vbus");
> > +   if (IS_ERR(phy->vbus))
> > +           dev_dbg(phy->dev, "Failed to get vbus\n");
> > +
> > +   phy->xo_clk = devm_clk_get(phy->dev, "xo");
> > +   if (IS_ERR(phy->xo_clk)) {
> > +           dev_dbg(phy->dev, "cannot get TCXO buffer handle\n");
> > +           return PTR_ERR(phy->xo_clk);
> > +   }
> > +
> > +   phy->sleep_a_clk = devm_clk_get(phy->dev, "sleep_a");
> 
> What means the "_a" in clock name?

They are 2 PHY's on 8074 chip. This drivers is supposed to 
operate on PHY 0, thus sleep_a. PHY 1 is using sleep_b. Take a look 
here http://www.spinics.net/lists/arm-kernel/msg276945.html


> 
> > +   if (IS_ERR(phy->sleep_a_clk)) {
> > +           dev_dbg(phy->dev, "failed to get sleep_a clock\n");
> > +           return PTR_ERR(phy->sleep_a_clk);
> > +   }
> > +
> > +   clk_prepare_enable(phy->xo_clk);
> > +
> > +   phy->base               = base;
> > +   phy->phy.dev            = phy->dev;
> > +   phy->phy.label          = "msm-dw-hsphy";
> > +   phy->phy.init           = msm_dw_hs_phy_init;
> > +   phy->phy.shutdown       = msm_dw_hs_phy_shutdown;
> > +   phy->phy.set_vbus       = msm_dw_hs_phy_set_vbus;
> > +   phy->phy.type           = USB_PHY_TYPE_USB2;
> > +   phy->phy.state          = OTG_STATE_UNDEFINED;
> > +
> > +   usb_add_phy_dev(&phy->phy);
> 
> this function returns int, why you don't checked it?
> 

Functions will fail only if we do not pass correct 'dev' instance, 
which could not happen here. Anyway will check for error.

> > +
> > +   return 0;
> > +}

<snip>

> > +
> > +static struct platform_driver msm_dw_hs_driver = {
> > +   .probe          = msm_dw_hs_probe,
> > +   .remove         = msm_dw_hs_remove,
> > +   .driver         = {
> > +           .name   = "msm-dw-hsphy",
> > +           .owner  = THIS_MODULE,
> > +           .of_match_table = msm_dw_hs_id_table,
> > +   },
> > +};
> > +
> > +module_platform_driver(msm_dw_hs_driver);
> > +
> > +MODULE_ALIAS("platform:msm-dw-hsphy");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("DesignWare USB3 MSM HSPHY driver");
> > diff --git a/drivers/usb/phy/phy-msm-dw-ss.c 
> > b/drivers/usb/phy/phy-msm-dw-ss.c
> > new file mode 100644
> > index 0000000..6734fa1
> > --- /dev/null
> > +++ b/drivers/usb/phy/phy-msm-dw-ss.c
> > @@ -0,0 +1,375 @@
> > +/* Copyright (c) 2013, Code Aurora Forum. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/usb/phy.h>
> > +
> > +/**
> > + *  USB QSCRATCH Hardware registers
> > + */
> > +#define PHY_CTRL_REG                       (0x00)
> > +#define PHY_PARAM_CTRL_1           (0x04)
> > +#define PHY_PARAM_CTRL_2           (0x08)
> > +#define CR_PROTOCOL_DATA_IN_REG            (0x0c)
> > +#define CR_PROTOCOL_DATA_OUT_REG   (0x10)
> > +#define CR_PROTOCOL_CAP_ADDR_REG   (0x14)
> > +#define CR_PROTOCOL_CAP_DATA_REG   (0x18)
> > +#define CR_PROTOCOL_READ_REG               (0x1c)
> > +#define CR_PROTOCOL_WRITE_REG              (0x20)
> 
> Remove braces.

sure.

> 

<snip>

> > +
> > +static int msm_dw_ss_phy_init(struct usb_phy *x)
> > +{
> > +   struct msm_dw_ss_phy *phy = phy_to_dw_phy(x);
> > +   u32 data = 0;
> > +   int ret;
> > +
> > +   ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_MIN, USB_VDDCX_MAX);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot set voltage for vddcx\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_enable(phy->vddcx);
> > +   if (ret) {
> > +           dev_err(phy->dev, "cannot enable vddcx\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_set_voltage(phy->v1p8, PHY_1P8_VOL_MIN,
> > +                               PHY_1P8_VOL_MAX);
> > +   if (ret) {
> > +           regulator_disable(phy->vddcx);
> > +           dev_err(phy->dev, "cannot set v1p8\n");
> > +           return ret;
> > +   }
> > +
> > +   ret = regulator_enable(phy->v1p8);
> > +   if (ret) {
> > +           regulator_disable(phy->vddcx);
> > +           dev_err(phy->dev, "cannot enable v1p8\n");
> > +           return ret;
> > +   }
> > +
> > +   clk_prepare_enable(phy->ref_clk);
> > +   usleep_range(1000, 1200);
> > +
> > +   /* SSPHY: Use ref_clk from pads and set its parameters */
> > +   msm_dw_ss_write(phy->base, PHY_CTRL_REG, 0x10210002);
> 
> Defines?
> 
> > +   msleep(30);
> > +   /* Assert SSPHY reset */
> > +   msm_dw_ss_write(phy->base, PHY_CTRL_REG, 0x10210082);
> > +   usleep_range(2000, 2200);
> > +   /* De-assert SSPHY reset - power and ref_clock must be ON */
> > +   msm_dw_ss_write(phy->base, PHY_CTRL_REG, 0x10210002);
> > +   usleep_range(2000, 2200);
> > +   /* Ref clock must be stable now, enable ref clock for HS mode */
> > +   msm_dw_ss_write(phy->base, PHY_CTRL_REG, 0x10210102);
> > +   usleep_range(2000, 2200);
> 
> You can extract the bits and add defines with names got from comments.

You know as much as I know, suggestions for define names?

> 
> > +
> > +   /*
> > +    * WORKAROUND: There is SSPHY suspend bug due to which USB enumerates
> > +    * in HS mode instead of SS mode. Workaround it by asserting
> > +    * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus mode
> > +    */
> > +   data = msm_dw_ss_read_phycreg(phy->base, 0x102d);
> > +   data |= (1 << 7);
> > +   msm_dw_ss_write_phycreg(phy->base, 0x102D, data);
> > +
> 
> Defines? 

ok. this one is easy :-)

> Also you mixing uppper and lower case in hex numberes, use
> lower case.
> 

will fix it.

> > +   data = msm_dw_ss_read_phycreg(phy->base, 0x1010);
> > +   data &= ~0xff0;
> > +   data |= 0x20;
> > +   msm_dw_ss_write_phycreg(phy->base, 0x1010, data);
> > +
> > +   /*
> > +    * Fix RX Equalization setting as follows
> > +    * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
> > +    * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
> > +    * LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
> > +    * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
> > +    */
> > +   data = msm_dw_ss_read_phycreg(phy->base, 0x1006);
> 
> Defines?

Ok this is also easy, but will #define make code easy to read. 
We have such nice comment here.

> 
> > +   data &= ~(1 << 6);
> > +   data |= (1 << 7);
> > +   data &= ~(0x7 << 8);
> > +   data |= (0x3 << 8);
> > +   data |= (0x1 << 11);
> > +   msm_dw_ss_write_phycreg(phy->base, 0x1006, data);
> 
> You mixing two styles. The first is using magic numbers combined with
> comment and the second style is mixing magic bits and masks. Could you
> use one defines on all places?

Will try to make it consistent.

> 
> > +
> > +   /*
> > +    * Set EQ and TX launch amplitudes as follows
> > +    * LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
> > +    * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
> > +    * LANE0.TX_OVRD_DRV_LO.EN set to 1.
> > +    */
> > +   data = msm_dw_ss_read_phycreg(phy->base, 0x1002);
> > +   data &= ~0x3f80;
> > +   data |= (0x16 << 7);
> > +   data &= ~0x7f;
> > +   data |= (0x7f | (1 << 14));
> > +   msm_dw_ss_write_phycreg(phy->base, 0x1002, data);
> > +
> > +   /*
> > +    * Set the QSCRATCH PHY_PARAM_CTRL1 parameters as follows
> > +    * TX_FULL_SWING [26:20] amplitude to 127
> > +    * TX_DEEMPH_3_5DB [13:8] to 22
> > +    * LOS_BIAS [2:0] to 0x5
> > +    */
> > +   msm_dw_ss_write_readback(phy->base, PHY_PARAM_CTRL_1,
> > +                              0x07f03f07, 0x07f01605);
> > +   return 0;
> > +}
> > +
> > +static int msm_dw_ss_probe(struct platform_device *pdev)
> > +{
> > +   struct msm_dw_ss_phy    *phy;
> > +   struct resource         *res;
> > +   void __iomem            *base;
> > +
> > +   phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> > +   if (!phy)
> > +           return -ENOMEM;
> > +
> > +   platform_set_drvdata(pdev, phy);
> > +
> > +   phy->dev = &pdev->dev;
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   base = devm_ioremap_resource(phy->dev, res);
> > +   if (IS_ERR(base))
> > +           return PTR_ERR(base);
> > +
> > +   phy->vddcx = devm_regulator_get(phy->dev, "vddcx");
> > +   if (IS_ERR(phy->vddcx)) {
> > +           dev_dbg(phy->dev, "cannot get vddcx\n");
> > +           return  PTR_ERR(phy->vddcx);
> > +   }
> > +
> > +   phy->v1p8 = devm_regulator_get(phy->dev, "v1p8");
> > +   if (IS_ERR(phy->v1p8)) {
> > +           dev_dbg(phy->dev, "cannot get v1p8\n");
> > +           return  PTR_ERR(phy->v1p8);
> > +   }
> > +
> > +   phy->xo_clk = devm_clk_get(phy->dev, "xo");
> > +   if (IS_ERR(phy->xo_clk)) {
> > +           dev_dbg(phy->dev, "cannot get TCXO buffer handle\n");
> > +           return PTR_ERR(phy->xo_clk);
> > +   }
> > +
> > +   phy->ref_clk = devm_clk_get(phy->dev, "ref");
> > +   if (IS_ERR(phy->ref_clk)) {
> > +           dev_dbg(phy->dev, "cannot get ref clock handle\n");
> > +           return PTR_ERR(phy->ref_clk);
> > +   }
> > +
> > +   clk_prepare_enable(phy->xo_clk);
> > +
> > +   phy->base               = base;
> > +   phy->phy.dev            = phy->dev;
> > +   phy->phy.label          = "msm-dw-ssphy";
> > +   phy->phy.init           = msm_dw_ss_phy_init;
> > +   phy->phy.shutdown       = msm_dw_ss_phy_shutdown;
> > +   phy->phy.type           = USB_PHY_TYPE_USB3;
> > +
> > +   usb_add_phy_dev(&phy->phy);
> 
> Check the error, please.

Same comment.

> 
> > +
> > +   return 0;
> > +}
> > +

<snip> 
> > 
> 
> regards,
> Stan

Thanks,
Ivan


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

Reply via email to