2015-08-04 15:20:59

by Kishon Vijay Abraham I

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

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


2015-08-04 15:23:09

by Kishon Vijay Abraham I

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

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

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
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

2015-08-04 15:21:04

by Kishon Vijay Abraham I

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

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 <[email protected]>
Acked-by: Roger Quadros <[email protected]>
---
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

2015-08-04 15:21:18

by Kishon Vijay Abraham I

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

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 <[email protected]>
---
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

2015-08-04 15:21:15

by Kishon Vijay Abraham I

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

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 <[email protected]>
Acked-by: Roger Quadros <[email protected]>
---
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

2015-08-04 15:21:21

by Kishon Vijay Abraham I

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

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 <[email protected]>
Acked-by: Roger Quadros <[email protected]>
---
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

2015-08-04 15:22:29

by Kishon Vijay Abraham I

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

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 <[email protected]>
---
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

2015-08-04 15:21:22

by Kishon Vijay Abraham I

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

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 <[email protected]>
---
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

2015-08-04 16:00:37

by Felipe Balbi

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

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 <[email protected]>

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


Attachments:
(No filename) (636.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-04 16:06:39

by Felipe Balbi

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

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


Attachments:
(No filename) (380.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-04 16:07:26

by Felipe Balbi

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

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 <[email protected]>
> Acked-by: Roger Quadros <[email protected]>
> ---
> 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


Attachments:
(No filename) (1.27 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-05 08:23:39

by Roger Quadros

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


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 <[email protected]>
> ---
> 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

2015-08-05 08:26:06

by Roger Quadros

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

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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2015-08-05 08:35:54

by Roger Quadros

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

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 <[email protected]>
> ---
> 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

2015-08-05 14:08:21

by Kishon Vijay Abraham I

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

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 <[email protected]>
>
> 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.

2015-08-05 14:12:35

by Kishon Vijay Abraham I

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

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

2015-08-05 14:18:37

by Kishon Vijay Abraham I

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

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 <[email protected]>
>>> ---
>>> 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

2015-08-05 14:22:06

by Kishon Vijay Abraham I

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

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 <[email protected]>
>> ---
>> 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

2015-08-05 15:35:11

by Felipe Balbi

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

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


Attachments:
(No filename) (728.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-07 08:27:30

by Roger Quadros

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

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 <[email protected]>
>>>> ---
>>>> 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