All of lore.kernel.org
 help / color / mirror / Atom feed
From: peter.chen@freescale.com (Peter Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
Date: Mon, 9 Feb 2015 10:11:35 +0800	[thread overview]
Message-ID: <20150209021133.GB3045@shlinux2> (raw)
In-Reply-To: <1423284966-3485-5-git-send-email-zhangfei.gao@linaro.org>

On Sat, Feb 07, 2015 at 12:56:06PM +0800, Zhangfei Gao wrote:
> Add usb phy controller for hi6220 platform
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>  drivers/usb/phy/Kconfig          |   9 ++
>  drivers/usb/phy/Makefile         |   1 +
>  drivers/usb/phy/phy-hi6220-usb.c | 297 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 307 insertions(+)
>  create mode 100644 drivers/usb/phy/phy-hi6220-usb.c
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index c6d0c8e..405a3d0 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -173,6 +173,15 @@ config USB_MXS_PHY
>  
>  	  MXS Phy is used by some of the i.MX SoCs, for example imx23/28/6x.
>  
> +config USB_HI6220_PHY
> +	tristate "hi6220 USB PHY support"
> +	select USB_PHY
> +	select MFD_SYSCON
> +	help
> +	  Enable this to support the HISILICON HI6220 USB PHY.
> +
> +	  To compile this driver as a module, choose M here.
> +
>  config USB_RCAR_PHY
>  	tristate "Renesas R-Car USB PHY support"
>  	depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 75f2bba..00172d3 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
>  obj-$(CONFIG_TWL6030_USB)		+= phy-twl6030-usb.o
>  obj-$(CONFIG_USB_EHCI_TEGRA)		+= phy-tegra-usb.o
>  obj-$(CONFIG_USB_GPIO_VBUS)		+= phy-gpio-vbus-usb.o
> +obj-$(CONFIG_USB_HI6220_PHY)		+= phy-hi6220-usb.o
>  obj-$(CONFIG_USB_ISP1301)		+= phy-isp1301.o
>  obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
>  obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
> diff --git a/drivers/usb/phy/phy-hi6220-usb.c b/drivers/usb/phy/phy-hi6220-usb.c
> new file mode 100644
> index 0000000..8092bca
> --- /dev/null
> +++ b/drivers/usb/phy/phy-hi6220-usb.c
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2015 Linaro Ltd.
> + * Copyright (c) 2015 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/otg.h>
> +
> +#define SC_PERIPH_CTRL4			0x00c
> +
> +#define CTRL4_PICO_SIDDQ		BIT(6)
> +#define CTRL4_PICO_OGDISABLE		BIT(8)
> +#define CTRL4_PICO_VBUSVLDEXT		BIT(10)
> +#define CTRL4_PICO_VBUSVLDEXTSEL	BIT(11)
> +#define CTRL4_OTG_PHY_SEL		BIT(21)
> +
> +#define SC_PERIPH_CTRL5			0x010
> +
> +#define CTRL5_USBOTG_RES_SEL		BIT(3)
> +#define CTRL5_PICOPHY_ACAENB		BIT(4)
> +#define CTRL5_PICOPHY_BC_MODE		BIT(5)
> +#define CTRL5_PICOPHY_CHRGSEL		BIT(6)
> +#define CTRL5_PICOPHY_VDATSRCEND	BIT(7)
> +#define CTRL5_PICOPHY_VDATDETENB	BIT(8)
> +#define CTRL5_PICOPHY_DCDENB		BIT(9)
> +#define CTRL5_PICOPHY_IDDIG		BIT(10)
> +
> +#define SC_PERIPH_CTRL8			0x018
> +#define SC_PERIPH_RSTEN0		0x300
> +#define SC_PERIPH_RSTDIS0		0x304
> +
> +#define RST0_USBOTG_BUS			BIT(4)
> +#define RST0_POR_PICOPHY		BIT(5)
> +#define RST0_USBOTG			BIT(6)
> +#define RST0_USBOTG_32K			BIT(7)
> +
> +#define EYE_PATTERN_PARA		0x7053348c
> +
> +struct hi6220_priv {
> +	struct usb_phy phy;
> +	struct delayed_work work;
> +	struct regmap *reg;
> +	struct clk *clk;
> +	struct regulator *vcc;
> +	int gpio_vbus;
> +	int gpio_id;
> +	enum usb_otg_state state;
> +};
> +
> +static void hi6220_start_periphrals(struct hi6220_priv *priv, bool on)
> +{
> +	struct usb_otg *otg = priv->phy.otg;
> +
> +	if (!otg->gadget)
> +		return;
> +
> +	if (on)
> +		usb_gadget_connect(otg->gadget);
> +	else
> +		usb_gadget_disconnect(otg->gadget);
> +}
> +
> +static void hi6220_detect_work(struct work_struct *work)
> +{
> +	struct hi6220_priv *priv =
> +		container_of(work, struct hi6220_priv, work.work);
> +	int gpio_id, gpio_vubs;

%s/gpio_vubs/gpio_vbus

> +	enum usb_otg_state state;
> +
> +	if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
> +		return;
> +
> +	gpio_id = gpio_get_value_cansleep(priv->gpio_id);
> +	gpio_vubs = gpio_get_value_cansleep(priv->gpio_vbus);
> +
> +	if (gpio_vubs == 0) {
> +		if (gpio_id == 1)
> +			state = OTG_STATE_B_PERIPHERAL;
> +		else
> +			state = OTG_STATE_A_HOST;
> +	} else {
> +		state = OTG_STATE_A_HOST;
> +	}
> +
> +	if (priv->state != state) {
> +		hi6220_start_periphrals(priv, state == OTG_STATE_B_PERIPHERAL);
> +		priv->state = state;
> +	}
> +}
> +
> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
> +{
> +	struct hi6220_priv *priv = (struct hi6220_priv *)data;
> +
> +	/* add debounce time */
> +	schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> +	return IRQ_HANDLED;
> +}
> +
> +static int hi6220_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
> +{
> +	otg->gadget = gadget;
> +	return 0;
> +}
> +
> +static void hi6220_phy_setup(struct hi6220_priv *priv, bool on)
> +{
> +	struct regmap *reg = priv->reg;
> +	u32 val, mask;
> +	int ret;
> +
> +	if (priv->reg == NULL)
> +		return;
> +
> +	if (on) {
> +		val = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> +		      RST0_USBOTG | RST0_USBOTG_32K;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_RSTDIS0, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_read(reg, SC_PERIPH_CTRL5, &val);
> +		val = CTRL5_USBOTG_RES_SEL | CTRL5_PICOPHY_ACAENB;
> +		mask = val | CTRL5_PICOPHY_BC_MODE;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL5, mask, val);
> +		if (ret)
> +			return;
> +
> +		val =  CTRL4_PICO_VBUSVLDEXT | CTRL4_PICO_VBUSVLDEXTSEL |
> +		       CTRL4_OTG_PHY_SEL;
> +		mask = val | CTRL4_PICO_SIDDQ | CTRL4_PICO_OGDISABLE;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL4, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_write(reg, SC_PERIPH_CTRL8, EYE_PATTERN_PARA);
> +		if (ret)
> +			return;
> +	} else {
> +		val = CTRL4_PICO_SIDDQ;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL4, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_read(reg, SC_PERIPH_CTRL4, &val);
> +
> +		val = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> +		      RST0_USBOTG | RST0_USBOTG_32K;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_RSTEN0, mask, val);
> +		if (ret)
> +			return;
> +	}

