diff mbox

[v5,3/4] dt-bindings: phy: Add support for QMP phy

Message ID 1489050441-3240-4-git-send-email-vivek.gautam@codeaurora.org
State Not Applicable, archived
Headers show

Commit Message

Vivek Gautam March 9, 2017, 9:07 a.m. UTC
Qualcomm chipsets have QMP phy controller that provides
support to a number of controller, viz. PCIe, UFS, and USB.
Adding dt binding information for the same.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: Rob Herring <robh@kernel.org>
---

Hi Rob,
I have removed your Acked-by tag because of the change in bindings.
Please consider adding your Ack again if you are fine with these
updated bindings.

Changes since v4:
 - Added bindings for child nodes. Each phy lane is represented by child
   node with its own register space (for tx, rx and pcs blocks), and clocks
   and resets for power control facility.
 - Removed register space and lane offsets for tx, rx and pcs blocks from
   qmp phy node.
 - #phy-cells is now part of each child node and thus must be 0.
 - Added information on list of mandatory clocks and resets for each phy.

Changes since v3:
 - Added #clock-cells = <1>, indicating that phy is a clock provider.

Changes since v2:
 - Removed binding for "ref_clk_src" since we don't request this
   clock in the driver.
 - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
 - Using 'phy' for the node name.

Changes since v1:
 - New patch, forked out of the original driver patch:
   "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
 - Added 'Acked-by' from Rob.
 - Updated bindings to include mem resource as a list of
   offset - length pair for serdes block and for each lane.
 - Added a new binding for 'lane-offsets' that contains offsets
   to tx, rx and pcs blocks from each lane base address.

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 106 +++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt

Comments

Bjorn Andersson March 9, 2017, 11:07 a.m. UTC | #1
On Thu 09 Mar 10:07 CET 2017, Vivek Gautam wrote:

[..]
> +	phy@34000 {
> +		compatible = "qcom,msm8996-qmp-pcie-phy";
> +		reg = <0x034000 0x488>;

Drop the leading 0 from the address.

> +		#clock-cells = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
> +			<&gcc GCC_PCIE_CLKREF_CLK>;
> +		clock-names = "aux", "cfg_ahb", "ref";
> +
> +		vdda-phy-supply = <&pm8994_l28>;
> +		vdda-pll-supply = <&pm8994_l12>;
> +
> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
> +		reset-names = "phy", "common", "cfg";
> +
> +		pciephy_0: lane@0 {

The "@xyz" part should match the first value in "reg", i.e. 35000 here.

> +			reg = <0x035000 0x130>,
> +				<0x035200 0x200>,
> +				<0x035400 0x1dc>;
> +			#phy-cells = <0>;
> +
> +			clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
> +			clock-names = "pipe0";
> +			resets = <&gcc GCC_PCIE_0_PHY_BCR>;
> +			reset-names = "lane0";
> +		};
> +
> +		pciephy_1: lane@1 {
> +		...
> +		...
> +	};

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vivek Gautam March 10, 2017, 6:13 a.m. UTC | #2
On 03/09/2017 04:37 PM, Bjorn Andersson wrote:
> On Thu 09 Mar 10:07 CET 2017, Vivek Gautam wrote:
>
> [..]
>> +	phy@34000 {
>> +		compatible = "qcom,msm8996-qmp-pcie-phy";
>> +		reg = <0x034000 0x488>;
> Drop the leading 0 from the address.

Okay, will drop it.

>
>> +		#clock-cells = <1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +
>> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>> +			<&gcc GCC_PCIE_CLKREF_CLK>;
>> +		clock-names = "aux", "cfg_ahb", "ref";
>> +
>> +		vdda-phy-supply = <&pm8994_l28>;
>> +		vdda-pll-supply = <&pm8994_l12>;
>> +
>> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
>> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
>> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>> +		reset-names = "phy", "common", "cfg";
>> +
>> +		pciephy_0: lane@0 {
> The "@xyz" part should match the first value in "reg", i.e. 35000 here.

Right, i think this came from my older version of patches. Will correct it.

Regards
Vivek
>
>> +			reg = <0x035000 0x130>,
>> +				<0x035200 0x200>,
>> +				<0x035400 0x1dc>;
>> +			#phy-cells = <0>;
>> +
>> +			clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>> +			clock-names = "pipe0";
>> +			resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>> +			reset-names = "lane0";
>> +		};
>> +
>> +		pciephy_1: lane@1 {
>> +		...
>> +		...
>> +	};
> Regards,
> Bjorn
Rob Herring March 15, 2017, 8:34 p.m. UTC | #3
On Thu, Mar 09, 2017 at 02:37:20PM +0530, Vivek Gautam wrote:
> Qualcomm chipsets have QMP phy controller that provides
> support to a number of controller, viz. PCIe, UFS, and USB.
> Adding dt binding information for the same.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Cc: Rob Herring <robh@kernel.org>
> ---
> 
> Hi Rob,
> I have removed your Acked-by tag because of the change in bindings.
> Please consider adding your Ack again if you are fine with these
> updated bindings.
> 
> Changes since v4:
>  - Added bindings for child nodes. Each phy lane is represented by child
>    node with its own register space (for tx, rx and pcs blocks), and clocks
>    and resets for power control facility.
>  - Removed register space and lane offsets for tx, rx and pcs blocks from
>    qmp phy node.
>  - #phy-cells is now part of each child node and thus must be 0.
>  - Added information on list of mandatory clocks and resets for each phy.
> 
> Changes since v3:
>  - Added #clock-cells = <1>, indicating that phy is a clock provider.
> 
> Changes since v2:
>  - Removed binding for "ref_clk_src" since we don't request this
>    clock in the driver.

Humm, sounds suspicious. You should describe clocks connected to the h/w 
block, not what the driver uses (currently).

>  - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>  - Using 'phy' for the node name.
> 
> Changes since v1:
>  - New patch, forked out of the original driver patch:
>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>  - Added 'Acked-by' from Rob.
>  - Updated bindings to include mem resource as a list of
>    offset - length pair for serdes block and for each lane.
>  - Added a new binding for 'lane-offsets' that contains offsets
>    to tx, rx and pcs blocks from each lane base address.
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 106 +++++++++++++++++++++
>  1 file changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt

Acked-by: Rob Herring <robh@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vivek Gautam March 20, 2017, 8:26 a.m. UTC | #4
Hi Rob,


On Thu, Mar 16, 2017 at 2:04 AM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Mar 09, 2017 at 02:37:20PM +0530, Vivek Gautam wrote:
>> Qualcomm chipsets have QMP phy controller that provides
>> support to a number of controller, viz. PCIe, UFS, and USB.
>> Adding dt binding information for the same.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Cc: Rob Herring <robh@kernel.org>
>> ---
>>
>> Hi Rob,
>> I have removed your Acked-by tag because of the change in bindings.
>> Please consider adding your Ack again if you are fine with these
>> updated bindings.
>>
>> Changes since v4:
>>  - Added bindings for child nodes. Each phy lane is represented by child
>>    node with its own register space (for tx, rx and pcs blocks), and clocks
>>    and resets for power control facility.
>>  - Removed register space and lane offsets for tx, rx and pcs blocks from
>>    qmp phy node.
>>  - #phy-cells is now part of each child node and thus must be 0.
>>  - Added information on list of mandatory clocks and resets for each phy.
>>
>> Changes since v3:
>>  - Added #clock-cells = <1>, indicating that phy is a clock provider.
>>
>> Changes since v2:
>>  - Removed binding for "ref_clk_src" since we don't request this
>>    clock in the driver.
>
> Humm, sounds suspicious. You should describe clocks connected to the h/w
> block, not what the driver uses (currently).

The "ref_clk_src" clock is not really going to the phy block. After
cross-checking
the hardware documents, we decided to remove this.
On the SoC, this is an external clock (outside of the clock
controller) that comes
to a clock pad that feeds few clock tx blocks. These blocks supply the reference
clock to each IP blocks.
Output of these tx blocks is what we refer to as "ref" clock in our bindings.

>
>>  - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>>  - Using 'phy' for the node name.
>>
>> Changes since v1:
>>  - New patch, forked out of the original driver patch:
>>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>>  - Added 'Acked-by' from Rob.
>>  - Updated bindings to include mem resource as a list of
>>    offset - length pair for serdes block and for each lane.
>>  - Added a new binding for 'lane-offsets' that contains offsets
>>    to tx, rx and pcs blocks from each lane base address.
>>
>>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 106 +++++++++++++++++++++
>>  1 file changed, 106 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks for reviewing.

Best Regards
Vivek
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
new file mode 100644
index 000000000000..5595c3fabe0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -0,0 +1,106 @@ 
+Qualcomm QMP PHY controller
+===========================
+
+QMP phy controller supports physical layer functionality for a number of
+controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
+
+Required properties:
+ - compatible: compatible list, contains:
+	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
+	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
+
+ - reg: offset and length of register set for PHY's common serdes block.
+
+ - #clock-cells: must be 1
+    - 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
+
+ - clocks: a list of phandles and clock-specifier pairs,
+	   one for each entry in clock-names.
+ - clock-names: "cfg_ahb" for phy config clock,
+		"aux" for phy aux clock,
+		"ref" for 19.2 MHz ref clk,
+		For "qcom,msm8996-qmp-pcie-phy" must contain:
+			"aux", "cfg_ahb", "ref".
+		For "qcom,msm8996-qmp-usb3-phy" must contain:
+			"aux", "cfg_ahb", "ref".
+
+ - resets: a list of phandles and reset controller specifier pairs,
+	   one for each entry in reset-names.
+ - reset-names: "phy" for reset of phy block,
+		"common" for phy common block reset,
+		"cfg" for phy's ahb cfg block reset (Optional).
+		For "qcom,msm8996-qmp-pcie-phy" must contain:
+		 "phy", "common", "cfg".
+		For "qcom,msm8996-qmp-usb3-phy" must contain
+		 "phy", "common".
+
+ - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
+ - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
+
+Optional properties:
+ - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
+			pll block.
+
+Required nodes:
+ - Each device node of QMP phy is required to have as many child nodes as
+   the number of lanes the PHY has.
+
+Required properties for child node:
+ - reg: list of offset and length pairs of register sets for PHY blocks -
+	tx, rx and pcs.
+
+ - #phy-cells: must be 0
+
+ - clocks: a list of phandles and clock-specifier pairs,
+	   one for each entry in clock-names.
+ - clock-names: Must contain following for pcie and usb qmp phys:
+		 "pipe<lane-number>" for pipe clock specific to each lane.
+
+ - resets: a list of phandles and reset controller specifier pairs,
+	   one for each entry in reset-names.
+ - reset-names: Must contain following for pcie qmp phys:
+		 "lane<lane-number>" for reset specific to each lane.
+
+Example:
+	phy@34000 {
+		compatible = "qcom,msm8996-qmp-pcie-phy";
+		reg = <0x034000 0x488>;
+		#clock-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
+			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
+			<&gcc GCC_PCIE_CLKREF_CLK>;
+		clock-names = "aux", "cfg_ahb", "ref";
+
+		vdda-phy-supply = <&pm8994_l28>;
+		vdda-pll-supply = <&pm8994_l12>;
+
+		resets = <&gcc GCC_PCIE_PHY_BCR>,
+			<&gcc GCC_PCIE_PHY_COM_BCR>,
+			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
+		reset-names = "phy", "common", "cfg";
+
+		pciephy_0: lane@0 {
+			reg = <0x035000 0x130>,
+				<0x035200 0x200>,
+				<0x035400 0x1dc>;
+			#phy-cells = <0>;
+
+			clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
+			clock-names = "pipe0";
+			resets = <&gcc GCC_PCIE_0_PHY_BCR>;
+			reset-names = "lane0";
+		};
+
+		pciephy_1: lane@1 {
+		...
+		...
+	};