[PATCH 0/7] phy: use syscon framework APIs to set ctrl mod reg

[PATCH 0/7] phy: use syscon framework APIs to set ctrl mod reg

[Kishon Vijay Abraham I]

This series is split from [1] to include only the PHY patches.

This series is basically to deprecate using phy-omap-control and use
syscon APIs to program the control module registers.

Changes from [1] in PHY patches include
*) cleanup ti_pipe3_probe
*) have mask, power_on and power_off values in usb_phy_data for
   omap-usb2 phy

Did basic enumeration testing in the below platforms.
*) Tested PCIe, SATA and USB in dra7
*) Tested SATA and USB in omap5
*) Tested USB(dwc3) in am43xx_evm
*) Tested USB(musb) in omap4 panda after including [2]

All the testing was done both before applying the dt patches and after
applying the dt patches (dt patches will be posted shortly).

[1] -> https://lkml.org/lkml/2015/6/23/189
[2] -> http://permalink.gmane.org/gmane.linux.kernel/2012427

Kishon Vijay Abraham I (7):
  phy: ti-pipe3: cleanup ti_pipe3_probe()
  phy: ti-pipe3: use ti_pipe3_power_off to power off the PHY during
    probe
  phy: ti-pipe3: use *syscon* framework API to power on/off the PHY
  phy: ti-pipe3: use *syscon* framework API to set PCS value of the PHY
  phy: omap-usb2: use omap_usb_power_off to power off the PHY during
    probe
  phy: omap-usb2: Add a new compatible string for USB2 PHY2
  phy: omap-usb2: use *syscon* framework API to power on/off the PHY

 Documentation/devicetree/bindings/phy/ti-phy.txt |   20 +-
 drivers/phy/phy-omap-usb2.c                      |   96 ++++++--
 drivers/phy/phy-ti-pipe3.c                       |  283 ++++++++++++++++------
 include/linux/phy/omap_usb.h                     |   23 ++
 4 files changed, 329 insertions(+), 93 deletions(-)

-- 
1.7.9.5

[PATCH 1/7] phy: ti-pipe3: cleanup ti_pipe3_probe()

[Kishon Vijay Abraham I]

No functional change. Add separate functions for pll,
clocks and syscon to make ti_pipe3_probe clean.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/phy/phy-ti-pipe3.c |  165 ++++++++++++++++++++++++++++----------------
 1 file changed, 104 insertions(+), 61 deletions(-)

diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index 08020dc..072d308 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -308,51 +308,45 @@ static struct phy_ops ops = {
 
 static const struct of_device_id ti_pipe3_id_table[];
 
-static int ti_pipe3_probe(struct platform_device *pdev)
+static int ti_pipe3_get_pll_base(struct ti_pipe3 *phy)
 {
-	struct ti_pipe3 *phy;
-	struct phy *generic_phy;
-	struct phy_provider *phy_provider;
 	struct resource *res;
-	struct device_node *node = pdev->dev.of_node;
-	struct device_node *control_node;
-	struct platform_device *control_pdev;
 	const struct of_device_id *match;
-	struct clk *clk;
+	struct device *dev = phy->dev;
+	struct device_node *node = dev->of_node;
+	struct platform_device *pdev = to_platform_device(dev);
 
-	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
-	if (!phy)
-		return -ENOMEM;
+	if (of_device_is_compatible(node, "ti,phy-pipe3-pcie"))
+		return 0;
 
-	phy->dev		= &pdev->dev;
+	match = of_match_device(ti_pipe3_id_table, dev);
+	if (!match)
+		return -EINVAL;
 
-	if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
-		match = of_match_device(ti_pipe3_id_table, &pdev->dev);
-		if (!match)
-			return -EINVAL;
+	phy->dpll_map = (struct pipe3_dpll_map *)match->data;
+	if (!phy->dpll_map) {
+		dev_err(dev, "no DPLL data\n");
+		return -EINVAL;
+	}
 
-		phy->dpll_map = (struct pipe3_dpll_map *)match->data;
-		if (!phy->dpll_map) {
-			dev_err(&pdev->dev, "no DPLL data\n");
-			return -EINVAL;
-		}
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   "pll_ctrl");
+	phy->pll_ctrl_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(phy->pll_ctrl_base))
+		return PTR_ERR(phy->pll_ctrl_base);
 
-		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   "pll_ctrl");
-		phy->pll_ctrl_base = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(phy->pll_ctrl_base))
-			return PTR_ERR(phy->pll_ctrl_base);
+	return 0;
+}
 
-		phy->sys_clk = devm_clk_get(phy->dev, "sysclk");
-		if (IS_ERR(phy->sys_clk)) {
-			dev_err(&pdev->dev, "unable to get sysclk\n");
-			return -EINVAL;
-		}
-	}
+static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
+{
+	struct clk *clk;
+	struct device *dev = phy->dev;
+	struct device_node *node = dev->of_node;
 
-	phy->refclk = devm_clk_get(phy->dev, "refclk");
+	phy->refclk = devm_clk_get(dev, "refclk");
 	if (IS_ERR(phy->refclk)) {
-		dev_err(&pdev->dev, "unable to get refclk\n");
+		dev_err(dev, "unable to get refclk\n");
 		/* older DTBs have missing refclk in SATA PHY
 		 * so don't bail out in case of SATA PHY.
 		 */
@@ -361,76 +355,125 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	}
 
 	if (!of_device_is_compatible(node, "ti,phy-pipe3-sata")) {
-		phy->wkupclk = devm_clk_get(phy->dev, "wkupclk");
+		phy->wkupclk = devm_clk_get(dev, "wkupclk");
 		if (IS_ERR(phy->wkupclk)) {
-			dev_err(&pdev->dev, "unable to get wkupclk\n");
+			dev_err(dev, "unable to get wkupclk\n");
 			return PTR_ERR(phy->wkupclk);
 		}
 	} else {
 		phy->wkupclk = ERR_PTR(-ENODEV);
-		phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node,
-							"syscon-pllreset");
-		if (IS_ERR(phy->dpll_reset_syscon)) {
-			dev_info(&pdev->dev,
-				 "can't get syscon-pllreset, sata dpll won't idle\n");
-			phy->dpll_reset_syscon = NULL;
-		} else {
-			if (of_property_read_u32_index(node,
-						       "syscon-pllreset", 1,
-						       &phy->dpll_reset_reg)) {
-				dev_err(&pdev->dev,
-					"couldn't get pllreset reg. offset\n");
-				return -EINVAL;
-			}
+	}
+
+	if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
+		phy->sys_clk = devm_clk_get(dev, "sysclk");
+		if (IS_ERR(phy->sys_clk)) {
+			dev_err(dev, "unable to get sysclk\n");
+			return -EINVAL;
 		}
 	}
 
 	if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
-
-		clk = devm_clk_get(phy->dev, "dpll_ref");
+		clk = devm_clk_get(dev, "dpll_ref");
 		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "unable to get dpll ref clk\n");
+			dev_err(dev, "unable to get dpll ref clk\n");
 			return PTR_ERR(clk);
 		}
 		clk_set_rate(clk, 1500000000);
 
-		clk = devm_clk_get(phy->dev, "dpll_ref_m2");
+		clk = devm_clk_get(dev, "dpll_ref_m2");
 		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "unable to get dpll ref m2 clk\n");
+			dev_err(dev, "unable to get dpll ref m2 clk\n");
 			return PTR_ERR(clk);
 		}
 		clk_set_rate(clk, 100000000);
 
-		clk = devm_clk_get(phy->dev, "phy-div");
+		clk = devm_clk_get(dev, "phy-div");
 		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "unable to get phy-div clk\n");
+			dev_err(dev, "unable to get phy-div clk\n");
 			return PTR_ERR(clk);
 		}
 		clk_set_rate(clk, 100000000);
 
