Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.

However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/phy/Kconfig         |    1 +
 drivers/usb/phy/phy-omap-usb2.c |   45 +++++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..cc55993 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
 config OMAP_USB2
 	tristate "OMAP USB2 PHY Driver"
 	depends on ARCH_OMAP2PLUS
+	depends on GENERIC_PHY
 	select OMAP_CONTROL_USB
 	help
 	  Enable this to support the transceiver that is part of SOC. This
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..751b30f 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
 #include <linux/usb/omap_control_usb.h>
+#include <linux/phy/phy.h>
 
 /**
  * omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static int omap_usb_power_off(struct phy *x)
+{
+	struct omap_usb *phy = phy_get_drvdata(x);
+
+	omap_control_usb_phy_power(phy->control_dev, 0);
+
+	return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+	struct omap_usb *phy = phy_get_drvdata(x);
+
+	omap_control_usb_phy_power(phy->control_dev, 1);
+
+	return 0;
+}
+
+static struct phy_ops ops = {
+	.power_on	= omap_usb_power_on,
+	.power_off	= omap_usb_power_off,
+	.owner		= THIS_MODULE,
+};
+
 static int omap_usb2_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
+	struct phy			*generic_phy;
 	struct usb_otg			*otg;
+	struct phy_provider		*phy_provider;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
+	phy_provider = devm_of_phy_provider_register(phy->dev,
+			of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
 	phy->control_dev = omap_get_control_dev();
 	if (IS_ERR(phy->control_dev)) {
 		dev_dbg(&pdev->dev, "Failed to get control device\n");
@@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	otg->start_srp		= omap_usb_start_srp;
 	otg->phy		= &phy->phy;
 
+	platform_set_drvdata(pdev, phy);
+	pm_runtime_enable(phy->dev);
+
+	generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2");
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, phy);
+
 	phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
 	if (IS_ERR(phy->wkupclk)) {
 		dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
@@ -174,10 +215,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	usb_add_phy_dev(&phy->phy);
 
-	platform_set_drvdata(pdev, phy);
-
-	pm_runtime_enable(phy->dev);
-
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 03/15] usb: phy: twl4030: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Used the generic PHY framework API to create the PHY. For powering on
and powering off the PHY, power_on and power_off ops are used. Once the
MUSB OMAP glue is adapted to the new framework, the suspend and resume
ops of usb phy library will be removed.

However using the old usb phy library cannot be completely removed
because otg is intertwined with phy and moving to the new
framework completely will break otg. Once we have a separate otg state machine,
we can get rid of the usb phy library.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/usb/phy/phy-twl4030-usb.c |   50 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index 8f78d2d..9051756 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -33,6 +33,7 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/usb/otg.h>
+#include <linux/phy/phy.h>
 #include <linux/usb/musb-omap.h>
 #include <linux/usb/ulpi.h>
 #include <linux/i2c/twl.h>
@@ -431,6 +432,14 @@ static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
 	dev_dbg(twl->dev, "%s\n", __func__);
 }
 
+static int twl4030_phy_power_off(struct phy *phy)
+{
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+	twl4030_phy_suspend(twl, 0);
+	return 0;
+}
+
 static void __twl4030_phy_resume(struct twl4030_usb *twl)
 {
 	twl4030_phy_power(twl, 1);
@@ -459,6 +468,14 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
 	}
 }
 
+static int twl4030_phy_power_on(struct phy *phy)
+{
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+	twl4030_phy_resume(twl);
+	return 0;
+}
+
 static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
 {
 	/* Enable writing to power configuration registers */
@@ -602,13 +619,22 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
 	status = twl4030_usb_linkstat(twl);
 	twl->linkstat = status;
 
-	if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID)
+	if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID) {
 		omap_musb_mailbox(twl->linkstat);
+		twl4030_phy_resume(twl);
+	}
 
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
 	return 0;
 }
 
+static int twl4030_phy_init(struct phy *phy)
+{
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+	return twl4030_usb_phy_init(&twl->phy);
+}
+
 static int twl4030_set_suspend(struct usb_phy *x, int suspend)
 {
 	struct twl4030_usb *twl = phy_to_twl(x);
@@ -646,13 +672,22 @@ static int twl4030_set_host(struct usb_otg *otg, struct usb_bus *host)
 	return 0;
 }
 
+static const struct phy_ops ops = {
+	.init		= twl4030_phy_init,
+	.power_on	= twl4030_phy_power_on,
+	.power_off	= twl4030_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
 static int twl4030_usb_probe(struct platform_device *pdev)
 {
 	struct twl4030_usb_data *pdata = pdev->dev.platform_data;
 	struct twl4030_usb	*twl;
+	struct phy		*phy;
 	int			status, err;
 	struct usb_otg		*otg;
 	struct device_node	*np = pdev->dev.of_node;
+	struct phy_provider	*phy_provider;
 
 	twl = devm_kzalloc(&pdev->dev, sizeof *twl, GFP_KERNEL);
 	if (!twl)
@@ -689,6 +724,19 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	otg->set_host		= twl4030_set_host;
 	otg->set_peripheral	= twl4030_set_peripheral;
 
+	phy_provider = devm_of_phy_provider_register(twl->dev,
+		of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(twl->dev, 0, &ops, "twl4030");
+	if (IS_ERR(phy)) {
+		dev_dbg(&pdev->dev, "Failed to create PHY\n");
+		return PTR_ERR(phy);
+	}
+
+	phy_set_drvdata(phy, twl);
+
 	/* init spinlock for workqueue */
 	spin_lock_init(&twl->lock);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 04/15] ARM: OMAP: USB: Add phy binding information
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

In order for controllers to get PHY in case of non dt boot, the phy
binding information (phy device name) should be added in the platform
data of the controller.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/usb-musb.c |    3 +++
 include/linux/usb/musb.h       |    3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 8c4de27..6aa7cbf 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -85,6 +85,9 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
 	musb_plat.mode = board_data->mode;
 	musb_plat.extvbus = board_data->extvbus;
 
+	if (cpu_is_omap34xx())
+		musb_plat.phy_label = "twl4030";
+
 	if (soc_is_am35xx()) {
 		oh_name = "am35x_otg_hs";
 		name = "musb-am35x";
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..596f8c8 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -104,6 +104,9 @@ struct musb_hdrc_platform_data {
 	/* for clk_get() */
 	const char	*clock;
 
+	/* phy label */
+	const char	*phy_label;
+
 	/* (HOST or OTG) switch VBUS on/off */
 	int		(*set_vbus)(struct device *dev, int is_on);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 05/15] ARM: dts: omap: update usb_otg_hs data
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
binding in order for the driver to use the new generic PHY framework.
Also updated the Documentation to include the binding information.
The PHY binding information can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 Documentation/devicetree/bindings/usb/omap-usb.txt |    5 +++++
 Documentation/devicetree/bindings/usb/usb-phy.txt  |    6 ++++++
 arch/arm/boot/dts/omap3-beagle-xm.dts              |    2 ++
 arch/arm/boot/dts/omap3-evm.dts                    |    2 ++
 arch/arm/boot/dts/omap3-overo.dtsi                 |    2 ++
 arch/arm/boot/dts/omap4.dtsi                       |    3 +++
 arch/arm/boot/dts/twl4030.dtsi                     |    1 +
 7 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..825790d 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -19,6 +19,9 @@ OMAP MUSB GLUE
  - power : Should be "50". This signifies the controller can supply up to
    100mA when operating in host mode.
  - usb-phy : the phandle for the PHY device
+ - phys : the phandle for the PHY device (used by generic PHY framework)
+ - phy-names : the names of the PHY corresponding to the PHYs present in the
+   *phy* phandle.
 
 Optional properties:
  - ctrl-module : phandle of the control module this glue uses to write to
@@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
 	num-eps = <16>;
 	ram-bits = <12>;
 	ctrl-module = <&omap_control_usb>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 };
 
 Board specific device node entry
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 61496f5..c0245c8 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -5,6 +5,8 @@ OMAP USB2 PHY
 Required properties:
  - compatible: Should be "ti,omap-usb2"
  - reg : Address and length of the register set for the device.
+ - #phy-cells: determine the number of cells that should be given in the
+   phandle while referencing this phy.
 
 Optional properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
@@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
 	compatible = "ti,omap-usb2";
 	reg = <0x4a0ad080 0x58>;
 	ctrl-module = <&omap_control_usb>;
+	#phy-cells = <0>;
 };
 
 OMAP USB3 PHY
@@ -25,6 +28,8 @@ Required properties:
  - reg : Address and length of the register set for the device.
  - reg-names: The names of the register addresses corresponding to the registers
    filled in "reg".
+ - #phy-cells: determine the number of cells that should be given in the
+   phandle while referencing this phy.
 
 Optional properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
@@ -39,4 +44,5 @@ usb3phy@4a084400 {
 	      <0x4a084c00 0x40>;
 	reg-names = "phy_rx", "phy_tx", "pll_ctrl";
 	ctrl-module = <&omap_control_usb>;
+	#phy-cells = <0>;
 };
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index afdb164..533b2da 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -144,6 +144,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 7d4329d..4134dd0 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -70,6 +70,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index 8f1abec..a461d2f 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -76,6 +76,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..1e8e2fe 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -520,6 +520,7 @@
 				compatible = "ti,omap-usb2";
 				reg = <0x4a0ad080 0x58>;
 				ctrl-module = <&omap_control_usb>;
+				#phy-cells = <0>;
 			};
 		};
 
@@ -658,6 +659,8 @@
 			interrupt-names = "mc", "dma";
 			ti,hwmods = "usb_otg_hs";
 			usb-phy = <&usb2_phy>;
+			phys = <&usb2_phy>;
+			phy-names = "usb2-phy";
 			multipoint = <1>;
 			num-eps = <16>;
 			ram-bits = <12>;
diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
index b3034da..ce4cd6f 100644
--- a/arch/arm/boot/dts/twl4030.dtsi
+++ b/arch/arm/boot/dts/twl4030.dtsi
@@ -80,6 +80,7 @@
 		usb1v8-supply = <&vusb1v8>;
 		usb3v1-supply = <&vusb3v1>;
 		usb_mode = <1>;
+		#phy-cells = <0>;
 	};
 
 	twl_pwm: pwm {
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 06/15] usb: musb: omap2430: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on and
power_off to align with the new PHY framework.

musb->xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/musb/Kconfig     |    1 +
 drivers/usb/musb/musb_core.c |    1 +
 drivers/usb/musb/musb_core.h |    3 +++
 drivers/usb/musb/omap2430.c  |   26 ++++++++++++++++++++------
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 797e3fd..01381ac 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,6 +76,7 @@ config USB_MUSB_TUSB6010
 config USB_MUSB_OMAP2PLUS
 	tristate "OMAP2430 and onwards"
 	depends on ARCH_OMAP2PLUS
+	depends on GENERIC_PHY
 
 config USB_MUSB_AM35X
 	tristate "AM35x"
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 29a24ce..cca12c0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1814,6 +1814,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb->min_power = plat->min_power;
 	musb->ops = plat->platform_ops;
 	musb->port_mode = plat->mode;
+	musb->phy_label = plat->phy_label;
 
 	/* The musb_platform_init() call:
 	 *   - adjusts musb->mregs
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7d341c3..8f017ab 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
 #include <linux/usb.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/musb.h>
+#include <linux/phy/phy.h>
 
 struct musb;
 struct musb_hw_ep;
@@ -346,6 +347,7 @@ struct musb {
 	u16			int_tx;
 
 	struct usb_phy		*xceiv;
+	struct phy		*phy;
 
 	int nIrq;
 	unsigned		irq_wake:1;
@@ -424,6 +426,7 @@ struct musb {
 	unsigned                double_buffer_not_ok:1;
 
 	struct musb_hdrc_config	*config;
+	const char		*phy_label;
 
 #ifdef MUSB_CONFIG_PROC_FS
 	struct proc_dir_entry *proc_entry;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 6708a3b..87dac0f 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,11 +348,21 @@ static int omap2430_musb_init(struct musb *musb)
 	 * up through ULPI.  TWL4030-family PMICs include one,
 	 * which needs a driver, drivers aren't always needed.
 	 */
