Based and tested on 'usb-next' branch of Greg's usb tree, with relevant
device tree patches[1]
Changes from v2:
- Added two patches in the series for some cleanup.
usb: ohci-exynos: Use struct device instead of platform_device
usb: ehci-exynos: Use struct device instead of platform_device
- Addressed review comments.
-- Moved exynos_ohci_phyg_on()/off routines inside
exynos_ohci_phy_enable()/disable.
-- Added functions exynos_ehci_phy_enable() and exynos_ehci_phy_disable()
and moved exynos_ehci_phyg_on()/off routines respectively in them.
-- Added necessary checks.
Kamil Debski (1):
usb: ehci-exynos: Change to use phy provided by the generic phy
framework
Vivek Gautam (3):
usb: ohci-exynos: Use struct device instead of platform_device
usb: ehci-exynos: Use struct device instead of platform_device
usb: ohci-exynos: Add facility to use phy provided by the generic phy
framework
.../devicetree/bindings/usb/exynos-usb.txt | 37 +++++
drivers/usb/host/ehci-exynos.c | 148 +++++++++++++++++---
drivers/usb/host/ohci-exynos.c | 141 ++++++++++++++++---
3 files changed, 280 insertions(+), 46 deletions(-)
--
1.7.10.4
From: Kamil Debski <[email protected]>
Add the phy provider, supplied by new Exynos-usb2phy using
Generic phy framework.
Keeping the support for older USB phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ehci-exynos.
Once we move to new phy in the device nodes for ehci, we can
remove the support for older phys.
Signed-off-by: Kamil Debski <[email protected]>
[[email protected]: Addressed review comments from mailing list]
[[email protected]: Kept the code for old usb-phy, and just
added support for new exynos5-usb2phy in generic phy framework]
[[email protected]: Edited the commit message]
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Alan Stern <[email protected]>
---
.../devicetree/bindings/usb/exynos-usb.txt | 18 +++
drivers/usb/host/ehci-exynos.c | 143 +++++++++++++++++---
2 files changed, 141 insertions(+), 20 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 03b7e43..4f368b0 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,15 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be "usbhost".
+ - port: if in the SoC there are EHCI phys, they should be listed here.
+ One phy per port. Each port should have its 'reg' entry.
+ - reg: port number on EHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings; specifying phy used by port.
+ - phy-names: from the *Generic PHY* bindings; specifying name of phy
+ used by the port.
Optional properties:
- samsung,vbus-gpio: if present, specifies the GPIO that
@@ -27,6 +36,15 @@ Example:
clocks = <&clock 285>;
clock-names = "usbhost";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ phys = <&usb2phy 1>;
+ phy-names = "host";
+ status = "disabled";
+ };
};
OHCI
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 4d763dc..caeadb4 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/usb/phy.h>
#include <linux/usb/samsung_usb_phy.h>
@@ -42,14 +43,119 @@
static const char hcd_name[] = "ehci-exynos";
static struct hc_driver __read_mostly exynos_ehci_hc_driver;
+#define PHY_NUMBER 3
struct exynos_ehci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
+static int exynos_ehci_get_phy(struct device *dev,
+ struct exynos_ehci_hcd *exynos_ehci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ehci->phy)) {
+ ret = PTR_ERR(exynos_ehci->phy);
+ /* This is the case when PHY config is disabled */
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dev_dbg(dev, "Failed to get usb2 phy\n");
+ exynos_ehci->phy = NULL;
+ ret = 0;
+ } else if (ret == -EPROBE_DEFER) {
+ goto fail_phy;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ goto fail_phy;
+ }
+ } else {
+ exynos_ehci->otg = exynos_ehci->phy->otg;
+ }
+
+ for_each_available_child_of_node(dev->of_node, child) {
+ ret = of_property_read_u32(child, "reg", &phy_number);
+ if (ret) {
+ dev_err(dev, "Failed to parse device tree\n");
+ of_node_put(child);
+ goto fail_phy;
+ }
+ if (phy_number >= PHY_NUMBER) {
+ dev_err(dev, "Invalid number of PHYs\n");
+ of_node_put(child);
+ ret = -EINVAL;
+ goto fail_phy;
+ }
+ phy = devm_of_phy_get(dev, child, 0);
+ of_node_put(child);
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ /* This is the case when PHY config is disabled */
+ if (ret == -ENOSYS || ret == -ENODEV) {
+ dev_dbg(dev, "Failed to get usb2 phy\n");
+ phy = NULL;
+ ret = 0;
+ } else if (ret == -EPROBE_DEFER) {
+ goto fail_phy;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ goto fail_phy;
+ }
+ }
+ exynos_ehci->phy_g[phy_number] = phy;
+ }
+
+fail_phy:
+ return ret;
+}
+
+static int exynos_ehci_phy_enable(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
+ int i;
+ int ret = 0;
+
+ if (exynos_ehci->phy) {
+ ret = usb_phy_init(exynos_ehci->phy);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+ if (exynos_ehci->phy_g[i])
+ ret = phy_power_on(exynos_ehci->phy_g[i]);
+ if (ret)
+ for (i--; i >= 0; i--)
+ if (exynos_ehci->phy_g[i])
+ phy_power_off(exynos_ehci->phy_g[i]);
+
+ return ret;
+}
+
+static int exynos_ehci_phy_disable(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
+ int i;
+ int ret = 0;
+
+ if (exynos_ehci->phy)
+ usb_phy_shutdown(exynos_ehci->phy);
+
+ for (i = 0; i < PHY_NUMBER; i++)
+ if (exynos_ehci->phy_g[i])
+ ret = phy_power_off(exynos_ehci->phy_g[i]);
+
+ return ret;
+}
+
static void exynos_setup_vbus_gpio(struct device *dev)
{
int err;
@@ -74,7 +180,6 @@ static int exynos_ehci_probe(struct platform_device *pdev)
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
struct resource *res;
- struct usb_phy *phy;
int irq;
int err;
@@ -101,15 +206,9 @@ static int exynos_ehci_probe(struct platform_device *pdev)
"samsung,exynos5440-ehci"))
goto skip_phy;
- phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(phy)) {
- usb_put_hcd(hcd);
- dev_warn(&pdev->dev, "no platform data or transceiver defined\n");
- return -EPROBE_DEFER;
- } else {
- exynos_ehci->phy = phy;
- exynos_ehci->otg = phy->otg;
- }
+ err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
+ if (err)
+ goto fail_clk;
skip_phy:
@@ -151,8 +250,11 @@ skip_phy:
if (exynos_ehci->otg)
exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
- if (exynos_ehci->phy)
- usb_phy_init(exynos_ehci->phy);
+ err = exynos_ehci_phy_enable(&pdev->dev);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to enable USB phy\n");
+ goto fail_io;
+ }
ehci = hcd_to_ehci(hcd);
ehci->caps = hcd->regs;
@@ -172,8 +274,7 @@ skip_phy:
return 0;
fail_add_hcd:
- if (exynos_ehci->phy)
- usb_phy_shutdown(exynos_ehci->phy);
+ exynos_ehci_phy_disable(&pdev->dev);
fail_io:
clk_disable_unprepare(exynos_ehci->clk);
fail_clk:
@@ -191,8 +292,7 @@ static int exynos_ehci_remove(struct platform_device *pdev)
if (exynos_ehci->otg)
exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
- if (exynos_ehci->phy)
- usb_phy_shutdown(exynos_ehci->phy);
+ exynos_ehci_phy_disable(&pdev->dev);
clk_disable_unprepare(exynos_ehci->clk);
@@ -217,8 +317,7 @@ static int exynos_ehci_suspend(struct device *dev)
if (exynos_ehci->otg)
exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
- if (exynos_ehci->phy)
- usb_phy_shutdown(exynos_ehci->phy);
+ exynos_ehci_phy_disable(dev);
clk_disable_unprepare(exynos_ehci->clk);
@@ -229,14 +328,18 @@ static int exynos_ehci_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
+ int ret;
clk_prepare_enable(exynos_ehci->clk);
if (exynos_ehci->otg)
exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
- if (exynos_ehci->phy)
- usb_phy_init(exynos_ehci->phy);
+ ret = exynos_ehci_phy_enable(dev);
+ if (ret) {
+ dev_err(dev, "Failed to enable USB phy\n");
+ return ret;
+ }
/* DMA burst Enable */
writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
--
1.7.10.4
Change to use struct device instead of struct platform_device
for some static functions.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Alan Stern <[email protected]>
---
drivers/usb/host/ehci-exynos.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 7f425ac..4d763dc 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -50,9 +50,8 @@ struct exynos_ehci_hcd {
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
-static void exynos_setup_vbus_gpio(struct platform_device *pdev)
+static void exynos_setup_vbus_gpio(struct device *dev)
{
- struct device *dev = &pdev->dev;
int err;
int gpio;
@@ -88,7 +87,7 @@ static int exynos_ehci_probe(struct platform_device *pdev)
if (err)
return err;
- exynos_setup_vbus_gpio(pdev);
+ exynos_setup_vbus_gpio(&pdev->dev);
hcd = usb_create_hcd(&exynos_ehci_hc_driver,
&pdev->dev, dev_name(&pdev->dev));
--
1.7.10.4
Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Alan Stern <[email protected]>
---
.../devicetree/bindings/usb/exynos-usb.txt | 19 +++
drivers/usb/host/ohci-exynos.c | 129 +++++++++++++++++---
2 files changed, 132 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..03b7e43 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,15 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be "usbhost".
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+ One phy per port. Each port should have its 'reg' entry.
+ - reg: port number on OHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings specifying phy used by port.
+ - phy-names: from the *Generic PHY* bindings specifying name of phy
+ used by the port.
Example:
usb@12120000 {
@@ -47,6 +56,16 @@ Example:
clocks = <&clock 285>;
clock-names = "usbhost";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ phys = <&usb2phy 1>;
+ phy-names = "host";
+ status = "disabled";
+ };
+
};
DWC3
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 05f00e3..011ccde 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
#include <linux/usb/phy.h>
#include <linux/usb/samsung_usb_phy.h>
#include <linux/usb.h>
@@ -33,28 +34,122 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver;
#define to_exynos_ohci(hcd) (struct exynos_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
+#define PHY_NUMBER 3
struct exynos_ohci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
-static void exynos_ohci_phy_enable(struct device *dev)
+static int exynos_ohci_get_phy(struct device *dev,
+ struct exynos_ohci_hcd *exynos_ohci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ohci->phy)) {
+ ret = PTR_ERR(exynos_ohci->phy);
+ /* This is the case when PHY config is disabled */
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dev_dbg(dev, "Failed to get usb2 phy\n");
+ exynos_ohci->phy = NULL;
+ ret = 0;
+ } else if (ret == -EPROBE_DEFER) {
+ goto fail_phy;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ goto fail_phy;
+ }
+ } else {
+ exynos_ohci->otg = exynos_ohci->phy->otg;
+ }
+
+ /* Getting generic phy:
+ * We are keeping both types of phys as a part of transiting OHCI
+ * to generic phy framework, so that in absence of supporting dts
+ * changes the functionality doesn't break.
+ * Once we move the ohci dt nodes to use new generic phys,
+ * we can remove support for older PHY in this driver.
+ */
+ for_each_available_child_of_node(dev->of_node, child) {
+ ret = of_property_read_u32(child, "reg", &phy_number);
+ if (ret) {
+ dev_err(dev, "Failed to parse device tree\n");
+ of_node_put(child);
+ goto fail_phy;
+ }
+ if (phy_number >= PHY_NUMBER) {
+ dev_err(dev, "Invalid number of PHYs\n");
+ of_node_put(child);
+ ret = -EINVAL;
+ goto fail_phy;
+ }
+ phy = devm_of_phy_get(dev, child, 0);
+ of_node_put(child);
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ /* This is the case when PHY config is disabled */
+ if (ret == -ENOSYS || ret == -ENODEV) {
+ dev_dbg(dev, "Failed to get usb2 phy\n");
+ phy = NULL;
+ ret = 0;
+ } else if (ret == -EPROBE_DEFER) {
+ goto fail_phy;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ goto fail_phy;
+ }
+ }
+ exynos_ohci->phy_g[phy_number] = phy;
+ }
+
+fail_phy:
+ return ret;
+}
+
+static int exynos_ohci_phy_enable(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
+ int i;
+ int ret = 0;
- if (exynos_ohci->phy)
- usb_phy_init(exynos_ohci->phy);
+ if (exynos_ohci->phy) {
+ ret = usb_phy_init(exynos_ohci->phy);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+ if (exynos_ohci->phy_g[i])
+ ret = phy_power_on(exynos_ohci->phy_g[i]);
+ if (ret)
+ for (i--; i >= 0; i--)
+ if (exynos_ohci->phy_g[i])
+ phy_power_off(exynos_ohci->phy_g[i]);
+
+ return ret;
}
-static void exynos_ohci_phy_disable(struct device *dev)
+static int exynos_ohci_phy_disable(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
+ int i;
+ int ret = 0;
if (exynos_ohci->phy)
usb_phy_shutdown(exynos_ohci->phy);
+
+ for (i = 0; i < PHY_NUMBER; i++)
+ if (exynos_ohci->phy_g[i])
+ ret = phy_power_off(exynos_ohci->phy_g[i]);
+
+ return ret;
}
static int exynos_ohci_probe(struct platform_device *pdev)
@@ -62,7 +157,6 @@ static int exynos_ohci_probe(struct platform_device *pdev)
struct exynos_ohci_hcd *exynos_ohci;
struct usb_hcd *hcd;
struct resource *res;
- struct usb_phy *phy;
int irq;
int err;
@@ -88,15 +182,9 @@ static int exynos_ohci_probe(struct platform_device *pdev)
"samsung,exynos5440-ohci"))
goto skip_phy;
- phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(phy)) {
- usb_put_hcd(hcd);
- dev_warn(&pdev->dev, "no platform data or transceiver defined\n");
- return -EPROBE_DEFER;
- } else {
- exynos_ohci->phy = phy;
- exynos_ohci->otg = phy->otg;
- }
+ err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
+ if (err)
+ goto fail_clk;
skip_phy:
exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
@@ -139,7 +227,11 @@ skip_phy:
platform_set_drvdata(pdev, hcd);
- exynos_ohci_phy_enable(&pdev->dev);
+ err = exynos_ohci_phy_enable(&pdev->dev);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to enable USB phy\n");
+ goto fail_io;
+ }
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
@@ -210,13 +302,18 @@ static int exynos_ohci_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
+ int ret;
clk_prepare_enable(exynos_ohci->clk);
if (exynos_ohci->otg)
exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
- exynos_ohci_phy_enable(dev);
+ ret = exynos_ohci_phy_enable(dev);
+ if (ret) {
+ dev_err(dev, "Failed to enable USB phy\n");
+ return ret;
+ }
ohci_resume(hcd, false);
--
1.7.10.4
Change to use struct device instead of struct platform_device
for some static functions.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Alan Stern <[email protected]>
---
drivers/usb/host/ohci-exynos.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 9cf80cb..05f00e3 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -39,18 +39,18 @@ struct exynos_ohci_hcd {
struct usb_otg *otg;
};
-static void exynos_ohci_phy_enable(struct platform_device *pdev)
+static void exynos_ohci_phy_enable(struct device *dev)
{
- struct usb_hcd *hcd = platform_get_drvdata(pdev);
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
if (exynos_ohci->phy)
usb_phy_init(exynos_ohci->phy);
}
-static void exynos_ohci_phy_disable(struct platform_device *pdev)
+static void exynos_ohci_phy_disable(struct device *dev)
{
- struct usb_hcd *hcd = platform_get_drvdata(pdev);
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
if (exynos_ohci->phy)
@@ -139,7 +139,7 @@ skip_phy:
platform_set_drvdata(pdev, hcd);
- exynos_ohci_phy_enable(pdev);
+ exynos_ohci_phy_enable(&pdev->dev);
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
@@ -150,7 +150,7 @@ skip_phy:
return 0;
fail_add_hcd:
- exynos_ohci_phy_disable(pdev);
+ exynos_ohci_phy_disable(&pdev->dev);
fail_io:
clk_disable_unprepare(exynos_ohci->clk);
fail_clk:
@@ -168,7 +168,7 @@ static int exynos_ohci_remove(struct platform_device *pdev)
if (exynos_ohci->otg)
exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
- exynos_ohci_phy_disable(pdev);
+ exynos_ohci_phy_disable(&pdev->dev);
clk_disable_unprepare(exynos_ohci->clk);
@@ -190,7 +190,6 @@ static int exynos_ohci_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
- struct platform_device *pdev = to_platform_device(dev);
bool do_wakeup = device_may_wakeup(dev);
int rc = ohci_suspend(hcd, do_wakeup);
@@ -200,7 +199,7 @@ static int exynos_ohci_suspend(struct device *dev)
if (exynos_ohci->otg)
exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
- exynos_ohci_phy_disable(pdev);
+ exynos_ohci_phy_disable(dev);
clk_disable_unprepare(exynos_ohci->clk);
@@ -211,14 +210,13 @@ static int exynos_ohci_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
- struct platform_device *pdev = to_platform_device(dev);
clk_prepare_enable(exynos_ohci->clk);
if (exynos_ohci->otg)
exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
- exynos_ohci_phy_enable(pdev);
+ exynos_ohci_phy_enable(dev);
ohci_resume(hcd, false);
--
1.7.10.4
On Mon, Apr 28, 2014 at 2:55 PM, Vivek Gautam <[email protected]> wrote:
> Based and tested on 'usb-next' branch of Greg's usb tree, with relevant
> device tree patches[1]
[1] [PATCH v7 0/2] dts: Add usb2phy to Exynos 5250
http://www.spinics.net/lists/linux-usb/msg106427.html
>
> Changes from v2:
> - Added two patches in the series for some cleanup.
> usb: ohci-exynos: Use struct device instead of platform_device
> usb: ehci-exynos: Use struct device instead of platform_device
> - Addressed review comments.
> -- Moved exynos_ohci_phyg_on()/off routines inside
> exynos_ohci_phy_enable()/disable.
> -- Added functions exynos_ehci_phy_enable() and exynos_ehci_phy_disable()
> and moved exynos_ehci_phyg_on()/off routines respectively in them.
> -- Added necessary checks.
>
> Kamil Debski (1):
> usb: ehci-exynos: Change to use phy provided by the generic phy
> framework
>
> Vivek Gautam (3):
> usb: ohci-exynos: Use struct device instead of platform_device
> usb: ehci-exynos: Use struct device instead of platform_device
> usb: ohci-exynos: Add facility to use phy provided by the generic phy
> framework
>
> .../devicetree/bindings/usb/exynos-usb.txt | 37 +++++
> drivers/usb/host/ehci-exynos.c | 148 +++++++++++++++++---
> drivers/usb/host/ohci-exynos.c | 141 ++++++++++++++++---
> 3 files changed, 280 insertions(+), 46 deletions(-)
>
> --
> 1.7.10.4
>
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
On Monday, April 28, 2014 6:25 PM, Vivek Gautam wrote:
>
> Change to use struct device instead of struct platform_device
> for some static functions.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Jingoo Han <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Best regards,
Jingoo Han
> Cc: Alan Stern <[email protected]>
> ---
> drivers/usb/host/ohci-exynos.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 9cf80cb..05f00e3 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -39,18 +39,18 @@ struct exynos_ohci_hcd {
> struct usb_otg *otg;
> };
>
> -static void exynos_ohci_phy_enable(struct platform_device *pdev)
> +static void exynos_ohci_phy_enable(struct device *dev)
> {
> - struct usb_hcd *hcd = platform_get_drvdata(pdev);
> + struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>
> if (exynos_ohci->phy)
> usb_phy_init(exynos_ohci->phy);
> }
>
> -static void exynos_ohci_phy_disable(struct platform_device *pdev)
> +static void exynos_ohci_phy_disable(struct device *dev)
> {
> - struct usb_hcd *hcd = platform_get_drvdata(pdev);
> + struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>
> if (exynos_ohci->phy)
> @@ -139,7 +139,7 @@ skip_phy:
>
> platform_set_drvdata(pdev, hcd);
>
> - exynos_ohci_phy_enable(pdev);
> + exynos_ohci_phy_enable(&pdev->dev);
>
> err = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (err) {
> @@ -150,7 +150,7 @@ skip_phy:
> return 0;
>
> fail_add_hcd:
> - exynos_ohci_phy_disable(pdev);
> + exynos_ohci_phy_disable(&pdev->dev);
> fail_io:
> clk_disable_unprepare(exynos_ohci->clk);
> fail_clk:
> @@ -168,7 +168,7 @@ static int exynos_ohci_remove(struct platform_device *pdev)
> if (exynos_ohci->otg)
> exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
>
> - exynos_ohci_phy_disable(pdev);
> + exynos_ohci_phy_disable(&pdev->dev);
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> @@ -190,7 +190,6 @@ static int exynos_ohci_suspend(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> - struct platform_device *pdev = to_platform_device(dev);
> bool do_wakeup = device_may_wakeup(dev);
> int rc = ohci_suspend(hcd, do_wakeup);
>
> @@ -200,7 +199,7 @@ static int exynos_ohci_suspend(struct device *dev)
> if (exynos_ohci->otg)
> exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
>
> - exynos_ohci_phy_disable(pdev);
> + exynos_ohci_phy_disable(dev);
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> @@ -211,14 +210,13 @@ static int exynos_ohci_resume(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> - struct platform_device *pdev = to_platform_device(dev);
>
> clk_prepare_enable(exynos_ohci->clk);
>
> if (exynos_ohci->otg)
> exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
>
> - exynos_ohci_phy_enable(pdev);
> + exynos_ohci_phy_enable(dev);
>
> ohci_resume(hcd, false);
>
> --
> 1.7.10.4
On Monday, April 28, 2014 6:26 PM, Vivek Gautam wrote:
>
> Change to use struct device instead of struct platform_device
> for some static functions.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Jingoo Han <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Best regards,
Jingoo Han
> Cc: Alan Stern <[email protected]>
> ---
> drivers/usb/host/ehci-exynos.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 7f425ac..4d763dc 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -50,9 +50,8 @@ struct exynos_ehci_hcd {
>
> #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
>
> -static void exynos_setup_vbus_gpio(struct platform_device *pdev)
> +static void exynos_setup_vbus_gpio(struct device *dev)
> {
> - struct device *dev = &pdev->dev;
> int err;
> int gpio;
>
> @@ -88,7 +87,7 @@ static int exynos_ehci_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> - exynos_setup_vbus_gpio(pdev);
> + exynos_setup_vbus_gpio(&pdev->dev);
>
> hcd = usb_create_hcd(&exynos_ehci_hc_driver,
> &pdev->dev, dev_name(&pdev->dev));
> --
> 1.7.10.4
On Mon, 28 Apr 2014, Vivek Gautam wrote:
> Add support to consume phy provided by Generic phy framework.
> Keeping the support for older usb-phy intact right now, in order
> to prevent any functionality break in absence of relevant
> device tree side change for ohci-exynos.
> Once we move to new phy in the device nodes for ohci, we can
> remove the support for older phys.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Alan Stern <[email protected]>
> ---
> +static int exynos_ohci_phy_enable(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> + int i;
> + int ret = 0;
>
> - if (exynos_ohci->phy)
> - usb_phy_init(exynos_ohci->phy);
> + if (exynos_ohci->phy) {
> + ret = usb_phy_init(exynos_ohci->phy);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> + if (exynos_ohci->phy_g[i])
> + ret = phy_power_on(exynos_ohci->phy_g[i]);
> + if (ret)
> + for (i--; i >= 0; i--)
> + if (exynos_ohci->phy_g[i])
> + phy_power_off(exynos_ohci->phy_g[i]);
Do you want to call usb_phy_shutdown() at this point?
> +
> + return ret;
> }
>
> -static void exynos_ohci_phy_disable(struct device *dev)
> +static int exynos_ohci_phy_disable(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> + int i;
> + int ret = 0;
>
> if (exynos_ohci->phy)
> usb_phy_shutdown(exynos_ohci->phy);
> +
> + for (i = 0; i < PHY_NUMBER; i++)
> + if (exynos_ohci->phy_g[i])
> + ret = phy_power_off(exynos_ohci->phy_g[i]);
> +
> + return ret;
> }
This return value is practically meaningless. It is the status from
the last PHY only; any errors involving the other PHYs have been lost.
You may as well make this function return void.
> @@ -210,13 +302,18 @@ static int exynos_ohci_resume(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> + int ret;
>
> clk_prepare_enable(exynos_ohci->clk);
>
> if (exynos_ohci->otg)
> exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
>
> - exynos_ohci_phy_enable(dev);
> + ret = exynos_ohci_phy_enable(dev);
> + if (ret) {
> + dev_err(dev, "Failed to enable USB phy\n");
Do you want to call clk_disable_unprepare() here?
> + return ret;
> + }
>
> ohci_resume(hcd, false);
Alan Stern
Hi,
On Mon, Apr 28, 2014 at 9:11 PM, Alan Stern <[email protected]> wrote:
> On Mon, 28 Apr 2014, Vivek Gautam wrote:
>
>> Add support to consume phy provided by Generic phy framework.
>> Keeping the support for older usb-phy intact right now, in order
>> to prevent any functionality break in absence of relevant
>> device tree side change for ohci-exynos.
>> Once we move to new phy in the device nodes for ohci, we can
>> remove the support for older phys.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Jingoo Han <[email protected]>
>> Cc: Alan Stern <[email protected]>
>> ---
>
>> +static int exynos_ohci_phy_enable(struct device *dev)
>> {
>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>> + int i;
>> + int ret = 0;
>>
>> - if (exynos_ohci->phy)
>> - usb_phy_init(exynos_ohci->phy);
>> + if (exynos_ohci->phy) {
>> + ret = usb_phy_init(exynos_ohci->phy);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
>> + if (exynos_ohci->phy_g[i])
>> + ret = phy_power_on(exynos_ohci->phy_g[i]);
>> + if (ret)
>> + for (i--; i >= 0; i--)
>> + if (exynos_ohci->phy_g[i])
>> + phy_power_off(exynos_ohci->phy_g[i]);
>
> Do you want to call usb_phy_shutdown() at this point?
Yes, you are right. We should be calling usb_phy_shutdown() here. But
the two phy-provider
drivers should never work together, so one of the above PHYs will not exist.
Anyways, for code correctness too, we should be doing as you suggested.
I will change this.
>
>> +
>> + return ret;
>> }
>>
>> -static void exynos_ohci_phy_disable(struct device *dev)
>> +static int exynos_ohci_phy_disable(struct device *dev)
>> {
>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>> + int i;
>> + int ret = 0;
>>
>> if (exynos_ohci->phy)
>> usb_phy_shutdown(exynos_ohci->phy);
>> +
>> + for (i = 0; i < PHY_NUMBER; i++)
>> + if (exynos_ohci->phy_g[i])
>> + ret = phy_power_off(exynos_ohci->phy_g[i]);
>> +
>> + return ret;
>> }
>
> This return value is practically meaningless. It is the status from
> the last PHY only; any errors involving the other PHYs have been lost.
>
> You may as well make this function return void.
Right, i will make this function return void and remove 'ret' from it.
>
>> @@ -210,13 +302,18 @@ static int exynos_ohci_resume(struct device *dev)
>> {
>> struct usb_hcd *hcd = dev_get_drvdata(dev);
>> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>> + int ret;
>>
>> clk_prepare_enable(exynos_ohci->clk);
>>
>> if (exynos_ohci->otg)
>> exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
>>
>> - exynos_ohci_phy_enable(dev);
>> + ret = exynos_ohci_phy_enable(dev);
>> + if (ret) {
>> + dev_err(dev, "Failed to enable USB phy\n");
>
> Do you want to call clk_disable_unprepare() here?
Yes, we should be calling clk_disable_unprepate() here to avoid the
warning in the next suspend cycle.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India