-		phy->div_clk = devm_clk_get(phy->dev, "div-clk");
+		phy->div_clk = devm_clk_get(dev, "div-clk");
 		if (IS_ERR(phy->div_clk)) {
-			dev_err(&pdev->dev, "unable to get div-clk\n");
+			dev_err(dev, "unable to get div-clk\n");
 			return PTR_ERR(phy->div_clk);
 		}
 	} else {
 		phy->div_clk = ERR_PTR(-ENODEV);
 	}
 
+	return 0;
+}
+
+static int ti_pipe3_get_sysctrl(struct ti_pipe3 *phy)
+{
+	struct device *dev = phy->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *control_node;
+	struct platform_device *control_pdev;
+
 	control_node = of_parse_phandle(node, "ctrl-module", 0);
 	if (!control_node) {
-		dev_err(&pdev->dev, "Failed to get control device phandle\n");
+		dev_err(dev, "Failed to get control device phandle\n");
 		return -EINVAL;
 	}
 
 	control_pdev = of_find_device_by_node(control_node);
 	if (!control_pdev) {
-		dev_err(&pdev->dev, "Failed to get control device\n");
+		dev_err(dev, "Failed to get control device\n");
 		return -EINVAL;
 	}
 
 	phy->control_dev = &control_pdev->dev;
 
+	if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) {
+		phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node,
+							"syscon-pllreset");
+		if (IS_ERR(phy->dpll_reset_syscon)) {
+			dev_info(dev,
+				 "can't get syscon-pllreset, sata dpll won't idle\n");
+			phy->dpll_reset_syscon = NULL;
+		} else {
+			if (of_property_read_u32_index(node, "syscon-pllreset",
+						       1,
+						       &phy->dpll_reset_reg)) {
+				dev_err(dev,
+					"can't get pllreset reg. offset\n");
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int ti_pipe3_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct ti_pipe3 *phy;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+	struct device_node *node = pdev->dev.of_node;
+
+	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->dev		= &pdev->dev;
+
+	ret = ti_pipe3_get_pll_base(phy);
+	if (ret)
+		return ret;
+
+	ret = ti_pipe3_get_sysctrl(phy);
+	if (ret)
+		return ret;
+
+	ret = ti_pipe3_get_clk(phy);
+	if (ret)
+		return ret;
+
 	omap_control_phy_power(phy->control_dev, 0);
 
 	platform_set_drvdata(pdev, phy);
-- 
1.7.9.5

Re: [PATCH 1/7] phy: ti-pipe3: cleanup ti_pipe3_probe()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

On Tue, Aug 04, 2015 at 08:50:40PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Add separate functions for pll,
> clocks and syscon to make ti_pipe3_probe clean.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>

I think this needs to be splitted into smaller patches.

Seems like the very first patch would be to introduce a local struct
device pointer to be used. Than add each and every new function on their
own patch. This will help big time making your refactoring a lot clearer
and, should anything go wrong, we can atomically revert a specific
change, instead of loosing the entire refactor.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/7] phy: ti-pipe3: cleanup ti_pipe3_probe()

[Kishon Vijay Abraham I]

Hi,

On Tuesday 04 August 2015 09:30 PM, Felipe Balbi wrote:
> On Tue, Aug 04, 2015 at 08:50:40PM +0530, Kishon Vijay Abraham I wrote:
>> No functional change. Add separate functions for pll,
>> clocks and syscon to make ti_pipe3_probe clean.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> 
> I think this needs to be splitted into smaller patches.

yeah sure.

Thanks
Kishon

> 
> Seems like the very first patch would be to introduce a local struct
> device pointer to be used. Than add each and every new function on their
> own patch. This will help big time making your refactoring a lot clearer
> and, should anything go wrong, we can atomically revert a specific
> change, instead of loosing the entire refactor.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] phy: ti-pipe3: use ti_pipe3_power_off to power off the PHY during probe

[Kishon Vijay Abraham I]

No functional change. Previously omap_control_phy_power() was used to power
off the PHY during probe. But once PIPE3 driver is adapted to use syscon,
omap_control_phy_power() cannot be used. Hence used ti_pipe3_power_off
to power off the PHY.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-ti-pipe3.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index 072d308..9782c16 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -474,8 +474,6 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	omap_control_phy_power(phy->control_dev, 0);
-
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
 
@@ -494,6 +492,8 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 		return PTR_ERR(generic_phy);
 
 	phy_set_drvdata(generic_phy, phy);
+	ti_pipe3_power_off(generic_phy);
+
 	phy_provider = devm_of_phy_provider_register(phy->dev,
 			of_phy_simple_xlate);
 	if (IS_ERR(phy_provider))
-- 
1.7.9.5

Re: [PATCH 2/7] phy: ti-pipe3: use ti_pipe3_power_off to power off the PHY during probe

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On Tue, Aug 04, 2015 at 08:50:41PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Previously omap_control_phy_power() was used to power

there is a slight functional change. You moved PHY power off from before
to after pm_runtime_enable(), clk_prepare_enable() and creation of the
PHY device. That ought to have a slight impact on how the driver
behaves.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/7] phy: ti-pipe3: use ti_pipe3_power_off to power off the PHY during probe

[Kishon Vijay Abraham I]

Hi,

On Tuesday 04 August 2015 09:36 PM, Felipe Balbi wrote:
> On Tue, Aug 04, 2015 at 08:50:41PM +0530, Kishon Vijay Abraham I wrote:
>> No functional change. Previously omap_control_phy_power() was used to power
> 
> there is a slight functional change. You moved PHY power off from before
> to after pm_runtime_enable(), clk_prepare_enable() and creation of the
> PHY device. That ought to have a slight impact on how the driver
> behaves.

okay. I'll modify the commit log. I think it won't impact the functionality as
such since the ->init and ->power_on callbacks won't be invoked at that point.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] phy: ti-pipe3: use ti_pipe3_power_off to power off the PHY during probe

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

On Wed, Aug 05, 2015 at 07:42:24PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 04 August 2015 09:36 PM, Felipe Balbi wrote:
> > On Tue, Aug 04, 2015 at 08:50:41PM +0530, Kishon Vijay Abraham I wrote:
> >> No functional change. Previously omap_control_phy_power() was used to power
> > 
> > there is a slight functional change. You moved PHY power off from before
> > to after pm_runtime_enable(), clk_prepare_enable() and creation of the
> > PHY device. That ought to have a slight impact on how the driver
> > behaves.
> 
> okay. I'll modify the commit log. I think it won't impact the functionality as
> such since the ->init and ->power_on callbacks won't be invoked at that point.

all right, then

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 3/7] phy: ti-pipe3: use *syscon* framework API to power on/off the PHY

[Kishon Vijay Abraham I]

Deprecate using phy-omap-control driver to power on/off the PHY and
use *syscon* framework to do the same.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/phy/ti-phy.txt |   10 ++-
 drivers/phy/phy-ti-pipe3.c                       |   90 ++++++++++++++++++----
 2 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index 9cf9446..e06f980 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -77,8 +77,6 @@ Required properties:
    * "div-clk" - apll clock
 
 Optional properties:
- - ctrl-module : phandle of the control module used by PHY driver to power on
-   the PHY.
  - id: If there are multiple instance of the same type, in order to
    differentiate between each instance "id" can be used (e.g., multi-lane PCIe
    PHY). If "id" is not provided, it is set to default value of '1'.
@@ -86,6 +84,14 @@ Optional properties:
    CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0
    register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy.
 