-	if (dev->parent->of_node)
+	if (dev->parent->of_node) {
+		musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
+		/* We can't totally remove musb->xceiv as of now because
+		 * musb core uses xceiv.state and xceiv.otg. Once we have
+		 * a separate state machine to handle otg, these can be moved
+		 * out of xceiv and then we can start using the generic PHY
+		 * framework
+		 */
 		musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
 		    "usb-phy", 0);
-	else
+	} else {
 		musb->xceiv = devm_usb_get_phy_dev(dev, 0);
+		musb->phy = devm_phy_get(dev, musb->phy_label);
+	}
 
 	if (IS_ERR(musb->xceiv)) {
 		status = PTR_ERR(musb->xceiv);
@@ -364,6 +374,10 @@ static int omap2430_musb_init(struct musb *musb)
 		return -EPROBE_DEFER;
 	}
 
+	if (IS_ERR(musb->phy)) {
+		pr_err("HS USB OTG: no PHY configured\n");
+		return PTR_ERR(musb->phy);
+	}
 	musb->isr = omap2430_musb_interrupt;
 
 	status = pm_runtime_get_sync(dev);
@@ -397,7 +411,7 @@ static int omap2430_musb_init(struct musb *musb)
 	if (glue->status != OMAP_MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
-	usb_phy_init(musb->xceiv);
+	phy_init(musb->phy);
 
 	pm_runtime_put_noidle(musb->controller);
 	return 0;
@@ -460,6 +474,7 @@ static int omap2430_musb_exit(struct musb *musb)
 	del_timer_sync(&musb_idle_timer);
 
 	omap2430_low_level_exit(musb);
+	phy_exit(musb->phy);
 
 	return 0;
 }
@@ -633,7 +648,7 @@ static int omap2430_runtime_suspend(struct device *dev)
 				OTG_INTERFSEL);
 
 		omap2430_low_level_exit(musb);
-		usb_phy_set_suspend(musb->xceiv, 1);
+		phy_power_off(musb->phy);
 	}
 
 	return 0;
@@ -648,8 +663,7 @@ static int omap2430_runtime_resume(struct device *dev)
 		omap2430_low_level_init(musb);
 		musb_writel(musb->mregs, OTG_INTERFSEL,
 				musb->context.otg_interfsel);
-
-		usb_phy_set_suspend(musb->xceiv, 0);
+		phy_power_on(musb->phy);
 	}
 
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 07/15] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Now that omap-usb2 is adapted to the new generic PHY framework,
*set_suspend* ops can be removed from omap-usb2 driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/usb/phy/phy-omap-usb2.c |   25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 751b30f..3f2b125 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -97,29 +97,6 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
 	return 0;
 }
 
-static int omap_usb2_suspend(struct usb_phy *x, int suspend)
-{
-	u32 ret;
-	struct omap_usb *phy = phy_to_omapusb(x);
-
-	if (suspend && !phy->is_suspended) {
-		omap_control_usb_phy_power(phy->control_dev, 0);
-		pm_runtime_put_sync(phy->dev);
-		phy->is_suspended = 1;
-	} else if (!suspend && phy->is_suspended) {
-		ret = pm_runtime_get_sync(phy->dev);
-		if (ret < 0) {
-			dev_err(phy->dev, "get_sync failed with err %d\n",
-									ret);
-			return ret;
-		}
-		omap_control_usb_phy_power(phy->control_dev, 1);
-		phy->is_suspended = 0;
-	}
-
-	return 0;
-}
-
 static int omap_usb_power_off(struct phy *x)
 {
 	struct omap_usb *phy = phy_get_drvdata(x);
@@ -167,7 +144,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	phy->phy.dev		= phy->dev;
 	phy->phy.label		= "omap-usb2";
-	phy->phy.set_suspend	= omap_usb2_suspend;
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
@@ -182,7 +158,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	phy->is_suspended	= 1;
 	omap_control_usb_phy_power(phy->control_dev, 0);
 
 	otg->set_host		= omap_usb_set_host;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 08/15] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

Now that twl4030-usb is adapted to the new generic PHY framework,
*set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/usb/phy/phy-twl4030-usb.c |   57 +++++++++----------------------------
 1 file changed, 13 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index 9051756..44f8b1b 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -422,25 +422,20 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 	}
 }
 
-static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
+static int twl4030_phy_power_off(struct phy *phy)
 {
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
+
 	if (twl->asleep)
-		return;
+		return 0;
 
 	twl4030_phy_power(twl, 0);
 	twl->asleep = 1;
 	dev_dbg(twl->dev, "%s\n", __func__);
-}
-
-static int twl4030_phy_power_off(struct phy *phy)
-{
-	struct twl4030_usb *twl = phy_get_drvdata(phy);
-
-	twl4030_phy_suspend(twl, 0);
 	return 0;
 }
 
-static void __twl4030_phy_resume(struct twl4030_usb *twl)
+static void __twl4030_phy_power_on(struct twl4030_usb *twl)
 {
 	twl4030_phy_power(twl, 1);
 	twl4030_i2c_access(twl, 1);
@@ -449,11 +444,13 @@ static void __twl4030_phy_resume(struct twl4030_usb *twl)
 		twl4030_i2c_access(twl, 0);
 }
 
-static void twl4030_phy_resume(struct twl4030_usb *twl)
+static int twl4030_phy_power_on(struct phy *phy)
 {
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
+
 	if (!twl->asleep)
-		return;
-	__twl4030_phy_resume(twl);
+		return 0;
+	__twl4030_phy_power_on(twl);
 	twl->asleep = 0;
 	dev_dbg(twl->dev, "%s\n", __func__);
 
@@ -466,13 +463,6 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
 		cancel_delayed_work(&twl->id_workaround_work);
 		schedule_delayed_work(&twl->id_workaround_work, HZ);
 	}
-}
-
-static int twl4030_phy_power_on(struct phy *phy)
-{
-	struct twl4030_usb *twl = phy_get_drvdata(phy);
-
-	twl4030_phy_resume(twl);
 	return 0;
 }
 
@@ -604,9 +594,9 @@ static void twl4030_id_workaround_work(struct work_struct *work)
 	}
 }
 
