[PATCH 0/9] Tegra USB cleanup series

[PATCH 0/9] Tegra USB cleanup series

[Tuomas Tynkkynen]

Hi,

Here's a few cleanup patches for the Tegra USB drivers, to be applied on top
of Mikko's two patch sets. It mostly deals with removing all usage of platform
data and using standard helpers and enums instead of Tegra-specific ones.

Tuomas Tynkkynen (9):
  usb: phy: tegra: Remove unnecessary 'dev' field
  usb: host: tegra: Remove leftover code
  usb: tegra: host: Remove references to plat data
  ARM: tegra: Remove USB platform data
  usb: phy: tegra: Register as an USB PHY.
  usb: host: tegra: Locate a PHY via standard API
  usb: phy: tegra: Remove custom PHY locating APIs
  usb: phy: tegra: Use DT helpers for phy_type
  usb: phy: tegra: Use DT helpers for dr_mode

 arch/arm/mach-tegra/tegra.c             | 38 +------------
 drivers/usb/host/ehci-tegra.c           | 37 +++----------
 drivers/usb/phy/phy-tegra-usb.c         | 96 ++++++++++++++++-----------------
 include/linux/platform_data/tegra_usb.h | 32 -----------
 include/linux/usb/tegra_usb_phy.h       | 16 +-----
 5 files changed, 53 insertions(+), 166 deletions(-)
 delete mode 100644 include/linux/platform_data/tegra_usb.h

-- 
1.8.1.5

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

[PATCH 2/9] usb: host: tegra: Remove leftover code

[Tuomas Tynkkynen]

ehci-tegra calls devm_usb_get_phy, which will never succeed since the Tegra
PHY does not register itself with the PHY subsystem. It is also completely
redundant since the code has already located a PHY via an internal API.

Call otg_set_host unconditionally to simplify the code since it should
be safe to do so.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 14c1f35..06e8feb 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -58,7 +58,6 @@ static int (*orig_hub_control)(struct usb_hcd *hcd,
 struct tegra_ehci_hcd {
 	struct tegra_usb_phy *phy;
 	struct clk *clk;
-	struct usb_phy *transceiver;
 	int port_resuming;
 	bool needs_double_reset;
 	enum tegra_usb_phy_port_speed port_speed;
@@ -436,26 +435,18 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 		goto cleanup_phy;
 	}
 
-	if (pdata->operating_mode == TEGRA_USB_OTG) {
-		tegra->transceiver =
-			devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
-		if (!IS_ERR(tegra->transceiver))
-			otg_set_host(tegra->transceiver->otg, &hcd->self);
-	} else {
-		tegra->transceiver = ERR_PTR(-ENODEV);
-	}
+	otg_set_host(u_phy->otg, &hcd->self);
 
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to add USB HCD\n");
-		goto cleanup_transceiver;
+		goto cleanup_otg_set_host;
 	}
 
 	return err;
 
-cleanup_transceiver:
-	if (!IS_ERR(tegra->transceiver))
-		otg_set_host(tegra->transceiver->otg, NULL);
+cleanup_otg_set_host:
+	otg_set_host(u_phy->otg, NULL);
 cleanup_phy:
 	usb_phy_shutdown(hcd->phy);
 cleanup_clk_en:
@@ -473,8 +464,7 @@ static int tegra_ehci_remove(struct platform_device *pdev)
 	struct tegra_ehci_hcd *tegra =
 		(struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
 
-	if (!IS_ERR(tegra->transceiver))
-		otg_set_host(tegra->transceiver->otg, NULL);
+	otg_set_host(hcd->phy->otg, NULL);
 
 	usb_phy_shutdown(hcd->phy);
 	usb_remove_hcd(hcd);
-- 
1.8.1.5

[PATCH 3/9] usb: tegra: host: Remove references to plat data

[Tuomas Tynkkynen]

Platform data is not used in tegra-ehci anymore, so remove all
references to it.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 06e8feb..a208cea 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -27,7 +27,6 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_device.h>
-#include <linux/platform_data/tegra_usb.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/usb/ehci_def.h>
@@ -327,18 +326,11 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	struct usb_hcd *hcd;
 	struct ehci_hcd *ehci;
 	struct tegra_ehci_hcd *tegra;
-	struct tegra_ehci_platform_data *pdata;
 	int err = 0;
 	int irq;
 	struct device_node *np_phy;
 	struct usb_phy *u_phy;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(&pdev->dev, "Platform data missing\n");
-		return -EINVAL;
-	}
-
 	/* Right now device-tree probed devices don't get dma_mask set.
 	 * Since shared usb code relies on it, set it here for now.
 	 * Once we have dma capability bindings this can go away.
-- 
1.8.1.5

[PATCH 4/9] ARM: tegra: Remove USB platform data

[Tuomas Tynkkynen]

USB-related platform data is not used anymore in the Tegra USB drivers,
so remove all of it.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 arch/arm/mach-tegra/tegra.c             | 38 +--------------------------------
 include/linux/platform_data/tegra_usb.h | 32 ---------------------------
 include/linux/usb/tegra_usb_phy.h       |  5 -----
 3 files changed, 1 insertion(+), 74 deletions(-)
 delete mode 100644 include/linux/platform_data/tegra_usb.h

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 0d1e412..fc97cfd 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -29,7 +29,6 @@
 #include <linux/of_fdt.h>
 #include <linux/of_platform.h>
 #include <linux/pda_power.h>
-#include <linux/platform_data/tegra_usb.h>
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
@@ -46,40 +45,6 @@
 #include "fuse.h"
 #include "iomap.h"
 
-static struct tegra_ehci_platform_data tegra_ehci1_pdata = {
-	.operating_mode = TEGRA_USB_OTG,
-	.power_down_on_bus_suspend = 1,
-	.vbus_gpio = -1,
-};
-
-static struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
-	.reset_gpio = -1,
-	.clk = "cdev2",
-};
-
-static struct tegra_ehci_platform_data tegra_ehci2_pdata = {
-	.phy_config = &tegra_ehci2_ulpi_phy_config,
-	.operating_mode = TEGRA_USB_HOST,
-	.power_down_on_bus_suspend = 1,
-	.vbus_gpio = -1,
-};
-
-static struct tegra_ehci_platform_data tegra_ehci3_pdata = {
-	.operating_mode = TEGRA_USB_HOST,
-	.power_down_on_bus_suspend = 1,
-	.vbus_gpio = -1,
-};
-
-static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
-	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5000000, "tegra-ehci.0",
-		       &tegra_ehci1_pdata),
-	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5004000, "tegra-ehci.1",
-		       &tegra_ehci2_pdata),
-	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5008000, "tegra-ehci.2",
-		       &tegra_ehci3_pdata),
-	{}
-};
-
 static void __init tegra_dt_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
@@ -112,8 +77,7 @@ static void __init tegra_dt_init(void)
 	 * devices
 	 */
 out:
-	of_platform_populate(NULL, of_default_bus_match_table,
-				tegra20_auxdata_lookup, parent);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
 }
 
 static void __init trimslice_init(void)
diff --git a/include/linux/platform_data/tegra_usb.h b/include/linux/platform_data/tegra_usb.h
deleted file mode 100644
index 66c673f..0000000
--- a/include/linux/platform_data/tegra_usb.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2010 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef _TEGRA_USB_H_
-#define _TEGRA_USB_H_
-
-enum tegra_usb_operating_modes {
-	TEGRA_USB_DEVICE,
-	TEGRA_USB_HOST,
-	TEGRA_USB_OTG,
-};
-
-struct tegra_ehci_platform_data {
-	enum tegra_usb_operating_modes operating_mode;
-	/* power down the phy on bus suspend */
-	int power_down_on_bus_suspend;
-	void *phy_config;
-	int vbus_gpio;
-};
-
-#endif /* _TEGRA_USB_H_ */
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index c2bc710..8474153 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -28,11 +28,6 @@ struct tegra_utmip_config {
 	u8 xcvr_lsrslew;
 };
 