You have return value check for regmap API, but no error message or
return value for hi6220_phy_setup, it looks strange.

> +}
> +
> +static int hi6220_phy_probe(struct platform_device *pdev)
> +{
> +	struct hi6220_priv *priv;
> +	struct usb_otg *otg;
> +	struct device_node *np = pdev->dev.of_node;
> +	int ret, irq;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> +	if (!otg)
> +		return -ENOMEM;
> +
> +	priv->phy.dev = &pdev->dev;
> +	priv->phy.otg = otg;
> +	priv->phy.label = "hi6220";
> +	priv->phy.type = USB_PHY_TYPE_USB2;
> +	otg->set_peripheral = hi6220_set_peripheral;
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->gpio_vbus = of_get_named_gpio(np, "hisilicon,gpio-vbus", 0);
> +	if (priv->gpio_vbus == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	if (!gpio_is_valid(priv->gpio_vbus)) {
> +		dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_vbus);
> +		return -ENODEV;
> +	}
> +
> +	priv->gpio_id = of_get_named_gpio(np, "hisilicon,gpio-id", 0);
> +	if (priv->gpio_id == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	if (!gpio_is_valid(priv->gpio_id)) {
> +		dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_id);
> +		return -ENODEV;
> +	}
> +
> +	priv->reg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +					"hisilicon,peripheral-syscon");
> +	if (IS_ERR(priv->reg))
> +		priv->reg = NULL;
> +

