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