[RFC PATCH v4 0/7] Reconstruct rockchip's PCIe and PCIe-PHY driver for per-lane PHY model

[RFC PATCH v4 0/7] Reconstruct rockchip's PCIe and PCIe-PHY driver for per-lane PHY model

[Shawn Lin]

Hi Kishon,

Could you kindly have a look at this patchset and give a 'yes' or 'no'
to this solution? Your opinion is the most important ont for it, otherwise
we should invent other bits to implement it.

This patchset is trying to reconstruct PCIe and PCIe-PHY driver
for rockchip platform in order to support per-lane PHY mode. And
we could idle the inactive lane(s) finally.

We deprecate the legacy PHY mode but the code could still
support it in order not to break backware compatibility of DTB. And I
organize the patches carefully so that we don't introduce git-bisect
issue.

Patchset was reviewed by Brian and fully tested by Jeffy. Rob acked
patch 6 & 7. Patch 5 for DTS changes for Rockchip part still need
ack from Heiko, and it would be fine for Heiko to pick it up after
pcie and pcie-phy changes got merged.


Changes in v4:
- use err variable to check return value of
  rockchip_pcie_get_phys
- add Brian's tag
- move rockchip_pcie_get_phys to where it stands in
  patch 1.
- print PHY index in err output log
- return u8 for rockchip_pcie_lane_map

Changes in v3:
- kill rockchip_pcie_manipulate_phys and related stuff
- use phys array
- improve the commit msg
- remove unnecessary forward declaration
- keep mutex inside struct rockchip_pcie_phy
- fix wrong check of args number
- move de-idle lanes after deasserting the reset
- use cached lanes_map to avoid powering off inactive
  lanes twice
- rename the commit tile

Changes in v2:
- deprecate legacy PHY model
- improve rockchip_pcie_phy_of_xlate
- fix wrong calculation of pwr_cnt and add new init_cnt
- add internal locking
- introduce per-lane data to simply the code

Shawn Lin (7):
  PCI: rockchip: split out rockchip_pcie_get_phys
  PCI: rockchip: introduce per-lanes PHYs support
  phy: rockcip-pcie: reconstruct driver to support per-lane PHYs
  PCI: rockchip: idle the inactive PHY(s)
  arm64: dts: rockchip: convert PCIe to use per-lane PHYs for rk3339
  dt-bindings: PCI: rockchip: convert to use per-lane PHY model
  dt-bindings: phy: convert to use per-lane Rockchip PCIe PHY

 .../devicetree/bindings/pci/rockchip-pcie.txt      |  25 ++++-
 .../devicetree/bindings/phy/rockchip-pcie-phy.txt  |   7 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi           |   8 +-
 drivers/pci/host/pcie-rockchip.c                   | 122 ++++++++++++++++----
 drivers/phy/rockchip/phy-rockchip-pcie.c           | 124 ++++++++++++++++++---
 5 files changed, 245 insertions(+), 41 deletions(-)

-- 
1.9.1

[RFC PATCH v4 1/7] PCI: rockchip: split out rockchip_pcie_get_phys

[Shawn Lin]

We plan to introduce per-lanes PHY, so split out new
function to make it easy in the future. No functional
change intended.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4:
- use err variable to check return value of
  rockchip_pcie_get_phys
- add Brian's tag

Changes in v3: None
Changes in v2: None

 drivers/pci/host/pcie-rockchip.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 5acf869..c42feab 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -853,6 +853,19 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip)
+{
+	struct device *dev = rockchip->dev;
+
+	rockchip->phy = devm_phy_get(dev, "pcie-phy");
+	if (IS_ERR(rockchip->phy)) {
+		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
+			dev_err(dev, "missing phy\n");
+		return PTR_ERR(rockchip->phy);
+	}
+
+	return 0;
+}
 
 /**
  * rockchip_pcie_parse_dt - Parse Device Tree
@@ -883,12 +896,9 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 	if (IS_ERR(rockchip->apb_base))
 		return PTR_ERR(rockchip->apb_base);
 
-	rockchip->phy = devm_phy_get(dev, "pcie-phy");
-	if (IS_ERR(rockchip->phy)) {
-		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
-			dev_err(dev, "missing phy\n");
-		return PTR_ERR(rockchip->phy);
-	}
+	err = rockchip_pcie_get_phys(rockchip);
+	if (err)
+		return err;
 
 	rockchip->lanes = 1;
 	err = of_property_read_u32(node, "num-lanes", &rockchip->lanes);
-- 
1.9.1

Re: [RFC PATCH v4 1/7] PCI: rockchip: split out rockchip_pcie_get_phys

[Kishon Vijay Abraham I]

On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
> We plan to introduce per-lanes PHY, so split out new
> function to make it easy in the future. No functional
> change intended.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> 
> Changes in v4:
> - use err variable to check return value of
>   rockchip_pcie_get_phys
> - add Brian's tag
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pci/host/pcie-rockchip.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 5acf869..c42feab 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -853,6 +853,19 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> +static int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip)
> +{
> +	struct device *dev = rockchip->dev;
> +
> +	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> +	if (IS_ERR(rockchip->phy)) {
> +		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
> +			dev_err(dev, "missing phy\n");
> +		return PTR_ERR(rockchip->phy);
> +	}
> +
> +	return 0;
> +}
>  
>  /**
>   * rockchip_pcie_parse_dt - Parse Device Tree
> @@ -883,12 +896,9 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  	if (IS_ERR(rockchip->apb_base))
>  		return PTR_ERR(rockchip->apb_base);
>  
> -	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> -	if (IS_ERR(rockchip->phy)) {
> -		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
> -			dev_err(dev, "missing phy\n");
> -		return PTR_ERR(rockchip->phy);
> -	}
> +	err = rockchip_pcie_get_phys(rockchip);
> +	if (err)
> +		return err;
>  
>  	rockchip->lanes = 1;
>  	err = of_property_read_u32(node, "num-lanes", &rockchip->lanes);
> 

[RFC PATCH v4 2/7] PCI: rockchip: introduce per-lanes PHYs support

[Shawn Lin]

We distinguish the legacy PHY with the newer per-lane
PHYs by adding legacy_phy flag. Note that the legacy phy
is still the first option to be searched in order not to
break the backward compatibility of DTB.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4:
- move rockchip_pcie_get_phys to where it stands in
  patch 1.
- print PHY index in err output log

Changes in v3:
- kill rockchip_pcie_manipulate_phys and related stuff
- use phys array
- improve the commit msg

Changes in v2: None

 drivers/pci/host/pcie-rockchip.c | 77 +++++++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index c42feab..f510349 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -47,6 +47,7 @@
 #define HIWORD_UPDATE_BIT(val)		HIWORD_UPDATE(val, val)
 
 #define ENCODE_LANES(x)			((((x) >> 1) & 3) << 4)
+#define MAX_LANE_NUM			4
 
 #define PCIE_CLIENT_BASE		0x0
 #define PCIE_CLIENT_CONFIG		(PCIE_CLIENT_BASE + 0x00)
@@ -210,7 +211,8 @@
 struct rockchip_pcie {
 	void	__iomem *reg_base;		/* DT axi-base */
 	void	__iomem *apb_base;		/* DT apb-base */