see my comments at your v1.

> +	INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
> +
> +	ret = devm_gpio_request_one(&pdev->dev, priv->gpio_vbus,
> +				    GPIOF_IN, "gpio_vbus");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "gpio request failed for gpio_vbus\n");
> +		return ret;
> +	}
> +
> +	ret = devm_gpio_request_one(&pdev->dev, priv->gpio_id,
> +				    GPIOF_IN, "gpio_id");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "gpio request failed for gpio_id\n");
> +		return ret;
> +	}
> +
> +	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> +	if (!IS_ERR(priv->vcc)) {

EPROBE_DEFER?

> +		ret = regulator_enable(priv->vcc);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to enable regulator\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		regulator_disable(priv->vcc);
> +		return PTR_ERR(priv->clk);
> +	}
> +	clk_prepare_enable(priv->clk);
> +
> +	irq = gpio_to_irq(priv->gpio_vbus);
> +	ret = devm_request_irq(&pdev->dev, gpio_to_irq(priv->gpio_vbus),
> +			       hiusb_gpio_intr, IRQF_NO_SUSPEND |
> +			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +			       "vbus_gpio_intr", priv);
> +	if (ret) {
> +		dev_err(&pdev->dev, "request gpio irq failed.\n");
> +		goto err_irq;
> +	}
> +
> +	hi6220_phy_setup(priv, true);
> +	ret = usb_add_phy_dev(&priv->phy);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't register transceiver\n");
> +		goto err_irq;
> +	}
> +	schedule_delayed_work(&priv->work, 0);
> +
> +	return 0;
> +err_irq:
> +	clk_disable_unprepare(priv->clk);
> +	regulator_disable(priv->vcc);
> +	return ret;
> +}
> +
> +static int hi6220_phy_remove(struct platform_device *pdev)
> +{
> +	struct hi6220_priv *priv = platform_get_drvdata(pdev);
> +
> +	hi6220_phy_setup(priv, false);
> +	clk_disable_unprepare(priv->clk);
> +	regulator_disable(priv->vcc);
> +	return 0;
> +}
> +
> +static const struct of_device_id hi6220_phy_of_match[] = {
> +	{.compatible = "hisilicon,hi6220-usb-phy",},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, hi6220_phy_of_match);
> +
> +static struct platform_driver hi6220_phy_driver = {
> +	.probe	= hi6220_phy_probe,
> +	.remove	= hi6220_phy_remove,
> +	.driver = {
> +		.name	= "hi6220-usb-phy",
> +		.of_match_table	= hi6220_phy_of_match,
> +	}
> +};
> +module_platform_driver(hi6220_phy_driver);
> +
> +MODULE_DESCRIPTION("HISILICON HI6220 USB PHY driver");
> +MODULE_ALIAS("platform:hi6220-usb-phy");
> +MODULE_LICENSE("GPL");
> -- 
> 1.9.1
> 

-- 

Best Regards,
Peter Chen

WARNING: multiple messages have this Message-ID (diff)
From: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: balbi-l0cyMroinI0@public.gmane.org,
	john.youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	Mian Yousaf Kaukab
	<yousaf.kaukab-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"dan . zhao" <dan.zhao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	Sergei Shtylyov
	<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
