Skip to contentSkip to conversation

phy: omap-usb: Support multiple instances and new types

PATCH Superseded Roger Quadros 3 participants 16 messages Aug 15, 2013 – Aug 16, 2013

Subsystems (4)

v2 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v2 cover phy: omap-usb: Support multiple instances and new types

Hi,

This patchset does the following:

* Get rid of omap_control_usb platform data as we support DT only.

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

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

* Get rid of "ti,type" DT property and introduce compatible strings for each type.

* 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 balbi/next.

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

v2:
- get rid of "ti,type" property and introduce compatible strings for each device type.

cheers,
-roger

---
Roger Quadros (8):
usb: phy: omap-control: Get rid of platform data
usb: phy: omap: Add new device 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 |   29 ++--
 arch/arm/boot/dts/omap4.dtsi                       |   17 ++-
 arch/arm/boot/dts/omap5.dtsi                       |   20 ++-
 drivers/usb/musb/omap2430.c                        |   22 ++-
 drivers/usb/phy/phy-omap-control.c                 |  189 +++++++++++---------
 drivers/usb/phy/phy-omap-usb2.c                    |   20 ++-
 drivers/usb/phy/phy-omap-usb3.c                    |   26 ++-
 include/linux/usb/omap_control_usb.h               |   33 ++---
 8 files changed, 206 insertions(+), 150 deletions(-)
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH v2 1/8 usb: phy: omap-control: Get rid of platform data

omap-control device is present from OMAP4 onwards which support device tree boots only. So get rid of platform data.

---
 drivers/usb/phy/phy-omap-control.c   |   18 +++++++-----------
 include/linux/usb/omap_control_usb.h |    4 ----
 2 files changed, 7 insertions(+), 15 deletions(-)
drivers/usb/phy/phy-omap-control.c +7 -11
index a4dda8e..3b9ee83 100644
@@ -197,8 +197,13 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 {
 	struct resource	*res;
 	struct device_node *np = pdev->dev.of_node;
-	struct omap_control_usb_platform_data *pdata =
-			dev_get_platdata(&pdev->dev);
+
+	if (np) {
+		of_property_read_u32(np, "ti,type", &control_usb->type);
+	} else {
+		/* We only support DT boot */
+		return -ENODEV;
+	}
 
 	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
 		GFP_KERNEL);
@@ -207,15 +212,6 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	if (np) {
-		of_property_read_u32(np, "ti,type", &control_usb->type);
-	} else if (pdata) {
-		control_usb->type = pdata->type;
-	} else {
-		dev_err(&pdev->dev, "no pdata present\n");
-		return -EINVAL;
-	}
-
 	control_usb->dev	= &pdev->dev;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
include/linux/usb/omap_control_usb.h +0 -4
index 27b5b8c..e2416b4 100644
@@ -31,10 +31,6 @@ struct omap_control_usb {
 	u32 type;
 };
 