+Deprecated properties:
+ - ctrl-module : phandle of the control module used by PHY driver to power on
+   the PHY.
+
+Recommended properies:
+ - syscon-phy-power : phandle/offset pair. Phandle to the system control
+   module and the register offset to power on/off the PHY.
+
 This is usually a subnode of ocp2scp to which it is connected.
 
 usb3phy@4a084400 {
diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index 9782c16..a7c20e8 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -56,6 +56,15 @@
 
 #define SATA_PLL_SOFT_RESET	BIT(18)
 
+#define PIPE3_PHY_PWRCTL_CLK_CMD_MASK	0x003FC000
+#define PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT	14
+
+#define PIPE3_PHY_PWRCTL_CLK_FREQ_MASK	0xFFC00000
+#define PIPE3_PHY_PWRCTL_CLK_FREQ_SHIFT	22
+
+#define PIPE3_PHY_TX_RX_POWERON		0x3
+#define PIPE3_PHY_TX_RX_POWEROFF	0x0
+
 /*
  * This is an Empirical value that works, need to confirm the actual
  * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status
@@ -86,8 +95,10 @@ struct ti_pipe3 {
 	struct clk		*refclk;
 	struct clk		*div_clk;
 	struct pipe3_dpll_map	*dpll_map;
+	struct regmap		*phy_power_syscon; /* ctrl. reg. acces */
 	struct regmap		*dpll_reset_syscon; /* ctrl. reg. acces */
 	unsigned int		dpll_reset_reg; /* reg. index within syscon */
+	unsigned int		power_reg; /* power reg. index within syscon */
 	bool			sata_refclk_enabled;
 };
 
@@ -144,18 +155,53 @@ static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy);
 
 static int ti_pipe3_power_off(struct phy *x)
 {
+	u32 val;
+	int ret;
 	struct ti_pipe3 *phy = phy_get_drvdata(x);
 
-	omap_control_phy_power(phy->control_dev, 0);
+	if (phy->phy_power_syscon) {
+		val = PIPE3_PHY_TX_RX_POWEROFF <<
+			PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT;
+
+		ret = regmap_update_bits(phy->phy_power_syscon, phy->power_reg,
+					 PIPE3_PHY_PWRCTL_CLK_CMD_MASK, val);
+		if (ret < 0)
+			return ret;
+	} else {
+		omap_control_phy_power(phy->control_dev, 0);
+	}
 
 	return 0;
 }
 
 static int ti_pipe3_power_on(struct phy *x)
 {
+	u32 val;
+	u32 mask;
+	int ret;
+	unsigned long rate;
 	struct ti_pipe3 *phy = phy_get_drvdata(x);
 
-	omap_control_phy_power(phy->control_dev, 1);
+	if (phy->phy_power_syscon) {
+		rate = clk_get_rate(phy->sys_clk);
+		if (!rate) {
+			dev_err(phy->dev, "Invalid clock rate\n");
+			return -EINVAL;
+		}
+		rate = rate / 1000000;
+		mask = OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_MASK |
+			  OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_MASK;
+		val = PIPE3_PHY_TX_RX_POWERON <<
+			PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT;
+		val |= rate << OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_SHIFT;
+
+		ret = regmap_update_bits(phy->phy_power_syscon, phy->power_reg,
+					 mask, val);
+		if (ret < 0)
+			return ret;
+	} else {
+		omap_control_phy_power(phy->control_dev, 1);
+	}
 
 	return 0;
 }
@@ -364,7 +410,8 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
 		phy->wkupclk = ERR_PTR(-ENODEV);
 	}
 
-	if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
+	if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie") ||
+	    phy->phy_power_syscon) {
 		phy->sys_clk = devm_clk_get(dev, "sysclk");
 		if (IS_ERR(phy->sys_clk)) {
 			dev_err(dev, "unable to get sysclk\n");
@@ -413,19 +460,36 @@ static int ti_pipe3_get_sysctrl(struct ti_pipe3 *phy)
 	struct device_node *control_node;
 	struct platform_device *control_pdev;
 
-	control_node = of_parse_phandle(node, "ctrl-module", 0);
-	if (!control_node) {
-		dev_err(dev, "Failed to get control device phandle\n");
-		return -EINVAL;
+	phy->phy_power_syscon = syscon_regmap_lookup_by_phandle(node,
+							"syscon-phy-power");
+	if (IS_ERR(phy->phy_power_syscon)) {
+		dev_dbg(dev,
+			"can't get syscon-phy-power, using control device\n");
+		phy->phy_power_syscon = NULL;
+	} else {
+		if (of_property_read_u32_index(node,
+					       "syscon-phy-power", 1,
+					       &phy->power_reg)) {
+			dev_err(dev, "couldn't get power reg. offset\n");
+			return -EINVAL;
+		}
 	}
 
-	control_pdev = of_find_device_by_node(control_node);
-	if (!control_pdev) {
-		dev_err(dev, "Failed to get control device\n");
-		return -EINVAL;
-	}
+	if (!phy->phy_power_syscon) {
+		control_node = of_parse_phandle(node, "ctrl-module", 0);
+		if (!control_node) {
+			dev_err(dev, "Failed to get control device phandle\n");
+			return -EINVAL;
+		}
 
-	phy->control_dev = &control_pdev->dev;
+		control_pdev = of_find_device_by_node(control_node);
+		if (!control_pdev) {
+			dev_err(dev, "Failed to get control device\n");
+			return -EINVAL;
+		}
+
+		phy->control_dev = &control_pdev->dev;
+	}
 
 	if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) {
 		phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node,
-- 
1.7.9.5

[PATCH 4/7] phy: ti-pipe3: use *syscon* framework API to set PCS value of the PHY

[Kishon Vijay Abraham I]

Deprecate using phy-omap-control driver to set PCS value of the PHY
and start using *syscon* API to do the same.

Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
Acked-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
 drivers/phy/phy-ti-pipe3.c                       |   34 +++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index e06f980..49e5b0c 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -83,6 +83,8 @@ Optional properties:
  - syscon-pllreset: Handle to system control region that contains the
    CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0
    register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy.
+ - syscon-pcs : phandle/offset pair. Phandle to the system control module and the
+   register offset to write the PCS delay value.
 
 Deprecated properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index a7c20e8..8dc606d 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -65,6 +65,9 @@
 #define PIPE3_PHY_TX_RX_POWERON		0x3
 #define PIPE3_PHY_TX_RX_POWEROFF	0x0
 
+#define PCIE_PCS_MASK			0xFF0000
+#define PCIE_PCS_DELAY_COUNT_SHIFT	0x10
+
 /*
  * This is an Empirical value that works, need to confirm the actual
  * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status
@@ -96,9 +99,11 @@ struct ti_pipe3 {
 	struct clk		*div_clk;
 	struct pipe3_dpll_map	*dpll_map;
 	struct regmap		*phy_power_syscon; /* ctrl. reg. acces */
+	struct regmap		*pcs_syscon; /* ctrl. reg. acces */
 	struct regmap		*dpll_reset_syscon; /* ctrl. reg. acces */
 	unsigned int		dpll_reset_reg; /* reg. index within syscon */
 	unsigned int		power_reg; /* power reg. index within syscon */
+	unsigned int		pcie_pcs_reg; /* pcs reg. index in syscon */
 	bool			sata_refclk_enabled;
 };
 
@@ -275,7 +280,16 @@ static int ti_pipe3_init(struct phy *x)
 	 * 18-1804.
 	 */
 	if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) {
-		omap_control_pcie_pcs(phy->control_dev, 0x96);
+		if (phy->pcs_syscon) {
+			val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
+			ret = regmap_update_bits(phy->pcs_syscon,
+						 phy->pcie_pcs_reg,
+						 PCIE_PCS_MASK, val);
+			if (ret < 0)
+				return ret;
+		} else {
+			omap_control_pcie_pcs(phy->control_dev, 0x96);
+		}
 		return 0;
 	}
 
@@ -491,6 +505,24 @@ static int ti_pipe3_get_sysctrl(struct ti_pipe3 *phy)
 		phy->control_dev = &control_pdev->dev;
 	}
 