Date: Mon, 9 Feb 2015 10:11:35 +0800	[thread overview]
Message-ID: <20150209021133.GB3045@shlinux2> (raw)
In-Reply-To: <1423284966-3485-5-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Sat, Feb 07, 2015 at 12:56:06PM +0800, Zhangfei Gao wrote:
> Add usb phy controller for hi6220 platform
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/usb/phy/Kconfig          |   9 ++
>  drivers/usb/phy/Makefile         |   1 +
>  drivers/usb/phy/phy-hi6220-usb.c | 297 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 307 insertions(+)
>  create mode 100644 drivers/usb/phy/phy-hi6220-usb.c
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index c6d0c8e..405a3d0 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -173,6 +173,15 @@ config USB_MXS_PHY
>  
>  	  MXS Phy is used by some of the i.MX SoCs, for example imx23/28/6x.
>  
> +config USB_HI6220_PHY
> +	tristate "hi6220 USB PHY support"
> +	select USB_PHY
> +	select MFD_SYSCON
> +	help
> +	  Enable this to support the HISILICON HI6220 USB PHY.
> +
> +	  To compile this driver as a module, choose M here.
> +
>  config USB_RCAR_PHY
>  	tristate "Renesas R-Car USB PHY support"
>  	depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 75f2bba..00172d3 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
>  obj-$(CONFIG_TWL6030_USB)		+= phy-twl6030-usb.o
>  obj-$(CONFIG_USB_EHCI_TEGRA)		+= phy-tegra-usb.o
>  obj-$(CONFIG_USB_GPIO_VBUS)		+= phy-gpio-vbus-usb.o
> +obj-$(CONFIG_USB_HI6220_PHY)		+= phy-hi6220-usb.o
>  obj-$(CONFIG_USB_ISP1301)		+= phy-isp1301.o
>  obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
>  obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
> diff --git a/drivers/usb/phy/phy-hi6220-usb.c b/drivers/usb/phy/phy-hi6220-usb.c
> new file mode 100644
> index 0000000..8092bca
> --- /dev/null
> +++ b/drivers/usb/phy/phy-hi6220-usb.c
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2015 Linaro Ltd.
> + * Copyright (c) 2015 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/otg.h>
> +
> +#define SC_PERIPH_CTRL4			0x00c
> +
> +#define CTRL4_PICO_SIDDQ		BIT(6)
> +#define CTRL4_PICO_OGDISABLE		BIT(8)
> +#define CTRL4_PICO_VBUSVLDEXT		BIT(10)
> +#define CTRL4_PICO_VBUSVLDEXTSEL	BIT(11)
> +#define CTRL4_OTG_PHY_SEL		BIT(21)
> +
> +#define SC_PERIPH_CTRL5			0x010
> +
> +#define CTRL5_USBOTG_RES_SEL		BIT(3)
> +#define CTRL5_PICOPHY_ACAENB		BIT(4)
> +#define CTRL5_PICOPHY_BC_MODE		BIT(5)
> +#define CTRL5_PICOPHY_CHRGSEL		BIT(6)
> +#define CTRL5_PICOPHY_VDATSRCEND	BIT(7)
> +#define CTRL5_PICOPHY_VDATDETENB	BIT(8)
> +#define CTRL5_PICOPHY_DCDENB		BIT(9)
> +#define CTRL5_PICOPHY_IDDIG		BIT(10)
> +
> +#define SC_PERIPH_CTRL8			0x018
> +#define SC_PERIPH_RSTEN0		0x300
> +#define SC_PERIPH_RSTDIS0		0x304
> +
> +#define RST0_USBOTG_BUS			BIT(4)
> +#define RST0_POR_PICOPHY		BIT(5)
> +#define RST0_USBOTG			BIT(6)
> +#define RST0_USBOTG_32K			BIT(7)
> +
> +#define EYE_PATTERN_PARA		0x7053348c
> +
> +struct hi6220_priv {
> +	struct usb_phy phy;
> +	struct delayed_work work;
> +	struct regmap *reg;
> +	struct clk *clk;
> +	struct regulator *vcc;
> +	int gpio_vbus;
> +	int gpio_id;
> +	enum usb_otg_state state;
> +};
> +
> +static void hi6220_start_periphrals(struct hi6220_priv *priv, bool on)
> +{
> +	struct usb_otg *otg = priv->phy.otg;
> +
> +	if (!otg->gadget)
> +		return;
> +
> +	if (on)
> +		usb_gadget_connect(otg->gadget);
> +	else
> +		usb_gadget_disconnect(otg->gadget);
> +}
> +
> +static void hi6220_detect_work(struct work_struct *work)
> +{
> +	struct hi6220_priv *priv =
> +		container_of(work, struct hi6220_priv, work.work);
> +	int gpio_id, gpio_vubs;

%s/gpio_vubs/gpio_vbus

> +	enum usb_otg_state state;
> +
> +	if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
> +		return;
> +
> +	gpio_id = gpio_get_value_cansleep(priv->gpio_id);
> +	gpio_vubs = gpio_get_value_cansleep(priv->gpio_vbus);
> +
> +	if (gpio_vubs == 0) {
> +		if (gpio_id == 1)
> +			state = OTG_STATE_B_PERIPHERAL;
> +		else
> +			state = OTG_STATE_A_HOST;
> +	} else {
> +		state = OTG_STATE_A_HOST;
> +	}
> +
> +	if (priv->state != state) {
> +		hi6220_start_periphrals(priv, state == OTG_STATE_B_PERIPHERAL);
> +		priv->state = state;
> +	}
> +}
> +
> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
> +{
> +	struct hi6220_priv *priv = (struct hi6220_priv *)data;
> +
> +	/* add debounce time */
> +	schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> +	return IRQ_HANDLED;
> +}
> +
> +static int hi6220_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
> +{
> +	otg->gadget = gadget;
> +	return 0;
> +}
> +
> +static void hi6220_phy_setup(struct hi6220_priv *priv, bool on)
> +{
> +	struct regmap *reg = priv->reg;
> +	u32 val, mask;
> +	int ret;
> +
> +	if (priv->reg == NULL)
> +		return;
> +
> +	if (on) {
> +		val = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> +		      RST0_USBOTG | RST0_USBOTG_32K;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_RSTDIS0, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_read(reg, SC_PERIPH_CTRL5, &val);
> +		val = CTRL5_USBOTG_RES_SEL | CTRL5_PICOPHY_ACAENB;
> +		mask = val | CTRL5_PICOPHY_BC_MODE;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL5, mask, val);
> +		if (ret)
> +			return;
> +
> +		val =  CTRL4_PICO_VBUSVLDEXT | CTRL4_PICO_VBUSVLDEXTSEL |
> +		       CTRL4_OTG_PHY_SEL;
> +		mask = val | CTRL4_PICO_SIDDQ | CTRL4_PICO_OGDISABLE;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL4, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_write(reg, SC_PERIPH_CTRL8, EYE_PATTERN_PARA);
> +		if (ret)
> +			return;
> +	} else {
> +		val = CTRL4_PICO_SIDDQ;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_CTRL4, mask, val);
> +		if (ret)
> +			return;
> +
> +		ret = regmap_read(reg, SC_PERIPH_CTRL4, &val);
> +
> +		val = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> +		      RST0_USBOTG | RST0_USBOTG_32K;
> +		mask = val;
> +		ret = regmap_update_bits(reg, SC_PERIPH_RSTEN0, mask, val);
> +		if (ret)
> +			return;
> +	}

