[PATCH v3 0/3] phy: Add new Exynos USB 2.0 PHY driver

[PATCH v3 0/3] phy: Add new Exynos USB 2.0 PHY driver

[Kamil Debski]

Hi,

This is the third version of the patchset adding the new Exynos USB 2.0 PHY
driver, which uses the Generic PHY Framework.

It contains numerous fixes and cleanups, which are the results of the feedback
to the second version.

Best wishes,
Kamil Debski

----------------
Changes from v2:
- rebase all patches to the usb-next branch
- fixes in the documentation file
  - remove wrong entries in the phy node (ranges, and #address- & #size-cells)
  - add clocks and clock-names as required properites
  - rephrase a few sentences
- fixes in the ehci-exynos.c file
  - move phy_name variable next to phy in exynos_ehci_hcd
  - remove otg from exynos_ehci_hcd as it was no longer used
  - move devm_phy_get after the Exynos5440 skip_phy check
- fixes in the s3c-hsotg.c file
  - cosmetic fixes (remove empty line that was wrongfully added)
- fixes in the main driver
  - remove cpu_type in favour for a boolean flag matched with the compatible
    value
  - rename files, structures, variables and Kconfig entires - change from simple
    "uphy" to "usb2_phy"
  - fix multiline comments style
  - simplify #ifdefs in of_device_id
  - fix Kconfig description
  - change dev_info to dev_dbg where reasonable
  - cosmetic changes (remove wrongful blank lines)
  - remove unnecessary reference counting

----------------
Changes from v1:
- the changes include minor fixes of the hardware initialization of the PHY
  module
- some other minor fixes were introduced

----------------------
Original cover letter:

Hi,

This patch adds a new drive for USB PHYs for Samsung SoCs. The driver is
using the Generic PHY Framework created by Kishon Vijay Abrahan I. It can
be found here https://lkml.org/lkml/2013/8/21/29. This patch adds support
to Exynos4 family of SoCs. Support for Exynos3 and Exynos5 is planned to
be added in the near future.

I welcome your comments.

----------------------

[1] https://lkml.org/lkml/2013/8/21/29

*** BLURB HERE ***

Kamil Debski (3):
  phy: Add new Exynos USB PHY driver
  usb: ehci-s5p: Change to use phy provided by the generic phy
    framework
  usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic
    phy framework

 .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
 drivers/phy/Kconfig                                |   23 +-
 drivers/phy/Makefile                               |    4 +
 drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
 drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
 drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
 drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
 drivers/usb/gadget/s3c-hsotg.c                     |   12 +-
 drivers/usb/host/ehci-exynos.c                     |   34 +-
 9 files changed, 1013 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
 create mode 100644 drivers/phy/phy-exynos-usb2.c
 create mode 100644 drivers/phy/phy-exynos-usb2.h
 create mode 100644 drivers/phy/phy-exynos4210-usb2.c
 create mode 100644 drivers/phy/phy-exynos4212-usb2.c

-- 
1.7.9.5

[PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[Kamil Debski]

Add a new driver for the Exynos USB PHY. The new driver uses the generic
PHY framework. The driver includes support for the Exynos 4x10 and 4x12
SoC families.

Signed-off-by: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
 drivers/phy/Kconfig                                |   23 +-
 drivers/phy/Makefile                               |    4 +
 drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
 drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
 drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
 drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
 7 files changed, 995 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
 create mode 100644 drivers/phy/phy-exynos-usb2.c
 create mode 100644 drivers/phy/phy-exynos-usb2.h
 create mode 100644 drivers/phy/phy-exynos4210-usb2.c
 create mode 100644 drivers/phy/phy-exynos4212-usb2.c

diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
new file mode 100644
index 0000000..c8fbc70
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
@@ -0,0 +1,52 @@
+Samsung S5P/EXYNOS SoC series USB PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be one of the listed compatibles:
+	- "samsung,exynos4210-usbphy"
+	- "samsung,exynos4212-usbphy"
+- reg : a list of registers used by phy driver
+	- first and obligatory is the location of phy modules registers
+	- second and also required is the location of isolation registers
+	  (isolation registers control the physical connection between the in
+	  SoC modules and outside of the SoC, this also can be called enable
+	  control in the documentation of the SoC)
+	- third is the location of the mode switch register, this only applies
+	  to SoCs that have such a feature; mode switching enables to have
+	  both host and device used the same SoC pins and is commonly used
+	  when OTG is supported
+- #phy-cells : from the generic phy bindings, must be 1;
+- clocks and clock-names:
+	- the "phy" clocks is required by the phy module
+	- other clocks are associated by name with their respective phys and
+	  are used to determine the value of the clock settings register
+
+The second cell in the PHY specifier identifies the PHY, its  meaning is
+compatible dependent. For the currently supported SoCs (Exynos 4210 and
+Exynos 4212) it is as follows:
+  0 - USB device,
+  1 - USB host,
+  2 - HSIC0,
+  3 - HSIC1,
+
+Example:
+
+For Exynos 4412 (compatible with Exynos 4212):
+
+exynos_usbphy: exynos-usbphy@125B0000 {
+	compatible = "samsung,exynos4212-usbphy";
+	reg = <0x125B0000 0x100 0x10020704 0x0c 0x1001021c 0x4>;
+	clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>,
+							<&clock 2>;
+	clock-names = "phy", "device", "host", "hsic0", "hsic1";
+	status = "okay";
+	#phy-cells = <1>;
+};
+
+Then the PHY can be used in other nodes such as:
+
+ehci@12580000 {
+	status = "okay";
+	phys = <&exynos_usbphy 2>;
+	phy-names = "hsic0";
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a344f3d..bdf0fab 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -14,7 +14,7 @@ config GENERIC_PHY
 	  API by which phy drivers can create PHY using the phy framework and
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
-
+	  
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	help
@@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
 	help
 	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
 
+config PHY_EXYNOS_USB2
+	tristate "Samsung USB 2.0 PHY driver"
+	help
+	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
+	  This driver provides common interface to interact, for Samsung
+	  USB 2.0 PHY driver.
+
+config PHY_EXYNOS4210_USB2
+	bool "Support for Exynos 4210"
+	depends on PHY_EXYNOS_USB2
+	depends on CPU_EXYNOS4210
+	help
+	  Enable USB PHY support for Exynos 4210
+
+config PHY_EXYNOS4212_USB2
+	bool "Support for Exynos 4212"
+	depends on PHY_EXYNOS_USB2
+	depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
+	help
+	  Enable USB PHY support for Exynos 4212
+ 
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d0caae9..c87bc65 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_EXYNOS5250_USB)	+= phy-exynos5250-usb.o
+obj-$(CONFIG_PHY_EXYNOS_USB2)		+= phy-exynos-usb2.o
+obj-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
+obj-$(CONFIG_PHY_EXYNOS4212_USB2)	+= phy-exynos4212-usb2.o
diff --git a/drivers/phy/phy-exynos-usb2.c b/drivers/phy/phy-exynos-usb2.c
new file mode 100644
index 0000000..3e9d525
--- /dev/null
+++ b/drivers/phy/phy-exynos-usb2.c
@@ -0,0 +1,234 @@
+/*
+ * Samsung S5P/EXYNOS SoC series USB PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include "phy-exynos-usb2.h"
+
+static int exynos_usb2_phy_power_on(struct phy *phy)
+{
+	struct usb2_phy_instance *inst = phy_get_drvdata(phy);
+	struct usb2_phy_driver *drv = inst->drv;
+	int ret;
+
+	dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
+							inst->cfg->label);
+	ret = clk_prepare_enable(drv->clk);
+	if (ret)
+		return ret;
+	if (inst->cfg->power_on) {
+		spin_lock(&drv->lock);
+		ret = inst->cfg->power_on(inst);
+		spin_unlock(&drv->lock);
+	}
+	clk_disable_unprepare(drv->clk);
+	return ret;
+}
+
+static int exynos_usb2_phy_power_off(struct phy *phy)
+{
+	struct usb2_phy_instance *inst = phy_get_drvdata(phy);
+	struct usb2_phy_driver *drv = inst->drv;
+	int ret;
+
+	dev_dbg(drv->dev, "Request to power_off \"%s\" usb phy\n",
+							inst->cfg->label);
+	ret = clk_prepare_enable(drv->clk);
+	if (ret)
+		return ret;
+	if (inst->cfg->power_off) {
+		spin_lock(&drv->lock);
+		ret = inst->cfg->power_off(inst);
+		spin_unlock(&drv->lock);
+	}
+	clk_disable_unprepare(drv->clk);
+	return ret;
+}
+
+static struct phy_ops exynos_usb2_phy_ops = {
+	.power_on	= exynos_usb2_phy_power_on,
+	.power_off	= exynos_usb2_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *exynos_usb2_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct usb2_phy_driver *drv;
+
+	drv = dev_get_drvdata(dev);
+	if (!drv)
+		return ERR_PTR(-EINVAL);
+
+	if (WARN_ON(args->args[0] >= drv->cfg->num_phys))
+		return ERR_PTR(-ENODEV);
+
+	return drv->usb2_phy_instances[args->args[0]].phy;
+}
+
+static const struct of_device_id exynos_usb2_phy_of_match[];
+
+static int exynos_usb2_phy_probe(struct platform_device *pdev)
+{
+	struct usb2_phy_driver *drv;
+	struct device *dev = &pdev->dev;
+	struct resource *mem;
+	struct phy_provider *phy_provider;
+
+	const struct of_device_id *match;
+	const struct usb2_phy_config *cfg;
+	struct clk *clk;
+
+	int i;
+
+	match = of_match_node(exynos_usb2_phy_of_match, pdev->dev.of_node);
+	if (!match) {
+		dev_err(dev, "of_match_node() failed\n");
+		return -EINVAL;
+	}
+	cfg = match->data;
+	if (!cfg) {
+		dev_err(dev, "Failed to get configuration\n");
+		return -EINVAL;
+	}
+
+	drv = devm_kzalloc(dev, sizeof(struct usb2_phy_driver) +
+		cfg->num_phys * sizeof(struct usb2_phy_instance), GFP_KERNEL);
+
+	if (!drv) {
+		dev_err(dev, "Failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, drv);
+	spin_lock_init(&drv->lock);
+
+	drv->cfg = cfg;
+	drv->dev = dev;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	drv->reg_phy = devm_ioremap_resource(dev, mem);
+	if (IS_ERR(drv->reg_phy)) {
+		dev_err(dev, "Failed to map register memory (phy)\n");
+		return PTR_ERR(drv->reg_phy);
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	drv->reg_isol = devm_ioremap_resource(dev, mem);
+	if (IS_ERR(drv->reg_isol)) {
+		dev_err(dev, "Failed to map register memory (isolation)\n");
+		return PTR_ERR(drv->reg_isol);
+	}
+
+	if (drv->cfg->has_mode_switch) {
+		mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+		drv->reg_mode = devm_ioremap_resource(dev, mem);
+		if (IS_ERR(drv->reg_mode)) {
+			dev_err(dev, "Failed to map register memory (mode switch)\n");
+			return PTR_ERR(drv->reg_mode);
+		}
+	}
+
+	phy_provider = devm_of_phy_provider_register(dev,
+							exynos_usb2_phy_xlate);
+	if (IS_ERR(phy_provider)) {
+		dev_err(drv->dev, "Failed to register phy provider\n");
+		return PTR_ERR(phy_provider);
+	}
+
+	drv->clk = devm_clk_get(dev, "phy");
+	if (IS_ERR(drv->clk)) {
+		dev_err(dev, "Failed to get clock of phy controller\n");
+		return PTR_ERR(drv->clk);
+	}
+
+	for (i = 0; i < drv->cfg->num_phys; i++) {
+		char *label = drv->cfg->phys[i].label;
+		struct usb2_phy_instance *p = &drv->usb2_phy_instances[i];
+
+		dev_dbg(dev, "Creating phy \"%s\"\n", label);
+		p->phy = devm_phy_create(dev, &exynos_usb2_phy_ops, NULL);
+		if (IS_ERR(p->phy)) {
+			dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
+						label);
+			return PTR_ERR(p->phy);
+		}
+
+		p->cfg = &drv->cfg->phys[i];
+		p->drv = drv;
+		phy_set_drvdata(p->phy, p);
+
+		clk = clk_get(dev, p->cfg->label);
+		if (IS_ERR(clk)) {
+			dev_err(dev, "Failed to get clock of \"%s\" phy\n",
+								p->cfg->label);
+			return PTR_ERR(clk);
+		}
+
+		p->rate = clk_get_rate(clk);
+
+		if (p->cfg->rate_to_clk) {
+			p->clk = p->cfg->rate_to_clk(p->rate);
+			if (p->clk == CLKSEL_ERROR) {
+				dev_err(dev, "Clock rate (%ld) not supported\n",
+								p->rate);
+				clk_put(clk);
+				return -EINVAL;
+			}
+		}
+		clk_put(clk);
+	}
+
+	return 0;
+}
+
+extern const struct usb2_phy_config exynos4210_usb2_phy_config;
+extern const struct usb2_phy_config exynos4212_usb2_phy_config;
+
+static const struct of_device_id exynos_usb2_phy_of_match[] = {
+#ifdef CONFIG_PHY_EXYNOS4210_USB2
+	{
+		.compatible = "samsung,exynos4210-usb2-phy",
+		.data = &exynos4210_usb2_phy_config,
+	},
+#endif
+#ifdef CONFIG_PHY_EXYNOS4212_USB2
+	{
+		.compatible = "samsung,exynos4212-usb2-phy",
+		.data = &exynos4212_usb2_phy_config,
+	},
+#endif
+	{ },
+};
+
+static struct platform_driver exynos_usb2_phy_driver = {
+	.probe	= exynos_usb2_phy_probe,
+	.driver = {
+		.of_match_table	= exynos_usb2_phy_of_match,
+		.name		= "exynos-usb2-phy",
+		.owner		= THIS_MODULE,
+	}
+};
+
+module_platform_driver(exynos_usb2_phy_driver);
+MODULE_DESCRIPTION("Samsung S5P/EXYNOS SoC USB PHY driver");
+MODULE_AUTHOR("Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-usb2-phy");
+
diff --git a/drivers/phy/phy-exynos-usb2.h b/drivers/phy/phy-exynos-usb2.h
new file mode 100644
index 0000000..91e4f73
--- /dev/null
+++ b/drivers/phy/phy-exynos-usb2.h
@@ -0,0 +1,87 @@
+/*
+ * Samsung S5P/EXYNOS SoC series USB PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _PHY_EXYNOS_USB2_H
+#define _PHY_EXYNOS_USB2_H
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define CLKSEL_ERROR                       -1
+
+#ifndef KHZ
+#define KHZ 1000
+#endif
+
+#ifndef MHZ
+#define MHZ (KHZ * KHZ)
+#endif
+
+enum phy_type {
+	PHY_DEVICE,
+	PHY_HOST,
+};
+
+enum usb2_phy_state {
+	STATE_OFF,
+	STATE_ON,
+};
+
+struct usb2_phy_driver;
+struct usb2_phy_instance;
+struct usb2_phy_config;
+
+struct usb2_phy_instance {
+	struct usb2_phy_driver *drv;
+	struct phy *phy;
+	const struct common_phy *cfg;
+	enum usb2_phy_state state;
+	u32 clk;
+	unsigned long rate;
+};
+
+struct usb2_phy_driver {
+	struct device *dev;
+	spinlock_t lock;
+	void __iomem *reg_phy;
+	void __iomem *reg_isol;
+	void __iomem *reg_mode;
+	const struct usb2_phy_config *cfg;
+	struct clk *clk;
+	struct usb2_phy_instance usb2_phy_instances[0];
+};
+
+struct common_phy {
+	char *label;
+	enum phy_type type;
+	unsigned int id;
+	u32 (*rate_to_clk)(unsigned long);
+	int (*power_on)(struct usb2_phy_instance*);
+	int (*power_off)(struct usb2_phy_instance*);
+};
+
+
+struct usb2_phy_config {
+	int num_phys;
+	const struct common_phy *phys;
+	char has_mode_switch;
+};
+
+#endif
+
diff --git a/drivers/phy/phy-exynos4210-usb2.c b/drivers/phy/phy-exynos4210-usb2.c
new file mode 100644
index 0000000..d04ee8e
--- /dev/null
+++ b/drivers/phy/phy-exynos4210-usb2.c
@@ -0,0 +1,272 @@
+/*
+ * Samsung S5P/EXYNOS SoC series USB PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include "phy-exynos-usb2.h"
+
+/* Exynos USB PHY registers */
+
+/* PHY power control */
+#define EXYNOS_4210_UPHYPWR			0x0
+
+#define EXYNOS_4210_UPHYPWR_PHY0_SUSPEND	(1 << 0)
+#define EXYNOS_4210_UPHYPWR_PHY0_PWR		(1 << 3)
+#define EXYNOS_4210_UPHYPWR_PHY0_OTG_PWR	(1 << 4)
+#define EXYNOS_4210_UPHYPWR_PHY0_SLEEP		(1 << 5)
+#define EXYNOS_4210_UPHYPWR_PHY0	( \
+	EXYNOS_4210_UPHYPWR_PHY0_SUSPEND | \
+	EXYNOS_4210_UPHYPWR_PHY0_PWR | \
+	EXYNOS_4210_UPHYPWR_PHY0_OTG_PWR | \
+	EXYNOS_4210_UPHYPWR_PHY0_SLEEP)
+
+#define EXYNOS_4210_UPHYPWR_PHY1_SUSPEND	(1 << 6)
+#define EXYNOS_4210_UPHYPWR_PHY1_PWR		(1 << 7)
+#define EXYNOS_4210_UPHYPWR_PHY1_SLEEP		(1 << 8)
+#define EXYNOS_4210_UPHYPWR_PHY1 ( \
+	EXYNOS_4210_UPHYPWR_PHY1_SUSPEND | \
+	EXYNOS_4210_UPHYPWR_PHY1_PWR | \
+	EXYNOS_4210_UPHYPWR_PHY1_SLEEP)
+
+#define EXYNOS_4210_UPHYPWR_HSCI0_SUSPEND	(1 << 9)
+#define EXYNOS_4210_UPHYPWR_HSCI0_SLEEP		(1 << 10)
+#define EXYNOS_4210_UPHYPWR_HSCI0 ( \
+	EXYNOS_4210_UPHYPWR_HSCI0_SUSPEND | \
+	EXYNOS_4210_UPHYPWR_HSCI0_SLEEP)
+
+#define EXYNOS_4210_UPHYPWR_HSCI1_SUSPEND	(1 << 11)
+#define EXYNOS_4210_UPHYPWR_HSCI1_SLEEP		(1 << 12)
+#define EXYNOS_4210_UPHYPWR_HSCI1 ( \
+	EXYNOS_4210_UPHYPWR_HSCI1_SUSPEND | \
+	EXYNOS_4210_UPHYPWR_HSCI1_SLEEP)
+
+/* PHY clock control */
+#define EXYNOS_4210_UPHYCLK			0x4
+
+#define EXYNOS_4210_UPHYCLK_PHYFSEL_MASK	(0x3 << 0)
+#define EXYNOS_4210_UPHYCLK_PHYFSEL_48MHZ	(0x0 << 0)
+#define EXYNOS_4210_UPHYCLK_PHYFSEL_24MHZ	(0x3 << 0)
+#define EXYNOS_4210_UPHYCLK_PHYFSEL_12MHZ	(0x2 << 0)
+
+#define EXYNOS_4210_UPHYCLK_PHY0_ID_PULLUP	(0x1 << 2)
+#define EXYNOS_4210_UPHYCLK_PHY0_COMMON_ON	(0x1 << 4)
+#define EXYNOS_4210_UPHYCLK_PHY1_COMMON_ON	(0x1 << 7)
+
+/* PHY reset control */
+#define EXYNOS_4210_UPHYRST			0x8
+
+#define EXYNOS_4210_URSTCON_PHY0		(1 << 0)
+#define EXYNOS_4210_URSTCON_OTG_HLINK		(1 << 1)
+#define EXYNOS_4210_URSTCON_OTG_PHYLINK		(1 << 2)
+#define EXYNOS_4210_URSTCON_PHY1_ALL		(1 << 3)
+#define EXYNOS_4210_URSTCON_PHY1_P0		(1 << 4)
+#define EXYNOS_4210_URSTCON_PHY1_P1P2		(1 << 5)
+#define EXYNOS_4210_URSTCON_HOST_LINK_ALL	(1 << 6)
+#define EXYNOS_4210_URSTCON_HOST_LINK_P0	(1 << 7)
+#define EXYNOS_4210_URSTCON_HOST_LINK_P1	(1 << 8)
+#define EXYNOS_4210_URSTCON_HOST_LINK_P2	(1 << 9)
+
+/* Isolation, configured in the power management unit */
+#define EXYNOS_4210_USB_ISOL_DEVICE_OFFSET	0x0
+#define EXYNOS_4210_USB_ISOL_DEVICE		(1 << 0)
+#define EXYNOS_4210_USB_ISOL_HOST_OFFSET	0x4
+#define EXYNOS_4210_USB_ISOL_HOST		(1 << 0)
+
+/* USBYPHY1 Floating prevention */
+#define EXYNOS_4210_UPHY1CON			0x34
+#define EXYNOS_4210_UPHY1CON_FLOAT_PREVENTION	0x1
+
+enum exynos4210_phy_id {
+	EXYNOS4210_DEVICE,
+	EXYNOS4210_HOST,
+	EXYNOS4210_HSIC0,
+	EXYNOS4210_HSIC1,
+	EXYNOS4210_NUM_PHYS,
+};
+
+/*
+ * exynos4210_rate_to_clk() converts the supplied clock rate to the value that
+ * can be written to the phy register.
+ */
+static u32 exynos4210_rate_to_clk(unsigned long rate)
+{
+	unsigned int clksel;
+
+	switch (rate) {
+	case 12 * MHZ:
+		clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_12MHZ;
+		break;
+	case 24 * MHZ:
+		clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_24MHZ;
+		break;
+	case 48 * MHZ:
+		clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_48MHZ;
+		break;
+	default:
+		clksel = CLKSEL_ERROR;
+	}
+
+	return clksel;
+}
+
+static void exynos4210_isol(struct usb2_phy_instance *inst, bool on)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+	u32 offset;
+	u32 mask;
+	u32 tmp;
+
+	if (!drv->reg_isol)
+		return;
+
+	switch (inst->cfg->id) {
+	case EXYNOS4210_DEVICE:
+		offset = EXYNOS_4210_USB_ISOL_DEVICE_OFFSET;
+		mask = EXYNOS_4210_USB_ISOL_DEVICE;
+		break;
+	case EXYNOS4210_HOST:
+		offset = EXYNOS_4210_USB_ISOL_HOST_OFFSET;
+		mask = EXYNOS_4210_USB_ISOL_HOST;
+		break;
+	default:
+		return;
+	};
+
+	tmp = readl(drv->reg_isol + offset);
+	if (on)
+		tmp &= ~mask;
+	else
+		tmp |= mask;
+	writel(tmp, drv->reg_isol + offset);
+}
+
+static void exynos4210_phy_pwr(struct usb2_phy_instance *inst, bool on)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+	u32 rstbits = 0;
+	u32 phypwr = 0;
+	u32 rst;
+	u32 pwr;
+
+	switch (inst->cfg->id) {
+	case EXYNOS4210_DEVICE:
+		phypwr =	EXYNOS_4210_UPHYPWR_PHY0;
+		rstbits =	EXYNOS_4210_URSTCON_PHY0;
+		break;
+	case EXYNOS4210_HOST:
+		phypwr =	EXYNOS_4210_UPHYPWR_PHY1;
+		rstbits =	EXYNOS_4210_URSTCON_PHY1_ALL |
+				EXYNOS_4210_URSTCON_PHY1_P0 |
+				EXYNOS_4210_URSTCON_PHY1_P1P2 |
+				EXYNOS_4210_URSTCON_HOST_LINK_ALL |
+				EXYNOS_4210_URSTCON_HOST_LINK_P0;
+		writel(on, drv->reg_phy + EXYNOS_4210_UPHY1CON);
+		break;
+	case EXYNOS4210_HSIC0:
+		phypwr =	EXYNOS_4210_UPHYPWR_HSCI0;
+		rstbits =	EXYNOS_4210_URSTCON_PHY1_P1P2 |
+				EXYNOS_4210_URSTCON_HOST_LINK_P1;
+		break;
+	case EXYNOS4210_HSIC1:
+		phypwr =	EXYNOS_4210_UPHYPWR_HSCI1;
+		rstbits =	EXYNOS_4210_URSTCON_PHY1_P1P2 |
+				EXYNOS_4210_URSTCON_HOST_LINK_P2;
+		break;
+	};
+
+	if (on) {
+		writel(inst->clk, drv->reg_phy + EXYNOS_4210_UPHYCLK);
+
+		pwr = readl(drv->reg_phy + EXYNOS_4210_UPHYPWR);
+		pwr &= ~phypwr;
+		writel(pwr, drv->reg_phy + EXYNOS_4210_UPHYPWR);
+
+		rst = readl(drv->reg_phy + EXYNOS_4210_UPHYRST);
+		rst |= rstbits;
+		writel(rst, drv->reg_phy + EXYNOS_4210_UPHYRST);
+		udelay(10);
+		rst &= ~rstbits;
+		writel(rst, drv->reg_phy + EXYNOS_4210_UPHYRST);
+	} else {
+		pwr = readl(drv->reg_phy + EXYNOS_4210_UPHYPWR);
+		pwr |= phypwr;
+		writel(pwr, drv->reg_phy + EXYNOS_4210_UPHYPWR);
+	}
+}
+
+static int exynos4210_power_on(struct usb2_phy_instance *inst)
+{
+	/* Order of initialisation is important - first power then isolation */
+	exynos4210_phy_pwr(inst, 1);
+	exynos4210_isol(inst, 0);
+
+	return 0;
+}
+
+static int exynos4210_power_off(struct usb2_phy_instance *inst)
+{
+	exynos4210_isol(inst, 1);
+	exynos4210_phy_pwr(inst, 0);
+
+	return 0;
+}
+
+
+static const struct common_phy exynos4210_phys[] = {
+	{
+		.label		= "device",
+		.type		= PHY_DEVICE,
+		.id		= EXYNOS4210_DEVICE,
+		.rate_to_clk	= exynos4210_rate_to_clk,
+		.power_on	= exynos4210_power_on,
+		.power_off	= exynos4210_power_off,
+	},
+	{
+		.label		= "host",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4210_HOST,
+		.rate_to_clk	= exynos4210_rate_to_clk,
+		.power_on	= exynos4210_power_on,
+		.power_off	= exynos4210_power_off,
+	},
+	{
+		.label		= "hsic0",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4210_HSIC0,
+		.rate_to_clk	= exynos4210_rate_to_clk,
+		.power_on	= exynos4210_power_on,
+		.power_off	= exynos4210_power_off,
+	},
+	{
+		.label		= "hsic1",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4210_HSIC1,
+		.rate_to_clk	= exynos4210_rate_to_clk,
+		.power_on	= exynos4210_power_on,
+		.power_off	= exynos4210_power_off,
+	},
+	{},
+};
+
+const struct usb2_phy_config exynos4210_usb2_phy_config = {
+	.num_phys		= EXYNOS4210_NUM_PHYS,
+	.phys			= exynos4210_phys,
+	.has_mode_switch	= 1,
+};
+
diff --git a/drivers/phy/phy-exynos4212-usb2.c b/drivers/phy/phy-exynos4212-usb2.c
new file mode 100644
index 0000000..654efe0
--- /dev/null
+++ b/drivers/phy/phy-exynos4212-usb2.c
@@ -0,0 +1,324 @@
+/*
+ * Samsung S5P/EXYNOS SoC series USB PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include "phy-exynos-usb2.h"
+
+/* Exynos USB PHY registers */
+
+/* PHY power control */
+#define EXYNOS_4212_UPHYPWR			0x0
+
+#define EXYNOS_4212_UPHYPWR_DEV_SUSPEND		(1 << 0)
+#define EXYNOS_4212_UPHYPWR_DEV_PWR		(1 << 3)
+#define EXYNOS_4212_UPHYPWR_DEV_OTG_PWR		(1 << 4)
+#define EXYNOS_4212_UPHYPWR_DEV_SLEEP		(1 << 5)
+#define EXYNOS_4212_UPHYPWR_DEV	( \
+	EXYNOS_4212_UPHYPWR_DEV_SUSPEND | \
+	EXYNOS_4212_UPHYPWR_DEV_PWR | \
+	EXYNOS_4212_UPHYPWR_DEV_OTG_PWR | \
+	EXYNOS_4212_UPHYPWR_DEV_SLEEP)
+
+#define EXYNOS_4212_UPHYPWR_HOST_SUSPEND	(1 << 6)
+#define EXYNOS_4212_UPHYPWR_HOST_PWR		(1 << 7)
+#define EXYNOS_4212_UPHYPWR_HOST_SLEEP		(1 << 8)
+#define EXYNOS_4212_UPHYPWR_HOST ( \
+	EXYNOS_4212_UPHYPWR_HOST_SUSPEND | \
+	EXYNOS_4212_UPHYPWR_HOST_PWR | \
+	EXYNOS_4212_UPHYPWR_HOST_SLEEP)
+
+#define EXYNOS_4212_UPHYPWR_HSCI0_SUSPEND	(1 << 9)
+#define EXYNOS_4212_UPHYPWR_HSCI0_PWR		(1 << 10)
+#define EXYNOS_4212_UPHYPWR_HSCI0_SLEEP		(1 << 11)
+#define EXYNOS_4212_UPHYPWR_HSCI0 ( \
+	EXYNOS_4212_UPHYPWR_HSCI0_SUSPEND | \
+	EXYNOS_4212_UPHYPWR_HSCI0_PWR | \
+	EXYNOS_4212_UPHYPWR_HSCI0_SLEEP)
+
+#define EXYNOS_4212_UPHYPWR_HSCI1_SUSPEND	(1 << 12)
+#define EXYNOS_4212_UPHYPWR_HSCI1_PWR		(1 << 13)
+#define EXYNOS_4212_UPHYPWR_HSCI1_SLEEP		(1 << 14)
+#define EXYNOS_4212_UPHYPWR_HSCI1 ( \
+	EXYNOS_4212_UPHYPWR_HSCI1_SUSPEND | \
+	EXYNOS_4212_UPHYPWR_HSCI1_PWR | \
+	EXYNOS_4212_UPHYPWR_HSCI1_SLEEP)
+
+/* PHY clock control */
+#define EXYNOS_4212_UPHYCLK			0x4
+
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_MASK	(0x7 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_9MHZ6	(0x0 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_10MHZ	(0x1 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_12MHZ	(0x2 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_19MHZ2	(0x3 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_20MHZ	(0x4 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_24MHZ	(0x5 << 0)
+#define EXYNOS_4212_UPHYCLK_PHYFSEL_50MHZ	(0x7 << 0)
+
+#define EXYNOS_4212_UPHYCLK_PHY0_ID_PULLUP	(0x1 << 3)
+#define EXYNOS_4212_UPHYCLK_PHY0_COMMON_ON	(0x1 << 4)
+#define EXYNOS_4212_UPHYCLK_PHY1_COMMON_ON	(0x1 << 7)
+
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_MASK	(0x7f << 10)
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_12MHZ	(0x24 << 10)
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_15MHZ	(0x1c << 10)
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_16MHZ	(0x1a << 10)
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_19MHZ2	(0x15 << 10)
+#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_20MHZ	(0x14 << 10)
+
+/* PHY reset control */
+#define EXYNOS_4212_UPHYRST			0x8
+
+#define EXYNOS_4212_URSTCON_DEVICE		(1 << 0)
+#define EXYNOS_4212_URSTCON_OTG_HLINK		(1 << 1)
+#define EXYNOS_4212_URSTCON_OTG_PHYLINK		(1 << 2)
+#define EXYNOS_4212_URSTCON_HOST_PHY		(1 << 3)
+#define EXYNOS_4212_URSTCON_PHY1		(1 << 4)
+#define EXYNOS_4212_URSTCON_HSIC0		(1 << 5)
+#define EXYNOS_4212_URSTCON_HSIC1		(1 << 6)
+#define EXYNOS_4212_URSTCON_HOST_LINK_ALL	(1 << 7)
+#define EXYNOS_4212_URSTCON_HOST_LINK_P0	(1 << 8)
+#define EXYNOS_4212_URSTCON_HOST_LINK_P1	(1 << 9)
+#define EXYNOS_4212_URSTCON_HOST_LINK_P2	(1 << 10)
+
+/* Isolation, configured in the power management unit */
+#define EXYNOS_4212_USB_ISOL_OFFSET		0x0
+#define EXYNOS_4212_USB_ISOL_OTG		(1 << 0)
+#define EXYNOS_4212_USB_ISOL_HSIC0_OFFSET	0x4
+#define EXYNOS_4212_USB_ISOL_HSIC0		(1 << 0)
+#define EXYNOS_4212_USB_ISOL_HSIC1_OFFSET	0x8
+#define EXYNOS_4212_USB_ISOL_HSIC1		(1 << 0)
+
+enum exynos4x12_phy_id {
+	EXYNOS4212_DEVICE,
+	EXYNOS4212_HOST,
+	EXYNOS4212_HSIC0,
+	EXYNOS4212_HSIC1,
+	EXYNOS4212_NUM_PHYS,
+};
+
+/*
+ * exynos4212_rate_to_clk() converts the supplied clock rate to the value that
+ * can be written to the phy register.
+ */
+static u32 exynos4212_rate_to_clk(unsigned long rate)
+{
+	unsigned int clksel;
+
+	/* EXYNOS_4212_UPHYCLK_PHYFSEL_MASK */
+
+	switch (rate) {
+	case 9600 * KHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_9MHZ6;
+		break;
+	case 10 * MHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_10MHZ;
+		break;
+	case 12 * MHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_12MHZ;
+		break;
+	case 19200 * KHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_19MHZ2;
+		break;
+	case 20 * MHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_20MHZ;
+		break;
+	case 24 * MHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_24MHZ;
+		break;
+	case 50 * MHZ:
+		clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_50MHZ;
+		break;
+	default:
+		clksel = CLKSEL_ERROR;
+	}
+
+	return clksel;
+}
+
+static void exynos4212_isol(struct usb2_phy_instance *inst, bool on)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+	u32 offset;
+	u32 mask;
+	u32 tmp;
+
+	if (!drv->reg_isol)
+		return;
+
+	switch (inst->cfg->id) {
+	case EXYNOS4212_DEVICE:
+		offset = EXYNOS_4212_USB_ISOL_OFFSET;
+		mask = EXYNOS_4212_USB_ISOL_OTG;
+		break;
+	case EXYNOS4212_HOST:
+		offset = EXYNOS_4212_USB_ISOL_OFFSET;
+		mask = EXYNOS_4212_USB_ISOL_OTG;
+		break;
+	case EXYNOS4212_HSIC0:
+		offset = EXYNOS_4212_USB_ISOL_HSIC0_OFFSET;
+		mask = EXYNOS_4212_USB_ISOL_HSIC0;
+		break;
+	case EXYNOS4212_HSIC1:
+		offset = EXYNOS_4212_USB_ISOL_HSIC1_OFFSET;
+		mask = EXYNOS_4212_USB_ISOL_HSIC1;
+		break;
+	default:
+		return;
+	};
+
+	tmp = readl(drv->reg_isol + offset);
+	if (on)
+		tmp &= ~mask;
+	else
+		tmp |= mask;
+	writel(tmp, drv->reg_isol + offset);
+}
+
+static void exynos4212_phy_pwr(struct usb2_phy_instance *inst, bool on)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+	u32 rstbits = 0;
+	u32 phypwr = 0;
+	u32 rst;
+	u32 pwr;
+
+	switch (inst->cfg->id) {
+	case EXYNOS4212_DEVICE:
+		phypwr =	EXYNOS_4212_UPHYPWR_DEV;
+		rstbits =	EXYNOS_4212_URSTCON_DEVICE;
+		break;
+	case EXYNOS4212_HOST:
+		phypwr =	EXYNOS_4212_UPHYPWR_HOST;
+		rstbits =	EXYNOS_4212_URSTCON_HOST_PHY;
+		break;
+	case EXYNOS4212_HSIC0:
+		phypwr =	EXYNOS_4212_UPHYPWR_HSCI0;
+		rstbits =	EXYNOS_4212_URSTCON_HSIC1 |
+				EXYNOS_4212_URSTCON_HOST_LINK_P0 |
+				EXYNOS_4212_URSTCON_HOST_PHY;
+		break;
+	case EXYNOS4212_HSIC1:
+		phypwr =	EXYNOS_4212_UPHYPWR_HSCI1;
+		rstbits =	EXYNOS_4212_URSTCON_HSIC1 |
+				EXYNOS_4212_URSTCON_HOST_LINK_P1;
+		break;
+	};
+
+	if (on) {
+		writel(inst->clk, drv->reg_phy + EXYNOS_4212_UPHYCLK);
+
+		pwr = readl(drv->reg_phy + EXYNOS_4212_UPHYPWR);
+		pwr &= ~phypwr;
+		writel(pwr, drv->reg_phy + EXYNOS_4212_UPHYPWR);
+
+		rst = readl(drv->reg_phy + EXYNOS_4212_UPHYRST);
+		rst |= rstbits;
+		writel(rst, drv->reg_phy + EXYNOS_4212_UPHYRST);
+		udelay(10);
+		rst &= ~rstbits;
+		writel(rst, drv->reg_phy + EXYNOS_4212_UPHYRST);
+	} else {
+		pwr = readl(drv->reg_phy + EXYNOS_4212_UPHYPWR);
+		pwr |= phypwr;
+		writel(pwr, drv->reg_phy + EXYNOS_4212_UPHYPWR);
+	}
+}
+
+static int exynos4212_power_on(struct usb2_phy_instance *inst)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+
+	inst->state = STATE_ON;
+	exynos4212_phy_pwr(inst, 1);
+	exynos4212_isol(inst, 0);
+
+	/* Power on the device, as it is necessary for HSIC to work */
+	if (inst->cfg->id == EXYNOS4212_HSIC0) {
+		struct usb2_phy_instance *device =
+					&drv->usb2_phy_instances[EXYNOS4212_DEVICE];
+		exynos4212_phy_pwr(device, 1);
+		exynos4212_isol(device, 0);
+	}
+
+	return 0;
+}
+
+static int exynos4212_power_off(struct usb2_phy_instance *inst)
+{
+	struct usb2_phy_driver *drv = inst->drv;
+	struct usb2_phy_instance *device =
+		&drv->usb2_phy_instances[EXYNOS4212_DEVICE];
+
+	inst->state = STATE_OFF;
+	exynos4212_isol(inst, 1);
+	exynos4212_phy_pwr(inst, 0);
+
+	if (inst->cfg->id == EXYNOS4212_HSIC0 && device->state != STATE_ON) {
+		exynos4212_isol(device, 1);
+		exynos4212_phy_pwr(device, 0);
+	}
+
+	return 0;
+}
+
+
+static const struct common_phy exynos4212_phys[] = {
+	{
+		.label		= "device",
+		.type		= PHY_DEVICE,
+		.id		= EXYNOS4212_DEVICE,
+		.rate_to_clk	= exynos4212_rate_to_clk,
+		.power_on	= exynos4212_power_on,
+		.power_off	= exynos4212_power_off,
+	},
+	{
+		.label		= "host",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4212_HOST,
+		.rate_to_clk	= exynos4212_rate_to_clk,
+		.power_on	= exynos4212_power_on,
+		.power_off	= exynos4212_power_off,
+	},
+	{
+		.label		= "hsic0",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4212_HSIC0,
+		.rate_to_clk	= exynos4212_rate_to_clk,
+		.power_on	= exynos4212_power_on,
+		.power_off	= exynos4212_power_off,
+	},
+	{
+		.label		= "hsic1",
+		.type		= PHY_HOST,
+		.id		= EXYNOS4212_HSIC1,
+		.rate_to_clk	= exynos4212_rate_to_clk,
+		.power_on	= exynos4212_power_on,
+		.power_off	= exynos4212_power_off,
+	},
+	{},
+};
+
+const struct usb2_phy_config exynos4212_usb2_phy_config = {
+	.num_phys		= EXYNOS4212_NUM_PHYS,
+	.phys			= exynos4212_phys,
+	.has_mode_switch	= 1,
+};
+
-- 
1.7.9.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 v3 1/3] phy: Add new Exynos USB PHY driver

[Jingoo Han]

On Wednesday, November 06, 2013 1:13 AM, Kamil Debski wrote:
> 
> Add a new driver for the Exynos USB PHY. The new driver uses the generic
> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> SoC families.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>  drivers/phy/Kconfig                                |   23 +-
>  drivers/phy/Makefile                               |    4 +
>  drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>  drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>  drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>  drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>  7 files changed, 995 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>  create mode 100644 drivers/phy/phy-exynos-usb2.c
>  create mode 100644 drivers/phy/phy-exynos-usb2.h
>  create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>  create mode 100644 drivers/phy/phy-exynos4212-usb2.c

[....]

> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..c87bc65 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
> +obj-$(CONFIG_PHY_EXYNOS5250_USB)	+= phy-exynos5250-usb.o

Hi Kamil,

Would you add 'phy-exynos5250-usb.c' file? :-)

Now, I am testing Exynos5250 USB HOST with your patchset.
However, it makes error because there is no PHY driver for
Exynos5250.

  WARNING: CPU: 0 PID: 18 at drivers/phy/phy-core.c:366 phy_get+0x1e8/0x224()
  Device: exynos-ehci
  missing string
  .....

Best regards,
Jingoo Han

RE: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[Kamil Debski]

Hi,

> From: Jingoo Han [mailto:jg1.han@samsung.com]
> Sent: Wednesday, November 06, 2013 2:03 AM
> 
> On Wednesday, November 06, 2013 1:13 AM, Kamil Debski wrote:
> >
> > Add a new driver for the Exynos USB PHY. The new driver uses the
> > generic PHY framework. The driver includes support for the Exynos
> 4x10
> > and 4x12 SoC families.
> >
> > Signed-off-by: Kamil Debski <k.debski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
> >  drivers/phy/Kconfig                                |   23 +-
> >  drivers/phy/Makefile                               |    4 +
> >  drivers/phy/phy-exynos-usb2.c                      |  234
> ++++++++++++++
> >  drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
> >  drivers/phy/phy-exynos4210-usb2.c                  |  272
> ++++++++++++++++
> >  drivers/phy/phy-exynos4212-usb2.c                  |  324
> ++++++++++++++++++++
> >  7 files changed, 995 insertions(+), 1 deletion(-)  create mode
> 100644
> > Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> >  create mode 100644 drivers/phy/phy-exynos-usb2.c  create mode 100644
> > drivers/phy/phy-exynos-usb2.h  create mode 100644
> > drivers/phy/phy-exynos4210-usb2.c  create mode 100644
> > drivers/phy/phy-exynos4212-usb2.c
> 
> [....]
> 
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index
> > d0caae9..c87bc65 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-
> video.o
> >  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> >  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
> >  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
> > +obj-$(CONFIG_PHY_EXYNOS5250_USB)	+= phy-exynos5250-usb.o
> 
> Hi Kamil,
> 
> Would you add 'phy-exynos5250-usb.c' file? :-)
> 
> Now, I am testing Exynos5250 USB HOST with your patchset.
> However, it makes error because there is no PHY driver for Exynos5250.
> 
>   WARNING: CPU: 0 PID: 18 at drivers/phy/phy-core.c:366
> phy_get+0x1e8/0x224()
>   Device: exynos-ehci
>   missing string
>   .....

