[PATCH 0/8] phy: rockchip-usb: correct pll handling and usb-uart

[PATCH 0/8] phy: rockchip-usb: correct pll handling and usb-uart

[Heiko Stuebner]

Patches 1-7 fix a long-standing issue with the clock-tree of Rockchip SoCs
namely our ignorance of the usbphy-internal pll that creates the needed
480MHz but is also a supply-clock back to the core clock-controller in
Rockchip SoCs.

Till now that was worked around using a virtual clock in the cru itself,
but that is of course ignorant of other parts then disabling the phy
behind the cru's back, thus breaking potential users of these clocks.


Patch 8, while not associated with the new pll handling, also builds
on the groundwork introduced there and adds support for the function
repurposing one of the phys as passthrough for uart-data. This enables
attaching a ttl converter to the D+ and D- pins of an usb cable to
receive uart data this way, when it is not really possible to attach
a regular serial console to a board.

One point of critique in my first iteration [0] of this was, that
due to when the reconfiguration happens we may miss parts of the logs
when earlycon is enabled. So far early_initcall gets used as the
unflattened devicetree is necessary to set this up. Doing this for
example in the early_param directly would require parsing the flattened
devicetree to get needed nodes and properties.

I still maintain that if you're working on anything before smp-bringup
you should use a real dev-board instead or try to solder uart cables
on hopefully available test-points :-) .


In any case, if patch 8 causes to much headache, it could be dropped
to not hinder the earlier 7 patches.

[0] http://comments.gmane.org/gmane.linux.ports.arm.rockchip/715


Heiko Stuebner (8):
  phy: rockchip-usb: fix clock get-put mismatch
  phy: rockchip-usb: introduce a common data-struct for the device
  phy: rockchip-usb: move per-phy init into a separate function
  phy: rockchip-usb: expose the phy-internal PLLs
  clk: rockchip: fix usbphy-related clocks
  ARM: dts: rockchip: add clock-cells for usb phy nodes
  ARM: dts: rockchip: assign usbphy480m_src to the new usbphy pll on
    veyron
  phy: rockchip-usb: add handler for usb-uart functionality

 .../devicetree/bindings/phy/rockchip-usb-phy.txt   |   6 +-
 Documentation/kernel-parameters.txt                |   6 +
 arch/arm/boot/dts/rk3066a.dtsi                     |   2 +
 arch/arm/boot/dts/rk3188.dtsi                      |   2 +
 arch/arm/boot/dts/rk3288-veyron.dtsi               |   2 +-
 arch/arm/boot/dts/rk3288.dtsi                      |   3 +
 drivers/clk/rockchip/clk-rk3188.c                  |  11 +-
 drivers/clk/rockchip/clk-rk3288.c                  |  16 +-
 drivers/phy/phy-rockchip-usb.c                     | 451 ++++++++++++++++++---
 9 files changed, 417 insertions(+), 82 deletions(-)

-- 
2.6.2

[PATCH 1/8] phy: rockchip-usb: fix clock get-put mismatch

[Heiko Stuebner]

Currently the phy driver only gets the optional clock reference but
never puts it again, neither during error handling nor on remove.
Fix that by moving the clk_put to a devm-action that gets called at
the right time when all other devm actions are done.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/phy/phy-rockchip-usb.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
index 91d6f34..dfc056b 100644
--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -90,6 +90,14 @@ static const struct phy_ops ops = {
 	.owner		= THIS_MODULE,
 };
 
+static void rockchip_usb_phy_action(void *data)
+{
+	struct rockchip_usb_phy *rk_phy = data;
+
+	if (rk_phy->clk)
+		clk_put(rk_phy->clk);
+}
+
 static int rockchip_usb_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -124,6 +132,13 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
 		if (IS_ERR(rk_phy->clk))
 			rk_phy->clk = NULL;
 
