[PATCH v2 0/4] phy: qcom-qmp: Fix clock-cells binding and provider

[PATCH v2 0/4] phy: qcom-qmp: Fix clock-cells binding and provider

[Evan Green]

This series fixes the QMP PHY bindings, which had specified #clock-cells
in the parent node, and had set it to 1. Putting it in the parent node is
wrong because the clock providers are the child nodes, so this change
moves it there. Having it set to 1 is also wrong, since nothing is ever
specified as to what should go in that cell. So this changes it to zero.
Finally, this change completes a little bit of code to actually allow these
exposed clocks to be pointed at in DT.

I had no idea how to fix up ipq8074.dtsi. It seems to be completely wrong in
that it doesn't specify #clock-cells at all, has no child nodes, and
specifies clock-output-names in the parent node. As far as I can tell this
doesn't work at all. But I can't add the child nodes myself because I don't know
1) how many there are, and 2) the registers in them. I also have no way to test it.

Thanks to Vivek for testing msm8996.

This patch sits atop the UFS device nodes series [1].

[1] https://lore.kernel.org/lkml/20181210192826.241350-1-evgreen@chromium.org/


Changes in v2:
 - Rebased onto phy/next

Evan Green (4):
  dt-bindings: phy-qcom-qmp: Move #clock-cells to child
  arm64: dts: qcom: msm8996: Fix QMP PHY #clock-cells
  arm64: dts: qcom: sdm845: Fix QMP PHY #clock-cells
  phy: qcom-qmp: Expose provided clocks to DT

 .../devicetree/bindings/phy/qcom-qmp-phy.txt  | 11 ++++-----
 arch/arm64/boot/dts/qcom/msm8996.dtsi         |  6 +++--
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  4 ++--
 drivers/phy/qualcomm/phy-qcom-qmp.c           | 23 ++++++++++++++++++-
 4 files changed, 33 insertions(+), 11 deletions(-)

-- 
2.18.1

[PATCH v2 1/4] dt-bindings: phy-qcom-qmp: Move #clock-cells to child

[Evan Green]

The phy-qcom-qmp bindings specified #clock-cells as 1. This was never used
because of_clk_add_provider() was never called, so there was no way anybody
could reference these clocks from DT. Furthermore, even if they could be
accessed, the bindings never specified what should go in that additional
cell.

Fix these incomplete and broken bindings. Move the #clock-cells into the
child node, since that is the actual clock provider, and not all
instances of qcom-qmp-phy are clock providers. Also set #clock-cells to
zero, since there's nothing to pass to it.

Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---

Changes in v2: None

 .../devicetree/bindings/phy/qcom-qmp-phy.txt          | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index c7a71e67ebdf5..68c67eeef7758 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -25,10 +25,6 @@ Required properties:
   - For all others:
     - The reg-names property shouldn't be defined.
 
- - #clock-cells: must be 1 (PCIe and USB3 PHYs only)
-    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
-      interface (for pipe based PHYs). These clock are then gate-controlled
-      by gcc.
  - #address-cells: must be 1
  - #size-cells: must be 1
  - ranges: must be present
@@ -94,6 +90,9 @@ Required properties for child node of PCIe and USB3 qmp phys:
 		- "pcie20_phy0_pipe_clk"	Pipe Clock parent
 			(or)
 		  "pcie20_phy1_pipe_clk"
+ - #clock-cells: must be 0
+    - Phy pll outputs pipe clocks for pipe based PHYs. These clocks are then
+      gate-controlled by the gcc.
 
  - resets: a list of phandles and reset controller specifier pairs,
 	   one for each entry in reset-names.