I have to rewrite the power_on/power_off function in Exynos 5250 and I will
include it in the next version.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[Kishon Vijay Abraham I]

Hi,

On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote:
> Add a new driver for the Exynos USB PHY. The new driver uses the generic
> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> SoC families.
>
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>   drivers/phy/Kconfig                                |   23 +-
>   drivers/phy/Makefile                               |    4 +
>   drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>   drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>   drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>   drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>   7 files changed, 995 insertions(+), 1 deletion(-)
>   create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>   create mode 100644 drivers/phy/phy-exynos-usb2.c
>   create mode 100644 drivers/phy/phy-exynos-usb2.h
>   create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>   create mode 100644 drivers/phy/phy-exynos4212-usb2.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> new file mode 100644
> index 0000000..c8fbc70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> @@ -0,0 +1,52 @@
> +Samsung S5P/EXYNOS SoC series USB PHY
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be one of the listed compatibles:
> +	- "samsung,exynos4210-usbphy"
> +	- "samsung,exynos4212-usbphy"
> +- reg : a list of registers used by phy driver
> +	- first and obligatory is the location of phy modules registers
> +	- second and also required is the location of isolation registers
> +	  (isolation registers control the physical connection between the in
> +	  SoC modules and outside of the SoC, this also can be called enable
> +	  control in the documentation of the SoC)
> +	- third is the location of the mode switch register, this only applies
> +	  to SoCs that have such a feature; mode switching enables to have
> +	  both host and device used the same SoC pins and is commonly used
> +	  when OTG is supported
> +- #phy-cells : from the generic phy bindings, must be 1;
> +- clocks and clock-names:
> +	- the "phy" clocks is required by the phy module
> +	- other clocks are associated by name with their respective phys and
> +	  are used to determine the value of the clock settings register
> +
> +The second cell in the PHY specifier identifies the PHY, its  meaning is
> +compatible dependent. For the currently supported SoCs (Exynos 4210 and
> +Exynos 4212) it is as follows:
> +  0 - USB device,
> +  1 - USB host,
> +  2 - HSIC0,
> +  3 - HSIC1,
> +
> +Example:
> +
> +For Exynos 4412 (compatible with Exynos 4212):
> +
> +exynos_usbphy: exynos-usbphy@125B0000 {
> +	compatible = "samsung,exynos4212-usbphy";
> +	reg = <0x125B0000 0x100 0x10020704 0x0c 0x1001021c 0x4>;
> +	clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>,
> +							<&clock 2>;
> +	clock-names = "phy", "device", "host", "hsic0", "hsic1";
> +	status = "okay";
> +	#phy-cells = <1>;
> +};
> +
> +Then the PHY can be used in other nodes such as:
> +
> +ehci@12580000 {
> +	status = "okay";
> +	phys = <&exynos_usbphy 2>;
> +	phy-names = "hsic0";
> +};
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..bdf0fab 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -14,7 +14,7 @@ config GENERIC_PHY
>   	  API by which phy drivers can create PHY using the phy framework and
>   	  phy users can obtain reference to the PHY. All the users of this
>   	  framework should select this config.
> -
> +	
spurious change
>   config PHY_EXYNOS_MIPI_VIDEO
>   	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>   	help
> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
>   	help
>   	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_EXYNOS_USB2
> +	tristate "Samsung USB 2.0 PHY driver"
> +	help
> +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
> +	  This driver provides common interface to interact, for Samsung
> +	  USB 2.0 PHY driver.