-struct omap_control_usb_platform_data {
-	u8 type;
-};
-
 enum omap_control_usb_mode {
 	USB_MODE_UNDEFINED = 0,
 	USB_MODE_HOST,
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH v2 1/8 usb: phy: omap-control: Get rid of platform data

Quoted 7 lines · * Roger Quadros | 2013-08-15 16:15:05 [+0300]:
drivers/usb/phy/phy-omap-control.c +0 -0
index a4dda8e..3b9ee83 100644
@@ -197,8 +197,13 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 {
 	struct resource	*res;

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 206

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +7 -2
 	struct device_node *np = pdev->dev.of_node;
-	struct omap_control_usb_platform_data *pdata =
-			dev_get_platdata(&pdev->dev);
+
+	if (np) {
+		of_property_read_u32(np, "ti,type", &control_usb->type);
+	} else {
+		/* We only support DT boot */
+		return -ENODEV;
+	}

what about

    if (!nop)
        return -EINVAL;

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 211

Quoted 8 lines
drivers/usb/phy/phy-omap-control.c +0 -2
 	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
 		GFP_KERNEL);
@@ -207,15 +212,6 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	if (np) {
-		of_property_read_u32(np, "ti,type", &control_usb->type);

and here you shift the property to the left and remove the other lines. But then you wanted to remove that ti,type thingy but I guess this will be part of another patch then. Since you can't do everything in one patch, it is okay.

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 217

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +0 -7
-	} else if (pdata) {
-		control_usb->type = pdata->type;
-	} else {
-		dev_err(&pdev->dev, "no pdata present\n");
-		return -EINVAL;
-	}
-
 	control_usb->dev	= &pdev->dev;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,

Sebastian

#3 lore raw

PATCH v2 1/8 usb: phy: omap-control: Get rid of platform data

Hi Sebastian,

Quoted 21 lines · Sebastian Andrzej Siewior
Quoted 7 lines · * Roger Quadros | 2013-08-15 16:15:05 [+0300]:
drivers/usb/phy/phy-omap-control.c +0 -0
index a4dda8e..3b9ee83 100644
@@ -197,8 +197,13 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 {
 	struct resource	*res;

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 206

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +7 -2
 	struct device_node *np = pdev->dev.of_node;
-	struct omap_control_usb_platform_data *pdata =
-			dev_get_platdata(&pdev->dev);
+
+	if (np) {
+		of_property_read_u32(np, "ti,type", &control_usb->type);
+	} else {
+		/* We only support DT boot */
+		return -ENODEV;
+	}

what about

Quoted 2 lines
    if (!nop)
        return -EINVAL;

OK, I can change it to -EINVAL.

Quoted 7 lines
Quoted 7 lines
drivers/usb/phy/phy-omap-control.c +0 -0
 	control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
 		GFP_KERNEL);
@@ -207,15 +212,6 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
Quoted 7 lines

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 211

Quoted 2 lines
drivers/usb/phy/phy-omap-control.c +0 -2
-	if (np) {
-		of_property_read_u32(np, "ti,type", &control_usb->type);

and here you shift the property to the left and remove the other lines. But then you wanted to remove that ti,type thingy but I guess this will be part of another patch then. Since you can't do everything in one patch, it is okay.

I've actually moved the ti,type stuff on the top. So there is no functional change other than getting rid of platform data.

Quoted 10 lines
Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +0 -7
-	} else if (pdata) {
-		control_usb->type = pdata->type;
-	} else {
-		dev_err(&pdev->dev, "no pdata present\n");
-		return -EINVAL;
-	}
-
 	control_usb->dev	= &pdev->dev;
 
Quoted 2 lines

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 217

Quoted 1 line
drivers/usb/phy/phy-omap-control.c +0 -0
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,

cheers,
-roger

#4 lore raw

PATCH v2 2/8 usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()

In reply to Roger Quadros

Add support for new device types and in the process rid of "ti,type" device tree property. The correct type of device will be determined from the compatible string instead.

Introduce a compatible string for each device type. At the moment we support 4 types Mailbox, USB2, USB3 and DRA7.

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.

---
 Documentation/devicetree/bindings/usb/omap-usb.txt |   29 ++--
 drivers/usb/phy/phy-omap-control.c                 |  148 ++++++++++++--------
 drivers/usb/phy/phy-omap-usb2.c                    |    4 +
 drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
 include/linux/usb/omap_control_usb.h               |   24 ++--
 5 files changed, 122 insertions(+), 89 deletions(-)
Documentation/devicetree/bindings/usb/omap-usb.txt +15 -14
index 57e71f6..1c6b54a 100644
@@ -73,22 +73,23 @@ omap_dwc3 {
 OMAP CONTROL USB
 
 Required properties:
- - compatible: Should be "ti,omap-control-usb"
+ - compatible: Should be one of
+ "ti,omap-control-usb" - if it has otghs_control mailbox register
+			e.g. on OMAP4.
+ "ti,usb2-control-usb" - if it has Power down bit in control_dev_conf register
+			e.g. USB2_PHY on OMAP5.
+ "ti,usb3-control-usb" - if it has DPLL and individual Rx & Tx power control
+			e.g. USB3 PHY and SATA PHY on OMAP5.
+ "ti,dra7-control-usb" - if it has both power down and power aux registers
+			e.g. USB2 PHY on DRA7 platform.
+
  - 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.
 
 omap_control_usb: omap-control-usb at 4a002300 {
 	compatible = "ti,omap-control-usb";
-	reg = <0x4a002300 0x4>,
-	      <0x4a00233c 0x4>;
-	reg-names = "control_dev_conf", "otghs_control";
-	ti,type = <1>;
+	reg = <0x4a00233c 0x4>;
+	reg-names = "otghs_control";
 };
drivers/usb/phy/phy-omap-control.c +88 -60

208 lines not on this page · raw message

drivers/usb/phy/phy-omap-usb2.c +4 -0
index 844ab68..376b367 100644
@@ -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) {
drivers/usb/phy/phy-omap-usb3.c +3 -3
index fc15694..4a7f27c 100644
@@ -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);
include/linux/usb/omap_control_usb.h +12 -12
index e2416b4..9eb6f9ca 100644
@@ -19,16 +19,23 @@
 #ifndef __OMAP_CONTROL_USB_H__
 #define __OMAP_CONTROL_USB_H__
 
+enum omap_control_usb_type {
+	OMAP_CTRL_TYPE_OMAP = 1,	/* Mailbox OTGHS_CONTROL */
+	OMAP_CTRL_TYPE_USB2,	/* USB2_PHY, power down in CONTROL_DEV_CONF */
+	OMAP_CTRL_TYPE_USB3,	/* USB3_PHY, DPLL & seperate Rx/Tx power */
+	OMAP_CTRL_TYPE_DRA7,	/* USB2 PHY, power and power_aux e.g. DRA7 */
+};
+
 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;
 
-	u32 type;
+	enum omap_control_usb_type type;
 };
 
 enum omap_control_usb_mode {
@@ -38,10 +45,6 @@ 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_PHY_PD		BIT(0)
 
 #define	OMAP_CTRL_DEV_AVALID		BIT(0)
@@ -59,10 +62,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
@@ -75,10 +79,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)
 {
Signature · 1 line
1.7.4.1
#5 lore raw

PATCH v2 2/8 usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()

Hi Roger,

Quoted 26 lines · Roger Quadros

Add support for new device types and in the process rid of "ti,type" device tree property. The correct type of device will be determined from the compatible string instead.

Introduce a compatible string for each device type. At the moment we support 4 types Mailbox, USB2, USB3 and DRA7.

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.

---
  Documentation/devicetree/bindings/usb/omap-usb.txt |   29 ++--
  drivers/usb/phy/phy-omap-control.c                 |  148 ++++++++++++--------
  drivers/usb/phy/phy-omap-usb2.c                    |    4 +
  drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
  include/linux/usb/omap_control_usb.h               |   24 ++--
  5 files changed, 122 insertions(+), 89 deletions(-)
Documentation/devicetree/bindings/usb/omap-usb.txt +0 -0
index 57e71f6..1c6b54a 100644

Documentation/devicetree/bindings/usb/omap-usb.txt omap_dwc3 { · line 78

Quoted 7 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +3 -1
@@ -73,22 +73,23 @@ omap_dwc3 {
  OMAP CONTROL USB
 
  Required properties:
- - compatible: Should be "ti,omap-control-usb"
+ - compatible: Should be one of
+ "ti,omap-control-usb" - if it has otghs_control mailbox register
+			e.g. on OMAP4.

How generic is that one? I mean if this is applicable for OMAP4 only, you'd better name it omap4-control-usb.

Quoted 10 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +7 -2
+ "ti,usb2-control-usb" - if it has Power down bit in control_dev_conf register
+			e.g. USB2_PHY on OMAP5.
+ "ti,usb3-control-usb" - if it has DPLL and individual Rx & Tx power control
+			e.g. USB3 PHY and SATA PHY on OMAP5.
+ "ti,dra7-control-usb" - if it has both power down and power aux registers
+			e.g. USB2 PHY on DRA7 platform.
+
   - 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.

Documentation/devicetree/bindings/usb/omap-usb.txt omap_dwc3 { · line 89

Quoted 10 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +3 -7
- - 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.

type1 and 4 are less obvious now that you have name, you should be more explicit and name them directly.

Quoted 161 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +2 -4
  omap_control_usb: omap-control-usb at 4a002300 {
  	compatible = "ti,omap-control-usb";
-	reg = <0x4a002300 0x4>,
-	      <0x4a00233c 0x4>;
-	reg-names = "control_dev_conf", "otghs_control";
-	ti,type = <1>;
+	reg = <0x4a00233c 0x4>;
+	reg-names = "otghs_control";
  };
drivers/usb/phy/phy-omap-control.c +60 -47
index 3b9ee83..078c46f 100644
@@ -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_TYPE_OMAP)
+		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_TYPE_USB2:
+		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_TYPE_USB3:
+		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_TYPE_DRA7:
+		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);
 
@@ -172,7 +187,7 @@ 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) || control_usb->type != OMAP_CTRL_TYPE_OMAP)
  		return;
 
  	ctrl_usb = dev_get_drvdata(dev);