+	if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
+		phy->pcs_syscon = syscon_regmap_lookup_by_phandle(node,
+								  "syscon-pcs");
+		if (IS_ERR(phy->pcs_syscon)) {
+			dev_dbg(dev,
+				"can't get syscon-pcs, using omap control\n");
+			phy->pcs_syscon = NULL;
+		} else {
+			if (of_property_read_u32_index(node,
+						       "syscon-pcs", 1,
+						       &phy->pcie_pcs_reg)) {
+				dev_err(dev,
+					"couldn't get pcie pcs reg. offset\n");
+				return -EINVAL;
+			}
+		}
+	}
+
 	if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) {
 		phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node,
 							"syscon-pllreset");
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] phy: omap-usb2: use omap_usb_power_off to power off the PHY during probe

[Kishon Vijay Abraham I]

No functional change. Previously omap_control_phy_power() was used to power
off the PHY during probe. But once phy-omap-usb2 driver is adapted to
use syscon, omap_control_phy_power() cannot be used. Hence used
omap_usb_power_off to power off the PHY.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-omap-usb2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index c1a4686..b5c266a 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -241,7 +241,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	}
 
 	phy->control_dev = &control_pdev->dev;
-	omap_control_phy_power(phy->control_dev, 0);
 
 	otg->set_host		= omap_usb_set_host;
 	otg->set_peripheral	= omap_usb_set_peripheral;
@@ -261,6 +260,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	}
 
 	phy_set_drvdata(generic_phy, phy);
+	omap_usb_power_off(generic_phy);
 
 	phy_provider = devm_of_phy_provider_register(phy->dev,
 			of_phy_simple_xlate);
-- 
1.7.9.5

Re: [PATCH 5/7] phy: omap-usb2: use omap_usb_power_off to power off the PHY during probe

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Tue, Aug 04, 2015 at 08:50:44PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Previously omap_control_phy_power() was used to power

same comment as before.

> off the PHY during probe. But once phy-omap-usb2 driver is adapted to
> use syscon, omap_control_phy_power() cannot be used. Hence used
> omap_usb_power_off to power off the PHY.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Acked-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/phy/phy-omap-usb2.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index c1a4686..b5c266a 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -241,7 +241,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	}
>  
>  	phy->control_dev = &control_pdev->dev;
> -	omap_control_phy_power(phy->control_dev, 0);
>  
>  	otg->set_host		= omap_usb_set_host;
>  	otg->set_peripheral	= omap_usb_set_peripheral;
> @@ -261,6 +260,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	}
>  
>  	phy_set_drvdata(generic_phy, phy);
> +	omap_usb_power_off(generic_phy);
>  
>  	phy_provider = devm_of_phy_provider_register(phy->dev,
>  			of_phy_simple_xlate);
> -- 
> 1.7.9.5
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2

[Kishon Vijay Abraham I]

The USB2 PHY2 has a different register map compared to USB2 PHY1
to power on/off the PHY. In order to handle it, add a new
"compatible" string.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
 drivers/phy/phy-omap-usb2.c                      |    9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index 49e5b0c..a061077 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -31,6 +31,8 @@ OMAP USB2 PHY
 
 Required properties:
  - compatible: Should be "ti,omap-usb2"
+	       Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
+	       in DRA7x
  - reg : Address and length of the register set for the device.
  - #phy-cells: determine the number of cells that should be given in the
    phandle while referencing this phy.
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index b5c266a..2f7220f 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
 	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
 };
 