I still think we can get rid of this helper driver and have a single 
driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2.
> +
> +config PHY_EXYNOS4210_USB2
> +	bool "Support for Exynos 4210"
> +	depends on PHY_EXYNOS_USB2
> +	depends on CPU_EXYNOS4210
> +	help
> +	  Enable USB PHY support for Exynos 4210
> +
> +config PHY_EXYNOS4212_USB2
> +	bool "Support for Exynos 4212"
> +	depends on PHY_EXYNOS_USB2
> +	depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
> +	help
> +	  Enable USB PHY support for Exynos 4212
> +
>   endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..c87bc65 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>   obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>   obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>   obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
> +obj-$(CONFIG_PHY_EXYNOS5250_USB)	+= phy-exynos5250-usb.o
> +obj-$(CONFIG_PHY_EXYNOS_USB2)		+= phy-exynos-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4212_USB2)	+= phy-exynos4212-usb2.o
> diff --git a/drivers/phy/phy-exynos-usb2.c b/drivers/phy/phy-exynos-usb2.c
> new file mode 100644
> index 0000000..3e9d525
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-usb2.c
> @@ -0,0 +1,234 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "phy-exynos-usb2.h"
> +
> +static int exynos_usb2_phy_power_on(struct phy *phy)
> +{
> +	struct usb2_phy_instance *inst = phy_get_drvdata(phy);
> +	struct usb2_phy_driver *drv = inst->drv;
> +	int ret;
> +
> +	dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> +							inst->cfg->label);
> +	ret = clk_prepare_enable(drv->clk);
> +	if (ret)
> +		return ret;
> +	if (inst->cfg->power_on) {
> +		spin_lock(&drv->lock);
> +		ret = inst->cfg->power_on(inst);
> +		spin_unlock(&drv->lock);
> +	}
> +	clk_disable_unprepare(drv->clk);
> +	return ret;
> +}
> +
> +static int exynos_usb2_phy_power_off(struct phy *phy)
> +{
> +	struct usb2_phy_instance *inst = phy_get_drvdata(phy);
> +	struct usb2_phy_driver *drv = inst->drv;
> +	int ret;
> +
> +	dev_dbg(drv->dev, "Request to power_off \"%s\" usb phy\n",
> +							inst->cfg->label);
> +	ret = clk_prepare_enable(drv->clk);
> +	if (ret)
> +		return ret;
> +	if (inst->cfg->power_off) {
> +		spin_lock(&drv->lock);
> +		ret = inst->cfg->power_off(inst);
> +		spin_unlock(&drv->lock);
> +	}
> +	clk_disable_unprepare(drv->clk);
> +	return ret;
> +}
> +
> +static struct phy_ops exynos_usb2_phy_ops = {
> +	.power_on	= exynos_usb2_phy_power_on,
> +	.power_off	= exynos_usb2_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy *exynos_usb2_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	struct usb2_phy_driver *drv;
> +
> +	drv = dev_get_drvdata(dev);
> +	if (!drv)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (WARN_ON(args->args[0] >= drv->cfg->num_phys))
> +		return ERR_PTR(-ENODEV);
> +
> +	return drv->usb2_phy_instances[args->args[0]].phy;
> +}
> +
> +static const struct of_device_id exynos_usb2_phy_of_match[];
> +
> +static int exynos_usb2_phy_probe(struct platform_device *pdev)
> +{
> +	struct usb2_phy_driver *drv;
> +	struct device *dev = &pdev->dev;
> +	struct resource *mem;
> +	struct phy_provider *phy_provider;
> +
> +	const struct of_device_id *match;
> +	const struct usb2_phy_config *cfg;
> +	struct clk *clk;
> +
> +	int i;
> +
> +	match = of_match_node(exynos_usb2_phy_of_match, pdev->dev.of_node);
> +	if (!match) {
> +		dev_err(dev, "of_match_node() failed\n");
> +		return -EINVAL;
> +	}
> +	cfg = match->data;
> +	if (!cfg) {
> +		dev_err(dev, "Failed to get configuration\n");
> +		return -EINVAL;
> +	}
> +
> +	drv = devm_kzalloc(dev, sizeof(struct usb2_phy_driver) +
> +		cfg->num_phys * sizeof(struct usb2_phy_instance), GFP_KERNEL);
> +
> +	if (!drv) {
> +		dev_err(dev, "Failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(dev, drv);
> +	spin_lock_init(&drv->lock);
> +
> +	drv->cfg = cfg;
> +	drv->dev = dev;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	drv->reg_phy = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(drv->reg_phy)) {
> +		dev_err(dev, "Failed to map register memory (phy)\n");
> +		return PTR_ERR(drv->reg_phy);
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	drv->reg_isol = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(drv->reg_isol)) {
> +		dev_err(dev, "Failed to map register memory (isolation)\n");
> +		return PTR_ERR(drv->reg_isol);
> +	}
> +
> +	if (drv->cfg->has_mode_switch) {
> +		mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +		drv->reg_mode = devm_ioremap_resource(dev, mem);
> +		if (IS_ERR(drv->reg_mode)) {
> +			dev_err(dev, "Failed to map register memory (mode switch)\n");
> +			return PTR_ERR(drv->reg_mode);
> +		}
> +	}
> +
> +	phy_provider = devm_of_phy_provider_register(dev,
> +							exynos_usb2_phy_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(drv->dev, "Failed to register phy provider\n");
> +		return PTR_ERR(phy_provider);
> +	}
> +
> +	drv->clk = devm_clk_get(dev, "phy");
> +	if (IS_ERR(drv->clk)) {
> +		dev_err(dev, "Failed to get clock of phy controller\n");
> +		return PTR_ERR(drv->clk);
> +	}
> +
> +	for (i = 0; i < drv->cfg->num_phys; i++) {
> +		char *label = drv->cfg->phys[i].label;
> +		struct usb2_phy_instance *p = &drv->usb2_phy_instances[i];
> +
> +		dev_dbg(dev, "Creating phy \"%s\"\n", label);
> +		p->phy = devm_phy_create(dev, &exynos_usb2_phy_ops, NULL);
> +		if (IS_ERR(p->phy)) {
> +			dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
> +						label);
> +			return PTR_ERR(p->phy);
> +		}
> +
> +		p->cfg = &drv->cfg->phys[i];
> +		p->drv = drv;
> +		phy_set_drvdata(p->phy, p);
> +
> +		clk = clk_get(dev, p->cfg->label);
> +		if (IS_ERR(clk)) {
> +			dev_err(dev, "Failed to get clock of \"%s\" phy\n",
> +								p->cfg->label);
> +			return PTR_ERR(clk);
> +		}
> +
> +		p->rate = clk_get_rate(clk);
> +
> +		if (p->cfg->rate_to_clk) {
> +			p->clk = p->cfg->rate_to_clk(p->rate);
> +			if (p->clk == CLKSEL_ERROR) {
> +				dev_err(dev, "Clock rate (%ld) not supported\n",
> +								p->rate);
> +				clk_put(clk);
> +				return -EINVAL;
> +			}
> +		}
> +		clk_put(clk);
> +	}
> +
> +	return 0;
> +}
> +
> +extern const struct usb2_phy_config exynos4210_usb2_phy_config;
> +extern const struct usb2_phy_config exynos4212_usb2_phy_config;
> +
> +static const struct of_device_id exynos_usb2_phy_of_match[] = {
> +#ifdef CONFIG_PHY_EXYNOS4210_USB2

I don't think you'll need #ifdef here. Anyways the driver data can be 
obtained using the appropriate compatible value in dt data no?

Thanks
Kishon

Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[Tomasz Figa]

Hi Kishon

On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote:
> > Add a new driver for the Exynos USB PHY. The new driver uses the generic
> > PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> > SoC families.
> >
> > Signed-off-by: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >   .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
> >   drivers/phy/Kconfig                                |   23 +-
> >   drivers/phy/Makefile                               |    4 +
> >   drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
> >   drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
> >   drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
> >   drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
> >   7 files changed, 995 insertions(+), 1 deletion(-)
> >   create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> >   create mode 100644 drivers/phy/phy-exynos-usb2.c
> >   create mode 100644 drivers/phy/phy-exynos-usb2.h
> >   create mode 100644 drivers/phy/phy-exynos4210-usb2.c
> >   create mode 100644 drivers/phy/phy-exynos4212-usb2.c
[snip]
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index a344f3d..bdf0fab 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
[snip]
> > @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
> >   	help
> >   	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
> >
> > +config PHY_EXYNOS_USB2
> > +	tristate "Samsung USB 2.0 PHY driver"
> > +	help
> > +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
> > +	  This driver provides common interface to interact, for Samsung
> > +	  USB 2.0 PHY driver.
> 
> I still think we can get rid of this helper driver and have a single 
> driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2.

This helper driver is a really nice way to avoid code duplication, while
still leaving the code clean and readable.

All the Samsung USB 2.0 PHYs require exactly the same semantics
(isolation, reference rate configuration, power up, power on), but each
one has completely different layout of registers and bits inside
registers.

Making a big single driver would end up being identical to the old Exynos
USB2PHY driver with a lot of if and switch statements inside most of
functions, which is not only ugly but makes any further extension hard.

In addition, this approach makes it possible to disable support for SoCs
that are not needed in particular use cases, allowing smaller kernel
images.

> > +
> > +config PHY_EXYNOS4210_USB2
> > +	bool "Support for Exynos 4210"
> > +	depends on PHY_EXYNOS_USB2
> > +	depends on CPU_EXYNOS4210
> > +	help
> > +	  Enable USB PHY support for Exynos 4210
> > +
> > +config PHY_EXYNOS4212_USB2
> > +	bool "Support for Exynos 4212"
> > +	depends on PHY_EXYNOS_USB2
> > +	depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
> > +	help
> > +	  Enable USB PHY support for Exynos 4212
> > +
> >   endmenu
[snip]
> > +extern const struct usb2_phy_config exynos4210_usb2_phy_config;
> > +extern const struct usb2_phy_config exynos4212_usb2_phy_config;
> > +
> > +static const struct of_device_id exynos_usb2_phy_of_match[] = {
> > +#ifdef CONFIG_PHY_EXYNOS4210_USB2
> 
> I don't think you'll need #ifdef here. Anyways the driver data can be 
> obtained using the appropriate compatible value in dt data no?

Huh?

This is not about driver data, but rather about the ability to match the
driver only to devices that are actually supported with selected Kconfig
options.

Best regards,
Tomasz

--
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 v3 1/3] phy: Add new Exynos USB PHY driver

[Kishon Vijay Abraham I]

Hi,

On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote:
> Hi Kishon
>
> On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote:
>>> Add a new driver for the Exynos USB PHY. The new driver uses the generic
>>> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
>>> SoC families.
>>>
>>> Signed-off-by: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>>    .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>>>    drivers/phy/Kconfig                                |   23 +-
>>>    drivers/phy/Makefile                               |    4 +
>>>    drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>>>    drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>>>    drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>>>    drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>>>    7 files changed, 995 insertions(+), 1 deletion(-)
>>>    create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>>>    create mode 100644 drivers/phy/phy-exynos-usb2.c
>>>    create mode 100644 drivers/phy/phy-exynos-usb2.h
>>>    create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>>>    create mode 100644 drivers/phy/phy-exynos4212-usb2.c
> [snip]
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index a344f3d..bdf0fab 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
> [snip]
>>> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
>>>    	help
>>>    	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
>>>
>>> +config PHY_EXYNOS_USB2
>>> +	tristate "Samsung USB 2.0 PHY driver"
>>> +	help
>>> +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
>>> +	  This driver provides common interface to interact, for Samsung
>>> +	  USB 2.0 PHY driver.
>>
>> I still think we can get rid of this helper driver and have a single
>> driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2.
>
> This helper driver is a really nice way to avoid code duplication, while
> still leaving the code clean and readable.
>
> All the Samsung USB 2.0 PHYs require exactly the same semantics
> (isolation, reference rate configuration, power up, power on), but each
> one has completely different layout of registers and bits inside
> registers.

I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] 
and couldn't find that big a difference in register layout. Of course 
there are a few changes in HSIC bit fields and PHYFSEL but that's only 
minimal and could well be handled in a single driver.