@@ -198,9 +213,7 @@ static int omap_control_usb_probe(struct platform_device *pdev)
  	struct resource	*res;
  	struct device_node *np = pdev->dev.of_node;
 
-	if (np) {
-		of_property_read_u32(np, "ti,type", &control_usb->type);
-	} else {
+	if (!np) {
  		/* We only support DT boot */
  		return -ENODEV;
  	}
@@ -212,31 +225,34 @@ static int omap_control_usb_probe(struct platform_device *pdev)
  		return -ENOMEM;
  	}
 
-	control_usb->dev	= &pdev->dev;
+	control_usb->dev = &pdev->dev;
+	control_usb->type = OMAP_CTRL_TYPE_OMAP;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 236

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +6 -4
-		"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 (of_device_is_compatible(np, "ti,usb2-control-usb"))
+		control_usb->type = OMAP_CTRL_TYPE_USB2;
+	else if (of_device_is_compatible(np, "ti,usb3-control-usb"))
+		control_usb->type = OMAP_CTRL_TYPE_USB3;
+	else if (of_device_is_compatible(np, "ti,dra7-control-usb"))
+		control_usb->type = OMAP_CTRL_TYPE_DRA7;

You can avoid that by adding the type directly in the data field of the omap_control_usb_id_table.
It will be more readable and scalable.

