Re: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
On 9 February 2015 at 22:26, zhangfei zhangfei@linaro.org wrote:
On 02/09/2015 10:11 AM, Peter Chen wrote:
+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
Yes, typo
+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.
There was dev_err(priv-dev, failed to setup phy\n);
Then I found priv-dev is the only one place to use, so I remove this for
simple.
+}
+
+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.
As replied in v1, EPROBE_DEFER does not needed.
syscon is register far earlier.
+ 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?
No, this is not needed, since regulator is registered earlier than device.
drivers/Makefile
# regulators early, since some subsystems
Re: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
On 02/09/2015 10:11 AM, Peter Chen wrote:
+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
Yes, typo
+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.
There was dev_err(priv-dev, failed to setup phy\n);
Then I found priv-dev is the only one place to use, so I remove this
for simple.
+}
+
+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.
As replied in v1, EPROBE_DEFER does not needed.
syscon is register far earlier.
+ 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?
No, this is not needed, since regulator is registered earlier than device.
drivers/Makefile
# regulators early, since some subsystems rely on them to initialize
obj-$(CONFIG_REGULATOR) += regulator/
EPROBE_DEFER should be the last option we rely on.
Thanks
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v2 4/4] usb: phy: add phy-hi6220-usb
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@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 000..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_VBUSVLDEXTBIT(10)
+#define CTRL4_PICO_VBUSVLDEXTSEL BIT(11)
+#define CTRL4_OTG_PHY_SELBIT(21)
+
+#define SC_PERIPH_CTRL5 0x010
+
+#define CTRL5_USBOTG_RES_SEL BIT(3)
+#define CTRL5_PICOPHY_ACAENB BIT(4)
+#define CTRL5_PICOPHY_BC_MODEBIT(5)
+#define CTRL5_PICOPHY_CHRGSELBIT(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_RSTDIS00x304
+
+#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) {
+
[PATCH v2 4/4] usb: phy: add phy-hi6220-usb
Add usb phy controller for hi6220 platform
Signed-off-by: Zhangfei Gao zhangfei@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 000..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_CTRL40x00c
+
+#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_CTRL50x010
+
+#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_IDDIGBIT(10)
+
+#define SC_PERIPH_CTRL80x018
+#define SC_PERIPH_RSTEN0 0x300
+#define SC_PERIPH_RSTDIS0 0x304
+
+#define RST0_USBOTG_BUSBIT(4)
+#define RST0_POR_PICOPHY BIT(5)
+#define RST0_USBOTGBIT(6)
+#define RST0_USBOTG_32KBIT(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;
+ 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