+		err = devm_add_action(dev, rockchip_usb_phy_action, rk_phy);
+		if (err) {
+			if (rk_phy->clk)
+				clk_put(rk_phy->clk);
+			return err;
+		}
+
 		rk_phy->phy = devm_phy_create(dev, child, &ops);
 		if (IS_ERR(rk_phy->phy)) {
 			dev_err(dev, "failed to create PHY\n");
-- 
2.6.2

[PATCH 2/8] phy: rockchip-usb: introduce a common data-struct for the device

[Heiko Stuebner]

This introduces a common struct that holds data belonging to
the umbrella device that contains all the phys and that we
want to use later.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/phy/phy-rockchip-usb.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
index dfc056b..dda1994 100644
--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -36,9 +36,14 @@
 #define SIDDQ_ON		BIT(13)
 #define SIDDQ_OFF		(0 << 13)
 
+struct rockchip_usb_phy_base {
+	struct device *dev;
+	struct regmap *reg_base;
+};
+
 struct rockchip_usb_phy {
+	struct rockchip_usb_phy_base *base;
 	unsigned int	reg_offset;
-	struct regmap	*reg_base;
 	struct clk	*clk;
 	struct phy	*phy;
 };
@@ -46,7 +51,7 @@ struct rockchip_usb_phy {
 static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
 					   bool siddq)
 {
-	return regmap_write(phy->reg_base, phy->reg_offset,
+	return regmap_write(phy->base->reg_base, phy->reg_offset,
 			    SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
 }
 
@@ -101,17 +106,23 @@ static void rockchip_usb_phy_action(void *data)
 static int rockchip_usb_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct rockchip_usb_phy_base *phy_base;
 	struct rockchip_usb_phy *rk_phy;
 	struct phy_provider *phy_provider;
 	struct device_node *child;
-	struct regmap *grf;
 	unsigned int reg_offset;
 	int err;
 
-	grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
-	if (IS_ERR(grf)) {
+	phy_base = devm_kzalloc(dev, sizeof(*phy_base), GFP_KERNEL);
+	if (!phy_base)
+		return -ENOMEM;
+
+	phy_base->dev = dev;
+	phy_base->reg_base = syscon_regmap_lookup_by_phandle(dev->of_node,
+							     "rockchip,grf");
+	if (IS_ERR(phy_base->reg_base)) {
 		dev_err(&pdev->dev, "Missing rockchip,grf property\n");
-		return PTR_ERR(grf);
+		return PTR_ERR(phy_base->reg_base);
 	}
 
 	for_each_available_child_of_node(dev->of_node, child) {
@@ -126,7 +137,6 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
 		}
 
 		rk_phy->reg_offset = reg_offset;
-		rk_phy->reg_base = grf;
 
 		rk_phy->clk = of_clk_get_by_name(child, "phyclk");
 		if (IS_ERR(rk_phy->clk))
-- 
2.6.2

[PATCH 3/8] phy: rockchip-usb: move per-phy init into a separate function

[Heiko Stuebner]

This unclutters the loop in probe a lot and makes current (and future)
error handling easier to read.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/phy/phy-rockchip-usb.c | 82 ++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 35 deletions(-)

diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
index dda1994..f10e130 100644
--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -103,14 +103,57 @@ static void rockchip_usb_phy_action(void *data)
 		clk_put(rk_phy->clk);
 }
 
+static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
+				 struct device_node *child)
+{
+	struct rockchip_usb_phy *rk_phy;
+	unsigned int reg_offset;
+	int err;
+
+	rk_phy = devm_kzalloc(base->dev, sizeof(*rk_phy), GFP_KERNEL);
+	if (!rk_phy)
+		return -ENOMEM;
+
+	rk_phy->base = base;
+
+	if (of_property_read_u32(child, "reg", &reg_offset)) {
+		dev_err(base->dev, "missing reg property in node %s\n",
+			child->name);
+		return -EINVAL;
+	}
+
+	rk_phy->reg_offset = reg_offset;
+
+	rk_phy->clk = of_clk_get_by_name(child, "phyclk");
+	if (IS_ERR(rk_phy->clk))
+		rk_phy->clk = NULL;
+
+	err = devm_add_action(base->dev, rockchip_usb_phy_action, rk_phy);
+	if (err)
+		goto err_devm_action;
+
+	rk_phy->phy = devm_phy_create(base->dev, child, &ops);
+	if (IS_ERR(rk_phy->phy)) {
+		dev_err(base->dev, "failed to create PHY\n");
+		return PTR_ERR(rk_phy->phy);
+	}
+	phy_set_drvdata(rk_phy->phy, rk_phy);
+
+	/* only power up usb phy when it use, so disable it when init*/
+	return rockchip_usb_phy_power(rk_phy, 1);
+
+err_devm_action:
+	if (rk_phy->clk)
+		clk_put(rk_phy->clk);
+	return err;
+}
+
 static int rockchip_usb_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rockchip_usb_phy_base *phy_base;
-	struct rockchip_usb_phy *rk_phy;
 	struct phy_provider *phy_provider;
 	struct device_node *child;
-	unsigned int reg_offset;
 	int err;
 
 	phy_base = devm_kzalloc(dev, sizeof(*phy_base), GFP_KERNEL);
@@ -126,39 +169,8 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
 	}
 
 	for_each_available_child_of_node(dev->of_node, child) {
-		rk_phy = devm_kzalloc(dev, sizeof(*rk_phy), GFP_KERNEL);
-		if (!rk_phy)
-			return -ENOMEM;
-
-		if (of_property_read_u32(child, "reg", &reg_offset)) {
-			dev_err(dev, "missing reg property in node %s\n",
-				child->name);
-			return -EINVAL;
-		}
-
-		rk_phy->reg_offset = reg_offset;
-
-		rk_phy->clk = of_clk_get_by_name(child, "phyclk");
-		if (IS_ERR(rk_phy->clk))
-			rk_phy->clk = NULL;
-
-		err = devm_add_action(dev, rockchip_usb_phy_action, rk_phy);
-		if (err) {
-			if (rk_phy->clk)
-				clk_put(rk_phy->clk);
-			return err;
-		}
-
-		rk_phy->phy = devm_phy_create(dev, child, &ops);
-		if (IS_ERR(rk_phy->phy)) {
-			dev_err(dev, "failed to create PHY\n");
-			return PTR_ERR(rk_phy->phy);
-		}
-		phy_set_drvdata(rk_phy->phy, rk_phy);
-
-		/* only power up usb phy when it use, so disable it when init*/
-		err = rockchip_usb_phy_power(rk_phy, 1);
-		if (err)
+		err = rockchip_usb_phy_init(phy_base, child);
+		if (err < 0)
 			return err;
 	}
 
-- 
2.6.2

[PATCH 4/8] phy: rockchip-usb: expose the phy-internal PLLs

[Heiko Stuebner]

The USB phys on Rockchip SoCs contain their own internal PLLs to create
the 480MHz needed. Additionally this PLL output is also fed back into the
core clock-controller as possible source for clocks like the GPU or others.

Until now this was modelled incorrectly with a "virtual" factor clock in
the clock controller. The one big caveat is that if we turn off the usb phy
via the siddq signal, all analog components get turned off, including the
PLLs. It is therefore possible that a source clock gets disabled without
the clock driver ever knowing, possibly making the system hang.

Therefore register the phy-plls as real clocks that the clock driver can
then reference again normally, making the clock hirarchy finally reflect
the actual hardware.

The phy-ops get converted to simply turning that new clock on and off
which in turn controls the siddq signal of the phy.

Through this the driver gains handling for platform-specific data, to
handle the phy->clock name association.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../devicetree/bindings/phy/rockchip-usb-phy.txt   |   6 +-
 drivers/phy/phy-rockchip-usb.c                     | 177 ++++++++++++++++++---
 2 files changed, 160 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
index 826454a..68498d5 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
@@ -1,7 +1,10 @@
 ROCKCHIP USB2 PHY
 
 Required properties:
- - compatible: rockchip,rk3288-usb-phy
+ - compatible: matching the soc type, one of
+     "rockchip,rk3066a-usb-phy"
+     "rockchip,rk3188-usb-phy"
+     "rockchip,rk3288-usb-phy"
  - rockchip,grf : phandle to the syscon managing the "general
    register files"
  - #address-cells: should be 1
@@ -21,6 +24,7 @@ required properties:
 Optional Properties:
 - clocks : phandle + clock specifier for the phy clocks
 - clock-names: string, clock name, must be "phyclk"
+- #clock-cells: for users of the phy-pll, should be 0
 
 Example:
 
diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
index f10e130..509497b 100644
--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -15,12 +15,14 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -36,18 +38,35 @@
 #define SIDDQ_ON		BIT(13)
 #define SIDDQ_OFF		(0 << 13)
 
+struct rockchip_usb_phys {
+	int reg;
+	const char *pll_name;
+};
+
+struct rockchip_usb_phy_pdata {
+	struct rockchip_usb_phys *phys;
+};
+
 struct rockchip_usb_phy_base {
 	struct device *dev;
 	struct regmap *reg_base;
+	const struct rockchip_usb_phy_pdata *pdata;
 };
 
 struct rockchip_usb_phy {
 	struct rockchip_usb_phy_base *base;
+	struct device_node *np;
 	unsigned int	reg_offset;
 	struct clk	*clk;
+	struct clk      *clk480m;
+	struct clk_hw	clk480m_hw;
 	struct phy	*phy;
 };
 
+/*
+ * Set siddq to 1 to power down usb phy analog blocks,
+ * set to 0 to enable.
+ */
 static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
 					   bool siddq)
 {
@@ -55,17 +74,57 @@ static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
 			    SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
 }
 
-static int rockchip_usb_phy_power_off(struct phy *_phy)
+static unsigned long rockchip_usb_phy480m_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
 {
-	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
-	int ret = 0;
+	return 480000000;
+}
+
+static void rockchip_usb_phy480m_disable(struct clk_hw *hw)
+{
+	struct rockchip_usb_phy *phy = container_of(hw,
+						    struct rockchip_usb_phy,
+						    clk480m_hw);
+
+	rockchip_usb_phy_power(phy, 1);
+}
+
+static int rockchip_usb_phy480m_enable(struct clk_hw *hw)
+{
+	struct rockchip_usb_phy *phy = container_of(hw,
+						    struct rockchip_usb_phy,
+						    clk480m_hw);
 
-	/* Power down usb phy analog blocks by set siddq 1 */
-	ret = rockchip_usb_phy_power(phy, 1);
-	if (ret)
+	return rockchip_usb_phy_power(phy, 0);
+}
+
+static int rockchip_usb_phy480m_is_enabled(struct clk_hw *hw)
+{
+	struct rockchip_usb_phy *phy = container_of(hw,
+						    struct rockchip_usb_phy,
+						    clk480m_hw);
+	int ret;
+	u32 val;
+
+	ret = regmap_read(phy->base->reg_base, phy->reg_offset, &val);
+	if (ret < 0)
 		return ret;
 
-	clk_disable_unprepare(phy->clk);
+	return (val & SIDDQ_ON) ? 0 : 1;
+}
+
+static const struct clk_ops rockchip_usb_phy480m_ops = {
+	.enable = rockchip_usb_phy480m_enable,
+	.disable = rockchip_usb_phy480m_disable,
+	.is_enabled = rockchip_usb_phy480m_is_enabled,
+	.recalc_rate = rockchip_usb_phy480m_recalc_rate,
+};
+
+static int rockchip_usb_phy_power_off(struct phy *_phy)
+{
+	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
+
+	clk_disable_unprepare(phy->clk480m);
 
 	return 0;
 }