This how it was done on gpio-omap for example.

Quoted 48 lines
drivers/usb/phy/phy-omap-control.c +19 -9
-	if (control_usb->type == OMAP_CTRL_DEV_TYPE1) {
+	if (control_usb->type == OMAP_CTRL_TYPE_OMAP) {
  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
  			"otghs_control");
  		control_usb->otghs_control = devm_ioremap_resource(
  			&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_TYPE_USB3) {
  		control_usb->sys_clk = devm_clk_get(control_usb->dev,
  			"sys_clkin");
  		if (IS_ERR(control_usb->sys_clk)) {
@@ -245,6 +261,15 @@ static int omap_control_usb_probe(struct platform_device *pdev)
  		}
  	}
 
+	if (control_usb->type == OMAP_CTRL_TYPE_DRA7) {
+		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);
 

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 285

Quoted 7 lines
drivers/usb/phy/phy-omap-control.c +3 -0
@@ -254,6 +279,9 @@ static int omap_control_usb_probe(struct platform_device *pdev)
  #ifdef CONFIG_OF
  static const struct of_device_id omap_control_usb_id_table[] = {
  	{ .compatible = "ti,omap-control-usb" },
+	{ .compatible = "ti,usb2-control-usb" },
+	{ .compatible = "ti,usb3-control-usb" },
+	{ .compatible = "ti,dra7-control-usb" },
  	{}

In that table you can add a data field with the proper type enum.

Regards,
Benoit

Quoted 115 lines
drivers/usb/phy/phy-omap-control.c +0 -0
  };
  MODULE_DEVICE_TABLE(of, omap_control_usb_id_table);
drivers/usb/phy/phy-omap-usb2.c +4 -0
index 844ab68..376b367 100644
@@ -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) {
drivers/usb/phy/phy-omap-usb3.c +3 -3
index fc15694..4a7f27c 100644
@@ -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);
include/linux/usb/omap_control_usb.h +12 -12
index e2416b4..9eb6f9ca 100644
@@ -19,16 +19,23 @@
  #ifndef __OMAP_CONTROL_USB_H__
  #define __OMAP_CONTROL_USB_H__
 