[1] -> http://diffchecker.com/py3tp68m
>
> Making a big single driver would end up being identical to the old Exynos
> USB2PHY driver with a lot of if and switch statements inside most of
> functions, which is not only ugly but makes any further extension hard.

Probably we shouldn't try to over design and just convert the old exynos 
usb2 phy driver to the generic phy framework to begin with?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v3 1/3] phy: Add new Exynos USB PHY driver

[Tomasz Figa]

On Wednesday 06 of November 2013 18:20:36 Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote:
> > Hi Kishon
> >
> > On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote:
> >> Hi,
> >>
> >> On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote:
> >>> Add a new driver for the Exynos USB PHY. The new driver uses the generic
> >>> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> >>> SoC families.
> >>>
> >>> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>> ---
> >>>    .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
> >>>    drivers/phy/Kconfig                                |   23 +-
> >>>    drivers/phy/Makefile                               |    4 +
> >>>    drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
> >>>    drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
> >>>    drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
> >>>    drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
> >>>    7 files changed, 995 insertions(+), 1 deletion(-)
> >>>    create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> >>>    create mode 100644 drivers/phy/phy-exynos-usb2.c
> >>>    create mode 100644 drivers/phy/phy-exynos-usb2.h
> >>>    create mode 100644 drivers/phy/phy-exynos4210-usb2.c
> >>>    create mode 100644 drivers/phy/phy-exynos4212-usb2.c
> > [snip]
> >>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> >>> index a344f3d..bdf0fab 100644
> >>> --- a/drivers/phy/Kconfig
> >>> +++ b/drivers/phy/Kconfig
> > [snip]
> >>> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
> >>>    	help
> >>>    	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
> >>>
> >>> +config PHY_EXYNOS_USB2
> >>> +	tristate "Samsung USB 2.0 PHY driver"
> >>> +	help
> >>> +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
> >>> +	  This driver provides common interface to interact, for Samsung
> >>> +	  USB 2.0 PHY driver.
> >>
> >> I still think we can get rid of this helper driver and have a single
> >> driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2.
> >
> > This helper driver is a really nice way to avoid code duplication, while
> > still leaving the code clean and readable.
> >
> > All the Samsung USB 2.0 PHYs require exactly the same semantics
> > (isolation, reference rate configuration, power up, power on), but each
> > one has completely different layout of registers and bits inside
> > registers.
> 
> I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] 
> and couldn't find that big a difference in register layout. Of course 
> there are a few changes in HSIC bit fields and PHYFSEL but that's only 
> minimal and could well be handled in a single driver.
> 
> [1] -> http://diffchecker.com/py3tp68m

This is quite a lot of differences, especially including shifted
bitfields... In addition there is another set of available PHYs
and inter-dependencies between them.

> >
> > Making a big single driver would end up being identical to the old Exynos
> > USB2PHY driver with a lot of if and switch statements inside most of
> > functions, which is not only ugly but makes any further extension hard.
> 
> Probably we shouldn't try to over design and just convert the old exynos 
> usb2 phy driver to the generic phy framework to begin with?

The old exynos USB2 PHY driver is incomplete and has very limited
functionality. It needs a complete redesign to support remaining features
and this is the reason we decided to develop a new driver from scratch.

I believe the way Kamil's driver is designed is definitely the way to go
with Samsung's USB2 PHY drivers, especially considering that support for
more SoCs using the same framework will be added - S3C64xx, S5PV210,
and Exynos5250.

Best regards.
Tomasz

RE: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[David Laight]

> > I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1]
> > and couldn't find that big a difference in register layout. Of course
> > there are a few changes in HSIC bit fields and PHYFSEL but that's only
> > minimal and could well be handled in a single driver.
> >
> > [1] -> http://diffchecker.com/py3tp68m
> 
> This is quite a lot of differences, especially including shifted
> bitfields... In addition there is another set of available PHYs
> and inter-dependencies between them.

Might be worth feeding both files through "sed -e 's/_421[02]_/_421x_/'"
prior to doing the diff.
And maybe changing the order of some definitions so they match.
Then the actual differences will be more obvious.

	David



--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v3 1/3] phy: Add new Exynos USB PHY driver

[Tomasz Figa]

Hi David,

On Wednesday 06 of November 2013 13:03:45 David Laight wrote:
> > > I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1]
> > > and couldn't find that big a difference in register layout. Of course
> > > there are a few changes in HSIC bit fields and PHYFSEL but that's only
> > > minimal and could well be handled in a single driver.
> > >
> > > [1] -> http://diffchecker.com/py3tp68m
> > 
> > This is quite a lot of differences, especially including shifted
> > bitfields... In addition there is another set of available PHYs
> > and inter-dependencies between them.
> 
> Might be worth feeding both files through "sed -e 's/_421[02]_/_421x_/'"
> prior to doing the diff.
> And maybe changing the order of some definitions so they match.
> Then the actual differences will be more obvious.

I have fed it already through my built-in sed when reading.

Still despite many similarities, I think there is enough difference to
justify having different callback functions for both, especially based on
my experience with the old Exynos USB2 PHY driver in drivers/usb/phy, when
trying to make it more complete.

Best regards,
Tomasz

Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver

[Tomasz Figa]

Hi Kamil,

Please see my comments inline.

On Tuesday 05 of November 2013 17:13:19 Kamil Debski wrote:
> Add a new driver for the Exynos USB PHY. The new driver uses the generic
> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> SoC families.
> 
> Signed-off-by: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>  drivers/phy/Kconfig                                |   23 +-
>  drivers/phy/Makefile                               |    4 +
>  drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>  drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>  drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>  drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>  7 files changed, 995 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>  create mode 100644 drivers/phy/phy-exynos-usb2.c
>  create mode 100644 drivers/phy/phy-exynos-usb2.h
>  create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>  create mode 100644 drivers/phy/phy-exynos4212-usb2.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> new file mode 100644
> index 0000000..c8fbc70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> @@ -0,0 +1,52 @@
> +Samsung S5P/EXYNOS SoC series USB PHY

I would not limit this only to S5P and newer series, especially that
I'm planning to add support for S3C64xx using this framework.

Instead I would call it Samsung SoC USB 1.1/2.0 PHY.

> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be one of the listed compatibles:
> +	- "samsung,exynos4210-usbphy"
> +	- "samsung,exynos4212-usbphy"
> +- reg : a list of registers used by phy driver
> +	- first and obligatory is the location of phy modules registers
> +	- second and also required is the location of isolation registers
> +	  (isolation registers control the physical connection between the in
> +	  SoC modules and outside of the SoC, this also can be called enable
> +	  control in the documentation of the SoC)
> +	- third is the location of the mode switch register, this only applies
> +	  to SoCs that have such a feature; mode switching enables to have
> +	  both host and device used the same SoC pins and is commonly used
> +	  when OTG is supported

You should consider using the PMU registers indirectly, as done in Leela
Krisha Amudala's series[1] adding PMU register handling to the watchdog
driver.

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24652

> +- #phy-cells : from the generic phy bindings, must be 1;
> +- clocks and clock-names:
> +	- the "phy" clocks is required by the phy module
> +	- other clocks are associated by name with their respective phys and
> +	  are used to determine the value of the clock settings register

Those names should be explicitly listed.

> +
> +The second cell in the PHY specifier identifies the PHY, its  meaning is

It should say "The first cell", I think?