-	struct	phy *phy;
+	bool    legacy_phy;
+	struct  phy *phys[MAX_LANE_NUM];
 	struct	reset_control *core_rst;
 	struct	reset_control *mgmt_rst;
 	struct	reset_control *mgmt_sticky_rst;
@@ -514,7 +516,7 @@ static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip)
 static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 {
 	struct device *dev = rockchip->dev;
-	int err;
+	int err, i;
 	u32 status;
 
 	gpiod_set_value(rockchip->ep_gpio, 0);
@@ -537,10 +539,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
-	err = phy_init(rockchip->phy);
-	if (err < 0) {
-		dev_err(dev, "fail to init phy, err %d\n", err);
-		return err;
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		err = phy_init(rockchip->phys[i]);
+		if (err) {
+			dev_err(dev, "init phy%d err %d\n", i, err);
+			return err;
+		}
 	}
 
 	err = reset_control_assert(rockchip->core_rst);
@@ -602,10 +606,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			    PCIE_CLIENT_MODE_RC,
 			    PCIE_CLIENT_CONFIG);
 
-	err = phy_power_on(rockchip->phy);
-	if (err) {
-		dev_err(dev, "fail to power on phy, err %d\n", err);
-		return err;
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		err = phy_power_on(rockchip->phys[i]);
+		if (err) {
+			dev_err(dev, "power on phy%d err %d\n", i, err);
+			return err;
+		}
 	}
 
 	/*
@@ -856,12 +862,38 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
 static int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip)
 {
 	struct device *dev = rockchip->dev;
+	struct phy *phy;
+	char *name;
+	u32 i;
+
+	rockchip->phys[0] = devm_phy_get(dev, "pcie-phy");
+	if (IS_ERR(rockchip->phys[0])) {
+		if (PTR_ERR(rockchip->phys[0]) == -EPROBE_DEFER)
+			return PTR_ERR(rockchip->phys[0]);
+		dev_dbg(dev, "missing legacy phy, and search for per-lane PHY\n");
+	} else {
+		rockchip->legacy_phy = true;
+		dev_warn(dev, "legacy phy model is deprecated!\n");
+		return 0;
+	}
+
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		name = kasprintf(GFP_KERNEL, "pcie-phy-%u", i);
+		if (!name)
+			return -ENOMEM;
+
+		phy = devm_of_phy_get(rockchip->dev,
+				      rockchip->dev->of_node, name);
+		kfree(name);
 
-	rockchip->phy = devm_phy_get(dev, "pcie-phy");
-	if (IS_ERR(rockchip->phy)) {
-		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
-			dev_err(dev, "missing phy\n");
-		return PTR_ERR(rockchip->phy);
+		if (IS_ERR(phy)) {
+			if (PTR_ERR(phy) != -EPROBE_DEFER)
+				dev_err(dev, "missing phy for lane %d: %ld\n",
+					i, PTR_ERR(phy));
+			return PTR_ERR(phy);
+		}
+
+		rockchip->phys[i] = phy;
 	}
 
 	return 0;
@@ -1283,7 +1315,7 @@ static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
 static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 {
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
-	int ret;
+	int ret, i;
 
 	/* disable core and cli int since we don't need to ack PME_ACK */
 	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
@@ -1296,8 +1328,10 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 		return ret;
 	}
 
-	phy_power_off(rockchip->phy);
-	phy_exit(rockchip->phy);
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		phy_power_off(rockchip->phys[i]);
+		phy_exit(rockchip->phys[i]);
+	}
 
 	clk_disable_unprepare(rockchip->clk_pcie_pm);
 	clk_disable_unprepare(rockchip->hclk_pcie);
@@ -1533,14 +1567,17 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
+	int i;
 
 	pci_stop_root_bus(rockchip->root_bus);
 	pci_remove_root_bus(rockchip->root_bus);
 	pci_unmap_iospace(rockchip->io);
 	irq_domain_remove(rockchip->irq_domain);
 
-	phy_power_off(rockchip->phy);
-	phy_exit(rockchip->phy);
+	for (i = 0; i < MAX_LANE_NUM; i++) {
+		phy_power_off(rockchip->phys[i]);
+		phy_exit(rockchip->phys[i]);
+	}
 
 	clk_disable_unprepare(rockchip->clk_pcie_pm);
 	clk_disable_unprepare(rockchip->hclk_pcie);
-- 
1.9.1

Re: [RFC PATCH v4 2/7] PCI: rockchip: introduce per-lanes PHYs support

[Kishon Vijay Abraham I]

On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
> We distinguish the legacy PHY with the newer per-lane
> PHYs by adding legacy_phy flag. Note that the legacy phy
> is still the first option to be searched in order not to
> break the backward compatibility of DTB.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> 
> Changes in v4:
> - move rockchip_pcie_get_phys to where it stands in
>   patch 1.
> - print PHY index in err output log
> 
> Changes in v3:
> - kill rockchip_pcie_manipulate_phys and related stuff
> - use phys array
> - improve the commit msg
> 
> Changes in v2: None
> 
>  drivers/pci/host/pcie-rockchip.c | 77 +++++++++++++++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index c42feab..f510349 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -47,6 +47,7 @@
>  #define HIWORD_UPDATE_BIT(val)		HIWORD_UPDATE(val, val)
>  
>  #define ENCODE_LANES(x)			((((x) >> 1) & 3) << 4)
> +#define MAX_LANE_NUM			4
>  
>  #define PCIE_CLIENT_BASE		0x0
>  #define PCIE_CLIENT_CONFIG		(PCIE_CLIENT_BASE + 0x00)
> @@ -210,7 +211,8 @@
>  struct rockchip_pcie {
>  	void	__iomem *reg_base;		/* DT axi-base */
>  	void	__iomem *apb_base;		/* DT apb-base */
> -	struct	phy *phy;
> +	bool    legacy_phy;
> +	struct  phy *phys[MAX_LANE_NUM];
>  	struct	reset_control *core_rst;
>  	struct	reset_control *mgmt_rst;
>  	struct	reset_control *mgmt_sticky_rst;
> @@ -514,7 +516,7 @@ static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip)
>  static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  {
>  	struct device *dev = rockchip->dev;
> -	int err;
> +	int err, i;
>  	u32 status;
>  
>  	gpiod_set_value(rockchip->ep_gpio, 0);
> @@ -537,10 +539,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  		return err;
>  	}
>  
> -	err = phy_init(rockchip->phy);
> -	if (err < 0) {
> -		dev_err(dev, "fail to init phy, err %d\n", err);
> -		return err;
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		err = phy_init(rockchip->phys[i]);
> +		if (err) {
> +			dev_err(dev, "init phy%d err %d\n", i, err);
> +			return err;
> +		}
>  	}
>  
>  	err = reset_control_assert(rockchip->core_rst);
> @@ -602,10 +606,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  			    PCIE_CLIENT_MODE_RC,
>  			    PCIE_CLIENT_CONFIG);
>  
> -	err = phy_power_on(rockchip->phy);
> -	if (err) {
> -		dev_err(dev, "fail to power on phy, err %d\n", err);
> -		return err;
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		err = phy_power_on(rockchip->phys[i]);
> +		if (err) {
> +			dev_err(dev, "power on phy%d err %d\n", i, err);
> +			return err;
> +		}
>  	}
>  
>  	/*
> @@ -856,12 +862,38 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
>  static int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip)
>  {
>  	struct device *dev = rockchip->dev;
> +	struct phy *phy;
> +	char *name;
> +	u32 i;
> +
> +	rockchip->phys[0] = devm_phy_get(dev, "pcie-phy");
> +	if (IS_ERR(rockchip->phys[0])) {
> +		if (PTR_ERR(rockchip->phys[0]) == -EPROBE_DEFER)
> +			return PTR_ERR(rockchip->phys[0]);
> +		dev_dbg(dev, "missing legacy phy, and search for per-lane PHY\n");
> +	} else {
> +		rockchip->legacy_phy = true;
> +		dev_warn(dev, "legacy phy model is deprecated!\n");
> +		return 0;
> +	}
> +
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		name = kasprintf(GFP_KERNEL, "pcie-phy-%u", i);
> +		if (!name)
> +			return -ENOMEM;
> +
> +		phy = devm_of_phy_get(rockchip->dev,
> +				      rockchip->dev->of_node, name);
> +		kfree(name);
>  
> -	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> -	if (IS_ERR(rockchip->phy)) {
> -		if (PTR_ERR(rockchip->phy) != -EPROBE_DEFER)
> -			dev_err(dev, "missing phy\n");
> -		return PTR_ERR(rockchip->phy);
> +		if (IS_ERR(phy)) {
> +			if (PTR_ERR(phy) != -EPROBE_DEFER)
> +				dev_err(dev, "missing phy for lane %d: %ld\n",
> +					i, PTR_ERR(phy));
> +			return PTR_ERR(phy);
> +		}
> +
> +		rockchip->phys[i] = phy;
>  	}
>  
>  	return 0;
> @@ -1283,7 +1315,7 @@ static int rockchip_pcie_wait_l2(struct rockchip_pcie *rockchip)
>  static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> -	int ret;
> +	int ret, i;
>  
>  	/* disable core and cli int since we don't need to ack PME_ACK */
>  	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) |
> @@ -1296,8 +1328,10 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  		return ret;
>  	}
>  
> -	phy_power_off(rockchip->phy);
> -	phy_exit(rockchip->phy);
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		phy_power_off(rockchip->phys[i]);
> +		phy_exit(rockchip->phys[i]);
> +	}
>  
>  	clk_disable_unprepare(rockchip->clk_pcie_pm);
>  	clk_disable_unprepare(rockchip->hclk_pcie);
> @@ -1533,14 +1567,17 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> +	int i;
>  
>  	pci_stop_root_bus(rockchip->root_bus);
>  	pci_remove_root_bus(rockchip->root_bus);
>  	pci_unmap_iospace(rockchip->io);
>  	irq_domain_remove(rockchip->irq_domain);
>  
> -	phy_power_off(rockchip->phy);
> -	phy_exit(rockchip->phy);
> +	for (i = 0; i < MAX_LANE_NUM; i++) {
> +		phy_power_off(rockchip->phys[i]);
> +		phy_exit(rockchip->phys[i]);
> +	}
>  
>  	clk_disable_unprepare(rockchip->clk_pcie_pm);
>  	clk_disable_unprepare(rockchip->hclk_pcie);
> 

[RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Shawn Lin]

This patch reconstructs the whole driver to support per-lane
PHYs. Note that we could also support the legacy PHY if you
don't provide argument to our of_xlate.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4: None
Changes in v3:
- remove unnecessary forward declaration
- keep mutex inside struct rockchip_pcie_phy
- fix wrong check of args number
- move de-idle lanes after deasserting the reset

Changes in v2:
- deprecate legacy PHY model
- improve rockchip_pcie_phy_of_xlate
- fix wrong calculation of pwr_cnt and add new init_cnt
- add internal locking
- introduce per-lane data to simply the code

 drivers/phy/rockchip/phy-rockchip-pcie.c | 124 +++++++++++++++++++++++++++----
 1 file changed, 110 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index 6904633..b5005a5 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -73,10 +73,39 @@ struct rockchip_pcie_data {
 struct rockchip_pcie_phy {
 	struct rockchip_pcie_data *phy_data;
 	struct regmap *reg_base;
+	struct phy_pcie_instance {
+		struct phy *phy;
+		u32 index;
+	} phys[PHY_MAX_LANE_NUM];
+	struct mutex pcie_mutex;
 	struct reset_control *phy_rst;
 	struct clk *clk_pciephy_ref;
+	int pwr_cnt;
+	int init_cnt;
 };
 
+static inline
+struct rockchip_pcie_phy *to_pcie_phy(struct phy_pcie_instance *inst)
+{
+	return container_of(inst, struct rockchip_pcie_phy,
+					phys[inst->index]);
+}
+
+static struct phy *rockchip_pcie_phy_of_xlate(struct device *dev,
+					      struct of_phandle_args *args)
+{
+	struct rockchip_pcie_phy *rk_phy = dev_get_drvdata(dev);
+
+	if (args->args_count == 0)
+		return rk_phy->phys[0].phy;
+
+	if (WARN_ON(args->args[0] >= PHY_MAX_LANE_NUM))
+		return ERR_PTR(-ENODEV);
+
+	return rk_phy->phys[args->args[0]].phy;
+}
+
+
 static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
 			      u32 addr, u32 data)
 {
@@ -116,29 +145,59 @@ static inline u32 phy_rd_cfg(struct rockchip_pcie_phy *rk_phy,
 
 static int rockchip_pcie_phy_power_off(struct phy *phy)
 {
-	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
+	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
+	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
 	int err = 0;
 
+	mutex_lock(&rk_phy->pcie_mutex);
+
+	regmap_write(rk_phy->reg_base,
+		     rk_phy->phy_data->pcie_laneoff,
+		     HIWORD_UPDATE(PHY_LANE_IDLE_OFF,
+				   PHY_LANE_IDLE_MASK,
+				   PHY_LANE_IDLE_A_SHIFT + inst->index));
+
+	if (--rk_phy->pwr_cnt)
+		goto err_out;
+
 	err = reset_control_assert(rk_phy->phy_rst);
 	if (err) {
 		dev_err(&phy->dev, "assert phy_rst err %d\n", err);
-		return err;
+		goto err_restore;
 	}
 
+err_out:
+	mutex_unlock(&rk_phy->pcie_mutex);
 	return 0;
+
+err_restore:
+	++rk_phy->pwr_cnt;
+	regmap_write(rk_phy->reg_base,
+		     rk_phy->phy_data->pcie_laneoff,
+		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
+				   PHY_LANE_IDLE_MASK,
+				   PHY_LANE_IDLE_A_SHIFT + inst->index));
+	mutex_unlock(&rk_phy->pcie_mutex);
+	return err;
 }
 
 static int rockchip_pcie_phy_power_on(struct phy *phy)
 {
-	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
+	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
+	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
 	int err = 0;
 	u32 status;
 	unsigned long timeout;
 
+	mutex_lock(&rk_phy->pcie_mutex);
+
+	if (rk_phy->pwr_cnt++)
+		goto err_out;
+
 	err = reset_control_deassert(rk_phy->phy_rst);
 	if (err) {
 		dev_err(&phy->dev, "deassert phy_rst err %d\n", err);
-		return err;
+		goto err_pwr_cnt;
 	}
 
 	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
@@ -146,6 +205,12 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
 				   PHY_CFG_ADDR_MASK,
 				   PHY_CFG_ADDR_SHIFT));
 
+	regmap_write(rk_phy->reg_base,
+		     rk_phy->phy_data->pcie_laneoff,
+		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
+				   PHY_LANE_IDLE_MASK,
+				   PHY_LANE_IDLE_A_SHIFT + inst->index));
+
 	/*
 	 * No documented timeout value for phy operation below,
 	 * so we make it large enough here. And we use loop-break
@@ -214,18 +279,29 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
 		goto err_pll_lock;
 	}
 
+err_out:
+	mutex_unlock(&rk_phy->pcie_mutex);
 	return 0;
 
 err_pll_lock:
 	reset_control_assert(rk_phy->phy_rst);
+err_pwr_cnt:
+	--rk_phy->pwr_cnt;
+	mutex_unlock(&rk_phy->pcie_mutex);
 	return err;
 }
 
 static int rockchip_pcie_phy_init(struct phy *phy)
 {
-	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
+	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
+	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
 	int err = 0;
 
+	mutex_lock(&rk_phy->pcie_mutex);
+
+	if (rk_phy->init_cnt++)
+		goto err_out;
+
 	err = clk_prepare_enable(rk_phy->clk_pciephy_ref);
 	if (err) {
 		dev_err(&phy->dev, "Fail to enable pcie ref clock.\n");
@@ -238,20 +314,33 @@ static int rockchip_pcie_phy_init(struct phy *phy)
 		goto err_reset;
 	}
 
-	return err;
+err_out:
+	mutex_unlock(&rk_phy->pcie_mutex);
+	return 0;
 
 err_reset:
+
 	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
 err_refclk:
+	--rk_phy->init_cnt;
+	mutex_unlock(&rk_phy->pcie_mutex);
 	return err;
 }
 
 static int rockchip_pcie_phy_exit(struct phy *phy)
 {
-	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
+	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
+	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
+
+	mutex_lock(&rk_phy->pcie_mutex);
+
+	if (--rk_phy->init_cnt)
+		goto err_init_cnt;
 
 	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
 
+err_init_cnt:
+	mutex_unlock(&rk_phy->pcie_mutex);
 	return 0;
 }
 
@@ -283,10 +372,10 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rockchip_pcie_phy *rk_phy;
-	struct phy *generic_phy;
 	struct phy_provider *phy_provider;
 	struct regmap *grf;
 	const struct of_device_id *of_id;
+	int i;
 
 	grf = syscon_node_to_regmap(dev->parent->of_node);
 	if (IS_ERR(grf)) {
@@ -305,6 +394,8 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
 	rk_phy->phy_data = (struct rockchip_pcie_data *)of_id->data;
 	rk_phy->reg_base = grf;
 
+	mutex_init(&rk_phy->pcie_mutex);
+
 	rk_phy->phy_rst = devm_reset_control_get(dev, "phy");
 	if (IS_ERR(rk_phy->phy_rst)) {
 		if (PTR_ERR(rk_phy->phy_rst) != -EPROBE_DEFER)
@@ -319,14 +410,19 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(rk_phy->clk_pciephy_ref);
 	}
 
-	generic_phy = devm_phy_create(dev, dev->of_node, &ops);
-	if (IS_ERR(generic_phy)) {
-		dev_err(dev, "failed to create PHY\n");
-		return PTR_ERR(generic_phy);
+	for (i = 0; i < PHY_MAX_LANE_NUM; i++) {
+		rk_phy->phys[i].phy = devm_phy_create(dev, dev->of_node, &ops);
+		if (IS_ERR(rk_phy->phys[i].phy)) {
+			dev_err(dev, "failed to create PHY%d\n", i);
+			return PTR_ERR(rk_phy->phys[i].phy);
+		}
+		rk_phy->phys[i].index = i;
+		phy_set_drvdata(rk_phy->phys[i].phy, &rk_phy->phys[i]);
 	}
 
-	phy_set_drvdata(generic_phy, rk_phy);
-	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	platform_set_drvdata(pdev, rk_phy);
+	phy_provider = devm_of_phy_provider_register(dev,
+					rockchip_pcie_phy_of_xlate);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 }
-- 
1.9.1

Re: [RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Kishon Vijay Abraham I]

On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
> This patch reconstructs the whole driver to support per-lane
> PHYs. Note that we could also support the legacy PHY if you
> don't provide argument to our of_xlate.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>

This design looks good! I'll queue this once I start queuing 4.14.

Thanks
Kishon

Re: [RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Shawn Lin]

Hi Kishon,

On 2017/7/19 15:50, Kishon Vijay Abraham I wrote:
> 
> 
> On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
>> This patch reconstructs the whole driver to support per-lane
>> PHYs. Note that we could also support the legacy PHY if you
>> don't provide argument to our of_xlate.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> Reviewed-by: Brian Norris <briannorris@chromium.org>
>> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> 
> This design looks good! I'll queue this once I start queuing 4.14.
> 

Great!  Thanks. :)

> Thanks
> Kishon
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 
> 

Re: [RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Kishon Vijay Abraham I]

Hi Shawn,

On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
> This patch reconstructs the whole driver to support per-lane
> PHYs. Note that we could also support the legacy PHY if you
> don't provide argument to our of_xlate.

one comment below which I failed to notice before..
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v4: None
> Changes in v3:
> - remove unnecessary forward declaration
> - keep mutex inside struct rockchip_pcie_phy
> - fix wrong check of args number
> - move de-idle lanes after deasserting the reset
> 
> Changes in v2:
> - deprecate legacy PHY model
> - improve rockchip_pcie_phy_of_xlate
> - fix wrong calculation of pwr_cnt and add new init_cnt
> - add internal locking
> - introduce per-lane data to simply the code
> 
>  drivers/phy/rockchip/phy-rockchip-pcie.c | 124 +++++++++++++++++++++++++++----
>  1 file changed, 110 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
> index 6904633..b5005a5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-pcie.c
> +++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
> @@ -73,10 +73,39 @@ struct rockchip_pcie_data {
>  struct rockchip_pcie_phy {
>  	struct rockchip_pcie_data *phy_data;
>  	struct regmap *reg_base;
> +	struct phy_pcie_instance {
> +		struct phy *phy;
> +		u32 index;
> +	} phys[PHY_MAX_LANE_NUM];
> +	struct mutex pcie_mutex;
>  	struct reset_control *phy_rst;
>  	struct clk *clk_pciephy_ref;
> +	int pwr_cnt;
> +	int init_cnt;
>  };
>  
> +static inline
> +struct rockchip_pcie_phy *to_pcie_phy(struct phy_pcie_instance *inst)
> +{
> +	return container_of(inst, struct rockchip_pcie_phy,
> +					phys[inst->index]);
> +}
> +
> +static struct phy *rockchip_pcie_phy_of_xlate(struct device *dev,
> +					      struct of_phandle_args *args)
> +{
> +	struct rockchip_pcie_phy *rk_phy = dev_get_drvdata(dev);
> +
> +	if (args->args_count == 0)
> +		return rk_phy->phys[0].phy;
> +
> +	if (WARN_ON(args->args[0] >= PHY_MAX_LANE_NUM))
> +		return ERR_PTR(-ENODEV);
> +
> +	return rk_phy->phys[args->args[0]].phy;
> +}
> +
> +
>  static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
>  			      u32 addr, u32 data)
>  {
> @@ -116,29 +145,59 @@ static inline u32 phy_rd_cfg(struct rockchip_pcie_phy *rk_phy,
>  
>  static int rockchip_pcie_phy_power_off(struct phy *phy)
>  {
> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>  	int err = 0;
>  
> +	mutex_lock(&rk_phy->pcie_mutex);
> +
> +	regmap_write(rk_phy->reg_base,
> +		     rk_phy->phy_data->pcie_laneoff,
> +		     HIWORD_UPDATE(PHY_LANE_IDLE_OFF,
> +				   PHY_LANE_IDLE_MASK,
> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
> +
> +	if (--rk_phy->pwr_cnt)
> +		goto err_out;
> +
>  	err = reset_control_assert(rk_phy->phy_rst);
>  	if (err) {
>  		dev_err(&phy->dev, "assert phy_rst err %d\n", err);
> -		return err;
> +		goto err_restore;
>  	}
>  
> +err_out:
> +	mutex_unlock(&rk_phy->pcie_mutex);
>  	return 0;
> +
> +err_restore:
> +	++rk_phy->pwr_cnt;
> +	regmap_write(rk_phy->reg_base,
> +		     rk_phy->phy_data->pcie_laneoff,
> +		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
> +				   PHY_LANE_IDLE_MASK,
> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
> +	mutex_unlock(&rk_phy->pcie_mutex);
> +	return err;
>  }
>  
>  static int rockchip_pcie_phy_power_on(struct phy *phy)
>  {
> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>  	int err = 0;
>  	u32 status;
>  	unsigned long timeout;
>  
> +	mutex_lock(&rk_phy->pcie_mutex);
> +
> +	if (rk_phy->pwr_cnt++)
> +		goto err_out;
> +
>  	err = reset_control_deassert(rk_phy->phy_rst);
>  	if (err) {
>  		dev_err(&phy->dev, "deassert phy_rst err %d\n", err);
> -		return err;
> +		goto err_pwr_cnt;
>  	}
>  
>  	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
> @@ -146,6 +205,12 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
>  				   PHY_CFG_ADDR_MASK,
>  				   PHY_CFG_ADDR_SHIFT));
>  
> +	regmap_write(rk_phy->reg_base,
> +		     rk_phy->phy_data->pcie_laneoff,
> +		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
> +				   PHY_LANE_IDLE_MASK,
> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
> +
>  	/*
>  	 * No documented timeout value for phy operation below,
>  	 * so we make it large enough here. And we use loop-break
> @@ -214,18 +279,29 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
>  		goto err_pll_lock;
>  	}
>  
> +err_out:
> +	mutex_unlock(&rk_phy->pcie_mutex);
>  	return 0;
>  
>  err_pll_lock:
>  	reset_control_assert(rk_phy->phy_rst);
> +err_pwr_cnt:
> +	--rk_phy->pwr_cnt;
> +	mutex_unlock(&rk_phy->pcie_mutex);
>  	return err;
>  }
>  
>  static int rockchip_pcie_phy_init(struct phy *phy)
>  {
> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>  	int err = 0;
>  
> +	mutex_lock(&rk_phy->pcie_mutex);
> +
> +	if (rk_phy->init_cnt++)
> +		goto err_out;
> +
>  	err = clk_prepare_enable(rk_phy->clk_pciephy_ref);
>  	if (err) {
>  		dev_err(&phy->dev, "Fail to enable pcie ref clock.\n");
> @@ -238,20 +314,33 @@ static int rockchip_pcie_phy_init(struct phy *phy)
>  		goto err_reset;
>  	}
>  
> -	return err;
> +err_out:
> +	mutex_unlock(&rk_phy->pcie_mutex);
> +	return 0;
>  
>  err_reset:
> +
>  	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
>  err_refclk:
> +	--rk_phy->init_cnt;
> +	mutex_unlock(&rk_phy->pcie_mutex);
>  	return err;
>  }
>  
>  static int rockchip_pcie_phy_exit(struct phy *phy)
>  {
> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
> +
> +	mutex_lock(&rk_phy->pcie_mutex);
> +
> +	if (--rk_phy->init_cnt)
> +		goto err_init_cnt;
>  
>  	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
>  
> +err_init_cnt:
> +	mutex_unlock(&rk_phy->pcie_mutex);
>  	return 0;
>  }
>  
> @@ -283,10 +372,10 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct rockchip_pcie_phy *rk_phy;
> -	struct phy *generic_phy;
>  	struct phy_provider *phy_provider;
>  	struct regmap *grf;
>  	const struct of_device_id *of_id;
> +	int i;
>  
>  	grf = syscon_node_to_regmap(dev->parent->of_node);
>  	if (IS_ERR(grf)) {
> @@ -305,6 +394,8 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>  	rk_phy->phy_data = (struct rockchip_pcie_data *)of_id->data;
>  	rk_phy->reg_base = grf;
>  
> +	mutex_init(&rk_phy->pcie_mutex);
> +
>  	rk_phy->phy_rst = devm_reset_control_get(dev, "phy");
>  	if (IS_ERR(rk_phy->phy_rst)) {
>  		if (PTR_ERR(rk_phy->phy_rst) != -EPROBE_DEFER)
> @@ -319,14 +410,19 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>  		return PTR_ERR(rk_phy->clk_pciephy_ref);
>  	}
>  
> -	generic_phy = devm_phy_create(dev, dev->of_node, &ops);
> -	if (IS_ERR(generic_phy)) {
> -		dev_err(dev, "failed to create PHY\n");
> -		return PTR_ERR(generic_phy);
> +	for (i = 0; i < PHY_MAX_LANE_NUM; i++) {
> +		rk_phy->phys[i].phy = devm_phy_create(dev, dev->of_node, &ops);
> +		if (IS_ERR(rk_phy->phys[i].phy)) {
> +			dev_err(dev, "failed to create PHY%d\n", i);
> +			return PTR_ERR(rk_phy->phys[i].phy);
> +		}
> +		rk_phy->phys[i].index = i;


For legacy dt, it would create more number of phys than required right? it
would be nice to fix that.

Thanks
Kishon

Re: [RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Shawn Lin]

Hi Kishon,

On 2017/7/19 16:08, Kishon Vijay Abraham I wrote:
> Hi Shawn,
> 
> On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
>> This patch reconstructs the whole driver to support per-lane
>> PHYs. Note that we could also support the legacy PHY if you
>> don't provide argument to our of_xlate.
> 
> one comment below which I failed to notice before..

I will fix it and respin v5.

BTW, are you fine with all these patches merged via Bjorn's
PCI tree in order to narrow down the timing gap for testing
linux-next? If yes, could you kindly ack patch 3 of V5
if it looks good to you? :)

Thanks.

>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> Reviewed-by: Brian Norris <briannorris@chromium.org>
>> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>> Changes in v4: None
>> Changes in v3:
>> - remove unnecessary forward declaration
>> - keep mutex inside struct rockchip_pcie_phy
>> - fix wrong check of args number
>> - move de-idle lanes after deasserting the reset
>>
>> Changes in v2:
>> - deprecate legacy PHY model
>> - improve rockchip_pcie_phy_of_xlate
>> - fix wrong calculation of pwr_cnt and add new init_cnt
>> - add internal locking
>> - introduce per-lane data to simply the code
>>
>>   drivers/phy/rockchip/phy-rockchip-pcie.c | 124 +++++++++++++++++++++++++++----
>>   1 file changed, 110 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
>> index 6904633..b5005a5 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-pcie.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
>> @@ -73,10 +73,39 @@ struct rockchip_pcie_data {
>>   struct rockchip_pcie_phy {
>>   	struct rockchip_pcie_data *phy_data;
>>   	struct regmap *reg_base;
>> +	struct phy_pcie_instance {
>> +		struct phy *phy;
>> +		u32 index;
>> +	} phys[PHY_MAX_LANE_NUM];
>> +	struct mutex pcie_mutex;
>>   	struct reset_control *phy_rst;
>>   	struct clk *clk_pciephy_ref;
>> +	int pwr_cnt;
>> +	int init_cnt;
>>   };
>>   
>> +static inline
>> +struct rockchip_pcie_phy *to_pcie_phy(struct phy_pcie_instance *inst)
>> +{
>> +	return container_of(inst, struct rockchip_pcie_phy,
>> +					phys[inst->index]);
>> +}
>> +
>> +static struct phy *rockchip_pcie_phy_of_xlate(struct device *dev,
>> +					      struct of_phandle_args *args)
>> +{
>> +	struct rockchip_pcie_phy *rk_phy = dev_get_drvdata(dev);
>> +
>> +	if (args->args_count == 0)
>> +		return rk_phy->phys[0].phy;
>> +
>> +	if (WARN_ON(args->args[0] >= PHY_MAX_LANE_NUM))
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	return rk_phy->phys[args->args[0]].phy;
>> +}
>> +
>> +
>>   static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
>>   			      u32 addr, u32 data)
>>   {
>> @@ -116,29 +145,59 @@ static inline u32 phy_rd_cfg(struct rockchip_pcie_phy *rk_phy,
>>   
>>   static int rockchip_pcie_phy_power_off(struct phy *phy)
>>   {
>> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
>> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>>   	int err = 0;
>>   
>> +	mutex_lock(&rk_phy->pcie_mutex);
>> +
>> +	regmap_write(rk_phy->reg_base,
>> +		     rk_phy->phy_data->pcie_laneoff,
>> +		     HIWORD_UPDATE(PHY_LANE_IDLE_OFF,
>> +				   PHY_LANE_IDLE_MASK,
>> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
>> +
>> +	if (--rk_phy->pwr_cnt)
>> +		goto err_out;
>> +
>>   	err = reset_control_assert(rk_phy->phy_rst);
>>   	if (err) {
>>   		dev_err(&phy->dev, "assert phy_rst err %d\n", err);
>> -		return err;
>> +		goto err_restore;
>>   	}
>>   
>> +err_out:
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>>   	return 0;
>> +
>> +err_restore:
>> +	++rk_phy->pwr_cnt;
>> +	regmap_write(rk_phy->reg_base,
>> +		     rk_phy->phy_data->pcie_laneoff,
>> +		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
>> +				   PHY_LANE_IDLE_MASK,
>> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>> +	return err;
>>   }
>>   
>>   static int rockchip_pcie_phy_power_on(struct phy *phy)
>>   {
>> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
>> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>>   	int err = 0;
>>   	u32 status;
>>   	unsigned long timeout;
>>   
>> +	mutex_lock(&rk_phy->pcie_mutex);
>> +
>> +	if (rk_phy->pwr_cnt++)
>> +		goto err_out;
>> +
>>   	err = reset_control_deassert(rk_phy->phy_rst);
>>   	if (err) {
>>   		dev_err(&phy->dev, "deassert phy_rst err %d\n", err);
>> -		return err;
>> +		goto err_pwr_cnt;
>>   	}
>>   
>>   	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
>> @@ -146,6 +205,12 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
>>   				   PHY_CFG_ADDR_MASK,
>>   				   PHY_CFG_ADDR_SHIFT));
>>   
>> +	regmap_write(rk_phy->reg_base,
>> +		     rk_phy->phy_data->pcie_laneoff,
>> +		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
>> +				   PHY_LANE_IDLE_MASK,
>> +				   PHY_LANE_IDLE_A_SHIFT + inst->index));
>> +
>>   	/*
>>   	 * No documented timeout value for phy operation below,
>>   	 * so we make it large enough here. And we use loop-break
>> @@ -214,18 +279,29 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
>>   		goto err_pll_lock;
>>   	}
>>   
>> +err_out:
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>>   	return 0;
>>   
>>   err_pll_lock:
>>   	reset_control_assert(rk_phy->phy_rst);
>> +err_pwr_cnt:
>> +	--rk_phy->pwr_cnt;
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>>   	return err;
>>   }
>>   
>>   static int rockchip_pcie_phy_init(struct phy *phy)
>>   {
>> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
>> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>>   	int err = 0;
>>   
>> +	mutex_lock(&rk_phy->pcie_mutex);
>> +
>> +	if (rk_phy->init_cnt++)
>> +		goto err_out;
>> +
>>   	err = clk_prepare_enable(rk_phy->clk_pciephy_ref);
>>   	if (err) {
>>   		dev_err(&phy->dev, "Fail to enable pcie ref clock.\n");
>> @@ -238,20 +314,33 @@ static int rockchip_pcie_phy_init(struct phy *phy)
>>   		goto err_reset;
>>   	}
>>   
>> -	return err;
>> +err_out:
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>> +	return 0;
>>   
>>   err_reset:
>> +
>>   	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
>>   err_refclk:
>> +	--rk_phy->init_cnt;
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>>   	return err;
>>   }
>>   
>>   static int rockchip_pcie_phy_exit(struct phy *phy)
>>   {
>> -	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +	struct phy_pcie_instance *inst = phy_get_drvdata(phy);
>> +	struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
>> +
>> +	mutex_lock(&rk_phy->pcie_mutex);
>> +
>> +	if (--rk_phy->init_cnt)
>> +		goto err_init_cnt;
>>   
>>   	clk_disable_unprepare(rk_phy->clk_pciephy_ref);
>>   
>> +err_init_cnt:
>> +	mutex_unlock(&rk_phy->pcie_mutex);
>>   	return 0;
>>   }
>>   
>> @@ -283,10 +372,10 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct rockchip_pcie_phy *rk_phy;
>> -	struct phy *generic_phy;
>>   	struct phy_provider *phy_provider;
>>   	struct regmap *grf;
>>   	const struct of_device_id *of_id;
>> +	int i;
>>   
>>   	grf = syscon_node_to_regmap(dev->parent->of_node);
>>   	if (IS_ERR(grf)) {
>> @@ -305,6 +394,8 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>>   	rk_phy->phy_data = (struct rockchip_pcie_data *)of_id->data;
>>   	rk_phy->reg_base = grf;
>>   
>> +	mutex_init(&rk_phy->pcie_mutex);
>> +
>>   	rk_phy->phy_rst = devm_reset_control_get(dev, "phy");
>>   	if (IS_ERR(rk_phy->phy_rst)) {
>>   		if (PTR_ERR(rk_phy->phy_rst) != -EPROBE_DEFER)
>> @@ -319,14 +410,19 @@ static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>>   		return PTR_ERR(rk_phy->clk_pciephy_ref);
>>   	}
>>   
>> -	generic_phy = devm_phy_create(dev, dev->of_node, &ops);
>> -	if (IS_ERR(generic_phy)) {
>> -		dev_err(dev, "failed to create PHY\n");
>> -		return PTR_ERR(generic_phy);
>> +	for (i = 0; i < PHY_MAX_LANE_NUM; i++) {
>> +		rk_phy->phys[i].phy = devm_phy_create(dev, dev->of_node, &ops);
>> +		if (IS_ERR(rk_phy->phys[i].phy)) {
>> +			dev_err(dev, "failed to create PHY%d\n", i);
>> +			return PTR_ERR(rk_phy->phys[i].phy);
>> +		}
>> +		rk_phy->phys[i].index = i;
> 
> 
> For legacy dt, it would create more number of phys than required right? it
> would be nice to fix that.
> 
> Thanks
> Kishon
> 
> 
> 

Re: [RFC PATCH v4 3/7] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Kishon Vijay Abraham I]

On Wednesday 19 July 2017 03:06 PM, Shawn Lin wrote:
> Hi Kishon,
> 
> On 2017/7/19 16:08, Kishon Vijay Abraham I wrote:
>> Hi Shawn,
>>
>> On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
>>> This patch reconstructs the whole driver to support per-lane
>>> PHYs. Note that we could also support the legacy PHY if you
>>> don't provide argument to our of_xlate.
>>
>> one comment below which I failed to notice before..
> 
> I will fix it and respin v5.
> 
> BTW, are you fine with all these patches merged via Bjorn's
> PCI tree in order to narrow down the timing gap for testing
> linux-next? If yes, could you kindly ack patch 3 of V5
> if it looks good to you? :)

sure!

Thanks
Kishon

[RFC PATCH v4 4/7] PCI: rockchip: idle the inactive PHY(s)

[Shawn Lin]

Check the status of all lanes and idle the inactive one(s).

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4:
- return u8 for rockchip_pcie_lane_map

Changes in v3:
- use cached lanes_map to avoid powering off inactive
  lanes twice

Changes in v2: None

 drivers/pci/host/pcie-rockchip.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index f510349..a8c51b0 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -15,6 +15,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/bitrev.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
@@ -112,6 +113,9 @@
 #define   PCIE_CORE_TXCREDIT_CFG1_MUI_SHIFT	16
 #define   PCIE_CORE_TXCREDIT_CFG1_MUI_ENCODE(x) \
 		(((x) >> 3) << PCIE_CORE_TXCREDIT_CFG1_MUI_SHIFT)
+#define PCIE_CORE_LANE_MAP             (PCIE_CORE_CTRL_MGMT_BASE + 0x200)
+#define   PCIE_CORE_LANE_MAP_MASK              0x0000000f
+#define   PCIE_CORE_LANE_MAP_REVERSE           BIT(16)
 #define PCIE_CORE_INT_STATUS		(PCIE_CORE_CTRL_MGMT_BASE + 0x20c)
 #define   PCIE_CORE_INT_PRFPE			BIT(0)
 #define   PCIE_CORE_INT_CRFPE			BIT(1)
@@ -229,6 +233,7 @@ struct rockchip_pcie {
 	struct	regulator *vpcie0v9; /* 0.9V power supply */
 	struct	gpio_desc *ep_gpio;
 	u32	lanes;
+	u8      lanes_map;
 	u8	root_bus_nr;
 	int	link_gen;
 	struct	device *dev;
@@ -301,6 +306,18 @@ static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip,
 	return 1;
 }
 
+static u8 rockchip_pcie_lane_map(struct rockchip_pcie *rockchip)
+{
+	u32 val = rockchip_pcie_read(rockchip, PCIE_CORE_LANE_MAP);
+	u8 map = val & PCIE_CORE_LANE_MAP_MASK;
+
+	/* The link may be using a reverse-indexed mapping. */
+	if (val & PCIE_CORE_LANE_MAP_REVERSE)
+		map = bitrev8(map) >> 4;
+
+	return map;
+}
+
 static int rockchip_pcie_rd_own_conf(struct rockchip_pcie *rockchip,
 				     int where, int size, u32 *val)
 {
@@ -697,6 +714,18 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			  PCIE_CORE_PL_CONF_LANE_SHIFT);
 	dev_dbg(dev, "current link width is x%d\n", status);
 
+	if (!rockchip->legacy_phy) {
+		/*  power off unused lane(s) */
+		rockchip->lanes_map = rockchip_pcie_lane_map(rockchip);
+		for (i = 0; i < MAX_LANE_NUM; i++) {
+			if (rockchip->lanes_map & BIT(i))
+				continue;
+
+			dev_dbg(dev, "idling lane %d\n", i);
+			phy_power_off(rockchip->phys[i]);
+		}
+	}
+
 	rockchip_pcie_write(rockchip, ROCKCHIP_VENDOR_ID,
 			    PCIE_CORE_CONFIG_VENDOR);
 	rockchip_pcie_write(rockchip,
@@ -1329,7 +1358,9 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
 	}
 
 	for (i = 0; i < MAX_LANE_NUM; i++) {
-		phy_power_off(rockchip->phys[i]);
+		/* inactive lane is already powered off */
+		if (rockchip->lanes_map & BIT(i))
+			phy_power_off(rockchip->phys[i]);
 		phy_exit(rockchip->phys[i]);
 	}
 
@@ -1575,7 +1606,9 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 	irq_domain_remove(rockchip->irq_domain);
 
 	for (i = 0; i < MAX_LANE_NUM; i++) {
-		phy_power_off(rockchip->phys[i]);
+		/* inactive lane is already powered off */
+		if (rockchip->lanes_map & BIT(i))
+			phy_power_off(rockchip->phys[i]);
 		phy_exit(rockchip->phys[i]);
 	}
 
-- 
1.9.1

Re: [RFC PATCH v4 4/7] PCI: rockchip: idle the inactive PHY(s)

[Kishon Vijay Abraham I]

On Wednesday 19 July 2017 12:52 PM, Shawn Lin wrote:
> Check the status of all lanes and idle the inactive one(s).
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> 
> Changes in v4:
> - return u8 for rockchip_pcie_lane_map
> 
> Changes in v3:
> - use cached lanes_map to avoid powering off inactive
>   lanes twice
> 
> Changes in v2: None
> 
>  drivers/pci/host/pcie-rockchip.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index f510349..a8c51b0 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -15,6 +15,7 @@
>   * (at your option) any later version.
>   */
>  
> +#include <linux/bitrev.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
> @@ -112,6 +113,9 @@
>  #define   PCIE_CORE_TXCREDIT_CFG1_MUI_SHIFT	16
>  #define   PCIE_CORE_TXCREDIT_CFG1_MUI_ENCODE(x) \
>  		(((x) >> 3) << PCIE_CORE_TXCREDIT_CFG1_MUI_SHIFT)
> +#define PCIE_CORE_LANE_MAP             (PCIE_CORE_CTRL_MGMT_BASE + 0x200)
> +#define   PCIE_CORE_LANE_MAP_MASK              0x0000000f
> +#define   PCIE_CORE_LANE_MAP_REVERSE           BIT(16)
>  #define PCIE_CORE_INT_STATUS		(PCIE_CORE_CTRL_MGMT_BASE + 0x20c)
>  #define   PCIE_CORE_INT_PRFPE			BIT(0)
>  #define   PCIE_CORE_INT_CRFPE			BIT(1)
> @@ -229,6 +233,7 @@ struct rockchip_pcie {
>  	struct	regulator *vpcie0v9; /* 0.9V power supply */
>  	struct	gpio_desc *ep_gpio;
>  	u32	lanes;
> +	u8      lanes_map;
>  	u8	root_bus_nr;
>  	int	link_gen;
>  	struct	device *dev;
> @@ -301,6 +306,18 @@ static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip,
>  	return 1;
>  }
>  
> +static u8 rockchip_pcie_lane_map(struct rockchip_pcie *rockchip)
> +{
> +	u32 val = rockchip_pcie_read(rockchip, PCIE_CORE_LANE_MAP);
> +	u8 map = val & PCIE_CORE_LANE_MAP_MASK;
> +
> +	/* The link may be using a reverse-indexed mapping. */
> +	if (val & PCIE_CORE_LANE_MAP_REVERSE)
> +		map = bitrev8(map) >> 4;
> +
> +	return map;
> +}
> +
>  static int rockchip_pcie_rd_own_conf(struct rockchip_pcie *rockchip,
>  				     int where, int size, u32 *val)
>  {
> @@ -697,6 +714,18 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  			  PCIE_CORE_PL_CONF_LANE_SHIFT);
>  	dev_dbg(dev, "current link width is x%d\n", status);
>  
> +	if (!rockchip->legacy_phy) {
> +		/*  power off unused lane(s) */
> +		rockchip->lanes_map = rockchip_pcie_lane_map(rockchip);
> +		for (i = 0; i < MAX_LANE_NUM; i++) {
> +			if (rockchip->lanes_map & BIT(i))
> +				continue;
> +
> +			dev_dbg(dev, "idling lane %d\n", i);
> +			phy_power_off(rockchip->phys[i]);
> +		}
> +	}
> +
>  	rockchip_pcie_write(rockchip, ROCKCHIP_VENDOR_ID,
>  			    PCIE_CORE_CONFIG_VENDOR);
>  	rockchip_pcie_write(rockchip,
> @@ -1329,7 +1358,9 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev)
>  	}
>  
>  	for (i = 0; i < MAX_LANE_NUM; i++) {
> -		phy_power_off(rockchip->phys[i]);
> +		/* inactive lane is already powered off */
> +		if (rockchip->lanes_map & BIT(i))
> +			phy_power_off(rockchip->phys[i]);
>  		phy_exit(rockchip->phys[i]);
>  	}
>  
> @@ -1575,7 +1606,9 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  	irq_domain_remove(rockchip->irq_domain);
>  
>  	for (i = 0; i < MAX_LANE_NUM; i++) {
> -		phy_power_off(rockchip->phys[i]);
> +		/* inactive lane is already powered off */
> +		if (rockchip->lanes_map & BIT(i))
> +			phy_power_off(rockchip->phys[i]);
>  		phy_exit(rockchip->phys[i]);
>  	}
>  
> 

[RFC PATCH v4 5/7] arm64: dts: rockchip: convert PCIe to use per-lane PHYs for rk3339

[Shawn Lin]

Convert all RK3399 platforms to use per-lane PHY model in
order to save more power by idling the unused lane(s).

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 69c56f7..5b78ce1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -238,8 +238,10 @@
 		linux,pci-domain = <0>;
 		max-link-speed = <1>;
 		msi-map = <0x0 &its 0x0 0x1000>;
-		phys = <&pcie_phy>;
-		phy-names = "pcie-phy";
+		phys = <&pcie_phy 0>, <&pcie_phy 1>,
+		       <&pcie_phy 2>, <&pcie_phy 3>;
+		phy-names = "pcie-phy-0", "pcie-phy-1",
+			    "pcie-phy-2", "pcie-phy-3";
 		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000
 			  0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
 		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
@@ -1295,7 +1297,7 @@
 			compatible = "rockchip,rk3399-pcie-phy";
 			clocks = <&cru SCLK_PCIEPHY_REF>;
 			clock-names = "refclk";
-			#phy-cells = <0>;
+			#phy-cells = <1>;
 			resets = <&cru SRST_PCIEPHY>;
 			reset-names = "phy";
 			status = "disabled";
-- 
1.9.1

[RFC PATCH v4 6/7] dt-bindings: PCI: rockchip: convert to use per-lane PHY model

[Shawn Lin]

Deprecate legacy PHY model and encourage to use per-lane PHY
model.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 25 ++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index 1453a73..41bced4 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -19,8 +19,6 @@ Required properties:
 	- "pm"
 - msi-map: Maps a Requester ID to an MSI controller and associated
 	msi-specifier data. See ./pci-msi.txt
-- phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
-- phy-names:  MUST be "pcie-phy".
 - interrupts: Three interrupt entries must be specified.
 - interrupt-names: Must include the following names
 	- "sys"
@@ -42,6 +40,18 @@ Required properties:
 	interrupt source. The value must be 1.
 - interrupt-map-mask and interrupt-map: standard PCI properties
 
+Required properties for legacy PHY model (deprecated):
+- phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
+- phy-names:  MUST be "pcie-phy".
+
+Required properties for per-lane PHY model (preferred):
+- phys: Must contain an phandle to a PHY for each entry in phy-names.
+- phy-names: Must include 4 entries for all 4 lanes even if some of
+  them won't be used for your cases. Entries are of the form "pcie-phy-N":
+  where N ranges from 0 to 3.
+  (see example below and you MUST also refer to ../phy/rockchip-pcie-phy.txt
+  for changing the #phy-cells of phy node to support it)
+
 Optional Property:
 - aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
 	using 24MHz OSC for RC's PHY.
@@ -95,6 +105,7 @@ pcie0: pcie@f8000000 {
 		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
 	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
 		      "pm", "pclk", "aclk";
+	/* deprecated legacy PHY model */
 	phys = <&pcie_phy>;
 	phy-names = "pcie-phy";
 	pinctrl-names = "default";
@@ -111,3 +122,13 @@ pcie0: pcie@f8000000 {
 		#interrupt-cells = <1>;
 	};
 };
+
+pcie0: pcie@f8000000 {
+	...
+
+	/* preferred per-lane PHY model */
+	phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
+	phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
+
+	...
+};
-- 
1.9.1

[RFC PATCH v4 7/7] dt-bindings: phy: convert to use per-lane Rockchip PCIe PHY

[Shawn Lin]

This patch deprecate the legacy PCIe PHY and encourage user
to use per-lane PHY mode by setting #phy-cells to 1.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---

Changes in v4: None
Changes in v3:
- rename the commit tile

Changes in v2: None

 Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt
index 0f6222a..b496042 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt
@@ -3,7 +3,6 @@ Rockchip PCIE PHY
 
 Required properties:
  - compatible: rockchip,rk3399-pcie-phy
- - #phy-cells: must be 0
  - clocks: Must contain an entry in clock-names.
 	See ../clocks/clock-bindings.txt for details.
  - clock-names: Must be "refclk"
@@ -11,6 +10,12 @@ Required properties:
 	See ../reset/reset.txt for details.
  - reset-names: Must be "phy"
 
+Required properties for legacy PHY mode (deprecated):
+ - #phy-cells: must be 0
+
+Required properties for per-lane PHY mode (preferred):
+ - #phy-cells: must be 1
+
 Example:
 
 grf: syscon@ff770000 {
-- 
1.9.1