+enum omap_control_usb_type {
+	OMAP_CTRL_TYPE_OMAP = 1,	/* Mailbox OTGHS_CONTROL */
+	OMAP_CTRL_TYPE_USB2,	/* USB2_PHY, power down in CONTROL_DEV_CONF */
+	OMAP_CTRL_TYPE_USB3,	/* USB3_PHY, DPLL & seperate Rx/Tx power */
+	OMAP_CTRL_TYPE_DRA7,	/* USB2 PHY, power and power_aux e.g. DRA7 */
+};
+
  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;
 
-	u32 type;
+	enum omap_control_usb_type type;
  };
 
  enum omap_control_usb_mode {
@@ -38,10 +45,6 @@ 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_PHY_PD		BIT(0)
 
  #define	OMAP_CTRL_DEV_AVALID		BIT(0)
@@ -59,10 +62,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
@@ -75,10 +79,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)
  {
#6 lore raw

PATCH v2 2/8 usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()

Hi Benoit,

Quoted 31 lines · Benoit Cousson

Hi Roger,

Quoted 28 lines · Roger Quadros

Add support for new device types and in the process rid of "ti,type" device tree property. The correct type of device will be determined from the compatible string instead.

Introduce a compatible string for each device type. At the moment we support 4 types Mailbox, USB2, USB3 and DRA7.

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.

---
  Documentation/devicetree/bindings/usb/omap-usb.txt |   29 ++--
  drivers/usb/phy/phy-omap-control.c                 |  148 ++++++++++++--------
  drivers/usb/phy/phy-omap-usb2.c                    |    4 +
  drivers/usb/phy/phy-omap-usb3.c                    |    6 +-
  include/linux/usb/omap_control_usb.h               |   24 ++--
  5 files changed, 122 insertions(+), 89 deletions(-)
Documentation/devicetree/bindings/usb/omap-usb.txt +0 -0
index 57e71f6..1c6b54a 100644
@@ -73,22 +73,23 @@ omap_dwc3 {
  OMAP CONTROL USB
 
Quoted 8 lines

Documentation/devicetree/bindings/usb/omap-usb.txt omap_dwc3 { · line 78

Quoted 5 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +3 -1
  Required properties:
- - compatible: Should be "ti,omap-control-usb"
+ - compatible: Should be one of
+ "ti,omap-control-usb" - if it has otghs_control mailbox register
+            e.g. on OMAP4.

How generic is that one? I mean if this is applicable for OMAP4 only, you'd better name it omap4-control-usb.

AFAIK it is OMAP4 only so i'll change it to omap4-control-usb.

Quoted 21 lines
Quoted 10 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +7 -2
+ "ti,usb2-control-usb" - if it has Power down bit in control_dev_conf register
+            e.g. USB2_PHY on OMAP5.
+ "ti,usb3-control-usb" - if it has DPLL and individual Rx & Tx power control
+            e.g. USB3 PHY and SATA PHY on OMAP5.
+ "ti,dra7-control-usb" - if it has both power down and power aux registers
+            e.g. USB2 PHY on DRA7 platform.
+
   - 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.

Documentation/devicetree/bindings/usb/omap-usb.txt omap_dwc3 { · line 89

Quoted 10 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +3 -7
- - 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.
Quoted 1 line

type1 and 4 are less obvious now that you have name, you should be more explicit and name them directly.

OK, I'll fix it.

Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/usb/omap-usb.txt +2 -4
  omap_control_usb: omap-control-usb at 4a002300 {
      compatible = "ti,omap-control-usb";
-    reg = <0x4a002300 0x4>,
-          <0x4a00233c 0x4>;
-    reg-names = "control_dev_conf", "otghs_control";
-    ti,type = <1>;
+    reg = <0x4a00233c 0x4>;
+    reg-names = "otghs_control";
  };
drivers/usb/phy/phy-omap-control.c +0 -0
index 3b9ee83..078c46f 100644
@@ -46,61 +46,76 @@ struct device *omap_get_control_dev(void)
  EXPORT_SYMBOL_GPL(omap_get_control_dev);
 

...

Quoted 17 lines
Quoted 6 lines
drivers/usb/phy/phy-omap-control.c +2 -2
-    control_usb->dev    = &pdev->dev;
+    control_usb->dev = &pdev->dev;
+    control_usb->type = OMAP_CTRL_TYPE_OMAP;
 
-    res = platform_get_resource_byname(pdev, IORESOURCE_MEM,

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 236

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +6 -4
-        "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 (of_device_is_compatible(np, "ti,usb2-control-usb"))
+        control_usb->type = OMAP_CTRL_TYPE_USB2;
+    else if (of_device_is_compatible(np, "ti,usb3-control-usb"))
+        control_usb->type = OMAP_CTRL_TYPE_USB3;
+    else if (of_device_is_compatible(np, "ti,dra7-control-usb"))
+        control_usb->type = OMAP_CTRL_TYPE_DRA7;
Quoted 4 lines

You can avoid that by adding the type directly in the data field of the omap_control_usb_id_table. It will be more readable and scalable.

This how it was done on gpio-omap for example.

OK. Thanks for the hint.

Quoted 48 lines
Quoted 48 lines
drivers/usb/phy/phy-omap-control.c +19 -9
-    if (control_usb->type == OMAP_CTRL_DEV_TYPE1) {
+    if (control_usb->type == OMAP_CTRL_TYPE_OMAP) {
          res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
              "otghs_control");
          control_usb->otghs_control = devm_ioremap_resource(
              &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_TYPE_USB3) {
          control_usb->sys_clk = devm_clk_get(control_usb->dev,
              "sys_clkin");
          if (IS_ERR(control_usb->sys_clk)) {
@@ -245,6 +261,15 @@ static int omap_control_usb_probe(struct platform_device *pdev)
          }
      }
 
+    if (control_usb->type == OMAP_CTRL_TYPE_DRA7) {
+        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);
 
Quoted 10 lines

drivers/usb/phy/phy-omap-control.c static int omap_control_usb_probe(struct platform_device *pdev) · line 285

Quoted 8 lines
drivers/usb/phy/phy-omap-control.c +3 -0
@@ -254,6 +279,9 @@ static int omap_control_usb_probe(struct platform_device *pdev)
  #ifdef CONFIG_OF
  static const struct of_device_id omap_control_usb_id_table[] = {
      { .compatible = "ti,omap-control-usb" },
+    { .compatible = "ti,usb2-control-usb" },
+    { .compatible = "ti,usb3-control-usb" },
+    { .compatible = "ti,dra7-control-usb" },
      {}

In that table you can add a data field with the proper type enum.

got it.

cheers,
-roger

#7 lore raw

PATCH v2 3/8 usb: phy: omap-usb2: Don't use omap_get_control_dev()

In reply to 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.

---
 drivers/usb/phy/phy-omap-usb2.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)
drivers/usb/phy/phy-omap-usb2.c +13 -3
index 376b367..77e0cf4 100644
@@ -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);
Signature · 1 line
1.7.4.1
#8 lore raw

PATCH v2 4/8 usb: phy: omap-usb3: Don't use omap_get_control_dev()

In reply to 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.

---
 drivers/usb/phy/phy-omap-usb3.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)
drivers/usb/phy/phy-omap-usb3.c +17 -3
index 4a7f27c..bad032e 100644
@@ -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);
Signature · 1 line
1.7.4.1
#9 lore raw

PATCH v2 5/8 usb: musb: omap2430: Don't use omap_get_control_dev()

In reply to 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.

---
 drivers/usb/musb/omap2430.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)
drivers/usb/musb/omap2430.c +18 -4
index ebb46ec..1db9588 100644
@@ -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);
 	}
