Tegra's USB driver dependency fix
Subsystems (3)
v3 cover Tegra's USB driver dependency fix
Tegra uses two different USB drivers for host / device controllers, one is tegra-ehci (host) and other is ChipIdea (UDC). Currently ChipIdea driver implicitly (and incorrectly) depends on Tegra's EHCI driver because Tegra's PHY driver is built only when host driver is built, also host controller shares HW resets with the device controller and host driver manages the resets solely. This small series fixes the dependency by moving out reset controls to the PHY driver and making them shared, then the driver build dependency is resolved.
Changelog:
v3:
Fixed Tegra's EHCI driver dependency on functions exported by the Tegra's PHY driver (was reported by "kbuild test robot" for v2) by making host driver to select the PHY driver in Kconfig.
v2:
Added missed USB_ULPI dependency to USB_TEGRA_PHY.
Corrected UTMI pads reset by moving reset assert/deassert to the PHY's probe.
Removed function names as per Thierry's suggestion.
Dmitry Osipenko (3):
usb: phy: tegra: Cleanup error messages
usb: tegra: Move utmi-pads reset from ehci-tegra to tegra-phy
usb: phy: Add Kconfig entry for Tegra PHY driver
drivers/usb/host/Kconfig | 4 +- drivers/usb/host/ehci-tegra.c | 87 +++++++++++------------ drivers/usb/phy/Kconfig | 9 +++ drivers/usb/phy/Makefile | 2 +- drivers/usb/phy/phy-tegra-usb.c | 140 ++++++++++++++++++++++++++++++-------- include/linux/usb/tegra_usb_phy.h | 2 + 6 files changed, 163 insertions(+), 81 deletions(-)
Signature · 1 line
2.16.3
PATCH v3 1/3 usb: phy: tegra: Cleanup error messages
Tegra's PHY driver has a mix of pr_err() and dev_err(), let's switch to dev_err() and use common errors message formatting across the driver for consistency.
--- drivers/usb/phy/phy-tegra-usb.c | 69 ++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 28 deletions(-)
drivers/usb/phy/phy-tegra-usb.c +41 -28
@@ -236,10 +236,14 @@ static void set_phcd(struct tegra_usb_phy *phy, bool enable)static int utmip_pad_open (struct tegra_usb_phy *phy) { +int err; + phy->pad_clk =devm_clk_get (phy->u_phy.dev,"utmi-pads" );if (IS_ERR (phy->pad_clk)) { -pr_err ("%s: can't get utmip pad clock\n" , __func__); -return PTR_ERR (phy->pad_clk); + err =PTR_ERR (phy->pad_clk); +dev_err (phy->u_phy.dev, +"Failed to get UTMIP pad clock: %d\n" , err); +return err; }return 0 ;
@@ -282,7 +286,7 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy)void __iomem *base = phy->pad_regs;if (!utmip_pad_count) { -pr_err ("%s: utmip pad already powered off\n" , __func__); +dev_err (phy->u_phy.dev,"UTMIP pad already powered off\n" );return -EINVAL ; }
@@ -338,7 +342,8 @@ static void utmi_phy_clk_disable(struct tegra_usb_phy *phy)set_phcd (phy, true);if (utmi_wait_register (base +USB_SUSP_CTRL ,USB_PHY_CLK_VALID ,0 ) <0 ) -pr_err ("%s: timeout waiting for phy to stabilize\n" , __func__); +dev_err (phy->u_phy.dev, +"Timeout waiting for PHY to stabilize on disable\n" ); }static void utmi_phy_clk_enable (struct tegra_usb_phy *phy)
@@ -370,7 +375,8 @@ static void utmi_phy_clk_enable(struct tegra_usb_phy *phy)if (utmi_wait_register (base +USB_SUSP_CTRL ,USB_PHY_CLK_VALID ,USB_PHY_CLK_VALID )) -pr_err ("%s: timeout waiting for phy to stabilize\n" , __func__); +dev_err (phy->u_phy.dev, +"Timeout waiting for PHY to stabilize on enable\n" ); }static int utmi_phy_power_on (struct tegra_usb_phy *phy)
@@ -617,15 +623,15 @@ static int ulpi_phy_power_on(struct tegra_usb_phy *phy) ret =gpio_direction_output (phy->reset_gpio,0 );if (ret <0 ) { -dev_err (phy->u_phy.dev,"gpio %d not set to 0\n" , - phy->reset_gpio); +dev_err (phy->u_phy.dev,"GPIO %d not set to 0: %d\n" , + phy->reset_gpio, ret);return ret; }msleep (5 ); ret =gpio_direction_output (phy->reset_gpio,1 );if (ret <0 ) { -dev_err (phy->u_phy.dev,"gpio %d not set to 1\n" , - phy->reset_gpio); +dev_err (phy->u_phy.dev,"GPIO %d not set to 1: %d\n" , + phy->reset_gpio, ret);return ret; }
@@ -661,13 +667,13 @@ static int ulpi_phy_power_on(struct tegra_usb_phy *phy)/* Fix VbusInvalid due to floating VBUS */ ret =usb_phy_io_write (phy->ulpi,0x40 ,0x08 );if (ret) { -pr_err ("%s: ulpi write failed\n" , __func__); +dev_err (phy->u_phy.dev,"ULPI write failed: %d\n" , ret);return ret; } ret =usb_phy_io_write (phy->ulpi,0x80 ,0x0B );if (ret) { -pr_err ("%s: ulpi write failed\n" , __func__); +dev_err (phy->u_phy.dev,"ULPI write failed: %d\n" , ret);return ret; }
@@ -728,28 +734,30 @@ static int ulpi_open(struct tegra_usb_phy *phy) phy->clk =devm_clk_get (phy->u_phy.dev,"ulpi-link" );if (IS_ERR (phy->clk)) { -pr_err ("%s: can't get ulpi clock\n" , __func__); -return PTR_ERR (phy->clk); + err =PTR_ERR (phy->clk); +dev_err (phy->u_phy.dev,"Failed to get ULPI clock: %d\n" , err); +return err; } err =devm_gpio_request (phy->u_phy.dev, phy->reset_gpio,"ulpi_phy_reset_b" );if (err <0 ) { -dev_err (phy->u_phy.dev,"request failed for gpio: %d\n" , - phy->reset_gpio); +dev_err (phy->u_phy.dev,"Request failed for GPIO %d: %d\n" , + phy->reset_gpio, err);return err; } err =gpio_direction_output (phy->reset_gpio,0 );if (err <0 ) { -dev_err (phy->u_phy.dev,"gpio %d direction not set to output\n" , - phy->reset_gpio); +dev_err (phy->u_phy.dev, +"GPIO %d direction not set to output: %d\n" , + phy->reset_gpio, err);return err; } phy->ulpi =otg_ulpi_create (&ulpi_viewport_access_ops,0 );if (!phy->ulpi) { -dev_err (phy->u_phy.dev,"otg_ulpi_create returned NULL\n" ); +dev_err (phy->u_phy.dev,"Failed to create ULPI OTG\n" ); err = -ENOMEM ;return err; }
@@ -766,8 +774,10 @@ static int tegra_usb_phy_init(struct tegra_usb_phy *phy) phy->pll_u =devm_clk_get (phy->u_phy.dev,"pll_u" );if (IS_ERR (phy->pll_u)) { -pr_err ("Can't get pll_u clock\n" ); -return PTR_ERR (phy->pll_u); + err =PTR_ERR (phy->pll_u); +dev_err (phy->u_phy.dev, +"Failed to get pll_u clock: %d\n" , err); +return err; } err =clk_prepare_enable (phy->pll_u);
@@ -782,7 +792,8 @@ static int tegra_usb_phy_init(struct tegra_usb_phy *phy) } }if (!phy->freq) { -pr_err ("invalid pll_u parent rate %ld\n" , parent_rate); +dev_err (phy->u_phy.dev,"Invalid pll_u parent rate %ld\n" , + parent_rate); err = -EINVAL ; goto fail; }
@@ -791,7 +802,7 @@ static int tegra_usb_phy_init(struct tegra_usb_phy *phy) err =regulator_enable (phy->vbus);if (err) {dev_err (phy->u_phy.dev, -"failed to enable usb vbus regulator: %d\n" , +"Failed to enable USB VBUS regulator: %d\n" , err); goto fail; }
@@ -855,7 +866,8 @@ static int read_utmi_param(struct platform_device *pdev, const char *param,int err =of_property_read_u32 (pdev->dev.of_node, param, &value); *dest = (u8 )value;if (err <0 ) -dev_err (&pdev->dev,"Failed to read USB UTMI parameter %s: %d\n" , +dev_err (&pdev->dev, +"Failed to read USB UTMI parameter %s: %d\n" , param, err);return err; }
@@ -871,14 +883,14 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy, res =platform_get_resource (pdev,IORESOURCE_MEM ,1 );if (!res) { -dev_err (&pdev->dev,"Failed to get UTMI Pad regs\n" ); +dev_err (&pdev->dev,"Failed to get UTMI pad regs\n" );return -ENXIO ; } tegra_phy->pad_regs =devm_ioremap (&pdev->dev, res->start,resource_size (res));if (!tegra_phy->pad_regs) { -dev_err (&pdev->dev,"Failed to remap UTMI Pad regs\n" ); +dev_err (&pdev->dev,"Failed to remap UTMI pad regs\n" );return -ENOMEM ; }
@@ -1020,15 +1032,16 @@ static int tegra_usb_phy_probe(struct platform_device *pdev) tegra_phy->reset_gpio =of_get_named_gpio (np,"nvidia,phy-reset-gpio" ,0 );if (!gpio_is_valid (tegra_phy->reset_gpio)) { -dev_err (&pdev->dev,"invalid gpio: %d\n" , - tegra_phy->reset_gpio); +dev_err (&pdev->dev, +"Invalid GPIO: %d\n" , tegra_phy->reset_gpio);return tegra_phy->reset_gpio; } tegra_phy->config =NULL ;break ;default : -dev_err (&pdev->dev,"phy_type is invalid or unsupported\n" ); +dev_err (&pdev->dev,"phy_type %u is invalid or unsupported\n" , + phy_type);return -EINVAL ; }
Signature · 1 line
2.16.3
PATCH v3 2/3 usb: tegra: Move utmi-pads reset from ehci-tegra to tegra-phy
In reply to Dmitry OsipenkoUTMI pads are shared by USB controllers and reset of UTMI pads is shared with the reset of USB1 controller. Currently reset of UTMI pads is done by the EHCI driver and ChipIdea UDC works because EHCI driver always happen to be probed first. Move reset controls from ehci-tegra to tegra-phy in order to resolve the problem.
--- drivers/usb/host/ehci-tegra.c | 87 ++++++++++++++++++--------------------- drivers/usb/phy/phy-tegra-usb.c | 79 ++++++++++++++++++++++++++++++++--- include/linux/usb/tegra_usb_phy.h | 2 + 3 files changed, 115 insertions(+), 53 deletions(-)
drivers/usb/host/ehci-tegra.c +39 -48
@@ -36,7 +36,6 @@#define DRV_NAME "tegra-ehci"static struct hc_driver __read_mostly tegra_ehci_hc_driver; -static bool usb1_reset_attempted;struct tegra_ehci_soc_config {bool has_hostpc;
@@ -51,67 +50,54 @@ struct tegra_ehci_hcd {enum tegra_usb_phy_port_speed port_speed; }; -/* -* The 1st USB controller contains some UTMI pad registers that are global for -* all the controllers on the chip. Those registers are also cleared when -* reset is asserted to the 1st controller. This means that the 1st controller -* can only be reset when no other controlled has finished probing. So we'll -* reset the 1st controller before doing any other setup on any of the -* controllers, and then never again. -* -* Since this is a PHY issue, the Tegra PHY driver should probably be doing -* the resetting of the USB controllers. But to keep compatibility with old -* device trees that don't have reset phandles in the PHYs, do it here. -* Those old DTs will be vulnerable to total USB breakage if the 1st EHCI -* device isn't the first one to finish probing, so warn them. -*/ static int tegra_reset_usb_controller (struct platform_device *pdev) {struct device_node *phy_np;struct usb_hcd *hcd =platform_get_drvdata (pdev);struct tegra_ehci_hcd *tegra = (struct tegra_ehci_hcd *)hcd_to_ehci (hcd)->priv; -bool has_utmi_pad_registers = false; +struct reset_control *rst; +int err; phy_np =of_parse_phandle (pdev->dev.of_node,"nvidia,phy" ,0 );if (!phy_np)return -ENOENT ; -if (of_property_read_bool (phy_np,"nvidia,has-utmi-pad-registers" )) - has_utmi_pad_registers = true; +/* +* The 1st USB controller contains some UTMI pad registers that are +* global for all the controllers on the chip. Those registers are +* also cleared when reset is asserted to the 1st controller. +*/ + rst =of_reset_control_get_shared (phy_np,"utmi-pads" ); +if (IS_ERR (rst)) { +dev_warn (&pdev->dev, +"can't get utmi-pads reset from the PHY\n" ); +dev_warn (&pdev->dev, +"continuing, but please update your DT\n" ); + }else { +/* +* PHY driver performs UTMI-pads reset in a case of +* non-legacy DT. +*/ +reset_control_put (rst); + } -if (!usb1_reset_attempted) { -struct reset_control *usb1_reset; +of_node_put (phy_np); -if (!has_utmi_pad_registers) - usb1_reset =of_reset_control_get (phy_np,"utmi-pads" ); -else - usb1_reset = tegra->rst; - -if (IS_ERR (usb1_reset)) { -dev_warn (&pdev->dev, -"can't get utmi-pads reset from the PHY\n" ); -dev_warn (&pdev->dev, -"continuing, but please update your DT\n" ); - }else { -reset_control_assert (usb1_reset); -udelay (1 ); -reset_control_deassert (usb1_reset); - -if (!has_utmi_pad_registers) -reset_control_put (usb1_reset); - } +/* reset control is shared, hence initialize it first */ + err =reset_control_deassert (tegra->rst); +if (err) +return err; - usb1_reset_attempted = true; - } + err =reset_control_assert (tegra->rst); +if (err) +return err; -if (!has_utmi_pad_registers) { -reset_control_assert (tegra->rst); -udelay (1 ); -reset_control_deassert (tegra->rst); - } +udelay (1 ); -of_node_put (phy_np); + err =reset_control_deassert (tegra->rst); +if (err) +return err;return 0 ; }
@@ -440,7 +426,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) goto cleanup_hcd_create; } - tegra->rst =devm_reset_control_get (&pdev->dev,"usb" ); + tegra->rst =devm_reset_control_get_shared (&pdev->dev,"usb" );if (IS_ERR (tegra->rst)) {dev_err (&pdev->dev,"Can't get ehci reset\n" ); err =PTR_ERR (tegra->rst);
@@ -452,8 +438,10 @@ static int tegra_ehci_probe(struct platform_device *pdev) goto cleanup_hcd_create; err =tegra_reset_usb_controller (pdev); -if (err) +if (err) { +dev_err (&pdev->dev,"Failed to reset controller\n" ); goto cleanup_clk_en; + } u_phy =devm_usb_get_phy_by_phandle (&pdev->dev,"nvidia,phy" ,0 );if (IS_ERR (u_phy)) {
@@ -538,6 +526,9 @@ static int tegra_ehci_remove(struct platform_device *pdev)usb_phy_shutdown (hcd->usb_phy);usb_remove_hcd (hcd); +reset_control_assert (tegra->rst); +udelay (1 ); +clk_disable_unprepare (tegra->clk);usb_put_hcd (hcd);
drivers/usb/phy/phy-tegra-usb.c +74 -5
97 lines not on this page · raw message
include/linux/usb/tegra_usb_phy.h +2 -0
@@ -17,6 +17,7 @@#define __TEGRA_USB_PHY_H#include <linux/clk.h> +#include <linux/reset.h> #include <linux/usb/otg.h> /*
@@ -76,6 +77,7 @@ struct tegra_usb_phy {bool is_legacy_phy;bool is_ulpi_phy;int reset_gpio; +struct reset_control *pad_rst; };void tegra_usb_phy_preresume (struct usb_phy *phy);
Signature · 1 line
2.16.3
PATCH v3 3/3 usb: phy: Add Kconfig entry for Tegra PHY driver
In reply to Dmitry OsipenkoTegra's EHCI driver has a build dependency on Tegra's PHY driver and currently Tegra's PHY driver is built only when Tegra's EHCI driver is built. Add own Kconfig entry for the Tegra's PHY driver so that drivers other than ehci-tegra (like ChipIdea UDC) could work with ehci-tegra driver being disabled in kernels config by allowing user to manually select the PHY driver.
--- drivers/usb/host/Kconfig | 4 +--- drivers/usb/phy/Kconfig | 9 +++++++++ drivers/usb/phy/Makefile | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-)
drivers/usb/host/Kconfig +1 -3
@@ -234,9 +234,7 @@ config USB_EHCI_TEGRA tristate "NVIDIA Tegra HCD support" depends on ARCH_TEGRA select USB_EHCI_ROOT_HUB_TT - select USB_PHY - select USB_ULPI - select USB_ULPI_VIEWPORT + select USB_TEGRA_PHY help This driver enables support for the internal USB Host Controllers found in NVIDIA Tegra SoCs. The controllers are EHCI compliant.
drivers/usb/phy/Kconfig +9 -0
@@ -159,6 +159,15 @@ config USB_MXS_PHY MXS Phy is used by some of the i.MX SoCs, for example imx23/28/6x. +config USB_TEGRA_PHY + tristate "NVIDIA Tegra USB PHY Driver" + depends on ARCH_TEGRA + select USB_PHY + select USB_ULPI + help + This driver provides PHY support for the USB controllers found + on NVIDIA Tegra SoC's. + config USB_ULPI bool "Generic ULPI Transceiver Driver" depends on ARM || ARM64
drivers/usb/phy/Makefile +1 -1
@@ -16,7 +16,7 @@ obj-$(CONFIG_AM335X_CONTROL_USB) += phy-am335x-control.o obj-$(CONFIG_AM335X_PHY_USB) += phy-am335x.o obj-$(CONFIG_OMAP_OTG) += phy-omap-otg.o obj-$(CONFIG_TWL6030_USB) += phy-twl6030-usb.o -obj-$(CONFIG_USB_EHCI_TEGRA) += phy-tegra-usb.o +obj-$(CONFIG_USB_TEGRA_PHY) += phy-tegra-usb.o obj-$(CONFIG_USB_GPIO_VBUS) += phy-gpio-vbus-usb.o obj-$(CONFIG_USB_ISP1301) += phy-isp1301.o obj-$(CONFIG_USB_MV_OTG) += phy-mv-usb.o
Signature · 1 line
2.16.3