Hi,

On 27/10/18 3:28 PM, Yu Chen wrote:
> This driver handles usb phy power on and shutdown for hi3660 Soc of
> Hisilicon.
> 
> Cc: Kishon Vijay Abraham I <kis...@ti.com>
> Cc: "David S. Miller" <da...@davemloft.net>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: Arnd Bergmann <a...@arndb.de>
> Cc: Shawn Guo <shawn...@kernel.org>
> Cc: Pengcheng Li <lpc...@hisilicon.com>
> Cc: Jianguo Sun <sunjiang...@huawei.com>
> Cc: Masahiro Yamada <yamada.masah...@socionext.com>
> Cc: Jiancheng Xue <xuejianch...@hisilicon.com>
> Cc: John Stultz <john.stu...@linaro.org>
> Cc: Binghui Wang <wangbing...@hisilicon.com>
> Signed-off-by: Yu Chen <cheny...@huawei.com>
> ---
>  MAINTAINERS                             |   2 +
>  drivers/phy/hisilicon/Kconfig           |  10 ++
>  drivers/phy/hisilicon/Makefile          |   1 +
>  drivers/phy/hisilicon/phy-hi3660-usb3.c | 254 
> ++++++++++++++++++++++++++++++++
>  4 files changed, 267 insertions(+)
>  create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e485449f7811..7adf167588ee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15294,7 +15294,9 @@ M:    Binghui Wang <wangbing...@hisilicon.com>
>  L:   linux-usb@vger.kernel.org
>  S:   Maintained
>  F:   Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> +F:   Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
>  F:   drivers/usb/dwc3/dwc3-hisi.c
> +F:   drivers/phy/hisilicon/phy-hi3660-usb3.c
>  
>  USB ISP116X DRIVER
>  M:   Olav Kongas <o...@artecdesign.ee>
> diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
> index b40ee54a1a50..3c142f08987c 100644
> --- a/drivers/phy/hisilicon/Kconfig
> +++ b/drivers/phy/hisilicon/Kconfig
> @@ -12,6 +12,16 @@ config PHY_HI6220_USB
>  
>         To compile this driver as a module, choose M here.
>  
> +config PHY_HI3660_USB
> +     tristate "hi3660 USB PHY support"
> +     depends on (ARCH_HISI && ARM64) || COMPILE_TEST
> +     select GENERIC_PHY
> +     select MFD_SYSCON
> +     help
> +       Enable this to support the HISILICON HI3660 USB PHY.
> +
> +       To compile this driver as a module, choose M here.
> +
>  config PHY_HISTB_COMBPHY
>       tristate "HiSilicon STB SoCs COMBPHY support"
>       depends on (ARCH_HISI && ARM64) || COMPILE_TEST
> diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
> index f662a4fe18d8..75ba64e2faf8 100644
> --- a/drivers/phy/hisilicon/Makefile
> +++ b/drivers/phy/hisilicon/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_PHY_HI6220_USB)         += phy-hi6220-usb.o
> +obj-$(CONFIG_PHY_HI3660_USB)         += phy-hi3660-usb3.o
>  obj-$(CONFIG_PHY_HISTB_COMBPHY)              += phy-histb-combphy.o
>  obj-$(CONFIG_PHY_HISI_INNO_USB2)     += phy-hisi-inno-usb2.o
>  obj-$(CONFIG_PHY_HIX5HD2_SATA)               += phy-hix5hd2-sata.o
> diff --git a/drivers/phy/hisilicon/phy-hi3660-usb3.c 
> b/drivers/phy/hisilicon/phy-hi3660-usb3.c
> new file mode 100644
> index 000000000000..041c542750e2
> --- /dev/null
> +++ b/drivers/phy/hisilicon/phy-hi3660-usb3.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Phy provider for USB 3.0 controller on HiSilicon 3660 platform
> + *
> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
> + *           http://www.huawei.com
> + *
> + * Authors: Yu Chen <cheny...@huawei.com>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License 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.
> + */

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

Reply via email to