[PATCH 0/7] phy: omap-usb: Support multiple instances and new types

[PATCH 0/7] phy: omap-usb: Support multiple instances and new types

[Roger Quadros]

Hi,

This patchset does the following:

* Restructure and add support for new PHY types. We now support the follwing
four types

 TYPE1 - if it has otghs_control mailbox register (e.g. on OMAP4)
 TYPE2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY
 TYPE3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY
 TYPE4 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7

* Have only one power control API "omap_control_usb_phy_power()" instead of a
different one for each PHY type.

* Get rid of omap_get_control_dev() so that we can support multiple instances
of the control device. We take advantage of the fact that omap control USB device
is only present on OMAP4 onwards and hence only supports DT boot. The users
of control USB device can get a reference to it from the device node's phandle.

Patches are based on top of v3.11-rc3 with balbi/next on top.

Smoke tested on OMAP4 panda with MUSB in gadget mode (g_zero).

You can find the patches in branch
 usb-control-module
in git tree
 git://github.com/rogerq/linux.git

cheers,
-roger

Roger Quadros (7):
  usb: phy: omap: Add new PHY types and remove
    omap_control_usb3_phy_power()
  usb: phy: omap-usb2: Don't use omap_get_control_dev()
  usb: phy: omap-usb3: Don't use omap_get_control_dev()
  usb: musb: omap2430: Don't use omap_get_control_dev()
  usb: phy: omap: get rid of omap_get_control_dev()
  ARM: dts: omap4: update omap-control-usb nodes
  ARM: dts: omap5: update omap-control-usb node

 Documentation/devicetree/bindings/usb/omap-usb.txt |   23 ++--
 arch/arm/boot/dts/omap4.dtsi                       |   17 ++-
 arch/arm/boot/dts/omap5.dtsi                       |   18 ++-
 drivers/usb/musb/omap2430.c                        |   22 +++-
 drivers/usb/phy/phy-omap-control.c                 |  160 ++++++++++---------
 drivers/usb/phy/phy-omap-usb2.c                    |   20 ++-
 drivers/usb/phy/phy-omap-usb3.c                    |   26 +++-
 include/linux/usb/omap_control_usb.h               |   23 +--
 8 files changed, 182 insertions(+), 127 deletions(-)

-- 
1.7.4.1

[PATCH 1/7] usb: phy: omap: Add new PHY types and remove omap_control_usb3_phy_power()

[Roger Quadros]

TYPE2 meaning has changed. It is now a USB2 phy with Power down bit in
control_dev_conf register.

Introduce TYPE3 and TYPE4 PHY. TYPE3 is USB3 phy with DPLL and individual
TX/RX power control. TYPE4 is USB2 phy with power aux register.

Update DT binding information to reflect these changes.

Also get rid of omap_control_usb3_phy_power(). Just one function
i.e. omap_control_usb_phy_power() will now take care of all PHY types.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/usb/omap-usb.txt |   23 ++--
 drivers/usb/phy/phy-omap-control.c                 |  129 +++++++++++---------
 drivers/usb/phy/phy-omap-usb2.c                    |    4 +
 drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
 include/linux/usb/omap_control_usb.h               |   18 ++--
 5 files changed, 100 insertions(+), 80 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..e2606ab 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -75,20 +75,19 @@ OMAP CONTROL USB
 Required properties:
  - compatible: Should be "ti,omap-control-usb"
  - reg : Address and length of the register set for the device. It contains
-   the address of "control_dev_conf" and "otghs_control" or "phy_power_usb"
-   depending upon omap4 or omap5.
- - reg-names: The names of the register addresses corresponding to the registers
-   filled in "reg".
- - ti,type: This is used to differentiate whether the control module has
-   usb mailbox or usb3 phy power. omap4 has usb mailbox in control module to
-   notify events to the musb core and omap5 has usb3 phy power register to
-   power on usb3 phy. Should be "1" if it has mailbox and "2" if it has usb3
-   phy power.
+   the address of "otghs_control" for type 1 or "power" register for other types.
+   For type 4, it must also contain "power_aux" register.
+ - reg-names: should be otghs_control for type 1 and "power" for other types.
+ - ti,type: This is used to specify the type of control register.
+   Should be one of the following:
+   1 - if it has otghs_control mailbox register (e.g. on OMAP4)
+   2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY
+   3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY
+   4 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7
 
 omap_control_usb: omap-control-usb@4a002300 {
 	compatible = "ti,omap-control-usb";
-	reg = <0x4a002300 0x4>,
-	      <0x4a00233c 0x4>;
-	reg-names = "control_dev_conf", "otghs_control";
+	reg = <0x4a00233c 0x4>;
+	reg-names = "otghs_control";
 	ti,type = <1>;
 };
diff --git a/drivers/usb/phy/phy-omap-control.c b/drivers/usb/phy/phy-omap-control.c
index a4dda8e..cf93c5a 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -46,61 +46,76 @@ struct device *omap_get_control_dev(void)
 EXPORT_SYMBOL_GPL(omap_get_control_dev);
 
 /**
- * omap_control_usb3_phy_power - power on/off the serializer using control
- *	module
+ * omap_control_usb_phy_power - power on/off the phy using control module reg
  * @dev: the control module device
- * @on: 0 to off and 1 to on based on powering on or off the PHY
- *
- * usb3 PHY driver should call this API to power on or off the PHY.
+ * @on: 0 or 1, based on powering on or off the PHY
  */