@@ -104,7 +103,6 @@ Example:
 	phy@34000 {
 		compatible = "qcom,msm8996-qmp-pcie-phy";
 		reg = <0x34000 0x488>;
-		#clock-cells = <1>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
@@ -126,6 +124,7 @@ Example:
 			reg = <0x35000 0x130>,
 				<0x35200 0x200>,
 				<0x35400 0x1dc>;
+			#clock-cells = <0>;
 			#phy-cells = <0>;
 
 			clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
@@ -143,7 +142,6 @@ Example:
 	phy@88eb000 {
 		compatible = "qcom,sdm845-qmp-usb3-uni-phy";
 		reg = <0x88eb000 0x18c>;
-		#clock-cells = <1>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
@@ -163,6 +161,7 @@ Example:
 			      <0x88eb400 0x1fc>,
 			      <0x88eb800 0x218>,
 			      <0x88eb600 0x70>;
+			#clock-cells = <0>;
 			#phy-cells = <0>;
 			clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
 			clock-names = "pipe0";
-- 
2.18.1

[PATCH v2 2/4] arm64: dts: qcom: msm8996: Fix QMP PHY #clock-cells

[Evan Green]

Move #clock-cells into the child node and set it to 0 to conform to the
proper binding specification.

Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---

Changes in v2: None

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index b29fe80d72883..44a494c70fa11 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -760,7 +760,6 @@
 		phy@34000 {
 			compatible = "qcom,msm8996-qmp-pcie-phy";
 			reg = <0x34000 0x488>;
-			#clock-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -783,6 +782,7 @@
 				reg = <0x035000 0x130>,
 					<0x035200 0x200>,
 					<0x035400 0x1dc>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 
 				clock-output-names = "pcie_0_pipe_clk_src";
@@ -796,6 +796,7 @@
 				reg = <0x036000 0x130>,
 					<0x036200 0x200>,
 					<0x036400 0x1dc>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 
 				clock-output-names = "pcie_1_pipe_clk_src";
@@ -809,6 +810,7 @@
 				reg = <0x037000 0x130>,
 					<0x037200 0x200>,
 					<0x037400 0x1dc>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 
 				clock-output-names = "pcie_2_pipe_clk_src";
@@ -822,7 +824,6 @@
 		phy@7410000 {
 			compatible = "qcom,msm8996-qmp-usb3-phy";
 			reg = <0x7410000 0x1c4>;
-			#clock-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -844,6 +845,7 @@
 				reg = <0x7410200 0x200>,
 					<0x7410400 0x130>,
 					<0x7410600 0x1a8>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 
 				clock-output-names = "usb3_phy_pipe_clk_src";
-- 
2.18.1

[PATCH v2 3/4] arm64: dts: qcom: sdm845: Fix QMP PHY #clock-cells

[Evan Green]

Move #clock-cells into the child node for instances of the qcom-qmp-phy
nodes, and set it to zero, in accordance with the proper bindings. PHYs
that don't provide clocks don't have #clock-cells, and so are left alone.

Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---

Changes in v2: None

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 47bb7edc03afe..fedd6578776a2 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1170,7 +1170,6 @@
 			      <0x88e8000 0x10>;
 			reg-names = "reg-base", "dp_com";
 			status = "disabled";
-			#clock-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -1192,6 +1191,7 @@
 				      <0x88e9600 0x128>,
 				      <0x88e9800 0x200>,
 				      <0x88e9a00 0x100>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
 				clock-names = "pipe0";
@@ -1203,7 +1203,6 @@
 			compatible = "qcom,sdm845-qmp-usb3-uni-phy";
 			reg = <0x88eb000 0x18c>;
 			status = "disabled";
-			#clock-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -1223,6 +1222,7 @@
 				      <0x88eb400 0x1fc>,
 				      <0x88eb800 0x218>,
 				      <0x88e9600 0x70>;
+				#clock-cells = <0>;
 				#phy-cells = <0>;
 				clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
 				clock-names = "pipe0";
-- 
2.18.1

[PATCH v2 4/4] phy: qcom-qmp: Expose provided clocks to DT

[Evan Green]

Register a simple clock provider for the PHY pipe clock sources so that
device tree users can point at these clocks via phandles to the lane
nodes.

Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Vivek Gautam <vivek.gautam@codeaurora.org>

---

Changes in v2:
 - Rebased onto phy/next

 drivers/phy/qualcomm/phy-qcom-qmp.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 8204d55e2d650..b4006818e1b65 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -1542,6 +1542,11 @@ static int qcom_qmp_phy_clk_init(struct device *dev)
 	return devm_clk_bulk_get(dev, num, qmp->clks);
 }
 
+static void phy_pipe_clk_release_provider(void *res)
+{
+	of_clk_del_provider(res);
+}
+
 /*
  * Register a fixed rate pipe clock.
  *
@@ -1588,7 +1593,23 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, struct device_node *np)
 	fixed->fixed_rate = 125000000;
 	fixed->hw.init = &init;
 
-	return devm_clk_hw_register(qmp->dev, &fixed->hw);
+	ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
+	if (ret)
+		return ret;
+
+	ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
+	if (ret)
+		return ret;
+
+	/*
+	 * Roll a devm action because the clock provider is the child node, but
+	 * the child node is not actually a device.
+	 */
+	ret = devm_add_action(qmp->dev, phy_pipe_clk_release_provider, np);
+	if (ret)
+		phy_pipe_clk_release_provider(np);
+
+	return ret;
 }
 
 static const struct phy_ops qcom_qmp_phy_gen_ops = {
-- 
2.18.1