| Message ID | 20190304103846.2060-9-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: meson: Add support for USB on Amlogic G12A | expand |
Hi, On Mon, 2019-03-04 at 11:38 +0100, Neil Armstrong wrote: > Adds support for Amlogic G12A USB Control Glue HW. > > The Amlogic G12A SoC Family embeds 2 USB Controllers : > - a DWC3 IP configured as Host for USB2 and USB3 > - a DWC2 IP configured as Peripheral USB2 Only > > A glue connects these both controllers to 2 USB2 PHYs, and optionnally > to an USB3+PCIE Combo PHY shared with the PCIE controller. > > The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including > routing of the OTG PHY between the DWC3 and DWC2 controllers, and > setups the on-chip OTG mode selection for this PHY. > > This drivers supports the on-probe setup of the OTG mode, and manually > via a debugfs interface. The IRQ mode change detect is yet to be added > in a future patchset, mainly due to lack of hardware to validate on. > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/usb/dwc3/Kconfig | 10 + > drivers/usb/dwc3/Makefile | 1 + > drivers/usb/dwc3/dwc3-meson-g12a.c | 601 +++++++++++++++++++++++++++++ > 3 files changed, 612 insertions(+) > create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index 1a0404fda596..21ce7368d325 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -93,6 +93,16 @@ config USB_DWC3_KEYSTONE > Support of USB2/3 functionality in TI Keystone2 platforms. > Say 'Y' or 'M' here if you have one such device > > +config USB_DWC3_MESON_G12A > + tristate "Amlogic Meson G12A Platforms" > + depends on OF && COMMON_CLK > + depends on ARCH_MESON || COMPILE_TEST > + default USB_DWC3 > + select USB_ROLE_SWITCH > + help > + Support USB2/3 functionality in Amlogic G12A platforms. > + Say 'Y' or 'M' if you have one such device. > + > config USB_DWC3_OF_SIMPLE > tristate "Generic OF Simple Glue Layer" > depends on OF && COMMON_CLK > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile > index 6e3ef6144e5d..ae86da0dc5bd 100644 > --- a/drivers/usb/dwc3/Makefile > +++ b/drivers/usb/dwc3/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o > obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o > obj-$(CONFIG_USB_DWC3_HAPS) += dwc3-haps.o > obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o > +obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o > obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o > obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o > obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c > new file mode 100644 > index 000000000000..75942614a034 > --- /dev/null > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c > @@ -0,0 +1,601 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * USB Glue for Amlogic G12A SoCs > + * > + * Copyright (c) 2019 BayLibre, SAS > + * Author: Neil Armstrong <narmstrong@baylibre.com> > + */ > + > +/* > + * The USB is organized with a glue around the DWC3 Controller IP as : > + * - Control registers for each USB2 Ports > + * - Control registers for the USB PHY layer > + * - SuperSpeed PHY can be enabled only if port is used > + * > + * TOFIX: > + * - Add dynamic OTG switching with ID change interrupt > + */ > + [...] > + > +static int dwc3_meson_g12a_probe(struct platform_device *pdev) > +{ > + struct dwc3_meson_g12a *priv; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + void __iomem *base; > + struct resource *res; > + enum phy_mode otg_id; > + int ret, i; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + priv->regmap = devm_regmap_init_mmio(dev, base, > + &phy_meson_g12a_usb3_regmap_conf); > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + priv->vbus = devm_regulator_get_optional(dev, "vbus"); > + if (IS_ERR(priv->vbus)) { > + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) > + return PTR_ERR(priv->vbus); > + priv->vbus = NULL; > + } > + > + priv->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(priv->clk)) > + return PTR_ERR(priv->clk); > + > + ret = clk_prepare_enable(priv->clk); > + if (ret) > + return ret; > + > + platform_set_drvdata(pdev, priv); > + priv->dev = dev; > + > + priv->reset = devm_reset_control_get(dev, NULL); > + if (IS_ERR(priv->reset)) { > + ret = PTR_ERR(priv->reset); > + dev_err(dev, "failed to get device reset, err=%d\n", ret); > + return ret; > + } > + > + ret = reset_control_reset(priv->reset); > + if (ret) > + return ret; > + > + ret = dwc3_meson_g12a_get_phys(priv); > + if (ret) > + return ret; > + > + ret = regulator_enable(priv->vbus); > + if (ret) > + return ret; > + > + /* Get dr_mode */ > + priv->otg_mode = usb_get_dr_mode(dev); > + > + dwc3_meson_g12a_usb_init(priv); > + > + /* Set PHY Power */ > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + ret = phy_power_on(priv->phys[i]); > + if (ret) > + goto err_phys_put; > + } > + > + /* Init PHYs */ > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + ret = phy_init(priv->phys[i]); > + if (ret) > + goto err_phys_power; > + } usually phy_init() first > + > + ret = of_platform_populate(np, NULL, NULL, dev); > + if (ret) { > + clk_disable_unprepare(priv->clk); > + clk_put(priv->clk); > + > + goto err_phys_exit; > + } > + > + /* Setup OTG mode corresponding to the ID pin */ > + if (priv->otg_mode == USB_DR_MODE_OTG) { > + /* TOFIX Handle ID mode toggling via IRQ */ > + otg_id = dwc3_meson_g12a_get_id(priv); > + if (otg_id != priv->otg_phy_mode) { > + if (dwc3_meson_g12a_otg_mode_set(priv, otg_id)) > + dev_warn(dev, "Failed to switch OTG mode\n"); > + } > + } > + > + /* Setup role switcher */ > + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, > + "snps,dwc3"); > + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); > + priv->switch_desc.allow_userspace_control = true; > + priv->switch_desc.set = dwc3_meson_g12a_role_set; > + priv->switch_desc.get = dwc3_meson_g12a_role_get; to me, use a local variable for switch_desc > + > + priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc); > + if (IS_ERR(priv->role_switch)) > + dev_warn(dev, "Unable to register Role Switch\n"); > + > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_get_sync(dev); > + > + return 0; > + > +err_phys_exit: > + for (i = 0 ; i < PHY_COUNT ; ++i) > + phy_exit(priv->phys[i]); > + > +err_phys_power: > + for (i = 0 ; i < PHY_COUNT ; ++i) > + phy_power_off(priv->phys[i]); > + > +err_phys_put: > + for (i = 0 ; i < PHY_COUNT ; ++i) > + phy_put(priv->phys[i]); > + The phys are got by devm_phy_optional_get(), no need call phy_put() > + return ret; > +} > + > +static int dwc3_meson_g12a_remove(struct platform_device *pdev) > +{ > + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); > + struct device *dev = &pdev->dev; > + int i; > + > + usb_role_switch_unregister(priv->role_switch); > + > + of_platform_depopulate(dev); > + > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + phy_power_off(priv->phys[i]); > + phy_exit(priv->phys[i]); > + phy_put(priv->phys[i]); ditto > + } > + > + clk_disable_unprepare(priv->clk); > + clk_put(priv->clk); > + > + pm_runtime_disable(dev); > + pm_runtime_put_noidle(dev); > + pm_runtime_set_suspended(dev); > + > + return 0; > +} > + > +static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + > + clk_disable(priv->clk); > + > + return 0; > +} > + > +static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + > + return clk_enable(priv->clk); > +} > + > +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + int i; > + > + for (i = 0 ; i < PHY_COUNT ; ++i) > + if (priv->phys[i]) > + phy_exit(priv->phys[i]); > + > + reset_control_assert(priv->reset); > + > + return 0; > +} > + > +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + int i, ret; > + > + reset_control_deassert(priv->reset); > + > + dwc3_meson_g12a_usb_init(priv); > + > + /* Init PHYs */ > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + if (priv->phys[i]) { > + ret = phy_init(priv->phys[i]); > + if (ret) > + return ret; > + } > + } > + > + return 0; > +} > + > +static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume) > + SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend, > + dwc3_meson_g12a_runtime_resume, NULL) > +}; > + > +static const struct of_device_id dwc3_meson_g12a_match[] = { > + { .compatible = "amlogic,meson-g12a-usb-ctrl" }, > + { /* Sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match); > + > +static struct platform_driver dwc3_meson_g12a_driver = { > + .probe = dwc3_meson_g12a_probe, > + .remove = dwc3_meson_g12a_remove, > + .driver = { > + .name = "dwc3-meson-g12a", > + .of_match_table = dwc3_meson_g12a_match, > + .pm = &dwc3_meson_g12a_dev_pm_ops, > + }, > +}; > + > +module_platform_driver(dwc3_meson_g12a_driver); > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer"); > +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
On 07/03/2019 03:02, Chunfeng Yun wrote: > Hi, > On Mon, 2019-03-04 at 11:38 +0100, Neil Armstrong wrote: >> Adds support for Amlogic G12A USB Control Glue HW. >> >> The Amlogic G12A SoC Family embeds 2 USB Controllers : >> - a DWC3 IP configured as Host for USB2 and USB3 >> - a DWC2 IP configured as Peripheral USB2 Only >> >> A glue connects these both controllers to 2 USB2 PHYs, and optionnally >> to an USB3+PCIE Combo PHY shared with the PCIE controller. >> >> The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including >> routing of the OTG PHY between the DWC3 and DWC2 controllers, and >> setups the on-chip OTG mode selection for this PHY. >> >> This drivers supports the on-probe setup of the OTG mode, and manually >> via a debugfs interface. The IRQ mode change detect is yet to be added >> in a future patchset, mainly due to lack of hardware to validate on. >> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >> --- >> drivers/usb/dwc3/Kconfig | 10 + >> drivers/usb/dwc3/Makefile | 1 + >> drivers/usb/dwc3/dwc3-meson-g12a.c | 601 +++++++++++++++++++++++++++++ >> 3 files changed, 612 insertions(+) >> create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c >> >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >> index 1a0404fda596..21ce7368d325 100644 >> --- a/drivers/usb/dwc3/Kconfig >> +++ b/drivers/usb/dwc3/Kconfig >> @@ -93,6 +93,16 @@ config USB_DWC3_KEYSTONE >> Support of USB2/3 functionality in TI Keystone2 platforms. >> Say 'Y' or 'M' here if you have one such device >> >> +config USB_DWC3_MESON_G12A >> + tristate "Amlogic Meson G12A Platforms" >> + depends on OF && COMMON_CLK >> + depends on ARCH_MESON || COMPILE_TEST >> + default USB_DWC3 >> + select USB_ROLE_SWITCH >> + help >> + Support USB2/3 functionality in Amlogic G12A platforms. >> + Say 'Y' or 'M' if you have one such device. >> + >> config USB_DWC3_OF_SIMPLE >> tristate "Generic OF Simple Glue Layer" >> depends on OF && COMMON_CLK >> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile >> index 6e3ef6144e5d..ae86da0dc5bd 100644 >> --- a/drivers/usb/dwc3/Makefile >> +++ b/drivers/usb/dwc3/Makefile >> @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o >> obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o >> obj-$(CONFIG_USB_DWC3_HAPS) += dwc3-haps.o >> obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o >> +obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o >> obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o >> obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o >> obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o >> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c >> new file mode 100644 >> index 000000000000..75942614a034 >> --- /dev/null >> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c >> @@ -0,0 +1,601 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * USB Glue for Amlogic G12A SoCs >> + * >> + * Copyright (c) 2019 BayLibre, SAS >> + * Author: Neil Armstrong <narmstrong@baylibre.com> >> + */ >> + >> +/* >> + * The USB is organized with a glue around the DWC3 Controller IP as : >> + * - Control registers for each USB2 Ports >> + * - Control registers for the USB PHY layer >> + * - SuperSpeed PHY can be enabled only if port is used >> + * >> + * TOFIX: >> + * - Add dynamic OTG switching with ID change interrupt >> + */ >> + > [...] >> + >> +static int dwc3_meson_g12a_probe(struct platform_device *pdev) >> +{ >> + struct dwc3_meson_g12a *priv; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; >> + void __iomem *base; >> + struct resource *res; >> + enum phy_mode otg_id; >> + int ret, i; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(base)) >> + return PTR_ERR(base); >> + >> + priv->regmap = devm_regmap_init_mmio(dev, base, >> + &phy_meson_g12a_usb3_regmap_conf); >> + if (IS_ERR(priv->regmap)) >> + return PTR_ERR(priv->regmap); >> + >> + priv->vbus = devm_regulator_get_optional(dev, "vbus"); >> + if (IS_ERR(priv->vbus)) { >> + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) >> + return PTR_ERR(priv->vbus); >> + priv->vbus = NULL; >> + } >> + >> + priv->clk = devm_clk_get(dev, NULL); >> + if (IS_ERR(priv->clk)) >> + return PTR_ERR(priv->clk); >> + >> + ret = clk_prepare_enable(priv->clk); >> + if (ret) >> + return ret; >> + >> + platform_set_drvdata(pdev, priv); >> + priv->dev = dev; >> + >> + priv->reset = devm_reset_control_get(dev, NULL); >> + if (IS_ERR(priv->reset)) { >> + ret = PTR_ERR(priv->reset); >> + dev_err(dev, "failed to get device reset, err=%d\n", ret); >> + return ret; >> + } >> + >> + ret = reset_control_reset(priv->reset); >> + if (ret) >> + return ret; >> + >> + ret = dwc3_meson_g12a_get_phys(priv); >> + if (ret) >> + return ret; >> + >> + ret = regulator_enable(priv->vbus); >> + if (ret) >> + return ret; >> + >> + /* Get dr_mode */ >> + priv->otg_mode = usb_get_dr_mode(dev); >> + >> + dwc3_meson_g12a_usb_init(priv); >> + >> + /* Set PHY Power */ >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + ret = phy_power_on(priv->phys[i]); >> + if (ret) >> + goto err_phys_put; >> + } >> + >> + /* Init PHYs */ >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + ret = phy_init(priv->phys[i]); >> + if (ret) >> + goto err_phys_power; >> + } > usually phy_init() first OK, i usually saw power_on before, but I'll swap them. > >> + >> + ret = of_platform_populate(np, NULL, NULL, dev); >> + if (ret) { >> + clk_disable_unprepare(priv->clk); >> + clk_put(priv->clk); >> + >> + goto err_phys_exit; >> + } >> + >> + /* Setup OTG mode corresponding to the ID pin */ >> + if (priv->otg_mode == USB_DR_MODE_OTG) { >> + /* TOFIX Handle ID mode toggling via IRQ */ >> + otg_id = dwc3_meson_g12a_get_id(priv); >> + if (otg_id != priv->otg_phy_mode) { >> + if (dwc3_meson_g12a_otg_mode_set(priv, otg_id)) >> + dev_warn(dev, "Failed to switch OTG mode\n"); >> + } >> + } >> + >> + /* Setup role switcher */ >> + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, >> + "snps,dwc3"); >> + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); >> + priv->switch_desc.allow_userspace_control = true; >> + priv->switch_desc.set = dwc3_meson_g12a_role_set; >> + priv->switch_desc.get = dwc3_meson_g12a_role_get; > to me, use a local variable for switch_desc I'm never fan of changing a local global variable for a driver instance, is it strictly required ? > >> + >> + priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc); >> + if (IS_ERR(priv->role_switch)) >> + dev_warn(dev, "Unable to register Role Switch\n"); >> + >> + pm_runtime_set_active(dev); >> + pm_runtime_enable(dev); >> + pm_runtime_get_sync(dev); >> + >> + return 0; >> + >> +err_phys_exit: >> + for (i = 0 ; i < PHY_COUNT ; ++i) >> + phy_exit(priv->phys[i]); >> + >> +err_phys_power: >> + for (i = 0 ; i < PHY_COUNT ; ++i) >> + phy_power_off(priv->phys[i]); >> + >> +err_phys_put: >> + for (i = 0 ; i < PHY_COUNT ; ++i) >> + phy_put(priv->phys[i]); >> + > The phys are got by devm_phy_optional_get(), no need call phy_put() You are right, this is a leftover from v1. >> + return ret; >> +} >> + >> +static int dwc3_meson_g12a_remove(struct platform_device *pdev) >> +{ >> + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); >> + struct device *dev = &pdev->dev; >> + int i; >> + >> + usb_role_switch_unregister(priv->role_switch); >> + >> + of_platform_depopulate(dev); >> + >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + phy_power_off(priv->phys[i]); >> + phy_exit(priv->phys[i]); >> + phy_put(priv->phys[i]); > ditto Thanks for the review, Neil >> + } >> + >> + clk_disable_unprepare(priv->clk); >> + clk_put(priv->clk); >> + >> + pm_runtime_disable(dev); >> + pm_runtime_put_noidle(dev); >> + pm_runtime_set_suspended(dev); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + >> + clk_disable(priv->clk); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + >> + return clk_enable(priv->clk); >> +} >> + >> +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + int i; >> + >> + for (i = 0 ; i < PHY_COUNT ; ++i) >> + if (priv->phys[i]) >> + phy_exit(priv->phys[i]); >> + >> + reset_control_assert(priv->reset); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + int i, ret; >> + >> + reset_control_deassert(priv->reset); >> + >> + dwc3_meson_g12a_usb_init(priv); >> + >> + /* Init PHYs */ >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + if (priv->phys[i]) { >> + ret = phy_init(priv->phys[i]); >> + if (ret) >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = { >> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume) >> + SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend, >> + dwc3_meson_g12a_runtime_resume, NULL) >> +}; >> + >> +static const struct of_device_id dwc3_meson_g12a_match[] = { >> + { .compatible = "amlogic,meson-g12a-usb-ctrl" }, >> + { /* Sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match); >> + >> +static struct platform_driver dwc3_meson_g12a_driver = { >> + .probe = dwc3_meson_g12a_probe, >> + .remove = dwc3_meson_g12a_remove, >> + .driver = { >> + .name = "dwc3-meson-g12a", >> + .of_match_table = dwc3_meson_g12a_match, >> + .pm = &dwc3_meson_g12a_dev_pm_ops, >> + }, >> +}; >> + >> +module_platform_driver(dwc3_meson_g12a_driver); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer"); >> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>"); > >
On Thu, 2019-03-07 at 10:45 +0100, Neil Armstrong wrote: > On 07/03/2019 03:02, Chunfeng Yun wrote: > > Hi, > > On Mon, 2019-03-04 at 11:38 +0100, Neil Armstrong wrote: > >> Adds support for Amlogic G12A USB Control Glue HW. > >> > >> The Amlogic G12A SoC Family embeds 2 USB Controllers : > >> - a DWC3 IP configured as Host for USB2 and USB3 > >> - a DWC2 IP configured as Peripheral USB2 Only > >> > >> A glue connects these both controllers to 2 USB2 PHYs, and optionnally > >> to an USB3+PCIE Combo PHY shared with the PCIE controller. > >> > >> The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including > >> routing of the OTG PHY between the DWC3 and DWC2 controllers, and > >> setups the on-chip OTG mode selection for this PHY. > >> > >> This drivers supports the on-probe setup of the OTG mode, and manually > >> via a debugfs interface. The IRQ mode change detect is yet to be added > >> in a future patchset, mainly due to lack of hardware to validate on. > >> > >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > >> --- > >> drivers/usb/dwc3/Kconfig | 10 + > >> drivers/usb/dwc3/Makefile | 1 + > >> drivers/usb/dwc3/dwc3-meson-g12a.c | 601 +++++++++++++++++++++++++++++ > >> 3 files changed, 612 insertions(+) > >> create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c > >> > >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > >> index 1a0404fda596..21ce7368d325 100644 > >> --- a/drivers/usb/dwc3/Kconfig > >> +++ b/drivers/usb/dwc3/Kconfig > >> @@ -93,6 +93,16 @@ config USB_DWC3_KEYSTONE > >> Support of USB2/3 functionality in TI Keystone2 platforms. > >> Say 'Y' or 'M' here if you have one such device > >> > >> +config USB_DWC3_MESON_G12A > >> + tristate "Amlogic Meson G12A Platforms" > >> + depends on OF && COMMON_CLK > >> + depends on ARCH_MESON || COMPILE_TEST > >> + default USB_DWC3 > >> + select USB_ROLE_SWITCH > >> + help > >> + Support USB2/3 functionality in Amlogic G12A platforms. > >> + Say 'Y' or 'M' if you have one such device. > >> + > >> config USB_DWC3_OF_SIMPLE > >> tristate "Generic OF Simple Glue Layer" > >> depends on OF && COMMON_CLK > >> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile > >> index 6e3ef6144e5d..ae86da0dc5bd 100644 > >> --- a/drivers/usb/dwc3/Makefile > >> +++ b/drivers/usb/dwc3/Makefile > >> @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o > >> obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o > >> obj-$(CONFIG_USB_DWC3_HAPS) += dwc3-haps.o > >> obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o > >> +obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o > >> obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o > >> obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o > >> obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o > >> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c > >> new file mode 100644 > >> index 000000000000..75942614a034 > >> --- /dev/null > >> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c > >> @@ -0,0 +1,601 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * USB Glue for Amlogic G12A SoCs > >> + * > >> + * Copyright (c) 2019 BayLibre, SAS > >> + * Author: Neil Armstrong <narmstrong@baylibre.com> > >> + */ > >> + > >> +/* > >> + * The USB is organized with a glue around the DWC3 Controller IP as : > >> + * - Control registers for each USB2 Ports > >> + * - Control registers for the USB PHY layer > >> + * - SuperSpeed PHY can be enabled only if port is used > >> + * > >> + * TOFIX: > >> + * - Add dynamic OTG switching with ID change interrupt > >> + */ > >> + > > [...] > >> + > >> +static int dwc3_meson_g12a_probe(struct platform_device *pdev) > >> +{ > >> + struct dwc3_meson_g12a *priv; > >> + struct device *dev = &pdev->dev; > >> + struct device_node *np = dev->of_node; > >> + void __iomem *base; > >> + struct resource *res; > >> + enum phy_mode otg_id; > >> + int ret, i; > >> + > >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > >> + if (!priv) > >> + return -ENOMEM; > >> + > >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> + base = devm_ioremap_resource(dev, res); > >> + if (IS_ERR(base)) > >> + return PTR_ERR(base); > >> + > >> + priv->regmap = devm_regmap_init_mmio(dev, base, > >> + &phy_meson_g12a_usb3_regmap_conf); > >> + if (IS_ERR(priv->regmap)) > >> + return PTR_ERR(priv->regmap); > >> + > >> + priv->vbus = devm_regulator_get_optional(dev, "vbus"); > >> + if (IS_ERR(priv->vbus)) { > >> + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) > >> + return PTR_ERR(priv->vbus); > >> + priv->vbus = NULL; > >> + } > >> + > >> + priv->clk = devm_clk_get(dev, NULL); > >> + if (IS_ERR(priv->clk)) > >> + return PTR_ERR(priv->clk); > >> + > >> + ret = clk_prepare_enable(priv->clk); > >> + if (ret) > >> + return ret; > >> + > >> + platform_set_drvdata(pdev, priv); > >> + priv->dev = dev; > >> + > >> + priv->reset = devm_reset_control_get(dev, NULL); > >> + if (IS_ERR(priv->reset)) { > >> + ret = PTR_ERR(priv->reset); > >> + dev_err(dev, "failed to get device reset, err=%d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = reset_control_reset(priv->reset); > >> + if (ret) > >> + return ret; > >> + > >> + ret = dwc3_meson_g12a_get_phys(priv); > >> + if (ret) > >> + return ret; > >> + > >> + ret = regulator_enable(priv->vbus); > >> + if (ret) > >> + return ret; > >> + > >> + /* Get dr_mode */ > >> + priv->otg_mode = usb_get_dr_mode(dev); > >> + > >> + dwc3_meson_g12a_usb_init(priv); > >> + > >> + /* Set PHY Power */ > >> + for (i = 0 ; i < PHY_COUNT ; ++i) { > >> + ret = phy_power_on(priv->phys[i]); > >> + if (ret) > >> + goto err_phys_put; > >> + } > >> + > >> + /* Init PHYs */ > >> + for (i = 0 ; i < PHY_COUNT ; ++i) { > >> + ret = phy_init(priv->phys[i]); > >> + if (ret) > >> + goto err_phys_power; > >> + } > > usually phy_init() first > > OK, i usually saw power_on before, but I'll swap them. > > > > >> + > >> + ret = of_platform_populate(np, NULL, NULL, dev); > >> + if (ret) { > >> + clk_disable_unprepare(priv->clk); > >> + clk_put(priv->clk); > >> + > >> + goto err_phys_exit; > >> + } > >> + > >> + /* Setup OTG mode corresponding to the ID pin */ > >> + if (priv->otg_mode == USB_DR_MODE_OTG) { > >> + /* TOFIX Handle ID mode toggling via IRQ */ > >> + otg_id = dwc3_meson_g12a_get_id(priv); > >> + if (otg_id != priv->otg_phy_mode) { > >> + if (dwc3_meson_g12a_otg_mode_set(priv, otg_id)) > >> + dev_warn(dev, "Failed to switch OTG mode\n"); > >> + } > >> + } > >> + > >> + /* Setup role switcher */ > >> + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, > >> + "snps,dwc3"); > >> + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); > >> + priv->switch_desc.allow_userspace_control = true; > >> + priv->switch_desc.set = dwc3_meson_g12a_role_set; > >> + priv->switch_desc.get = dwc3_meson_g12a_role_get; > > to me, use a local variable for switch_desc > > I'm never fan of changing a local global variable for a driver instance, > is it strictly required ? No :) > > > > >> + > >> + priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc); > >> + if (IS_ERR(priv->role_switch)) > >> + dev_warn(dev, "Unable to register Role Switch\n"); > >> + > >> + pm_runtime_set_active(dev); > >> + pm_runtime_enable(dev); > >> + pm_runtime_get_sync(dev); > >> + > >> + return 0; > >> + > >> +err_phys_exit: > >> + for (i = 0 ; i < PHY_COUNT ; ++i) > >> + phy_exit(priv->phys[i]); > >> + > >> +err_phys_power: > >> + for (i = 0 ; i < PHY_COUNT ; ++i) > >> + phy_power_off(priv->phys[i]); > >> + > >> +err_phys_put: > >> + for (i = 0 ; i < PHY_COUNT ; ++i) > >> + phy_put(priv->phys[i]); > >> + > > The phys are got by devm_phy_optional_get(), no need call phy_put() > > You are right, this is a leftover from v1. > > >> + return ret; > >> +} > >> + > >> +static int dwc3_meson_g12a_remove(struct platform_device *pdev) > >> +{ > >> + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); > >> + struct device *dev = &pdev->dev; > >> + int i; > >> + > >> + usb_role_switch_unregister(priv->role_switch); > >> + > >> + of_platform_depopulate(dev); > >> + > >> + for (i = 0 ; i < PHY_COUNT ; ++i) { > >> + phy_power_off(priv->phys[i]); > >> + phy_exit(priv->phys[i]); > >> + phy_put(priv->phys[i]); > > ditto > > Thanks for the review, > Neil > > >> + } > >> + > >> + clk_disable_unprepare(priv->clk); > >> + clk_put(priv->clk); > >> + > >> + pm_runtime_disable(dev); > >> + pm_runtime_put_noidle(dev); > >> + pm_runtime_set_suspended(dev); > >> + > >> + return 0; > >> +} > >> + > >> +static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev) > >> +{ > >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > >> + > >> + clk_disable(priv->clk); > >> + > >> + return 0; > >> +} > >> + > >> +static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev) > >> +{ > >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > >> + > >> + return clk_enable(priv->clk); > >> +} > >> + > >> +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) > >> +{ > >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > >> + int i; > >> + > >> + for (i = 0 ; i < PHY_COUNT ; ++i) > >> + if (priv->phys[i]) > >> + phy_exit(priv->phys[i]); > >> + > >> + reset_control_assert(priv->reset); > >> + > >> + return 0; > >> +} > >> + > >> +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) > >> +{ > >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > >> + int i, ret; > >> + > >> + reset_control_deassert(priv->reset); > >> + > >> + dwc3_meson_g12a_usb_init(priv); > >> + > >> + /* Init PHYs */ > >> + for (i = 0 ; i < PHY_COUNT ; ++i) { > >> + if (priv->phys[i]) { > >> + ret = phy_init(priv->phys[i]); > >> + if (ret) > >> + return ret; > >> + } > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = { > >> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume) > >> + SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend, > >> + dwc3_meson_g12a_runtime_resume, NULL) > >> +}; > >> + > >> +static const struct of_device_id dwc3_meson_g12a_match[] = { > >> + { .compatible = "amlogic,meson-g12a-usb-ctrl" }, > >> + { /* Sentinel */ } > >> +}; > >> +MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match); > >> + > >> +static struct platform_driver dwc3_meson_g12a_driver = { > >> + .probe = dwc3_meson_g12a_probe, > >> + .remove = dwc3_meson_g12a_remove, > >> + .driver = { > >> + .name = "dwc3-meson-g12a", > >> + .of_match_table = dwc3_meson_g12a_match, > >> + .pm = &dwc3_meson_g12a_dev_pm_ops, > >> + }, > >> +}; > >> + > >> +module_platform_driver(dwc3_meson_g12a_driver); > >> +MODULE_LICENSE("GPL v2"); > >> +MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer"); > >> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>"); > > > > >
On Thu, Mar 7, 2019 at 10:46 AM Neil Armstrong <narmstrong@baylibre.com> wrote: [...] > >> + /* Setup role switcher */ > >> + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, > >> + "snps,dwc3"); > >> + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); > >> + priv->switch_desc.allow_userspace_control = true; > >> + priv->switch_desc.set = dwc3_meson_g12a_role_set; > >> + priv->switch_desc.get = dwc3_meson_g12a_role_get; > > to me, use a local variable for switch_desc > > I'm never fan of changing a local global variable for a driver instance, > is it strictly required ? usb_role_switch_register (which is called a few lines below this) copies all data from "struct usb_role_switch_desc" to a newly allocated "struct usb_role_switch" in other words: you could make it a stack-variable personally I have no preference so I leave the decision up to you Regards Martin
Hi Neil, On Mon, Mar 4, 2019 at 11:41 AM Neil Armstrong <narmstrong@baylibre.com> wrote: [...] > +static void dwc3_meson_g12a_usb_init_mode(struct dwc3_meson_g12a *priv) > +{ > + if (priv->otg_phy_mode == PHY_MODE_USB_DEVICE) { > + regmap_update_bits(priv->regmap, USB_R0, > + USB_R0_U2D_ACT, USB_R0_U2D_ACT); > + regmap_update_bits(priv->regmap, USB_R0, > + USB_R0_U2D_SS_SCALEDOWN_MODE_MASK, 0); > + regmap_update_bits(priv->regmap, USB_R4, > + USB_R4_P21_SLEEP_M0, USB_R4_P21_SLEEP_M0); > + } else { > + regmap_update_bits(priv->regmap, USB_R0, > + USB_R0_U2D_ACT, 0); > + regmap_update_bits(priv->regmap, USB_R4, > + USB_R4_P21_SLEEP_M0, 0); > + } > +} I was already confused by the name of this function in v1. do you think "dwc3_meson_g12a_usb_otg_apply_mode" is a suitable name? [...] > +static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv, > + enum phy_mode mode) > +{ > + int ret; > + > + if (!priv->phys[USB2_OTG_PHY]) > + return -EINVAL; > + > + if (mode == PHY_MODE_USB_HOST) > + dev_info(priv->dev, "switching to Host Mode\n"); > + else > + dev_info(priv->dev, "switching to Device Mode\n"); > + > + if (priv->vbus) { > + if (mode == PHY_MODE_USB_DEVICE) > + ret = regulator_disable(priv->vbus); > + else > + ret = regulator_enable(priv->vbus); do we need to track the regulator status (whether it's enabled or not)? the regulator framework WARN()s if it detects "unbalanced disables for <regulator>" (I haven't tested this on one of my boards yet, so maybe it works because the callers of dwc3_meson_g12a_otg_mode_set() protect against this by not calling this function if the mode doesn't change) > + if (ret) > + return ret; > + } > + > + priv->otg_phy_mode = mode; > + > + dwc3_meson_g12a_usb2_set_mode(priv, USB2_OTG_PHY, mode); > + > + dwc3_meson_g12a_usb_init_mode(priv); > + > + return phy_set_mode(priv->phys[USB2_OTG_PHY], mode); > + } this is the only place where phy_set_mode is called I'm fine with keeping it but then it should be consistent at least for all USB2 PHYs. I suggest to either move the phy_set_mode call to dwc3_meson_g12a_usb2_set_mode or to remove it. [...] > +static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + enum phy_mode mode; > + > + if (role == USB_ROLE_NONE) > + return 0; > + > + mode = role == USB_ROLE_HOST ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE; (without surrounding parens I find the "role == USB_ROLE_HOST" part hard to distinguish from the "mode =" operation. if more people think this way then please speak up - otherwise it's probably just my personal taste) [...] > +static struct device *dwc3_meson_g12_find_child(struct device *dev, > + const char *compatible) > +{ > + struct platform_device *pdev; > + struct device_node *np; > + > + np = of_find_compatible_node(dev->of_node, NULL, compatible); > + if (!np) > + return NULL; > + > + pdev = of_find_device_by_node(np); maybe switch to of_get_compatible_child() here? This was done for the MMC driver in c483a5cc9d09f4 ("mmc: meson-mx-sdio: fix OF child-node lookup"), but I'm not sure if the problem described there also applies to dwc3_meson_g12_find_child [...] > +static int dwc3_meson_g12a_probe(struct platform_device *pdev) > +{ > + struct dwc3_meson_g12a *priv; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + void __iomem *base; > + struct resource *res; > + enum phy_mode otg_id; > + int ret, i; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + priv->regmap = devm_regmap_init_mmio(dev, base, > + &phy_meson_g12a_usb3_regmap_conf); > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + priv->vbus = devm_regulator_get_optional(dev, "vbus"); > + if (IS_ERR(priv->vbus)) { > + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) > + return PTR_ERR(priv->vbus); > + priv->vbus = NULL; > + } > + > + priv->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(priv->clk)) > + return PTR_ERR(priv->clk); > + > + ret = clk_prepare_enable(priv->clk); > + if (ret) > + return ret; > + > + platform_set_drvdata(pdev, priv); > + priv->dev = dev; > + > + priv->reset = devm_reset_control_get(dev, NULL); > + if (IS_ERR(priv->reset)) { > + ret = PTR_ERR(priv->reset); > + dev_err(dev, "failed to get device reset, err=%d\n", ret); > + return ret; > + } clk_prepare_enable is called a few lines above but this (and a few more) error-paths don't call clk_disable_unprepare. Jerome suggested in the dwmac-meson8b driver that I use something like, which will even allow you to drop the clk_disable_unprepare call from dwc3_meson_g12a_remove and catch all error cases in dwc3_meson_g12a_probe at the same time: devm_add_action_or_reset(dev, (void(*)(void *))clk_disable_unprepare, priv->clk); (if you go with this then you also need to remove the clk_disable_unprepare after of_platform_populate) [...] > +static int dwc3_meson_g12a_remove(struct platform_device *pdev) > +{ > + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); > + struct device *dev = &pdev->dev; > + int i; > + > + usb_role_switch_unregister(priv->role_switch); > + > + of_platform_depopulate(dev); > + > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + phy_power_off(priv->phys[i]); > + phy_exit(priv->phys[i]); > + phy_put(priv->phys[i]); > + } > + > + clk_disable_unprepare(priv->clk); > + clk_put(priv->clk); priv->clk is obtained with devm_clk_get so the common clock framework will call clk_put for us automatically [...] > +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + int i; > + > + for (i = 0 ; i < PHY_COUNT ; ++i) > + if (priv->phys[i]) > + phy_exit(priv->phys[i]); phy_init is NULL-safe, so you can drop the NULL-check above > + > + reset_control_assert(priv->reset); > + > + return 0; > +} > + > +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) > +{ > + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); > + int i, ret; > + > + reset_control_deassert(priv->reset); > + > + dwc3_meson_g12a_usb_init(priv); > + > + /* Init PHYs */ > + for (i = 0 ; i < PHY_COUNT ; ++i) { > + if (priv->phys[i]) { > + ret = phy_init(priv->phys[i]); phy_init is NULL-safe, so you can drop the NULL-check above Regards Martin
On 11/03/2019 22:56, Martin Blumenstingl wrote: > Hi Neil, > > On Mon, Mar 4, 2019 at 11:41 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > [...] >> +static void dwc3_meson_g12a_usb_init_mode(struct dwc3_meson_g12a *priv) >> +{ >> + if (priv->otg_phy_mode == PHY_MODE_USB_DEVICE) { >> + regmap_update_bits(priv->regmap, USB_R0, >> + USB_R0_U2D_ACT, USB_R0_U2D_ACT); >> + regmap_update_bits(priv->regmap, USB_R0, >> + USB_R0_U2D_SS_SCALEDOWN_MODE_MASK, 0); >> + regmap_update_bits(priv->regmap, USB_R4, >> + USB_R4_P21_SLEEP_M0, USB_R4_P21_SLEEP_M0); >> + } else { >> + regmap_update_bits(priv->regmap, USB_R0, >> + USB_R0_U2D_ACT, 0); >> + regmap_update_bits(priv->regmap, USB_R4, >> + USB_R4_P21_SLEEP_M0, 0); >> + } >> +} > I was already confused by the name of this function in v1. > do you think "dwc3_meson_g12a_usb_otg_apply_mode" is a suitable name? Yep changed > > [...] >> +static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv, >> + enum phy_mode mode) >> +{ >> + int ret; >> + >> + if (!priv->phys[USB2_OTG_PHY]) >> + return -EINVAL; >> + >> + if (mode == PHY_MODE_USB_HOST) >> + dev_info(priv->dev, "switching to Host Mode\n"); >> + else >> + dev_info(priv->dev, "switching to Device Mode\n"); >> + >> + if (priv->vbus) { >> + if (mode == PHY_MODE_USB_DEVICE) >> + ret = regulator_disable(priv->vbus); >> + else >> + ret = regulator_enable(priv->vbus); > do we need to track the regulator status (whether it's enabled or not)? > the regulator framework WARN()s if it detects "unbalanced disables for > <regulator>" > (I haven't tested this on one of my boards yet, so maybe it works > because the callers of dwc3_meson_g12a_otg_mode_set() protect against > this by not calling this function if the mode doesn't change) I handled this case by enabling the regulator _before_, as we default in Host/OTG mode in probe, then eventually switch to device. In this case, we don't have to track the status, and further calls to dwc3_meson_g12a_otg_mode_set() will only change the regulator on mode change. > >> + if (ret) >> + return ret; >> + } >> + >> + priv->otg_phy_mode = mode; >> + >> + dwc3_meson_g12a_usb2_set_mode(priv, USB2_OTG_PHY, mode); >> + >> + dwc3_meson_g12a_usb_init_mode(priv); >> + >> + return phy_set_mode(priv->phys[USB2_OTG_PHY], mode); >> + } > this is the only place where phy_set_mode is called > I'm fine with keeping it but then it should be consistent at least for > all USB2 PHYs. > I suggest to either move the phy_set_mode call to > dwc3_meson_g12a_usb2_set_mode or to remove it. I will remove it, it's useless for now. > > [...] >> +static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + enum phy_mode mode; >> + >> + if (role == USB_ROLE_NONE) >> + return 0; >> + >> + mode = role == USB_ROLE_HOST ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE; > (without surrounding parens I find the "role == USB_ROLE_HOST" part > hard to distinguish from the "mode =" operation. if more people think > this way then please speak up - otherwise it's probably just my > personal taste) Yep, I added parenthesis and changed indentation to make it clearer. > > [...] >> +static struct device *dwc3_meson_g12_find_child(struct device *dev, >> + const char *compatible) >> +{ >> + struct platform_device *pdev; >> + struct device_node *np; >> + >> + np = of_find_compatible_node(dev->of_node, NULL, compatible); >> + if (!np) >> + return NULL; >> + >> + pdev = of_find_device_by_node(np); > maybe switch to of_get_compatible_child() here? This was done for the > MMC driver in c483a5cc9d09f4 ("mmc: meson-mx-sdio: fix OF child-node > lookup"), but I'm not sure if the problem described there also applies > to dwc3_meson_g12_find_child of_get_compatible_child() is new to me, switching to it ! > > [...] >> +static int dwc3_meson_g12a_probe(struct platform_device *pdev) >> +{ >> + struct dwc3_meson_g12a *priv; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; >> + void __iomem *base; >> + struct resource *res; >> + enum phy_mode otg_id; >> + int ret, i; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(base)) >> + return PTR_ERR(base); >> + >> + priv->regmap = devm_regmap_init_mmio(dev, base, >> + &phy_meson_g12a_usb3_regmap_conf); >> + if (IS_ERR(priv->regmap)) >> + return PTR_ERR(priv->regmap); >> + >> + priv->vbus = devm_regulator_get_optional(dev, "vbus"); >> + if (IS_ERR(priv->vbus)) { >> + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) >> + return PTR_ERR(priv->vbus); >> + priv->vbus = NULL; >> + } >> + >> + priv->clk = devm_clk_get(dev, NULL); >> + if (IS_ERR(priv->clk)) >> + return PTR_ERR(priv->clk); >> + >> + ret = clk_prepare_enable(priv->clk); >> + if (ret) >> + return ret; >> + >> + platform_set_drvdata(pdev, priv); >> + priv->dev = dev; >> + >> + priv->reset = devm_reset_control_get(dev, NULL); >> + if (IS_ERR(priv->reset)) { >> + ret = PTR_ERR(priv->reset); >> + dev_err(dev, "failed to get device reset, err=%d\n", ret); >> + return ret; >> + } > clk_prepare_enable is called a few lines above but this (and a few > more) error-paths don't call clk_disable_unprepare. > Jerome suggested in the dwmac-meson8b driver that I use something > like, which will even allow you to drop the clk_disable_unprepare call > from dwc3_meson_g12a_remove and catch all error cases in > dwc3_meson_g12a_probe at the same time: > devm_add_action_or_reset(dev, (void(*)(void > *))clk_disable_unprepare, priv->clk); > (if you go with this then you also need to remove the > clk_disable_unprepare after of_platform_populate) Indeed I totally forgot this, thanks ! > > [...] >> +static int dwc3_meson_g12a_remove(struct platform_device *pdev) >> +{ >> + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); >> + struct device *dev = &pdev->dev; >> + int i; >> + >> + usb_role_switch_unregister(priv->role_switch); >> + >> + of_platform_depopulate(dev); >> + >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + phy_power_off(priv->phys[i]); >> + phy_exit(priv->phys[i]); >> + phy_put(priv->phys[i]); >> + } >> + >> + clk_disable_unprepare(priv->clk); >> + clk_put(priv->clk); > priv->clk is obtained with devm_clk_get so the common clock framework > will call clk_put for us automatically Right > > [...] >> +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + int i; >> + >> + for (i = 0 ; i < PHY_COUNT ; ++i) >> + if (priv->phys[i]) >> + phy_exit(priv->phys[i]); > phy_init is NULL-safe, so you can drop the NULL-check above Forgot this one > >> + >> + reset_control_assert(priv->reset); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) >> +{ >> + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); >> + int i, ret; >> + >> + reset_control_deassert(priv->reset); >> + >> + dwc3_meson_g12a_usb_init(priv); >> + >> + /* Init PHYs */ >> + for (i = 0 ; i < PHY_COUNT ; ++i) { >> + if (priv->phys[i]) { >> + ret = phy_init(priv->phys[i]); > phy_init is NULL-safe, so you can drop the NULL-check above Good catch... > > > Regards > Martin > Thanks !! Neil
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 1a0404fda596..21ce7368d325 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -93,6 +93,16 @@ config USB_DWC3_KEYSTONE Support of USB2/3 functionality in TI Keystone2 platforms. Say 'Y' or 'M' here if you have one such device +config USB_DWC3_MESON_G12A + tristate "Amlogic Meson G12A Platforms" + depends on OF && COMMON_CLK + depends on ARCH_MESON || COMPILE_TEST + default USB_DWC3 + select USB_ROLE_SWITCH + help + Support USB2/3 functionality in Amlogic G12A platforms. + Say 'Y' or 'M' if you have one such device. + config USB_DWC3_OF_SIMPLE tristate "Generic OF Simple Glue Layer" depends on OF && COMMON_CLK diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile index 6e3ef6144e5d..ae86da0dc5bd 100644 --- a/drivers/usb/dwc3/Makefile +++ b/drivers/usb/dwc3/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o obj-$(CONFIG_USB_DWC3_HAPS) += dwc3-haps.o obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o +obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c new file mode 100644 index 000000000000..75942614a034 --- /dev/null +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c @@ -0,0 +1,601 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * USB Glue for Amlogic G12A SoCs + * + * Copyright (c) 2019 BayLibre, SAS + * Author: Neil Armstrong <narmstrong@baylibre.com> + */ + +/* + * The USB is organized with a glue around the DWC3 Controller IP as : + * - Control registers for each USB2 Ports + * - Control registers for the USB PHY layer + * - SuperSpeed PHY can be enabled only if port is used + * + * TOFIX: + * - Add dynamic OTG switching with ID change interrupt + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/platform_device.h> +#include <linux/clk.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/reset.h> +#include <linux/phy/phy.h> +#include <linux/usb/otg.h> +#include <linux/usb/role.h> +#include <linux/regulator/consumer.h> + +/* USB2 Ports Control Registers */ + +#define U2P_REG_SIZE 0x20 + +#define U2P_R0 0x0 + #define U2P_R0_HOST_DEVICE BIT(0) + #define U2P_R0_POWER_OK BIT(1) + #define U2P_R0_HAST_MODE BIT(2) + #define U2P_R0_POWER_ON_RESET BIT(3) + #define U2P_R0_ID_PULLUP BIT(4) + #define U2P_R0_DRV_VBUS BIT(5) + +#define U2P_R1 0x4 + #define U2P_R1_PHY_READY BIT(0) + #define U2P_R1_ID_DIG BIT(1) + #define U2P_R1_OTG_SESSION_VALID BIT(2) + #define U2P_R1_VBUS_VALID BIT(3) + +/* USB Glue Control Registers */ + +#define USB_R0 0x80 + #define USB_R0_P30_LANE0_TX2RX_LOOPBACK BIT(17) + #define USB_R0_P30_LANE0_EXT_PCLK_REQ BIT(18) + #define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK GENMASK(28, 19) + #define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK GENMASK(30, 29) + #define USB_R0_U2D_ACT BIT(31) + +#define USB_R1 0x84 + #define USB_R1_U3H_BIGENDIAN_GS BIT(0) + #define USB_R1_U3H_PME_ENABLE BIT(1) + #define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASK GENMASK(4, 2) + #define USB_R1_U3H_HUB_PORT_PERM_ATTACH_MASK GENMASK(9, 7) + #define USB_R1_U3H_HOST_U2_PORT_DISABLE_MASK GENMASK(13, 12) + #define USB_R1_U3H_HOST_U3_PORT_DISABLE BIT(16) + #define USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT BIT(17) + #define USB_R1_U3H_HOST_MSI_ENABLE BIT(18) + #define USB_R1_U3H_FLADJ_30MHZ_REG_MASK GENMASK(24, 19) + #define USB_R1_P30_PCS_TX_SWING_FULL_MASK GENMASK(31, 25) + +#define USB_R2 0x88 + #define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK GENMASK(25, 20) + #define USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK GENMASK(31, 26) + +#define USB_R3 0x8c + #define USB_R3_P30_SSC_ENABLE BIT(0) + #define USB_R3_P30_SSC_RANGE_MASK GENMASK(3, 1) + #define USB_R3_P30_SSC_REF_CLK_SEL_MASK GENMASK(12, 4) + #define USB_R3_P30_REF_SSP_EN BIT(13) + +#define USB_R4 0x90 + #define USB_R4_P21_PORT_RESET_0 BIT(0) + #define USB_R4_P21_SLEEP_M0 BIT(1) + #define USB_R4_MEM_PD_MASK GENMASK(3, 2) + #define USB_R4_P21_ONLY BIT(4) + +#define USB_R5 0x94 + #define USB_R5_ID_DIG_SYNC BIT(0) + #define USB_R5_ID_DIG_REG BIT(1) + #define USB_R5_ID_DIG_CFG_MASK GENMASK(3, 2) + #define USB_R5_ID_DIG_EN_0 BIT(4) + #define USB_R5_ID_DIG_EN_1 BIT(5) + #define USB_R5_ID_DIG_CURR BIT(6) + #define USB_R5_ID_DIG_IRQ BIT(7) + #define USB_R5_ID_DIG_TH_MASK GENMASK(15, 8) + #define USB_R5_ID_DIG_CNT_MASK GENMASK(23, 16) + +enum { + USB2_HOST_PHY = 0, + USB2_OTG_PHY, + USB3_HOST_PHY, + PHY_COUNT, +}; + +static const char *phy_names[PHY_COUNT] = { + "usb2-phy0", "usb2-phy1", "usb3-phy0", +}; + +struct dwc3_meson_g12a { + struct device *dev; + struct regmap *regmap; + struct clk *clk; + struct reset_control *reset; + struct phy *phys[PHY_COUNT]; + enum usb_dr_mode otg_mode; + enum phy_mode otg_phy_mode; + unsigned int usb2_ports; + unsigned int usb3_ports; + struct regulator *vbus; + struct usb_role_switch_desc switch_desc; + struct usb_role_switch *role_switch; +}; + +static void dwc3_meson_g12a_usb2_set_mode(struct dwc3_meson_g12a *priv, + int i, enum phy_mode mode) +{ + if (mode == PHY_MODE_USB_HOST) + regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i), + U2P_R0_HOST_DEVICE, + U2P_R0_HOST_DEVICE); + else + regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i), + U2P_R0_HOST_DEVICE, 0); +} + +static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv) +{ + int i; + + if (priv->otg_mode == USB_DR_MODE_PERIPHERAL) + priv->otg_phy_mode = PHY_MODE_USB_DEVICE; + else + priv->otg_phy_mode = PHY_MODE_USB_HOST; + + for (i = 0 ; i < USB3_HOST_PHY ; ++i) { + if (!priv->phys[i]) + continue; + + regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i), + U2P_R0_POWER_ON_RESET, + U2P_R0_POWER_ON_RESET); + + if (i == USB2_OTG_PHY) { + regmap_update_bits(priv->regmap, + U2P_R0 + (U2P_REG_SIZE * i), + U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS, + U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS); + + dwc3_meson_g12a_usb2_set_mode(priv, i, + priv->otg_phy_mode); + } else + dwc3_meson_g12a_usb2_set_mode(priv, i, + PHY_MODE_USB_HOST); + + regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i), + U2P_R0_POWER_ON_RESET, 0); + } + + return 0; +} + +static void dwc3_meson_g12a_usb3_init(struct dwc3_meson_g12a *priv) +{ + regmap_update_bits(priv->regmap, USB_R3, + USB_R3_P30_SSC_RANGE_MASK | + USB_R3_P30_REF_SSP_EN, + USB_R3_P30_SSC_ENABLE | + FIELD_PREP(USB_R3_P30_SSC_RANGE_MASK, 2) | + USB_R3_P30_REF_SSP_EN); + udelay(2); + + regmap_update_bits(priv->regmap, USB_R2, + USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK, + FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK, 0x15)); + + regmap_update_bits(priv->regmap, USB_R2, + USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK, + FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK, 0x20)); + + udelay(2); + + regmap_update_bits(priv->regmap, USB_R1, + USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT, + USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT); + + regmap_update_bits(priv->regmap, USB_R1, + USB_R1_P30_PCS_TX_SWING_FULL_MASK, + FIELD_PREP(USB_R1_P30_PCS_TX_SWING_FULL_MASK, 127)); +} + +static void dwc3_meson_g12a_usb_init_mode(struct dwc3_meson_g12a *priv) +{ + if (priv->otg_phy_mode == PHY_MODE_USB_DEVICE) { + regmap_update_bits(priv->regmap, USB_R0, + USB_R0_U2D_ACT, USB_R0_U2D_ACT); + regmap_update_bits(priv->regmap, USB_R0, + USB_R0_U2D_SS_SCALEDOWN_MODE_MASK, 0); + regmap_update_bits(priv->regmap, USB_R4, + USB_R4_P21_SLEEP_M0, USB_R4_P21_SLEEP_M0); + } else { + regmap_update_bits(priv->regmap, USB_R0, + USB_R0_U2D_ACT, 0); + regmap_update_bits(priv->regmap, USB_R4, + USB_R4_P21_SLEEP_M0, 0); + } +} + +static int dwc3_meson_g12a_usb_init(struct dwc3_meson_g12a *priv) +{ + int ret; + + ret = dwc3_meson_g12a_usb2_init(priv); + if (ret) + return ret; + + regmap_update_bits(priv->regmap, USB_R1, + USB_R1_U3H_FLADJ_30MHZ_REG_MASK, + FIELD_PREP(USB_R1_U3H_FLADJ_30MHZ_REG_MASK, 0x20)); + + regmap_update_bits(priv->regmap, USB_R5, + USB_R5_ID_DIG_EN_0, + USB_R5_ID_DIG_EN_0); + regmap_update_bits(priv->regmap, USB_R5, + USB_R5_ID_DIG_EN_1, + USB_R5_ID_DIG_EN_1); + regmap_update_bits(priv->regmap, USB_R5, + USB_R5_ID_DIG_TH_MASK, + FIELD_PREP(USB_R5_ID_DIG_TH_MASK, 0xff)); + + /* If we have an actual SuperSpeed port, initialize it */ + if (priv->usb3_ports) + dwc3_meson_g12a_usb3_init(priv); + + dwc3_meson_g12a_usb_init_mode(priv); + + return 0; +} + +static const struct regmap_config phy_meson_g12a_usb3_regmap_conf = { + .reg_bits = 8, + .val_bits = 32, + .reg_stride = 4, + .max_register = USB_R5, +}; + +static int dwc3_meson_g12a_get_phys(struct dwc3_meson_g12a *priv) +{ + int i; + + for (i = 0 ; i < PHY_COUNT ; ++i) { + priv->phys[i] = devm_phy_optional_get(priv->dev, phy_names[i]); + if (!priv->phys[i]) + continue; + + if (IS_ERR(priv->phys[i])) + return PTR_ERR(priv->phys[i]); + + if (i == USB3_HOST_PHY) + priv->usb3_ports++; + else + priv->usb2_ports++; + } + + dev_info(priv->dev, "USB2 ports: %d\n", priv->usb2_ports); + dev_info(priv->dev, "USB3 ports: %d\n", priv->usb3_ports); + + return 0; +} + +static enum phy_mode dwc3_meson_g12a_get_id(struct dwc3_meson_g12a *priv) +{ + u32 reg; + + regmap_read(priv->regmap, USB_R5, ®); + + if (reg & (USB_R5_ID_DIG_SYNC | USB_R5_ID_DIG_REG)) + return PHY_MODE_USB_DEVICE; + + return PHY_MODE_USB_HOST; +} + +static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv, + enum phy_mode mode) +{ + int ret; + + if (!priv->phys[USB2_OTG_PHY]) + return -EINVAL; + + if (mode == PHY_MODE_USB_HOST) + dev_info(priv->dev, "switching to Host Mode\n"); + else + dev_info(priv->dev, "switching to Device Mode\n"); + + if (priv->vbus) { + if (mode == PHY_MODE_USB_DEVICE) + ret = regulator_disable(priv->vbus); + else + ret = regulator_enable(priv->vbus); + if (ret) + return ret; + } + + priv->otg_phy_mode = mode; + + dwc3_meson_g12a_usb2_set_mode(priv, USB2_OTG_PHY, mode); + + dwc3_meson_g12a_usb_init_mode(priv); + + return phy_set_mode(priv->phys[USB2_OTG_PHY], mode); +} + +static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + enum phy_mode mode; + + if (role == USB_ROLE_NONE) + return 0; + + mode = role == USB_ROLE_HOST ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE; + + if (mode == priv->otg_phy_mode) + return 0; + + return dwc3_meson_g12a_otg_mode_set(priv, mode); +} + +static enum usb_role dwc3_meson_g12a_role_get(struct device *dev) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + + return priv->otg_phy_mode == PHY_MODE_USB_HOST ? + USB_ROLE_HOST : USB_ROLE_DEVICE; +} + +static struct device *dwc3_meson_g12_find_child(struct device *dev, + const char *compatible) +{ + struct platform_device *pdev; + struct device_node *np; + + np = of_find_compatible_node(dev->of_node, NULL, compatible); + if (!np) + return NULL; + + pdev = of_find_device_by_node(np); + of_node_put(np); + if (!pdev) + return NULL; + + return &pdev->dev; +} + +static int dwc3_meson_g12a_probe(struct platform_device *pdev) +{ + struct dwc3_meson_g12a *priv; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + void __iomem *base; + struct resource *res; + enum phy_mode otg_id; + int ret, i; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + priv->regmap = devm_regmap_init_mmio(dev, base, + &phy_meson_g12a_usb3_regmap_conf); + if (IS_ERR(priv->regmap)) + return PTR_ERR(priv->regmap); + + priv->vbus = devm_regulator_get_optional(dev, "vbus"); + if (IS_ERR(priv->vbus)) { + if (PTR_ERR(priv->vbus) == -EPROBE_DEFER) + return PTR_ERR(priv->vbus); + priv->vbus = NULL; + } + + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + ret = clk_prepare_enable(priv->clk); + if (ret) + return ret; + + platform_set_drvdata(pdev, priv); + priv->dev = dev; + + priv->reset = devm_reset_control_get(dev, NULL); + if (IS_ERR(priv->reset)) { + ret = PTR_ERR(priv->reset); + dev_err(dev, "failed to get device reset, err=%d\n", ret); + return ret; + } + + ret = reset_control_reset(priv->reset); + if (ret) + return ret; + + ret = dwc3_meson_g12a_get_phys(priv); + if (ret) + return ret; + + ret = regulator_enable(priv->vbus); + if (ret) + return ret; + + /* Get dr_mode */ + priv->otg_mode = usb_get_dr_mode(dev); + + dwc3_meson_g12a_usb_init(priv); + + /* Set PHY Power */ + for (i = 0 ; i < PHY_COUNT ; ++i) { + ret = phy_power_on(priv->phys[i]); + if (ret) + goto err_phys_put; + } + + /* Init PHYs */ + for (i = 0 ; i < PHY_COUNT ; ++i) { + ret = phy_init(priv->phys[i]); + if (ret) + goto err_phys_power; + } + + ret = of_platform_populate(np, NULL, NULL, dev); + if (ret) { + clk_disable_unprepare(priv->clk); + clk_put(priv->clk); + + goto err_phys_exit; + } + + /* Setup OTG mode corresponding to the ID pin */ + if (priv->otg_mode == USB_DR_MODE_OTG) { + /* TOFIX Handle ID mode toggling via IRQ */ + otg_id = dwc3_meson_g12a_get_id(priv); + if (otg_id != priv->otg_phy_mode) { + if (dwc3_meson_g12a_otg_mode_set(priv, otg_id)) + dev_warn(dev, "Failed to switch OTG mode\n"); + } + } + + /* Setup role switcher */ + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, + "snps,dwc3"); + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); + priv->switch_desc.allow_userspace_control = true; + priv->switch_desc.set = dwc3_meson_g12a_role_set; + priv->switch_desc.get = dwc3_meson_g12a_role_get; + + priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc); + if (IS_ERR(priv->role_switch)) + dev_warn(dev, "Unable to register Role Switch\n"); + + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); + + return 0; + +err_phys_exit: + for (i = 0 ; i < PHY_COUNT ; ++i) + phy_exit(priv->phys[i]); + +err_phys_power: + for (i = 0 ; i < PHY_COUNT ; ++i) + phy_power_off(priv->phys[i]); + +err_phys_put: + for (i = 0 ; i < PHY_COUNT ; ++i) + phy_put(priv->phys[i]); + + return ret; +} + +static int dwc3_meson_g12a_remove(struct platform_device *pdev) +{ + struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev); + struct device *dev = &pdev->dev; + int i; + + usb_role_switch_unregister(priv->role_switch); + + of_platform_depopulate(dev); + + for (i = 0 ; i < PHY_COUNT ; ++i) { + phy_power_off(priv->phys[i]); + phy_exit(priv->phys[i]); + phy_put(priv->phys[i]); + } + + clk_disable_unprepare(priv->clk); + clk_put(priv->clk); + + pm_runtime_disable(dev); + pm_runtime_put_noidle(dev); + pm_runtime_set_suspended(dev); + + return 0; +} + +static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + + clk_disable(priv->clk); + + return 0; +} + +static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + + return clk_enable(priv->clk); +} + +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + int i; + + for (i = 0 ; i < PHY_COUNT ; ++i) + if (priv->phys[i]) + phy_exit(priv->phys[i]); + + reset_control_assert(priv->reset); + + return 0; +} + +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev) +{ + struct dwc3_meson_g12a *priv = dev_get_drvdata(dev); + int i, ret; + + reset_control_deassert(priv->reset); + + dwc3_meson_g12a_usb_init(priv); + + /* Init PHYs */ + for (i = 0 ; i < PHY_COUNT ; ++i) { + if (priv->phys[i]) { + ret = phy_init(priv->phys[i]); + if (ret) + return ret; + } + } + + return 0; +} + +static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume) + SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend, + dwc3_meson_g12a_runtime_resume, NULL) +}; + +static const struct of_device_id dwc3_meson_g12a_match[] = { + { .compatible = "amlogic,meson-g12a-usb-ctrl" }, + { /* Sentinel */ } +}; +MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match); + +static struct platform_driver dwc3_meson_g12a_driver = { + .probe = dwc3_meson_g12a_probe, + .remove = dwc3_meson_g12a_remove, + .driver = { + .name = "dwc3-meson-g12a", + .of_match_table = dwc3_meson_g12a_match, + .pm = &dwc3_meson_g12a_dev_pm_ops, + }, +}; + +module_platform_driver(dwc3_meson_g12a_driver); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer"); +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
Adds support for Amlogic G12A USB Control Glue HW. The Amlogic G12A SoC Family embeds 2 USB Controllers : - a DWC3 IP configured as Host for USB2 and USB3 - a DWC2 IP configured as Peripheral USB2 Only A glue connects these both controllers to 2 USB2 PHYs, and optionnally to an USB3+PCIE Combo PHY shared with the PCIE controller. The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including routing of the OTG PHY between the DWC3 and DWC2 controllers, and setups the on-chip OTG mode selection for this PHY. This drivers supports the on-probe setup of the OTG mode, and manually via a debugfs interface. The IRQ mode change detect is yet to be added in a future patchset, mainly due to lack of hardware to validate on. Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/usb/dwc3/Kconfig | 10 + drivers/usb/dwc3/Makefile | 1 + drivers/usb/dwc3/dwc3-meson-g12a.c | 601 +++++++++++++++++++++++++++++ 3 files changed, 612 insertions(+) create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c