> +compatible dependent. For the currently supported SoCs (Exynos 4210 and
> +Exynos 4212) it is as follows:
> +  0 - USB device,
> +  1 - USB host,
> +  2 - HSIC0,
> +  3 - HSIC1,
> +
> +Example:
> +
> +For Exynos 4412 (compatible with Exynos 4212):
> +
> +exynos_usbphy: exynos-usbphy@125B0000 {

nit: Node names should be generic and the label is slightly too long, so
a better example would be:

usbphy: phy@125B0000 {

> +	compatible = "samsung,exynos4212-usbphy";
> +	reg = <0x125B0000 0x100 0x10020704 0x0c 0x1001021c 0x4>;
> +	clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>,
> +							<&clock 2>;
> +	clock-names = "phy", "device", "host", "hsic0", "hsic1";
> +	status = "okay";
> +	#phy-cells = <1>;
> +};
> +
> +Then the PHY can be used in other nodes such as:
> +
> +ehci@12580000 {
> +	status = "okay";
> +	phys = <&exynos_usbphy 2>;
> +	phy-names = "hsic0";
> +};
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..bdf0fab 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -14,7 +14,7 @@ config GENERIC_PHY
>  	  API by which phy drivers can create PHY using the phy framework and
>  	  phy users can obtain reference to the PHY. All the users of this
>  	  framework should select this config.
> -
> +	  

Stray white space added.

>  config PHY_EXYNOS_MIPI_VIDEO
>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>  	help
> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
>  	help
>  	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
>  
> +config PHY_EXYNOS_USB2

Wouldn't PHY_SAMSUNG_USB2 be better here?

> +	tristate "Samsung USB 2.0 PHY driver"
> +	help
> +	  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
> +	  This driver provides common interface to interact, for Samsung
> +	  USB 2.0 PHY driver.

IMHO this option should not be visible. Instead the options below should
select it. Also the options below should be made tristate.

> +
> +config PHY_EXYNOS4210_USB2
> +	bool "Support for Exynos 4210"
> +	depends on PHY_EXYNOS_USB2
> +	depends on CPU_EXYNOS4210
> +	help
> +	  Enable USB PHY support for Exynos 4210
> +
> +config PHY_EXYNOS4212_USB2
> +	bool "Support for Exynos 4212"
> +	depends on PHY_EXYNOS_USB2
> +	depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
> +	help
> +	  Enable USB PHY support for Exynos 4212
> + 
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..c87bc65 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
> +obj-$(CONFIG_PHY_EXYNOS5250_USB)	+= phy-exynos5250-usb.o

Missed when removing Exynos5250 support from this series? :)

> +obj-$(CONFIG_PHY_EXYNOS_USB2)		+= phy-exynos-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4212_USB2)	+= phy-exynos4212-usb2.o
> diff --git a/drivers/phy/phy-exynos-usb2.c b/drivers/phy/phy-exynos-usb2.c
> new file mode 100644
> index 0000000..3e9d525
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-usb2.c
> @@ -0,0 +1,234 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver

As I mentioned above, I'd prefer Samsung SoC USB 1.1/2.0 PHY driver. Same
for remaining files that are not Exynos-specific.

> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "phy-exynos-usb2.h"
> +
> +static int exynos_usb2_phy_power_on(struct phy *phy)

s/exynos/samsung/ (not literally, but most of them should be changed)

> +{
> +	struct usb2_phy_instance *inst = phy_get_drvdata(phy);
> +	struct usb2_phy_driver *drv = inst->drv;
> +	int ret;
> +
> +	dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> +							inst->cfg->label);
> +	ret = clk_prepare_enable(drv->clk);
> +	if (ret)
> +		return ret;
> +	if (inst->cfg->power_on) {
> +		spin_lock(&drv->lock);
> +		ret = inst->cfg->power_on(inst);
> +		spin_unlock(&drv->lock);
> +	}
> +	clk_disable_unprepare(drv->clk);
> +	return ret;
> +}
[snip]
> +static struct phy *exynos_usb2_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	struct usb2_phy_driver *drv;
> +
> +	drv = dev_get_drvdata(dev);
> +	if (!drv)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (WARN_ON(args->args[0] >= drv->cfg->num_phys))
> +		return ERR_PTR(-ENODEV);
> +
> +	return drv->usb2_phy_instances[args->args[0]].phy;
> +}
> +
> +static const struct of_device_id exynos_usb2_phy_of_match[];

What about moving the definition here?

> +
> +static int exynos_usb2_phy_probe(struct platform_device *pdev)
> +{
> +	struct usb2_phy_driver *drv;
> +	struct device *dev = &pdev->dev;
> +	struct resource *mem;
> +	struct phy_provider *phy_provider;
> +

nit: Stray blank line.

> +	const struct of_device_id *match;
> +	const struct usb2_phy_config *cfg;
> +	struct clk *clk;
> +

nit: Stray blank line.

> +	int i;
> +

Just to be safe, you should check if pdev->dev.of_node is not NULL here,
to make sure that the driver got instantiated from device tree.

> +	match = of_match_node(exynos_usb2_phy_of_match, pdev->dev.of_node);
> +	if (!match) {
> +		dev_err(dev, "of_match_node() failed\n");
> +		return -EINVAL;
> +	}
> +	cfg = match->data;
> +	if (!cfg) {

Is this even possible?

> +		dev_err(dev, "Failed to get configuration\n");
> +		return -EINVAL;
> +	}
> +
> +	drv = devm_kzalloc(dev, sizeof(struct usb2_phy_driver) +
> +		cfg->num_phys * sizeof(struct usb2_phy_instance), GFP_KERNEL);
> +

nit: Unnecessary blank line.

> +	if (!drv) {
> +		dev_err(dev, "Failed to allocate memory\n");

kmalloc() and friends already print an error message for you.

> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(dev, drv);
> +	spin_lock_init(&drv->lock);
> +
> +	drv->cfg = cfg;
> +	drv->dev = dev;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	drv->reg_phy = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(drv->reg_phy)) {
> +		dev_err(dev, "Failed to map register memory (phy)\n");
> +		return PTR_ERR(drv->reg_phy);
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	drv->reg_isol = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(drv->reg_isol)) {
> +		dev_err(dev, "Failed to map register memory (isolation)\n");
> +		return PTR_ERR(drv->reg_isol);
> +	}
> +
> +	if (drv->cfg->has_mode_switch) {
> +		mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +		drv->reg_mode = devm_ioremap_resource(dev, mem);
> +		if (IS_ERR(drv->reg_mode)) {
> +			dev_err(dev, "Failed to map register memory (mode switch)\n");
> +			return PTR_ERR(drv->reg_mode);
> +		}
> +	}
> +
> +	phy_provider = devm_of_phy_provider_register(dev,
> +							exynos_usb2_phy_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(drv->dev, "Failed to register phy provider\n");
> +		return PTR_ERR(phy_provider);
> +	}

The provider should be registered as the last thing in the sequence, as
the driver must be ready for handling PHY requests as soon as
of_phy_provider_register() returns.

> +
> +	drv->clk = devm_clk_get(dev, "phy");
> +	if (IS_ERR(drv->clk)) {
> +		dev_err(dev, "Failed to get clock of phy controller\n");
> +		return PTR_ERR(drv->clk);
> +	}
> +
> +	for (i = 0; i < drv->cfg->num_phys; i++) {
> +		char *label = drv->cfg->phys[i].label;
> +		struct usb2_phy_instance *p = &drv->usb2_phy_instances[i];
> +
> +		dev_dbg(dev, "Creating phy \"%s\"\n", label);
> +		p->phy = devm_phy_create(dev, &exynos_usb2_phy_ops, NULL);
> +		if (IS_ERR(p->phy)) {
> +			dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
> +						label);
> +			return PTR_ERR(p->phy);
> +		}
> +
> +		p->cfg = &drv->cfg->phys[i];
> +		p->drv = drv;
> +		phy_set_drvdata(p->phy, p);
> +
> +		clk = clk_get(dev, p->cfg->label);
> +		if (IS_ERR(clk)) {
> +			dev_err(dev, "Failed to get clock of \"%s\" phy\n",
> +								p->cfg->label);
> +			return PTR_ERR(clk);
> +		}
> +
> +		p->rate = clk_get_rate(clk);
> +
> +		if (p->cfg->rate_to_clk) {
> +			p->clk = p->cfg->rate_to_clk(p->rate);
> +			if (p->clk == CLKSEL_ERROR) {

Introducing custom error codes does not sound like a good idea.

What about simply making the p->clk a signed integer or separating the
value and status returned by this function and returning standard error
codes?

> +				dev_err(dev, "Clock rate (%ld) not supported\n",
> +								p->rate);
> +				clk_put(clk);
> +				return -EINVAL;
> +			}

Technically this should happen at the time of calling PHY enable, while
a reference to the clock should be kept through the whole PHY lifetime.
In addition, the clock should be prepare_enabled before it is used.

So, to recall, here you could call devm_clk_get(...), then
clk_prepare_enable() and clk_get_rate() when the PHY is being enabled
and finally clk_disable_unprepare() when it is being disabled.

> +		}
> +		clk_put(clk);
> +	}
> +
> +	return 0;
> +}
> +
> +extern const struct usb2_phy_config exynos4210_usb2_phy_config;
> +extern const struct usb2_phy_config exynos4212_usb2_phy_config;

phy-exynos-usb2.h would be a better place for these externs.

> +
> +static const struct of_device_id exynos_usb2_phy_of_match[] = {
> +#ifdef CONFIG_PHY_EXYNOS4210_USB2
> +	{
> +		.compatible = "samsung,exynos4210-usb2-phy",
> +		.data = &exynos4210_usb2_phy_config,
> +	},
> +#endif
> +#ifdef CONFIG_PHY_EXYNOS4212_USB2
> +	{
> +		.compatible = "samsung,exynos4212-usb2-phy",
> +		.data = &exynos4212_usb2_phy_config,
> +	},
> +#endif
> +	{ },
> +};
> +
> +static struct platform_driver exynos_usb2_phy_driver = {
> +	.probe	= exynos_usb2_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_usb2_phy_of_match,
> +		.name		= "exynos-usb2-phy",
> +		.owner		= THIS_MODULE,
> +	}
> +};
> +
> +module_platform_driver(exynos_usb2_phy_driver);
> +MODULE_DESCRIPTION("Samsung S5P/EXYNOS SoC USB PHY driver");
> +MODULE_AUTHOR("Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:exynos-usb2-phy");
> +
> diff --git a/drivers/phy/phy-exynos-usb2.h b/drivers/phy/phy-exynos-usb2.h
> new file mode 100644
> index 0000000..91e4f73
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-usb2.h
> @@ -0,0 +1,87 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _PHY_EXYNOS_USB2_H
> +#define _PHY_EXYNOS_USB2_H
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define CLKSEL_ERROR                       -1

Yuck.

> +
> +#ifndef KHZ
> +#define KHZ 1000
> +#endif
> +
> +#ifndef MHZ
> +#define MHZ (KHZ * KHZ)
> +#endif

Do you need the ifndef's above?

> +
> +enum phy_type {
> +	PHY_DEVICE,
> +	PHY_HOST,
> +};
> +
> +enum usb2_phy_state {
> +	STATE_OFF,
> +	STATE_ON,
> +};

Hmm? What is the purpose of this enum? If I got it right, it could be
replaced with a simple bool is_enabled.

> +
> +struct usb2_phy_driver;
> +struct usb2_phy_instance;
> +struct usb2_phy_config;
> +
> +struct usb2_phy_instance {
> +	struct usb2_phy_driver *drv;
> +	struct phy *phy;
> +	const struct common_phy *cfg;
> +	enum usb2_phy_state state;
> +	u32 clk;
> +	unsigned long rate;
> +};
> +
> +struct usb2_phy_driver {
> +	struct device *dev;
> +	spinlock_t lock;
> +	void __iomem *reg_phy;
> +	void __iomem *reg_isol;
> +	void __iomem *reg_mode;
> +	const struct usb2_phy_config *cfg;
> +	struct clk *clk;
> +	struct usb2_phy_instance usb2_phy_instances[0];

It's quite a long name for a field. What about simply calling it instances
instead?

Also, I'm not sure about this, but shouldn't it be instances[] not [0]?

> +};
> +
> +struct common_phy {
> +	char *label;
> +	enum phy_type type;
> +	unsigned int id;
> +	u32 (*rate_to_clk)(unsigned long);
> +	int (*power_on)(struct usb2_phy_instance*);
> +	int (*power_off)(struct usb2_phy_instance*);
> +};
> +
> +
> +struct usb2_phy_config {
> +	int num_phys;
> +	const struct common_phy *phys;
> +	char has_mode_switch;
> +};

Names of structs used in this driver are quite generic and might lead to
collisions in future, especially struct common_phy. Maybe they should be
namespaced with samsung_ prefix?