-static int twl4030_usb_phy_init(struct usb_phy *phy)
+static int twl4030_phy_init(struct phy *phy)
 {
-	struct twl4030_usb *twl = phy_to_twl(phy);
+	struct twl4030_usb *twl = phy_get_drvdata(phy);
 	enum omap_musb_vbus_id_status status;
 
 	/*
@@ -621,32 +611,13 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
 
 	if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID) {
 		omap_musb_mailbox(twl->linkstat);
-		twl4030_phy_resume(twl);
+		twl4030_phy_power_on(phy);
 	}
 
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
 	return 0;
 }
 
-static int twl4030_phy_init(struct phy *phy)
-{
-	struct twl4030_usb *twl = phy_get_drvdata(phy);
-
-	return twl4030_usb_phy_init(&twl->phy);
-}
-
-static int twl4030_set_suspend(struct usb_phy *x, int suspend)
-{
-	struct twl4030_usb *twl = phy_to_twl(x);
-
-	if (suspend)
-		twl4030_phy_suspend(twl, 1);
-	else
-		twl4030_phy_resume(twl);
-
-	return 0;
-}
-
 static int twl4030_set_peripheral(struct usb_otg *otg,
 					struct usb_gadget *gadget)
 {
@@ -717,8 +688,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	twl->phy.label		= "twl4030";
 	twl->phy.otg		= otg;
 	twl->phy.type		= USB_PHY_TYPE_USB2;
-	twl->phy.set_suspend	= twl4030_set_suspend;
-	twl->phy.init		= twl4030_usb_phy_init;
 
 	otg->phy		= &twl->phy;
 	otg->set_host		= twl4030_set_host;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 09/15] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Add a PHY provider driver for the Samsung S5P/Exynos SoC MIPI CSI-2
receiver and MIPI DSI transmitter DPHYs.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 .../devicetree/bindings/phy/samsung-phy.txt        |   14 ++
 drivers/phy/Kconfig                                |    9 ++
 drivers/phy/Makefile                               |    3 +-
 drivers/phy/phy-exynos-mipi-video.c                |  169 ++++++++++++++++++++
 4 files changed, 194 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
 create mode 100644 drivers/phy/phy-exynos-mipi-video.c

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
new file mode 100644
index 0000000..5ff208c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -0,0 +1,14 @@
+Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,s5pv210-mipi-video-phy";
+- reg : offset and length of the MIPI DPHY register set;
+- #phy-cells : from the generic phy bindings, must be 1;
+
+For "samsung,s5pv210-mipi-video-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and its meaning is as follows:
+  0 - MIPI CSIS 0,
+  1 - MIPI DSIM 0,
+  2 - MIPI CSIS 1,
+  3 - MIPI DSIM 1.
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..6f446d0 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,12 @@ menuconfig GENERIC_PHY
 	  devices present in the kernel. This layer will have the generic
 	  API by which phy drivers can create PHY using the phy framework and
 	  phy users can obtain reference to the PHY.
+
+if GENERIC_PHY
+
+config PHY_EXYNOS_MIPI_VIDEO
+	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
+	help
+	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
+	  S5P and EXYNOS SoCs.
+endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..71d8841 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -2,4 +2,5 @@
 # Makefile for the phy drivers.
 #
 
-obj-$(CONFIG_GENERIC_PHY)	+= phy-core.o
+obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
new file mode 100644
index 0000000..7e7fcd7
--- /dev/null
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -0,0 +1,169 @@
+/*
+ * Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@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/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>
+
+/* MIPI_PHYn_CONTROL register offset: n = 0..1 */
+#define EXYNOS_MIPI_PHY_CONTROL(n)	((n) * 4)
+#define EXYNOS_MIPI_PHY_ENABLE		(1 << 0)
+#define EXYNOS_MIPI_PHY_SRESETN		(1 << 1)
+#define EXYNOS_MIPI_PHY_MRESETN		(1 << 2)
+#define EXYNOS_MIPI_PHY_RESET_MASK	(3 << 1)
+
+enum exynos_mipi_phy_id {
+	EXYNOS_MIPI_PHY_ID_CSIS0,
+	EXYNOS_MIPI_PHY_ID_DSIM0,
+	EXYNOS_MIPI_PHY_ID_CSIS1,
+	EXYNOS_MIPI_PHY_ID_DSIM1,
+	EXYNOS_MIPI_PHYS_NUM
+};
+
+#define IS_EXYNOS_MIPI_DSIM_PHY_ID(id) \
+	((id) = EXYNOS_MIPI_PHY_ID_DSIM0 || (id) = EXYNOS_MIPI_PHY_ID_DSIM0)
+
+struct exynos_mipi_video_phy {
+	spinlock_t slock;
+	struct phy *phys[EXYNOS_MIPI_PHYS_NUM];
+	void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_mipi_video_phy *state,
+			enum exynos_mipi_phy_id id, unsigned int on)
+{
+	void __iomem *addr;
+	u32 reg, reset;
+
+	addr = state->regs + EXYNOS_MIPI_PHY_CONTROL(id / 2);
+
+	if (IS_EXYNOS_MIPI_DSIM_PHY_ID(id))
+		reset = EXYNOS_MIPI_PHY_MRESETN;
+	else
+		reset = EXYNOS_MIPI_PHY_SRESETN;
+
+	spin_lock(&state->slock);
+	reg = readl(addr);
+	if (on)
+		reg |= reset;
+	else
+		reg &= ~reset;
+	writel(reg, addr);
+
+	/* Clear ENABLE bit only if MRESETN, SRESETN bits are not set. */
+	if (on)
+		reg |= EXYNOS_MIPI_PHY_ENABLE;
+	else if (!(reg & EXYNOS_MIPI_PHY_RESET_MASK))
+		reg &= ~EXYNOS_MIPI_PHY_ENABLE;
+
+	writel(reg, addr);
+	spin_unlock(&state->slock);
+	return 0;
+}
+
+static int exynos_mipi_video_phy_power_on(struct phy *phy)
+{
+	struct exynos_mipi_video_phy *state = phy_get_drvdata(phy);
+	return __set_phy_state(state, phy->id, 1);
+}
+
+static int exynos_mipi_video_phy_power_off(struct phy *phy)
+{
+	struct exynos_mipi_video_phy *state = phy_get_drvdata(phy);
+	return __set_phy_state(state, phy->id, 0);
+}
+
+static struct phy *exynos_mipi_video_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct exynos_mipi_video_phy *state = dev_get_drvdata(dev);
+
+	if (WARN_ON(args->args[0] > EXYNOS_MIPI_PHYS_NUM))
+		return ERR_PTR(-ENODEV);
+
+	return state->phys[args->args[0]];
+}
+
+static struct phy_ops exynos_mipi_video_phy_ops = {
+	.power_on	= exynos_mipi_video_phy_power_on,
+	.power_off	= exynos_mipi_video_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
+{
+	struct exynos_mipi_video_phy *state;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	int i;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	state->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(state->regs))
+		return PTR_ERR(state->regs);
+
+	dev_set_drvdata(dev, state);
+	spin_lock_init(&state->slock);
+
+	phy_provider = devm_of_phy_provider_register(dev,
+					exynos_mipi_video_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	for (i = 0; i < EXYNOS_MIPI_PHYS_NUM; i++) {
+		char label[8];
+
+		snprintf(label, sizeof(label), "%s.%d",
+				IS_EXYNOS_MIPI_DSIM_PHY_ID(i) ?
+				"dsim" : "csis", i / 2);
+
+		state->phys[i] = devm_phy_create(dev, i,
+				&exynos_mipi_video_phy_ops, label);
+		if (IS_ERR(state->phys[i])) {
+			dev_err(dev, "failed to create PHY %s\n", label);
+			return PTR_ERR(state->phys[i]);
+		}
+		phy_set_drvdata(state->phys[i], state);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id exynos_mipi_video_phy_of_match[] = {
+	{ .compatible = "samsung,s5pv210-mipi-video-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_mipi_video_phy_of_match);
+
+static struct platform_driver exynos_mipi_video_phy_driver = {
+	.probe	= exynos_mipi_video_phy_probe,
+	.driver = {
+		.of_match_table	= exynos_mipi_video_phy_of_match,
+		.name  = "exynos-mipi-video-phy",
+		.owner = THIS_MODULE,
+	}
+};
+module_platform_driver(exynos_mipi_video_phy_driver);
+
+MODULE_DESCRIPTION("Samsung S5P/EXYNOS SoC MIPI CSI-2/DSI PHY driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 10/15] video: exynos_mipi_dsim: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Use the generic PHY API instead of the platform callback to control
the MIPI DSIM DPHY. The 'phy_label' field is added to the platform
data structure to allow PHY lookup on non-dt platforms.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/video/exynos/exynos_mipi_dsi.c |   19 ++++++++++---------
 include/video/exynos_mipi_dsim.h       |    6 ++++--
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 32e5406..248e444 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -30,6 +30,7 @@
 #include <linux/interrupt.h>
 #include <linux/kthread.h>
 #include <linux/notifier.h>
+#include <linux/phy/phy.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
 #include <linux/err.h>
@@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
 		exynos_mipi_regulator_enable(dsim);
 
 		/* enable MIPI-DSI PHY. */
-		if (dsim->pd->phy_enable)
-			dsim->pd->phy_enable(pdev, true);
+		phy_power_on(dsim->phy);
 
 		clk_enable(dsim->clock);
 
@@ -373,6 +373,10 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dsim->phy = devm_phy_get(&pdev->dev, dsim_pd->phy_label);
+	if (IS_ERR(dsim->phy))
+		return PTR_ERR(dsim->phy);
+
 	dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
 	if (IS_ERR(dsim->clock)) {
 		dev_err(&pdev->dev, "failed to get dsim clock source\n");
@@ -439,8 +443,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	exynos_mipi_regulator_enable(dsim);
 
 	/* enable MIPI-DSI PHY. */
-	if (dsim->pd->phy_enable)
-		dsim->pd->phy_enable(pdev, true);
+	phy_power_on(dsim->phy);
 
 	exynos_mipi_update_cfg(dsim);
 
@@ -504,9 +507,8 @@ static int exynos_mipi_dsi_suspend(struct device *dev)
 	if (client_drv && client_drv->suspend)
 		client_drv->suspend(client_dev);
 
-	/* enable MIPI-DSI PHY. */
-	if (dsim->pd->phy_enable)
-		dsim->pd->phy_enable(pdev, false);
+	/* disable MIPI-DSI PHY. */
+	phy_power_off(dsim->phy);
 
 	clk_disable(dsim->clock);
 
@@ -536,8 +538,7 @@ static int exynos_mipi_dsi_resume(struct device *dev)
 	exynos_mipi_regulator_enable(dsim);
 
 	/* enable MIPI-DSI PHY. */
-	if (dsim->pd->phy_enable)
-		dsim->pd->phy_enable(pdev, true);
+	phy_power_on(dsim->phy);
 
 	clk_enable(dsim->clock);
 
diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
index 89dc88a..0e7e43b 100644
--- a/include/video/exynos_mipi_dsim.h
+++ b/include/video/exynos_mipi_dsim.h
@@ -216,6 +216,7 @@ struct mipi_dsim_config {
  *	automatically.
  * @e_clk_src: select byte clock source.
  * @pd: pointer to MIPI-DSI driver platform data.
+ * @phy: pointer to the generic PHY
  */
 struct mipi_dsim_device {
 	struct device			*dev;
@@ -236,6 +237,7 @@ struct mipi_dsim_device {
 	bool				suspended;
 
 	struct mipi_dsim_platform_data	*pd;
+	struct phy			*phy;
 };
 
 /*
@@ -248,7 +250,7 @@ struct mipi_dsim_device {
  * @enabled: indicate whether mipi controller got enabled or not.
  * @lcd_panel_info: pointer for lcd panel specific structure.
  *	this structure specifies width, height, timing and polarity and so on.
- * @phy_enable: pointer to a callback controlling D-PHY enable/reset
+ * @phy_label: the generic PHY label
  */
 struct mipi_dsim_platform_data {
 	char				lcd_panel_name[PANEL_NAME_SIZE];
@@ -257,7 +259,7 @@ struct mipi_dsim_platform_data {
 	unsigned int			enabled;
 	void				*lcd_panel_info;
 
-	int (*phy_enable)(struct platform_device *pdev, bool on);
+	const char			*phy_label;
 };
 
 /*
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 11/15] exynos4-is: Use the generic MIPI CSIS PHY driver
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Use the generic PHY API instead of the platform callback to control
the MIPI CSIS DPHY. The 'phy_label' field is added to the platform
data structure to allow PHY lookup on non-dt platforms

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/media/platform/exynos4-is/mipi-csis.c |   16 +++++++++++++---
 include/linux/platform_data/mipi-csis.h       |   11 ++---------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index 0914230..94028ce 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -20,6 +20,7 @@
 #include <linux/memory.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_data/mipi-csis.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -180,6 +181,7 @@ struct csis_drvdata {
  * @sd: v4l2_subdev associated with CSIS device instance
  * @index: the hardware instance index
  * @pdev: CSIS platform device
+ * @phy: pointer to the CSIS generic PHY
  * @regs: mmaped I/O registers memory
  * @supplies: CSIS regulator supplies
  * @clock: CSIS clocks
@@ -203,6 +205,8 @@ struct csis_state {
 	struct v4l2_subdev sd;
 	u8 index;
 	struct platform_device *pdev;
+	struct phy *phy;
+	const char *phy_label;
 	void __iomem *regs;
 	struct regulator_bulk_data supplies[CSIS_NUM_SUPPLIES];
 	struct clk *clock[NUM_CSIS_CLOCKS];
@@ -742,6 +746,7 @@ static int s5pcsis_get_platform_data(struct platform_device *pdev,
 	state->index = max(0, pdev->id);
 	state->max_num_lanes = state->index ? CSIS1_MAX_LANES :
 					      CSIS0_MAX_LANES;
+	state->phy_label = pdata->phy_label;
 	return 0;
 }
 
@@ -779,8 +784,9 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
 					"samsung,csis-wclk");
 
 	state->num_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
-
 	of_node_put(node);
+
+	state->phy_label = "csis";
 	return 0;
 }
 #else
@@ -829,6 +835,10 @@ static int s5pcsis_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	state->phy = devm_phy_get(dev, state->phy_label);
+	if (IS_ERR(state->phy))
+		return PTR_ERR(state->phy);
+
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	state->regs = devm_ioremap_resource(dev, mem_res);
 	if (IS_ERR(state->regs))
@@ -922,7 +932,7 @@ static int s5pcsis_pm_suspend(struct device *dev, bool runtime)
 	mutex_lock(&state->lock);
 	if (state->flags & ST_POWERED) {
 		s5pcsis_stop_stream(state);
-		ret = s5p_csis_phy_enable(state->index, false);
+		ret = phy_power_off(state->phy);
 		if (ret)
 			goto unlock;
 		ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES,
@@ -958,7 +968,7 @@ static int s5pcsis_pm_resume(struct device *dev, bool runtime)
 					    state->supplies);
 		if (ret)
 			goto unlock;
-		ret = s5p_csis_phy_enable(state->index, true);
+		ret = phy_power_on(state->phy);
 		if (!ret) {
 			state->flags |= ST_POWERED;
 		} else {
diff --git a/include/linux/platform_data/mipi-csis.h b/include/linux/platform_data/mipi-csis.h
index bf34e17..9214317 100644
--- a/include/linux/platform_data/mipi-csis.h
+++ b/include/linux/platform_data/mipi-csis.h
@@ -17,21 +17,14 @@
  * @wclk_source: CSI wrapper clock selection: 0 - bus clock, 1 - ext. SCLK_CAM
  * @lanes:       number of data lanes used
  * @hs_settle:   HS-RX settle time
+ * @phy_label:	 the generic PHY label
  */
 struct s5p_platform_mipi_csis {
 	unsigned long clk_rate;
 	u8 wclk_source;
 	u8 lanes;
 	u8 hs_settle;
+	const char *phy_label;
 };
 
-/**
- * s5p_csis_phy_enable - global MIPI-CSI receiver D-PHY control
- * @id:     MIPI-CSIS harware instance index (0...1)
- * @on:     true to enable D-PHY and deassert its reset
- *          false to disable D-PHY
- * @return: 0 on success, or negative error code on failure
- */
-int s5p_csis_phy_enable(int id, bool on);
-
 #endif /* __PLAT_SAMSUNG_MIPI_CSIS_H_ */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 12/15] ARM: Samsung: Remove the MIPI PHY setup code
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Generic PHY drivers are used to handle the MIPI CSIS and MIPI DSIM
DPHYs so we can remove now unused code at arch/arm/plat-samsung.
In case there is any board file for S5PV210 platforms using MIPI
CSIS/DSIM (not any upstream currently) it should use the generic
PHY API to bind the PHYs to respective PHY consumer drivers and
a platform device for the PHY provider should be defined.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 arch/arm/mach-exynos/include/mach/regs-pmu.h    |    5 --
 arch/arm/mach-s5pv210/include/mach/regs-clock.h |    4 --
 arch/arm/plat-samsung/Kconfig                   |    5 --
 arch/arm/plat-samsung/Makefile                  |    1 -
 arch/arm/plat-samsung/setup-mipiphy.c           |   60 -----------------------
 5 files changed, 75 deletions(-)
 delete mode 100644 arch/arm/plat-samsung/setup-mipiphy.c

diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index 57344b7..2cdb63e 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -44,11 +44,6 @@
 #define S5P_DAC_PHY_CONTROL			S5P_PMUREG(0x070C)
 #define S5P_DAC_PHY_ENABLE			(1 << 0)
 
-#define S5P_MIPI_DPHY_CONTROL(n)		S5P_PMUREG(0x0710 + (n) * 4)
-#define S5P_MIPI_DPHY_ENABLE			(1 << 0)
-#define S5P_MIPI_DPHY_SRESETN			(1 << 1)
-#define S5P_MIPI_DPHY_MRESETN			(1 << 2)
-
 #define S5P_INFORM0				S5P_PMUREG(0x0800)
 #define S5P_INFORM1				S5P_PMUREG(0x0804)
 #define S5P_INFORM2				S5P_PMUREG(0x0808)
diff --git a/arch/arm/mach-s5pv210/include/mach/regs-clock.h b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
index 032de66..e345584 100644
--- a/arch/arm/mach-s5pv210/include/mach/regs-clock.h
+++ b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
@@ -147,10 +147,6 @@
 #define S5P_HDMI_PHY_CONTROL	S5P_CLKREG(0xE804)
 #define S5P_USB_PHY_CONTROL	S5P_CLKREG(0xE80C)
 #define S5P_DAC_PHY_CONTROL	S5P_CLKREG(0xE810)
-#define S5P_MIPI_DPHY_CONTROL(x) S5P_CLKREG(0xE814)
-#define S5P_MIPI_DPHY_ENABLE	(1 << 0)
-#define S5P_MIPI_DPHY_SRESETN	(1 << 1)
-#define S5P_MIPI_DPHY_MRESETN	(1 << 2)
 
 #define S5P_INFORM0		S5P_CLKREG(0xF000)
 #define S5P_INFORM1		S5P_CLKREG(0xF004)
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 3dc5cbe..db2d814 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -402,11 +402,6 @@ config S3C24XX_PWM
 	  Support for exporting the PWM timer blocks via the pwm device
 	  system
 
-config S5P_SETUP_MIPIPHY
-	bool
-	help
-	  Compile in common setup code for MIPI-CSIS and MIPI-DSIM devices
-
 config S3C_SETUP_CAMIF
 	bool
 	help
diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile
index 98d07d8..98f1e31 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -41,7 +41,6 @@ obj-$(CONFIG_S5P_DEV_UART)	+= s5p-dev-uart.o
 obj-$(CONFIG_SAMSUNG_DEV_BACKLIGHT)	+= dev-backlight.o
 
 obj-$(CONFIG_S3C_SETUP_CAMIF)	+= setup-camif.o
-obj-$(CONFIG_S5P_SETUP_MIPIPHY)	+= setup-mipiphy.o
 
 # DMA support
 
diff --git a/arch/arm/plat-samsung/setup-mipiphy.c b/arch/arm/plat-samsung/setup-mipiphy.c
deleted file mode 100644
index 66df315..0000000
--- a/arch/arm/plat-samsung/setup-mipiphy.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2011 Samsung Electronics Co., Ltd.
- *
- * S5P - Helper functions for MIPI-CSIS and MIPI-DSIM D-PHY control
- *
- * 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/export.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/spinlock.h>
-#include <mach/regs-clock.h>
-
-static int __s5p_mipi_phy_control(int id, bool on, u32 reset)
-{
-	static DEFINE_SPINLOCK(lock);
-	void __iomem *addr;
-	unsigned long flags;
-	u32 cfg;
-
-	id = max(0, id);
-	if (id > 1)
-		return -EINVAL;
-
-	addr = S5P_MIPI_DPHY_CONTROL(id);
-
-	spin_lock_irqsave(&lock, flags);
-
-	cfg = __raw_readl(addr);
-	cfg = on ? (cfg | reset) : (cfg & ~reset);
-	__raw_writel(cfg, addr);
-
-	if (on) {
-		cfg |= S5P_MIPI_DPHY_ENABLE;
-	} else if (!(cfg & (S5P_MIPI_DPHY_SRESETN |
-			    S5P_MIPI_DPHY_MRESETN) & ~reset)) {
-		cfg &= ~S5P_MIPI_DPHY_ENABLE;
-	}
-
-	__raw_writel(cfg, addr);
-	spin_unlock_irqrestore(&lock, flags);
-
-	return 0;
-}
-
-int s5p_csis_phy_enable(int id, bool on)
-{
-	return __s5p_mipi_phy_control(id, on, S5P_MIPI_DPHY_SRESETN);
-}
-EXPORT_SYMBOL(s5p_csis_phy_enable);
-
-int s5p_dsim_phy_enable(struct platform_device *pdev, bool on)
-{
-	return __s5p_mipi_phy_control(pdev->id, on, S5P_MIPI_DPHY_MRESETN);
-}
-EXPORT_SYMBOL(s5p_dsim_phy_enable);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 13/15] phy: Add driver for Exynos DP PHY
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Jingoo Han <jg1.han@samsung.com>

Add a PHY provider driver for the Samsung Exynos SoC Display Port PHY.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 .../devicetree/bindings/phy/samsung-phy.txt        |    8 ++
 drivers/phy/Kconfig                                |    6 ++
 drivers/phy/Makefile                               |    1 +
 drivers/phy/phy-exynos-dp-video.c                  |  111 ++++++++++++++++++++
 4 files changed, 126 insertions(+)
 create mode 100644 drivers/phy/phy-exynos-dp-video.c

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..c0fccaa 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
   1 - MIPI DSIM 0,
   2 - MIPI CSIS 1,
   3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series Display Port PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the Display Port PHY register set;
+- #phy-cells : from the generic PHY bindings, must be 0;
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 6f446d0..ed0b1b8 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -19,4 +19,10 @@ config PHY_EXYNOS_MIPI_VIDEO
 	help
 	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
 	  S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+	tristate "EXYNOS SoC series Display Port PHY driver"
+	depends on OF
+	help
+	  Support for Display Port PHY found on Samsung EXYNOS SoCs.
 endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 71d8841..0fd1340 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..3c8e247
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,111 @@
+/*
+ * Samsung EXYNOS SoC series Display Port PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@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/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>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE		(1 << 0)
+
+struct exynos_dp_video_phy {
+	void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+	u32 reg;
+
+	reg = readl(state->regs);
+	if (on)
+		reg |= EXYNOS_DPTX_PHY_ENABLE;
+	else
+		reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+	writel(reg, state->regs);
+
+	return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+	.power_on	= exynos_dp_video_phy_power_on,
+	.power_off	= exynos_dp_video_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+	struct exynos_dp_video_phy *state;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	struct phy *phy;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	state->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(state->regs))
+		return PTR_ERR(state->regs);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, NULL);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create Display Port PHY\n");
+		return PTR_ERR(phy);
+	}
+	phy_set_drvdata(phy, state);
+
+	return 0;
+}
+
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+	{ .compatible = "samsung,exynos5250-dp-video-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+	.probe	= exynos_dp_video_phy_probe,
+	.driver = {
+		.name	= "exynos-dp-video-phy",
+		.owner	= THIS_MODULE,
+		.of_match_table	= exynos_dp_video_phy_of_match,
+	}
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 14/15] video: exynos_dp: remove non-DT support for Exynos Display Port
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Jingoo Han <jg1.han@samsung.com>

Exynos Display Port can be used only for Exynos SoCs. In addition,
non-DT for EXYNOS SoCs is not supported from v3.11; thus, there is
no need to support non-DT for Exynos Display Port.

The 'include/video/exynos_dp.h' file has been used for non-DT
support and the content of file include/video/exynos_dp.h is moved
to drivers/video/exynos/exynos_dp_core.h. Thus, the 'exynos_dp.h'
file is removed. Also, 'struct exynos_dp_platdata' is removed,
because it is not used any more.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/video/exynos/Kconfig          |    2 +-
 drivers/video/exynos/exynos_dp_core.c |  116 +++++++----------------------
 drivers/video/exynos/exynos_dp_core.h |  109 +++++++++++++++++++++++++++
 drivers/video/exynos/exynos_dp_reg.c  |    2 -
 include/video/exynos_dp.h             |  131 ---------------------------------
 5 files changed, 135 insertions(+), 225 deletions(-)
 delete mode 100644 include/video/exynos_dp.h

diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 1b035b2..fab9019 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -29,7 +29,7 @@ config EXYNOS_LCD_S6E8AX0
 
 config EXYNOS_DP
 	bool "EXYNOS DP driver support"
-	depends on ARCH_EXYNOS
+	depends on OF && ARCH_EXYNOS
 	default n
 	help
 	  This enables support for DP device.
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 12bbede..05fed7d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -20,8 +20,6 @@
 #include <linux/delay.h>
 #include <linux/of.h>
 
-#include <video/exynos_dp.h>
-
 #include "exynos_dp_core.h"
 
 static int exynos_dp_init_dp(struct exynos_dp_device *dp)
@@ -894,26 +892,17 @@ static void exynos_dp_hotplug(struct work_struct *work)
 		dev_err(dp->dev, "unable to config video\n");
 }
 
-#ifdef CONFIG_OF
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
+static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
 {
 	struct device_node *dp_node = dev->of_node;
-	struct exynos_dp_platdata *pd;
 	struct video_info *dp_video_config;
 
-	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
-	if (!pd) {
-		dev_err(dev, "memory allocation for pdata failed\n");
-		return ERR_PTR(-ENOMEM);
-	}
 	dp_video_config = devm_kzalloc(dev,
 				sizeof(*dp_video_config), GFP_KERNEL);
-
 	if (!dp_video_config) {
 		dev_err(dev, "memory allocation for video config failed\n");
 		return ERR_PTR(-ENOMEM);
 	}
-	pd->video_info = dp_video_config;
 
 	dp_video_config->h_sync_polarity  		of_property_read_bool(dp_node, "hsync-active-high");
@@ -960,7 +949,7 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
 		return ERR_PTR(-EINVAL);
 	}
 
-	return pd;
+	return dp_video_config;
 }
 
 static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
@@ -1003,48 +992,30 @@ err:
 
 static void exynos_dp_phy_init(struct exynos_dp_device *dp)
 {
-	u32 reg;
+	if (dp->phy_addr) {
+		u32 reg;
 
-	reg = __raw_readl(dp->phy_addr);
-	reg |= dp->enable_mask;
-	__raw_writel(reg, dp->phy_addr);
+		reg = __raw_readl(dp->phy_addr);
+		reg |= dp->enable_mask;
+		__raw_writel(reg, dp->phy_addr);
+	}
 }
 
 static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
 {
-	u32 reg;
-
-	reg = __raw_readl(dp->phy_addr);
-	reg &= ~(dp->enable_mask);
-	__raw_writel(reg, dp->phy_addr);
-}
-#else
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
-{
-	return NULL;
-}
-
-static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
-{
-	return -EINVAL;
-}
-
-static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-{
-	return;
-}
+	if (dp->phy_addr) {
+		u32 reg;
 
-static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
-{
-	return;
+		reg = __raw_readl(dp->phy_addr);
+		reg &= ~(dp->enable_mask);
+		__raw_writel(reg, dp->phy_addr);
+	}
 }
-#endif /* CONFIG_OF */
 
 static int exynos_dp_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct exynos_dp_device *dp;
-	struct exynos_dp_platdata *pdata;
 
 	int ret = 0;
 
@@ -1057,21 +1028,13 @@ static int exynos_dp_probe(struct platform_device *pdev)
 
 	dp->dev = &pdev->dev;
 
-	if (pdev->dev.of_node) {
-		pdata = exynos_dp_dt_parse_pdata(&pdev->dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
+	dp->video_info = exynos_dp_dt_parse_pdata(&pdev->dev);
+	if (IS_ERR(dp->video_info))
+		return PTR_ERR(dp->video_info);
 
-		ret = exynos_dp_dt_parse_phydata(dp);
-		if (ret)
-			return ret;
-	} else {
-		pdata = pdev->dev.platform_data;
-		if (!pdata) {
-			dev_err(&pdev->dev, "no platform data\n");
-			return -EINVAL;
-		}
-	}
+	ret = exynos_dp_dt_parse_phydata(dp);
+	if (ret)
+		return ret;
 
 	dp->clock = devm_clk_get(&pdev->dev, "dp");
 	if (IS_ERR(dp->clock)) {
@@ -1095,15 +1058,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
 
 	INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
 
-	dp->video_info = pdata->video_info;
-
-	if (pdev->dev.of_node) {
-		if (dp->phy_addr)
-			exynos_dp_phy_init(dp);
-	} else {
-		if (pdata->phy_init)
-			pdata->phy_init();
-	}
+	exynos_dp_phy_init(dp);
 
 	exynos_dp_init_dp(dp);
 
@@ -1121,18 +1076,11 @@ static int exynos_dp_probe(struct platform_device *pdev)
 
 static int exynos_dp_remove(struct platform_device *pdev)
 {
-	struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
 	struct exynos_dp_device *dp = platform_get_drvdata(pdev);
 
 	flush_work(&dp->hotplug_work);
 
-	if (pdev->dev.of_node) {
-		if (dp->phy_addr)
-			exynos_dp_phy_exit(dp);
-	} else {
-		if (pdata->phy_exit)
-			pdata->phy_exit();
-	}
+	exynos_dp_phy_exit(dp);
 
 	clk_disable_unprepare(dp->clock);
 
@@ -1143,20 +1091,13 @@ static int exynos_dp_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int exynos_dp_suspend(struct device *dev)
 {
-	struct exynos_dp_platdata *pdata = dev->platform_data;
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
 	disable_irq(dp->irq);
 
 	flush_work(&dp->hotplug_work);
 
-	if (dev->of_node) {
-		if (dp->phy_addr)
-			exynos_dp_phy_exit(dp);
-	} else {
-		if (pdata->phy_exit)
-			pdata->phy_exit();
-	}
+	exynos_dp_phy_exit(dp);
 
 	clk_disable_unprepare(dp->clock);
 
@@ -1165,16 +1106,9 @@ static int exynos_dp_suspend(struct device *dev)
 
 static int exynos_dp_resume(struct device *dev)
 {
-	struct exynos_dp_platdata *pdata = dev->platform_data;
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
-	if (dev->of_node) {
-		if (dp->phy_addr)
-			exynos_dp_phy_init(dp);
-	} else {
-		if (pdata->phy_init)
-			pdata->phy_init();
-	}
+	exynos_dp_phy_init(dp);
 
 	clk_prepare_enable(dp->clock);
 
@@ -1203,7 +1137,7 @@ static struct platform_driver exynos_dp_driver = {
 		.name	= "exynos-dp",
 		.owner	= THIS_MODULE,
 		.pm	= &exynos_dp_pm_ops,
-		.of_match_table = of_match_ptr(exynos_dp_match),
+		.of_match_table = exynos_dp_match,
 	},
 };
 
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 6c567bbf..56cfec8 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,99 @@
 #ifndef _EXYNOS_DP_CORE_H
 #define _EXYNOS_DP_CORE_H
 
+#define DP_TIMEOUT_LOOP_COUNT 100
+#define MAX_CR_LOOP 5
+#define MAX_EQ_LOOP 5
+
+enum link_rate_type {
+	LINK_RATE_1_62GBPS = 0x06,
+	LINK_RATE_2_70GBPS = 0x0a
+};
+
+enum link_lane_count_type {
+	LANE_COUNT1 = 1,
+	LANE_COUNT2 = 2,
+	LANE_COUNT4 = 4
+};
+
+enum link_training_state {
+	START,
+	CLOCK_RECOVERY,
+	EQUALIZER_TRAINING,
+	FINISHED,
+	FAILED
+};
+
+enum voltage_swing_level {
+	VOLTAGE_LEVEL_0,
+	VOLTAGE_LEVEL_1,
+	VOLTAGE_LEVEL_2,
+	VOLTAGE_LEVEL_3,
+};
+
+enum pre_emphasis_level {
+	PRE_EMPHASIS_LEVEL_0,
+	PRE_EMPHASIS_LEVEL_1,
+	PRE_EMPHASIS_LEVEL_2,
+	PRE_EMPHASIS_LEVEL_3,
+};
+
+enum pattern_set {
+	PRBS7,
+	D10_2,
+	TRAINING_PTN1,
+	TRAINING_PTN2,
+	DP_NONE
+};
+
+enum color_space {
+	COLOR_RGB,
+	COLOR_YCBCR422,
+	COLOR_YCBCR444
+};
+
+enum color_depth {
+	COLOR_6,
+	COLOR_8,
+	COLOR_10,
+	COLOR_12
+};
+
+enum color_coefficient {
+	COLOR_YCBCR601,
+	COLOR_YCBCR709
+};
+
+enum dynamic_range {
+	VESA,
+	CEA
+};
+
+enum pll_status {
+	PLL_UNLOCKED,
+	PLL_LOCKED
+};
+
+enum clock_recovery_m_value_type {
+	CALCULATED_M,
+	REGISTER_M
+};
+
+enum video_timing_recognition_type {
+	VIDEO_TIMING_FROM_CAPTURE,
+	VIDEO_TIMING_FROM_REGISTER
+};
+
+enum analog_power_block {
+	AUX_BLOCK,
+	CH0_BLOCK,
+	CH1_BLOCK,
+	CH2_BLOCK,
+	CH3_BLOCK,
+	ANALOG_TOTAL,
+	POWER_ALL
+};
+
 enum dp_irq_type {
 	DP_IRQ_TYPE_HP_CABLE_IN,
 	DP_IRQ_TYPE_HP_CABLE_OUT,
@@ -20,6 +113,22 @@ enum dp_irq_type {
 	DP_IRQ_TYPE_UNKNOWN,
 };
 
+struct video_info {
+	char *name;
+
+	bool h_sync_polarity;
+	bool v_sync_polarity;
+	bool interlaced;
+
+	enum color_space color_space;
+	enum dynamic_range dynamic_range;
+	enum color_coefficient ycbcr_coeff;
+	enum color_depth color_depth;
+
+	enum link_rate_type link_rate;
+	enum link_lane_count_type lane_count;
+};
+
 struct link_train {
 	int eq_loop;
 	int cr_loop[4];
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 29d9d03..b70da50 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -14,8 +14,6 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 
-#include <video/exynos_dp.h>
-
 #include "exynos_dp_core.h"
 #include "exynos_dp_reg.h"
 
diff --git a/include/video/exynos_dp.h b/include/video/exynos_dp.h
deleted file mode 100644
index bd8cabd..0000000
--- a/include/video/exynos_dp.h
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * Samsung SoC DP device support
- *
- * Copyright (C) 2012 Samsung Electronics Co., Ltd.
- * Author: Jingoo Han <jg1.han@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.
- */
-
-#ifndef _EXYNOS_DP_H
-#define _EXYNOS_DP_H
-
-#define DP_TIMEOUT_LOOP_COUNT 100
-#define MAX_CR_LOOP 5
-#define MAX_EQ_LOOP 5
-
-enum link_rate_type {
-	LINK_RATE_1_62GBPS = 0x06,
-	LINK_RATE_2_70GBPS = 0x0a
-};
-
-enum link_lane_count_type {
-	LANE_COUNT1 = 1,
-	LANE_COUNT2 = 2,
-	LANE_COUNT4 = 4
-};
-
-enum link_training_state {
-	START,
-	CLOCK_RECOVERY,
-	EQUALIZER_TRAINING,
-	FINISHED,
-	FAILED
-};
-
-enum voltage_swing_level {
-	VOLTAGE_LEVEL_0,
-	VOLTAGE_LEVEL_1,
-	VOLTAGE_LEVEL_2,
-	VOLTAGE_LEVEL_3,
-};
-
-enum pre_emphasis_level {
-	PRE_EMPHASIS_LEVEL_0,
-	PRE_EMPHASIS_LEVEL_1,
-	PRE_EMPHASIS_LEVEL_2,
-	PRE_EMPHASIS_LEVEL_3,
-};
-
-enum pattern_set {
-	PRBS7,
-	D10_2,
-	TRAINING_PTN1,
-	TRAINING_PTN2,
-	DP_NONE
-};
-
-enum color_space {
-	COLOR_RGB,
-	COLOR_YCBCR422,
-	COLOR_YCBCR444
-};
-
-enum color_depth {
-	COLOR_6,
-	COLOR_8,
-	COLOR_10,
-	COLOR_12
-};
-
-enum color_coefficient {
-	COLOR_YCBCR601,
-	COLOR_YCBCR709
-};
-
-enum dynamic_range {
-	VESA,
-	CEA
-};
-
-enum pll_status {
-	PLL_UNLOCKED,
-	PLL_LOCKED
-};
-
-enum clock_recovery_m_value_type {
-	CALCULATED_M,
-	REGISTER_M
-};
-
-enum video_timing_recognition_type {
-	VIDEO_TIMING_FROM_CAPTURE,
-	VIDEO_TIMING_FROM_REGISTER
-};
-
-enum analog_power_block {
-	AUX_BLOCK,
-	CH0_BLOCK,
-	CH1_BLOCK,
-	CH2_BLOCK,
-	CH3_BLOCK,
-	ANALOG_TOTAL,
-	POWER_ALL
-};
-
-struct video_info {
-	char *name;
-
-	bool h_sync_polarity;
-	bool v_sync_polarity;
-	bool interlaced;
-
-	enum color_space color_space;
-	enum dynamic_range dynamic_range;
-	enum color_coefficient ycbcr_coeff;
-	enum color_depth color_depth;
-
-	enum link_rate_type link_rate;
-	enum link_lane_count_type lane_count;
-};
-
-struct exynos_dp_platdata {
-	struct video_info *video_info;
-
-	void (*phy_init)(void);
-	void (*phy_exit)(void);
-};
-
-#endif /* _EXYNOS_DP_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 15/15] video: exynos_dp: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-07-18  6:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-1-git-send-email-kishon@ti.com>

From: Jingoo Han <jg1.han@samsung.com>

Use the generic PHY API to control the DP PHY.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 .../devicetree/bindings/video/exynos_dp.txt          |   18 +++++++++---------
 drivers/video/exynos/exynos_dp_core.c                |   16 ++++++++++++----
 drivers/video/exynos/exynos_dp_core.h                |    1 +
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
index 84f10c1..2f56376 100644
--- a/Documentation/devicetree/bindings/video/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -6,10 +6,10 @@ We use two nodes:
 	-dptx-phy node(defined inside dp-controller node)
 
 For the DP-PHY initialization, we use the dptx-phy node.
-Required properties for dptx-phy:
-	-reg:
+Required properties for dptx-phy: deprecated, use phys and phy-names
+	-reg: deprecated
 		Base address of DP PHY register.
-	-samsung,enable-mask:
+	-samsung,enable-mask: deprecated
 		The bit-mask used to enable/disable DP PHY.
 
 For the Panel initialization, we read data from dp-controller node.
@@ -25,6 +25,10 @@ Required properties for dp-controller:
 		from common clock binding: handle to dp clock.
 	-clock-names:
 		from common clock binding: Shall be "dp".
+	-phys:
+		from general PHY binding: the phandle for the PHY device.
+	-phy-names:
+		from general PHY binding: Should be "dp".
 	-interrupt-parent:
 		phandle to Interrupt combiner node.
 	-samsung,color-space:
@@ -67,12 +71,8 @@ SOC specific portion:
 		interrupt-parent = <&combiner>;
 		clocks = <&clock 342>;
 		clock-names = "dp";
-
-		dptx-phy {
-			reg = <0x10040720>;
-			samsung,enable-mask = <1>;
-		};
-
+		phys = <&dp_phy>;
+		phy-names = "dp";
 	};
 
 Board Specific portion:
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 05fed7d..5e1a715 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 
 #include "exynos_dp_core.h"
 
@@ -960,8 +961,11 @@ static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
 
 	dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
 	if (!dp_phy_node) {
-		dev_err(dp->dev, "could not find dptx-phy node\n");
-		return -ENODEV;
+		dp->phy = devm_phy_get(dp->dev, "dp");
+		if (IS_ERR(dp->phy))
+			return PTR_ERR(dp->phy);
+		else
+			return 0;
 	}
 
 	if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
@@ -992,7 +996,9 @@ err:
 
 static void exynos_dp_phy_init(struct exynos_dp_device *dp)
 {
-	if (dp->phy_addr) {
+	if (dp->phy) {
+		phy_power_on(dp->phy);
+	} else if (dp->phy_addr) {
 		u32 reg;
 
 		reg = __raw_readl(dp->phy_addr);
@@ -1003,7 +1009,9 @@ static void exynos_dp_phy_init(struct exynos_dp_device *dp)
 
 static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
 {
-	if (dp->phy_addr) {
+	if (dp->phy) {
+		phy_power_off(dp->phy);
+	} else if (dp->phy_addr) {
 		u32 reg;
 
 		reg = __raw_readl(dp->phy_addr);
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 56cfec8..607e36d 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -151,6 +151,7 @@ struct exynos_dp_device {
 	struct video_info	*video_info;
 	struct link_train	link_train;
 	struct work_struct	hotplug_work;
+	struct phy		*phy;
 };
 
 /* exynos_dp_reg.c */
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH 04/15] ARM: OMAP: USB: Add phy binding information
From: Tony Lindgren @ 2013-07-18  7:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-5-git-send-email-kishon@ti.com>

* Kishon Vijay Abraham I <kishon@ti.com> [130717 23:53]:
> In order for controllers to get PHY in case of non dt boot, the phy
> binding information (phy device name) should be added in the platform
> data of the controller.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> ---
>  arch/arm/mach-omap2/usb-musb.c |    3 +++
>  include/linux/usb/musb.h       |    3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 8c4de27..6aa7cbf 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -85,6 +85,9 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>  	musb_plat.mode = board_data->mode;
>  	musb_plat.extvbus = board_data->extvbus;
>  
> +	if (cpu_is_omap34xx())
> +		musb_plat.phy_label = "twl4030";
> +
>  	if (soc_is_am35xx()) {
>  		oh_name = "am35x_otg_hs";
>  		name = "musb-am35x";

I don't think there's a USB PHY on non-twl4030 chips, so this should
be OK:

Acked-by: Tony Lindgren <tony@atomide.com>


> diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
> index 053c268..596f8c8 100644
> --- a/include/linux/usb/musb.h
> +++ b/include/linux/usb/musb.h
> @@ -104,6 +104,9 @@ struct musb_hdrc_platform_data {
>  	/* for clk_get() */
>  	const char	*clock;
>  
> +	/* phy label */
> +	const char	*phy_label;
> +
>  	/* (HOST or OTG) switch VBUS on/off */
>  	int		(*set_vbus)(struct device *dev, int is_on);
>  
> -- 
> 1.7.10.4
> 

^ permalink raw reply

* Re: [PATCH 05/15] ARM: dts: omap: update usb_otg_hs data
From: Tony Lindgren @ 2013-07-18  7:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-6-git-send-email-kishon@ti.com>

* Kishon Vijay Abraham I <kishon@ti.com> [130717 23:53]:
> Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
> binding in order for the driver to use the new generic PHY framework.
> Also updated the Documentation to include the binding information.
> The PHY binding information can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

In general the .dts changes should be separate to avoid pointless
merge conflicts. But sounds like things will stop working for
USB unless we do it like this so:

Acked-by: Tony Lindgren <tony@atomide.com>


> ---
>  Documentation/devicetree/bindings/usb/omap-usb.txt |    5 +++++
>  Documentation/devicetree/bindings/usb/usb-phy.txt  |    6 ++++++
>  arch/arm/boot/dts/omap3-beagle-xm.dts              |    2 ++
>  arch/arm/boot/dts/omap3-evm.dts                    |    2 ++
>  arch/arm/boot/dts/omap3-overo.dtsi                 |    2 ++
>  arch/arm/boot/dts/omap4.dtsi                       |    3 +++
>  arch/arm/boot/dts/twl4030.dtsi                     |    1 +
>  7 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index 57e71f6..825790d 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -19,6 +19,9 @@ OMAP MUSB GLUE
>   - power : Should be "50". This signifies the controller can supply up to
>     100mA when operating in host mode.
>   - usb-phy : the phandle for the PHY device
> + - phys : the phandle for the PHY device (used by generic PHY framework)
> + - phy-names : the names of the PHY corresponding to the PHYs present in the
> +   *phy* phandle.
>  
>  Optional properties:
>   - ctrl-module : phandle of the control module this glue uses to write to
> @@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
>  	num-eps = <16>;
>  	ram-bits = <12>;
>  	ctrl-module = <&omap_control_usb>;
> +	phys = <&usb2_phy>;
> +	phy-names = "usb2-phy";
>  };
>  
>  Board specific device node entry
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> index 61496f5..c0245c8 100644
> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -5,6 +5,8 @@ OMAP USB2 PHY
>  Required properties:
>   - compatible: Should be "ti,omap-usb2"
>   - reg : Address and length of the register set for the device.
> + - #phy-cells: determine the number of cells that should be given in the
> +   phandle while referencing this phy.
>  
>  Optional properties:
>   - ctrl-module : phandle of the control module used by PHY driver to power on
> @@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
>  	compatible = "ti,omap-usb2";
>  	reg = <0x4a0ad080 0x58>;
>  	ctrl-module = <&omap_control_usb>;
> +	#phy-cells = <0>;
>  };
>  
>  OMAP USB3 PHY
> @@ -25,6 +28,8 @@ Required properties:
>   - reg : Address and length of the register set for the device.
>   - reg-names: The names of the register addresses corresponding to the registers
>     filled in "reg".
> + - #phy-cells: determine the number of cells that should be given in the
> +   phandle while referencing this phy.
>  
>  Optional properties:
>   - ctrl-module : phandle of the control module used by PHY driver to power on
> @@ -39,4 +44,5 @@ usb3phy@4a084400 {
>  	      <0x4a084c00 0x40>;
>  	reg-names = "phy_rx", "phy_tx", "pll_ctrl";
>  	ctrl-module = <&omap_control_usb>;
> +	#phy-cells = <0>;
>  };
> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
> index afdb164..533b2da 100644
> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> @@ -144,6 +144,8 @@
>  &usb_otg_hs {
>  	interface-type = <0>;
>  	usb-phy = <&usb2_phy>;
> +	phys = <&usb2_phy>;
> +	phy-names = "usb2-phy";
>  	mode = <3>;
>  	power = <50>;
>  };
> diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
> index 7d4329d..4134dd0 100644
> --- a/arch/arm/boot/dts/omap3-evm.dts
> +++ b/arch/arm/boot/dts/omap3-evm.dts
> @@ -70,6 +70,8 @@
>  &usb_otg_hs {
>  	interface-type = <0>;
>  	usb-phy = <&usb2_phy>;
> +	phys = <&usb2_phy>;
> +	phy-names = "usb2-phy";
>  	mode = <3>;
>  	power = <50>;
>  };
> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
> index 8f1abec..a461d2f 100644
> --- a/arch/arm/boot/dts/omap3-overo.dtsi
> +++ b/arch/arm/boot/dts/omap3-overo.dtsi
> @@ -76,6 +76,8 @@
>  &usb_otg_hs {
>  	interface-type = <0>;
>  	usb-phy = <&usb2_phy>;
> +	phys = <&usb2_phy>;
> +	phy-names = "usb2-phy";
>  	mode = <3>;
>  	power = <50>;
>  };
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 22d9f2b..1e8e2fe 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -520,6 +520,7 @@
>  				compatible = "ti,omap-usb2";
>  				reg = <0x4a0ad080 0x58>;
>  				ctrl-module = <&omap_control_usb>;
> +				#phy-cells = <0>;
>  			};
>  		};
>  
> @@ -658,6 +659,8 @@
>  			interrupt-names = "mc", "dma";
>  			ti,hwmods = "usb_otg_hs";
>  			usb-phy = <&usb2_phy>;
> +			phys = <&usb2_phy>;
> +			phy-names = "usb2-phy";
>  			multipoint = <1>;
>  			num-eps = <16>;
>  			ram-bits = <12>;
> diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
> index b3034da..ce4cd6f 100644
> --- a/arch/arm/boot/dts/twl4030.dtsi
> +++ b/arch/arm/boot/dts/twl4030.dtsi
> @@ -80,6 +80,7 @@
>  		usb1v8-supply = <&vusb1v8>;
>  		usb3v1-supply = <&vusb3v1>;
>  		usb_mode = <1>;
> +		#phy-cells = <0>;
>  	};
>  
>  	twl_pwm: pwm {
> -- 
> 1.7.10.4
> 

^ permalink raw reply

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Greg KH @ 2013-07-18  7:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-2-git-send-email-kishon@ti.com>

On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
> +struct phy_provider *__of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args));
> +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args))
> +
> +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
> +register the phy_provider and it takes device, owner and of_xlate as
> +arguments. For the dt boot case, all PHY providers should use one of the above
> +2 APIs to register the PHY provider.

Why do you have __ for the prefix of a public function?  Is that really
the way that OF handles this type of thing?

> --- /dev/null
> +++ b/drivers/phy/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# PHY
> +#
> +
> +menuconfig GENERIC_PHY
> +	tristate "PHY Subsystem"
> +	help
> +	  Generic PHY support.
> +
> +	  This framework is designed to provide a generic interface for PHY
> +	  devices present in the kernel. This layer will have the generic
> +	  API by which phy drivers can create PHY using the phy framework and
> +	  phy users can obtain reference to the PHY.

Again, please reverse this.  The drivers that use it should select it,
not depend on it, which will then enable this option.  I will never know
if I need to enable it, and based on your follow-on patches, if I don't,
drivers that were working just fine, now disappeared from my build,
which isn't nice, and a pain to notice and fix up.

> +/**
> + * phy_create() - create a new phy
> + * @dev: device that is creating the new phy
> + * @id: id of the phy
> + * @ops: function pointers for performing phy operations
> + * @label: label given to the phy
> + *
> + * Called to create a phy using phy framework.
> + */
> +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
> +	const char *label)
> +{
> +	int ret;
> +	struct phy *phy;
> +
> +	if (!dev) {
> +		dev_WARN(dev, "no device provided for PHY\n");
> +		ret = -EINVAL;
> +		goto err0;
> +	}
> +
> +	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
> +	if (!phy) {
> +		ret = -ENOMEM;
> +		goto err0;
> +	}
> +
> +	device_initialize(&phy->dev);
> +	mutex_init(&phy->mutex);
> +
> +	phy->dev.class = phy_class;
> +	phy->dev.parent = dev;
> +	phy->dev.of_node = dev->of_node;
> +	phy->id = id;
> +	phy->ops = ops;
> +	phy->label = kstrdup(label, GFP_KERNEL);
> +
> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);

Your naming is odd, no "phy" anywhere in it?  You rely on the sender to
never send a duplicate name.id pair?  Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?

> +static inline int phy_pm_runtime_get(struct phy *phy)
> +{
> +	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
> +		return -EINVAL;

Why would phy ever not be valid and a error pointer?  And why dump the
stack if that happens, that seems really extreme.

> +
> +	if (!pm_runtime_enabled(&phy->dev))
> +		return -ENOTSUPP;
> +
> +	return pm_runtime_get(&phy->dev);
> +}

This, and the other inline functions in this .h file seem huge, why are
they inline and not in the .c file?  There's no speed issues, and it
should save space overall in the .c file.  Please move them.


> +static inline int phy_init(struct phy *phy)
> +{
> +	int ret;
> +
> +	ret = phy_pm_runtime_get_sync(phy);
> +	if (ret < 0 && ret != -ENOTSUPP)
> +		return ret;
> +
> +	mutex_lock(&phy->mutex);
> +	if (phy->init_count++ = 0 && phy->ops->init) {
> +		ret = phy->ops->init(phy);
> +		if (ret < 0) {
> +			dev_err(&phy->dev, "phy init failed --> %d\n", ret);
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	mutex_unlock(&phy->mutex);
> +	phy_pm_runtime_put(phy);
> +	return ret;
> +}
> +
> +static inline int phy_exit(struct phy *phy)
> +{
> +	int ret;
> +
> +	ret = phy_pm_runtime_get_sync(phy);
> +	if (ret < 0 && ret != -ENOTSUPP)
> +		return ret;
> +
> +	mutex_lock(&phy->mutex);
> +	if (--phy->init_count = 0 && phy->ops->exit) {
> +		ret = phy->ops->exit(phy);
> +		if (ret < 0) {
> +			dev_err(&phy->dev, "phy exit failed --> %d\n", ret);
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	mutex_unlock(&phy->mutex);
> +	phy_pm_runtime_put(phy);
> +	return ret;
> +}
> +
> +static inline int phy_power_on(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	ret = phy_pm_runtime_get_sync(phy);
> +	if (ret < 0 && ret != -ENOTSUPP)
> +		return ret;
> +
> +	mutex_lock(&phy->mutex);
> +	if (phy->power_count++ = 0 && phy->ops->power_on) {
> +		ret = phy->ops->power_on(phy);
> +		if (ret < 0) {
> +			dev_err(&phy->dev, "phy poweron failed --> %d\n", ret);
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	mutex_unlock(&phy->mutex);
> +
> +	return ret;
> +}
> +
> +static inline int phy_power_off(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	mutex_lock(&phy->mutex);
> +	if (--phy->power_count = 0 && phy->ops->power_off) {
> +		ret =  phy->ops->power_off(phy);
> +		if (ret < 0) {
> +			dev_err(&phy->dev, "phy poweroff failed --> %d\n", ret);
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	mutex_unlock(&phy->mutex);
> +	phy_pm_runtime_put(phy);
> +
> +	return ret;
> +}

Look at those 3 functions, they are all "real" and not an inline
function at all, please move them.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
From: Greg KH @ 2013-07-18  7:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374129984-765-3-git-send-email-kishon@ti.com>

On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
> Used the generic PHY framework API to create the PHY. Now the power off and
> power on are done in omap_usb_power_off and omap_usb_power_on respectively.
> 
> However using the old USB PHY library cannot be completely removed
> because OTG is intertwined with PHY and moving to the new framework
> will break OTG. Once we have a separate OTG state machine, we
> can get rid of the USB PHY library.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/usb/phy/Kconfig         |    1 +
>  drivers/usb/phy/phy-omap-usb2.c |   45 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 3622fff..cc55993 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
>  config OMAP_USB2
>  	tristate "OMAP USB2 PHY Driver"
>  	depends on ARCH_OMAP2PLUS
> +	depends on GENERIC_PHY
>  	select OMAP_CONTROL_USB
>  	help
>  	  Enable this to support the transceiver that is part of SOC. This
> diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
> index 844ab68..751b30f 100644
> --- a/drivers/usb/phy/phy-omap-usb2.c
> +++ b/drivers/usb/phy/phy-omap-usb2.c
> @@ -28,6 +28,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/delay.h>
>  #include <linux/usb/omap_control_usb.h>
> +#include <linux/phy/phy.h>
>  
>  /**
>   * omap_usb2_set_comparator - links the comparator present in the sytem with
> @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
>  	return 0;
>  }
>  
> +static int omap_usb_power_off(struct phy *x)
> +{
> +	struct omap_usb *phy = phy_get_drvdata(x);
> +
> +	omap_control_usb_phy_power(phy->control_dev, 0);
> +
> +	return 0;
> +}
> +
> +static int omap_usb_power_on(struct phy *x)
> +{
> +	struct omap_usb *phy = phy_get_drvdata(x);
> +
> +	omap_control_usb_phy_power(phy->control_dev, 1);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops ops = {
> +	.power_on	= omap_usb_power_on,
> +	.power_off	= omap_usb_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
>  static int omap_usb2_probe(struct platform_device *pdev)
>  {
>  	struct omap_usb			*phy;
> +	struct phy			*generic_phy;
>  	struct usb_otg			*otg;
> +	struct phy_provider		*phy_provider;
>  
>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>  	if (!phy) {
> @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	phy->phy.otg		= otg;
>  	phy->phy.type		= USB_PHY_TYPE_USB2;
>  
> +	phy_provider = devm_of_phy_provider_register(phy->dev,
> +			of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
>  	phy->control_dev = omap_get_control_dev();
>  	if (IS_ERR(phy->control_dev)) {
>  		dev_dbg(&pdev->dev, "Failed to get control device\n");
> @@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	otg->start_srp		= omap_usb_start_srp;
>  	otg->phy		= &phy->phy;
>  
> +	platform_set_drvdata(pdev, phy);
> +	pm_runtime_enable(phy->dev);
> +
> +	generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2");
> +	if (IS_ERR(generic_phy))
> +		return PTR_ERR(generic_phy);

So, if I have two of these controllers in my system, I can't create the
second phy because the name for it will be identical to the first?
That's why the phy core should handle the id, and not rely on the
drivers to set it, as they have no idea how many they have in the
system.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v4 3/5] at91/avr32/atmel_lcdfb: prepare clk before calling enable
From: Boris BREZILLON @ 2013-07-18  7:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1374132753-15206-1-git-send-email-b.brezillon@overkiz.com>

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/video/atmel_lcdfb.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index ece49d5..bf9c5d0 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -954,14 +954,14 @@ static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
 
 static void atmel_lcdfb_start_clock(struct atmel_lcdfb_info *sinfo)
 {
-	clk_enable(sinfo->bus_clk);
-	clk_enable(sinfo->lcdc_clk);
+	clk_prepare_enable(sinfo->bus_clk);
+	clk_prepare_enable(sinfo->lcdc_clk);
 }
 
 static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
 {
-	clk_disable(sinfo->bus_clk);
-	clk_disable(sinfo->lcdc_clk);
+	clk_disable_unprepare(sinfo->bus_clk);
+	clk_disable_unprepare(sinfo->lcdc_clk);
 }
 
 #ifdef CONFIG_OF
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-18  9:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130718072004.GA16720@kroah.com>

Hi,

On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
> On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
>> +struct phy_provider *__of_phy_provider_register(struct device *dev,
>> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
>> +	struct of_phandle_args *args));
>> +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
>> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
>> +	struct of_phandle_args *args))
>> +
>> +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
>> +register the phy_provider and it takes device, owner and of_xlate as
>> +arguments. For the dt boot case, all PHY providers should use one of the above
>> +2 APIs to register the PHY provider.
> 
> Why do you have __ for the prefix of a public function?  Is that really
> the way that OF handles this type of thing?

I have a macro of_phy_provider_register/devm_of_phy_provider_register that
calls these functions and should be used by the PHY drivers. Probably I should
make a mention of it in the Documentation.
> 
>> --- /dev/null
>> +++ b/drivers/phy/Kconfig
>> @@ -0,0 +1,13 @@
>> +#
>> +# PHY
>> +#
>> +
>> +menuconfig GENERIC_PHY
>> +	tristate "PHY Subsystem"
>> +	help
>> +	  Generic PHY support.
>> +
>> +	  This framework is designed to provide a generic interface for PHY
>> +	  devices present in the kernel. This layer will have the generic
>> +	  API by which phy drivers can create PHY using the phy framework and
>> +	  phy users can obtain reference to the PHY.
> 
> Again, please reverse this.  The drivers that use it should select it,
> not depend on it, which will then enable this option.  I will never know
> if I need to enable it, and based on your follow-on patches, if I don't,
> drivers that were working just fine, now disappeared from my build,
> which isn't nice, and a pain to notice and fix up.

ok.
> 
>> +/**
>> + * phy_create() - create a new phy
>> + * @dev: device that is creating the new phy
>> + * @id: id of the phy
>> + * @ops: function pointers for performing phy operations
>> + * @label: label given to the phy
>> + *
>> + * Called to create a phy using phy framework.
>> + */
>> +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
>> +	const char *label)
>> +{
>> +	int ret;
>> +	struct phy *phy;
>> +
>> +	if (!dev) {
>> +		dev_WARN(dev, "no device provided for PHY\n");
>> +		ret = -EINVAL;
>> +		goto err0;
>> +	}
>> +
>> +	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
>> +	if (!phy) {
>> +		ret = -ENOMEM;
>> +		goto err0;
>> +	}
>> +
>> +	device_initialize(&phy->dev);
>> +	mutex_init(&phy->mutex);
>> +
>> +	phy->dev.class = phy_class;
>> +	phy->dev.parent = dev;
>> +	phy->dev.of_node = dev->of_node;
>> +	phy->id = id;
>> +	phy->ops = ops;
>> +	phy->label = kstrdup(label, GFP_KERNEL);
>> +
>> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
> 
> Your naming is odd, no "phy" anywhere in it?  You rely on the sender to
> never send a duplicate name.id pair?  Why not create your own ids based
> on the number of phys in the system, like almost all other classes and
> subsystems do?

hmm.. some PHY drivers use the id they provide to perform some of their
internal operation as in [1] (This is used only if a single PHY provider
implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO
to give the PHY drivers an option to use auto id.

[1] ->
http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
> 
>> +static inline int phy_pm_runtime_get(struct phy *phy)
>> +{
>> +	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
>> +		return -EINVAL;
> 
> Why would phy ever not be valid and a error pointer?  And why dump the
> stack if that happens, that seems really extreme.

hmm.. there might be cases where the same controller in one soc needs PHY
control and in some other soc does not need PHY control. In such cases, we
might get error pointer here.
I'll change WARN to dev_err.
> 
>> +
>> +	if (!pm_runtime_enabled(&phy->dev))
>> +		return -ENOTSUPP;
>> +
>> +	return pm_runtime_get(&phy->dev);
>> +}
> 
> This, and the other inline functions in this .h file seem huge, why are
> they inline and not in the .c file?  There's no speed issues, and it
> should save space overall in the .c file.  Please move them.

ok
> 
> 
>> +static inline int phy_init(struct phy *phy)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_pm_runtime_get_sync(phy);
>> +	if (ret < 0 && ret != -ENOTSUPP)
>> +		return ret;
>> +
>> +	mutex_lock(&phy->mutex);
>> +	if (phy->init_count++ = 0 && phy->ops->init) {
>> +		ret = phy->ops->init(phy);
>> +		if (ret < 0) {
>> +			dev_err(&phy->dev, "phy init failed --> %d\n", ret);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&phy->mutex);
>> +	phy_pm_runtime_put(phy);
>> +	return ret;
>> +}
>> +
>> +static inline int phy_exit(struct phy *phy)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_pm_runtime_get_sync(phy);
>> +	if (ret < 0 && ret != -ENOTSUPP)
>> +		return ret;
>> +
>> +	mutex_lock(&phy->mutex);
>> +	if (--phy->init_count = 0 && phy->ops->exit) {
>> +		ret = phy->ops->exit(phy);
>> +		if (ret < 0) {
>> +			dev_err(&phy->dev, "phy exit failed --> %d\n", ret);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&phy->mutex);
>> +	phy_pm_runtime_put(phy);
>> +	return ret;
>> +}
>> +
>> +static inline int phy_power_on(struct phy *phy)
>> +{
>> +	int ret = -ENOTSUPP;
>> +
>> +	ret = phy_pm_runtime_get_sync(phy);
>> +	if (ret < 0 && ret != -ENOTSUPP)
>> +		return ret;
>> +
>> +	mutex_lock(&phy->mutex);
>> +	if (phy->power_count++ = 0 && phy->ops->power_on) {
>> +		ret = phy->ops->power_on(phy);
>> +		if (ret < 0) {
>> +			dev_err(&phy->dev, "phy poweron failed --> %d\n", ret);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&phy->mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static inline int phy_power_off(struct phy *phy)
>> +{
>> +	int ret = -ENOTSUPP;
>> +
>> +	mutex_lock(&phy->mutex);
>> +	if (--phy->power_count = 0 && phy->ops->power_off) {
>> +		ret =  phy->ops->power_off(phy);
>> +		if (ret < 0) {
>> +			dev_err(&phy->dev, "phy poweroff failed --> %d\n", ret);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&phy->mutex);
>> +	phy_pm_runtime_put(phy);
>> +
>> +	return ret;
>> +}
> 
> Look at those 3 functions, they are all "real" and not an inline
> function at all, please move them.

Alright.

Thanks
Kishon

^ permalink raw reply

* Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-18  9:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130718072149.GB16720@kroah.com>

On Thursday 18 July 2013 12:51 PM, Greg KH wrote:
> On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
>> Used the generic PHY framework API to create the PHY. Now the power off and
>> power on are done in omap_usb_power_off and omap_usb_power_on respectively.
>>
>> However using the old USB PHY library cannot be completely removed
>> because OTG is intertwined with PHY and moving to the new framework
>> will break OTG. Once we have a separate OTG state machine, we
>> can get rid of the USB PHY library.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Acked-by: Felipe Balbi <balbi@ti.com>
>> ---
>>  drivers/usb/phy/Kconfig         |    1 +
>>  drivers/usb/phy/phy-omap-usb2.c |   45 +++++++++++++++++++++++++++++++++++----
>>  2 files changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index 3622fff..cc55993 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
>>  config OMAP_USB2
>>  	tristate "OMAP USB2 PHY Driver"
>>  	depends on ARCH_OMAP2PLUS
>> +	depends on GENERIC_PHY
>>  	select OMAP_CONTROL_USB
>>  	help
>>  	  Enable this to support the transceiver that is part of SOC. This
>> diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
>> index 844ab68..751b30f 100644
>> --- a/drivers/usb/phy/phy-omap-usb2.c
>> +++ b/drivers/usb/phy/phy-omap-usb2.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/pm_runtime.h>
>>  #include <linux/delay.h>
>>  #include <linux/usb/omap_control_usb.h>
>> +#include <linux/phy/phy.h>
>>  
>>  /**
>>   * omap_usb2_set_comparator - links the comparator present in the sytem with
>> @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
>>  	return 0;
>>  }
>>  
>> +static int omap_usb_power_off(struct phy *x)
>> +{
>> +	struct omap_usb *phy = phy_get_drvdata(x);
>> +
>> +	omap_control_usb_phy_power(phy->control_dev, 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int omap_usb_power_on(struct phy *x)
>> +{
>> +	struct omap_usb *phy = phy_get_drvdata(x);
>> +
>> +	omap_control_usb_phy_power(phy->control_dev, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct phy_ops ops = {
>> +	.power_on	= omap_usb_power_on,
>> +	.power_off	= omap_usb_power_off,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>>  static int omap_usb2_probe(struct platform_device *pdev)
>>  {
>>  	struct omap_usb			*phy;
>> +	struct phy			*generic_phy;
>>  	struct usb_otg			*otg;
>> +	struct phy_provider		*phy_provider;
>>  
>>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>  	if (!phy) {
>> @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	phy->phy.otg		= otg;
>>  	phy->phy.type		= USB_PHY_TYPE_USB2;
>>  
>> +	phy_provider = devm_of_phy_provider_register(phy->dev,
>> +			of_phy_simple_xlate);
>> +	if (IS_ERR(phy_provider))
>> +		return PTR_ERR(phy_provider);
>> +
>>  	phy->control_dev = omap_get_control_dev();
>>  	if (IS_ERR(phy->control_dev)) {
>>  		dev_dbg(&pdev->dev, "Failed to get control device\n");
>> @@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	otg->start_srp		= omap_usb_start_srp;
>>  	otg->phy		= &phy->phy;
>>  
>> +	platform_set_drvdata(pdev, phy);
>> +	pm_runtime_enable(phy->dev);
>> +
>> +	generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2");
>> +	if (IS_ERR(generic_phy))
>> +		return PTR_ERR(generic_phy);
> 
> So, if I have two of these controllers in my system, I can't create the
> second phy because the name for it will be identical to the first?
> That's why the phy core should handle the id, and not rely on the
> drivers to set it, as they have no idea how many they have in the
> system.

hmm.. for such cases I'll have something like PLATFORM_DEVID_AUTO.

Thanks
Kishon

^ permalink raw reply

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Greg KH @ 2013-07-18 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51E7AE88.3050007@ti.com>

On Thu, Jul 18, 2013 at 02:29:52PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
> > On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
> >> +struct phy_provider *__of_phy_provider_register(struct device *dev,
> >> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> >> +	struct of_phandle_args *args));
> >> +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
> >> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> >> +	struct of_phandle_args *args))
> >> +
> >> +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
> >> +register the phy_provider and it takes device, owner and of_xlate as
> >> +arguments. For the dt boot case, all PHY providers should use one of the above
> >> +2 APIs to register the PHY provider.
> > 
> > Why do you have __ for the prefix of a public function?  Is that really
> > the way that OF handles this type of thing?
> 
> I have a macro of_phy_provider_register/devm_of_phy_provider_register that
> calls these functions and should be used by the PHY drivers. Probably I should
> make a mention of it in the Documentation.

Yes, mention those as you never want to be calling __* functions
directly, right?

> >> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
> > 
> > Your naming is odd, no "phy" anywhere in it?  You rely on the sender to
> > never send a duplicate name.id pair?  Why not create your own ids based
> > on the number of phys in the system, like almost all other classes and
> > subsystems do?
> 
> hmm.. some PHY drivers use the id they provide to perform some of their
> internal operation as in [1] (This is used only if a single PHY provider
> implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO
> to give the PHY drivers an option to use auto id.
> 
> [1] ->
> http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html

No, who cares about the id?  No one outside of the phy core ever should,
because you pass back the only pointer that they really do care about,
if they need to do anything with the device.  Use that, and then you can
rip out all of the "search for a phy by a string" logic, as that's not
needed either.  Just stick to the pointer, it's easier, and safer that
way.

> >> +static inline int phy_pm_runtime_get(struct phy *phy)
> >> +{
> >> +	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
> >> +		return -EINVAL;
> > 
> > Why would phy ever not be valid and a error pointer?  And why dump the
> > stack if that happens, that seems really extreme.
> 
> hmm.. there might be cases where the same controller in one soc needs PHY
> control and in some other soc does not need PHY control. In such cases, we
> might get error pointer here.
> I'll change WARN to dev_err.

I still don't understand.  You have control over the code that calls
these functions, just ensure that they pass in a valid pointer, it's
that simple.  Or am I missing something?

thanks,

greg k-h

^ permalink raw reply

* RE: [PATCH v2 0/5] ARM: vf610: Add DCU framebuffer driver for Vybrid VF610 platform
From: Wang Huan-B18965 @ 2013-07-19  3:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1373609276-14566-1-git-send-email-b18965@freescale.com>

Hi, Jean-Christophe,

      Could you please help to review these patches? 

      Thanks a lot!


Best Regards,
Alison Wang

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces@lists.infradead.org] On Behalf Of Alison Wang
> Sent: Friday, July 12, 2013 2:08 PM
> To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com; shawn.guo@linaro.org;
> Estevam Fabio-R49496; linux-fbdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: Jin Zhengxiong-R64188
> Subject: [PATCH v2 0/5] ARM: vf610: Add DCU framebuffer driver for
> Vybrid VF610 platform
> 
> This series contain DCU framebuffer driver for Freescale Vybrid VF610
> platform.
> 
> The Display Controller Unit (DCU) module is a system master that
> fetches graphics stored in internal or external memory and displays
> them on a TFT LCD panel. A wide range of panel sizes is supported and
> the timing of the interface signals is highly configurable.
> Graphics are read directly from memory and then blended in real-time,
> which allows for dynamic content creation with minimal CPU intervention.
> 
> The features:
> 
> (1) Full RGB888 output to TFT LCD panel.
> (2) For the current LCD panel, WQVGA "480x272" is tested.
> (3) Blending of each pixel using up to 4 source layers dependent on
> size of panel.
> (4) Each graphic layer can be placed with one pixel resolution in
> either axis.
> (5) Each graphic layer support RGB565 and RGB888 direct colors without
> alpha channel and BGRA8888 direct colors with an alpha channel.
> (6) Each graphic layer support alpha blending with 8-bit resolution.
> 
> Changes in v2:
> - Add a document for DCU framebuffer driver under
> Documentation/devicetree/bindings/fb/.
> 
> ----------------------------------------------------------------
> Alison Wang (5):
>       ARM: dts: vf610: Add DCU and TCON nodes
>       ARM: dts: vf610-twr: Enable DCU and TCON devices
>       ARM: clk: vf610: Add DCU and TCON clock support
>       fb: Add DCU framebuffer driver for Vybrid VF610 platform
>       Documentation: DT: Add DCU framebuffer driver
> 
>  Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt |   36 ++++
>  arch/arm/boot/dts/vf610-twr.dts                     |   10 +
>  arch/arm/boot/dts/vf610.dtsi                        |   19 +-
>  arch/arm/mach-imx/clk-vf610.c                       |    5 +
>  drivers/video/Kconfig                               |    9 +
>  drivers/video/Makefile                              |    1 +
>  drivers/video/fsl-dcu-fb.c                          | 1091
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++
>  include/dt-bindings/clock/vf610-clock.h             |    3 +-
>  8 files changed, 1172 insertions(+), 2 deletions(-)  create mode
> 100644 Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt
>  create mode 100644 drivers/video/fsl-dcu-fb.c
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



^ permalink raw reply

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Greg KH @ 2013-07-19  5:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51E8D086.809@ti.com>

On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
> >>>> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
> >>>
> >>> Your naming is odd, no "phy" anywhere in it?  You rely on the sender to
> >>> never send a duplicate name.id pair?  Why not create your own ids based
> >>> on the number of phys in the system, like almost all other classes and
> >>> subsystems do?
> >>
> >> hmm.. some PHY drivers use the id they provide to perform some of their
> >> internal operation as in [1] (This is used only if a single PHY provider
> >> implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO
> >> to give the PHY drivers an option to use auto id.
> >>
> >> [1] ->
> >> http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
> > 
> > No, who cares about the id?  No one outside of the phy core ever should,
> > because you pass back the only pointer that they really do care about,
> > if they need to do anything with the device.  Use that, and then you can
> 
> hmm.. ok.
> 
> > rip out all of the "search for a phy by a string" logic, as that's not
> 
> Actually this is needed for non-dt boot case. In the case of dt boot, we use a
> phandle by which the controller can get a reference to the phy. But in the case
> of non-dt boot, the controller can get a reference to the phy only by label.

I don't understand.  They registered the phy, and got back a pointer to
it.  Why can't they save it in their local structure to use it again
later if needed?  They should never have to "ask" for the device, as the
device id might be unknown if there are multiple devices in the system.

Or am I missing something?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-07-19  5:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130718154954.GA31961@kroah.com>

Hi,

On Thursday 18 July 2013 09:19 PM, Greg KH wrote:
> On Thu, Jul 18, 2013 at 02:29:52PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
>>> On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
>>>> +struct phy_provider *__of_phy_provider_register(struct device *dev,
>>>> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
>>>> +	struct of_phandle_args *args));
>>>> +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
>>>> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
>>>> +	struct of_phandle_args *args))
>>>> +
>>>> +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
>>>> +register the phy_provider and it takes device, owner and of_xlate as
>>>> +arguments. For the dt boot case, all PHY providers should use one of the above
>>>> +2 APIs to register the PHY provider.
>>>
>>> Why do you have __ for the prefix of a public function?  Is that really
>>> the way that OF handles this type of thing?
>>
>> I have a macro of_phy_provider_register/devm_of_phy_provider_register that
>> calls these functions and should be used by the PHY drivers. Probably I should
>> make a mention of it in the Documentation.
> 
> Yes, mention those as you never want to be calling __* functions
> directly, right?

correct.
> 
>>>> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
>>>
>>> Your naming is odd, no "phy" anywhere in it?  You rely on the sender to
>>> never send a duplicate name.id pair?  Why not create your own ids based
>>> on the number of phys in the system, like almost all other classes and
>>> subsystems do?
>>
>> hmm.. some PHY drivers use the id they provide to perform some of their
>> internal operation as in [1] (This is used only if a single PHY provider
>> implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO
>> to give the PHY drivers an option to use auto id.
>>
>> [1] ->
>> http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
> 
> No, who cares about the id?  No one outside of the phy core ever should,
> because you pass back the only pointer that they really do care about,
> if they need to do anything with the device.  Use that, and then you can

hmm.. ok.

> rip out all of the "search for a phy by a string" logic, as that's not

Actually this is needed for non-dt boot case. In the case of dt boot, we use a
phandle by which the controller can get a reference to the phy. But in the case
of non-dt boot, the controller can get a reference to the phy only by label.
> needed either.  Just stick to the pointer, it's easier, and safer that
> way.
> 
>>>> +static inline int phy_pm_runtime_get(struct phy *phy)
>>>> +{
>>>> +	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
>>>> +		return -EINVAL;
>>>
>>> Why would phy ever not be valid and a error pointer?  And why dump the
>>> stack if that happens, that seems really extreme.
>>
>> hmm.. there might be cases where the same controller in one soc needs PHY
>> control and in some other soc does not need PHY control. In such cases, we
>> might get error pointer here.
>> I'll change WARN to dev_err.
> 
> I still don't understand.  You have control over the code that calls
> these functions, just ensure that they pass in a valid pointer, it's
> that simple.  Or am I missing something?

You are right. Valid pointer check can be done in controller code as well.

Thanks
Kishon

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox