[PATCH v2 0/5] arm64: dts: qcom: sdm845: Add UFS DT nodes

[PATCH v2 0/5] arm64: dts: qcom: sdm845: Add UFS DT nodes

[Evan Green]

This series updates the device tree bindings for the QMP PHY to
properly specify the registers for dual-lane PHYs. It then updates
the driver to use those new registers. Then it adds the DT nodes for
UFS on SDM845, and activates them on the MTP platform. Finally, it
fixes up the USB3 PHY on SDM845, which is also a dual lane PHY.

The DT binding change is needed because it was discovered that the UFS PHY
driver was reaching beyond its register region, which only happened to work by
virtue of the page size granularity.

Changes since v1:
 - Added dt bindings change, corresponding driver fixup, and USB PHY fixup
 - Renamed ufsphy to phy (Vivek).
 - Removed #clock-cells (Vivek).

This applies atop linux-next 20181018 with the addition of Doug's
changes [1] and [2].

[1] https://lore.kernel.org/lkml/20181012213632.252346-1-dianders@chromium.org/
[2] https://lore.kernel.org/lkml/20181012213926.253765-1-dianders@chromium.org/

Can Guo (1):
  arm64: dts: qcom: sdm845: Add UFS nodes for sdm845-mtp

Evan Green (4):
  dt-bindings: phy-qcom-qmp: Fix register underspecification
  phy: qcom-qmp: Utilize fully-specified DT registers
  arm64: dts: qcom: sdm845: add UFS controller
  arm64: dts: qcom: sdm845: Add USB PHY lane two

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 73 ++++++++++++++++++---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts            | 14 ++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi               | 75 +++++++++++++++++++++-
 drivers/phy/qualcomm/phy-qcom-qmp.c                | 51 +++++++++++----
 4 files changed, 190 insertions(+), 23 deletions(-)

-- 
2.16.4

[PATCH v2 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification

[Evan Green]

This change adds register regions for the second lane of dual-lane nodes.
This additional specification is needed so that the driver can stop
reaching beyond the tx and rx register allocations to get at the
second lane registers in a dual-lane PHY.

While in there, document #clock-cells as optional for PHYs that don't
provide a pipe clock. Also, document the pcs_misc register region, which
was being quietly supplied and used.

Signed-off-by: Evan Green <evgreen@chromium.org>

---
This applies atop linux-next 20181018 with the addition of Doug's
changes [1] and [2].

[1] https://lore.kernel.org/lkml/20181012213632.252346-1-dianders@chromium.org/
[2] https://lore.kernel.org/lkml/20181012213926.253765-1-dianders@chromium.org/

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 73 +++++++++++++++++++---
 1 file changed, 65 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index fbc198d5dd39..297a7c753fc8 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -25,7 +25,7 @@ Required properties:
   - For all others:
     - The reg-names property shouldn't be defined.
 
- - #clock-cells: must be 1
+ - #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.
@@ -82,23 +82,26 @@ 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:
+Required properties for child nodes of PCIe PHYs (one child per lane):
  - reg: list of offset and length pairs of register sets for PHY blocks -
-	- index 0: tx
-	- index 1: rx
-	- index 2: pcs
-	- index 3: pcs_misc (optional)
+	tx, rx, pcs, and pcs_misc (optional).
+ - #phy-cells: must be 0
 
+Required properties for a single "lanes" child node of non-PCIe PHYs:
+ - reg: list of offset and length pairs of register sets for PHY blocks
+	For 1-lane devices:
+		tx, rx, pcs, and (optionally) pcs_misc
+	For 2-lane devices:
+		tx0, rx0, pcs, tx1, rx1, and (optionally) pcs_misc
  - #phy-cells: must be 0
 
-Required properties child node of pcie and usb3 qmp phys:
+Required properties for child node of PCIe and USB3 qmp phys:
  - clocks: a list of phandles and clock-specifier pairs,
 	   one for each entry in clock-names.
  - clock-names: Must contain following:
 		 "pipe<lane-number>" for pipe clock specific to each lane.
  - clock-output-names: Name of the PHY clock that will be the parent for
 		       the above pipe clock.
-
 	For "qcom,ipq8074-qmp-pcie-phy":
 		- "pcie20_phy0_pipe_clk"	Pipe Clock parent
 			(or)
@@ -150,3 +153,57 @@ Example:
 		...
 		...
 	};
+
+	phy@88eb000 {
+		compatible = "qcom,sdm845-qmp-usb3-uni-phy";
+		reg = <0x88eb000 0x18c>;
+		status = "disabled";
+		#clock-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK>,
+			 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+			 <&gcc GCC_USB3_SEC_CLKREF_CLK>,
+			 <&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>;
+		clock-names = "aux", "cfg_ahb", "ref", "com_aux";
+
+		resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR>,
+			 <&gcc GCC_USB3_PHY_SEC_BCR>;
+		reset-names = "phy", "common";
+
+		lane@88eb200 {
+			reg = <0x88eb200 0x128>,
+			      <0x88eb400 0x1fc>,
+			      <0x88eb800 0x218>,
+			      <0x88e9600 0x70>;
+			#phy-cells = <0>;
+			clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
+			clock-names = "pipe0";
+			clock-output-names = "usb3_uni_phy_pipe_clk_src";
+		};
+	};
+
+	phy@1d87000 {
+		compatible = "qcom,sdm845-qmp-ufs-phy";
+		reg = <0x1d87000 0x18c>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		clock-names = "ref",
+			      "ref_aux";
+		clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
+			 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+
+		status = "disabled";
+
+		lanes@1d87400 {
+			reg = <0x1d87400 0x108>,
+			      <0x1d87600 0x1e0>,
+			      <0x1d87c00 0x1dc>,
+			      <0x1d87800 0x108>,
+			      <0x1d87a00 0x1e0>;
+			#phy-cells = <0>;
+		};
+	};
-- 
2.16.4

Re: [PATCH v2 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification

[Doug Anderson]

Hi,

On Thu, Oct 18, 2018 at 2:09 PM Evan Green <evgreen@chromium.org> wrote:
>
> This change adds register regions for the second lane of dual-lane nodes.
> This additional specification is needed so that the driver can stop
> reaching beyond the tx and rx register allocations to get at the
> second lane registers in a dual-lane PHY.
>
> While in there, document #clock-cells as optional for PHYs that don't
> provide a pipe clock. Also, document the pcs_misc register region, which
> was being quietly supplied and used.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
>
> ---
> This applies atop linux-next 20181018 with the addition of Doug's
> changes [1] and [2].
>
> [1] https://lore.kernel.org/lkml/20181012213632.252346-1-dianders@chromium.org/
> [2] https://lore.kernel.org/lkml/20181012213926.253765-1-dianders@chromium.org/
>
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 73 +++++++++++++++++++---
>  1 file changed, 65 insertions(+), 8 deletions(-)

This all makes sense to me and seems like the right compromise to make

The only current SoC that uses tx2/rx2 is SDM845 and the support of
that SoC is in its infancy in mainline.  Thus I don't mind that we say
that all 5 registers are "required" even though there is an existing
device tree out there that don't include tx2/rx2 for USB.  Currently
patch #2 in this series still makes old device trees "work" (as well
as they used to) but I'm all for that being very temporary code and
that officially tx2/rx2 are not optional.


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

Re: [PATCH v2 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification

[Rob Herring]

On Thu, Oct 18, 2018 at 02:09:29PM -0700, Evan Green wrote:
> This change adds register regions for the second lane of dual-lane nodes.
> This additional specification is needed so that the driver can stop
> reaching beyond the tx and rx register allocations to get at the
> second lane registers in a dual-lane PHY.
> 
> While in there, document #clock-cells as optional for PHYs that don't
> provide a pipe clock. Also, document the pcs_misc register region, which
> was being quietly supplied and used.
> 
> Signed-off-by: Evan Green <evgreen@chromium.org>
> 
> ---
> This applies atop linux-next 20181018 with the addition of Doug's
> changes [1] and [2].
> 
> [1] https://lore.kernel.org/lkml/20181012213632.252346-1-dianders@chromium.org/
> [2] https://lore.kernel.org/lkml/20181012213926.253765-1-dianders@chromium.org/
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 73 +++++++++++++++++++---
>  1 file changed, 65 insertions(+), 8 deletions(-)

One nit below, otherwise:

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> index fbc198d5dd39..297a7c753fc8 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -25,7 +25,7 @@ Required properties:
>    - For all others:
>      - The reg-names property shouldn't be defined.
>  
> - - #clock-cells: must be 1
> + - #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.
> @@ -82,23 +82,26 @@ 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:
> +Required properties for child nodes of PCIe PHYs (one child per lane):
>   - reg: list of offset and length pairs of register sets for PHY blocks -
> -	- index 0: tx
> -	- index 1: rx
> -	- index 2: pcs
> -	- index 3: pcs_misc (optional)
> +	tx, rx, pcs, and pcs_misc (optional).
> + - #phy-cells: must be 0
>  
> +Required properties for a single "lanes" child node of non-PCIe PHYs:
> + - reg: list of offset and length pairs of register sets for PHY blocks
> +	For 1-lane devices:
> +		tx, rx, pcs, and (optionally) pcs_misc
> +	For 2-lane devices:
> +		tx0, rx0, pcs, tx1, rx1, and (optionally) pcs_misc
>   - #phy-cells: must be 0
>  
> -Required properties child node of pcie and usb3 qmp phys:
> +Required properties for child node of PCIe and USB3 qmp phys:
>   - clocks: a list of phandles and clock-specifier pairs,
>  	   one for each entry in clock-names.
>   - clock-names: Must contain following:
>  		 "pipe<lane-number>" for pipe clock specific to each lane.
>   - clock-output-names: Name of the PHY clock that will be the parent for
>  		       the above pipe clock.
> -
>  	For "qcom,ipq8074-qmp-pcie-phy":
>  		- "pcie20_phy0_pipe_clk"	Pipe Clock parent
>  			(or)
> @@ -150,3 +153,57 @@ Example:
>  		...
>  		...
>  	};
> +
> +	phy@88eb000 {
> +		compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> +		reg = <0x88eb000 0x18c>;
> +		status = "disabled";

Don't show status in examples.

> +		#clock-cells = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK>,
> +			 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +			 <&gcc GCC_USB3_SEC_CLKREF_CLK>,
> +			 <&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>;
> +		clock-names = "aux", "cfg_ahb", "ref", "com_aux";
> +
> +		resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR>,
> +			 <&gcc GCC_USB3_PHY_SEC_BCR>;
> +		reset-names = "phy", "common";
> +
> +		lane@88eb200 {
> +			reg = <0x88eb200 0x128>,
> +			      <0x88eb400 0x1fc>,
> +			      <0x88eb800 0x218>,
> +			      <0x88e9600 0x70>;
> +			#phy-cells = <0>;
> +			clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
> +			clock-names = "pipe0";
> +			clock-output-names = "usb3_uni_phy_pipe_clk_src";
> +		};
> +	};
> +
> +	phy@1d87000 {
> +		compatible = "qcom,sdm845-qmp-ufs-phy";
> +		reg = <0x1d87000 0x18c>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +		clock-names = "ref",
> +			      "ref_aux";
> +		clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
> +			 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
> +
> +		status = "disabled";
> +
> +		lanes@1d87400 {
> +			reg = <0x1d87400 0x108>,
> +			      <0x1d87600 0x1e0>,
> +			      <0x1d87c00 0x1dc>,
> +			      <0x1d87800 0x108>,
> +			      <0x1d87a00 0x1e0>;
> +			#phy-cells = <0>;
> +		};
> +	};
> -- 
> 2.16.4
> 

Re: [PATCH v2 1/5] dt-bindings: phy-qcom-qmp: Fix register underspecification

[Evan Green]