> +
> +#endif
> +
[snip]
> diff --git a/drivers/phy/phy-exynos4212-usb2.c b/drivers/phy/phy-exynos4212-usb2.c
> new file mode 100644
> index 0000000..654efe0
> --- /dev/null
> +++ b/drivers/phy/phy-exynos4212-usb2.c
[snip]
> +static int exynos4212_power_on(struct usb2_phy_instance *inst)
> +{
> +	struct usb2_phy_driver *drv = inst->drv;
> +
> +	inst->state = STATE_ON;
> +	exynos4212_phy_pwr(inst, 1);
> +	exynos4212_isol(inst, 0);
> +
> +	/* Power on the device, as it is necessary for HSIC to work */
> +	if (inst->cfg->id == EXYNOS4212_HSIC0) {

Is it also needed for HSIC1?

> +		struct usb2_phy_instance *device =
> +					&drv->usb2_phy_instances[EXYNOS4212_DEVICE];
> +		exynos4212_phy_pwr(device, 1);
> +		exynos4212_isol(device, 0);

Are you sure that you also need to disable the isolation, not just enable
the PHY?

Also what happens if you enable the device first and then HSIC? Wouldn't
it cause transmission errors on device link, due to PHY being reset?

Probably a simple && !device->is_enabled in the if clause above could
help, like you did in power off callback.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v3 1/3] phy: Add new Exynos USB PHY driver

[Yuvaraj Cd]

On Tue, Nov 5, 2013 at 9:43 PM, Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> Add a new driver for the Exynos USB PHY. The new driver uses the generic
> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> SoC families.
>
> Signed-off-by: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/phy/samsung-usbphy.txt     |   52 ++++
>  drivers/phy/Kconfig                                |   23 +-
>  drivers/phy/Makefile                               |    4 +
>  drivers/phy/phy-exynos-usb2.c                      |  234 ++++++++++++++
>  drivers/phy/phy-exynos-usb2.h                      |   87 ++++++
>  drivers/phy/phy-exynos4210-usb2.c                  |  272 ++++++++++++++++
>  drivers/phy/phy-exynos4212-usb2.c                  |  324 ++++++++++++++++++++
>  7 files changed, 995 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt
>  create mode 100644 drivers/phy/phy-exynos-usb2.c
>  create mode 100644 drivers/phy/phy-exynos-usb2.h
>  create mode 100644 drivers/phy/phy-exynos4210-usb2.c
>  create mode 100644 drivers/phy/phy-exynos4212-usb2.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> new file mode 100644
> index 0000000..c8fbc70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> @@ -0,0 +1,52 @@
> +Samsung S5P/EXYNOS SoC series USB PHY
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be one of the listed compatibles:
> +       - "samsung,exynos4210-usbphy"
> +       - "samsung,exynos4212-usbphy"
> +- reg : a list of registers used by phy driver
> +       - first and obligatory is the location of phy modules registers
> +       - second and also required is the location of isolation registers
> +         (isolation registers control the physical connection between the in
> +         SoC modules and outside of the SoC, this also can be called enable
> +         control in the documentation of the SoC)
> +       - third is the location of the mode switch register, this only applies
> +         to SoCs that have such a feature; mode switching enables to have
> +         both host and device used the same SoC pins and is commonly used
> +         when OTG is supported
> +- #phy-cells : from the generic phy bindings, must be 1;
> +- clocks and clock-names:
> +       - the "phy" clocks is required by the phy module
> +       - other clocks are associated by name with their respective phys and
> +         are used to determine the value of the clock settings register
> +
> +The second cell in the PHY specifier identifies the PHY, its  meaning is
> +compatible dependent. For the currently supported SoCs (Exynos 4210 and
> +Exynos 4212) it is as follows:
> +  0 - USB device,
> +  1 - USB host,
> +  2 - HSIC0,
> +  3 - HSIC1,
> +
> +Example:
> +
> +For Exynos 4412 (compatible with Exynos 4212):
> +
> +exynos_usbphy: exynos-usbphy@125B0000 {
> +       compatible = "samsung,exynos4212-usbphy";
> +       reg = <0x125B0000 0x100 0x10020704 0x0c 0x1001021c 0x4>;
> +       clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>,
> +                                                       <&clock 2>;
> +       clock-names = "phy", "device", "host", "hsic0", "hsic1";
> +       status = "okay";
> +       #phy-cells = <1>;
> +};
> +
> +Then the PHY can be used in other nodes such as:
> +
> +ehci@12580000 {
> +       status = "okay";
> +       phys = <&exynos_usbphy 2>;
> +       phy-names = "hsic0";
> +};
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..bdf0fab 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -14,7 +14,7 @@ config GENERIC_PHY
>           API by which phy drivers can create PHY using the phy framework and
>           phy users can obtain reference to the PHY. All the users of this
>           framework should select this config.
> -
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>         tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>         help
> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO
>         help
>           Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_EXYNOS_USB2
> +       tristate "Samsung USB 2.0 PHY driver"
> +       help
> +         Enable this to support Samsung USB phy helper driver for Samsung SoCs.
> +         This driver provides common interface to interact, for Samsung
> +         USB 2.0 PHY driver.
> +
> +config PHY_EXYNOS4210_USB2
> +       bool "Support for Exynos 4210"
> +       depends on PHY_EXYNOS_USB2
> +       depends on CPU_EXYNOS4210
> +       help
> +         Enable USB PHY support for Exynos 4210
> +
> +config PHY_EXYNOS4212_USB2
> +       bool "Support for Exynos 4212"
> +       depends on PHY_EXYNOS_USB2
> +       depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
> +       help
> +         Enable USB PHY support for Exynos 4212
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..c87bc65 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)       += phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)    += phy-exynos-mipi-video.o
>  obj-$(CONFIG_OMAP_USB2)                        += phy-omap-usb2.o
>  obj-$(CONFIG_TWL4030_USB)              += phy-twl4030-usb.o
> +obj-$(CONFIG_PHY_EXYNOS5250_USB)       += phy-exynos5250-usb.o
> +obj-$(CONFIG_PHY_EXYNOS_USB2)          += phy-exynos-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4210_USB2)      += phy-exynos4210-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS4212_USB2)      += phy-exynos4212-usb2.o
> diff --git a/drivers/phy/phy-exynos-usb2.c b/drivers/phy/phy-exynos-usb2.c
> new file mode 100644
> index 0000000..3e9d525
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-usb2.c
> @@ -0,0 +1,234 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "phy-exynos-usb2.h"
> +
> +static int exynos_usb2_phy_power_on(struct phy *phy)
> +{
> +       struct usb2_phy_instance *inst = phy_get_drvdata(phy);
> +       struct usb2_phy_driver *drv = inst->drv;
> +       int ret;
> +
> +       dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> +                                                       inst->cfg->label);
> +       ret = clk_prepare_enable(drv->clk);
> +       if (ret)
> +               return ret;
> +       if (inst->cfg->power_on) {
> +               spin_lock(&drv->lock);
> +               ret = inst->cfg->power_on(inst);
> +               spin_unlock(&drv->lock);
> +       }
> +       clk_disable_unprepare(drv->clk);
> +       return ret;
> +}
> +
> +static int exynos_usb2_phy_power_off(struct phy *phy)
> +{
> +       struct usb2_phy_instance *inst = phy_get_drvdata(phy);
> +       struct usb2_phy_driver *drv = inst->drv;
> +       int ret;
> +
> +       dev_dbg(drv->dev, "Request to power_off \"%s\" usb phy\n",
> +                                                       inst->cfg->label);
> +       ret = clk_prepare_enable(drv->clk);
> +       if (ret)
> +               return ret;
> +       if (inst->cfg->power_off) {
> +               spin_lock(&drv->lock);
> +               ret = inst->cfg->power_off(inst);
> +               spin_unlock(&drv->lock);
> +       }
> +       clk_disable_unprepare(drv->clk);
> +       return ret;
> +}
> +
> +static struct phy_ops exynos_usb2_phy_ops = {
> +       .power_on       = exynos_usb2_phy_power_on,
> +       .power_off      = exynos_usb2_phy_power_off,
> +       .owner          = THIS_MODULE,
> +};
> +
> +static struct phy *exynos_usb2_phy_xlate(struct device *dev,
> +                                       struct of_phandle_args *args)
> +{
> +       struct usb2_phy_driver *drv;
> +
> +       drv = dev_get_drvdata(dev);
> +       if (!drv)
> +               return ERR_PTR(-EINVAL);
> +
> +       if (WARN_ON(args->args[0] >= drv->cfg->num_phys))
> +               return ERR_PTR(-ENODEV);
> +
> +       return drv->usb2_phy_instances[args->args[0]].phy;
> +}
> +
> +static const struct of_device_id exynos_usb2_phy_of_match[];
> +
> +static int exynos_usb2_phy_probe(struct platform_device *pdev)
> +{
> +       struct usb2_phy_driver *drv;
> +       struct device *dev = &pdev->dev;
> +       struct resource *mem;
> +       struct phy_provider *phy_provider;
> +
> +       const struct of_device_id *match;
> +       const struct usb2_phy_config *cfg;
> +       struct clk *clk;
> +
> +       int i;
> +
> +       match = of_match_node(exynos_usb2_phy_of_match, pdev->dev.of_node);
> +       if (!match) {
> +               dev_err(dev, "of_match_node() failed\n");
> +               return -EINVAL;
> +       }
> +       cfg = match->data;
> +       if (!cfg) {
> +               dev_err(dev, "Failed to get configuration\n");
> +               return -EINVAL;
> +       }
> +
> +       drv = devm_kzalloc(dev, sizeof(struct usb2_phy_driver) +
> +               cfg->num_phys * sizeof(struct usb2_phy_instance), GFP_KERNEL);
> +
> +       if (!drv) {
> +               dev_err(dev, "Failed to allocate memory\n");
> +               return -ENOMEM;
> +       }
> +
> +       dev_set_drvdata(dev, drv);
> +       spin_lock_init(&drv->lock);
> +
> +       drv->cfg = cfg;
> +       drv->dev = dev;
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       drv->reg_phy = devm_ioremap_resource(dev, mem);
> +       if (IS_ERR(drv->reg_phy)) {
> +               dev_err(dev, "Failed to map register memory (phy)\n");
> +               return PTR_ERR(drv->reg_phy);
> +       }
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       drv->reg_isol = devm_ioremap_resource(dev, mem);
> +       if (IS_ERR(drv->reg_isol)) {
> +               dev_err(dev, "Failed to map register memory (isolation)\n");
> +               return PTR_ERR(drv->reg_isol);
> +       }
> +
> +       if (drv->cfg->has_mode_switch) {
> +               mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +               drv->reg_mode = devm_ioremap_resource(dev, mem);
> +               if (IS_ERR(drv->reg_mode)) {
> +                       dev_err(dev, "Failed to map register memory (mode switch)\n");
> +                       return PTR_ERR(drv->reg_mode);
> +               }
> +       }
> +
> +       phy_provider = devm_of_phy_provider_register(dev,
> +                                                       exynos_usb2_phy_xlate);
> +       if (IS_ERR(phy_provider)) {
> +               dev_err(drv->dev, "Failed to register phy provider\n");
> +               return PTR_ERR(phy_provider);
> +       }
> +
> +       drv->clk = devm_clk_get(dev, "phy");
> +       if (IS_ERR(drv->clk)) {
> +               dev_err(dev, "Failed to get clock of phy controller\n");
> +               return PTR_ERR(drv->clk);
> +       }
> +
> +       for (i = 0; i < drv->cfg->num_phys; i++) {
> +               char *label = drv->cfg->phys[i].label;
> +               struct usb2_phy_instance *p = &drv->usb2_phy_instances[i];
> +
> +               dev_dbg(dev, "Creating phy \"%s\"\n", label);
> +               p->phy = devm_phy_create(dev, &exynos_usb2_phy_ops, NULL);
> +               if (IS_ERR(p->phy)) {
> +                       dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
> +                                               label);
> +                       return PTR_ERR(p->phy);
> +               }
> +
> +               p->cfg = &drv->cfg->phys[i];
> +               p->drv = drv;
> +               phy_set_drvdata(p->phy, p);
> +
> +               clk = clk_get(dev, p->cfg->label);
> +               if (IS_ERR(clk)) {
> +                       dev_err(dev, "Failed to get clock of \"%s\" phy\n",
> +                                                               p->cfg->label);
> +                       return PTR_ERR(clk);
> +               }
> +
> +               p->rate = clk_get_rate(clk);
> +
> +               if (p->cfg->rate_to_clk) {
> +                       p->clk = p->cfg->rate_to_clk(p->rate);
> +                       if (p->clk == CLKSEL_ERROR) {
> +                               dev_err(dev, "Clock rate (%ld) not supported\n",
> +                                                               p->rate);
> +                               clk_put(clk);
> +                               return -EINVAL;
> +                       }
> +               }
> +               clk_put(clk);
> +       }
> +
> +       return 0;
> +}
> +
> +extern const struct usb2_phy_config exynos4210_usb2_phy_config;
> +extern const struct usb2_phy_config exynos4212_usb2_phy_config;
> +
> +static const struct of_device_id exynos_usb2_phy_of_match[] = {
> +#ifdef CONFIG_PHY_EXYNOS4210_USB2
> +       {
> +               .compatible = "samsung,exynos4210-usb2-phy",
> +               .data = &exynos4210_usb2_phy_config,
> +       },
> +#endif
> +#ifdef CONFIG_PHY_EXYNOS4212_USB2
> +       {
> +               .compatible = "samsung,exynos4212-usb2-phy",
> +               .data = &exynos4212_usb2_phy_config,
> +       },
> +#endif
> +       { },
> +};
> +
> +static struct platform_driver exynos_usb2_phy_driver = {
> +       .probe  = exynos_usb2_phy_probe,
> +       .driver = {
> +               .of_match_table = exynos_usb2_phy_of_match,
> +               .name           = "exynos-usb2-phy",
> +               .owner          = THIS_MODULE,
> +       }
> +};
> +
> +module_platform_driver(exynos_usb2_phy_driver);
> +MODULE_DESCRIPTION("Samsung S5P/EXYNOS SoC USB PHY driver");
> +MODULE_AUTHOR("Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:exynos-usb2-phy");
> +
> diff --git a/drivers/phy/phy-exynos-usb2.h b/drivers/phy/phy-exynos-usb2.h
> new file mode 100644
> index 0000000..91e4f73
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-usb2.h
> @@ -0,0 +1,87 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _PHY_EXYNOS_USB2_H
> +#define _PHY_EXYNOS_USB2_H
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define CLKSEL_ERROR                       -1
> +
> +#ifndef KHZ
> +#define KHZ 1000
> +#endif
> +
> +#ifndef MHZ
> +#define MHZ (KHZ * KHZ)
> +#endif
> +
> +enum phy_type {
> +       PHY_DEVICE,
> +       PHY_HOST,
> +};
> +
> +enum usb2_phy_state {
> +       STATE_OFF,
> +       STATE_ON,
> +};
> +
> +struct usb2_phy_driver;
> +struct usb2_phy_instance;
> +struct usb2_phy_config;
> +
> +struct usb2_phy_instance {
> +       struct usb2_phy_driver *drv;
> +       struct phy *phy;
> +       const struct common_phy *cfg;
> +       enum usb2_phy_state state;
> +       u32 clk;
> +       unsigned long rate;
> +};
> +
> +struct usb2_phy_driver {
> +       struct device *dev;
> +       spinlock_t lock;
> +       void __iomem *reg_phy;
> +       void __iomem *reg_isol;
> +       void __iomem *reg_mode;
> +       const struct usb2_phy_config *cfg;
> +       struct clk *clk;
> +       struct usb2_phy_instance usb2_phy_instances[0];
> +};
> +
> +struct common_phy {
> +       char *label;
> +       enum phy_type type;
> +       unsigned int id;
> +       u32 (*rate_to_clk)(unsigned long);
> +       int (*power_on)(struct usb2_phy_instance*);
> +       int (*power_off)(struct usb2_phy_instance*);
> +};
> +
> +
> +struct usb2_phy_config {
> +       int num_phys;
> +       const struct common_phy *phys;
> +       char has_mode_switch;
> +};
> +
> +#endif
> +
> diff --git a/drivers/phy/phy-exynos4210-usb2.c b/drivers/phy/phy-exynos4210-usb2.c
> new file mode 100644
> index 0000000..d04ee8e
> --- /dev/null
> +++ b/drivers/phy/phy-exynos4210-usb2.c
> @@ -0,0 +1,272 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "phy-exynos-usb2.h"
> +
> +/* Exynos USB PHY registers */
> +
> +/* PHY power control */
> +#define EXYNOS_4210_UPHYPWR                    0x0
> +
> +#define EXYNOS_4210_UPHYPWR_PHY0_SUSPEND       (1 << 0)
> +#define EXYNOS_4210_UPHYPWR_PHY0_PWR           (1 << 3)
> +#define EXYNOS_4210_UPHYPWR_PHY0_OTG_PWR       (1 << 4)
> +#define EXYNOS_4210_UPHYPWR_PHY0_SLEEP         (1 << 5)
> +#define EXYNOS_4210_UPHYPWR_PHY0       ( \
> +       EXYNOS_4210_UPHYPWR_PHY0_SUSPEND | \
> +       EXYNOS_4210_UPHYPWR_PHY0_PWR | \
> +       EXYNOS_4210_UPHYPWR_PHY0_OTG_PWR | \
> +       EXYNOS_4210_UPHYPWR_PHY0_SLEEP)
> +
> +#define EXYNOS_4210_UPHYPWR_PHY1_SUSPEND       (1 << 6)
> +#define EXYNOS_4210_UPHYPWR_PHY1_PWR           (1 << 7)
> +#define EXYNOS_4210_UPHYPWR_PHY1_SLEEP         (1 << 8)
> +#define EXYNOS_4210_UPHYPWR_PHY1 ( \
> +       EXYNOS_4210_UPHYPWR_PHY1_SUSPEND | \
> +       EXYNOS_4210_UPHYPWR_PHY1_PWR | \
> +       EXYNOS_4210_UPHYPWR_PHY1_SLEEP)
> +
> +#define EXYNOS_4210_UPHYPWR_HSCI0_SUSPEND      (1 << 9)
> +#define EXYNOS_4210_UPHYPWR_HSCI0_SLEEP                (1 << 10)
> +#define EXYNOS_4210_UPHYPWR_HSCI0 ( \
> +       EXYNOS_4210_UPHYPWR_HSCI0_SUSPEND | \
> +       EXYNOS_4210_UPHYPWR_HSCI0_SLEEP)
> +
> +#define EXYNOS_4210_UPHYPWR_HSCI1_SUSPEND      (1 << 11)
> +#define EXYNOS_4210_UPHYPWR_HSCI1_SLEEP                (1 << 12)
> +#define EXYNOS_4210_UPHYPWR_HSCI1 ( \
> +       EXYNOS_4210_UPHYPWR_HSCI1_SUSPEND | \
> +       EXYNOS_4210_UPHYPWR_HSCI1_SLEEP)
> +
> +/* PHY clock control */
> +#define EXYNOS_4210_UPHYCLK                    0x4
> +
> +#define EXYNOS_4210_UPHYCLK_PHYFSEL_MASK       (0x3 << 0)
> +#define EXYNOS_4210_UPHYCLK_PHYFSEL_48MHZ      (0x0 << 0)
> +#define EXYNOS_4210_UPHYCLK_PHYFSEL_24MHZ      (0x3 << 0)
> +#define EXYNOS_4210_UPHYCLK_PHYFSEL_12MHZ      (0x2 << 0)
> +
> +#define EXYNOS_4210_UPHYCLK_PHY0_ID_PULLUP     (0x1 << 2)
> +#define EXYNOS_4210_UPHYCLK_PHY0_COMMON_ON     (0x1 << 4)
> +#define EXYNOS_4210_UPHYCLK_PHY1_COMMON_ON     (0x1 << 7)
> +
> +/* PHY reset control */
> +#define EXYNOS_4210_UPHYRST                    0x8
> +
> +#define EXYNOS_4210_URSTCON_PHY0               (1 << 0)
> +#define EXYNOS_4210_URSTCON_OTG_HLINK          (1 << 1)
> +#define EXYNOS_4210_URSTCON_OTG_PHYLINK                (1 << 2)
> +#define EXYNOS_4210_URSTCON_PHY1_ALL           (1 << 3)
> +#define EXYNOS_4210_URSTCON_PHY1_P0            (1 << 4)
> +#define EXYNOS_4210_URSTCON_PHY1_P1P2          (1 << 5)
> +#define EXYNOS_4210_URSTCON_HOST_LINK_ALL      (1 << 6)
> +#define EXYNOS_4210_URSTCON_HOST_LINK_P0       (1 << 7)
> +#define EXYNOS_4210_URSTCON_HOST_LINK_P1       (1 << 8)
> +#define EXYNOS_4210_URSTCON_HOST_LINK_P2       (1 << 9)
> +
> +/* Isolation, configured in the power management unit */
> +#define EXYNOS_4210_USB_ISOL_DEVICE_OFFSET     0x0
> +#define EXYNOS_4210_USB_ISOL_DEVICE            (1 << 0)
> +#define EXYNOS_4210_USB_ISOL_HOST_OFFSET       0x4
> +#define EXYNOS_4210_USB_ISOL_HOST              (1 << 0)
> +
> +/* USBYPHY1 Floating prevention */
> +#define EXYNOS_4210_UPHY1CON                   0x34
> +#define EXYNOS_4210_UPHY1CON_FLOAT_PREVENTION  0x1
> +
> +enum exynos4210_phy_id {
> +       EXYNOS4210_DEVICE,
> +       EXYNOS4210_HOST,
> +       EXYNOS4210_HSIC0,
> +       EXYNOS4210_HSIC1,
> +       EXYNOS4210_NUM_PHYS,
> +};
> +
> +/*
> + * exynos4210_rate_to_clk() converts the supplied clock rate to the value that
> + * can be written to the phy register.
> + */
> +static u32 exynos4210_rate_to_clk(unsigned long rate)
> +{
> +       unsigned int clksel;
> +
> +       switch (rate) {
> +       case 12 * MHZ:
> +               clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_12MHZ;
> +               break;
> +       case 24 * MHZ:
> +               clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_24MHZ;
> +               break;
> +       case 48 * MHZ:
> +               clksel = EXYNOS_4210_UPHYCLK_PHYFSEL_48MHZ;
> +               break;
> +       default:
> +               clksel = CLKSEL_ERROR;
> +       }
> +
> +       return clksel;
> +}
> +
> +static void exynos4210_isol(struct usb2_phy_instance *inst, bool on)
Can we have this functionality as xxxx_pwr_on and plugged into generic phy ops?
> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +       u32 offset;
> +       u32 mask;
> +       u32 tmp;
> +
> +       if (!drv->reg_isol)
> +               return;
> +
> +       switch (inst->cfg->id) {
> +       case EXYNOS4210_DEVICE:
> +               offset = EXYNOS_4210_USB_ISOL_DEVICE_OFFSET;
> +               mask = EXYNOS_4210_USB_ISOL_DEVICE;
> +               break;
> +       case EXYNOS4210_HOST:
> +               offset = EXYNOS_4210_USB_ISOL_HOST_OFFSET;
> +               mask = EXYNOS_4210_USB_ISOL_HOST;
> +               break;
> +       default:
> +               return;
> +       };
> +
> +       tmp = readl(drv->reg_isol + offset);
> +       if (on)
> +               tmp &= ~mask;
> +       else
> +               tmp |= mask;
> +       writel(tmp, drv->reg_isol + offset);
> +}
> +
> +static void exynos4210_phy_pwr(struct usb2_phy_instance *inst, bool on)

It would be better to rename this as xxxx_phy_init and plugged into
phy_ops .init routine.
With this, we can use phy_ops->init in ehci-exynos driver to do the
init and power_on which
would be more aligned with the generic PHY framework.
@Kishon: Please correct me if i am wrong.

> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +       u32 rstbits = 0;
> +       u32 phypwr = 0;
> +       u32 rst;
> +       u32 pwr;
> +
> +       switch (inst->cfg->id) {
> +       case EXYNOS4210_DEVICE:
> +               phypwr =        EXYNOS_4210_UPHYPWR_PHY0;
> +               rstbits =       EXYNOS_4210_URSTCON_PHY0;
> +               break;
> +       case EXYNOS4210_HOST:
> +               phypwr =        EXYNOS_4210_UPHYPWR_PHY1;
> +               rstbits =       EXYNOS_4210_URSTCON_PHY1_ALL |
> +                               EXYNOS_4210_URSTCON_PHY1_P0 |
> +                               EXYNOS_4210_URSTCON_PHY1_P1P2 |
> +                               EXYNOS_4210_URSTCON_HOST_LINK_ALL |
> +                               EXYNOS_4210_URSTCON_HOST_LINK_P0;
> +               writel(on, drv->reg_phy + EXYNOS_4210_UPHY1CON);
> +               break;
> +       case EXYNOS4210_HSIC0:
> +               phypwr =        EXYNOS_4210_UPHYPWR_HSCI0;
> +               rstbits =       EXYNOS_4210_URSTCON_PHY1_P1P2 |
> +                               EXYNOS_4210_URSTCON_HOST_LINK_P1;
> +               break;
> +       case EXYNOS4210_HSIC1:
> +               phypwr =        EXYNOS_4210_UPHYPWR_HSCI1;
> +               rstbits =       EXYNOS_4210_URSTCON_PHY1_P1P2 |
> +                               EXYNOS_4210_URSTCON_HOST_LINK_P2;
> +               break;
> +       };
> +
> +       if (on) {
> +               writel(inst->clk, drv->reg_phy + EXYNOS_4210_UPHYCLK);
> +
> +               pwr = readl(drv->reg_phy + EXYNOS_4210_UPHYPWR);
> +               pwr &= ~phypwr;
> +               writel(pwr, drv->reg_phy + EXYNOS_4210_UPHYPWR);
> +
> +               rst = readl(drv->reg_phy + EXYNOS_4210_UPHYRST);
> +               rst |= rstbits;
> +               writel(rst, drv->reg_phy + EXYNOS_4210_UPHYRST);
> +               udelay(10);
> +               rst &= ~rstbits;
> +               writel(rst, drv->reg_phy + EXYNOS_4210_UPHYRST);
> +       } else {
> +               pwr = readl(drv->reg_phy + EXYNOS_4210_UPHYPWR);
> +               pwr |= phypwr;
> +               writel(pwr, drv->reg_phy + EXYNOS_4210_UPHYPWR);
> +       }
> +}
> +
> +static int exynos4210_power_on(struct usb2_phy_instance *inst)
> +{
> +       /* Order of initialisation is important - first power then isolation */
> +       exynos4210_phy_pwr(inst, 1);
> +       exynos4210_isol(inst, 0);
> +
> +       return 0;
> +}
> +
> +static int exynos4210_power_off(struct usb2_phy_instance *inst)
> +{
> +       exynos4210_isol(inst, 1);
> +       exynos4210_phy_pwr(inst, 0);
> +
> +       return 0;
> +}
> +
> +
> +static const struct common_phy exynos4210_phys[] = {
> +       {
> +               .label          = "device",
> +               .type           = PHY_DEVICE,
> +               .id             = EXYNOS4210_DEVICE,
> +               .rate_to_clk    = exynos4210_rate_to_clk,
> +               .power_on       = exynos4210_power_on,
> +               .power_off      = exynos4210_power_off,
> +       },
> +       {
> +               .label          = "host",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4210_HOST,
> +               .rate_to_clk    = exynos4210_rate_to_clk,
> +               .power_on       = exynos4210_power_on,
> +               .power_off      = exynos4210_power_off,
> +       },
> +       {
> +               .label          = "hsic0",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4210_HSIC0,
> +               .rate_to_clk    = exynos4210_rate_to_clk,
> +               .power_on       = exynos4210_power_on,
> +               .power_off      = exynos4210_power_off,
> +       },
> +       {
> +               .label          = "hsic1",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4210_HSIC1,
> +               .rate_to_clk    = exynos4210_rate_to_clk,
> +               .power_on       = exynos4210_power_on,
> +               .power_off      = exynos4210_power_off,
> +       },
> +       {},
> +};
> +
> +const struct usb2_phy_config exynos4210_usb2_phy_config = {
> +       .num_phys               = EXYNOS4210_NUM_PHYS,
> +       .phys                   = exynos4210_phys,
> +       .has_mode_switch        = 1,
> +};
> +
> diff --git a/drivers/phy/phy-exynos4212-usb2.c b/drivers/phy/phy-exynos4212-usb2.c
> new file mode 100644
> index 0000000..654efe0
> --- /dev/null
> +++ b/drivers/phy/phy-exynos4212-usb2.c
> @@ -0,0 +1,324 @@
> +/*
> + * Samsung S5P/EXYNOS SoC series USB PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski <k.debski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "phy-exynos-usb2.h"
> +
> +/* Exynos USB PHY registers */
> +
> +/* PHY power control */
> +#define EXYNOS_4212_UPHYPWR                    0x0
> +
> +#define EXYNOS_4212_UPHYPWR_DEV_SUSPEND                (1 << 0)
> +#define EXYNOS_4212_UPHYPWR_DEV_PWR            (1 << 3)
> +#define EXYNOS_4212_UPHYPWR_DEV_OTG_PWR                (1 << 4)
> +#define EXYNOS_4212_UPHYPWR_DEV_SLEEP          (1 << 5)
> +#define EXYNOS_4212_UPHYPWR_DEV        ( \
> +       EXYNOS_4212_UPHYPWR_DEV_SUSPEND | \
> +       EXYNOS_4212_UPHYPWR_DEV_PWR | \
> +       EXYNOS_4212_UPHYPWR_DEV_OTG_PWR | \
> +       EXYNOS_4212_UPHYPWR_DEV_SLEEP)
> +
> +#define EXYNOS_4212_UPHYPWR_HOST_SUSPEND       (1 << 6)
> +#define EXYNOS_4212_UPHYPWR_HOST_PWR           (1 << 7)
> +#define EXYNOS_4212_UPHYPWR_HOST_SLEEP         (1 << 8)
> +#define EXYNOS_4212_UPHYPWR_HOST ( \
> +       EXYNOS_4212_UPHYPWR_HOST_SUSPEND | \
> +       EXYNOS_4212_UPHYPWR_HOST_PWR | \
> +       EXYNOS_4212_UPHYPWR_HOST_SLEEP)
> +
> +#define EXYNOS_4212_UPHYPWR_HSCI0_SUSPEND      (1 << 9)
> +#define EXYNOS_4212_UPHYPWR_HSCI0_PWR          (1 << 10)
> +#define EXYNOS_4212_UPHYPWR_HSCI0_SLEEP                (1 << 11)
> +#define EXYNOS_4212_UPHYPWR_HSCI0 ( \
> +       EXYNOS_4212_UPHYPWR_HSCI0_SUSPEND | \
> +       EXYNOS_4212_UPHYPWR_HSCI0_PWR | \
> +       EXYNOS_4212_UPHYPWR_HSCI0_SLEEP)
> +
> +#define EXYNOS_4212_UPHYPWR_HSCI1_SUSPEND      (1 << 12)
> +#define EXYNOS_4212_UPHYPWR_HSCI1_PWR          (1 << 13)
> +#define EXYNOS_4212_UPHYPWR_HSCI1_SLEEP                (1 << 14)
> +#define EXYNOS_4212_UPHYPWR_HSCI1 ( \
> +       EXYNOS_4212_UPHYPWR_HSCI1_SUSPEND | \
> +       EXYNOS_4212_UPHYPWR_HSCI1_PWR | \
> +       EXYNOS_4212_UPHYPWR_HSCI1_SLEEP)
> +
> +/* PHY clock control */
> +#define EXYNOS_4212_UPHYCLK                    0x4
> +
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_MASK       (0x7 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_9MHZ6      (0x0 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_10MHZ      (0x1 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_12MHZ      (0x2 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_19MHZ2     (0x3 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_20MHZ      (0x4 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_24MHZ      (0x5 << 0)
> +#define EXYNOS_4212_UPHYCLK_PHYFSEL_50MHZ      (0x7 << 0)
> +
> +#define EXYNOS_4212_UPHYCLK_PHY0_ID_PULLUP     (0x1 << 3)
> +#define EXYNOS_4212_UPHYCLK_PHY0_COMMON_ON     (0x1 << 4)
> +#define EXYNOS_4212_UPHYCLK_PHY1_COMMON_ON     (0x1 << 7)
> +
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_MASK   (0x7f << 10)
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_12MHZ  (0x24 << 10)
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_15MHZ  (0x1c << 10)
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_16MHZ  (0x1a << 10)
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_19MHZ2 (0x15 << 10)
> +#define EXYNOS_4212_UPHYCLK_HSIC_REFCLK_20MHZ  (0x14 << 10)
> +
> +/* PHY reset control */
> +#define EXYNOS_4212_UPHYRST                    0x8
> +
> +#define EXYNOS_4212_URSTCON_DEVICE             (1 << 0)
> +#define EXYNOS_4212_URSTCON_OTG_HLINK          (1 << 1)
> +#define EXYNOS_4212_URSTCON_OTG_PHYLINK                (1 << 2)
> +#define EXYNOS_4212_URSTCON_HOST_PHY           (1 << 3)
> +#define EXYNOS_4212_URSTCON_PHY1               (1 << 4)
> +#define EXYNOS_4212_URSTCON_HSIC0              (1 << 5)
> +#define EXYNOS_4212_URSTCON_HSIC1              (1 << 6)
> +#define EXYNOS_4212_URSTCON_HOST_LINK_ALL      (1 << 7)
> +#define EXYNOS_4212_URSTCON_HOST_LINK_P0       (1 << 8)
> +#define EXYNOS_4212_URSTCON_HOST_LINK_P1       (1 << 9)
> +#define EXYNOS_4212_URSTCON_HOST_LINK_P2       (1 << 10)
> +
> +/* Isolation, configured in the power management unit */
> +#define EXYNOS_4212_USB_ISOL_OFFSET            0x0
> +#define EXYNOS_4212_USB_ISOL_OTG               (1 << 0)
> +#define EXYNOS_4212_USB_ISOL_HSIC0_OFFSET      0x4
> +#define EXYNOS_4212_USB_ISOL_HSIC0             (1 << 0)
> +#define EXYNOS_4212_USB_ISOL_HSIC1_OFFSET      0x8
> +#define EXYNOS_4212_USB_ISOL_HSIC1             (1 << 0)
> +
> +enum exynos4x12_phy_id {
> +       EXYNOS4212_DEVICE,
> +       EXYNOS4212_HOST,
> +       EXYNOS4212_HSIC0,
> +       EXYNOS4212_HSIC1,
> +       EXYNOS4212_NUM_PHYS,
> +};
> +
> +/*
> + * exynos4212_rate_to_clk() converts the supplied clock rate to the value that
> + * can be written to the phy register.
> + */
> +static u32 exynos4212_rate_to_clk(unsigned long rate)
> +{
> +       unsigned int clksel;
> +
> +       /* EXYNOS_4212_UPHYCLK_PHYFSEL_MASK */
> +
> +       switch (rate) {
> +       case 9600 * KHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_9MHZ6;
> +               break;
> +       case 10 * MHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_10MHZ;
> +               break;
> +       case 12 * MHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_12MHZ;
> +               break;
> +       case 19200 * KHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_19MHZ2;
> +               break;
> +       case 20 * MHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_20MHZ;
> +               break;
> +       case 24 * MHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_24MHZ;
> +               break;
> +       case 50 * MHZ:
> +               clksel = EXYNOS_4212_UPHYCLK_PHYFSEL_50MHZ;
> +               break;
> +       default:
> +               clksel = CLKSEL_ERROR;
> +       }
> +
> +       return clksel;
> +}
> +
> +static void exynos4212_isol(struct usb2_phy_instance *inst, bool on)
> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +       u32 offset;
> +       u32 mask;
> +       u32 tmp;
> +
> +       if (!drv->reg_isol)
> +               return;
> +
> +       switch (inst->cfg->id) {
> +       case EXYNOS4212_DEVICE:
> +               offset = EXYNOS_4212_USB_ISOL_OFFSET;
> +               mask = EXYNOS_4212_USB_ISOL_OTG;
> +               break;
> +       case EXYNOS4212_HOST:
> +               offset = EXYNOS_4212_USB_ISOL_OFFSET;
> +               mask = EXYNOS_4212_USB_ISOL_OTG;
> +               break;
> +       case EXYNOS4212_HSIC0:
> +               offset = EXYNOS_4212_USB_ISOL_HSIC0_OFFSET;
> +               mask = EXYNOS_4212_USB_ISOL_HSIC0;
> +               break;
> +       case EXYNOS4212_HSIC1:
> +               offset = EXYNOS_4212_USB_ISOL_HSIC1_OFFSET;
> +               mask = EXYNOS_4212_USB_ISOL_HSIC1;
> +               break;
> +       default:
> +               return;
> +       };
> +
> +       tmp = readl(drv->reg_isol + offset);
> +       if (on)
> +               tmp &= ~mask;
> +       else
> +               tmp |= mask;
> +       writel(tmp, drv->reg_isol + offset);
> +}
> +
> +static void exynos4212_phy_pwr(struct usb2_phy_instance *inst, bool on)
> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +       u32 rstbits = 0;
> +       u32 phypwr = 0;
> +       u32 rst;
> +       u32 pwr;
> +
> +       switch (inst->cfg->id) {
> +       case EXYNOS4212_DEVICE:
> +               phypwr =        EXYNOS_4212_UPHYPWR_DEV;
> +               rstbits =       EXYNOS_4212_URSTCON_DEVICE;
> +               break;
> +       case EXYNOS4212_HOST:
> +               phypwr =        EXYNOS_4212_UPHYPWR_HOST;
> +               rstbits =       EXYNOS_4212_URSTCON_HOST_PHY;
> +               break;
> +       case EXYNOS4212_HSIC0:
> +               phypwr =        EXYNOS_4212_UPHYPWR_HSCI0;
> +               rstbits =       EXYNOS_4212_URSTCON_HSIC1 |
> +                               EXYNOS_4212_URSTCON_HOST_LINK_P0 |
> +                               EXYNOS_4212_URSTCON_HOST_PHY;
> +               break;
> +       case EXYNOS4212_HSIC1:
> +               phypwr =        EXYNOS_4212_UPHYPWR_HSCI1;
> +               rstbits =       EXYNOS_4212_URSTCON_HSIC1 |
> +                               EXYNOS_4212_URSTCON_HOST_LINK_P1;
> +               break;
> +       };
> +
> +       if (on) {
> +               writel(inst->clk, drv->reg_phy + EXYNOS_4212_UPHYCLK);
> +
> +               pwr = readl(drv->reg_phy + EXYNOS_4212_UPHYPWR);
> +               pwr &= ~phypwr;
> +               writel(pwr, drv->reg_phy + EXYNOS_4212_UPHYPWR);
> +
> +               rst = readl(drv->reg_phy + EXYNOS_4212_UPHYRST);
> +               rst |= rstbits;
> +               writel(rst, drv->reg_phy + EXYNOS_4212_UPHYRST);
> +               udelay(10);
> +               rst &= ~rstbits;
> +               writel(rst, drv->reg_phy + EXYNOS_4212_UPHYRST);
> +       } else {
> +               pwr = readl(drv->reg_phy + EXYNOS_4212_UPHYPWR);
> +               pwr |= phypwr;
> +               writel(pwr, drv->reg_phy + EXYNOS_4212_UPHYPWR);
> +       }
> +}
> +
> +static int exynos4212_power_on(struct usb2_phy_instance *inst)
> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +
> +       inst->state = STATE_ON;
> +       exynos4212_phy_pwr(inst, 1);
> +       exynos4212_isol(inst, 0);
> +
> +       /* Power on the device, as it is necessary for HSIC to work */
> +       if (inst->cfg->id == EXYNOS4212_HSIC0) {
> +               struct usb2_phy_instance *device =
> +                                       &drv->usb2_phy_instances[EXYNOS4212_DEVICE];
> +               exynos4212_phy_pwr(device, 1);
> +               exynos4212_isol(device, 0);
> +       }
> +
> +       return 0;
> +}
> +
> +static int exynos4212_power_off(struct usb2_phy_instance *inst)
> +{
> +       struct usb2_phy_driver *drv = inst->drv;
> +       struct usb2_phy_instance *device =
> +               &drv->usb2_phy_instances[EXYNOS4212_DEVICE];
> +
> +       inst->state = STATE_OFF;
> +       exynos4212_isol(inst, 1);
> +       exynos4212_phy_pwr(inst, 0);
> +
> +       if (inst->cfg->id == EXYNOS4212_HSIC0 && device->state != STATE_ON) {
> +               exynos4212_isol(device, 1);
> +               exynos4212_phy_pwr(device, 0);
> +       }
> +
> +       return 0;
> +}
> +
> +
> +static const struct common_phy exynos4212_phys[] = {
> +       {
> +               .label          = "device",
> +               .type           = PHY_DEVICE,
> +               .id             = EXYNOS4212_DEVICE,
> +               .rate_to_clk    = exynos4212_rate_to_clk,
> +               .power_on       = exynos4212_power_on,
> +               .power_off      = exynos4212_power_off,
> +       },
> +       {
> +               .label          = "host",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4212_HOST,
> +               .rate_to_clk    = exynos4212_rate_to_clk,
> +               .power_on       = exynos4212_power_on,
> +               .power_off      = exynos4212_power_off,
> +       },
> +       {
> +               .label          = "hsic0",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4212_HSIC0,
> +               .rate_to_clk    = exynos4212_rate_to_clk,
> +               .power_on       = exynos4212_power_on,
> +               .power_off      = exynos4212_power_off,
> +       },
> +       {
> +               .label          = "hsic1",
> +               .type           = PHY_HOST,
> +               .id             = EXYNOS4212_HSIC1,
> +               .rate_to_clk    = exynos4212_rate_to_clk,
> +               .power_on       = exynos4212_power_on,
> +               .power_off      = exynos4212_power_off,
> +       },
> +       {},
> +};
> +
> +const struct usb2_phy_config exynos4212_usb2_phy_config = {
> +       .num_phys               = EXYNOS4212_NUM_PHYS,
> +       .phys                   = exynos4212_phys,
> +       .has_mode_switch        = 1,
> +};
> +
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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 v3 2/3] usb: ehci-s5p: Change to use phy provided by the generic phy framework

[Kamil Debski]

Change the phy provider used from the old usb phy specific to a new one
using the generic phy framework.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/host/ehci-exynos.c |   34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 8898c01..974001b 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -19,12 +19,12 @@
 #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>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ehci.h"
 
@@ -44,8 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
 struct exynos_ehci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
+	struct phy *phy;
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -75,7 +74,8 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 	struct usb_hcd *hcd;
 	struct ehci_hcd *ehci;
 	struct resource *res;
-	struct usb_phy *phy;
+	struct phy *phy;
+	const char *phy_name;
 	int irq;
 	int err;
 
@@ -98,12 +98,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 	exynos_ehci = to_exynos_ehci(hcd);
-
 	if (of_device_is_compatible(pdev->dev.of_node,
 					"samsung,exynos5440-ehci"))
 		goto skip_phy;
 
-	phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
+	phy_name = of_get_property(pdev->dev.of_node, "phy-names", NULL);
+	phy =  devm_phy_get(&pdev->dev, phy_name);
 	if (IS_ERR(phy)) {
 		usb_put_hcd(hcd);
 		dev_warn(&pdev->dev, "no platform data or transceiver defined\n");
@@ -149,11 +149,8 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	if (exynos_ehci->phy)
-		usb_phy_init(exynos_ehci->phy);
+		phy_power_on(exynos_ehci->phy);
 
 	ehci = hcd_to_ehci(hcd);
 	ehci->caps = hcd->regs;
@@ -173,7 +170,7 @@ skip_phy:
 
 fail_add_hcd:
 	if (exynos_ehci->phy)
-		usb_phy_shutdown(exynos_ehci->phy);
+		phy_power_off(exynos_ehci->phy);
 fail_io:
 	clk_disable_unprepare(exynos_ehci->clk);
 fail_clk:
@@ -188,11 +185,8 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	if (exynos_ehci->phy)
-		usb_phy_shutdown(exynos_ehci->phy);
+		phy_power_off(exynos_ehci->phy);
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
@@ -212,11 +206,8 @@ static int exynos_ehci_suspend(struct device *dev)
 
 	rc = ehci_suspend(hcd, do_wakeup);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	if (exynos_ehci->phy)
-		usb_phy_shutdown(exynos_ehci->phy);
+		phy_power_off(exynos_ehci->phy);
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
@@ -230,11 +221,8 @@ static int exynos_ehci_resume(struct device *dev)
 
 	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);
+		phy_power_on(exynos_ehci->phy);
 
 	/* DMA burst Enable */
 	writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
-- 
1.7.9.5

Re: [PATCH v3 2/3] usb: ehci-s5p: Change to use phy provided by the generic phy framework

[Jingoo Han]

On Wednesday, November 06, 2013 1:13 AM, Kamil Debski wrote:
> 
> Change the phy provider used from the old usb phy specific to a new one
> using the generic phy framework.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/usb/host/ehci-exynos.c |   34 +++++++++++-----------------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 8898c01..974001b 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c

[.....]

> @@ -98,12 +98,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  	exynos_ehci = to_exynos_ehci(hcd);
> -
>  	if (of_device_is_compatible(pdev->dev.of_node,
>  					"samsung,exynos5440-ehci"))
>  		goto skip_phy;
> 
> -	phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
> +	phy_name = of_get_property(pdev->dev.of_node, "phy-names", NULL);
> +	phy =  devm_phy_get(&pdev->dev, phy_name);
>  	if (IS_ERR(phy)) {
>  		usb_put_hcd(hcd);
>  		dev_warn(&pdev->dev, "no platform data or transceiver defined\n");

Hi Kamil,

The following build error happens.

drivers/usb/host/ehci-exynos.c: In function 'exynos_ehci_probe'
drivers/usb/host/ehci-exynos.c:113:14: error: 'struct exynos_ehci_hcd' has no member named 'otg'
drivers/usb/host/ehci-exynos.c:113:25: error: 'struct phy' has no member named 'otg'

Would you add the following to this patch?

--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -110,7 +110,6 @@ static int exynos_ehci_probe(struct platform_device *pdev)
                return -EPROBE_DEFER;
        } else {
                exynos_ehci->phy = phy;
-               exynos_ehci->otg = phy->otg;
        }

Best regards,
Jingoo Han

Re: [PATCH v3 2/3] usb: ehci-s5p: Change to use phy provided by the generic phy framework

[Tomasz Figa]

Hi Kamil,

On Tuesday 05 of November 2013 17:13:20 Kamil Debski wrote:
> Change the phy provider used from the old usb phy specific to a new one
> using the generic phy framework.

I believe that until Exynos5250 also gets converted to the new PHY driver,
support for the old USB PHY API should remain in this driver.

As for the patch itself, please see my comments inline.

> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/usb/host/ehci-exynos.c |   34 +++++++++++-----------------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 

This patch is changing a DT binding, but there is no update to the
documentation.

> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 8898c01..974001b 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -19,12 +19,12 @@
>  #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>
>  #include <linux/usb.h>
>  #include <linux/usb/hcd.h>
> -#include <linux/usb/otg.h>
>  
>  #include "ehci.h"
>  
> @@ -44,8 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
>  
>  struct exynos_ehci_hcd {
>  	struct clk *clk;
> -	struct usb_phy *phy;
> -	struct usb_otg *otg;
> +	struct phy *phy;
>  };
>  
>  #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
> @@ -75,7 +74,8 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>  	struct usb_hcd *hcd;
>  	struct ehci_hcd *ehci;
>  	struct resource *res;
> -	struct usb_phy *phy;
> +	struct phy *phy;
> +	const char *phy_name;
>  	int irq;
>  	int err;
>  
> @@ -98,12 +98,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  	exynos_ehci = to_exynos_ehci(hcd);
> -
>  	if (of_device_is_compatible(pdev->dev.of_node,
>  					"samsung,exynos5440-ehci"))
>  		goto skip_phy;
>  
> -	phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
> +	phy_name = of_get_property(pdev->dev.of_node, "phy-names", NULL);
> +	phy =  devm_phy_get(&pdev->dev, phy_name);

This is definitely not the way you should parse PHY DT bindings. PHY names
are supposed to be fixed in the binding for each compatible value, which
means that you should call here devm_phy_get() with a static name.

Moreover, this driver needs one PHY per port, not just one PHY, so the
design needs to be completely changed and this patch is not really enough
to correctly support USB 2.0 on Exynos.

Best regards,
Tomasz

[PATCH v3 3/3] usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic phy framework

[Kamil Debski]

Change the used phy driver to the new Exynos USB phy driver that uses the
generic phy framework.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/s3c-hsotg.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index bb31262..dc7f20c 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -31,6 +31,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/phy/phy.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
@@ -162,7 +163,7 @@ struct s3c_hsotg_ep {
 struct s3c_hsotg {
 	struct device		 *dev;
 	struct usb_gadget_driver *driver;
-	struct usb_phy		*phy;
+	struct phy		 *phy;
 	struct s3c_hsotg_plat	 *plat;
 
 	spinlock_t              lock;
@@ -2905,9 +2906,10 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
 	dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
 
 	if (hsotg->phy)
-		usb_phy_init(hsotg->phy);
+		phy_power_on(hsotg->phy);
 	else if (hsotg->plat->phy_init)
 		hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
+
 }
 
 /**
@@ -2922,7 +2924,7 @@ static void s3c_hsotg_phy_disable(struct s3c_hsotg *hsotg)
 	struct platform_device *pdev = to_platform_device(hsotg->dev);
 
 	if (hsotg->phy)
-		usb_phy_shutdown(hsotg->phy);
+		phy_power_off(hsotg->phy);
 	else if (hsotg->plat->phy_exit)
 		hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
 }
@@ -3529,7 +3531,7 @@ static void s3c_hsotg_delete_debug(struct s3c_hsotg *hsotg)
 static int s3c_hsotg_probe(struct platform_device *pdev)
 {
 	struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
-	struct usb_phy *phy;
+	struct phy *phy;
 	struct device *dev = &pdev->dev;
 	struct s3c_hsotg_ep *eps;
 	struct s3c_hsotg *hsotg;
@@ -3544,7 +3546,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+	phy = devm_phy_get(&pdev->dev, "device");
 	if (IS_ERR(phy)) {
 		/* Fallback for pdata */
 		plat = dev_get_platdata(&pdev->dev);
-- 
1.7.9.5

Re: [PATCH v3 3/3] usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic phy framework

[Tomasz Figa]

Hi Kamil,

This patch is changing a DT binding, but you haven't updated relevant
documentation. Please remember about it in next version.

On Tuesday 05 of November 2013 17:13:21 Kamil Debski wrote:
> Change the used phy driver to the new Exynos USB phy driver that uses the
> generic phy framework.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index bb31262..dc7f20c 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -31,6 +31,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> @@ -162,7 +163,7 @@ struct s3c_hsotg_ep {
>  struct s3c_hsotg {
>  	struct device		 *dev;
>  	struct usb_gadget_driver *driver;
> -	struct usb_phy		*phy;
> +	struct phy		 *phy;
>  	struct s3c_hsotg_plat	 *plat;
>  
>  	spinlock_t              lock;
> @@ -2905,9 +2906,10 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
>  	dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
>  
>  	if (hsotg->phy)
> -		usb_phy_init(hsotg->phy);
> +		phy_power_on(hsotg->phy);
>  	else if (hsotg->plat->phy_init)
>  		hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
> +

nit: Stray blank line.

Best regards,
Tomasz

Re: [PATCH v3 3/3] usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic phy framework

[Matt Porter]

On Tue, Nov 05, 2013 at 05:13:21PM +0100, Kamil Debski wrote:
> Change the used phy driver to the new Exynos USB phy driver that uses the
> generic phy framework.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index bb31262..dc7f20c 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -31,6 +31,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> @@ -162,7 +163,7 @@ struct s3c_hsotg_ep {
>  struct s3c_hsotg {
>  	struct device		 *dev;
>  	struct usb_gadget_driver *driver;
> -	struct usb_phy		*phy;
> +	struct phy		 *phy;
>  	struct s3c_hsotg_plat	 *plat;
>  
>  	spinlock_t              lock;
> @@ -2905,9 +2906,10 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg)
>  	dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
>  
>  	if (hsotg->phy)
> -		usb_phy_init(hsotg->phy);
> +		phy_power_on(hsotg->phy);
>  	else if (hsotg->plat->phy_init)
>  		hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
> +

Stray whitespace

>  }
>  
>  /**
> @@ -2922,7 +2924,7 @@ static void s3c_hsotg_phy_disable(struct s3c_hsotg *hsotg)
>  	struct platform_device *pdev = to_platform_device(hsotg->dev);
>  
>  	if (hsotg->phy)
> -		usb_phy_shutdown(hsotg->phy);
> +		phy_power_off(hsotg->phy);
>  	else if (hsotg->plat->phy_exit)
>  		hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
>  }
> @@ -3529,7 +3531,7 @@ static void s3c_hsotg_delete_debug(struct s3c_hsotg *hsotg)
>  static int s3c_hsotg_probe(struct platform_device *pdev)
>  {
>  	struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
> -	struct usb_phy *phy;
> +	struct phy *phy;
>  	struct device *dev = &pdev->dev;
>  	struct s3c_hsotg_ep *eps;
>  	struct s3c_hsotg *hsotg;
> @@ -3544,7 +3546,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> -	phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +	phy = devm_phy_get(&pdev->dev, "device");

A more descriptive string would be useful here IMHO. Since this is USB
2.0 IP, perhaps "usb2-phy" like musb uses.

	phy-names = "usb2-phy";

looks far more unique than

	phy-names = "device";

in the dts.

Also, this has impact on the samsung-hsotg.txt binding. It should be
updated to reflect the required generic phy properties that will be
used when booting from DT:

	- phys: phy provider specifier
	- phy-names: shall be "device" 

[or "usb2-phy" if the above suggestion is adopted]

I've rebased my BCM281xx UDC series [1] against this s3c-hsotg generic
phy support and it's working fine. I do add phy_init/phy_exit support
which is probably better coming in with my series as my phy driver
makes use of the init method.

Tested-by: Matt Porter <matt.porter@linaro.org>

-Matt

[1] http://www.kernelhub.org/?msg=367354&p=2