-void omap_control_usb3_phy_power(struct device *dev, bool on)
+void omap_control_usb_phy_power(struct device *dev, int on)
 {
-	u32 val;
+	u32 val, val_aux;
 	unsigned long rate;
-	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
+	struct omap_control_usb	*control_usb;
 
-	if (control_usb->type != OMAP_CTRL_DEV_TYPE2)
+	if (IS_ERR(dev) || !dev) {
+		pr_err("%s: invalid device\n", __func__);
 		return;
+	}
 
-	rate = clk_get_rate(control_usb->sys_clk);
-	rate = rate/1000000;
+	control_usb = dev_get_drvdata(dev);
+	if (!control_usb) {
+		dev_err(dev, "%s: invalid control usb device\n", __func__);
+		return;
+	}
 
-	val = readl(control_usb->phy_power);
+	if (control_usb->type == OMAP_CTRL_DEV_TYPE1)
+		return;
 
-	if (on) {
-		val &= ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
-			OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
-		val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON <<
-			OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
-		val |= rate << OMAP_CTRL_USB_PWRCTL_CLK_FREQ_SHIFT;
-	} else {
-		val &= ~OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK;
-		val |= OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF <<
-			OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
-	}
+	val = readl(control_usb->power);
 
-	writel(val, control_usb->phy_power);
-}
-EXPORT_SYMBOL_GPL(omap_control_usb3_phy_power);
+	switch (control_usb->type) {
+	case OMAP_CTRL_DEV_TYPE2:
+		if (on)
+			val &= ~OMAP_CTRL_DEV_PHY_PD;
+		else
+			val |= OMAP_CTRL_DEV_PHY_PD;
+		break;
 
-/**
- * omap_control_usb_phy_power - power on/off the phy using control module reg
- * @dev: the control module device
- * @on: 0 or 1, based on powering on or off the PHY
- */
-void omap_control_usb_phy_power(struct device *dev, int on)
-{
-	u32 val;
-	struct omap_control_usb	*control_usb = dev_get_drvdata(dev);
+	case OMAP_CTRL_DEV_TYPE3:
+		rate = clk_get_rate(control_usb->sys_clk);
+		rate = rate/1000000;
+
+		if (on) {
+			val &= ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
+					OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
+			val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON <<
+				OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
+			val |= rate << OMAP_CTRL_USB_PWRCTL_CLK_FREQ_SHIFT;
+		} else {
+			val &= ~OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK;
+			val |= OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF <<
+				OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
+		}
+		break;
 
-	val = readl(control_usb->dev_conf);
+	case OMAP_CTRL_DEV_TYPE4:
+		val_aux = readl(control_usb->power_aux);
+		if (on) {
+			val &= ~OMAP_CTRL_USB2_PHY_PD;
+			val_aux |= 0x100;
+		} else {
+			val |= OMAP_CTRL_USB2_PHY_PD;
+			val_aux &= ~0x100;
+		}
 
-	if (on)
-		val &= ~OMAP_CTRL_DEV_PHY_PD;
-	else
-		val |= OMAP_CTRL_DEV_PHY_PD;
+		writel(val_aux, control_usb->power_aux);
+		break;
+	default:
+		dev_err(dev, "%s: type %d not recognized\n",
+					__func__, control_usb->type);
+		break;
+	}
 
-	writel(val, control_usb->dev_conf);
+	writel(val, control_usb->power);
 }
 EXPORT_SYMBOL_GPL(omap_control_usb_phy_power);
 
@@ -218,12 +233,6 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 
 	control_usb->dev	= &pdev->dev;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-		"control_dev_conf");
-	control_usb->dev_conf = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(control_usb->dev_conf))
-		return PTR_ERR(control_usb->dev_conf);
-
 	if (control_usb->type == OMAP_CTRL_DEV_TYPE1) {
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 			"otghs_control");
@@ -231,16 +240,17 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 			&pdev->dev, res);
 		if (IS_ERR(control_usb->otghs_control))
 			return PTR_ERR(control_usb->otghs_control);
-	}
-
-	if (control_usb->type == OMAP_CTRL_DEV_TYPE2) {
+	} else {
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-			"phy_power_usb");
-		control_usb->phy_power = devm_ioremap_resource(
-			&pdev->dev, res);
-		if (IS_ERR(control_usb->phy_power))
-			return PTR_ERR(control_usb->phy_power);
+				"power");
+		control_usb->power = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(control_usb->power)) {
+			dev_err(&pdev->dev, "Couldn't get power register\n");
+			return PTR_ERR(control_usb->power);
+		}
+	}
 
+	if (control_usb->type == OMAP_CTRL_DEV_TYPE3) {
 		control_usb->sys_clk = devm_clk_get(control_usb->dev,
 			"sys_clkin");
 		if (IS_ERR(control_usb->sys_clk)) {
@@ -249,6 +259,15 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (control_usb->type == OMAP_CTRL_DEV_TYPE4) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+				"power_aux");
+		control_usb->power_aux = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(control_usb->power_aux)) {
+			dev_err(&pdev->dev, "Couldn't get power_aux register\n");
+			return PTR_ERR(control_usb->power_aux);
+		}
+	}
 
 	dev_set_drvdata(control_usb->dev, control_usb);
 
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..376b367 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -123,6 +123,10 @@ static int omap_usb2_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
 	struct usb_otg			*otg;
+	struct device_node *node = pdev->dev.of_node;
+
+	if (!node)
+		return -ENODEV;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy) {
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index fc15694..4a7f27c 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -100,7 +100,7 @@ static int omap_usb3_suspend(struct usb_phy *x, int suspend)
 			udelay(1);
 		} while (--timeout);
 
-		omap_control_usb3_phy_power(phy->control_dev, 0);
+		omap_control_usb_phy_power(phy->control_dev, 0);
 
 		phy->is_suspended	= 1;
 	} else if (!suspend && phy->is_suspended) {
@@ -189,7 +189,7 @@ static int omap_usb3_init(struct usb_phy *x)
 	if (ret)
 		return ret;
 
-	omap_control_usb3_phy_power(phy->control_dev, 1);
+	omap_control_usb_phy_power(phy->control_dev, 1);
 
 	return 0;
 }
@@ -245,7 +245,7 @@ static int omap_usb3_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	omap_control_usb3_phy_power(phy->control_dev, 0);
+	omap_control_usb_phy_power(phy->control_dev, 0);
 	usb_add_phy_dev(&phy->phy);
 
 	platform_set_drvdata(pdev, phy);
diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
index 27b5b8c..7f027d3 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -22,9 +22,9 @@
 struct omap_control_usb {
 	struct device *dev;
 
-	u32 __iomem *dev_conf;
 	u32 __iomem *otghs_control;
-	u32 __iomem *phy_power;
+	u32 __iomem *power;
+	u32 __iomem *power_aux;
 
 	struct clk *sys_clk;
 
@@ -42,9 +42,10 @@ enum omap_control_usb_mode {
 	USB_MODE_DISCONNECT,
 };
 
-/* To differentiate ctrl module IP having either mailbox or USB3 PHY power */
-#define	OMAP_CTRL_DEV_TYPE1		0x1
-#define	OMAP_CTRL_DEV_TYPE2		0x2
+#define	OMAP_CTRL_DEV_TYPE1 0x1	/* Mailbox OTGHS_CONTROL */
+#define	OMAP_CTRL_DEV_TYPE2 0x2	/* USB2_PHY, power down in CONTROL_DEV_CONF */
+#define	OMAP_CTRL_DEV_TYPE3 0x3	/* USB3_PHY, DPLL & seperate Rx/Tx power */
+#define	OMAP_CTRL_DEV_TYPE4 0x4	/* USB2 PHY, power and power_aux e.g. DRA7 */
 
 #define	OMAP_CTRL_DEV_PHY_PD		BIT(0)
 
@@ -63,10 +64,11 @@ enum omap_control_usb_mode {
 #define	OMAP_CTRL_USB3_PHY_TX_RX_POWERON	0x3
 #define	OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF	0x0
 
+#define OMAP_CTRL_USB2_PHY_PD		BIT(28)
+
 #if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
 extern struct device *omap_get_control_dev(void);
 extern void omap_control_usb_phy_power(struct device *dev, int on);
-extern void omap_control_usb3_phy_power(struct device *dev, bool on);
 extern void omap_control_usb_set_mode(struct device *dev,
 	enum omap_control_usb_mode mode);
 #else
@@ -79,10 +81,6 @@ static inline void omap_control_usb_phy_power(struct device *dev, int on)
 {
 }
 
-static inline void omap_control_usb3_phy_power(struct device *dev, int on)
-{
-}
-
 static inline void omap_control_usb_set_mode(struct device *dev,
 	enum omap_control_usb_mode mode)
 {
-- 
1.7.4.1

Re: [PATCH 1/7] usb: phy: omap: Add new PHY types and remove omap_control_usb3_phy_power()

[Kishon Vijay Abraham I]

Hi,

On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
> TYPE2 meaning has changed. It is now a USB2 phy with Power down bit in
> control_dev_conf register.
>
> Introduce TYPE3 and TYPE4 PHY. TYPE3 is USB3 phy with DPLL and individual
> TX/RX power control. TYPE4 is USB2 phy with power aux register.
>
> Update DT binding information to reflect these changes.
>
> Also get rid of omap_control_usb3_phy_power(). Just one function
> i.e. omap_control_usb_phy_power() will now take care of all PHY types.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
but one minor comment below.
> ---
>   Documentation/devicetree/bindings/usb/omap-usb.txt |   23 ++--
>   drivers/usb/phy/phy-omap-control.c                 |  129 +++++++++++---------
>   drivers/usb/phy/phy-omap-usb2.c                    |    4 +
>   drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
>   include/linux/usb/omap_control_usb.h               |   18 ++--
>   5 files changed, 100 insertions(+), 80 deletions(-)
>
.
.
<snip>
.
.

> +	omap_control_usb_phy_power(phy->control_dev, 0);
>   	usb_add_phy_dev(&phy->phy);
>
>   	platform_set_drvdata(pdev, phy);
> diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
> index 27b5b8c..7f027d3 100644
> --- a/include/linux/usb/omap_control_usb.h
> +++ b/include/linux/usb/omap_control_usb.h
> @@ -22,9 +22,9 @@

>   struct omap_control_usb {
>   	struct device *dev;
>
> -	u32 __iomem *dev_conf;
>   	u32 __iomem *otghs_control;
> -	u32 __iomem *phy_power;
> +	u32 __iomem *power;
> +	u32 __iomem *power_aux;
>
>   	struct clk *sys_clk;
>
you can remove omap_control_usb_platform_data from here.

Thanks
Kishon

Re: [PATCH 1/7] usb: phy: omap: Add new PHY types and remove omap_control_usb3_phy_power()

[Roger Quadros]

On 08/06/2013 02:45 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
>> TYPE2 meaning has changed. It is now a USB2 phy with Power down bit in
>> control_dev_conf register.
>>
>> Introduce TYPE3 and TYPE4 PHY. TYPE3 is USB3 phy with DPLL and individual
>> TX/RX power control. TYPE4 is USB2 phy with power aux register.
>>
>> Update DT binding information to reflect these changes.
>>
>> Also get rid of omap_control_usb3_phy_power(). Just one function
>> i.e. omap_control_usb_phy_power() will now take care of all PHY types.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> but one minor comment below.
>> ---
>>   Documentation/devicetree/bindings/usb/omap-usb.txt |   23 ++--
>>   drivers/usb/phy/phy-omap-control.c                 |  129 +++++++++++---------
>>   drivers/usb/phy/phy-omap-usb2.c                    |    4 +
>>   drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
>>   include/linux/usb/omap_control_usb.h               |   18 ++--
>>   5 files changed, 100 insertions(+), 80 deletions(-)
>>
> .
> .
> <snip>
> .
> .
> 
>> +    omap_control_usb_phy_power(phy->control_dev, 0);
>>       usb_add_phy_dev(&phy->phy);
>>
>>       platform_set_drvdata(pdev, phy);
>> diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
>> index 27b5b8c..7f027d3 100644
>> --- a/include/linux/usb/omap_control_usb.h
>> +++ b/include/linux/usb/omap_control_usb.h
>> @@ -22,9 +22,9 @@
> 
>>   struct omap_control_usb {
>>       struct device *dev;
>>
>> -    u32 __iomem *dev_conf;
>>       u32 __iomem *otghs_control;
>> -    u32 __iomem *phy_power;
>> +    u32 __iomem *power;
>> +    u32 __iomem *power_aux;
>>
>>       struct clk *sys_clk;
>>
> you can remove omap_control_usb_platform_data from here.
> 
OK.

cheers,
-roger

[PATCH 2/7] usb: phy: omap-usb2: Don't use omap_get_control_dev()

[Roger Quadros]

omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

As we don't support non-DT boot, we just bail out on probe
if device node is not present.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/phy/phy-omap-usb2.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 376b367..77e0cf4 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/of_platform.h>
 
 /**
  * omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -124,6 +125,8 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	struct omap_usb			*phy;
 	struct usb_otg			*otg;
 	struct device_node *node = pdev->dev.of_node;
+	struct device_node *control_node;
+	struct platform_device *control_pdev;
 
 	if (!node)
 		return -ENODEV;
@@ -148,11 +151,18 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
-	phy->control_dev = omap_get_control_dev();
-	if (IS_ERR(phy->control_dev)) {
-		dev_dbg(&pdev->dev, "Failed to get control device\n");
+	control_node = of_parse_phandle(node, "ctrl-module", 0);
+	if (!control_node) {
+		dev_err(&pdev->dev, "Failed to get control device phandle\n");
 		return -ENODEV;
 	}
+	control_pdev = of_find_device_by_node(control_node);
+	if (!control_pdev) {
+		dev_err(&pdev->dev, "Failed to get control device\n");
+		return -ENODEV;
+	}
+
+	phy->control_dev = &control_pdev->dev;
 
 	phy->is_suspended	= 1;
 	omap_control_usb_phy_power(phy->control_dev, 0);
-- 
1.7.4.1

Re: [PATCH 2/7] usb: phy: omap-usb2: Don't use omap_get_control_dev()

[Kishon Vijay Abraham I]

On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
> omap_get_control_dev() is being deprecated as it doesn't support
> multiple instances. As control device is present only from OMAP4
> onwards which supports DT only, we use phandles to get the
> reference to the control device.
>
> As we don't support non-DT boot, we just bail out on probe
> if device node is not present.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/usb/phy/phy-omap-usb2.c |   16 +++++++++++++---
>   1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
> index 376b367..77e0cf4 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/of_platform.h>
>
>   /**
>    * omap_usb2_set_comparator - links the comparator present in the sytem with
> @@ -124,6 +125,8 @@ static int omap_usb2_probe(struct platform_device *pdev)
>   	struct omap_usb			*phy;
>   	struct usb_otg			*otg;
>   	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *control_node;
> +	struct platform_device *control_pdev;
>
>   	if (!node)
>   		return -ENODEV;
> @@ -148,11 +151,18 @@ static int omap_usb2_probe(struct platform_device *pdev)
>   	phy->phy.otg		= otg;
>   	phy->phy.type		= USB_PHY_TYPE_USB2;
>
> -	phy->control_dev = omap_get_control_dev();
> -	if (IS_ERR(phy->control_dev)) {
> -		dev_dbg(&pdev->dev, "Failed to get control device\n");
> +	control_node = of_parse_phandle(node, "ctrl-module", 0);
> +	if (!control_node) {
> +		dev_err(&pdev->dev, "Failed to get control device phandle\n");
>   		return -ENODEV;
>   	}
> +	control_pdev = of_find_device_by_node(control_node);
> +	if (!control_pdev) {
> +		dev_err(&pdev->dev, "Failed to get control device\n");
> +		return -ENODEV;
> +	}
> +
> +	phy->control_dev = &control_pdev->dev;
>
>   	phy->is_suspended	= 1;
>   	omap_control_usb_phy_power(phy->control_dev, 0);
>

[PATCH 3/7] usb: phy: omap-usb3: Don't use omap_get_control_dev()

[Roger Quadros]

omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

As we don't support non-DT boot, we just bail out on probe
if device node is not present.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/phy/phy-omap-usb3.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index 4a7f27c..bad032e 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -26,6 +26,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
 #include <linux/usb/omap_control_usb.h>
+#include <linux/of_platform.h>
 
 #define	PLL_STATUS		0x00000004
 #define	PLL_GO			0x00000008
@@ -198,6 +199,12 @@ static int omap_usb3_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
 	struct resource			*res;
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *control_node;
+	struct platform_device *control_pdev;
+
+	if (!node)
+		return -ENODEV;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy) {
@@ -239,11 +246,18 @@ static int omap_usb3_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	phy->control_dev = omap_get_control_dev();
-	if (IS_ERR(phy->control_dev)) {
-		dev_dbg(&pdev->dev, "Failed to get control device\n");
+	control_node = of_parse_phandle(node, "ctrl-module", 0);
+	if (!control_node) {
+		dev_err(&pdev->dev, "Failed to get control device phandle\n");
 		return -ENODEV;
 	}
+	control_pdev = of_find_device_by_node(control_node);
+	if (!control_pdev) {
+		dev_err(&pdev->dev, "Failed to get control device\n");
+		return -ENODEV;
+	}
+
+	phy->control_dev = &control_pdev->dev;
 
 	omap_control_usb_phy_power(phy->control_dev, 0);
 	usb_add_phy_dev(&phy->phy);
-- 
1.7.4.1

Re: [PATCH 3/7] usb: phy: omap-usb3: Don't use omap_get_control_dev()

[Kishon Vijay Abraham I]

On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
> omap_get_control_dev() is being deprecated as it doesn't support
> multiple instances. As control device is present only from OMAP4
> onwards which supports DT only, we use phandles to get the
> reference to the control device.
>
> As we don't support non-DT boot, we just bail out on probe
> if device node is not present.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Tested-by: Kishon Vijay Abraham I <kishon@ti.com>


> ---
>   drivers/usb/phy/phy-omap-usb3.c |   20 +++++++++++++++++---
>   1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
> index 4a7f27c..bad032e 100644
> --- a/drivers/usb/phy/phy-omap-usb3.c
> +++ b/drivers/usb/phy/phy-omap-usb3.c
> @@ -26,6 +26,7 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/delay.h>
>   #include <linux/usb/omap_control_usb.h>
> +#include <linux/of_platform.h>
>
>   #define	PLL_STATUS		0x00000004
>   #define	PLL_GO			0x00000008
> @@ -198,6 +199,12 @@ static int omap_usb3_probe(struct platform_device *pdev)
>   {
>   	struct omap_usb			*phy;
>   	struct resource			*res;
> +	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *control_node;
> +	struct platform_device *control_pdev;
> +
> +	if (!node)
> +		return -ENODEV;
>
>   	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>   	if (!phy) {
> @@ -239,11 +246,18 @@ static int omap_usb3_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>
> -	phy->control_dev = omap_get_control_dev();
> -	if (IS_ERR(phy->control_dev)) {
> -		dev_dbg(&pdev->dev, "Failed to get control device\n");
> +	control_node = of_parse_phandle(node, "ctrl-module", 0);
> +	if (!control_node) {
> +		dev_err(&pdev->dev, "Failed to get control device phandle\n");
>   		return -ENODEV;
>   	}
> +	control_pdev = of_find_device_by_node(control_node);
> +	if (!control_pdev) {
> +		dev_err(&pdev->dev, "Failed to get control device\n");
> +		return -ENODEV;
> +	}
> +
> +	phy->control_dev = &control_pdev->dev;
>
>   	omap_control_usb_phy_power(phy->control_dev, 0);
>   	usb_add_phy_dev(&phy->phy);
>

[PATCH 4/7] usb: musb: omap2430: Don't use omap_get_control_dev()

[Roger Quadros]

omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/musb/omap2430.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index ebb46ec..1db9588 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -38,6 +38,7 @@
 #include <linux/delay.h>
 #include <linux/usb/musb-omap.h>
 #include <linux/usb/omap_control_usb.h>
+#include <linux/of_platform.h>
 
 #include "musb_core.h"
 #include "omap2430.h"
@@ -547,12 +548,25 @@ static int omap2430_probe(struct platform_device *pdev)
 	}
 
 	if (pdata->has_mailbox) {
-		glue->control_otghs = omap_get_control_dev();
-		if (IS_ERR(glue->control_otghs)) {
-			dev_vdbg(&pdev->dev, "Failed to get control device\n");
-			ret = PTR_ERR(glue->control_otghs);
+		struct device_node *control_node;
+		struct platform_device *control_pdev;
+
+		control_node = of_parse_phandle(np, "ctrl-module", 0);
+		if (!control_node) {
+			dev_err(&pdev->dev, "Failed to get control device phandle\n");
+			ret = -ENODEV;
+			goto err2;
+		}
+
+		control_pdev = of_find_device_by_node(control_node);
+		if (!control_pdev) {
+			dev_err(&pdev->dev, "Failed to get control device\n");
+			ret = -ENODEV;
 			goto err2;
 		}
+
+		glue->control_otghs = &control_pdev->dev;
+
 	} else {
 		glue->control_otghs = ERR_PTR(-ENODEV);
 	}
-- 
1.7.4.1

Re: [PATCH 4/7] usb: musb: omap2430: Don't use omap_get_control_dev()

[Kishon Vijay Abraham I]

Hi,

On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
> omap_get_control_dev() is being deprecated as it doesn't support
> multiple instances. As control device is present only from OMAP4
> onwards which supports DT only, we use phandles to get the
> reference to the control device.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>   drivers/usb/musb/omap2430.c |   22 ++++++++++++++++++----
>   1 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index ebb46ec..1db9588 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -38,6 +38,7 @@
>   #include <linux/delay.h>
>   #include <linux/usb/musb-omap.h>
>   #include <linux/usb/omap_control_usb.h>
> +#include <linux/of_platform.h>
>
>   #include "musb_core.h"
>   #include "omap2430.h"
> @@ -547,12 +548,25 @@ static int omap2430_probe(struct platform_device *pdev)
>   	}
>
>   	if (pdata->has_mailbox) {

I think you can remove this this check all together. Mailbox is present 
only from OMAP4 so it can be dt only.

Thanks
Kishon

Re: [PATCH 4/7] usb: musb: omap2430: Don't use omap_get_control_dev()

[Roger Quadros]

On 08/06/2013 02:51 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
>> omap_get_control_dev() is being deprecated as it doesn't support
>> multiple instances. As control device is present only from OMAP4
>> onwards which supports DT only, we use phandles to get the
>> reference to the control device.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   drivers/usb/musb/omap2430.c |   22 ++++++++++++++++++----
>>   1 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>> index ebb46ec..1db9588 100644
>> --- a/drivers/usb/musb/omap2430.c
>> +++ b/drivers/usb/musb/omap2430.c
>> @@ -38,6 +38,7 @@
>>   #include <linux/delay.h>
>>   #include <linux/usb/musb-omap.h>
>>   #include <linux/usb/omap_control_usb.h>
>> +#include <linux/of_platform.h>
>>
>>   #include "musb_core.h"
>>   #include "omap2430.h"
>> @@ -547,12 +548,25 @@ static int omap2430_probe(struct platform_device *pdev)
>>       }
>>
>>       if (pdata->has_mailbox) {
> 
> I think you can remove this this check all together. Mailbox is present only from OMAP4 so it can be dt only.
> 
OK.

cheers,
-roger

[PATCH 5/7] usb: phy: omap: get rid of omap_get_control_dev()

[Roger Quadros]

This function was preventing us from supporting multiple
instances. Get rid of it. Since we support DT boots only,
users can get the control device phandle from the DT node.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/phy/phy-omap-control.c   |   31 ++++++++++---------------------
 include/linux/usb/omap_control_usb.h |    5 -----
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-control.c b/drivers/usb/phy/phy-omap-control.c
index cf93c5a..015eeef 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -25,26 +25,6 @@
 #include <linux/clk.h>
 #include <linux/usb/omap_control_usb.h>
 
-static struct omap_control_usb *control_usb;
-
-/**
- * omap_get_control_dev - returns the device pointer for this control device
- *
- * This API should be called to get the device pointer for this control
- * module device. This device pointer should be used for called other
- * exported API's in this driver.
- *
- * To be used by PHY driver and glue driver.
- */
-struct device *omap_get_control_dev(void)
-{
-	if (!control_usb)
-		return ERR_PTR(-ENODEV);
-
-	return control_usb->dev;
-}
-EXPORT_SYMBOL_GPL(omap_get_control_dev);
-
 /**
  * omap_control_usb_phy_power - power on/off the phy using control module reg
  * @dev: the control module device
@@ -187,11 +167,19 @@ void omap_control_usb_set_mode(struct device *dev,
 {
 	struct omap_control_usb	*ctrl_usb;
 
-	if (IS_ERR(dev) || control_usb->type != OMAP_CTRL_DEV_TYPE1)
+	if (IS_ERR(dev) || !dev)
 		return;
 
 	ctrl_usb = dev_get_drvdata(dev);
 
+	if (!ctrl_usb) {
+		dev_err(dev, "Invalid control usb device\n");
+		return;
+	}
+
+	if (ctrl_usb->type != OMAP_CTRL_DEV_TYPE1)
+		return;
+
 	switch (mode) {
 	case USB_MODE_HOST:
 		omap_control_usb_host_mode(ctrl_usb);
@@ -214,6 +202,7 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct omap_control_usb_platform_data *pdata =
 			dev_get_platdata(&pdev->dev);
+	struct omap_control_usb *control_usb;
 
 	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
 		GFP_KERNEL);
diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
index 7f027d3..8b48584 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -67,15 +67,10 @@ enum omap_control_usb_mode {
 #define OMAP_CTRL_USB2_PHY_PD		BIT(28)
 
 #if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
-extern struct device *omap_get_control_dev(void);
 extern void omap_control_usb_phy_power(struct device *dev, int on);
 extern void omap_control_usb_set_mode(struct device *dev,
 	enum omap_control_usb_mode mode);
 #else
-static inline struct device *omap_get_control_dev(void)
-{
-	return ERR_PTR(-ENODEV);
-}
 
 static inline void omap_control_usb_phy_power(struct device *dev, int on)
 {
-- 
1.7.4.1

Re: [PATCH 5/7] usb: phy: omap: get rid of omap_get_control_dev()

[Kishon Vijay Abraham I]

On Thursday 01 August 2013 07:35 PM, Roger Quadros wrote:
> This function was preventing us from supporting multiple
> instances. Get rid of it. Since we support DT boots only,
> users can get the control device phandle from the DT node.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Tested-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/usb/phy/phy-omap-control.c   |   31 ++++++++++---------------------
>   include/linux/usb/omap_control_usb.h |    5 -----
>   2 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-omap-control.c b/drivers/usb/phy/phy-omap-control.c
> index cf93c5a..015eeef 100644
> --- a/drivers/usb/phy/phy-omap-control.c
> +++ b/drivers/usb/phy/phy-omap-control.c
> @@ -25,26 +25,6 @@
>   #include <linux/clk.h>
>   #include <linux/usb/omap_control_usb.h>
>
> -static struct omap_control_usb *control_usb;
> -
> -/**
> - * omap_get_control_dev - returns the device pointer for this control device
> - *
> - * This API should be called to get the device pointer for this control
> - * module device. This device pointer should be used for called other
> - * exported API's in this driver.
> - *
> - * To be used by PHY driver and glue driver.
> - */
> -struct device *omap_get_control_dev(void)
> -{
> -	if (!control_usb)
> -		return ERR_PTR(-ENODEV);
> -
> -	return control_usb->dev;
> -}
> -EXPORT_SYMBOL_GPL(omap_get_control_dev);
> -
>   /**
>    * omap_control_usb_phy_power - power on/off the phy using control module reg
>    * @dev: the control module device
> @@ -187,11 +167,19 @@ void omap_control_usb_set_mode(struct device *dev,
>   {
>   	struct omap_control_usb	*ctrl_usb;
>
> -	if (IS_ERR(dev) || control_usb->type != OMAP_CTRL_DEV_TYPE1)
> +	if (IS_ERR(dev) || !dev)
>   		return;
>
>   	ctrl_usb = dev_get_drvdata(dev);
>
> +	if (!ctrl_usb) {
> +		dev_err(dev, "Invalid control usb device\n");
> +		return;
> +	}
> +
> +	if (ctrl_usb->type != OMAP_CTRL_DEV_TYPE1)
> +		return;
> +
>   	switch (mode) {
>   	case USB_MODE_HOST:
>   		omap_control_usb_host_mode(ctrl_usb);
> @@ -214,6 +202,7 @@ static int omap_control_usb_probe(struct platform_device *pdev)
>   	struct device_node *np = pdev->dev.of_node;
>   	struct omap_control_usb_platform_data *pdata =
>   			dev_get_platdata(&pdev->dev);
> +	struct omap_control_usb *control_usb;
>
>   	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
>   		GFP_KERNEL);
> diff --git a/include/linux/usb/omap_control_usb.h b/include/linux/usb/omap_control_usb.h
> index 7f027d3..8b48584 100644
> --- a/include/linux/usb/omap_control_usb.h
> +++ b/include/linux/usb/omap_control_usb.h
> @@ -67,15 +67,10 @@ enum omap_control_usb_mode {
>   #define OMAP_CTRL_USB2_PHY_PD		BIT(28)
>
>   #if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
> -extern struct device *omap_get_control_dev(void);
>   extern void omap_control_usb_phy_power(struct device *dev, int on);
>   extern void omap_control_usb_set_mode(struct device *dev,
>   	enum omap_control_usb_mode mode);
>   #else
> -static inline struct device *omap_get_control_dev(void)
> -{
> -	return ERR_PTR(-ENODEV);
> -}
>
>   static inline void omap_control_usb_phy_power(struct device *dev, int on)
>   {
>

[PATCH 6/7] ARM: dts: omap4: update omap-control-usb nodes

[Roger Quadros]

Split otghs_ctrl and USB2 PHY power down into separate
omap-control-usb nodes. Update ti,mode property.

CC: Benoit Cousson <benoit.cousson@linaro.org>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..9a6fa27 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -519,7 +519,7 @@
 			usb2_phy: usb2phy@4a0ad080 {
 				compatible = "ti,omap-usb2";
 				reg = <0x4a0ad080 0x58>;
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb2phy>;
 			};
 		};
 
@@ -643,11 +643,17 @@
 			};
 		};
 
-		omap_control_usb: omap-control-usb@4a002300 {
+		omap_control_usb2phy: omap-control-usb@4a002300 {
 			compatible = "ti,omap-control-usb";
-			reg = <0x4a002300 0x4>,
-			      <0x4a00233c 0x4>;
-			reg-names = "control_dev_conf", "otghs_control";
+			reg = <0x4a002300 0x4>;
+			reg-names = "power";
+			ti,type = <2>;
+		};
+
+		omap_control_usbotg: omap-control-usb@4a00233c {
+			compatible = "ti,omap-control-usb";
+			reg = <0x4a00233c 0x4>;
+			reg-names = "otghs_control";
 			ti,type = <1>;
 		};
 
@@ -662,6 +668,7 @@
 			num-eps = <16>;
 			ram-bits = <12>;
 			ti,has-mailbox;
+			ctrl-module = <&omap_control_usbotg>;
 		};
 	};
 };
-- 
1.7.4.1

Re: [PATCH 6/7] ARM: dts: omap4: update omap-control-usb nodes

[Roger Quadros]

+Benoit

On 08/01/2013 05:05 PM, Roger Quadros wrote:
> Split otghs_ctrl and USB2 PHY power down into separate
> omap-control-usb nodes. Update ti,mode property.
> 
> CC: Benoit Cousson <benoit.cousson@linaro.org>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi |   17 ++++++++++++-----
>  1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 22d9f2b..9a6fa27 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -519,7 +519,7 @@
>  			usb2_phy: usb2phy@4a0ad080 {
>  				compatible = "ti,omap-usb2";
>  				reg = <0x4a0ad080 0x58>;
> -				ctrl-module = <&omap_control_usb>;
> +				ctrl-module = <&omap_control_usb2phy>;
>  			};
>  		};
>  
> @@ -643,11 +643,17 @@
>  			};
>  		};
>  
> -		omap_control_usb: omap-control-usb@4a002300 {
> +		omap_control_usb2phy: omap-control-usb@4a002300 {
>  			compatible = "ti,omap-control-usb";
> -			reg = <0x4a002300 0x4>,
> -			      <0x4a00233c 0x4>;
> -			reg-names = "control_dev_conf", "otghs_control";
> +			reg = <0x4a002300 0x4>;
> +			reg-names = "power";
> +			ti,type = <2>;
> +		};
> +
> +		omap_control_usbotg: omap-control-usb@4a00233c {
> +			compatible = "ti,omap-control-usb";
> +			reg = <0x4a00233c 0x4>;
> +			reg-names = "otghs_control";
>  			ti,type = <1>;
>  		};
>  
> @@ -662,6 +668,7 @@
>  			num-eps = <16>;
>  			ram-bits = <12>;
>  			ti,has-mailbox;
> +			ctrl-module = <&omap_control_usbotg>;
>  		};
>  	};
>  };
> 

Re: [PATCH 6/7] ARM: dts: omap4: update omap-control-usb nodes

[Benoit Cousson]

Hi Roger,

On 01/08/2013 16:05, Roger Quadros wrote:
> Split otghs_ctrl and USB2 PHY power down into separate
> omap-control-usb nodes. Update ti,mode property.

Nit: I guess you mean ti,type?

> CC: Benoit Cousson <benoit.cousson@linaro.org>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>   arch/arm/boot/dts/omap4.dtsi |   17 ++++++++++++-----
>   1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 22d9f2b..9a6fa27 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -519,7 +519,7 @@
>   			usb2_phy: usb2phy@4a0ad080 {
>   				compatible = "ti,omap-usb2";
>   				reg = <0x4a0ad080 0x58>;
> -				ctrl-module = <&omap_control_usb>;
> +				ctrl-module = <&omap_control_usb2phy>;
>   			};
>   		};
>   
> @@ -643,11 +643,17 @@
>   			};
>   		};
>   
> -		omap_control_usb: omap-control-usb@4a002300 {
> +		omap_control_usb2phy: omap-control-usb@4a002300 {
>   			compatible = "ti,omap-control-usb";
> -			reg = <0x4a002300 0x4>,
> -			      <0x4a00233c 0x4>;
> -			reg-names = "control_dev_conf", "otghs_control";
> +			reg = <0x4a002300 0x4>;
> +			reg-names = "power";
> +			ti,type = <2>;

Now that we can use the C preprocessor, it will be nice to use a macro instead of the value.

TYPE1 - if it has otghs_control mailbox register (e.g. on OMAP4)
TYPE2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY
TYPE3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY
TYPE4 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7

Well, assuming you can find macro names that can explain a little bit what the type is about :-)

That being said...
Do you really need to expose the type here? Maybe with just a set of different compatible string you can figure out in the driver what type we are talking about.
It is always better to minimize the amount of information we put in DT as soon as we can infer it from the compatible string.

So instead of using a generic "ti,omap-control-usb" string + "ti,type" you can potentially use several specific strings: ti,omap4-control-usb, ti,dra7-control-usb...
Since the DT gurus are recommending to use specific compatible string as much as possible, this is maybe a better approach.

Regards,
Benoit

Re: [PATCH 6/7] ARM: dts: omap4: update omap-control-usb nodes

[Roger Quadros]

On 08/14/2013 11:41 AM, Benoit Cousson wrote:
> Hi Roger,
> 
> On 01/08/2013 16:05, Roger Quadros wrote:
>> Split otghs_ctrl and USB2 PHY power down into separate
>> omap-control-usb nodes. Update ti,mode property.
> 
> Nit: I guess you mean ti,type?

Right :).

> 
>> CC: Benoit Cousson <benoit.cousson@linaro.org>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   arch/arm/boot/dts/omap4.dtsi |   17 ++++++++++++-----
>>   1 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 22d9f2b..9a6fa27 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -519,7 +519,7 @@
>>   			usb2_phy: usb2phy@4a0ad080 {
>>   				compatible = "ti,omap-usb2";
>>   				reg = <0x4a0ad080 0x58>;
>> -				ctrl-module = <&omap_control_usb>;
>> +				ctrl-module = <&omap_control_usb2phy>;
>>   			};
>>   		};
>>   
>> @@ -643,11 +643,17 @@
>>   			};
>>   		};
>>   
>> -		omap_control_usb: omap-control-usb@4a002300 {
>> +		omap_control_usb2phy: omap-control-usb@4a002300 {
>>   			compatible = "ti,omap-control-usb";
>> -			reg = <0x4a002300 0x4>,
>> -			      <0x4a00233c 0x4>;
>> -			reg-names = "control_dev_conf", "otghs_control";
>> +			reg = <0x4a002300 0x4>;
>> +			reg-names = "power";
>> +			ti,type = <2>;
> 
> Now that we can use the C preprocessor, it will be nice to use a macro instead of the value.
> 
> TYPE1 - if it has otghs_control mailbox register (e.g. on OMAP4)
> TYPE2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY
> TYPE3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY
> TYPE4 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7
> 
> Well, assuming you can find macro names that can explain a little bit what the type is about :-)
> 
> That being said...
> Do you really need to expose the type here? Maybe with just a set of different compatible string you can figure out in the driver what type we are talking about.
> It is always better to minimize the amount of information we put in DT as soon as we can infer it from the compatible string.
> 
> So instead of using a generic "ti,omap-control-usb" string + "ti,type" you can potentially use several specific strings: ti,omap4-control-usb, ti,dra7-control-usb...
> Since the DT gurus are recommending to use specific compatible string as much as possible, this is maybe a better approach.
> 

Good point. I'll get rid of the type altogether and use the compatible string instead.

cheers,
-roger

[PATCH 7/7] ARM: dts: omap5: update omap-control-usb node

[Roger Quadros]

Split USB2 PHY and USB3 PHY into separate omap-control-usb
nodes. Update ti,mode property.

CC: Benoit Cousson <benoit.cousson@linaro.org>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap5.dtsi |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 07be2cd..af8ef77 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -626,14 +626,20 @@
 			hw-caps-temp-alert;
 		};
 
-		omap_control_usb: omap-control-usb@4a002300 {
+		omap_control_usb2phy: omap-control-usb@4a002300 {
 			compatible = "ti,omap-control-usb";
-			reg = <0x4a002300 0x4>,
-			      <0x4a002370 0x4>;
-			reg-names = "control_dev_conf", "phy_power_usb";
+			reg = <0x4a002300 0x4>;
+			reg-names = "power";
 			ti,type = <2>;
 		};
 
+		omap_control_usb3phy: omap-control-usb@0x4a002370 {
+			compatible = "ti,omap-control-usb";
+			reg = <0x4a002370 0x4>;
+			reg-names = "power";
+			ti,type = <3>;
+		};
+
 		omap_dwc3@4a020000 {
 			compatible = "ti,dwc3";
 			ti,hwmods = "usb_otg_ss";
@@ -661,7 +667,7 @@
 			usb2_phy: usb2phy@4a084000 {
 				compatible = "ti,omap-usb2";
 				reg = <0x4a084000 0x7c>;
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb2phy>;
 			};
 
 			usb3_phy: usb3phy@4a084400 {
@@ -670,7 +676,7 @@
 				      <0x4a084800 0x64>,
 				      <0x4a084c00 0x40>;
 				reg-names = "phy_rx", "phy_tx", "pll_ctrl";
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb3phy>;
 			};
 		};
 
-- 
1.7.4.1

Re: [PATCH 7/7] ARM: dts: omap5: update omap-control-usb node

[Roger Quadros]

+Benoit

On 08/01/2013 05:05 PM, Roger Quadros wrote:
> Split USB2 PHY and USB3 PHY into separate omap-control-usb
> nodes. Update ti,mode property.
> 
> CC: Benoit Cousson <benoit.cousson@linaro.org>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/omap5.dtsi |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 07be2cd..af8ef77 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -626,14 +626,20 @@
>  			hw-caps-temp-alert;
>  		};
>  
> -		omap_control_usb: omap-control-usb@4a002300 {
> +		omap_control_usb2phy: omap-control-usb@4a002300 {
>  			compatible = "ti,omap-control-usb";
> -			reg = <0x4a002300 0x4>,
> -			      <0x4a002370 0x4>;
> -			reg-names = "control_dev_conf", "phy_power_usb";
> +			reg = <0x4a002300 0x4>;
> +			reg-names = "power";
>  			ti,type = <2>;
>  		};
>  
> +		omap_control_usb3phy: omap-control-usb@0x4a002370 {
> +			compatible = "ti,omap-control-usb";
> +			reg = <0x4a002370 0x4>;
> +			reg-names = "power";
> +			ti,type = <3>;
> +		};
> +
>  		omap_dwc3@4a020000 {
>  			compatible = "ti,dwc3";
>  			ti,hwmods = "usb_otg_ss";
> @@ -661,7 +667,7 @@
>  			usb2_phy: usb2phy@4a084000 {
>  				compatible = "ti,omap-usb2";
>  				reg = <0x4a084000 0x7c>;
> -				ctrl-module = <&omap_control_usb>;
> +				ctrl-module = <&omap_control_usb2phy>;
>  			};
>  
>  			usb3_phy: usb3phy@4a084400 {
> @@ -670,7 +676,7 @@
>  				      <0x4a084800 0x64>,
>  				      <0x4a084c00 0x40>;
>  				reg-names = "phy_rx", "phy_tx", "pll_ctrl";
> -				ctrl-module = <&omap_control_usb>;
> +				ctrl-module = <&omap_control_usb3phy>;
>  			};
>  		};
>  
> 

Re: [PATCH 0/7] phy: omap-usb: Support multiple instances and new types

[Roger Quadros]

+Benoit

Hi Felipe,

Any comments on this series?

cheers,
-roger

On 08/01/2013 05:05 PM, Roger Quadros wrote:
> Hi,
> 
> This patchset does the following:
> 
> * Restructure and add support for new PHY types. We now support the follwing
> four types
> 
>  TYPE1 - if it has otghs_control mailbox register (e.g. on OMAP4)
>  TYPE2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY
>  TYPE3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY
>  TYPE4 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7
> 
> * Have only one power control API "omap_control_usb_phy_power()" instead of a
> different one for each PHY type.
> 
> * Get rid of omap_get_control_dev() so that we can support multiple instances
> of the control device. We take advantage of the fact that omap control USB device
> is only present on OMAP4 onwards and hence only supports DT boot. The users
> of control USB device can get a reference to it from the device node's phandle.
> 
> Patches are based on top of v3.11-rc3 with balbi/next on top.
> 
> Smoke tested on OMAP4 panda with MUSB in gadget mode (g_zero).
> 
> You can find the patches in branch
>  usb-control-module
> in git tree
>  git://github.com/rogerq/linux.git
> 
> cheers,
> -roger
> 
> Roger Quadros (7):
>   usb: phy: omap: Add new PHY types and remove
>     omap_control_usb3_phy_power()
>   usb: phy: omap-usb2: Don't use omap_get_control_dev()
>   usb: phy: omap-usb3: Don't use omap_get_control_dev()
>   usb: musb: omap2430: Don't use omap_get_control_dev()
>   usb: phy: omap: get rid of omap_get_control_dev()
>   ARM: dts: omap4: update omap-control-usb nodes
>   ARM: dts: omap5: update omap-control-usb node
> 
>  Documentation/devicetree/bindings/usb/omap-usb.txt |   23 ++--
>  arch/arm/boot/dts/omap4.dtsi                       |   17 ++-
>  arch/arm/boot/dts/omap5.dtsi                       |   18 ++-
>  drivers/usb/musb/omap2430.c                        |   22 +++-
>  drivers/usb/phy/phy-omap-control.c                 |  160 ++++++++++---------
>  drivers/usb/phy/phy-omap-usb2.c                    |   20 ++-
>  drivers/usb/phy/phy-omap-usb3.c                    |   26 +++-
>  include/linux/usb/omap_control_usb.h               |   23 +--
>  8 files changed, 182 insertions(+), 127 deletions(-)
>