On Mon, Oct 22, 2018 at 5:29 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Oct 18, 2018 at 02:09:29PM -0700, Evan Green wrote:
> > This change adds register regions for the second lane of dual-lane nodes.
> > This additional specification is needed so that the driver can stop
> > reaching beyond the tx and rx register allocations to get at the
> > second lane registers in a dual-lane PHY.
> >
> > While in there, document #clock-cells as optional for PHYs that don't
> > provide a pipe clock. Also, document the pcs_misc register region, which
> > was being quietly supplied and used.
> >
> > Signed-off-by: Evan Green <evgreen@chromium.org>
> >
> > ---
> > This applies atop linux-next 20181018 with the addition of Doug's
> > changes [1] and [2].
> >
> > [1] https://lore.kernel.org/lkml/20181012213632.252346-1-dianders@chromium.org/
> > [2] https://lore.kernel.org/lkml/20181012213926.253765-1-dianders@chromium.org/
> >
> >  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 73 +++++++++++++++++++---
> >  1 file changed, 65 insertions(+), 8 deletions(-)
>
> One nit below, otherwise:
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> > index fbc198d5dd39..297a7c753fc8 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> > +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> > @@ -25,7 +25,7 @@ Required properties:
> > +
> > +     phy@88eb000 {
> > +             compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> > +             reg = <0x88eb000 0x18c>;
> > +             status = "disabled";
>
> Don't show status in examples.

I'll fix it. Thanks Rob.

[PATCH v2 2/5] phy: qcom-qmp: Utilize fully-specified DT registers

[Evan Green]

This change utilizes the newly fixed up DT bindings to get the tx2
and rx2 register regions for the second lane of dual-lane PHYs. Before
this change, the driver was simply using lane one's register region and
adding 0x400, which reached well beyond the DT-specified register
allocation. This would have been a crash were it not for the page size on
ARM64. Fix the driver not to rely on the magic of virtual memory by using
the newly specified DT register regions for tx2 and rx2.

In order to support existing device trees, this change also contains a
fallback mode for when those new register regions don't exist, which
reverts to the original behavior of overreaching and prints a complaint.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 51 +++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index a83332411026..957da2ef37c1 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -72,6 +72,9 @@
 
 #define MAX_PROP_NAME				32
 
+/* Define the assumed distance between lanes for underspecified device trees. */
+#define QMP_PHY_LEGACY_LANE_STRIDE		0x400
+
 struct qmp_phy_init_tbl {
 	unsigned int offset;
 	unsigned int val;
@@ -733,9 +736,6 @@ struct qmp_phy_cfg {
 	bool has_phy_dp_com_ctrl;
 	/* true, if PHY has secondary tx/rx lanes to be configured */
 	bool is_dual_lane_phy;
-	/* Register offset of secondary tx/rx lanes for USB DP combo PHY */
-	unsigned int tx_b_lane_offset;
-	unsigned int rx_b_lane_offset;
 
 	/* true, if PCS block has no separate SW_RESET register */
 	bool no_pcs_sw_reset;
@@ -748,6 +748,8 @@ struct qmp_phy_cfg {
  * @tx: iomapped memory space for lane's tx
  * @rx: iomapped memory space for lane's rx
  * @pcs: iomapped memory space for lane's pcs
+ * @tx2: iomapped memory space for second lane's tx (in dual lane PHYs)
+ * @rx2: iomapped memory space for second lane's rx (in dual lane PHYs)
  * @pcs_misc: iomapped memory space for lane's pcs_misc
  * @pipe_clk: pipe lock
  * @index: lane index
@@ -759,6 +761,8 @@ struct qmp_phy {
 	void __iomem *tx;
 	void __iomem *rx;
 	void __iomem *pcs;
+	void __iomem *tx2;
+	void __iomem *rx2;
 	void __iomem *pcs_misc;
 	struct clk *pipe_clk;
 	unsigned int index;
@@ -975,8 +979,6 @@ static const struct qmp_phy_cfg qmp_v3_usb3phy_cfg = {
 
 	.has_phy_dp_com_ctrl	= true,
 	.is_dual_lane_phy	= true,
-	.tx_b_lane_offset	= 0x400,
-	.rx_b_lane_offset	= 0x400,
 };
 
 static const struct qmp_phy_cfg qmp_v3_usb3_uniphy_cfg = {
@@ -1031,9 +1033,6 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
 	.mask_pcs_ready		= PCS_READY,
 
 	.is_dual_lane_phy	= true,
-	.tx_b_lane_offset	= 0x400,
-	.rx_b_lane_offset	= 0x400,
-
 	.no_pcs_sw_reset	= true,
 };
 
@@ -1238,12 +1237,12 @@ static int qcom_qmp_phy_init(struct phy *phy)
 	qcom_qmp_phy_configure(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num);
 	/* Configuration for other LANE for USB-DP combo PHY */
 	if (cfg->is_dual_lane_phy)
-		qcom_qmp_phy_configure(tx + cfg->tx_b_lane_offset, cfg->regs,
+		qcom_qmp_phy_configure(qphy->tx2, cfg->regs,
 				       cfg->tx_tbl, cfg->tx_tbl_num);
 
 	qcom_qmp_phy_configure(rx, cfg->regs, cfg->rx_tbl, cfg->rx_tbl_num);
 	if (cfg->is_dual_lane_phy)
-		qcom_qmp_phy_configure(rx + cfg->rx_b_lane_offset, cfg->regs,
+		qcom_qmp_phy_configure(qphy->rx2, cfg->regs,
 				       cfg->rx_tbl, cfg->rx_tbl_num);
 
 	qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
@@ -1614,8 +1613,9 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id)
 
 	/*
 	 * Get memory resources for each phy lane:
-	 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2; and
-	 * pcs_misc (optional) -> 3.
+	 * Resources are indexed as: tx -> 0; rx -> 1; pcs -> 2.
+	 * For dual lane PHYs: tx2 -> 3, rx2 -> 4, pcs_misc (optional) -> 5
+	 * For single lane PHYs: pcs_misc (optional) -> 3.
 	 */
 	qphy->tx = of_iomap(np, 0);
 	if (!qphy->tx)
@@ -1629,7 +1629,32 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id)
 	if (!qphy->pcs)
 		return -ENOMEM;
 
-	qphy->pcs_misc = of_iomap(np, 3);
+	/*
+	 * If this is a dual-lane PHY, then there should be registers for the
+	 * second lane. Some old device trees did not specify this, so fall
+	 * back to old legacy behavior of assuming they can be reached at an
+	 * offset from the first lane.
+	 */
+	if (qmp->cfg->is_dual_lane_phy) {
+		qphy->tx2 = of_iomap(np, 3);
+		qphy->rx2 = of_iomap(np, 4);
+		if (!qphy->tx2 || !qphy->rx2) {
+			dev_warn(dev,
+				 "Underspecified device tree, falling back to legacy register regions\n");
+
+			/* In the old version, pcs_misc is at index 3. */
+			qphy->pcs_misc = qphy->tx2;
+			qphy->tx2 = qphy->tx + QMP_PHY_LEGACY_LANE_STRIDE;
+			qphy->rx2 = qphy->rx + QMP_PHY_LEGACY_LANE_STRIDE;
+
+		} else {
+			qphy->pcs_misc = of_iomap(np, 5);
+		}
+
+	} else {
+		qphy->pcs_misc = of_iomap(np, 3);
+	}
+
 	if (!qphy->pcs_misc)
 		dev_vdbg(dev, "PHY pcs_misc-reg not used\n");
 
-- 
2.16.4

Re: [PATCH v2 2/5] phy: qcom-qmp: Utilize fully-specified DT registers

[Doug Anderson]

Hi,

On Thu, Oct 18, 2018 at 2:09 PM Evan Green <evgreen@chromium.org> wrote:
>
> This change utilizes the newly fixed up DT bindings to get the tx2
> and rx2 register regions for the second lane of dual-lane PHYs. Before
> this change, the driver was simply using lane one's register region and
> adding 0x400, which reached well beyond the DT-specified register
> allocation. This would have been a crash were it not for the page size on
> ARM64. Fix the driver not to rely on the magic of virtual memory by using
> the newly specified DT register regions for tx2 and rx2.
>
> In order to support existing device trees, this change also contains a
> fallback mode for when those new register regions don't exist, which
> reverts to the original behavior of overreaching and prints a complaint.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 51 +++++++++++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 13 deletions(-)

Looks good to me.  This seems like the right compromise for moving
forward to fix the problem.

A few notes:

* The only current SoC that uses tx2/rx2 is SDM845 and the support of
that SoC is in its infancy in mainline.  Thus I don't mind that using
existing device trees with this new patch will print a warning and
continue to take advantage of the Linux "bug" that the whole page
could be accessed.  We should fix the old DTS files ASAP and then
remove the nod to backward compatibility in the code.

* This should land _before_ dts patches land.  If dts changes land
without this change then the old code will end up mapping tx2 and
using it as pcsmisc.  ...so either we'll need to delay landing the
device tree patch by one major version or Andy / Kishon will need to
coordinate.


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

[PATCH v2 3/5] arm64: dts: qcom: sdm845: add UFS controller

[Evan Green]

This change adds the UFS controller and PHY to SDM845.

Signed-off-by: Evan Green <evgreen@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 67 ++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index b72bdb0a31a5..9c72edb678ec 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -808,6 +808,73 @@
 			};
 		};
 
+		ufshc1: ufshc@1d84000 {
+			compatible = "qcom,sdm845-ufshc", "qcom,ufshc",
+				     "jedec,ufs-2.0";
+			reg = <0x1d84000 0x2500>;
+			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&ufsphy1_lanes>;
+			phy-names = "ufsphy";
+			lanes-per-direction = <2>;
+			power-domains = <&gcc UFS_PHY_GDSC>;
+
+			clock-names =
+				"core_clk",
+				"bus_aggr_clk",
+				"iface_clk",
+				"core_clk_unipro",
+				"ref_clk",
+				"tx_lane0_sync_clk",
+				"rx_lane0_sync_clk",
+				"rx_lane1_sync_clk";
+			clocks =
+				<&gcc GCC_UFS_PHY_AXI_CLK>,
+				<&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+				<&gcc GCC_UFS_PHY_AHB_CLK>,
+				<&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+				<&rpmhcc RPMH_CXO_CLK>,
+				<&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+				<&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
+				<&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
+			freq-table-hz =
+				<50000000 200000000>,
+				<0 0>,
+				<0 0>,
+				<37500000 150000000>,
+				<0 0>,
+				<0 0>,
+				<0 0>,
+				<0 0>;
+
+			resets = <&gcc GCC_UFS_PHY_BCR>;
+			reset-names = "rst";
+
+			status = "disabled";
+		};
+
+		ufsphy1: phy@1d87000 {
+			compatible = "qcom,sdm845-qmp-ufs-phy";
+			reg = <0x1d87000 0x18c>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			clock-names = "ref",
+				      "ref_aux";
+			clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
+				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+
+			status = "disabled";
+
+			ufsphy1_lanes: lanes@1d87400 {
+				reg = <0x1d87400 0x108>,
+				      <0x1d87600 0x1e0>,
+				      <0x1d87c00 0x1dc>,
+				      <0x1d87800 0x108>,
+				      <0x1d87a00 0x1e0>;
+				#phy-cells = <0>;
+			};
+		};
+
 		tcsr_mutex_regs: syscon@1f40000 {
 			compatible = "syscon";
 			reg = <0x1f40000 0x40000>;
-- 
2.16.4

Re: [PATCH v2 3/5] arm64: dts: qcom: sdm845: add UFS controller

[Doug Anderson]

Hi,
On Thu, Oct 18, 2018 at 2:10 PM Evan Green <evgreen@chromium.org> wrote:
>
> This change adds the UFS controller and PHY to SDM845.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 67 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)

I'm already in the Signed-off-by since I did some touching of this
patch, so not sure it makes sense to provide my Reviewed-by on this
patch.  ...but I'll at least make the note that it seems good and
matches my understanding


As mentioned in patch #2 ideally this patch should land after the
driver fix so the old driver doesn't try to use the "tx2" range as
"pcs_misc".

-Doug

[PATCH v2 4/5] arm64: dts: qcom: sdm845: Add UFS nodes for sdm845-mtp

[Evan Green]

From: Can Guo <cang@codeaurora.org>

This change enables the UFS host controller and PHY on sdm845-mtp.

Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index eedfaf8922e2..d5fddea71a85 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -356,6 +356,20 @@
 	status = "okay";
 };
 
+&ufshc1 {
+	status = "okay";
+
+	vcc-supply = <&vreg_l20a_2p95>;
+	vcc-max-microamp = <600000>;
+};
+
+&ufsphy1 {
+	status = "okay";
+
+	vdda-phy-supply = <&vdda_ufs1_core>;
+	vdda-pll-supply = <&vdda_ufs1_1p2>;
+};
+
 &usb_1 {
 	status = "okay";
 };
-- 
2.16.4

Re: [PATCH v2 4/5] arm64: dts: qcom: sdm845: Add UFS nodes for sdm845-mtp

[Doug Anderson]

Hi,

On Thu, Oct 18, 2018 at 2:10 PM Evan Green <evgreen@chromium.org> wrote:
>
> From: Can Guo <cang@codeaurora.org>
>
> This change enables the UFS host controller and PHY on sdm845-mtp.
>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Evan Green <evgreen@chromium.org>
> Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

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

[PATCH v2 5/5] arm64: dts: qcom: sdm845: Add USB PHY lane two

[Evan Green]

This change adds the second lane registers for the USB PHY, now that the
QMP phy bindings have been updated. This way the driver can stop
reaching beyond its register region to get at the second lane.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 9c72edb678ec..f28c50e93f5a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1188,10 +1188,12 @@
 				 <&gcc GCC_USB3_PHY_PRIM_BCR>;
 			reset-names = "phy", "common";
 
-			usb_1_ssphy: lane@88e9200 {
+			usb_1_ssphy: lanes@88e9200 {
 				reg = <0x88e9200 0x128>,
 				      <0x88e9400 0x200>,
 				      <0x88e9c00 0x218>,
+				      <0x88e9600 0x128>,
+				      <0x88e9800 0x200>,
 				      <0x88e9a00 0x100>;
 				#phy-cells = <0>;
 				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
@@ -1219,10 +1221,12 @@
 				 <&gcc GCC_USB3_PHY_SEC_BCR>;
 			reset-names = "phy", "common";
 
-			usb_2_ssphy: lane@88eb200 {
+			usb_2_ssphy: lanes@88eb200 {
 				reg = <0x88eb200 0x128>,
 				      <0x88eb400 0x1fc>,
 				      <0x88eb800 0x218>,
+				      <0x88eb600 0x128>,
+				      <0x88eb800 0x1fc>,
 				      <0x88e9600 0x70>;
 				#phy-cells = <0>;
 				clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
-- 
2.16.4

Re: [PATCH v2 5/5] arm64: dts: qcom: sdm845: Add USB PHY lane two

[Doug Anderson]

Hi,

On Thu, Oct 18, 2018 at 2:10 PM Evan Green <evgreen@chromium.org> wrote:
>
> This change adds the second lane registers for the USB PHY, now that the
> QMP phy bindings have been updated. This way the driver can stop
> reaching beyond its register region to get at the second lane.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 9c72edb678ec..f28c50e93f5a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1188,10 +1188,12 @@
>                                  <&gcc GCC_USB3_PHY_PRIM_BCR>;
>                         reset-names = "phy", "common";
>
> -                       usb_1_ssphy: lane@88e9200 {
> +                       usb_1_ssphy: lanes@88e9200 {
>                                 reg = <0x88e9200 0x128>,
>                                       <0x88e9400 0x200>,
>                                       <0x88e9c00 0x218>,
> +                                     <0x88e9600 0x128>,
> +                                     <0x88e9800 0x200>,
>                                       <0x88e9a00 0x100>;
>                                 #phy-cells = <0>;
>                                 clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> @@ -1219,10 +1221,12 @@
>                                  <&gcc GCC_USB3_PHY_SEC_BCR>;
>                         reset-names = "phy", "common";
>
> -                       usb_2_ssphy: lane@88eb200 {
> +                       usb_2_ssphy: lanes@88eb200 {
>                                 reg = <0x88eb200 0x128>,
>                                       <0x88eb400 0x1fc>,
>                                       <0x88eb800 0x218>,
> +                                     <0x88eb600 0x128>,
> +                                     <0x88eb800 0x1fc>,

This is wrong and needs to be fixed.  The
"qcom,sdm845-qmp-usb3-uni-phy" is not dual-lane and thus shouldn't
have tx2/rx2.  The driver knows "qcom,sdm845-qmp-usb3-uni-phy" is not
dual lane and thus will try to map tx2 as pcs_misc.


Also overall I'll continue to point out that this patch needs to land
_after_ the driver fix since the new driver can cope with either style
device tree (at least temporarily) but the old driver can't cope with
the new device tree.  See patch #2 comments for details.

-Doug