Signature · 1 line
1.7.4.1
#10 lore raw

PATCH v2 6/8 usb: phy: omap: get rid of omap_get_control_dev()

In reply to 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.

---
 drivers/usb/phy/phy-omap-control.c   |   31 ++++++++++---------------------
 include/linux/usb/omap_control_usb.h |    5 -----
 2 files changed, 10 insertions(+), 26 deletions(-)
drivers/usb/phy/phy-omap-control.c +10 -21
index 078c46f..d144c14 100644
@@ -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_TYPE_OMAP)
+	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_TYPE_OMAP)
+		return;
+
 	switch (mode) {
 	case USB_MODE_HOST:
 		omap_control_usb_host_mode(ctrl_usb);
@@ -212,6 +200,7 @@ static int omap_control_usb_probe(struct platform_device *pdev)
 {
 	struct resource	*res;
 	struct device_node *np = pdev->dev.of_node;
+	struct omap_control_usb *control_usb;
 
 	if (!np) {
 		/* We only support DT boot */
include/linux/usb/omap_control_usb.h +0 -5
index 9eb6f9ca..aa41bd5 100644
@@ -65,15 +65,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)
 {
Signature · 1 line
1.7.4.1
#11 lore raw

PATCH v2 6/8 usb: phy: omap: get rid of omap_get_control_dev()

Quoted 12 lines · * Roger Quadros | 2013-08-15 16:15:10 [+0300]:
drivers/usb/phy/phy-omap-control.c +1 -1
index 078c46f..d144c14 100644
@@ -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_TYPE_OMAP)
+	if (IS_ERR(dev) || !dev)
 		return;
 

drivers/usb/phy/phy-omap-control.c void omap_control_usb_set_mode(struct device *dev, · line 182

Quoted 10 lines
drivers/usb/phy/phy-omap-control.c +8 -0
 	ctrl_usb = dev_get_drvdata(dev);
 
+	if (!ctrl_usb) {
+		dev_err(dev, "Invalid control usb device\n");
+		return;
+	}
+
+	if (ctrl_usb->type != OMAP_CTRL_TYPE_OMAP)
+		return;
+

I don't like that part where you can call this function even before the driver probed the device. This shouldn't happen since you have hard module dependency and the proper compatible string in your device. But this kind of thing won't happen in the am335x reset driver.

Sebastian

#12 lore raw

PATCH v2 6/8 usb: phy: omap: get rid of omap_get_control_dev()

Quoted 16 lines · Sebastian Andrzej Siewior
Quoted 14 lines · * Roger Quadros | 2013-08-15 16:15:10 [+0300]:
drivers/usb/phy/phy-omap-control.c +1 -1
index 078c46f..d144c14 100644
@@ -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_TYPE_OMAP)
+	if (IS_ERR(dev) || !dev)
 		return;
 
 	ctrl_usb = dev_get_drvdata(dev);
 
Quoted 13 lines

drivers/usb/phy/phy-omap-control.c void omap_control_usb_set_mode(struct device *dev, · line 182

Quoted 8 lines
drivers/usb/phy/phy-omap-control.c +8 -0
+	if (!ctrl_usb) {
+		dev_err(dev, "Invalid control usb device\n");
+		return;
+	}
+
+	if (ctrl_usb->type != OMAP_CTRL_TYPE_OMAP)
+		return;
+

I don't like that part where you can call this function even before the driver probed the device. This shouldn't happen since you have hard module dependency and the proper compatible string in your device. But this kind of thing won't happen in the am335x reset driver.

I don't like that part either, but omap_control_usb_set_mode is being called from outside this driver (e.g. musb) and we are not making any changes to that behaviour in this patch.

I think the issue you mentioned is being fixed by Kishon's extcon migration series.

cheers,
-roger

#13 lore raw

PATCH v2 6/8 usb: phy: omap: get rid of omap_get_control_dev()

Quoted 10 lines · Roger Quadros
Quoted 4 lines

I don't like that part where you can call this function even before the driver probed the device. This shouldn't happen since you have hard module dependency and the proper compatible string in your device. But this kind of thing won't happen in the am335x reset driver.

I don't like that part either, but omap_control_usb_set_mode is being called from outside this driver (e.g. musb) and we are not making any changes to that behaviour in this patch.

I think the issue you mentioned is being fixed by Kishon's extcon migration series.

Okay then.

Quoted 4 lines

cheers,
-roger

Sebastian

#14 lore raw

PATCH v2 7/8 ARM: dts: omap4: update omap-control-usb nodes

In reply to Roger Quadros

Split otghs_ctrl and USB2 PHY power down into separate omap-control-usb nodes. Get rid of "ti,type" property.

---
 arch/arm/boot/dts/omap4.dtsi |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)
arch/arm/boot/dts/omap4.dtsi +11 -6
index 22d9f2b..ca08cf2 100644
@@ -519,7 +519,7 @@
 			usb2_phy: usb2phy at 4a0ad080 {
 				compatible = "ti,omap-usb2";
 				reg = <0x4a0ad080 0x58>;
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb2phy>;
 			};
 		};
 
@@ -643,12 +643,16 @@
 			};
 		};
 