You have return value check for regmap API, but no error message or
return value for hi6220_phy_setup, it looks strange.

> +}
> +
> +static int hi6220_phy_probe(struct platform_device *pdev)
> +{
> +	struct hi6220_priv *priv;
> +	struct usb_otg *otg;
> +	struct device_node *np = pdev->dev.of_node;
> +	int ret, irq;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> +	if (!otg)
> +		return -ENOMEM;
> +
> +	priv->phy.dev = &pdev->dev;
> +	priv->phy.otg = otg;
> +	priv->phy.label = "hi6220";
> +	priv->phy.type = USB_PHY_TYPE_USB2;
> +	otg->set_peripheral = hi6220_set_peripheral;
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->gpio_vbus = of_get_named_gpio(np, "hisilicon,gpio-vbus", 0);
> +	if (priv->gpio_vbus == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	if (!gpio_is_valid(priv->gpio_vbus)) {
> +		dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_vbus);
> +		return -ENODEV;
> +	}
> +
> +	priv->gpio_id = of_get_named_gpio(np, "hisilicon,gpio-id", 0);
> +	if (priv->gpio_id == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	if (!gpio_is_valid(priv->gpio_id)) {
> +		dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_id);
> +		return -ENODEV;
> +	}
> +
> +	priv->reg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +					"hisilicon,peripheral-syscon");
> +	if (IS_ERR(priv->reg))
> +		priv->reg = NULL;
> +

see my comments at your v1.

> +	INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
> +
> +	ret = devm_gpio_request_one(&pdev->dev, priv->gpio_vbus,
> +				    GPIOF_IN, "gpio_vbus");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "gpio request failed for gpio_vbus\n");
> +		return ret;
> +	}
> +
> +	ret = devm_gpio_request_one(&pdev->dev, priv->gpio_id,
> +				    GPIOF_IN, "gpio_id");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "gpio request failed for gpio_id\n");
> +		return ret;
> +	}
> +
> +	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> +	if (!IS_ERR(priv->vcc)) {

EPROBE_DEFER?

> +		ret = regulator_enable(priv->vcc);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to enable regulator\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		regulator_disable(priv->vcc);
> +		return PTR_ERR(priv->clk);
> +	}
> +	clk_prepare_enable(priv->clk);
> +
> +	irq = gpio_to_irq(priv->gpio_vbus);
> +	ret = devm_request_irq(&pdev->dev, gpio_to_irq(priv->gpio_vbus),
> +			       hiusb_gpio_intr, IRQF_NO_SUSPEND |
> +			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +			       "vbus_gpio_intr", priv);
> +	if (ret) {
> +		dev_err(&pdev->dev, "request gpio irq failed.\n");
> +		goto err_irq;
> +	}
> +
> +	hi6220_phy_setup(priv, true);
> +	ret = usb_add_phy_dev(&priv->phy);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Can't register transceiver\n");
> +		goto err_irq;
> +	}
> +	schedule_delayed_work(&priv->work, 0);
> +
> +	return 0;
> +err_irq:
> +	clk_disable_unprepare(priv->clk);
> +	regulator_disable(priv->vcc);
> +	return ret;
> +}
> +
> +static int hi6220_phy_remove(struct platform_device *pdev)
> +{
> +	struct hi6220_priv *priv = platform_get_drvdata(pdev);
> +
> +	hi6220_phy_setup(priv, false);
> +	clk_disable_unprepare(priv->clk);
> +	regulator_disable(priv->vcc);
> +	return 0;
> +}
> +
> +static const struct of_device_id hi6220_phy_of_match[] = {
> +	{.compatible = "hisilicon,hi6220-usb-phy",},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, hi6220_phy_of_match);
> +
> +static struct platform_driver hi6220_phy_driver = {
> +	.probe	= hi6220_phy_probe,
> +	.remove	= hi6220_phy_remove,
> +	.driver = {
> +		.name	= "hi6220-usb-phy",
> +		.of_match_table	= hi6220_phy_of_match,
> +	}
> +};
> +module_platform_driver(hi6220_phy_driver);
> +
> +MODULE_DESCRIPTION("HISILICON HI6220 USB PHY driver");
> +MODULE_ALIAS("platform:hi6220-usb-phy");
> +MODULE_LICENSE("GPL");
> -- 
> 1.9.1
> 

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-02-09  2:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-07  4:56 [PATCH v2 0/4] add usb support for hi6220 Zhangfei Gao
2015-02-07  4:56 ` Zhangfei Gao
2015-02-07  4:56 ` [PATCH v2 1/4] Documentation: dt-bindings: add dt binding info for hi6220 dwc2 Zhangfei Gao
2015-02-07  4:56   ` Zhangfei Gao
2015-02-07  4:56 ` [PATCH v2 2/4] Documentation: dt-bindings: add dt binding info for hi6220 Zhangfei Gao
2015-02-07  4:56   ` Zhangfei Gao
2015-02-07  4:56 ` [PATCH v2 3/4] usb: dwc2: platform: add hi6220 support Zhangfei Gao
2015-02-07  4:56   ` Zhangfei Gao
2015-02-07  4:56 ` [PATCH v2 4/4] usb: phy: add phy-hi6220-usb Zhangfei Gao
2015-02-07  4:56   ` Zhangfei Gao
2015-02-09  2:11   ` Peter Chen [this message]
2015-02-09  2:11     ` Peter Chen
2015-02-09 14:26     ` zhangfei
2015-02-09 14:26       ` zhangfei
2015-02-09 16:00       ` Zhangfei Gao
2015-02-09 16:00         ` Zhangfei Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150209021133.GB3045@shlinux2 \
    --to=peter.chen@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.