@@ -73,20 +132,8 @@ static int rockchip_usb_phy_power_off(struct phy *_phy)
 static int rockchip_usb_phy_power_on(struct phy *_phy)
 {
 	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
-	int ret = 0;
-
-	ret = clk_prepare_enable(phy->clk);
-	if (ret)
-		return ret;
-
-	/* Power up usb phy analog blocks by set siddq 0 */
-	ret = rockchip_usb_phy_power(phy, 0);
-	if (ret) {
-		clk_disable_unprepare(phy->clk);
-		return ret;
-	}
 
-	return 0;
+	return clk_prepare_enable(phy->clk480m);
 }
 
 static const struct phy_ops ops = {
@@ -99,6 +146,9 @@ static void rockchip_usb_phy_action(void *data)
 {
 	struct rockchip_usb_phy *rk_phy = data;
 
+	of_clk_del_provider(rk_phy->np);
+	clk_unregister(rk_phy->clk480m);
+
 	if (rk_phy->clk)
 		clk_put(rk_phy->clk);
 }
@@ -108,13 +158,16 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
 {
 	struct rockchip_usb_phy *rk_phy;
 	unsigned int reg_offset;
-	int err;
+	const char *clk_name;
+	struct clk_init_data init;
+	int err, i;
 
 	rk_phy = devm_kzalloc(base->dev, sizeof(*rk_phy), GFP_KERNEL);
 	if (!rk_phy)
 		return -ENOMEM;
 
 	rk_phy->base = base;
+	rk_phy->np = child;
 
 	if (of_property_read_u32(child, "reg", &reg_offset)) {
 		dev_err(base->dev, "missing reg property in node %s\n",
@@ -128,6 +181,46 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
 	if (IS_ERR(rk_phy->clk))
 		rk_phy->clk = NULL;
 
+	i = 0;
+	init.name = NULL;
+	while (base->pdata->phys[i].reg) {
+		if (base->pdata->phys[i].reg == reg_offset) {
+			init.name = base->pdata->phys[i].pll_name;
+			break;
+		}
+		i++;
+	}
+
+	if (!init.name) {
+		dev_err(base->dev, "phy data not found\n");
+		return -EINVAL;
+	}
+
+	if (rk_phy->clk) {
+		clk_name = __clk_get_name(rk_phy->clk);
+		init.flags = 0;
+		init.parent_names = &clk_name;
+		init.num_parents = 1;
+	} else {
+		init.flags = CLK_IS_ROOT;
+		init.parent_names = NULL;
+		init.num_parents = 0;
+	}
+
+	init.ops = &rockchip_usb_phy480m_ops;
+	rk_phy->clk480m_hw.init = &init;
+
+	rk_phy->clk480m = clk_register(base->dev, &rk_phy->clk480m_hw);
+	if (IS_ERR(rk_phy->clk480m)) {
+		err = PTR_ERR(rk_phy->clk480m);
+		goto err_clk;
+	}
+
+	err = of_clk_add_provider(child, of_clk_src_simple_get,
+				  rk_phy->clk480m);
+	if (err < 0)
+		goto err_clk_prov;
+
 	err = devm_add_action(base->dev, rockchip_usb_phy_action, rk_phy);
 	if (err)
 		goto err_devm_action;
@@ -143,16 +236,46 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
 	return rockchip_usb_phy_power(rk_phy, 1);
 
 err_devm_action:
+	of_clk_del_provider(child);
+err_clk_prov:
+	clk_unregister(rk_phy->clk480m);
+err_clk:
 	if (rk_phy->clk)
 		clk_put(rk_phy->clk);
 	return err;
 }
 
+static const struct rockchip_usb_phy_pdata rk3066a_pdata = {
+	.phys = (struct rockchip_usb_phys[]){
+		{ .reg = 0x17c, .pll_name = "sclk_otgphy0_480m" },
+		{ .reg = 0x188, .pll_name = "sclk_otgphy1_480m" },
+		{ /* sentinel */ }
+	},
+};
+
+static const struct rockchip_usb_phy_pdata rk3188_pdata = {
+	.phys = (struct rockchip_usb_phys[]){
+		{ .reg = 0x10c, .pll_name = "sclk_otgphy0_480m" },
+		{ .reg = 0x11c, .pll_name = "sclk_otgphy1_480m" },
+		{ /* sentinel */ }
+	},
+};
+
+static const struct rockchip_usb_phy_pdata rk3288_pdata = {
+	.phys = (struct rockchip_usb_phys[]){
+		{ .reg = 0x320, .pll_name = "sclk_otgphy0_480m" },
+		{ .reg = 0x334, .pll_name = "sclk_otgphy1_480m" },
+		{ .reg = 0x348, .pll_name = "sclk_otgphy2_480m" },
+		{ /* sentinel */ }
+	},
+};
+
 static int rockchip_usb_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rockchip_usb_phy_base *phy_base;
 	struct phy_provider *phy_provider;
+	const struct of_device_id *match;
 	struct device_node *child;
 	int err;
 
@@ -160,6 +283,14 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
 	if (!phy_base)
 		return -ENOMEM;
 
+	match = of_match_device(dev->driver->of_match_table, dev);
+	if (!match || !match->data) {
+		dev_err(dev, "missing phy data\n");
+		return -EINVAL;
+	}
+
+	phy_base->pdata = match->data;
+
 	phy_base->dev = dev;
 	phy_base->reg_base = syscon_regmap_lookup_by_phandle(dev->of_node,
 							     "rockchip,grf");
@@ -179,7 +310,9 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id rockchip_usb_phy_dt_ids[] = {
-	{ .compatible = "rockchip,rk3288-usb-phy" },
+	{ .compatible = "rockchip,rk3066a-usb-phy", .data = &rk3066a_pdata },
+	{ .compatible = "rockchip,rk3188-usb-phy", .data = &rk3188_pdata },
+	{ .compatible = "rockchip,rk3288-usb-phy", .data = &rk3288_pdata },
 	{}
 };
 
-- 
2.6.2

[PATCH 5/8] clk: rockchip: fix usbphy-related clocks

[Heiko Stuebner]

The otgphy clocks really only drive the phy blocks. These in turn
contain plls that then generate the 480m clocks the clock controller
uses to supply some other clocks like uart0, gpu or the video-codec.

So fix this structure to actually respect that hirarchy and removed
that usb480m fixed-rate clock working as a placeholder till now, as
this wouldn't even work if the supplying phy gets turned off while
its pll-output gets used elsewhere.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3188.c | 11 +++--------
 drivers/clk/rockchip/clk-rk3288.c | 16 +++++-----------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index abb4760..7836a97 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -319,9 +319,9 @@ static struct rockchip_clk_branch common_clk_branches[] __initdata = {
 	 * the 480m are generated inside the usb block from these clocks,
 	 * but they are also a source for the hsicphy clock.
 	 */
-	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "usb480m", CLK_IGNORE_UNUSED,
+	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "xin24m", CLK_IGNORE_UNUSED,
 			RK2928_CLKGATE_CON(1), 5, GFLAGS),
-	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "usb480m", CLK_IGNORE_UNUSED,
+	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "xin24m", CLK_IGNORE_UNUSED,
 			RK2928_CLKGATE_CON(1), 6, GFLAGS),
 
 	COMPOSITE(0, "mac_src", mux_mac_p, 0,
@@ -635,7 +635,7 @@ static struct clk_div_table div_rk3188_aclk_core_t[] = {
 	{ /* sentinel */ },
 };
 
-PNAME(mux_hsicphy_p)		= { "sclk_otgphy0", "sclk_otgphy1",
+PNAME(mux_hsicphy_p)		= { "sclk_otgphy0_480m", "sclk_otgphy1_480m",
 				    "gpll", "cpll" };
 
 static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
@@ -739,11 +739,6 @@ static void __init rk3188_common_clk_init(struct device_node *np)
 		pr_warn("%s: could not register clock xin12m: %ld\n",
 			__func__, PTR_ERR(clk));
 
-	clk = clk_register_fixed_factor(NULL, "usb480m", "xin24m", 0, 20, 1);
-	if (IS_ERR(clk))
-		pr_warn("%s: could not register clock usb480m: %ld\n",
-			__func__, PTR_ERR(clk));
-
 	rockchip_clk_register_branches(common_clk_branches,
 				  ARRAY_SIZE(common_clk_branches));
 
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 9040878..7c8a3e9 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -195,8 +195,8 @@ PNAME(mux_hsadcout_p)	= { "hsadc_src", "ext_hsadc" };
 PNAME(mux_edp_24m_p)	= { "ext_edp_24m", "xin24m" };
 PNAME(mux_tspout_p)	= { "cpll", "gpll", "npll", "xin27m" };
 
-PNAME(mux_usbphy480m_p)		= { "sclk_otgphy1", "sclk_otgphy2",
-				    "sclk_otgphy0" };
+PNAME(mux_usbphy480m_p)		= { "sclk_otgphy1_480m", "sclk_otgphy2_480m",
+				    "sclk_otgphy0_480m" };
 PNAME(mux_hsicphy480m_p)	= { "cpll", "gpll", "usbphy480m_src" };
 PNAME(mux_hsicphy12m_p)		= { "hsicphy12m_xin12m", "hsicphy12m_usbphy" };
 
@@ -506,11 +506,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 			RK3288_CLKSEL_CON(35), 6, 2, MFLAGS, 0, 5, DFLAGS,
 			RK3288_CLKGATE_CON(4), 10, GFLAGS),
 
-	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "usb480m", CLK_IGNORE_UNUSED,
+	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "xin24m", CLK_IGNORE_UNUSED,
 			RK3288_CLKGATE_CON(13), 4, GFLAGS),
-	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "usb480m", CLK_IGNORE_UNUSED,
+	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "xin24m", CLK_IGNORE_UNUSED,
 			RK3288_CLKGATE_CON(13), 5, GFLAGS),
-	GATE(SCLK_OTGPHY2, "sclk_otgphy2", "usb480m", CLK_IGNORE_UNUSED,
+	GATE(SCLK_OTGPHY2, "sclk_otgphy2", "xin24m", CLK_IGNORE_UNUSED,
 			RK3288_CLKGATE_CON(13), 6, GFLAGS),
 	GATE(SCLK_OTG_ADP, "sclk_otg_adp", "xin32k", CLK_IGNORE_UNUSED,
 			RK3288_CLKGATE_CON(13), 7, GFLAGS),
@@ -874,12 +874,6 @@ static void __init rk3288_clk_init(struct device_node *np)
 		pr_warn("%s: could not register clock xin12m: %ld\n",
 			__func__, PTR_ERR(clk));
 
-
-	clk = clk_register_fixed_factor(NULL, "usb480m", "xin24m", 0, 20, 1);
-	if (IS_ERR(clk))
-		pr_warn("%s: could not register clock usb480m: %ld\n",
-			__func__, PTR_ERR(clk));
-
 	clk = clk_register_fixed_factor(NULL, "hclk_vcodec_pre",
 					"hclk_vcodec_pre_v", 0, 1, 4);
 	if (IS_ERR(clk))
-- 
2.6.2

[PATCH 6/8] ARM: dts: rockchip: add clock-cells for usb phy nodes

[Heiko Stuebner]

Add the #clock-cells properties for the usbphy nodes as they
provide the pll-clocks now.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3066a.dtsi | 2 ++
 arch/arm/boot/dts/rk3188.dtsi  | 2 ++
 arch/arm/boot/dts/rk3288.dtsi  | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index 946f187..3e4b41b 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -181,6 +181,7 @@
 			reg = <0x17c>;
 			clocks = <&cru SCLK_OTGPHY0>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 
 		usbphy1: usb-phy1 {
@@ -188,6 +189,7 @@
 			reg = <0x188>;
 			clocks = <&cru SCLK_OTGPHY1>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 6399942..48a287e 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -156,6 +156,7 @@
 			reg = <0x10c>;
 			clocks = <&cru SCLK_OTGPHY0>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 
 		usbphy1: usb-phy1 {
@@ -163,6 +164,7 @@
 			reg = <0x11c>;
 			clocks = <&cru SCLK_OTGPHY1>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index c64a116..51a5d29 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -943,6 +943,7 @@
 			reg = <0x320>;
 			clocks = <&cru SCLK_OTGPHY0>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 
 		usbphy1: usb-phy1 {
@@ -950,6 +951,7 @@
 			reg = <0x334>;
 			clocks = <&cru SCLK_OTGPHY1>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 
 		usbphy2: usb-phy2 {
@@ -957,6 +959,7 @@
 			reg = <0x348>;
 			clocks = <&cru SCLK_OTGPHY2>;
 			clock-names = "phyclk";
+			#clock-cells = <0>;
 		};
 	};
 
-- 
2.6.2

[PATCH 7/8] ARM: dts: rockchip: assign usbphy480m_src to the new usbphy pll on veyron

[Heiko Stuebner]

Veyron devices try to always set the source for usbphy480m to the usbphy0
that is the phy connected to the otg controller, because the firmware-
default is usbphy1, the ehci-controller connected to the internal camera
that might get turned off way easier to save power.

In the mainline kernel we currently don't use the usbphy480m_src at all,
as it mainly powers the uart0 source that is connected to the bluetooth
component of the wifi/bt combo.

So move that assignment over to the new real pll clock inside the usbphy.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index d4263ed..c8329b5 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -410,7 +410,7 @@
 	status = "okay";
 
 	assigned-clocks = <&cru SCLK_USBPHY480M_SRC>;
-	assigned-clock-parents = <&cru SCLK_OTGPHY0>;
+	assigned-clock-parents = <&usbphy0>;
 	dr_mode = "host";
 };
 
-- 
2.6.2

[PATCH 8/8] phy: rockchip-usb: add handler for usb-uart functionality

[Heiko Stuebner]

Most newer Rockchip SoCs provide the possibility to use a usb-phy
as passthrough for the debug uart (uart2), making it possible to
for example get console output without needing to open the device.

This patch adds an early_initcall to enable this functionality
conditionally via the commandline and also disables the corresponding
usb controller in the devicetree.

Currently only data for the rk3288 is provided, but at least the
rk3188 and arm64 rk3368 also provide this functionality and will be
enabled later.

On a spliced usb cable the signals are tx on white wire(D+) and
rx on green wire(D-).

The one caveat is that currently the reconfiguration of the phy
happens as early_initcall, as the code depends on the unflattened
devicetree being available. Everything is fine if only a regular
console is active as the console-replay will happen after the
reconfiguation. But with earlycon active output up to smp-init
currently will get lost.

The phy is an optional property for the connected dwc2 controller,
so we still provide the phy device but fail all phy-ops with -EBUSY
to make sure the dwc2 does not try to transmit anything on the
repurposed phy.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/kernel-parameters.txt |   6 +
 drivers/phy/phy-rockchip-usb.c      | 231 ++++++++++++++++++++++++++++++------
 2 files changed, 201 insertions(+), 36 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c6dd5f3..8d9a86e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3370,6 +3370,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	ro		[KNL] Mount root device read-only on boot
 
+	rockchip.usb_uart
+			Enable the uart passthrough on the designated usb port
+			on Rockchip SoCs. When active, the signals of the
+			debug-uart get routed to the D+ and D- pins of the usb
+			port and the regular usb controller gets disabled.
+
 	root=		[KNL] Root filesystem
 			See name_to_dev_t comment in init/do_mounts.c.
 
diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
index 509497b..4c2f5ee 100644
--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -30,21 +30,23 @@
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 
-/*
- * The higher 16-bit of this register is used for write protection
- * only if BIT(13 + 16) set to 1 the BIT(13) can be written.
- */
-#define SIDDQ_WRITE_ENA	BIT(29)
-#define SIDDQ_ON		BIT(13)
-#define SIDDQ_OFF		(0 << 13)
+static int enable_usb_uart;
+
+#define HIWORD_UPDATE(val, mask) \
+		((val) | (mask) << 16)
+
+#define UOC_CON0_SIDDQ BIT(13)
 
 struct rockchip_usb_phys {
 	int reg;
 	const char *pll_name;
 };
 
+struct rockchip_usb_phy_base;
 struct rockchip_usb_phy_pdata {
 	struct rockchip_usb_phys *phys;
+	int (*init_usb_uart)(struct regmap *grf);
+	int usb_uart_phy;
 };
 
 struct rockchip_usb_phy_base {
@@ -61,6 +63,7 @@ struct rockchip_usb_phy {
 	struct clk      *clk480m;
 	struct clk_hw	clk480m_hw;
 	struct phy	*phy;
+	bool		uart_enabled;
 };
 
 /*
@@ -70,8 +73,9 @@ struct rockchip_usb_phy {
 static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
 					   bool siddq)
 {
-	return regmap_write(phy->base->reg_base, phy->reg_offset,
-			    SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
+	u32 val = HIWORD_UPDATE(siddq ? UOC_CON0_SIDDQ : 0, UOC_CON0_SIDDQ);
+
+	return regmap_write(phy->base->reg_base, phy->reg_offset, val);
 }
 
 static unsigned long rockchip_usb_phy480m_recalc_rate(struct clk_hw *hw,
@@ -110,7 +114,7 @@ static int rockchip_usb_phy480m_is_enabled(struct clk_hw *hw)
 	if (ret < 0)
 		return ret;
 
-	return (val & SIDDQ_ON) ? 0 : 1;
+	return (val & UOC_CON0_SIDDQ) ? 0 : 1;
 }
 
 static const struct clk_ops rockchip_usb_phy480m_ops = {
@@ -124,6 +128,9 @@ static int rockchip_usb_phy_power_off(struct phy *_phy)
 {
 	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
 
+	if (phy->uart_enabled)
+		return -EBUSY;
+
 	clk_disable_unprepare(phy->clk480m);
 
 	return 0;
@@ -133,6 +140,9 @@ static int rockchip_usb_phy_power_on(struct phy *_phy)
 {
 	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
 
+	if (phy->uart_enabled)
+		return -EBUSY;
+
 	return clk_prepare_enable(phy->clk480m);
 }
 
@@ -146,8 +156,10 @@ static void rockchip_usb_phy_action(void *data)
 {
 	struct rockchip_usb_phy *rk_phy = data;
 
-	of_clk_del_provider(rk_phy->np);
-	clk_unregister(rk_phy->clk480m);
+	if (!rk_phy->uart_enabled) {
+		of_clk_del_provider(rk_phy->np);
+		clk_unregister(rk_phy->clk480m);
+	}
 
 	if (rk_phy->clk)
 		clk_put(rk_phy->clk);
@@ -196,30 +208,35 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
 		return -EINVAL;
 	}
 
-	if (rk_phy->clk) {
-		clk_name = __clk_get_name(rk_phy->clk);
-		init.flags = 0;
-		init.parent_names = &clk_name;
-		init.num_parents = 1;
+	if (enable_usb_uart && base->pdata->usb_uart_phy == i) {
+		dev_dbg(base->dev, "phy%d used as uart output\n", i);
+		rk_phy->uart_enabled = true;
 	} else {
-		init.flags = CLK_IS_ROOT;
-		init.parent_names = NULL;
-		init.num_parents = 0;
-	}
+		if (rk_phy->clk) {
+			clk_name = __clk_get_name(rk_phy->clk);
+			init.flags = 0;
+			init.parent_names = &clk_name;
+			init.num_parents = 1;
+		} else {
+			init.flags = CLK_IS_ROOT;
+			init.parent_names = NULL;
+			init.num_parents = 0;
+		}
 
-	init.ops = &rockchip_usb_phy480m_ops;
-	rk_phy->clk480m_hw.init = &init;
+		init.ops = &rockchip_usb_phy480m_ops;
+		rk_phy->clk480m_hw.init = &init;
 
-	rk_phy->clk480m = clk_register(base->dev, &rk_phy->clk480m_hw);
-	if (IS_ERR(rk_phy->clk480m)) {
-		err = PTR_ERR(rk_phy->clk480m);
-		goto err_clk;
-	}
+		rk_phy->clk480m = clk_register(base->dev, &rk_phy->clk480m_hw);
+		if (IS_ERR(rk_phy->clk480m)) {
+			err = PTR_ERR(rk_phy->clk480m);
+			goto err_clk;
+		}
 
-	err = of_clk_add_provider(child, of_clk_src_simple_get,
-				  rk_phy->clk480m);
-	if (err < 0)
-		goto err_clk_prov;
+		err = of_clk_add_provider(child, of_clk_src_simple_get,
+					rk_phy->clk480m);
+		if (err < 0)
+			goto err_clk_prov;
+	}
 
 	err = devm_add_action(base->dev, rockchip_usb_phy_action, rk_phy);
 	if (err)
@@ -232,13 +249,21 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
 	}
 	phy_set_drvdata(rk_phy->phy, rk_phy);
 
-	/* only power up usb phy when it use, so disable it when init*/
-	return rockchip_usb_phy_power(rk_phy, 1);
+	/*
+	 * When acting as uart-pipe, just keep clock on otherwise
+	 * only power up usb phy when it use, so disable it when init
+	 */
+	if (rk_phy->uart_enabled)
+		return clk_prepare_enable(rk_phy->clk);
+	else
+		return rockchip_usb_phy_power(rk_phy, 1);
 
 err_devm_action:
-	of_clk_del_provider(child);
+	if (!rk_phy->uart_enabled)
+		of_clk_del_provider(child);
 err_clk_prov:
-	clk_unregister(rk_phy->clk480m);
+	if (!rk_phy->uart_enabled)
+		clk_unregister(rk_phy->clk480m);
 err_clk:
 	if (rk_phy->clk)
 		clk_put(rk_phy->clk);
@@ -261,6 +286,86 @@ static const struct rockchip_usb_phy_pdata rk3188_pdata = {
 	},
 };
 
+#define RK3288_UOC0_CON0				0x320
+#define RK3288_UOC0_CON0_COMMON_ON_N			BIT(0)
+#define RK3288_UOC0_CON0_DISABLE			BIT(4)
+
+#define RK3288_UOC0_CON2				0x328
+#define RK3288_UOC0_CON2_SOFT_CON_SEL			BIT(2)
+
+#define RK3288_UOC0_CON3				0x32c
+#define RK3288_UOC0_CON3_UTMI_SUSPENDN			BIT(0)
+#define RK3288_UOC0_CON3_UTMI_OPMODE_NODRIVING		(1 << 1)
+#define RK3288_UOC0_CON3_UTMI_OPMODE_MASK		(3 << 1)
+#define RK3288_UOC0_CON3_UTMI_XCVRSEELCT_FSTRANSC	(1 << 3)
+#define RK3288_UOC0_CON3_UTMI_XCVRSEELCT_MASK		(3 << 3)
+#define RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED		BIT(5)
+#define RK3288_UOC0_CON3_BYPASSDMEN			BIT(6)
+#define RK3288_UOC0_CON3_BYPASSSEL			BIT(7)
+
+/*
+ * Enable the bypass of uart2 data through the otg usb phy.
+ * Original description in the TRM.
+ * 1. Disable the OTG block by setting OTGDISABLE0 to 1’b1.
+ * 2. Disable the pull-up resistance on the D+ line by setting
+ *    OPMODE0[1:0] to 2’b01.
+ * 3. To ensure that the XO, Bias, and PLL blocks are powered down in Suspend
+ *    mode, set COMMONONN to 1’b1.
+ * 4. Place the USB PHY in Suspend mode by setting SUSPENDM0 to 1’b0.
+ * 5. Set BYPASSSEL0 to 1’b1.
+ * 6. To transmit data, controls BYPASSDMEN0, and BYPASSDMDATA0.
+ * To receive data, monitor FSVPLUS0.
+ *
+ * The actual code in the vendor kernel does some things differently.
+ */
+static int __init rk3288_init_usb_uart(struct regmap *grf)
+{
+	u32 val;
+	int ret;
+
+	/*
+	 * COMMON_ON and DISABLE settings are described in the TRM,
+	 * but were not present in the original code.
+	 * Also disable the analog phy components to save power.
+	 */
+	val = HIWORD_UPDATE(RK3288_UOC0_CON0_COMMON_ON_N
+				| RK3288_UOC0_CON0_DISABLE
+				| UOC_CON0_SIDDQ,
+			    RK3288_UOC0_CON0_COMMON_ON_N
+				| RK3288_UOC0_CON0_DISABLE
+				| UOC_CON0_SIDDQ);
+	ret = regmap_write(grf, RK3288_UOC0_CON0, val);
+	if (ret)
+		return ret;
+
+	val = HIWORD_UPDATE(RK3288_UOC0_CON2_SOFT_CON_SEL,
+			    RK3288_UOC0_CON2_SOFT_CON_SEL);
+	ret = regmap_write(grf, RK3288_UOC0_CON2, val);
+	if (ret)
+		return ret;
+
+	val = HIWORD_UPDATE(RK3288_UOC0_CON3_UTMI_OPMODE_NODRIVING
+				| RK3288_UOC0_CON3_UTMI_XCVRSEELCT_FSTRANSC
+				| RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED,
+			    RK3288_UOC0_CON3_UTMI_SUSPENDN
+				| RK3288_UOC0_CON3_UTMI_OPMODE_MASK
+				| RK3288_UOC0_CON3_UTMI_XCVRSEELCT_MASK
+				| RK3288_UOC0_CON3_UTMI_TERMSEL_FULLSPEED);
+	ret = regmap_write(grf, RK3288_UOC0_CON3, val);
+	if (ret)
+		return ret;
+
+	val = HIWORD_UPDATE(RK3288_UOC0_CON3_BYPASSSEL
+				| RK3288_UOC0_CON3_BYPASSDMEN,
+			    RK3288_UOC0_CON3_BYPASSSEL
+				| RK3288_UOC0_CON3_BYPASSDMEN);
+	ret = regmap_write(grf, RK3288_UOC0_CON3, val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static const struct rockchip_usb_phy_pdata rk3288_pdata = {
 	.phys = (struct rockchip_usb_phys[]){
 		{ .reg = 0x320, .pll_name = "sclk_otgphy0_480m" },
@@ -268,6 +373,8 @@ static const struct rockchip_usb_phy_pdata rk3288_pdata = {
 		{ .reg = 0x348, .pll_name = "sclk_otgphy2_480m" },
 		{ /* sentinel */ }
 	},
+	.init_usb_uart = rk3288_init_usb_uart,
+	.usb_uart_phy = 0,
 };
 
 static int rockchip_usb_phy_probe(struct platform_device *pdev)
@@ -328,6 +435,58 @@ static struct platform_driver rockchip_usb_driver = {
 
 module_platform_driver(rockchip_usb_driver);
 
+static int __init rockchip_init_usb_uart(void)
+{
+	const struct of_device_id *match;
+	const struct rockchip_usb_phy_pdata *data;
+	struct device_node *np;
+	struct regmap *grf;
+	int ret;
+
+	if (!enable_usb_uart)
+		return 0;
+
+	np = of_find_matching_node_and_match(NULL, rockchip_usb_phy_dt_ids,
+					     &match);
+	if (!np) {
+		pr_err("%s: failed to find usbphy node\n", __func__);
+		return -ENOTSUPP;
+	}
+
+	pr_debug("%s: using settings for %s\n", __func__, match->compatible);
+	data = match->data;
+
+	if (!data->init_usb_uart) {
+		pr_err("%s: usb-uart not available on %s\n",
+		       __func__, match->compatible);
+		return -ENOTSUPP;
+	}
+
+	grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	if (IS_ERR(grf)) {
+		pr_err("%s: Missing rockchip,grf property, %lu\n",
+		       __func__, PTR_ERR(grf));
+		return PTR_ERR(grf);
+	}
+
+	ret = data->init_usb_uart(grf);
+	if (ret) {
+		pr_err("%s: could not init usb_uart, %d\n", __func__, ret);
+		enable_usb_uart = 0;
+		return ret;
+	}
+
+	return 0;
+}
+early_initcall(rockchip_init_usb_uart);
+
+static int __init rockchip_usb_uart(char *buf)
+{
+	enable_usb_uart = true;
+	return 0;
+}
+early_param("rockchip.usb_uart", rockchip_usb_uart);
+
 MODULE_AUTHOR("Yunzhi Li <lyz@rock-chips.com>");
 MODULE_DESCRIPTION("Rockchip USB 2.0 PHY driver");
 MODULE_LICENSE("GPL v2");
-- 
2.6.2

Re: [PATCH 1/8] phy: rockchip-usb: fix clock get-put mismatch

[Doug Anderson]

Heiko

On Wed, Nov 4, 2015 at 1:44 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> Currently the phy driver only gets the optional clock reference but
> never puts it again, neither during error handling nor on remove.
> Fix that by moving the clk_put to a devm-action that gets called at
> the right time when all other devm actions are done.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/phy/phy-rockchip-usb.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH 2/8] phy: rockchip-usb: introduce a common data-struct for the device

[Doug Anderson]

Hi,

On Wed, Nov 4, 2015 at 1:44 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> This introduces a common struct that holds data belonging to
> the umbrella device that contains all the phys and that we
> want to use later.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/phy/phy-rockchip-usb.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> index dfc056b..dda1994 100644
> --- a/drivers/phy/phy-rockchip-usb.c
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -36,9 +36,14 @@
>  #define SIDDQ_ON               BIT(13)
>  #define SIDDQ_OFF              (0 << 13)
>
> +struct rockchip_usb_phy_base {
> +       struct device *dev;
> +       struct regmap *reg_base;
> +};
> +
>  struct rockchip_usb_phy {
> +       struct rockchip_usb_phy_base *base;
>         unsigned int    reg_offset;
> -       struct regmap   *reg_base;
>         struct clk      *clk;
>         struct phy      *phy;
>  };
> @@ -46,7 +51,7 @@ struct rockchip_usb_phy {
>  static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
>                                            bool siddq)
>  {
> -       return regmap_write(phy->reg_base, phy->reg_offset,
> +       return regmap_write(phy->base->reg_base, phy->reg_offset,
>                             SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
>  }
>
> @@ -101,17 +106,23 @@ static void rockchip_usb_phy_action(void *data)
>  static int rockchip_usb_phy_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> +       struct rockchip_usb_phy_base *phy_base;
>         struct rockchip_usb_phy *rk_phy;
>         struct phy_provider *phy_provider;
>         struct device_node *child;
> -       struct regmap *grf;
>         unsigned int reg_offset;
>         int err;
>
> -       grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
> -       if (IS_ERR(grf)) {
> +       phy_base = devm_kzalloc(dev, sizeof(*phy_base), GFP_KERNEL);
> +       if (!phy_base)
> +               return -ENOMEM;
> +
> +       phy_base->dev = dev;
> +       phy_base->reg_base = syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                                            "rockchip,grf");
> +       if (IS_ERR(phy_base->reg_base)) {
>                 dev_err(&pdev->dev, "Missing rockchip,grf property\n");
> -               return PTR_ERR(grf);
> +               return PTR_ERR(phy_base->reg_base);
>         }
>
>         for_each_available_child_of_node(dev->of_node, child) {
> @@ -126,7 +137,6 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
>                 }
>
>                 rk_phy->reg_offset = reg_offset;
> -               rk_phy->reg_base = grf;

I'm probably missing something, but I would have expected a line line:

  rk_phy->base = phy_base;

Otherwise how does "base" get assigned?  Ah, I see.  You forgot it in
this patch and then cheated and slipped it in in patch #3.  ;)  For
nice bisectability it probably belongs here, too...

-Doug

Re: [PATCH 3/8] phy: rockchip-usb: move per-phy init into a separate function

[Doug Anderson]

Heiko,

On Wed, Nov 4, 2015 at 1:44 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>         for_each_available_child_of_node(dev->of_node, child) {
> -               rk_phy = devm_kzalloc(dev, sizeof(*rk_phy), GFP_KERNEL);
> -               if (!rk_phy)
> -                       return -ENOMEM;
> -
> -               if (of_property_read_u32(child, "reg", &reg_offset)) {
> -                       dev_err(dev, "missing reg property in node %s\n",
> -                               child->name);
> -                       return -EINVAL;
> -               }
> -
> -               rk_phy->reg_offset = reg_offset;
> -
> -               rk_phy->clk = of_clk_get_by_name(child, "phyclk");
> -               if (IS_ERR(rk_phy->clk))
> -                       rk_phy->clk = NULL;
> -
> -               err = devm_add_action(dev, rockchip_usb_phy_action, rk_phy);
> -               if (err) {
> -                       if (rk_phy->clk)
> -                               clk_put(rk_phy->clk);
> -                       return err;
> -               }
> -
> -               rk_phy->phy = devm_phy_create(dev, child, &ops);
> -               if (IS_ERR(rk_phy->phy)) {
> -                       dev_err(dev, "failed to create PHY\n");
> -                       return PTR_ERR(rk_phy->phy);
> -               }
> -               phy_set_drvdata(rk_phy->phy, rk_phy);
> -
> -               /* only power up usb phy when it use, so disable it when init*/
> -               err = rockchip_usb_phy_power(rk_phy, 1);
> -               if (err)
> +               err = rockchip_usb_phy_init(phy_base, child);
> +               if (err < 0)
>                         return err;

You've also included the minor unrelated tweak to check for erorrs
with "err < 0" rather than "err" that has the incredibly small chance
to affect behavior, making this not a 100% noop change.

I don't personally care that much since it's terribly unlikely to
affect anything.  ...but you get a 1-line better diffstat if you don't
change it and it's unlikely to matter much either way...

Reviewed-by: Douglas Anderson <dianders@chromium.org>


-Doug

Re: [PATCH 2/8] phy: rockchip-usb: introduce a common data-struct for the device

[Heiko Stuebner]

Am Mittwoch, 4. November 2015, 15:46:04 schrieb Doug Anderson:
> Hi,
> 
> On Wed, Nov 4, 2015 at 1:44 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > This introduces a common struct that holds data belonging to
> > the umbrella device that contains all the phys and that we
> > want to use later.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  drivers/phy/phy-rockchip-usb.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> > index dfc056b..dda1994 100644
> > --- a/drivers/phy/phy-rockchip-usb.c
> > +++ b/drivers/phy/phy-rockchip-usb.c
> > @@ -36,9 +36,14 @@
> >  #define SIDDQ_ON               BIT(13)
> >  #define SIDDQ_OFF              (0 << 13)
> >
> > +struct rockchip_usb_phy_base {
> > +       struct device *dev;
> > +       struct regmap *reg_base;
> > +};
> > +
> >  struct rockchip_usb_phy {
> > +       struct rockchip_usb_phy_base *base;
> >         unsigned int    reg_offset;
> > -       struct regmap   *reg_base;
> >         struct clk      *clk;
> >         struct phy      *phy;
> >  };
> > @@ -46,7 +51,7 @@ struct rockchip_usb_phy {
> >  static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> >                                            bool siddq)
> >  {
> > -       return regmap_write(phy->reg_base, phy->reg_offset,
> > +       return regmap_write(phy->base->reg_base, phy->reg_offset,
> >                             SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
> >  }
> >
> > @@ -101,17 +106,23 @@ static void rockchip_usb_phy_action(void *data)
> >  static int rockchip_usb_phy_probe(struct platform_device *pdev)
> >  {
> >         struct device *dev = &pdev->dev;
> > +       struct rockchip_usb_phy_base *phy_base;
> >         struct rockchip_usb_phy *rk_phy;
> >         struct phy_provider *phy_provider;
> >         struct device_node *child;
> > -       struct regmap *grf;
> >         unsigned int reg_offset;
> >         int err;
> >
> > -       grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
> > -       if (IS_ERR(grf)) {
> > +       phy_base = devm_kzalloc(dev, sizeof(*phy_base), GFP_KERNEL);
> > +       if (!phy_base)
> > +               return -ENOMEM;
> > +
> > +       phy_base->dev = dev;
> > +       phy_base->reg_base = syscon_regmap_lookup_by_phandle(dev->of_node,
> > +                                                            "rockchip,grf");
> > +       if (IS_ERR(phy_base->reg_base)) {
> >                 dev_err(&pdev->dev, "Missing rockchip,grf property\n");
> > -               return PTR_ERR(grf);
> > +               return PTR_ERR(phy_base->reg_base);
> >         }
> >
> >         for_each_available_child_of_node(dev->of_node, child) {
> > @@ -126,7 +137,6 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
> >                 }
> >
> >                 rk_phy->reg_offset = reg_offset;
> > -               rk_phy->reg_base = grf;
> 
> I'm probably missing something, but I would have expected a line line:
> 
>   rk_phy->base = phy_base;
> 
> Otherwise how does "base" get assigned?  Ah, I see.  You forgot it in
> this patch and then cheated and slipped it in in patch #3.  ;)  For
> nice bisectability it probably belongs here, too...

Thanks for that catch and yep, it should definitly be here too. While
I did a compile-test for the individual steps, I guess I did not do
runtime tests for each.

I guess this is what happens when you try to separate a final work into
separate steps :-) . I'll send a revised version hopefully tomorrow.


Heiko

Re: [PATCH 1/8] phy: rockchip-usb: fix clock get-put mismatch

[Kishon Vijay Abraham I]

Hi,

On Thursday 05 November 2015 03:14 AM, Heiko Stuebner wrote:
> Currently the phy driver only gets the optional clock reference but
> never puts it again, neither during error handling nor on remove.
> Fix that by moving the clk_put to a devm-action that gets called at
> the right time when all other devm actions are done.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/phy/phy-rockchip-usb.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> index 91d6f34..dfc056b 100644
> --- a/drivers/phy/phy-rockchip-usb.c
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -90,6 +90,14 @@ static const struct phy_ops ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +static void rockchip_usb_phy_action(void *data)
> +{
> +	struct rockchip_usb_phy *rk_phy = data;
> +
> +	if (rk_phy->clk)
> +		clk_put(rk_phy->clk);
> +}
> +
>  static int rockchip_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -124,6 +132,13 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
>  		if (IS_ERR(rk_phy->clk))
>  			rk_phy->clk = NULL;
>  
> +		err = devm_add_action(dev, rockchip_usb_phy_action, rk_phy);
> +		if (err) {
> +			if (rk_phy->clk)
> +				clk_put(rk_phy->clk);

If devm_add_action is added before clk_get this check wouldn't be required at all.

Thanks
Kishon

Re: [PATCH 4/8] phy: rockchip-usb: expose the phy-internal PLLs

[Kishon Vijay Abraham I]

Hi,

On Thursday 05 November 2015 03:14 AM, Heiko Stuebner wrote:
> The USB phys on Rockchip SoCs contain their own internal PLLs to create
> the 480MHz needed. Additionally this PLL output is also fed back into the
> core clock-controller as possible source for clocks like the GPU or others.
> 
> Until now this was modelled incorrectly with a "virtual" factor clock in
> the clock controller. The one big caveat is that if we turn off the usb phy
> via the siddq signal, all analog components get turned off, including the
> PLLs. It is therefore possible that a source clock gets disabled without
> the clock driver ever knowing, possibly making the system hang.
> 
> Therefore register the phy-plls as real clocks that the clock driver can
> then reference again normally, making the clock hirarchy finally reflect
> the actual hardware.
> 
> The phy-ops get converted to simply turning that new clock on and off
> which in turn controls the siddq signal of the phy.
> 
> Through this the driver gains handling for platform-specific data, to
> handle the phy->clock name association.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  .../devicetree/bindings/phy/rockchip-usb-phy.txt   |   6 +-
>  drivers/phy/phy-rockchip-usb.c                     | 177 ++++++++++++++++++---
>  2 files changed, 160 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
> index 826454a..68498d5 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
> @@ -1,7 +1,10 @@
>  ROCKCHIP USB2 PHY
>  
>  Required properties:
> - - compatible: rockchip,rk3288-usb-phy
> + - compatible: matching the soc type, one of
> +     "rockchip,rk3066a-usb-phy"
> +     "rockchip,rk3188-usb-phy"
> +     "rockchip,rk3288-usb-phy"

Looks like this adds driver support for new SoC? Maybe create a separate patch
for that?
>   - rockchip,grf : phandle to the syscon managing the "general
>     register files"
>   - #address-cells: should be 1
> @@ -21,6 +24,7 @@ required properties:
>  Optional Properties:
>  - clocks : phandle + clock specifier for the phy clocks
>  - clock-names: string, clock name, must be "phyclk"
> +- #clock-cells: for users of the phy-pll, should be 0
>  
>  Example:
>  
> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> index f10e130..509497b 100644
> --- a/drivers/phy/phy-rockchip-usb.c
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -15,12 +15,14 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
> @@ -36,18 +38,35 @@
>  #define SIDDQ_ON		BIT(13)
>  #define SIDDQ_OFF		(0 << 13)
>  
> +struct rockchip_usb_phys {
> +	int reg;
> +	const char *pll_name;
> +};
> +
> +struct rockchip_usb_phy_pdata {
> +	struct rockchip_usb_phys *phys;
> +};
> +
>  struct rockchip_usb_phy_base {
>  	struct device *dev;
>  	struct regmap *reg_base;
> +	const struct rockchip_usb_phy_pdata *pdata;
>  };
>  
>  struct rockchip_usb_phy {
>  	struct rockchip_usb_phy_base *base;
> +	struct device_node *np;
>  	unsigned int	reg_offset;
>  	struct clk	*clk;
> +	struct clk      *clk480m;
> +	struct clk_hw	clk480m_hw;
>  	struct phy	*phy;
>  };
>  
> +/*
> + * Set siddq to 1 to power down usb phy analog blocks,
> + * set to 0 to enable.
> + */

Not related to $patch.
>  static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
>  					   bool siddq)
>  {
> @@ -55,17 +74,57 @@ static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
>  			    SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
>  }

Thanks
Kishon

RE: [PATCH 5/8] clk: rockchip: fix usbphy-related clocks

[Michael Turquette]

Heiko Stuebner wrote:
> The otgphy clocks really only drive the phy blocks. These in turn
> contain plls that then generate the 480m clocks the clock controller
> uses to supply some other clocks like uart0, gpu or the video-codec.
> 
> So fix this structure to actually respect that hirarchy and removed
> that usb480m fixed-rate clock working as a placeholder till now, as
> this wouldn't even work if the supplying phy gets turned off while
> its pll-output gets used elsewhere.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Acked-by: Michael Turquette <mturquette@baylibre.com>

> ---
>  drivers/clk/rockchip/clk-rk3188.c | 11 +++--------
>  drivers/clk/rockchip/clk-rk3288.c | 16 +++++-----------
>  2 files changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index abb4760..7836a97 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -319,9 +319,9 @@ static struct rockchip_clk_branch common_clk_branches[] __initdata = {
>  	 * the 480m are generated inside the usb block from these clocks,
>  	 * but they are also a source for the hsicphy clock.
>  	 */
> -	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "usb480m", CLK_IGNORE_UNUSED,
> +	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "xin24m", CLK_IGNORE_UNUSED,
>  			RK2928_CLKGATE_CON(1), 5, GFLAGS),
> -	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "usb480m", CLK_IGNORE_UNUSED,
> +	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "xin24m", CLK_IGNORE_UNUSED,
>  			RK2928_CLKGATE_CON(1), 6, GFLAGS),
>  
>  	COMPOSITE(0, "mac_src", mux_mac_p, 0,
> @@ -635,7 +635,7 @@ static struct clk_div_table div_rk3188_aclk_core_t[] = {
>  	{ /* sentinel */ },
>  };
>  
> -PNAME(mux_hsicphy_p)		= { "sclk_otgphy0", "sclk_otgphy1",
> +PNAME(mux_hsicphy_p)		= { "sclk_otgphy0_480m", "sclk_otgphy1_480m",
>  				    "gpll", "cpll" };
>  
>  static struct rockchip_clk_branch rk3188_clk_branches[] __initdata = {
> @@ -739,11 +739,6 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>  		pr_warn("%s: could not register clock xin12m: %ld\n",
>  			__func__, PTR_ERR(clk));
>  
> -	clk = clk_register_fixed_factor(NULL, "usb480m", "xin24m", 0, 20, 1);
> -	if (IS_ERR(clk))
> -		pr_warn("%s: could not register clock usb480m: %ld\n",
> -			__func__, PTR_ERR(clk));
> -
>  	rockchip_clk_register_branches(common_clk_branches,
>  				  ARRAY_SIZE(common_clk_branches));
>  
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 9040878..7c8a3e9 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -195,8 +195,8 @@ PNAME(mux_hsadcout_p)	= { "hsadc_src", "ext_hsadc" };
>  PNAME(mux_edp_24m_p)	= { "ext_edp_24m", "xin24m" };
>  PNAME(mux_tspout_p)	= { "cpll", "gpll", "npll", "xin27m" };
>  
> -PNAME(mux_usbphy480m_p)		= { "sclk_otgphy1", "sclk_otgphy2",
> -				    "sclk_otgphy0" };
> +PNAME(mux_usbphy480m_p)		= { "sclk_otgphy1_480m", "sclk_otgphy2_480m",
> +				    "sclk_otgphy0_480m" };
>  PNAME(mux_hsicphy480m_p)	= { "cpll", "gpll", "usbphy480m_src" };
>  PNAME(mux_hsicphy12m_p)		= { "hsicphy12m_xin12m", "hsicphy12m_usbphy" };
>  
> @@ -506,11 +506,11 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>  			RK3288_CLKSEL_CON(35), 6, 2, MFLAGS, 0, 5, DFLAGS,
>  			RK3288_CLKGATE_CON(4), 10, GFLAGS),
>  
> -	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "usb480m", CLK_IGNORE_UNUSED,
> +	GATE(SCLK_OTGPHY0, "sclk_otgphy0", "xin24m", CLK_IGNORE_UNUSED,
>  			RK3288_CLKGATE_CON(13), 4, GFLAGS),
> -	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "usb480m", CLK_IGNORE_UNUSED,
> +	GATE(SCLK_OTGPHY1, "sclk_otgphy1", "xin24m", CLK_IGNORE_UNUSED,
>  			RK3288_CLKGATE_CON(13), 5, GFLAGS),
> -	GATE(SCLK_OTGPHY2, "sclk_otgphy2", "usb480m", CLK_IGNORE_UNUSED,
> +	GATE(SCLK_OTGPHY2, "sclk_otgphy2", "xin24m", CLK_IGNORE_UNUSED,
>  			RK3288_CLKGATE_CON(13), 6, GFLAGS),
>  	GATE(SCLK_OTG_ADP, "sclk_otg_adp", "xin32k", CLK_IGNORE_UNUSED,
>  			RK3288_CLKGATE_CON(13), 7, GFLAGS),
> @@ -874,12 +874,6 @@ static void __init rk3288_clk_init(struct device_node *np)
>  		pr_warn("%s: could not register clock xin12m: %ld\n",
>  			__func__, PTR_ERR(clk));
>  
> -
> -	clk = clk_register_fixed_factor(NULL, "usb480m", "xin24m", 0, 20, 1);
> -	if (IS_ERR(clk))
> -		pr_warn("%s: could not register clock usb480m: %ld\n",
> -			__func__, PTR_ERR(clk));
> -
>  	clk = clk_register_fixed_factor(NULL, "hclk_vcodec_pre",
>  					"hclk_vcodec_pre_v", 0, 1, 4);
>  	if (IS_ERR(clk))
> -- 
> 2.6.2
> 

RE: [PATCH 6/8] ARM: dts: rockchip: add clock-cells for usb phy nodes

[Michael Turquette]

Heiko Stuebner wrote:
> Add the #clock-cells properties for the usbphy nodes as they
> provide the pll-clocks now.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Reviewed-by: Michael Turquette <mturquette@baylibre.com>

> ---
>  arch/arm/boot/dts/rk3066a.dtsi | 2 ++
>  arch/arm/boot/dts/rk3188.dtsi  | 2 ++
>  arch/arm/boot/dts/rk3288.dtsi  | 3 +++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
> index 946f187..3e4b41b 100644
> --- a/arch/arm/boot/dts/rk3066a.dtsi
> +++ b/arch/arm/boot/dts/rk3066a.dtsi
> @@ -181,6 +181,7 @@
>  			reg = <0x17c>;
>  			clocks = <&cru SCLK_OTGPHY0>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  
>  		usbphy1: usb-phy1 {
> @@ -188,6 +189,7 @@
>  			reg = <0x188>;
>  			clocks = <&cru SCLK_OTGPHY1>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  	};
>  
> diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
> index 6399942..48a287e 100644
> --- a/arch/arm/boot/dts/rk3188.dtsi
> +++ b/arch/arm/boot/dts/rk3188.dtsi
> @@ -156,6 +156,7 @@
>  			reg = <0x10c>;
>  			clocks = <&cru SCLK_OTGPHY0>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  
>  		usbphy1: usb-phy1 {
> @@ -163,6 +164,7 @@
>  			reg = <0x11c>;
>  			clocks = <&cru SCLK_OTGPHY1>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  	};
>  
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index c64a116..51a5d29 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -943,6 +943,7 @@
>  			reg = <0x320>;
>  			clocks = <&cru SCLK_OTGPHY0>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  
>  		usbphy1: usb-phy1 {
> @@ -950,6 +951,7 @@
>  			reg = <0x334>;
>  			clocks = <&cru SCLK_OTGPHY1>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  
>  		usbphy2: usb-phy2 {
> @@ -957,6 +959,7 @@
>  			reg = <0x348>;
>  			clocks = <&cru SCLK_OTGPHY2>;
>  			clock-names = "phyclk";
> +			#clock-cells = <0>;
>  		};
>  	};
>  
> -- 
> 2.6.2
> 

Re: [PATCH 5/8] clk: rockchip: fix usbphy-related clocks

[Heiko Stübner]

Hi Mike

Am Freitag, 4. Dezember 2015, 16:07:23 schrieb Michael Turquette:
> Heiko Stuebner wrote:
> > The otgphy clocks really only drive the phy blocks. These in turn
> > contain plls that then generate the 480m clocks the clock controller
> > uses to supply some other clocks like uart0, gpu or the video-codec.
> > 
> > So fix this structure to actually respect that hirarchy and removed
> > that usb480m fixed-rate clock working as a placeholder till now, as
> > this wouldn't even work if the supplying phy gets turned off while
> > its pll-output gets used elsewhere.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Acked-by: Michael Turquette <mturquette@baylibre.com>

Thanks for Ack + Review, but it seems you found a slightly old version in your 
inbox :-)

So right new we have,

- [PATCH 5/8] clk: rockchip: fix usbphy-related clocks
Acked-by: Michael Turquette <mturquette@baylibre.com>
- [PATCH 6/8] ARM: dts: rockchip: add clock-cells for usb phy nodes
Reviewed-by: Michael Turquette <mturquette@baylibre.com>


which should translate nicely to the two equivalent patches in v3:
[PATCH v3 6/8] ARM: dts: rockchip: add clock-cells for usb phy nodes
[PATCH v3 7/8] clk: rockchip: fix usbphy-related clocks


but I guess Kishon, was also looking for an Ack on the usbphy-code actually 
exposing these clocks in
[PATCH v3 5/8] phy: rockchip-usb: expose the phy-internal PLLs


If you could also take a look at that patch, I would be glad :-)


Thanks
Heiko