-struct tegra_ulpi_config {
-	int reset_gpio;
-	const char *clk;
-};
-
 enum tegra_usb_phy_port_speed {
 	TEGRA_USB_PHY_PORT_SPEED_FULL = 0,
 	TEGRA_USB_PHY_PORT_SPEED_LOW,
-- 
1.8.1.5

Re: [PATCH 4/9] ARM: tegra: Remove USB platform data

[Stephen Warren]

On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
> USB-related platform data is not used anymore in the Tegra USB drivers,
> so remove all of it.

Acked-by: Stephen Warren <swarren@nvidia.com>

[PATCH 6/9] usb: host: tegra: Locate a PHY via standard API

[Tuomas Tynkkynen]

Use devm_get_phy_by_phandle to get a PHY device instead of the custom
Tegra functions.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index a208cea..db8031f 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -328,7 +328,6 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	struct tegra_ehci_hcd *tegra;
 	int err = 0;
 	int irq;
-	struct device_node *np_phy;
 	struct usb_phy *u_phy;
 
 	/* Right now device-tree probed devices don't get dma_mask set.
@@ -367,13 +366,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	udelay(1);
 	tegra_periph_reset_deassert(tegra->clk);
 
-	np_phy = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
-	if (!np_phy) {
-		err = -ENODEV;
-		goto cleanup_clk_en;
-	}
-
-	u_phy = tegra_usb_get_phy(np_phy);
+	u_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "nvidia,phy", 0);
 	if (IS_ERR(u_phy)) {
 		err = PTR_ERR(u_phy);
 		goto cleanup_clk_en;
-- 
1.8.1.5

[PATCH 7/9] usb: phy: tegra: Remove custom PHY locating APIs

[Tuomas Tynkkynen]

The Tegra EHCI driver is no longer using these custom functions, so they
can be removed.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/usb/phy/phy-tegra-usb.c   | 24 ------------------------
 include/linux/usb/tegra_usb_phy.h |  2 --
 2 files changed, 26 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index b81310d..2cba13e 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -965,29 +965,5 @@ static struct platform_driver tegra_usb_phy_driver = {
 };
 module_platform_driver(tegra_usb_phy_driver);
 
-static int tegra_usb_phy_match(struct device *dev, void *data)
-{
-	struct tegra_usb_phy *tegra_phy = dev_get_drvdata(dev);
-	struct device_node *dn = data;
-
-	return (tegra_phy->u_phy.dev->of_node == dn) ? 1 : 0;
-}
-
-struct usb_phy *tegra_usb_get_phy(struct device_node *dn)
-{
-	struct device *dev;
-	struct tegra_usb_phy *tegra_phy;
-
-	dev = driver_find_device(&tegra_usb_phy_driver.driver, NULL, dn,
-				 tegra_usb_phy_match);
-	if (!dev)
-		return ERR_PTR(-EPROBE_DEFER);
-
-	tegra_phy = dev_get_drvdata(dev);
-
-	return &tegra_phy->u_phy;
-}
-EXPORT_SYMBOL_GPL(tegra_usb_get_phy);
-
 MODULE_DESCRIPTION("Tegra USB PHY driver");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index 8474153..a095c98 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -60,8 +60,6 @@ struct tegra_usb_phy {
 	int reset_gpio;
 };
 
-struct usb_phy *tegra_usb_get_phy(struct device_node *dn);
-
 void tegra_usb_phy_preresume(struct usb_phy *phy);
 
 void tegra_usb_phy_postresume(struct usb_phy *phy);
-- 
1.8.1.5

[PATCH 1/9] usb: phy: tegra: Remove unnecessary 'dev' field

[Tuomas Tynkkynen]

struct usb_phy already has a field for the device pointer, so this
unnecessary field can be removed.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/phy/phy-tegra-usb.c   | 27 +++++++++++++++------------
 include/linux/usb/tegra_usb_phy.h |  1 -
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 1a86da8..ddcac0b 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -211,7 +211,7 @@ static void set_phcd(struct tegra_usb_phy *phy, bool enable)
 
 static int utmip_pad_open(struct tegra_usb_phy *phy)
 {
-	phy->pad_clk = devm_clk_get(phy->dev, "utmi-pads");
+	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);
@@ -540,13 +540,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->dev, "gpio %d not set to 0\n", phy->reset_gpio);
+		dev_err(phy->u_phy.dev, "gpio %d not set to 0\n",
+			phy->reset_gpio);
 		return ret;
 	}
 	msleep(5);
 	ret = gpio_direction_output(phy->reset_gpio, 1);
 	if (ret < 0) {
-		dev_err(phy->dev, "gpio %d not set to 1\n", phy->reset_gpio);
+		dev_err(phy->u_phy.dev, "gpio %d not set to 1\n",
+			phy->reset_gpio);
 		return ret;
 	}
 
@@ -649,29 +651,30 @@ static int ulpi_open(struct tegra_usb_phy *phy)
 {
 	int err;
 
-	phy->clk = devm_clk_get(phy->dev, "ulpi-link");
+	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 = devm_gpio_request(phy->dev, phy->reset_gpio, "ulpi_phy_reset_b");
+	err = devm_gpio_request(phy->u_phy.dev, phy->reset_gpio,
+		"ulpi_phy_reset_b");
 	if (err < 0) {
-		dev_err(phy->dev, "request failed for gpio: %d\n",
+		dev_err(phy->u_phy.dev, "request failed for gpio: %d\n",
 		       phy->reset_gpio);
 		return err;
 	}
 
 	err = gpio_direction_output(phy->reset_gpio, 0);
 	if (err < 0) {
-		dev_err(phy->dev, "gpio %d direction not set to output\n",
+		dev_err(phy->u_phy.dev, "gpio %d direction not set to output\n",
 		       phy->reset_gpio);
 		return err;
 	}
 
 	phy->ulpi = otg_ulpi_create(&ulpi_viewport_access_ops, 0);
 	if (!phy->ulpi) {
-		dev_err(phy->dev, "otg_ulpi_create returned NULL\n");
+		dev_err(phy->u_phy.dev, "otg_ulpi_create returned NULL\n");
 		err = -ENOMEM;
 		return err;
 	}
@@ -686,7 +689,7 @@ static int tegra_usb_phy_init(struct tegra_usb_phy *phy)
 	int i;
 	int err;
 
-	phy->pll_u = devm_clk_get(phy->dev, "pll_u");
+	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);
@@ -712,7 +715,7 @@ static int tegra_usb_phy_init(struct tegra_usb_phy *phy)
 	if (!IS_ERR(phy->vbus)) {
 		err = regulator_enable(phy->vbus);
 		if (err) {
-			dev_err(phy->dev,
+			dev_err(phy->u_phy.dev,
 				"failed to enable usb vbus regulator: %d\n", err);
 			goto fail;
 		}
@@ -919,7 +922,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		tegra_phy->vbus = ERR_PTR(-ENODEV);
 	}
 
-	tegra_phy->dev = &pdev->dev;
+	tegra_phy->u_phy.dev = &pdev->dev;
 	err = tegra_usb_phy_init(tegra_phy);
 	if (err < 0)
 		return err;
@@ -952,7 +955,7 @@ static int tegra_usb_phy_match(struct device *dev, void *data)
 	struct tegra_usb_phy *tegra_phy = dev_get_drvdata(dev);
 	struct device_node *dn = data;
 
-	return (tegra_phy->dev->of_node == dn) ? 1 : 0;
+	return (tegra_phy->u_phy.dev->of_node == dn) ? 1 : 0;
 }
 
 struct usb_phy *tegra_usb_get_phy(struct device_node *dn)
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index 2b5fa94..c2bc710 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -60,7 +60,6 @@ struct tegra_usb_phy {
 	void *config;
 	struct usb_phy *ulpi;
 	struct usb_phy u_phy;
-	struct device *dev;
 	bool is_legacy_phy;
 	bool is_ulpi_phy;
 	int reset_gpio;
-- 
1.8.1.5

[PATCH 5/9] usb: phy: tegra: Register as an USB PHY.

[Tuomas Tynkkynen]

Register the Tegra PHY device instances with the PHY subsystem so that
the Tegra EHCI driver can locate a PHY via the standard APIs.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/phy/phy-tegra-usb.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index ddcac0b..b81310d 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -927,6 +927,12 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	err = usb_add_phy_dev(&tegra_phy->u_phy);
+	if (err < 0) {
+		tegra_usb_phy_close(&tegra_phy->u_phy);
+		return err;
+	}
+
 	tegra_phy->u_phy.shutdown = tegra_usb_phy_close;
 	tegra_phy->u_phy.set_suspend = tegra_usb_phy_suspend;
 
@@ -934,6 +940,14 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int tegra_usb_phy_remove(struct platform_device *pdev)
+{
+	struct tegra_usb_phy *tegra_phy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&tegra_phy->u_phy);
+	return 0;
+}
+
 static struct of_device_id tegra_usb_phy_id_table[] = {
 	{ .compatible = "nvidia,tegra20-usb-phy", },
 	{ },
@@ -942,6 +956,7 @@ MODULE_DEVICE_TABLE(of, tegra_usb_phy_id_table);
 
 static struct platform_driver tegra_usb_phy_driver = {
 	.probe		= tegra_usb_phy_probe,
+	.remove		= tegra_usb_phy_remove,
 	.driver		= {
 		.name	= "tegra-phy",
 		.owner	= THIS_MODULE,
-- 
1.8.1.5

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

Re: [PATCH 5/9] usb: phy: tegra: Register as an USB PHY.

[Stephen Warren]

On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
> Register the Tegra PHY device instances with the PHY subsystem so that
> the Tegra EHCI driver can locate a PHY via the standard APIs.

> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c

> +	err = usb_add_phy_dev(&tegra_phy->u_phy);
> +	if (err < 0) {
> +		tegra_usb_phy_close(&tegra_phy->u_phy);
> +		return err;
> +	}

Don't you want to do that a bit later? In particular ...

>  	tegra_phy->u_phy.shutdown = tegra_usb_phy_close;
>  	tegra_phy->u_phy.set_suspend = tegra_usb_phy_suspend;
> 
> 	dev_set_drvdata(&pdev->dev, tegra_phy);

... that all happens *after* the call to usb_add_phy_dev() call, yet I
think the USB core could legally call any of the PHY methods after the
call to usb_add_phy_dev(), and that probably requires all those methods
pointers and drvdata to be set up? I suggest moving the call to
usb_add_phy_dev() to the very end of the function.

> +static int tegra_usb_phy_remove(struct platform_device *pdev)
> +{
> +	struct tegra_usb_phy *tegra_phy = platform_get_drvdata(pdev);
> +
> +	usb_remove_phy(&tegra_phy->u_phy);
> +	return 0;
> +}

Nit: I'd typically expect to see a blank line before the return statement.

[PATCH 8/9] usb: phy: tegra: Use DT helpers for phy_type

[Tuomas Tynkkynen]

Use the new of_usb_get_phy_mode helper function for parsing phy_type
from the device tree.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/phy/phy-tegra-usb.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 2cba13e..13049ce 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -31,6 +31,7 @@
 #include <linux/of_gpio.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/ulpi.h>
+#include <linux/usb/of.h>
 #include <asm/mach-types.h>
 #include <linux/usb/ehci_def.h>
 #include <linux/usb/tegra_usb_phy.h>
@@ -859,6 +860,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct tegra_usb_phy *tegra_phy = NULL;
 	struct device_node *np = pdev->dev.of_node;
+	enum usb_phy_interface phy_type;
 	int err;
 
 	tegra_phy = devm_kzalloc(&pdev->dev, sizeof(*tegra_phy), GFP_KERNEL);
@@ -883,12 +885,12 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	tegra_phy->is_legacy_phy =
 		of_property_read_bool(np, "nvidia,has-legacy-mode");
 
-	err = of_property_match_string(np, "phy_type", "ulpi");
-	if (err < 0) {
+	phy_type = of_usb_get_phy_mode(np);
+	if (phy_type == USBPHY_INTERFACE_MODE_UTMI) {
 		err = utmi_phy_probe(tegra_phy, pdev);
 		if (err < 0)
 			return err;
-	} else {
+	} else if (phy_type == USBPHY_INTERFACE_MODE_ULPI) {
 		tegra_phy->is_ulpi_phy = true;
 
 		tegra_phy->reset_gpio =
@@ -898,8 +900,10 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 				tegra_phy->reset_gpio);
 			return tegra_phy->reset_gpio;
 		}
-
 		tegra_phy->config = NULL;
+	} else {
+		dev_err(&pdev->dev, "phy_type is invalid or unsupported\n");
+		return -EINVAL;
 	}
 
 	err = of_property_match_string(np, "dr_mode", "otg");
-- 
1.8.1.5

[PATCH 9/9] usb: phy: tegra: Use DT helpers for dr_mode

[Tuomas Tynkkynen]

Use the new of_usb_get_dr_mode helper function for parsing dr_mode
from the device tree. Also replace the usage of the custom
tegra_usb_phy_mode enum with the standard enum.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/phy/phy-tegra-usb.c   | 20 ++++++++------------
 include/linux/usb/tegra_usb_phy.h |  8 +-------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 13049ce..32d4394 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -377,7 +377,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
 		UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
 	writel(val, base + UTMIP_PLL_CFG1);
 
-	if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE) {
+	if (phy->mode == USB_DR_MODE_PERIPHERAL) {
 		val = readl(base + USB_SUSP_CTRL);
 		val &= ~(USB_WAKE_ON_CNNT_EN_DEV | USB_WAKE_ON_DISCON_EN_DEV);
 		writel(val, base + USB_SUSP_CTRL);
@@ -412,7 +412,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
 
 	if (phy->is_legacy_phy) {
 		val = readl(base + UTMIP_SPARE_CFG0);
-		if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE)
+		if (phy->mode == USB_DR_MODE_PERIPHERAL)
 			val &= ~FUSE_SETUP_SEL;
 		else
 			val |= FUSE_SETUP_SEL;
@@ -453,7 +453,7 @@ static int utmi_phy_power_off(struct tegra_usb_phy *phy)
 
 	utmi_phy_clk_disable(phy);
 
-	if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE) {
+	if (phy->mode == USB_DR_MODE_PERIPHERAL) {
 		val = readl(base + USB_SUSP_CTRL);
 		val &= ~USB_WAKEUP_DEBOUNCE_COUNT(~0);
 		val |= USB_WAKE_ON_CNNT_EN_DEV | USB_WAKEUP_DEBOUNCE_COUNT(5);
@@ -906,15 +906,11 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	err = of_property_match_string(np, "dr_mode", "otg");
-	if (err < 0) {
-		err = of_property_match_string(np, "dr_mode", "peripheral");
-		if (err < 0)
-			tegra_phy->mode = TEGRA_USB_PHY_MODE_HOST;
-		else
-			tegra_phy->mode = TEGRA_USB_PHY_MODE_DEVICE;
-	} else
-		tegra_phy->mode = TEGRA_USB_PHY_MODE_OTG;
+	tegra_phy->mode = of_usb_get_dr_mode(np);
+	if (tegra_phy->mode == USB_DR_MODE_UNKNOWN) {
+		dev_err(&pdev->dev, "dr_mode is invalid\n");
+		return -EINVAL;
+	}
 
 	/* On some boards, the VBUS regulator doesn't need to be controlled */
 	if (of_find_property(np, "vbus-supply", NULL)) {
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index a095c98..d3db274 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -34,12 +34,6 @@ enum tegra_usb_phy_port_speed {
 	TEGRA_USB_PHY_PORT_SPEED_HIGH,
 };
 
-enum tegra_usb_phy_mode {
-	TEGRA_USB_PHY_MODE_DEVICE,
-	TEGRA_USB_PHY_MODE_HOST,
-	TEGRA_USB_PHY_MODE_OTG,
-};
-
 struct tegra_xtal_freq;
 
 struct tegra_usb_phy {
@@ -51,7 +45,7 @@ struct tegra_usb_phy {
 	struct clk *pll_u;
 	struct clk *pad_clk;
 	struct regulator *vbus;
-	enum tegra_usb_phy_mode mode;
+	enum usb_dr_mode mode;
 	void *config;
 	struct usb_phy *ulpi;
 	struct usb_phy u_phy;
-- 
1.8.1.5

Re: [PATCH 9/9] usb: phy: tegra: Use DT helpers for dr_mode

[Stephen Warren]

On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
> Use the new of_usb_get_dr_mode helper function for parsing dr_mode
> from the device tree. Also replace the usage of the custom
> tegra_usb_phy_mode enum with the standard enum.

> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> +	tegra_phy->mode = of_usb_get_dr_mode(np);
> +	if (tegra_phy->mode == USB_DR_MODE_UNKNOWN) {
> +		dev_err(&pdev->dev, "dr_mode is invalid\n");
> +		return -EINVAL;
> +	}

Unfortunately, of_usb_get_dr_mode() returns USB_DR_MODE_UNKNOWN if the
property is missing, rather than defaulting to host mode as the original
code here did. I would suggest solving this by:

> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> index 675384d..6391de5 100644
> --- a/drivers/usb/usb-common.c
> +++ b/drivers/usb/usb-common.c
> @@ -103,7 +103,7 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
>  
>         err = of_property_read_string(np, "dr_mode", &dr_mode);
>         if (err < 0)
> -               return USB_DR_MODE_UNKNOWN;
> +               return USB_DR_MODE_HOST;
>  
>         for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
>                 if (!strcmp(dr_mode, usb_dr_modes[i]))

This can't be done by the caller, since of_usb_get_dr_mode() returns
UNKNOWN in two cases, which the caller can't distinguish without
manually checking whether the property exists first:

a) Property is not present (which should default to HOST mode at least
for the Tegra binding, as in the patch I show above).

b) Property is present, but contains an invalid value, which probably
should cause the driver to error-out.

This is only a problem for dr_mode in the Tegra binding: dr_mode is an
optional property, whereas the phy_type property as parsed by patch 8/9
is mandatory, so this issue doesn't come up.

Re: [PATCH 0/9] Tegra USB cleanup series

[Stephen Warren]

On 06/28/2013 03:36 PM, Tuomas Tynkkynen wrote:
> Hi,
> 
> Here's a few cleanup patches for the Tegra USB drivers, to be applied on top
> of Mikko's two patch sets. It mostly deals with removing all usage of platform
> data and using standard helpers and enums instead of Tegra-specific ones.

With the issues in patches 5/9 and 9/9 fixed as I mentioned, this series,

Tested-by: Stephen Warren <swarren@nvidia.com>

Re: [PATCH 0/9] Tegra USB cleanup series

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Hi,

On Mon, Jul 01, 2013 at 04:03:41PM -0600, Stephen Warren wrote:
> On 06/28/2013 03:36 PM, Tuomas Tynkkynen wrote:
> > Hi,
> > 
> > Here's a few cleanup patches for the Tegra USB drivers, to be applied on top
> > of Mikko's two patch sets. It mostly deals with removing all usage of platform
> > data and using standard helpers and enums instead of Tegra-specific ones.
> 
> With the issues in patches 5/9 and 9/9 fixed as I mentioned, this series,
> 
> Tested-by: Stephen Warren <swarren@nvidia.com>

so, do you want me to apply these myself ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/9] Tegra USB cleanup series

[Stephen Warren]

On 07/24/2013 05:32 AM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jul 01, 2013 at 04:03:41PM -0600, Stephen Warren wrote:
>> On 06/28/2013 03:36 PM, Tuomas Tynkkynen wrote:
>>> Hi,
>>> 
>>> Here's a few cleanup patches for the Tegra USB drivers, to be
>>> applied on top of Mikko's two patch sets. It mostly deals with
>>> removing all usage of platform data and using standard helpers
>>> and enums instead of Tegra-specific ones.
>> 
>> With the issues in patches 5/9 and 9/9 fixed as I mentioned, this
>> series,
>> 
>> Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> so, do you want me to apply these myself ?

Yes, I believe if all of Tuomas's and Mikko's recent USB patches can
go through one of the USB trees, that would be best. Thanks!