-		omap_control_usb: omap-control-usb at 4a002300 {
+		omap_control_usb2phy: omap-control-usb at 4a002300 {
+			compatible = "ti,usb2-control-usb";
+			reg = <0x4a002300 0x4>;
+			reg-names = "power";
+		};
+
+		omap_control_usbotg: omap-control-usb at 4a00233c {
 			compatible = "ti,omap-control-usb";
-			reg = <0x4a002300 0x4>,
-			      <0x4a00233c 0x4>;
-			reg-names = "control_dev_conf", "otghs_control";
-			ti,type = <1>;
+			reg = <0x4a00233c 0x4>;
+			reg-names = "otghs_control";
 		};
 
 		usb_otg_hs: usb_otg_hs at 4a0ab000 {
@@ -662,6 +666,7 @@
 			num-eps = <16>;
 			ram-bits = <12>;
 			ti,has-mailbox;
+			ctrl-module = <&omap_control_usbotg>;
 		};
 	};
 };
Signature · 1 line
1.7.4.1
#15 lore raw

PATCH v2 8/8 ARM: dts: omap5: update omap-control-usb node

In reply to Roger Quadros

Split USB2 PHY and USB3 PHY into separate omap-control-usb nodes. Get rid of "ti,type" property.

---
 arch/arm/boot/dts/omap5.dtsi |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)