+static const struct usb_phy_data dra7x_usb2_phy2_data = {
+	.label = "dra7x_usb2_phy2",
+	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
+};
+
 static const struct usb_phy_data am437x_usb2_data = {
 	.label = "am437x_usb2",
 	.flags =  0,
@@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[] = {
 		.data = &dra7x_usb2_data,
 	},
 	{
+		.compatible = "ti,dra7x-usb2-phy2",
+		.data = &dra7x_usb2_phy2_data,
+	},
+	{
 		.compatible = "ti,am437x-usb2",
 		.data = &am437x_usb2_data,
 	},
-- 
1.7.9.5

Re: [PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2

[Roger Quadros]

On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
> The USB2 PHY2 has a different register map compared to USB2 PHY1
> to power on/off the PHY. In order to handle it, add a new
> "compatible" string.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
>  drivers/phy/phy-omap-usb2.c                      |    9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
> index 49e5b0c..a061077 100644
> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
> @@ -31,6 +31,8 @@ OMAP USB2 PHY
>  
>  Required properties:
>   - compatible: Should be "ti,omap-usb2"
> +	       Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
> +	       in DRA7x
>   - reg : Address and length of the register set for the device.
>   - #phy-cells: determine the number of cells that should be given in the
>     phandle while referencing this phy.
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index b5c266a..2f7220f 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>  };
>  
> +static const struct usb_phy_data dra7x_usb2_phy2_data = {
> +	.label = "dra7x_usb2_phy2",
> +	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
> +};
> +
>  static const struct usb_phy_data am437x_usb2_data = {
>  	.label = "am437x_usb2",
>  	.flags =  0,
> @@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[] = {
>  		.data = &dra7x_usb2_data,
>  	},
>  	{
> +		.compatible = "ti,dra7x-usb2-phy2",
> +		.data = &dra7x_usb2_phy2_data,

Why is this needed? You can reuse dra7x_usb2_data as is.

> +	},
> +	{
>  		.compatible = "ti,am437x-usb2",
>  		.data = &am437x_usb2_data,
>  	},
> 

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2

[Roger Quadros]

On 05/08/15 11:23, Roger Quadros wrote:
> 
> On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
>> The USB2 PHY2 has a different register map compared to USB2 PHY1
>> to power on/off the PHY. In order to handle it, add a new
>> "compatible" string.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
>>  drivers/phy/phy-omap-usb2.c                      |    9 +++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
>> index 49e5b0c..a061077 100644
>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
>> @@ -31,6 +31,8 @@ OMAP USB2 PHY
>>  
>>  Required properties:
>>   - compatible: Should be "ti,omap-usb2"
>> +	       Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
>> +	       in DRA7x
>>   - reg : Address and length of the register set for the device.
>>   - #phy-cells: determine the number of cells that should be given in the
>>     phandle while referencing this phy.
>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>> index b5c266a..2f7220f 100644
>> --- a/drivers/phy/phy-omap-usb2.c
>> +++ b/drivers/phy/phy-omap-usb2.c
>> @@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
>>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>>  };
>>  
>> +static const struct usb_phy_data dra7x_usb2_phy2_data = {
>> +	.label = "dra7x_usb2_phy2",
>> +	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>> +};
>> +
>>  static const struct usb_phy_data am437x_usb2_data = {
>>  	.label = "am437x_usb2",
>>  	.flags =  0,
>> @@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[] = {
>>  		.data = &dra7x_usb2_data,
>>  	},
>>  	{
>> +		.compatible = "ti,dra7x-usb2-phy2",
>> +		.data = &dra7x_usb2_phy2_data,
> 
> Why is this needed? You can reuse dra7x_usb2_data as is.

OK. I see why we need it in the next patch.
Probably both patches could be squashed.

What does .label indicate?
IMO the .label of both usb2 phys should be the same.
i.e. "dra7x_usb2"

cheers,
-roger

> 
>> +	},
>> +	{
>>  		.compatible = "ti,am437x-usb2",
>>  		.data = &am437x_usb2_data,
>>  	},
>>
> 
> cheers,
> -roger
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2

[Kishon Vijay Abraham I]

Hi Roger,

On Wednesday 05 August 2015 01:55 PM, Roger Quadros wrote:
> On 05/08/15 11:23, Roger Quadros wrote:
>>
>> On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
>>> The USB2 PHY2 has a different register map compared to USB2 PHY1
>>> to power on/off the PHY. In order to handle it, add a new
>>> "compatible" string.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
>>>  drivers/phy/phy-omap-usb2.c                      |    9 +++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> index 49e5b0c..a061077 100644
>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> @@ -31,6 +31,8 @@ OMAP USB2 PHY
>>>  
>>>  Required properties:
>>>   - compatible: Should be "ti,omap-usb2"
>>> +	       Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
>>> +	       in DRA7x
>>>   - reg : Address and length of the register set for the device.
>>>   - #phy-cells: determine the number of cells that should be given in the
>>>     phandle while referencing this phy.
>>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>>> index b5c266a..2f7220f 100644
>>> --- a/drivers/phy/phy-omap-usb2.c
>>> +++ b/drivers/phy/phy-omap-usb2.c
>>> @@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
>>>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>>>  };
>>>  
>>> +static const struct usb_phy_data dra7x_usb2_phy2_data = {
>>> +	.label = "dra7x_usb2_phy2",
>>> +	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>>> +};
>>> +
>>>  static const struct usb_phy_data am437x_usb2_data = {
>>>  	.label = "am437x_usb2",
>>>  	.flags =  0,
>>> @@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[] = {
>>>  		.data = &dra7x_usb2_data,
>>>  	},
>>>  	{
>>> +		.compatible = "ti,dra7x-usb2-phy2",
>>> +		.data = &dra7x_usb2_phy2_data,
>>
>> Why is this needed? You can reuse dra7x_usb2_data as is.
> 
> OK. I see why we need it in the next patch.
> Probably both patches could be squashed.
> 
> What does .label indicate?

it's actually used only in usb_add_phy() (drivers/usb/phy/phy.c). I don't think
it's actually important unless you strongly feel so.

Thanks
Kishon

Re: [PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2

[Roger Quadros]

On 05/08/15 17:18, Kishon Vijay Abraham I wrote:
> Hi Roger,
> 
> On Wednesday 05 August 2015 01:55 PM, Roger Quadros wrote:
>> On 05/08/15 11:23, Roger Quadros wrote:
>>>
>>> On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
>>>> The USB2 PHY2 has a different register map compared to USB2 PHY1
>>>> to power on/off the PHY. In order to handle it, add a new
>>>> "compatible" string.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/phy/ti-phy.txt |    2 ++
>>>>  drivers/phy/phy-omap-usb2.c                      |    9 +++++++++
>>>>  2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>>> index 49e5b0c..a061077 100644
>>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
>>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>>> @@ -31,6 +31,8 @@ OMAP USB2 PHY
>>>>  
>>>>  Required properties:
>>>>   - compatible: Should be "ti,omap-usb2"
>>>> +	       Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
>>>> +	       in DRA7x
>>>>   - reg : Address and length of the register set for the device.
>>>>   - #phy-cells: determine the number of cells that should be given in the
>>>>     phandle while referencing this phy.
>>>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>>>> index b5c266a..2f7220f 100644
>>>> --- a/drivers/phy/phy-omap-usb2.c
>>>> +++ b/drivers/phy/phy-omap-usb2.c
>>>> @@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
>>>>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>>>>  };
>>>>  
>>>> +static const struct usb_phy_data dra7x_usb2_phy2_data = {
>>>> +	.label = "dra7x_usb2_phy2",
>>>> +	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>>>> +};
>>>> +
>>>>  static const struct usb_phy_data am437x_usb2_data = {
>>>>  	.label = "am437x_usb2",
>>>>  	.flags =  0,
>>>> @@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[] = {
>>>>  		.data = &dra7x_usb2_data,
>>>>  	},
>>>>  	{
>>>> +		.compatible = "ti,dra7x-usb2-phy2",
>>>> +		.data = &dra7x_usb2_phy2_data,
>>>
>>> Why is this needed? You can reuse dra7x_usb2_data as is.
>>
>> OK. I see why we need it in the next patch.
>> Probably both patches could be squashed.
>>
>> What does .label indicate?
> 
> it's actually used only in usb_add_phy() (drivers/usb/phy/phy.c). I don't think
> it's actually important unless you strongly feel so.

I leave it to you then :).

cheers,
-roger

[PATCH 7/7] phy: omap-usb2: use *syscon* framework API to power on/off the PHY

[Kishon Vijay Abraham I]

Deprecate using phy-omap-control driver to power on/off the PHY,
and use *syscon* framework to do the same. This handles
powering on/off the PHY for the USB2 PHYs used in various TI SoCs.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/phy/ti-phy.txt |    6 +-
 drivers/phy/phy-omap-usb2.c                      |   85 +++++++++++++++++-----
 include/linux/phy/omap_usb.h                     |   23 ++++++
 3 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index a061077..a3b3945 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -42,10 +42,14 @@ Required properties:
    * "wkupclk" - wakeup clock.
    * "refclk" - reference clock (optional).
 
-Optional properties:
+Deprecated properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
    the PHY.
 
+Recommended properies:
+- syscon-phy-power : phandle/offset pair. Phandle to the system control
+  module and the register offset to power on/off the PHY.
+
 This is usually a subnode of ocp2scp to which it is connected.
 
 usb2phy@4a0ad080 {
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 2f7220f..531fe04 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -29,6 +29,8 @@
 #include <linux/delay.h>
 #include <linux/phy/omap_control_phy.h>
 #include <linux/phy/phy.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 #include <linux/of_platform.h>
 
 #define USB2PHY_DISCON_BYP_LATCH (1 << 31)
@@ -97,22 +99,40 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
 	return 0;
 }
 
-static int omap_usb_power_off(struct phy *x)
+static int omap_usb_phy_power(struct omap_usb *phy, int on)
 {
-	struct omap_usb *phy = phy_get_drvdata(x);
+	u32 val = 0;
+	int ret;
 
-	omap_control_phy_power(phy->control_dev, 0);
+	if (phy->syscon_phy_power) {
+		if (on)
+			val = phy->power_on;
+		else
+			val = phy->power_off;
+
+		ret = regmap_update_bits(phy->syscon_phy_power, phy->power_reg,
+					 phy->mask, val);
+		if (ret < 0)
+			return ret;
+	} else {
+		omap_control_phy_power(phy->control_dev, on);
+	}
 
 	return 0;
 }
 
-static int omap_usb_power_on(struct phy *x)
+static int omap_usb_power_off(struct phy *x)
 {
 	struct omap_usb *phy = phy_get_drvdata(x);
 
-	omap_control_phy_power(phy->control_dev, 1);
+	return omap_usb_phy_power(phy, false);
+}
 
-	return 0;
+static int omap_usb_power_on(struct phy *x)
+{
+	struct omap_usb *phy = phy_get_drvdata(x);
+
+	return omap_usb_phy_power(phy, true);
 }
 
 static int omap_usb_init(struct phy *x)
@@ -147,26 +167,38 @@ static struct phy_ops ops = {
 static const struct usb_phy_data omap_usb2_data = {
 	.label = "omap_usb2",
 	.flags = OMAP_USB2_HAS_START_SRP | OMAP_USB2_HAS_SET_VBUS,
+	.mask = OMAP_DEV_PHY_PD,
+	.power_off = OMAP_DEV_PHY_PD,
 };
 
 static const struct usb_phy_data omap5_usb2_data = {
 	.label = "omap5_usb2",
 	.flags = 0,
+	.mask = OMAP_DEV_PHY_PD,
+	.power_off = OMAP_DEV_PHY_PD,
 };
 
 static const struct usb_phy_data dra7x_usb2_data = {
 	.label = "dra7x_usb2",
 	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
+	.mask = OMAP_DEV_PHY_PD,
+	.power_off = OMAP_DEV_PHY_PD,
 };
 
 static const struct usb_phy_data dra7x_usb2_phy2_data = {
 	.label = "dra7x_usb2_phy2",
 	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
+	.mask = OMAP_USB2_PHY_PD,
+	.power_off = OMAP_USB2_PHY_PD,
 };
 
 static const struct usb_phy_data am437x_usb2_data = {
 	.label = "am437x_usb2",
 	.flags =  0,
+	.mask = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD |
+		AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
+	.power_on = AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
+	.power_off = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD,
 };
 
 static const struct of_device_id omap_usb2_id_table[] = {
@@ -228,6 +260,9 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	phy->phy.label		= phy_data->label;
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
+	phy->mask		= phy_data->mask;
+	phy->power_on		= phy_data->power_on;
+	phy->power_off		= phy_data->power_off;
 
 	if (phy_data->flags & OMAP_USB2_CALIBRATE_FALSE_DISCONNECT) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -237,20 +272,36 @@ static int omap_usb2_probe(struct platform_device *pdev)
 		phy->flags |= OMAP_USB2_CALIBRATE_FALSE_DISCONNECT;
 	}
 
-	control_node = of_parse_phandle(node, "ctrl-module", 0);
-	if (!control_node) {
-		dev_err(&pdev->dev, "Failed to get control device phandle\n");
-		return -EINVAL;
-	}
+	phy->syscon_phy_power = syscon_regmap_lookup_by_phandle(node,
+							"syscon-phy-power");
+	if (IS_ERR(phy->syscon_phy_power)) {
+		dev_info(&pdev->dev,
+			 "can't get syscon-phy-power, using control device\n");
+		phy->syscon_phy_power = NULL;
+
+		control_node = of_parse_phandle(node, "ctrl-module", 0);
+		if (!control_node) {
+			dev_err(&pdev->dev,
+				"Failed to get control device phandle\n");
+			return -EINVAL;
+		}
 
-	control_pdev = of_find_device_by_node(control_node);
-	if (!control_pdev) {
-		dev_err(&pdev->dev, "Failed to get control device\n");
-		return -EINVAL;
+		control_pdev = of_find_device_by_node(control_node);
+		if (!control_pdev) {
+			dev_err(&pdev->dev, "Failed to get control device\n");
+			return -EINVAL;
+		}
+		phy->control_dev = &control_pdev->dev;
+	} else {
+		if (of_property_read_u32_index(node,
+					       "syscon-phy-power", 1,
+					       &phy->power_reg)) {
+			dev_err(&pdev->dev,
+				"couldn't get power reg. offset\n");
+			return -EINVAL;
+		}
 	}
 
-	phy->control_dev = &control_pdev->dev;
-
 	otg->set_host		= omap_usb_set_host;
 	otg->set_peripheral	= omap_usb_set_peripheral;
 	if (phy_data->flags & OMAP_USB2_HAS_SET_VBUS)
diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index dc2c541..2e5fb87 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -30,6 +30,12 @@ struct usb_dpll_params {
 	u32	mf;
 };
 
+enum omap_usb_phy_type {
+	TYPE_USB2,    /* USB2_PHY, power down in CONTROL_DEV_CONF */
+	TYPE_DRA7USB2, /* USB2 PHY, power and power_aux e.g. DRA7 */
+	TYPE_AM437USB2, /* USB2 PHY, power e.g. AM437x */
+};
+
 struct omap_usb {
 	struct usb_phy		phy;
 	struct phy_companion	*comparator;
@@ -40,11 +46,20 @@ struct omap_usb {
 	struct clk		*wkupclk;
 	struct clk		*optclk;
 	u8			flags;
+	enum omap_usb_phy_type	type;
+	struct regmap		*syscon_phy_power; /* ctrl. reg. acces */
+	unsigned int		power_reg; /* power reg. index within syscon */
+	u32			mask;
+	u32			power_on;
+	u32			power_off;
 };
 
 struct usb_phy_data {
 	const char *label;
 	u8 flags;
+	u32 mask;
+	u32 power_on;
+	u32 power_off;
 };
 
 /* Driver Flags */
@@ -52,6 +67,14 @@ struct usb_phy_data {
 #define OMAP_USB2_HAS_SET_VBUS (1 << 1)
 #define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT (1 << 2)
 
+#define OMAP_DEV_PHY_PD		BIT(0)
+#define OMAP_USB2_PHY_PD	BIT(28)
+
+#define AM437X_USB2_PHY_PD		BIT(0)
+#define AM437X_USB2_OTG_PD		BIT(1)
+#define AM437X_USB2_OTGVDET_EN		BIT(19)
+#define AM437X_USB2_OTGSESSEND_EN	BIT(20)
+
 #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
 
 #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
-- 
1.7.9.5

Re: [PATCH 7/7] phy: omap-usb2: use *syscon* framework API to power on/off the PHY

[Roger Quadros]

On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
> Deprecate using phy-omap-control driver to power on/off the PHY,
> and use *syscon* framework to do the same. This handles
> powering on/off the PHY for the USB2 PHYs used in various TI SoCs.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  Documentation/devicetree/bindings/phy/ti-phy.txt |    6 +-
>  drivers/phy/phy-omap-usb2.c                      |   85 +++++++++++++++++-----
>  include/linux/phy/omap_usb.h                     |   23 ++++++
>  3 files changed, 96 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
> index a061077..a3b3945 100644
> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
> @@ -42,10 +42,14 @@ Required properties:
>     * "wkupclk" - wakeup clock.
>     * "refclk" - reference clock (optional).
>  
> -Optional properties:
> +Deprecated properties:
>   - ctrl-module : phandle of the control module used by PHY driver to power on
>     the PHY.
>  
> +Recommended properies:
> +- syscon-phy-power : phandle/offset pair. Phandle to the system control
> +  module and the register offset to power on/off the PHY.
> +
>  This is usually a subnode of ocp2scp to which it is connected.
>  
>  usb2phy@4a0ad080 {
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 2f7220f..531fe04 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -29,6 +29,8 @@
>  #include <linux/delay.h>
>  #include <linux/phy/omap_control_phy.h>
>  #include <linux/phy/phy.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>  #include <linux/of_platform.h>
>  
>  #define USB2PHY_DISCON_BYP_LATCH (1 << 31)
> @@ -97,22 +99,40 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
>  	return 0;
>  }
>  
> -static int omap_usb_power_off(struct phy *x)
> +static int omap_usb_phy_power(struct omap_usb *phy, int on)
>  {
> -	struct omap_usb *phy = phy_get_drvdata(x);
> +	u32 val = 0;

No need to initialize val.

> +	int ret;
>  
> -	omap_control_phy_power(phy->control_dev, 0);
> +	if (phy->syscon_phy_power) {
> +		if (on)
> +			val = phy->power_on;
> +		else
> +			val = phy->power_off;
> +
> +		ret = regmap_update_bits(phy->syscon_phy_power, phy->power_reg,
> +					 phy->mask, val);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		omap_control_phy_power(phy->control_dev, on);
> +	}
>  
>  	return 0;
>  }
>  
> -static int omap_usb_power_on(struct phy *x)
> +static int omap_usb_power_off(struct phy *x)
>  {
>  	struct omap_usb *phy = phy_get_drvdata(x);
>  
> -	omap_control_phy_power(phy->control_dev, 1);
> +	return omap_usb_phy_power(phy, false);
> +}
>  
> -	return 0;
> +static int omap_usb_power_on(struct phy *x)
> +{
> +	struct omap_usb *phy = phy_get_drvdata(x);
> +
> +	return omap_usb_phy_power(phy, true);
>  }
>  
>  static int omap_usb_init(struct phy *x)
> @@ -147,26 +167,38 @@ static struct phy_ops ops = {
>  static const struct usb_phy_data omap_usb2_data = {
>  	.label = "omap_usb2",
>  	.flags = OMAP_USB2_HAS_START_SRP | OMAP_USB2_HAS_SET_VBUS,
> +	.mask = OMAP_DEV_PHY_PD,
> +	.power_off = OMAP_DEV_PHY_PD,
>  };
>  
>  static const struct usb_phy_data omap5_usb2_data = {
>  	.label = "omap5_usb2",
>  	.flags = 0,
> +	.mask = OMAP_DEV_PHY_PD,
> +	.power_off = OMAP_DEV_PHY_PD,
>  };
>  
>  static const struct usb_phy_data dra7x_usb2_data = {
>  	.label = "dra7x_usb2",
>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
> +	.mask = OMAP_DEV_PHY_PD,
> +	.power_off = OMAP_DEV_PHY_PD,
>  };
>  
>  static const struct usb_phy_data dra7x_usb2_phy2_data = {
>  	.label = "dra7x_usb2_phy2",
>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
> +	.mask = OMAP_USB2_PHY_PD,
> +	.power_off = OMAP_USB2_PHY_PD,
>  };
>  
>  static const struct usb_phy_data am437x_usb2_data = {
>  	.label = "am437x_usb2",
>  	.flags =  0,
> +	.mask = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD |
> +		AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
> +	.power_on = AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
> +	.power_off = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD,

We're combining some OTG operations with PHY on/off. This is probably OK
for now but looks like we need a OTG phy driver for this?

>  };
>  
>  static const struct of_device_id omap_usb2_id_table[] = {
> @@ -228,6 +260,9 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	phy->phy.label		= phy_data->label;
>  	phy->phy.otg		= otg;
>  	phy->phy.type		= USB_PHY_TYPE_USB2;
> +	phy->mask		= phy_data->mask;
> +	phy->power_on		= phy_data->power_on;
> +	phy->power_off		= phy_data->power_off;
>  
>  	if (phy_data->flags & OMAP_USB2_CALIBRATE_FALSE_DISCONNECT) {
>  		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -237,20 +272,36 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  		phy->flags |= OMAP_USB2_CALIBRATE_FALSE_DISCONNECT;
>  	}
>  
> -	control_node = of_parse_phandle(node, "ctrl-module", 0);
> -	if (!control_node) {
> -		dev_err(&pdev->dev, "Failed to get control device phandle\n");
> -		return -EINVAL;
> -	}
> +	phy->syscon_phy_power = syscon_regmap_lookup_by_phandle(node,
> +							"syscon-phy-power");
> +	if (IS_ERR(phy->syscon_phy_power)) {
> +		dev_info(&pdev->dev,
> +			 "can't get syscon-phy-power, using control device\n");

dev_dbg()?

> +		phy->syscon_phy_power = NULL;
> +
> +		control_node = of_parse_phandle(node, "ctrl-module", 0);
> +		if (!control_node) {
> +			dev_err(&pdev->dev,
> +				"Failed to get control device phandle\n");
> +			return -EINVAL;
> +		}
>  
> -	control_pdev = of_find_device_by_node(control_node);
> -	if (!control_pdev) {
> -		dev_err(&pdev->dev, "Failed to get control device\n");
> -		return -EINVAL;
> +		control_pdev = of_find_device_by_node(control_node);
> +		if (!control_pdev) {
> +			dev_err(&pdev->dev, "Failed to get control device\n");
> +			return -EINVAL;
> +		}
> +		phy->control_dev = &control_pdev->dev;
> +	} else {
> +		if (of_property_read_u32_index(node,
> +					       "syscon-phy-power", 1,
> +					       &phy->power_reg)) {
> +			dev_err(&pdev->dev,
> +				"couldn't get power reg. offset\n");
> +			return -EINVAL;
> +		}
>  	}
>  
> -	phy->control_dev = &control_pdev->dev;
> -
>  	otg->set_host		= omap_usb_set_host;
>  	otg->set_peripheral	= omap_usb_set_peripheral;
>  	if (phy_data->flags & OMAP_USB2_HAS_SET_VBUS)
> diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
> index dc2c541..2e5fb87 100644
> --- a/include/linux/phy/omap_usb.h
> +++ b/include/linux/phy/omap_usb.h
> @@ -30,6 +30,12 @@ struct usb_dpll_params {
>  	u32	mf;
>  };
>  
> +enum omap_usb_phy_type {
> +	TYPE_USB2,    /* USB2_PHY, power down in CONTROL_DEV_CONF */
> +	TYPE_DRA7USB2, /* USB2 PHY, power and power_aux e.g. DRA7 */
> +	TYPE_AM437USB2, /* USB2 PHY, power e.g. AM437x */
> +};
> +
>  struct omap_usb {
>  	struct usb_phy		phy;
>  	struct phy_companion	*comparator;
> @@ -40,11 +46,20 @@ struct omap_usb {
>  	struct clk		*wkupclk;
>  	struct clk		*optclk;
>  	u8			flags;
> +	enum omap_usb_phy_type	type;
> +	struct regmap		*syscon_phy_power; /* ctrl. reg. acces */
> +	unsigned int		power_reg; /* power reg. index within syscon */
> +	u32			mask;
> +	u32			power_on;
> +	u32			power_off;
>  };
>  
>  struct usb_phy_data {
>  	const char *label;
>  	u8 flags;
> +	u32 mask;
> +	u32 power_on;
> +	u32 power_off;
>  };
>  
>  /* Driver Flags */
> @@ -52,6 +67,14 @@ struct usb_phy_data {
>  #define OMAP_USB2_HAS_SET_VBUS (1 << 1)
>  #define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT (1 << 2)
>  
> +#define OMAP_DEV_PHY_PD		BIT(0)
> +#define OMAP_USB2_PHY_PD	BIT(28)
> +
> +#define AM437X_USB2_PHY_PD		BIT(0)
> +#define AM437X_USB2_OTG_PD		BIT(1)
> +#define AM437X_USB2_OTGVDET_EN		BIT(19)
> +#define AM437X_USB2_OTGSESSEND_EN	BIT(20)
> +

Is all this used by anyone other than the phy driver?
If not we should move whatever possible into the .c file.

Could be a separate patch of course.


>  #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
>  
>  #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
> 

cheers,
-roger

Re: [PATCH 7/7] phy: omap-usb2: use *syscon* framework API to power on/off the PHY

[Kishon Vijay Abraham I]

Hi,

On Wednesday 05 August 2015 02:05 PM, Roger Quadros wrote:
> On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
>> Deprecate using phy-omap-control driver to power on/off the PHY,
>> and use *syscon* framework to do the same. This handles
>> powering on/off the PHY for the USB2 PHYs used in various TI SoCs.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/phy/ti-phy.txt |    6 +-
>>  drivers/phy/phy-omap-usb2.c                      |   85 +++++++++++++++++-----
>>  include/linux/phy/omap_usb.h                     |   23 ++++++
>>  3 files changed, 96 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
>> index a061077..a3b3945 100644
>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
>> @@ -42,10 +42,14 @@ Required properties:
>>     * "wkupclk" - wakeup clock.
>>     * "refclk" - reference clock (optional).
>>  
>> -Optional properties:
>> +Deprecated properties:
>>   - ctrl-module : phandle of the control module used by PHY driver to power on
>>     the PHY.
>>  
>> +Recommended properies:
>> +- syscon-phy-power : phandle/offset pair. Phandle to the system control
>> +  module and the register offset to power on/off the PHY.
>> +
>>  This is usually a subnode of ocp2scp to which it is connected.
>>  
>>  usb2phy@4a0ad080 {
>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>> index 2f7220f..531fe04 100644
>> --- a/drivers/phy/phy-omap-usb2.c
>> +++ b/drivers/phy/phy-omap-usb2.c
>> @@ -29,6 +29,8 @@
>>  #include <linux/delay.h>
>>  #include <linux/phy/omap_control_phy.h>
>>  #include <linux/phy/phy.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/regmap.h>
>>  #include <linux/of_platform.h>
>>  
>>  #define USB2PHY_DISCON_BYP_LATCH (1 << 31)
>> @@ -97,22 +99,40 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
>>  	return 0;
>>  }
>>  
>> -static int omap_usb_power_off(struct phy *x)
>> +static int omap_usb_phy_power(struct omap_usb *phy, int on)
>>  {
>> -	struct omap_usb *phy = phy_get_drvdata(x);
>> +	u32 val = 0;
> 
> No need to initialize val.

indeed.
> 
>> +	int ret;
>>  
>> -	omap_control_phy_power(phy->control_dev, 0);
>> +	if (phy->syscon_phy_power) {
>> +		if (on)
>> +			val = phy->power_on;
>> +		else
>> +			val = phy->power_off;
>> +
>> +		ret = regmap_update_bits(phy->syscon_phy_power, phy->power_reg,
>> +					 phy->mask, val);
>> +		if (ret < 0)
>> +			return ret;
>> +	} else {
>> +		omap_control_phy_power(phy->control_dev, on);
>> +	}
>>  
>>  	return 0;
>>  }
>>  
>> -static int omap_usb_power_on(struct phy *x)
>> +static int omap_usb_power_off(struct phy *x)
>>  {
>>  	struct omap_usb *phy = phy_get_drvdata(x);
>>  
>> -	omap_control_phy_power(phy->control_dev, 1);
>> +	return omap_usb_phy_power(phy, false);
>> +}
>>  
>> -	return 0;
>> +static int omap_usb_power_on(struct phy *x)
>> +{
>> +	struct omap_usb *phy = phy_get_drvdata(x);
>> +
>> +	return omap_usb_phy_power(phy, true);
>>  }
>>  
>>  static int omap_usb_init(struct phy *x)
>> @@ -147,26 +167,38 @@ static struct phy_ops ops = {
>>  static const struct usb_phy_data omap_usb2_data = {
>>  	.label = "omap_usb2",
>>  	.flags = OMAP_USB2_HAS_START_SRP | OMAP_USB2_HAS_SET_VBUS,
>> +	.mask = OMAP_DEV_PHY_PD,
>> +	.power_off = OMAP_DEV_PHY_PD,
>>  };
>>  
>>  static const struct usb_phy_data omap5_usb2_data = {
>>  	.label = "omap5_usb2",
>>  	.flags = 0,
>> +	.mask = OMAP_DEV_PHY_PD,
>> +	.power_off = OMAP_DEV_PHY_PD,
>>  };
>>  
>>  static const struct usb_phy_data dra7x_usb2_data = {
>>  	.label = "dra7x_usb2",
>>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>> +	.mask = OMAP_DEV_PHY_PD,
>> +	.power_off = OMAP_DEV_PHY_PD,
>>  };
>>  
>>  static const struct usb_phy_data dra7x_usb2_phy2_data = {
>>  	.label = "dra7x_usb2_phy2",
>>  	.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
>> +	.mask = OMAP_USB2_PHY_PD,
>> +	.power_off = OMAP_USB2_PHY_PD,
>>  };
>>  
>>  static const struct usb_phy_data am437x_usb2_data = {
>>  	.label = "am437x_usb2",
>>  	.flags =  0,
>> +	.mask = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD |
>> +		AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
>> +	.power_on = AM437X_USB2_OTGVDET_EN | AM437X_USB2_OTGSESSEND_EN,
>> +	.power_off = AM437X_USB2_PHY_PD | AM437X_USB2_OTG_PD,
> 
> We're combining some OTG operations with PHY on/off. This is probably OK
> for now but looks like we need a OTG phy driver for this?

Yes. Once we have that we can stop using the USB PHY library from this driver.
> 
>>  };
>>  
>>  static const struct of_device_id omap_usb2_id_table[] = {
>> @@ -228,6 +260,9 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	phy->phy.label		= phy_data->label;
>>  	phy->phy.otg		= otg;
>>  	phy->phy.type		= USB_PHY_TYPE_USB2;
>> +	phy->mask		= phy_data->mask;
>> +	phy->power_on		= phy_data->power_on;
>> +	phy->power_off		= phy_data->power_off;
>>  
>>  	if (phy_data->flags & OMAP_USB2_CALIBRATE_FALSE_DISCONNECT) {
>>  		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> @@ -237,20 +272,36 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  		phy->flags |= OMAP_USB2_CALIBRATE_FALSE_DISCONNECT;
>>  	}
>>  
>> -	control_node = of_parse_phandle(node, "ctrl-module", 0);
>> -	if (!control_node) {
>> -		dev_err(&pdev->dev, "Failed to get control device phandle\n");
>> -		return -EINVAL;
>> -	}
>> +	phy->syscon_phy_power = syscon_regmap_lookup_by_phandle(node,
>> +							"syscon-phy-power");
>> +	if (IS_ERR(phy->syscon_phy_power)) {
>> +		dev_info(&pdev->dev,
>> +			 "can't get syscon-phy-power, using control device\n");
> 
> dev_dbg()?

okay.
> 
>> +		phy->syscon_phy_power = NULL;
>> +
>> +		control_node = of_parse_phandle(node, "ctrl-module", 0);
>> +		if (!control_node) {
>> +			dev_err(&pdev->dev,
>> +				"Failed to get control device phandle\n");
>> +			return -EINVAL;
>> +		}
>>  
>> -	control_pdev = of_find_device_by_node(control_node);
>> -	if (!control_pdev) {
>> -		dev_err(&pdev->dev, "Failed to get control device\n");
>> -		return -EINVAL;
>> +		control_pdev = of_find_device_by_node(control_node);
>> +		if (!control_pdev) {
>> +			dev_err(&pdev->dev, "Failed to get control device\n");
>> +			return -EINVAL;
>> +		}
>> +		phy->control_dev = &control_pdev->dev;
>> +	} else {
>> +		if (of_property_read_u32_index(node,
>> +					       "syscon-phy-power", 1,
>> +					       &phy->power_reg)) {
>> +			dev_err(&pdev->dev,
>> +				"couldn't get power reg. offset\n");
>> +			return -EINVAL;
>> +		}
>>  	}
>>  
>> -	phy->control_dev = &control_pdev->dev;
>> -
>>  	otg->set_host		= omap_usb_set_host;
>>  	otg->set_peripheral	= omap_usb_set_peripheral;
>>  	if (phy_data->flags & OMAP_USB2_HAS_SET_VBUS)
>> diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
>> index dc2c541..2e5fb87 100644
>> --- a/include/linux/phy/omap_usb.h
>> +++ b/include/linux/phy/omap_usb.h
>> @@ -30,6 +30,12 @@ struct usb_dpll_params {
>>  	u32	mf;
>>  };
>>  
>> +enum omap_usb_phy_type {
>> +	TYPE_USB2,    /* USB2_PHY, power down in CONTROL_DEV_CONF */
>> +	TYPE_DRA7USB2, /* USB2 PHY, power and power_aux e.g. DRA7 */
>> +	TYPE_AM437USB2, /* USB2 PHY, power e.g. AM437x */
>> +};
>> +
>>  struct omap_usb {
>>  	struct usb_phy		phy;
>>  	struct phy_companion	*comparator;
>> @@ -40,11 +46,20 @@ struct omap_usb {
>>  	struct clk		*wkupclk;
>>  	struct clk		*optclk;
>>  	u8			flags;
>> +	enum omap_usb_phy_type	type;
>> +	struct regmap		*syscon_phy_power; /* ctrl. reg. acces */
>> +	unsigned int		power_reg; /* power reg. index within syscon */
>> +	u32			mask;
>> +	u32			power_on;
>> +	u32			power_off;
>>  };
>>  
>>  struct usb_phy_data {
>>  	const char *label;
>>  	u8 flags;
>> +	u32 mask;
>> +	u32 power_on;
>> +	u32 power_off;
>>  };
>>  
>>  /* Driver Flags */
>> @@ -52,6 +67,14 @@ struct usb_phy_data {
>>  #define OMAP_USB2_HAS_SET_VBUS (1 << 1)
>>  #define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT (1 << 2)
>>  
>> +#define OMAP_DEV_PHY_PD		BIT(0)
>> +#define OMAP_USB2_PHY_PD	BIT(28)
>> +
>> +#define AM437X_USB2_PHY_PD		BIT(0)
>> +#define AM437X_USB2_OTG_PD		BIT(1)
>> +#define AM437X_USB2_OTGVDET_EN		BIT(19)
>> +#define AM437X_USB2_OTGSESSEND_EN	BIT(20)
>> +
> 
> Is all this used by anyone other than the phy driver?
> If not we should move whatever possible into the .c file.
> 
> Could be a separate patch of course.

sure.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html