arch/arm/boot/dts/omap5.dtsi +12 -8
index 07be2cd..7cda18b 100644
@@ -626,12 +626,16 @@
 			hw-caps-temp-alert;
 		};
 
-		omap_control_usb: omap-control-usb at 4a002300 {
-			compatible = "ti,omap-control-usb";
-			reg = <0x4a002300 0x4>,
-			      <0x4a002370 0x4>;
-			reg-names = "control_dev_conf", "phy_power_usb";
-			ti,type = <2>;
+		omap_control_usb2phy: omap-control-usb at 4a002300 {
+			compatible = "ti,usb2-control-usb";
+			reg = <0x4a002300 0x4>;
+			reg-names = "power";
+		};
+
+		omap_control_usb3phy: omap-control-usb at 0x4a002370 {
+			compatible = "ti,usb3-control-usb";
+			reg = <0x4a002370 0x4>;
+			reg-names = "power";
 		};
 
 		omap_dwc3 at 4a020000 {
@@ -661,7 +665,7 @@
 			usb2_phy: usb2phy at 4a084000 {
 				compatible = "ti,omap-usb2";
 				reg = <0x4a084000 0x7c>;
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb2phy>;
 			};
 
 			usb3_phy: usb3phy at 4a084400 {
@@ -670,7 +674,7 @@
 				      <0x4a084800 0x64>,
 				      <0x4a084c00 0x40>;
 				reg-names = "phy_rx", "phy_tx", "pll_ctrl";
-				ctrl-module = <&omap_control_usb>;
+				ctrl-module = <&omap_control_usb3phy>;
 			};
 		};
 
Signature · 1 line
1.7.4.1