[PATCH V2 0/5] PCI: exynos: use the PHY generic framework

[PATCH V2 0/5] PCI: exynos: use the PHY generic framework

[Jaehoon Chung]

This patchset is for using PHY generic framework.
Current pci-exyons doesn't use the phy framework since there haven't been on
PHY subsystem when Exynos5440 had been upstremed.
The not using PHY framework make the difficult to upstream the other
Exynos SoCs.

Before upstreaming the other Exynos SoCs, it's goal what make to use the PHY framework.

This patchset has the below modifications:
1) Introduces the phy-pcie-pcie
2) Handles Phy controller from PHY framework for pci-exynos
3) Modifies the dt-binding of pci-exynos
- The using the getting configuration space address from ranges is old.
- Deprecated the old way.
4) Maintains the backward compatibility

NOTE: These patches based on below patches:
	http://patchwork.ozlabs.org/patch/706998/
	http://patchwork.ozlabs.org/patch/706997/
	http://patchwork.ozlabs.org/patch/706995/
	http://patchwork.ozlabs.org/patch/706994/
	http://patchwork.ozlabs.org/patch/703530/
	- This patch should be conflicted. so fixes the manually.
	http://patchwork.ozlabs.org/patch/708414/

Changelog on V2:
- Keep current codes for backward compatibility
- Fixes some typos
- Split the patches for removing the dependency
- Removes the unnecessary codes
- Change the patch's sequence
- Based on latest PCI git repository.(next branch)

Jaehoon Chung (5):
  Documetation: samsung-phy: add the exynos-pcie-phy binding
  phy: phy-exynos-pcie: Add support for Exynos PCIe phy
  Documetation: binding: modify the exynos5440 pcie binding
  PCI: exynos: support the using PHY generic framework
  ARM: dts: exynos5440: support the phy-pcie node for pcie

 .../bindings/pci/samsung,exynos5440-pcie.txt       |  29 +++
 .../devicetree/bindings/phy/samsung-phy.txt        |  17 ++
 arch/arm/boot/dts/exynos5440.dtsi                  |  34 ++-
 drivers/pci/host/pci-exynos.c                      |  61 ++++-
 drivers/phy/Kconfig                                |   9 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-exynos-pcie.c                      | 280 +++++++++++++++++++++
 7 files changed, 408 insertions(+), 23 deletions(-)
 create mode 100644 drivers/phy/phy-exynos-pcie.c

-- 
2.10.2

[PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[Jaehoon Chung]

Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
This is for using generic PHY framework.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog on V2:
- Remove the child node.
- Add 2nd address to the parent reg prop.

 Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 9872ba8..ab80bfe 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -191,3 +191,20 @@ Example:
 		usbdrdphy0 = &usb3_phy0;
 		usbdrdphy1 = &usb3_phy1;
 	};
+
+Samsung Exynos SoC series PCIe PHY controller
+--------------------------------------------------
+Required properties:
+- compatible : Should be set to "samsung,exynos5440-pcie-phy"
+- #phy-cells : Must be zero
+- reg : a register used by phy driver.
+	- First is for phy register, second is for block register.
+- reg-names : Must be set to "phy" and "block".
+
+Example:
+	pcie_phy0: pcie-phy@270000 {
+		#phy-cells = <0>;
+		compatible = "samsung,exynos5440-pcie-phy";
+		reg = <0x270000 0x1000>, <0x271000 0x40>;
+		reg-names = "phy", "block";
+	};
-- 
2.10.2

Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[Rob Herring]

On Wed, Jan 04, 2017 at 09:34:31PM +0900, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Remove the child node.
> - Add 2nd address to the parent reg prop.
> 
>  Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

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

Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[Rob Herring]

On Wed, Jan 04, 2017 at 09:34:31PM +0900, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Remove the child node.
> - Add 2nd address to the parent reg prop.
> 
>  Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[Alim Akhtar]

Hi Jaehoon,

On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Remove the child node.
> - Add 2nd address to the parent reg prop.
>
>  Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 9872ba8..ab80bfe 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -191,3 +191,20 @@ Example:
>  		usbdrdphy0 = &usb3_phy0;
>  		usbdrdphy1 = &usb3_phy1;
>  	};
> +
> +Samsung Exynos SoC series PCIe PHY controller
> +--------------------------------------------------
> +Required properties:
> +- compatible : Should be set to "samsung,exynos5440-pcie-phy"
> +- #phy-cells : Must be zero
> +- reg : a register used by phy driver.
> +	- First is for phy register, second is for block register.
> +- reg-names : Must be set to "phy" and "block".
> +
In general PHY uses a "reference clock" to work, if that is true for 
5440 also, will you consider adding an (may be) optional clock 
properties as well?

otherwise this binding looks ok to me.
> +Example:
> +	pcie_phy0: pcie-phy@270000 {
> +		#phy-cells = <0>;
> +		compatible = "samsung,exynos5440-pcie-phy";
> +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> +		reg-names = "phy", "block";
> +	};
>

Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[pankaj.dubey]

Hi,

On Thursday 05 January 2017 09:46 AM, Alim Akhtar wrote:
> Hi Jaehoon,
> 
> On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
>> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
>> This is for using generic PHY framework.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - Remove the child node.
>> - Add 2nd address to the parent reg prop.
>>
>>  Documentation/devicetree/bindings/phy/samsung-phy.txt | 17
>> +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> index 9872ba8..ab80bfe 100644
>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> @@ -191,3 +191,20 @@ Example:
>>          usbdrdphy0 = &usb3_phy0;
>>          usbdrdphy1 = &usb3_phy1;
>>      };
>> +
>> +Samsung Exynos SoC series PCIe PHY controller
>> +--------------------------------------------------
>> +Required properties:
>> +- compatible : Should be set to "samsung,exynos5440-pcie-phy"
>> +- #phy-cells : Must be zero
>> +- reg : a register used by phy driver.
>> +    - First is for phy register, second is for block register.
>> +- reg-names : Must be set to "phy" and "block".
>> +
> In general PHY uses a "reference clock" to work, if that is true for
> 5440 also, will you consider adding an (may be) optional clock
> properties as well?
> 

Yes, right, second clock, referred as "bus_clk" in pcie node should
actually refer to "phy" clock. From Exynos5433 DT patch also you are
mapping it to CLK_PCLK_PCIE_PHY which is a phy clk. This is same in
Exynos7 as well. So better we have clocks property defined in pcie-phy
binding. What do you say?

>From Exynos5440 UM, PCIe-Phy needs 250 MHz, clock and second clock used
as "bus_clk" is providing 250 MHz, so this can be moved in phy driver.


Thanks,
Pankaj Dubey

Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding

[pankaj.dubey]

Hi,

On Thursday 05 January 2017 09:46 AM, Alim Akhtar wrote:
> Hi Jaehoon,
> 
> On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
>> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
>> This is for using generic PHY framework.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>> Changelog on V2:
>> - Remove the child node.
>> - Add 2nd address to the parent reg prop.
>>
>>  Documentation/devicetree/bindings/phy/samsung-phy.txt | 17
>> +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> index 9872ba8..ab80bfe 100644
>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> @@ -191,3 +191,20 @@ Example:
>>          usbdrdphy0 = &usb3_phy0;
>>          usbdrdphy1 = &usb3_phy1;
>>      };
>> +
>> +Samsung Exynos SoC series PCIe PHY controller
>> +--------------------------------------------------
>> +Required properties:
>> +- compatible : Should be set to "samsung,exynos5440-pcie-phy"
>> +- #phy-cells : Must be zero
>> +- reg : a register used by phy driver.
>> +    - First is for phy register, second is for block register.
>> +- reg-names : Must be set to "phy" and "block".
>> +
> In general PHY uses a "reference clock" to work, if that is true for
> 5440 also, will you consider adding an (may be) optional clock
> properties as well?
> 

Yes, right, second clock, referred as "bus_clk" in pcie node should
actually refer to "phy" clock. From Exynos5433 DT patch also you are
mapping it to CLK_PCLK_PCIE_PHY which is a phy clk. This is same in
Exynos7 as well. So better we have clocks property defined in pcie-phy
binding. What do you say?

>From Exynos5440 UM, PCIe-Phy needs 250 MHz, clock and second clock used
as "bus_clk" is providing 250 MHz, so this can be moved in phy driver.


Thanks,
Pankaj Dubey
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

This patch supports to use Generic Phy framework for Exynos PCIe phy.
When Exynos that supported the pcie want to use the PCIe,
it needs to control the phy resgister.
But it should be more complex to control in their own PCIe device drivers.

Currently, there is an exynos5440 case to support the pcie.
So this driver is based on Exynos5440 PCIe.
In future, will support the Other exynos SoCs likes exynos5433, exynos7.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog on V2:
- Not include the codes relevant to pci-exynos.
- Remove the getting child node.

 drivers/phy/Kconfig           |   9 ++
 drivers/phy/Makefile          |   1 +
 drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 290 insertions(+)
 create mode 100644 drivers/phy/phy-exynos-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index e8eb7f2..2dddef4 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
 	  This driver provides PHY interface for USB 3.0 DRD controller
 	  present on Exynos5 SoC series.
 
+config PHY_EXYNOS_PCIE
+	bool "Exynos PCIe PHY driver"
+	depends on ARCH_EXYNOS && OF
+	depends on PCI_EXYNOS
+	select GENERIC_PHY
+	help
+	  Enable PCIe PHY support for Exynos SoC series.
+	  This driver provides PHY interface for Exynos PCIe controller.
+
 config PHY_PISTACHIO_USB
 	tristate "IMG Pistachio USB2.0 PHY driver"
 	depends on MACH_PISTACHIO
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 65eb2f4..081aeb4 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
 obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
+obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
 obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
new file mode 100644
index 0000000..b57f49b
--- /dev/null
+++ b/drivers/phy/phy-exynos-pcie.c
@@ -0,0 +1,280 @@
+/*
+ * Samsung EXYNOS SoC series PCIe PHY driver
+ *
+ * Phy provider for PCIe controller on Exynos SoC series
+ *
+ * Copyright (C) 2016 Samsung Electronics Co., Ltd.
+ * Jaehoon Chung <jh80.chung@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+
+/* PCIe Purple registers */
+#define PCIE_PHY_GLOBAL_RESET		0x000
+#define PCIE_PHY_COMMON_RESET		0x004
+#define PCIE_PHY_CMN_REG		0x008
+#define PCIE_PHY_MAC_RESET		0x00c
+#define PCIE_PHY_PLL_LOCKED		0x010
+#define PCIE_PHY_TRSVREG_RESET		0x020
+#define PCIE_PHY_TRSV_RESET		0x024
+
+/* PCIe PHY registers */
+#define PCIE_PHY_IMPEDANCE		0x004
+#define PCIE_PHY_PLL_DIV_0		0x008
+#define PCIE_PHY_PLL_BIAS		0x00c
+#define PCIE_PHY_DCC_FEEDBACK		0x014
+#define PCIE_PHY_PLL_DIV_1		0x05c
+#define PCIE_PHY_COMMON_POWER		0x064
+#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
+#define PCIE_PHY_TRSV0_EMP_LVL		0x084
+#define PCIE_PHY_TRSV0_DRV_LVL		0x088
+#define PCIE_PHY_TRSV0_RXCDR		0x0ac
+#define PCIE_PHY_TRSV0_POWER		0x0c4
+#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
+#define PCIE_PHY_TRSV0_LVCC		0x0dc
+#define PCIE_PHY_TRSV1_EMP_LVL		0x144
+#define PCIE_PHY_TRSV1_RXCDR		0x16c
+#define PCIE_PHY_TRSV1_POWER		0x184
+#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
+#define PCIE_PHY_TRSV1_LVCC		0x19c
+#define PCIE_PHY_TRSV2_EMP_LVL		0x204
+#define PCIE_PHY_TRSV2_RXCDR		0x22c
+#define PCIE_PHY_TRSV2_POWER		0x244
+#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
+#define PCIE_PHY_TRSV2_LVCC		0x25c
+#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
+#define PCIE_PHY_TRSV3_RXCDR		0x2ec
+#define PCIE_PHY_TRSV3_POWER		0x304
+#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
+#define PCIE_PHY_TRSV3_LVCC		0x31c
+
+struct exynos_pcie_phy_data {
+	struct phy_ops	*ops;
+};
+
+/* For Exynos pcie phy */
+struct exynos_pcie_phy {
+	const struct exynos_pcie_phy_data *drv_data;
+	void __iomem *phy_base;
+	void __iomem *blk_base; /* For exynos5440 */
+};
+
+static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
+{
+	writel(val, base + offset);
+}
+
+static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
+{
+	return readl(base + offset);
+}
+
+/* For Exynos5440 specific functions */
+static int exynos5440_pcie_phy_init(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+
+	/* DCC feedback control off */
+	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
+
+	/* set TX/RX impedance */
+	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
+
+	/* set 50Mhz PHY clock */
+	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
+	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
+
+	/* set TX Differential output for lane 0 */
+	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
+
+	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
+	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
+
+	/* set RX clock and data recovery bandwidth */
+	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
+	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
+	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
+	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
+	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
+
+	/* change TX Pre-emphasis Level Control for lanes */
+	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
+	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
+	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
+	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
+
+	/* set LVCC */
+	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
+	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
+	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
+	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
+
+	/* pulse for common reset */
+	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
+	udelay(500);
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
+
+	return 0;
+}
+
+static int exynos5440_pcie_phy_power_on(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+	u32 val;
+
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
+	val &= ~PCIE_PHY_COMMON_PD_CMN;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
+	val &= ~PCIE_PHY_TRSV0_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
+	val &= ~PCIE_PHY_TRSV1_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
+	val &= ~PCIE_PHY_TRSV2_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
+	val &= ~PCIE_PHY_TRSV3_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
+
+	return 0;
+}
+
+static int exynos5440_pcie_phy_power_off(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+	u32 val;
+
+	while (exynos_pcie_phy_readl(ep->phy_base,
+				PCIE_PHY_PLL_LOCKED) == 0) {
+		val = exynos_pcie_phy_readl(ep->blk_base,
+				PCIE_PHY_PLL_LOCKED);
+		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
+	}
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
+	val |= PCIE_PHY_COMMON_PD_CMN;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
+	val |= PCIE_PHY_TRSV0_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
+	val |= PCIE_PHY_TRSV1_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
+	val |= PCIE_PHY_TRSV2_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
+
+	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
+	val |= PCIE_PHY_TRSV3_PD_TSV;
+	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
+
+	return 0;
+}
+
+static int exynos5440_pcie_phy_reset(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
+	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
+	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
+
+	return 0;
+}
+
+static struct phy_ops exynos5440_phy_ops = {
+	.init	= exynos5440_pcie_phy_init,
+	.power_on = exynos5440_pcie_phy_power_on,
+	.power_off = exynos5440_pcie_phy_power_off,
+	.reset	= exynos5440_pcie_phy_reset,
+};
+
+static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
+	.ops		= &exynos5440_phy_ops,
+};
+
+static const struct of_device_id exynos_pcie_phy_match[] = {
+	{
+		.compatible = "samsung,exynos5440-pcie-phy",
+		.data = &exynos5440_pcie_phy_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
+
+static int exynos_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct exynos_pcie_phy *exynos_phy;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	const struct exynos_pcie_phy_data *drv_data;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -ENODEV;
+
+	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
+	if (!exynos_phy)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(exynos_phy->phy_base))
+		return PTR_ERR(exynos_phy->phy_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(exynos_phy->phy_base))
+		return PTR_ERR(exynos_phy->phy_base);
+
+	exynos_phy->drv_data = drv_data;
+
+	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
+	if (IS_ERR(generic_phy)) {
+		dev_err(dev, "failed to create PHY\n");
+		return PTR_ERR(generic_phy);
+	}
+
+	phy_set_drvdata(generic_phy, exynos_phy);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static struct platform_driver exynos_pcie_phy_driver = {
+	.probe	= exynos_pcie_phy_probe,
+	.driver = {
+		.of_match_table	= exynos_pcie_phy_match,
+		.name		= "exynos_pcie_phy",
+	}
+};
+module_platform_driver(exynos_pcie_phy_driver);
-- 
2.10.2

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Krzysztof Kozlowski]

On Wed, Jan 04, 2017 at 09:34:32PM +0900, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.

I have troubles understanding this. Please, work on the commit message.

For the code itself:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
... but commit message is really important to understand why/what was
done.

Best regards,
Krzysztof

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

On 01/05/2017 02:52 AM, Krzysztof Kozlowski wrote:
> On Wed, Jan 04, 2017 at 09:34:32PM +0900, Jaehoon Chung wrote:
>> This patch supports to use Generic Phy framework for Exynos PCIe phy.
>> When Exynos that supported the pcie want to use the PCIe,
>> it needs to control the phy resgister.
>> But it should be more complex to control in their own PCIe device drivers.
>>
>> Currently, there is an exynos5440 case to support the pcie.
>> So this driver is based on Exynos5440 PCIe.
>> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
> 
> I have troubles understanding this. Please, work on the commit message.
> 
> For the code itself:
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> ... but commit message is really important to understand why/what was
> done.

Will update the commit-msg. Thanks for comments.

Best Regards,
Jaehoon Chung

> 
> Best regards,
> Krzysztof
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

On Wednesday, January 4, 2017 7:35 AM, Jaehoon Chung wrote:
> 
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

Reviewed-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280
> ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Jingoo Han]

On Wednesday, January 4, 2017 7:35 AM, Jaehoon Chung wrote:
> 
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

Reviewed-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280
> ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[pankaj.dubey]

Hi Jaehoon,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 

Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>

Thanks,
Pankaj Dubey

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Alim Akhtar]

Hi,

On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
>
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
>
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>  	  This driver provides PHY interface for USB 3.0 DRD controller
>  	  present on Exynos5 SoC series.
>
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF

Please add a depends on COMPILE_TEST as well.
Apart from this looks ok.


> +	depends on PCI_EXYNOS
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>  config PHY_PISTACHIO_USB
>  	tristate "IMG Pistachio USB2.0 PHY driver"
>  	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
> + * Jaehoon Chung <jh80.chung@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {
> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {
> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);
>

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Vivek Gautam]

Hi Jaehoon,


On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
>
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
>
>   drivers/phy/Kconfig           |   9 ++
>   drivers/phy/Makefile          |   1 +
>   drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 290 insertions(+)
>   create mode 100644 drivers/phy/phy-exynos-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>   	  This driver provides PHY interface for USB 3.0 DRD controller
>   	  present on Exynos5 SoC series.
>   
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF
> +	depends on PCI_EXYNOS
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>   config PHY_PISTACHIO_USB
>   	tristate "IMG Pistachio USB2.0 PHY driver"
>   	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>   phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>   phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>   obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>   obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>   obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>   obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
> + * Jaehoon Chung <jh80.chung@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {

No timeout for this ? Is it possible that the PLL was never locked
and this ends up in infinite loop.

Please use a readl_poll_timeout() instead.

> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
it is possible that the while condition check above is true (the 
register reads to 0),
but this assignment makes ' val = 1' ?
If that the case then debug message below can be misleading.

> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);

Possibly, you don't want to fill up the console with these logs until 
the PLL is locked. dev_dbg() ?

> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {

const ?

> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);

MODULE_LICENSE("GPL") ??
MODULE_DESCRIPTION() as well.


Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Vivek Gautam]

Hi Jaehoon,


On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
>
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
>
>   drivers/phy/Kconfig           |   9 ++
>   drivers/phy/Makefile          |   1 +
>   drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 290 insertions(+)
>   create mode 100644 drivers/phy/phy-exynos-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>   	  This driver provides PHY interface for USB 3.0 DRD controller
>   	  present on Exynos5 SoC series.
>   
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF
> +	depends on PCI_EXYNOS
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>   config PHY_PISTACHIO_USB
>   	tristate "IMG Pistachio USB2.0 PHY driver"
>   	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>   phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>   phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>   obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>   obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>   obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>   obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
> + * Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {

No timeout for this ? Is it possible that the PLL was never locked
and this ends up in infinite loop.

Please use a readl_poll_timeout() instead.

> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
it is possible that the while condition check above is true (the 
register reads to 0),
but this assignment makes ' val = 1' ?
If that the case then debug message below can be misleading.

> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);

Possibly, you don't want to fill up the console with these logs until 
the PLL is locked. dev_dbg() ?

> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {

const ?

> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);

MODULE_LICENSE("GPL") ??
MODULE_DESCRIPTION() as well.


Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

Hi Vivek,

On 01/10/2017 03:07 PM, Vivek Gautam wrote:
> Hi Jaehoon,
> 
> 
> On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
>> This patch supports to use Generic Phy framework for Exynos PCIe phy.
>> When Exynos that supported the pcie want to use the PCIe,
>> it needs to control the phy resgister.
>> But it should be more complex to control in their own PCIe device drivers.
>>
>> Currently, there is an exynos5440 case to support the pcie.
>> So this driver is based on Exynos5440 PCIe.
>> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - Not include the codes relevant to pci-exynos.
>> - Remove the getting child node.
>>
>>   drivers/phy/Kconfig           |   9 ++
>>   drivers/phy/Makefile          |   1 +
>>   drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 290 insertions(+)
>>   create mode 100644 drivers/phy/phy-exynos-pcie.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f2..2dddef4 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>>         This driver provides PHY interface for USB 3.0 DRD controller
>>         present on Exynos5 SoC series.
>>   +config PHY_EXYNOS_PCIE
>> +    bool "Exynos PCIe PHY driver"
>> +    depends on ARCH_EXYNOS && OF
>> +    depends on PCI_EXYNOS
>> +    select GENERIC_PHY
>> +    help
>> +      Enable PCIe PHY support for Exynos SoC series.
>> +      This driver provides PHY interface for Exynos PCIe controller.
>> +
>>   config PHY_PISTACHIO_USB
>>       tristate "IMG Pistachio USB2.0 PHY driver"
>>       depends on MACH_PISTACHIO
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 65eb2f4..081aeb4 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)    += phy-exynos4x12-usb2.o
>>   phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)    += phy-exynos5250-usb2.o
>>   phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)    += phy-s5pv210-usb2.o
>>   obj-$(CONFIG_PHY_EXYNOS5_USBDRD)    += phy-exynos5-usbdrd.o
>> +obj-$(CONFIG_PHY_EXYNOS_PCIE)    += phy-exynos-pcie.o
>>   obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)    += phy-qcom-apq8064-sata.o
>>   obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>>   obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)    += phy-rockchip-inno-usb2.o
>> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
>> new file mode 100644
>> index 0000000..b57f49b
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos-pcie.c
>> @@ -0,0 +1,280 @@
>> +/*
>> + * Samsung EXYNOS SoC series PCIe PHY driver
>> + *
>> + * Phy provider for PCIe controller on Exynos SoC series
>> + *
>> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
>> + * Jaehoon Chung <jh80.chung@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/regmap.h>
>> +
>> +/* PCIe Purple registers */
>> +#define PCIE_PHY_GLOBAL_RESET        0x000
>> +#define PCIE_PHY_COMMON_RESET        0x004
>> +#define PCIE_PHY_CMN_REG        0x008
>> +#define PCIE_PHY_MAC_RESET        0x00c
>> +#define PCIE_PHY_PLL_LOCKED        0x010
>> +#define PCIE_PHY_TRSVREG_RESET        0x020
>> +#define PCIE_PHY_TRSV_RESET        0x024
>> +
>> +/* PCIe PHY registers */
>> +#define PCIE_PHY_IMPEDANCE        0x004
>> +#define PCIE_PHY_PLL_DIV_0        0x008
>> +#define PCIE_PHY_PLL_BIAS        0x00c
>> +#define PCIE_PHY_DCC_FEEDBACK        0x014
>> +#define PCIE_PHY_PLL_DIV_1        0x05c
>> +#define PCIE_PHY_COMMON_POWER        0x064
>> +#define PCIE_PHY_COMMON_PD_CMN        BIT(3)
>> +#define PCIE_PHY_TRSV0_EMP_LVL        0x084
>> +#define PCIE_PHY_TRSV0_DRV_LVL        0x088
>> +#define PCIE_PHY_TRSV0_RXCDR        0x0ac
>> +#define PCIE_PHY_TRSV0_POWER        0x0c4
>> +#define PCIE_PHY_TRSV0_PD_TSV        BIT(7)
>> +#define PCIE_PHY_TRSV0_LVCC        0x0dc
>> +#define PCIE_PHY_TRSV1_EMP_LVL        0x144
>> +#define PCIE_PHY_TRSV1_RXCDR        0x16c
>> +#define PCIE_PHY_TRSV1_POWER        0x184
>> +#define PCIE_PHY_TRSV1_PD_TSV        BIT(7)
>> +#define PCIE_PHY_TRSV1_LVCC        0x19c
>> +#define PCIE_PHY_TRSV2_EMP_LVL        0x204
>> +#define PCIE_PHY_TRSV2_RXCDR        0x22c
>> +#define PCIE_PHY_TRSV2_POWER        0x244
>> +#define PCIE_PHY_TRSV2_PD_TSV        BIT(7)
>> +#define PCIE_PHY_TRSV2_LVCC        0x25c
>> +#define PCIE_PHY_TRSV3_EMP_LVL        0x2c4
>> +#define PCIE_PHY_TRSV3_RXCDR        0x2ec
>> +#define PCIE_PHY_TRSV3_POWER        0x304
>> +#define PCIE_PHY_TRSV3_PD_TSV        BIT(7)
>> +#define PCIE_PHY_TRSV3_LVCC        0x31c
>> +
>> +struct exynos_pcie_phy_data {
>> +    struct phy_ops    *ops;
>> +};
>> +
>> +/* For Exynos pcie phy */
>> +struct exynos_pcie_phy {
>> +    const struct exynos_pcie_phy_data *drv_data;
>> +    void __iomem *phy_base;
>> +    void __iomem *blk_base; /* For exynos5440 */
>> +};
>> +
>> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
>> +{
>> +    writel(val, base + offset);
>> +}
>> +
>> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
>> +{
>> +    return readl(base + offset);
>> +}
>> +
>> +/* For Exynos5440 specific functions */
>> +static int exynos5440_pcie_phy_init(struct phy *phy)
>> +{
>> +    struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +    /* DCC feedback control off */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
>> +
>> +    /* set TX/RX impedance */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
>> +
>> +    /* set 50Mhz PHY clock */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
>> +
>> +    /* set TX Differential output for lane 0 */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
>> +
>> +    /* set TX Pre-emphasis Level Control for lane 0 to minimum */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
>> +
>> +    /* set RX clock and data recovery bandwidth */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
>> +
>> +    /* change TX Pre-emphasis Level Control for lanes */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
>> +
>> +    /* set LVCC */
>> +    exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
>> +    exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
>> +
>> +    /* pulse for common reset */
>> +    exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
>> +    udelay(500);
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> +
>> +    return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
>> +{
>> +    struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +    u32 val;
>> +
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> +    val &= ~PCIE_PHY_COMMON_PD_CMN;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> +    val &= ~PCIE_PHY_TRSV0_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> +    val &= ~PCIE_PHY_TRSV1_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> +    val &= ~PCIE_PHY_TRSV2_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> +    val &= ~PCIE_PHY_TRSV3_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> +    return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
>> +{
>> +    struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +    u32 val;
>> +
>> +    while (exynos_pcie_phy_readl(ep->phy_base,
>> +                PCIE_PHY_PLL_LOCKED) == 0) {
> 
> No timeout for this ? Is it possible that the PLL was never locked
> and this ends up in infinite loop.
> 
> Please use a readl_poll_timeout() instead.

Ok, will fix.

> 
>> +        val = exynos_pcie_phy_readl(ep->blk_base,
>> +                PCIE_PHY_PLL_LOCKED);
> it is possible that the while condition check above is true (the register reads to 0),
> but this assignment makes ' val = 1' ?
> If that the case then debug message below can be misleading.
> 
>> +        dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
> 
> Possibly, you don't want to fill up the console with these logs until the PLL is locked. dev_dbg() ?

Ok, will fix.

> 
>> +    }
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> +    val |= PCIE_PHY_COMMON_PD_CMN;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> +    val |= PCIE_PHY_TRSV0_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> +    val |= PCIE_PHY_TRSV1_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> +    val |= PCIE_PHY_TRSV2_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> +    val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> +    val |= PCIE_PHY_TRSV3_PD_TSV;
>> +    exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> +    return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_reset(struct phy *phy)
>> +{
>> +    struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
>> +    exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
>> +    exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
>> +
>> +    return 0;
>> +}
>> +
>> +static struct phy_ops exynos5440_phy_ops = {
> 
> const ?

Right.

> 
>> +    .init    = exynos5440_pcie_phy_init,
>> +    .power_on = exynos5440_pcie_phy_power_on,
>> +    .power_off = exynos5440_pcie_phy_power_off,
>> +    .reset    = exynos5440_pcie_phy_reset,
>> +};
>> +
>> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
>> +    .ops        = &exynos5440_phy_ops,
>> +};
>> +
>> +static const struct of_device_id exynos_pcie_phy_match[] = {
>> +    {
>> +        .compatible = "samsung,exynos5440-pcie-phy",
>> +        .data = &exynos5440_pcie_phy_data,
>> +    },
>> +    {},
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
>> +
>> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +    struct device *dev = &pdev->dev;
>> +    struct exynos_pcie_phy *exynos_phy;
>> +    struct phy *generic_phy;
>> +    struct phy_provider *phy_provider;
>> +    struct resource *res;
>> +    const struct exynos_pcie_phy_data *drv_data;
>> +
>> +    drv_data = of_device_get_match_data(dev);
>> +    if (!drv_data)
>> +        return -ENODEV;
>> +
>> +    exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
>> +    if (!exynos_phy)
>> +        return -ENOMEM;
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    exynos_phy->phy_base = devm_ioremap_resource(dev, res);
>> +    if (IS_ERR(exynos_phy->phy_base))
>> +        return PTR_ERR(exynos_phy->phy_base);
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +    exynos_phy->blk_base = devm_ioremap_resource(dev, res);
>> +    if (IS_ERR(exynos_phy->phy_base))
>> +        return PTR_ERR(exynos_phy->phy_base);
>> +
>> +    exynos_phy->drv_data = drv_data;
>> +
>> +    generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
>> +    if (IS_ERR(generic_phy)) {
>> +        dev_err(dev, "failed to create PHY\n");
>> +        return PTR_ERR(generic_phy);
>> +    }
>> +
>> +    phy_set_drvdata(generic_phy, exynos_phy);
>> +    phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +    return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static struct platform_driver exynos_pcie_phy_driver = {
>> +    .probe    = exynos_pcie_phy_probe,
>> +    .driver = {
>> +        .of_match_table    = exynos_pcie_phy_match,
>> +        .name        = "exynos_pcie_phy",
>> +    }
>> +};
>> +module_platform_driver(exynos_pcie_phy_driver);
> 
> MODULE_LICENSE("GPL") ??
> MODULE_DESCRIPTION() as well.

Will do.

I will send the Patch V3 within this week.

Best Regards,
Jaehoon Chung

> 
> 
> Regards
> Vivek
> 

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Kishon Vijay Abraham I]

Hi,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.

please re-write the commit message.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>  	  This driver provides PHY interface for USB 3.0 DRD controller
>  	  present on Exynos5 SoC series.
>  
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF

include COMPILE_TEST
> +	depends on PCI_EXYNOS

PCI_EXYNOS should depend on PHY_EXYNOS_PCIE if at all required. Or else do away
with this dependency.
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>  config PHY_PISTACHIO_USB
>  	tristate "IMG Pistachio USB2.0 PHY driver"
>  	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.

2017?
> + * Jaehoon Chung <jh80.chung@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024

Please use BIT() macro for bit definitions.
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);

I'm starting to dis-like all this hard-coded hw params. All this should come
from dt. Define a dt binding like this for all hw params..
	phy,tx-differential = <val, reg-offset, mask>

and have one API in phy-core to do all these settings.
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);

how did you get this delay value? Adding a comment might help.
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {
> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {
> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,

add .owner
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,

why do you need a wrapper for phy_ops?
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);
> 
Thanks
Kishon

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Kishon Vijay Abraham I]

Hi,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.

please re-write the commit message.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>  	  This driver provides PHY interface for USB 3.0 DRD controller
>  	  present on Exynos5 SoC series.
>  
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF

include COMPILE_TEST
> +	depends on PCI_EXYNOS

PCI_EXYNOS should depend on PHY_EXYNOS_PCIE if at all required. Or else do away
with this dependency.
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>  config PHY_PISTACHIO_USB
>  	tristate "IMG Pistachio USB2.0 PHY driver"
>  	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.

2017?
> + * Jaehoon Chung <jh80.chung@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024

Please use BIT() macro for bit definitions.
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);

I'm starting to dis-like all this hard-coded hw params. All this should come
from dt. Define a dt binding like this for all hw params..
	phy,tx-differential = <val, reg-offset, mask>

and have one API in phy-core to do all these settings.
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);

how did you get this delay value? Adding a comment might help.
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {
> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {
> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,

add .owner
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,

why do you need a wrapper for phy_ops?
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);
> 
Thanks
Kishon

Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

Hi,

On 01/16/2017 05:37 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
>> This patch supports to use Generic Phy framework for Exynos PCIe phy.
>> When Exynos that supported the pcie want to use the PCIe,
>> it needs to control the phy resgister.
>> But it should be more complex to control in their own PCIe device drivers.
>>
>> Currently, there is an exynos5440 case to support the pcie.
>> So this driver is based on Exynos5440 PCIe.
>> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
> 
> please re-write the commit message.

Will update the commit-message

>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - Not include the codes relevant to pci-exynos.
>> - Remove the getting child node.
>>
>>  drivers/phy/Kconfig           |   9 ++
>>  drivers/phy/Makefile          |   1 +
>>  drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 290 insertions(+)
>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f2..2dddef4 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>>  	  This driver provides PHY interface for USB 3.0 DRD controller
>>  	  present on Exynos5 SoC series.
>>  
>> +config PHY_EXYNOS_PCIE
>> +	bool "Exynos PCIe PHY driver"
>> +	depends on ARCH_EXYNOS && OF
> 
> include COMPILE_TEST

Ok.

>> +	depends on PCI_EXYNOS
> 
> PCI_EXYNOS should depend on PHY_EXYNOS_PCIE if at all required. Or else do away
> with this dependency.

Ok.

>> +	select GENERIC_PHY
>> +	help
>> +	  Enable PCIe PHY support for Exynos SoC series.
>> +	  This driver provides PHY interface for Exynos PCIe controller.
>> +
>>  config PHY_PISTACHIO_USB
>>  	tristate "IMG Pistachio USB2.0 PHY driver"
>>  	depends on MACH_PISTACHIO
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 65eb2f4..081aeb4 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
>> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
>> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
>> new file mode 100644
>> index 0000000..b57f49b
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos-pcie.c
>> @@ -0,0 +1,280 @@
>> +/*
>> + * Samsung EXYNOS SoC series PCIe PHY driver
>> + *
>> + * Phy provider for PCIe controller on Exynos SoC series
>> + *
>> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
> 
> 2017?

When i had posted the first version, it was 2016.. :)

>> + * Jaehoon Chung <jh80.chung@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/regmap.h>
>> +
>> +/* PCIe Purple registers */
>> +#define PCIE_PHY_GLOBAL_RESET		0x000
>> +#define PCIE_PHY_COMMON_RESET		0x004
>> +#define PCIE_PHY_CMN_REG		0x008
>> +#define PCIE_PHY_MAC_RESET		0x00c
>> +#define PCIE_PHY_PLL_LOCKED		0x010
>> +#define PCIE_PHY_TRSVREG_RESET		0x020
>> +#define PCIE_PHY_TRSV_RESET		0x024
> 
> Please use BIT() macro for bit definitions.

Ok.

>> +
>> +/* PCIe PHY registers */
>> +#define PCIE_PHY_IMPEDANCE		0x004
>> +#define PCIE_PHY_PLL_DIV_0		0x008
>> +#define PCIE_PHY_PLL_BIAS		0x00c
>> +#define PCIE_PHY_DCC_FEEDBACK		0x014
>> +#define PCIE_PHY_PLL_DIV_1		0x05c
>> +#define PCIE_PHY_COMMON_POWER		0x064
>> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
>> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
>> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
>> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
>> +#define PCIE_PHY_TRSV0_POWER		0x0c4
>> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
>> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
>> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
>> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
>> +#define PCIE_PHY_TRSV1_POWER		0x184
>> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
>> +#define PCIE_PHY_TRSV1_LVCC		0x19c
>> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
>> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
>> +#define PCIE_PHY_TRSV2_POWER		0x244
>> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
>> +#define PCIE_PHY_TRSV2_LVCC		0x25c
>> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
>> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
>> +#define PCIE_PHY_TRSV3_POWER		0x304
>> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>> +#define PCIE_PHY_TRSV3_LVCC		0x31c
>> +
>> +struct exynos_pcie_phy_data {
>> +	struct phy_ops	*ops;
>> +};
>> +
>> +/* For Exynos pcie phy */
>> +struct exynos_pcie_phy {
>> +	const struct exynos_pcie_phy_data *drv_data;
>> +	void __iomem *phy_base;
>> +	void __iomem *blk_base; /* For exynos5440 */
>> +};
>> +
>> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
>> +{
>> +	writel(val, base + offset);
>> +}
>> +
>> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
>> +{
>> +	return readl(base + offset);
>> +}
>> +
>> +/* For Exynos5440 specific functions */
>> +static int exynos5440_pcie_phy_init(struct phy *phy)
>> +{
>> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +	/* DCC feedback control off */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
>> +
>> +	/* set TX/RX impedance */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
>> +
>> +	/* set 50Mhz PHY clock */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
>> +
>> +	/* set TX Differential output for lane 0 */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
>> +
>> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
>> +
>> +	/* set RX clock and data recovery bandwidth */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
>> +
>> +	/* change TX Pre-emphasis Level Control for lanes */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
>> +
>> +	/* set LVCC */
>> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
>> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
> 
> I'm starting to dis-like all this hard-coded hw params. All this should come
> from dt. Define a dt binding like this for all hw params..
> 	phy,tx-differential = <val, reg-offset, mask>
> 
> and have one API in phy-core to do all these settings.

Will check.

>> +
>> +	/* pulse for common reset */
>> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
>> +	udelay(500);
> 
> how did you get this delay value? Adding a comment might help.

Well..Actually i don't know why udelay(500) was defined.
When i had started the refactoring pci-exynos, there was one problem..
I didn't have Exynos5440 TRM and information..i can't check anything for exyno5440.
(I don't know who is using EXYNOS5440 pci.)

the entire codes are just moved from pci-exynos.c.

When i want to upstream exynos5433 pcie, first step is the cleaning pci-exynos.c relevant to exynos5440.
It was too complex to support the other Exynos SoCs.

Otherwise, it needs to add the new file for exynos5433.

>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
>> +{
>> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +	u32 val;
>> +
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
>> +{
>> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +	u32 val;
>> +
>> +	while (exynos_pcie_phy_readl(ep->phy_base,
>> +				PCIE_PHY_PLL_LOCKED) == 0) {
>> +		val = exynos_pcie_phy_readl(ep->blk_base,
>> +				PCIE_PHY_PLL_LOCKED);
>> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
>> +	}
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> +	val |= PCIE_PHY_COMMON_PD_CMN;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> +	val |= PCIE_PHY_TRSV0_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> +	val |= PCIE_PHY_TRSV1_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> +	val |= PCIE_PHY_TRSV2_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> +	val |= PCIE_PHY_TRSV3_PD_TSV;
>> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_reset(struct phy *phy)
>> +{
>> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
>> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
>> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct phy_ops exynos5440_phy_ops = {
>> +	.init	= exynos5440_pcie_phy_init,
>> +	.power_on = exynos5440_pcie_phy_power_on,
>> +	.power_off = exynos5440_pcie_phy_power_off,
>> +	.reset	= exynos5440_pcie_phy_reset,
> 
> add .owner

Will fix.

>> +};
>> +
>> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
>> +	.ops		= &exynos5440_phy_ops,
> 
> why do you need a wrapper for phy_ops?

My main goal is the upstreaming exynos5433 pci, not exynos5440.
There are many different with exynos5440 and exynos5433.
As i know, only exynos5440 has the big different with other Exynos variants.

I think i can be removed in this patch. When i upstream the other SoCs, I will use the wrapper.

Best Regards,
Jaehoon Chung

>> +};
>> +
>> +static const struct of_device_id exynos_pcie_phy_match[] = {
>> +	{
>> +		.compatible = "samsung,exynos5440-pcie-phy",
>> +		.data = &exynos5440_pcie_phy_data,
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
>> +
>> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct exynos_pcie_phy *exynos_phy;
>> +	struct phy *generic_phy;
>> +	struct phy_provider *phy_provider;
>> +	struct resource *res;
>> +	const struct exynos_pcie_phy_data *drv_data;
>> +
>> +	drv_data = of_device_get_match_data(dev);
>> +	if (!drv_data)
>> +		return -ENODEV;
>> +
>> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
>> +	if (!exynos_phy)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(exynos_phy->phy_base))
>> +		return PTR_ERR(exynos_phy->phy_base);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(exynos_phy->phy_base))
>> +		return PTR_ERR(exynos_phy->phy_base);
>> +
>> +	exynos_phy->drv_data = drv_data;
>> +
>> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
>> +	if (IS_ERR(generic_phy)) {
>> +		dev_err(dev, "failed to create PHY\n");
>> +		return PTR_ERR(generic_phy);
>> +	}
>> +
>> +	phy_set_drvdata(generic_phy, exynos_phy);
>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +	return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static struct platform_driver exynos_pcie_phy_driver = {
>> +	.probe	= exynos_pcie_phy_probe,
>> +	.driver = {
>> +		.of_match_table	= exynos_pcie_phy_match,
>> +		.name		= "exynos_pcie_phy",
>> +	}
>> +};
>> +module_platform_driver(exynos_pcie_phy_driver);
>>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

[PATCH V2 3/5] Documetation: binding: modify the exynos5440 pcie binding

[Jaehoon Chung]

According to using PHY framework, updates the exynos5440-pcie binding.
For maintaining backward compatibility, leaves the current dt-binding.
(It should be deprecated.)

Recommends to use the Phy Framework and "config" property to follow
the designware-pcie binding.
If you use the old way, can see "mssing *config* reg space" message.
Because the getting configuration space address from range is old way.

NOTE: When use the "config" property, first name of 'reg-names' must be
set to "elbi". Otherwise driver can't maintain the backward capability.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog on V2:
- Describes more commit message
- Fixes the typos
- Adds the new example for using PHY framework
- Deprecated the old dt-binding description
- Removes 'phy-names'

 .../bindings/pci/samsung,exynos5440-pcie.txt       | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
index 4f9d23d..1d0af0e 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
@@ -7,8 +7,19 @@ Required properties:
 - compatible: "samsung,exynos5440-pcie"
 - reg: base addresses and lengths of the pcie controller,
 	the phy controller, additional register for the phy controller.
+	(Registers for the phy controller are DEPRECATED.
+	 Use the PHY framework.)
+- reg-names : First name should be set to "elbi".
+	And use the "config" instead of getting the confgiruation address space
+	from "ranges".
+	NOTE: When use the "config" property, reg-names must be set.
 - interrupts: A list of interrupt outputs for level interrupt,
 	pulse interrupt, special interrupt.
+- phys: From PHY binding. Phandle for the Generic PHY.
+	Refer to Documentation/devicetree/bindings/phy/samsung-phy.txt
+
+Other common properties refer to
+	Documentation/devicetree/binding/pci/designware-pcie.txt
 
 Example:
 
@@ -54,6 +65,24 @@ SoC specific DT Entry:
 		num-lanes = <4>;
 	};
 
+With using PHY framework:
+	pcie_phy0: pcie-phy@270000 {
+		...
+		reg = <0x270000 0x1000>, <0x271000 0x40>;
+		regn-names = "phy", "block";
+		...
+	};
+
+	pcie@290000 {
+		...
+		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
+		reg-names = "elbi", "config";
+		phys = <&pcie_phy0>;
+		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
+			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
+		...
+	};
+
 Board specific DT Entry:
 
 	pcie@290000 {
-- 
2.10.2

Re: [PATCH V2 3/5] Documetation: binding: modify the exynos5440 pcie binding

[Rob Herring]

On Wed, Jan 04, 2017 at 09:34:33PM +0900, Jaehoon Chung wrote:
> According to using PHY framework, updates the exynos5440-pcie binding.
> For maintaining backward compatibility, leaves the current dt-binding.
> (It should be deprecated.)
> 
> Recommends to use the Phy Framework and "config" property to follow
> the designware-pcie binding.
> If you use the old way, can see "mssing *config* reg space" message.
> Because the getting configuration space address from range is old way.
> 
> NOTE: When use the "config" property, first name of 'reg-names' must be
> set to "elbi". Otherwise driver can't maintain the backward capability.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Describes more commit message
> - Fixes the typos
> - Adds the new example for using PHY framework
> - Deprecated the old dt-binding description
> - Removes 'phy-names'
> 
>  .../bindings/pci/samsung,exynos5440-pcie.txt       | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)

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

Re: [PATCH V2 3/5] Documetation: binding: modify the exynos5440 pcie binding

[pankaj.dubey]

Hi Jaehoon,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> According to using PHY framework, updates the exynos5440-pcie binding.
> For maintaining backward compatibility, leaves the current dt-binding.
> (It should be deprecated.)
> 
> Recommends to use the Phy Framework and "config" property to follow
> the designware-pcie binding.
> If you use the old way, can see "mssing *config* reg space" message.
> Because the getting configuration space address from range is old way.
> 
> NOTE: When use the "config" property, first name of 'reg-names' must be
> set to "elbi". Otherwise driver can't maintain the backward capability.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Describes more commit message
> - Fixes the typos
> - Adds the new example for using PHY framework
> - Deprecated the old dt-binding description
> - Removes 'phy-names'
> 
>  .../bindings/pci/samsung,exynos5440-pcie.txt       | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> index 4f9d23d..1d0af0e 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> @@ -7,8 +7,19 @@ Required properties:
>  - compatible: "samsung,exynos5440-pcie"
>  - reg: base addresses and lengths of the pcie controller,
>  	the phy controller, additional register for the phy controller.
> +	(Registers for the phy controller are DEPRECATED.
> +	 Use the PHY framework.)
> +- reg-names : First name should be set to "elbi".
> +	And use the "config" instead of getting the confgiruation address space
> +	from "ranges".
> +	NOTE: When use the "config" property, reg-names must be set.
>  - interrupts: A list of interrupt outputs for level interrupt,
>  	pulse interrupt, special interrupt.
> +- phys: From PHY binding. Phandle for the Generic PHY.
> +	Refer to Documentation/devicetree/bindings/phy/samsung-phy.txt
> +
> +Other common properties refer to
> +	Documentation/devicetree/binding/pci/designware-pcie.txt
>  
>  Example:
>  
> @@ -54,6 +65,24 @@ SoC specific DT Entry:
>  		num-lanes = <4>;
>  	};
>  
> +With using PHY framework:
> +	pcie_phy0: pcie-phy@270000 {
> +		...
> +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> +		regn-names = "phy", "block";

typo: %s/regn-names/reg-names/

> +		...
> +	};
> +
> +	pcie@290000 {
> +		...
> +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
> +		reg-names = "elbi", "config";
> +		phys = <&pcie_phy0>;
> +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
> +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
> +		...
> +	};
> +
>  Board specific DT Entry:
>  
>  	pcie@290000 {
> 

Other than above one typo rest is looking fine.
Once you address it, feel free to add

Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>

Thanks,
Pankaj Dubey

Re: [PATCH V2 3/5] Documetation: binding: modify the exynos5440 pcie binding

[Alim Akhtar]

Hi Jaehoon,

On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> According to using PHY framework, updates the exynos5440-pcie binding.
> For maintaining backward compatibility, leaves the current dt-binding.
> (It should be deprecated.)
>
> Recommends to use the Phy Framework and "config" property to follow
> the designware-pcie binding.
> If you use the old way, can see "mssing *config* reg space" message.
> Because the getting configuration space address from range is old way.
>
> NOTE: When use the "config" property, first name of 'reg-names' must be
> set to "elbi". Otherwise driver can't maintain the backward capability.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

> Changelog on V2:
> - Describes more commit message
> - Fixes the typos
> - Adds the new example for using PHY framework
> - Deprecated the old dt-binding description
> - Removes 'phy-names'
>
>  .../bindings/pci/samsung,exynos5440-pcie.txt       | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> index 4f9d23d..1d0af0e 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> @@ -7,8 +7,19 @@ Required properties:
>  - compatible: "samsung,exynos5440-pcie"
>  - reg: base addresses and lengths of the pcie controller,
>  	the phy controller, additional register for the phy controller.
> +	(Registers for the phy controller are DEPRECATED.
> +	 Use the PHY framework.)
> +- reg-names : First name should be set to "elbi".
> +	And use the "config" instead of getting the confgiruation address space
> +	from "ranges".
> +	NOTE: When use the "config" property, reg-names must be set.
>  - interrupts: A list of interrupt outputs for level interrupt,
>  	pulse interrupt, special interrupt.
> +- phys: From PHY binding. Phandle for the Generic PHY.
> +	Refer to Documentation/devicetree/bindings/phy/samsung-phy.txt
> +
> +Other common properties refer to
> +	Documentation/devicetree/binding/pci/designware-pcie.txt
>
>  Example:
>
> @@ -54,6 +65,24 @@ SoC specific DT Entry:
>  		num-lanes = <4>;
>  	};
>
> +With using PHY framework:
> +	pcie_phy0: pcie-phy@270000 {
> +		...
> +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> +		regn-names = "phy", "block";
> +		...
> +	};
> +
> +	pcie@290000 {
> +		...
> +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
> +		reg-names = "elbi", "config";
> +		phys = <&pcie_phy0>;
> +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
> +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
> +		...
> +	};
> +
>  Board specific DT Entry:
>
>  	pcie@290000 {
>

[PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[Jaehoon Chung]

This patch is for using PHY generic framework.
To maintain backward compatibility, check whether phy is supported or
not with 'using_phy'.

And if someone use the old dt-file, display the "deprecated" message.
But it's still working fine with it.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog on V2:
- This patch is split from previous PATCH[1/4]
- Maintain the backward compatibility
- Adds 'using_phy' for cheching whether phy framework is used or not
- Adds 'DEPRECATED' message for old dt-binding way

 drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index feed0fd..34f2eed 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -21,6 +21,7 @@
 #include <linux/of_gpio.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/phy/phy.h>
 #include <linux/resource.h>
 #include <linux/signal.h>
 #include <linux/types.h>
@@ -110,6 +111,10 @@ struct exynos_pcie {
 	struct exynos_pcie_clk_res	*clk_res;
 	const struct exynos_pcie_ops	*ops;
 	int				reset_gpio;
+
+	/* For Generic PHY Framework */
+	bool				using_phy;
+	struct phy			*phy;
 };
 
 struct exynos_pcie_ops {
@@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev,
 	if (IS_ERR(ep->mem_res->elbi_base))
 		return PTR_ERR(ep->mem_res->elbi_base);
 
+	/* If using the PHY framework, doesn't need to get other resource */
+	if (ep->using_phy)
+		return 0;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(ep->mem_res->phy_base))
@@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
 	}
 
 	exynos_pcie_assert_core_reset(exynos_pcie);
-	exynos_pcie_assert_phy_reset(exynos_pcie);
-	exynos_pcie_deassert_phy_reset(exynos_pcie);
-	exynos_pcie_power_on_phy(exynos_pcie);
-	exynos_pcie_init_phy(exynos_pcie);
-
-	/* pulse for common reset */
-	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
-				PCIE_PHY_COMMON_RESET);
-	udelay(500);
-	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
-				PCIE_PHY_COMMON_RESET);
+
+	if (exynos_pcie->using_phy) {
+		phy_reset(exynos_pcie->phy);
+
+		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
+				PCIE_PWR_RESET);
+
+		phy_power_on(exynos_pcie->phy);
+		phy_init(exynos_pcie->phy);
+	} else {
+		exynos_pcie_assert_phy_reset(exynos_pcie);
+		exynos_pcie_deassert_phy_reset(exynos_pcie);
+		exynos_pcie_power_on_phy(exynos_pcie);
+		exynos_pcie_init_phy(exynos_pcie);
+
+		/* pulse for common reset */
+		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
+					PCIE_PHY_COMMON_RESET);
+		udelay(500);
+		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
+					PCIE_PHY_COMMON_RESET);
+	}
 
 	exynos_pcie_deassert_core_reset(exynos_pcie);
 	dw_pcie_setup_rc(pp);
@@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
 	if (!dw_pcie_wait_for_link(pp))
 		return 0;
 
+	if (exynos_pcie->using_phy) {
+		phy_power_off(exynos_pcie->phy);
+		return -ETIMEDOUT;
+	}
+
 	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
 				PCIE_PHY_PLL_LOCKED) == 0) {
 		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
@@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
 
 	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
 
+	/* Assume that controller doesn't use the PHY framework */
+	exynos_pcie->using_phy = false;
+
+	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
+	if (IS_ERR(exynos_pcie->phy)) {
+		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
+			return PTR_ERR(exynos_pcie->phy);
+		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
+	} else
+		exynos_pcie->using_phy = true;
+
 	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
 		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
 		if (ret)
@@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 fail_probe:
+	if (exynos_pcie->using_phy)
+		phy_exit(exynos_pcie->phy);
+
 	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
 		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
 	return ret;
-- 
2.10.2

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[Krzysztof Kozlowski]

On Wed, Jan 04, 2017 at 09:34:34PM +0900, Jaehoon Chung wrote:
> This patch is for using PHY generic framework.
> To maintain backward compatibility, check whether phy is supported or
> not with 'using_phy'.
> 
> And if someone use the old dt-file, display the "deprecated" message.
> But it's still working fine with it.

This needs improvements. How about:
"Switch the pci-exynos driver to generic PHY framework. At the same time
backward compatibility is preserved: warning will be printed for old
DTB.

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - This patch is split from previous PATCH[1/4]
> - Maintain the backward compatibility
> - Adds 'using_phy' for cheching whether phy framework is used or not
> - Adds 'DEPRECATED' message for old dt-binding way
> 
>  drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index feed0fd..34f2eed 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
>  #include <linux/resource.h>
>  #include <linux/signal.h>
>  #include <linux/types.h>
> @@ -110,6 +111,10 @@ struct exynos_pcie {
>  	struct exynos_pcie_clk_res	*clk_res;
>  	const struct exynos_pcie_ops	*ops;
>  	int				reset_gpio;
> +
> +	/* For Generic PHY Framework */
> +	bool				using_phy;
> +	struct phy			*phy;
>  };
>  
>  struct exynos_pcie_ops {
> @@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev,
>  	if (IS_ERR(ep->mem_res->elbi_base))
>  		return PTR_ERR(ep->mem_res->elbi_base);
>  
> +	/* If using the PHY framework, doesn't need to get other resource */
> +	if (ep->using_phy)
> +		return 0;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(ep->mem_res->phy_base))
> @@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>  	}
>  
>  	exynos_pcie_assert_core_reset(exynos_pcie);
> -	exynos_pcie_assert_phy_reset(exynos_pcie);
> -	exynos_pcie_deassert_phy_reset(exynos_pcie);
> -	exynos_pcie_power_on_phy(exynos_pcie);
> -	exynos_pcie_init_phy(exynos_pcie);
> -
> -	/* pulse for common reset */
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> -				PCIE_PHY_COMMON_RESET);
> -	udelay(500);
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> -				PCIE_PHY_COMMON_RESET);
> +
> +	if (exynos_pcie->using_phy) {
> +		phy_reset(exynos_pcie->phy);
> +
> +		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
> +				PCIE_PWR_RESET);
> +
> +		phy_power_on(exynos_pcie->phy);
> +		phy_init(exynos_pcie->phy);
> +	} else {
> +		exynos_pcie_assert_phy_reset(exynos_pcie);
> +		exynos_pcie_deassert_phy_reset(exynos_pcie);
> +		exynos_pcie_power_on_phy(exynos_pcie);
> +		exynos_pcie_init_phy(exynos_pcie);
> +
> +		/* pulse for common reset */
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +	}
>  
>  	exynos_pcie_deassert_core_reset(exynos_pcie);
>  	dw_pcie_setup_rc(pp);
> @@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>  	if (!dw_pcie_wait_for_link(pp))
>  		return 0;
>  
> +	if (exynos_pcie->using_phy) {
> +		phy_power_off(exynos_pcie->phy);
> +		return -ETIMEDOUT;
> +	}
> +
>  	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
>  				PCIE_PHY_PLL_LOCKED) == 0) {
>  		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
> @@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>  
>  	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
>  
> +	/* Assume that controller doesn't use the PHY framework */
> +	exynos_pcie->using_phy = false;
> +
> +	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
> +	if (IS_ERR(exynos_pcie->phy)) {
> +		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
> +			return PTR_ERR(exynos_pcie->phy);
> +		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
> +	} else
> +		exynos_pcie->using_phy = true;
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
>  		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
>  		if (ret)
> @@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>  	return 0;
>  
>  fail_probe:
> +	if (exynos_pcie->using_phy)
> +		phy_exit(exynos_pcie->phy);
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
>  		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
>  	return ret;
> -- 
> 2.10.2
> 

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[Jaehoon Chung]

On 01/05/2017 02:50 AM, Krzysztof Kozlowski wrote:
> On Wed, Jan 04, 2017 at 09:34:34PM +0900, Jaehoon Chung wrote:
>> This patch is for using PHY generic framework.
>> To maintain backward compatibility, check whether phy is supported or
>> not with 'using_phy'.
>>
>> And if someone use the old dt-file, display the "deprecated" message.
>> But it's still working fine with it.
> 
> This needs improvements. How about:
> "Switch the pci-exynos driver to generic PHY framework. At the same time
> backward compatibility is preserved: warning will be printed for old
> DTB.

Thanks for comments. Will describe the commit-msg in more detail.

Best Regards,
Jaehoon Chung

> 
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Best regards,
> Krzysztof
> 
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - This patch is split from previous PATCH[1/4]
>> - Maintain the backward compatibility
>> - Adds 'using_phy' for cheching whether phy framework is used or not
>> - Adds 'DEPRECATED' message for old dt-binding way
>>
>>  drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 50 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
>> index feed0fd..34f2eed 100644
>> --- a/drivers/pci/host/pci-exynos.c
>> +++ b/drivers/pci/host/pci-exynos.c
>> @@ -21,6 +21,7 @@
>>  #include <linux/of_gpio.h>
>>  #include <linux/pci.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>>  #include <linux/resource.h>
>>  #include <linux/signal.h>
>>  #include <linux/types.h>
>> @@ -110,6 +111,10 @@ struct exynos_pcie {
>>  	struct exynos_pcie_clk_res	*clk_res;
>>  	const struct exynos_pcie_ops	*ops;
>>  	int				reset_gpio;
>> +
>> +	/* For Generic PHY Framework */
>> +	bool				using_phy;
>> +	struct phy			*phy;
>>  };
>>  
>>  struct exynos_pcie_ops {
>> @@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev,
>>  	if (IS_ERR(ep->mem_res->elbi_base))
>>  		return PTR_ERR(ep->mem_res->elbi_base);
>>  
>> +	/* If using the PHY framework, doesn't need to get other resource */
>> +	if (ep->using_phy)
>> +		return 0;
>> +
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>  	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
>>  	if (IS_ERR(ep->mem_res->phy_base))
>> @@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>>  	}
>>  
>>  	exynos_pcie_assert_core_reset(exynos_pcie);
>> -	exynos_pcie_assert_phy_reset(exynos_pcie);
>> -	exynos_pcie_deassert_phy_reset(exynos_pcie);
>> -	exynos_pcie_power_on_phy(exynos_pcie);
>> -	exynos_pcie_init_phy(exynos_pcie);
>> -
>> -	/* pulse for common reset */
>> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
>> -				PCIE_PHY_COMMON_RESET);
>> -	udelay(500);
>> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
>> -				PCIE_PHY_COMMON_RESET);
>> +
>> +	if (exynos_pcie->using_phy) {
>> +		phy_reset(exynos_pcie->phy);
>> +
>> +		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
>> +				PCIE_PWR_RESET);
>> +
>> +		phy_power_on(exynos_pcie->phy);
>> +		phy_init(exynos_pcie->phy);
>> +	} else {
>> +		exynos_pcie_assert_phy_reset(exynos_pcie);
>> +		exynos_pcie_deassert_phy_reset(exynos_pcie);
>> +		exynos_pcie_power_on_phy(exynos_pcie);
>> +		exynos_pcie_init_phy(exynos_pcie);
>> +
>> +		/* pulse for common reset */
>> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
>> +					PCIE_PHY_COMMON_RESET);
>> +		udelay(500);
>> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
>> +					PCIE_PHY_COMMON_RESET);
>> +	}
>>  
>>  	exynos_pcie_deassert_core_reset(exynos_pcie);
>>  	dw_pcie_setup_rc(pp);
>> @@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>>  	if (!dw_pcie_wait_for_link(pp))
>>  		return 0;
>>  
>> +	if (exynos_pcie->using_phy) {
>> +		phy_power_off(exynos_pcie->phy);
>> +		return -ETIMEDOUT;
>> +	}
>> +
>>  	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
>>  				PCIE_PHY_PLL_LOCKED) == 0) {
>>  		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
>> @@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>>  
>>  	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
>>  
>> +	/* Assume that controller doesn't use the PHY framework */
>> +	exynos_pcie->using_phy = false;
>> +
>> +	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
>> +	if (IS_ERR(exynos_pcie->phy)) {
>> +		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
>> +			return PTR_ERR(exynos_pcie->phy);
>> +		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
>> +	} else
>> +		exynos_pcie->using_phy = true;
>> +
>>  	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
>>  		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
>>  		if (ret)
>> @@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>>  	return 0;
>>  
>>  fail_probe:
>> +	if (exynos_pcie->using_phy)
>> +		phy_exit(exynos_pcie->phy);
>> +
>>  	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
>>  		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
>>  	return ret;
>> -- 
>> 2.10.2
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

On Wednesday, January 4, 2017 7:35 AM, Jaehoon Chung wrote:
> 
> This patch is for using PHY generic framework.
> To maintain backward compatibility, check whether phy is supported or
> not with 'using_phy'.
> 
> And if someone use the old dt-file, display the "deprecated" message.
> But it's still working fine with it.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

It looks good!

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han


> ---
> Changelog on V2:
> - This patch is split from previous PATCH[1/4]
> - Maintain the backward compatibility
> - Adds 'using_phy' for cheching whether phy framework is used or not
> - Adds 'DEPRECATED' message for old dt-binding way
> 
>  drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++---
> -----
>  1 file changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index feed0fd..34f2eed 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
>  #include <linux/resource.h>
>  #include <linux/signal.h>
>  #include <linux/types.h>
> @@ -110,6 +111,10 @@ struct exynos_pcie {
>  	struct exynos_pcie_clk_res	*clk_res;
>  	const struct exynos_pcie_ops	*ops;
>  	int				reset_gpio;
> +
> +	/* For Generic PHY Framework */
> +	bool				using_phy;
> +	struct phy			*phy;
>  };
> 
>  struct exynos_pcie_ops {
> @@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct
> platform_device *pdev,
>  	if (IS_ERR(ep->mem_res->elbi_base))
>  		return PTR_ERR(ep->mem_res->elbi_base);
> 
> +	/* If using the PHY framework, doesn't need to get other resource
> */
> +	if (ep->using_phy)
> +		return 0;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(ep->mem_res->phy_base))
> @@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *exynos_pcie)
>  	}
> 
>  	exynos_pcie_assert_core_reset(exynos_pcie);
> -	exynos_pcie_assert_phy_reset(exynos_pcie);
> -	exynos_pcie_deassert_phy_reset(exynos_pcie);
> -	exynos_pcie_power_on_phy(exynos_pcie);
> -	exynos_pcie_init_phy(exynos_pcie);
> -
> -	/* pulse for common reset */
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> -				PCIE_PHY_COMMON_RESET);
> -	udelay(500);
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> -				PCIE_PHY_COMMON_RESET);
> +
> +	if (exynos_pcie->using_phy) {
> +		phy_reset(exynos_pcie->phy);
> +
> +		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
> +				PCIE_PWR_RESET);
> +
> +		phy_power_on(exynos_pcie->phy);
> +		phy_init(exynos_pcie->phy);
> +	} else {
> +		exynos_pcie_assert_phy_reset(exynos_pcie);
> +		exynos_pcie_deassert_phy_reset(exynos_pcie);
> +		exynos_pcie_power_on_phy(exynos_pcie);
> +		exynos_pcie_init_phy(exynos_pcie);
> +
> +		/* pulse for common reset */
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +	}
> 
>  	exynos_pcie_deassert_core_reset(exynos_pcie);
>  	dw_pcie_setup_rc(pp);
> @@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *exynos_pcie)
>  	if (!dw_pcie_wait_for_link(pp))
>  		return 0;
> 
> +	if (exynos_pcie->using_phy) {
> +		phy_power_off(exynos_pcie->phy);
> +		return -ETIMEDOUT;
> +	}
> +
>  	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
>  				PCIE_PHY_PLL_LOCKED) == 0) {
>  		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
> @@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct
> platform_device *pdev)
> 
>  	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> 
> +	/* Assume that controller doesn't use the PHY framework */
> +	exynos_pcie->using_phy = false;
> +
> +	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
> +	if (IS_ERR(exynos_pcie->phy)) {
> +		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
> +			return PTR_ERR(exynos_pcie->phy);
> +		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> exynos was deprecated!!\n");
> +	} else
> +		exynos_pcie->using_phy = true;
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
>  		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
>  		if (ret)
> @@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct
> platform_device *pdev)
>  	return 0;
> 
>  fail_probe:
> +	if (exynos_pcie->using_phy)
> +		phy_exit(exynos_pcie->phy);
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
>  		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
>  	return ret;
> --
> 2.10.2

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[Jingoo Han]

On Wednesday, January 4, 2017 7:35 AM, Jaehoon Chung wrote:
> 
> This patch is for using PHY generic framework.
> To maintain backward compatibility, check whether phy is supported or
> not with 'using_phy'.
> 
> And if someone use the old dt-file, display the "deprecated" message.
> But it's still working fine with it.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

It looks good!

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han


> ---
> Changelog on V2:
> - This patch is split from previous PATCH[1/4]
> - Maintain the backward compatibility
> - Adds 'using_phy' for cheching whether phy framework is used or not
> - Adds 'DEPRECATED' message for old dt-binding way
> 
>  drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++---
> -----
>  1 file changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index feed0fd..34f2eed 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
>  #include <linux/resource.h>
>  #include <linux/signal.h>
>  #include <linux/types.h>
> @@ -110,6 +111,10 @@ struct exynos_pcie {
>  	struct exynos_pcie_clk_res	*clk_res;
>  	const struct exynos_pcie_ops	*ops;
>  	int				reset_gpio;
> +
> +	/* For Generic PHY Framework */
> +	bool				using_phy;
> +	struct phy			*phy;
>  };
> 
>  struct exynos_pcie_ops {
> @@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct
> platform_device *pdev,
>  	if (IS_ERR(ep->mem_res->elbi_base))
>  		return PTR_ERR(ep->mem_res->elbi_base);
> 
> +	/* If using the PHY framework, doesn't need to get other resource
> */
> +	if (ep->using_phy)
> +		return 0;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(ep->mem_res->phy_base))
> @@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *exynos_pcie)
>  	}
> 
>  	exynos_pcie_assert_core_reset(exynos_pcie);
> -	exynos_pcie_assert_phy_reset(exynos_pcie);
> -	exynos_pcie_deassert_phy_reset(exynos_pcie);
> -	exynos_pcie_power_on_phy(exynos_pcie);
> -	exynos_pcie_init_phy(exynos_pcie);
> -
> -	/* pulse for common reset */
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> -				PCIE_PHY_COMMON_RESET);
> -	udelay(500);
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> -				PCIE_PHY_COMMON_RESET);
> +
> +	if (exynos_pcie->using_phy) {
> +		phy_reset(exynos_pcie->phy);
> +
> +		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
> +				PCIE_PWR_RESET);
> +
> +		phy_power_on(exynos_pcie->phy);
> +		phy_init(exynos_pcie->phy);
> +	} else {
> +		exynos_pcie_assert_phy_reset(exynos_pcie);
> +		exynos_pcie_deassert_phy_reset(exynos_pcie);
> +		exynos_pcie_power_on_phy(exynos_pcie);
> +		exynos_pcie_init_phy(exynos_pcie);
> +
> +		/* pulse for common reset */
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +	}
> 
>  	exynos_pcie_deassert_core_reset(exynos_pcie);
>  	dw_pcie_setup_rc(pp);
> @@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct
> exynos_pcie *exynos_pcie)
>  	if (!dw_pcie_wait_for_link(pp))
>  		return 0;
> 
> +	if (exynos_pcie->using_phy) {
> +		phy_power_off(exynos_pcie->phy);
> +		return -ETIMEDOUT;
> +	}
> +
>  	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
>  				PCIE_PHY_PLL_LOCKED) == 0) {
>  		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
> @@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct
> platform_device *pdev)
> 
>  	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> 
> +	/* Assume that controller doesn't use the PHY framework */
> +	exynos_pcie->using_phy = false;
> +
> +	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
> +	if (IS_ERR(exynos_pcie->phy)) {
> +		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
> +			return PTR_ERR(exynos_pcie->phy);
> +		dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> exynos was deprecated!!\n");
> +	} else
> +		exynos_pcie->using_phy = true;
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
>  		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
>  		if (ret)
> @@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct
> platform_device *pdev)
>  	return 0;
> 
>  fail_probe:
> +	if (exynos_pcie->using_phy)
> +		phy_exit(exynos_pcie->phy);
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
>  		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
>  	return ret;
> --
> 2.10.2

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[pankaj.dubey]

Hi Jaehoon,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> This patch is for using PHY generic framework.
> To maintain backward compatibility, check whether phy is supported or
> not with 'using_phy'.
> 
> And if someone use the old dt-file, display the "deprecated" message.
> But it's still working fine with it.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - This patch is split from previous PATCH[1/4]
> - Maintain the backward compatibility
> - Adds 'using_phy' for cheching whether phy framework is used or not
> - Adds 'DEPRECATED' message for old dt-binding way
> 

Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>

Thanks,
Pankaj Dubey

Re: [PATCH V2 4/5] PCI: exynos: support the using PHY generic framework

[Alim Akhtar]

Hi Jaehoon,

On 01/04/2017 06:04 PM, Jaehoon Chung wrote:
> This patch is for using PHY generic framework.
> To maintain backward compatibility, check whether phy is supported or
> not with 'using_phy'.
>
> And if someone use the old dt-file, display the "deprecated" message.
> But it's still working fine with it.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>


> Changelog on V2:
> - This patch is split from previous PATCH[1/4]
> - Maintain the backward compatibility
> - Adds 'using_phy' for cheching whether phy framework is used or not
> - Adds 'DEPRECATED' message for old dt-binding way
>
>  drivers/pci/host/pci-exynos.c | 61 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index feed0fd..34f2eed 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
>  #include <linux/resource.h>
>  #include <linux/signal.h>
>  #include <linux/types.h>
> @@ -110,6 +111,10 @@ struct exynos_pcie {
>  	struct exynos_pcie_clk_res	*clk_res;
>  	const struct exynos_pcie_ops	*ops;
>  	int				reset_gpio;
> +
> +	/* For Generic PHY Framework */
> +	bool				using_phy;
> +	struct phy			*phy;
>  };
>
>  struct exynos_pcie_ops {
> @@ -135,6 +140,10 @@ static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev,
>  	if (IS_ERR(ep->mem_res->elbi_base))
>  		return PTR_ERR(ep->mem_res->elbi_base);
>
> +	/* If using the PHY framework, doesn't need to get other resource */
> +	if (ep->using_phy)
> +		return 0;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	ep->mem_res->phy_base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(ep->mem_res->phy_base))
> @@ -396,17 +405,28 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>  	}
>
>  	exynos_pcie_assert_core_reset(exynos_pcie);
> -	exynos_pcie_assert_phy_reset(exynos_pcie);
> -	exynos_pcie_deassert_phy_reset(exynos_pcie);
> -	exynos_pcie_power_on_phy(exynos_pcie);
> -	exynos_pcie_init_phy(exynos_pcie);
> -
> -	/* pulse for common reset */
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> -				PCIE_PHY_COMMON_RESET);
> -	udelay(500);
> -	exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> -				PCIE_PHY_COMMON_RESET);
> +
> +	if (exynos_pcie->using_phy) {
> +		phy_reset(exynos_pcie->phy);
> +
> +		exynos_pcie_writel(exynos_pcie->mem_res->elbi_base, 1,
> +				PCIE_PWR_RESET);
> +
> +		phy_power_on(exynos_pcie->phy);
> +		phy_init(exynos_pcie->phy);
> +	} else {
> +		exynos_pcie_assert_phy_reset(exynos_pcie);
> +		exynos_pcie_deassert_phy_reset(exynos_pcie);
> +		exynos_pcie_power_on_phy(exynos_pcie);
> +		exynos_pcie_init_phy(exynos_pcie);
> +
> +		/* pulse for common reset */
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 1,
> +					PCIE_PHY_COMMON_RESET);
> +		udelay(500);
> +		exynos_pcie_writel(exynos_pcie->mem_res->block_base, 0,
> +					PCIE_PHY_COMMON_RESET);
> +	}
>
>  	exynos_pcie_deassert_core_reset(exynos_pcie);
>  	dw_pcie_setup_rc(pp);
> @@ -420,6 +440,11 @@ static int exynos_pcie_establish_link(struct exynos_pcie *exynos_pcie)
>  	if (!dw_pcie_wait_for_link(pp))
>  		return 0;
>
> +	if (exynos_pcie->using_phy) {
> +		phy_power_off(exynos_pcie->phy);
> +		return -ETIMEDOUT;
> +	}
> +
>  	while (exynos_pcie_readl(exynos_pcie->mem_res->phy_base,
>  				PCIE_PHY_PLL_LOCKED) == 0) {
>  		val = exynos_pcie_readl(exynos_pcie->mem_res->block_base,
> @@ -633,6 +658,17 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>
>  	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
>
> +	/* Assume that controller doesn't use the PHY framework */
> +	exynos_pcie->using_phy = false;
> +
> +	exynos_pcie->phy = devm_of_phy_get(dev, np, NULL);
> +	if (IS_ERR(exynos_pcie->phy)) {
> +		if (PTR_ERR(exynos_pcie->phy) == -EPROBE_DEFER)
> +			return PTR_ERR(exynos_pcie->phy);
> +		dev_warn(dev, "Use the 'phy' property. Current DT of pci-exynos was deprecated!!\n");
> +	} else
> +		exynos_pcie->using_phy = true;
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->get_mem_resources) {
>  		ret = exynos_pcie->ops->get_mem_resources(pdev, exynos_pcie);
>  		if (ret)
> @@ -657,6 +693,9 @@ static int __init exynos_pcie_probe(struct platform_device *pdev)
>  	return 0;
>
>  fail_probe:
> +	if (exynos_pcie->using_phy)
> +		phy_exit(exynos_pcie->phy);
> +
>  	if (exynos_pcie->ops && exynos_pcie->ops->deinit_clk_resources)
>  		exynos_pcie->ops->deinit_clk_resources(exynos_pcie);
>  	return ret;
>

[PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[Jaehoon Chung]

Add phy-pcie node for using Exynos5440 pcie.
And use the reg-names as "elbi" and "config".
Because the getting configuratioin space address from ranges is old way.
It also is helpful to distinguish more clearly.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog on V2:
- Removes the child-node
- Fixes the typo
- Removes the unnecessary comments

 arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index 2a2e570..feb074d 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -290,11 +290,22 @@
 		clock-names = "usbhost";
 	};
 
+	pcie_phy0: pcie-phy@270000 {
+		#phy-cells = <0>;
+		compatible = "samsung,exynos5440-pcie-phy";
+		reg = <0x270000 0x1000>, <0x271000 0x40>;
+	};
+
+	pcie_phy1: pcie-phy@272000 {
+		#phy-cells = <0>;
+		compatible = "samsung,exynos5440-pcie-phy";
+		reg = <0x272000 0x1000>, <0x271040 0x40>;
+	};
+
 	pcie_0: pcie@290000 {
 		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
-		reg = <0x290000 0x1000
-			0x270000 0x1000
-			0x271000 0x40>;
+		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
+		reg-names = "elbi", "config";
 		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -303,9 +314,9 @@
 		#address-cells = <3>;
 		#size-cells = <2>;
 		device_type = "pci";
-		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
-			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
-			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
+		phys = <&pcie_phy0>;
+		ranges = <0x81000000 0 0	  0x40001000 0 0x00010000
+			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>;
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 53>;
@@ -315,9 +326,8 @@
 
 	pcie_1: pcie@2a0000 {
 		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
-		reg = <0x2a0000 0x1000
-			0x272000 0x1000
-			0x271040 0x40>;
+		reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
+		reg-names = "elbi", "config";
 		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
@@ -326,9 +336,9 @@
 		#address-cells = <3>;
 		#size-cells = <2>;
 		device_type = "pci";
-		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
-			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
-			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
+		phys = <&pcie_phy1>;
+		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
+			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 56>;
-- 
2.10.2

Re: [PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[Krzysztof Kozlowski]

On Wed, Jan 04, 2017 at 09:34:35PM +0900, Jaehoon Chung wrote:
> Add phy-pcie node for using Exynos5440 pcie.
> And use the reg-names as "elbi" and "config".

'and' is only for joining in compound sentences, don't start with it.

> Because the getting configuratioin space address from ranges is old way.

Spell-check please.

> It also is helpful to distinguish more clearly.

Distinguish what? Please work on the commit msg, I am not picking 
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Removes the child-node
> - Fixes the typo
> - Removes the unnecessary comments
> 
>  arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
> index 2a2e570..feb074d 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -290,11 +290,22 @@
>  		clock-names = "usbhost";
>  	};
>  
> +	pcie_phy0: pcie-phy@270000 {
> +		#phy-cells = <0>;
> +		compatible = "samsung,exynos5440-pcie-phy";
> +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> +	};
> +
> +	pcie_phy1: pcie-phy@272000 {
> +		#phy-cells = <0>;
> +		compatible = "samsung,exynos5440-pcie-phy";
> +		reg = <0x272000 0x1000>, <0x271040 0x40>;
> +	};
> +
>  	pcie_0: pcie@290000 {
>  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> -		reg = <0x290000 0x1000
> -			0x270000 0x1000
> -			0x271000 0x40>;
> +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
> +		reg-names = "elbi", "config";
>  		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> @@ -303,9 +314,9 @@
>  		#address-cells = <3>;
>  		#size-cells = <2>;
>  		device_type = "pci";
> -		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
> -			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
> -			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
> +		phys = <&pcie_phy0>;
> +		ranges = <0x81000000 0 0	  0x40001000 0 0x00010000
> +			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>;

I think the comments were useful. You can leave them.

>  		#interrupt-cells = <1>;
>  		interrupt-map-mask = <0 0 0 0>;
>  		interrupt-map = <0x0 0 &gic 53>;
> @@ -315,9 +326,8 @@
>  
>  	pcie_1: pcie@2a0000 {
>  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> -		reg = <0x2a0000 0x1000
> -			0x272000 0x1000
> -			0x271040 0x40>;
> +		reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
> +		reg-names = "elbi", "config";
>  		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> @@ -326,9 +336,9 @@
>  		#address-cells = <3>;
>  		#size-cells = <2>;
>  		device_type = "pci";
> -		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
> -			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
> -			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
> +		phys = <&pcie_phy1>;
> +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
> +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;

I think the comments were useful. You can leave them.

This looks like depending on the changes in the driver, so I will need a
tag or stable branch from PCIe maintainers.

Best regards,
Krzysztof

Re: [PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[Jingoo Han]

On Wednesday, January 4, 2017 12:58 PM, Krzysztof Kozlowski wrote:
> 
> On Wed, Jan 04, 2017 at 09:34:35PM +0900, Jaehoon Chung wrote:
> > Add phy-pcie node for using Exynos5440 pcie.
> > And use the reg-names as "elbi" and "config".
> 
> 'and' is only for joining in compound sentences, don't start with it.
> 
> > Because the getting configuratioin space address from ranges is old way.
> 
> Spell-check please.
> 
> > It also is helpful to distinguish more clearly.
> 
> Distinguish what? Please work on the commit msg, I am not picking
> >
> > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > ---
> > Changelog on V2:
> > - Removes the child-node
> > - Fixes the typo
> > - Removes the unnecessary comments
> >
> >  arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++----------
> --
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5440.dtsi
> b/arch/arm/boot/dts/exynos5440.dtsi
> > index 2a2e570..feb074d 100644
> > --- a/arch/arm/boot/dts/exynos5440.dtsi
> > +++ b/arch/arm/boot/dts/exynos5440.dtsi
> > @@ -290,11 +290,22 @@
> >  		clock-names = "usbhost";
> >  	};
> >
> > +	pcie_phy0: pcie-phy@270000 {
> > +		#phy-cells = <0>;
> > +		compatible = "samsung,exynos5440-pcie-phy";
> > +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> > +	};
> > +
> > +	pcie_phy1: pcie-phy@272000 {
> > +		#phy-cells = <0>;
> > +		compatible = "samsung,exynos5440-pcie-phy";
> > +		reg = <0x272000 0x1000>, <0x271040 0x40>;
> > +	};
> > +
> >  	pcie_0: pcie@290000 {
> >  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> > -		reg = <0x290000 0x1000
> > -			0x270000 0x1000
> > -			0x271000 0x40>;
> > +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
> > +		reg-names = "elbi", "config";
> >  		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> > @@ -303,9 +314,9 @@
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> >  		device_type = "pci";
> > -		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000
> /* configuration space */
> > -			  0x81000000 0 0	  0x40001000 0 0x00010000   /*
> downstream I/O */
> > -			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /*
> non-prefetchable memory */
> > +		phys = <&pcie_phy0>;
> > +		ranges = <0x81000000 0 0	  0x40001000 0 0x00010000
> > +			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.
> 
> >  		#interrupt-cells = <1>;
> >  		interrupt-map-mask = <0 0 0 0>;
> >  		interrupt-map = <0x0 0 &gic 53>;
> > @@ -315,9 +326,8 @@
> >
> >  	pcie_1: pcie@2a0000 {
> >  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> > -		reg = <0x2a0000 0x1000
> > -			0x272000 0x1000
> > -			0x271040 0x40>;
> > +		reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
> > +		reg-names = "elbi", "config";
> >  		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > @@ -326,9 +336,9 @@
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> >  		device_type = "pci";
> > -		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000
> /* configuration space */
> > -			  0x81000000 0 0	  0x60001000 0 0x00010000   /*
> downstream I/O */
> > -			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /*
> non-prefetchable memory */
> > +		phys = <&pcie_phy1>;
> > +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
> > +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.

I think so, too.
Please leave the comments

Best regards,
Jingoo Han

> 
> This looks like depending on the changes in the driver, so I will need a
> tag or stable branch from PCIe maintainers.
> 
> Best regards,
> Krzysztof

Re: [PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[Jingoo Han]

On Wednesday, January 4, 2017 12:58 PM, Krzysztof Kozlowski wrote:
> 
> On Wed, Jan 04, 2017 at 09:34:35PM +0900, Jaehoon Chung wrote:
> > Add phy-pcie node for using Exynos5440 pcie.
> > And use the reg-names as "elbi" and "config".
> 
> 'and' is only for joining in compound sentences, don't start with it.
> 
> > Because the getting configuratioin space address from ranges is old way.
> 
> Spell-check please.
> 
> > It also is helpful to distinguish more clearly.
> 
> Distinguish what? Please work on the commit msg, I am not picking
> >
> > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > ---
> > Changelog on V2:
> > - Removes the child-node
> > - Fixes the typo
> > - Removes the unnecessary comments
> >
> >  arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++----------
> --
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5440.dtsi
> b/arch/arm/boot/dts/exynos5440.dtsi
> > index 2a2e570..feb074d 100644
> > --- a/arch/arm/boot/dts/exynos5440.dtsi
> > +++ b/arch/arm/boot/dts/exynos5440.dtsi
> > @@ -290,11 +290,22 @@
> >  		clock-names = "usbhost";
> >  	};
> >
> > +	pcie_phy0: pcie-phy@270000 {
> > +		#phy-cells = <0>;
> > +		compatible = "samsung,exynos5440-pcie-phy";
> > +		reg = <0x270000 0x1000>, <0x271000 0x40>;
> > +	};
> > +
> > +	pcie_phy1: pcie-phy@272000 {
> > +		#phy-cells = <0>;
> > +		compatible = "samsung,exynos5440-pcie-phy";
> > +		reg = <0x272000 0x1000>, <0x271040 0x40>;
> > +	};
> > +
> >  	pcie_0: pcie@290000 {
> >  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> > -		reg = <0x290000 0x1000
> > -			0x270000 0x1000
> > -			0x271000 0x40>;
> > +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
> > +		reg-names = "elbi", "config";
> >  		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> > @@ -303,9 +314,9 @@
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> >  		device_type = "pci";
> > -		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000
> /* configuration space */
> > -			  0x81000000 0 0	  0x40001000 0 0x00010000   /*
> downstream I/O */
> > -			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /*
> non-prefetchable memory */
> > +		phys = <&pcie_phy0>;
> > +		ranges = <0x81000000 0 0	  0x40001000 0 0x00010000
> > +			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.
> 
> >  		#interrupt-cells = <1>;
> >  		interrupt-map-mask = <0 0 0 0>;
> >  		interrupt-map = <0x0 0 &gic 53>;
> > @@ -315,9 +326,8 @@
> >
> >  	pcie_1: pcie@2a0000 {
> >  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> > -		reg = <0x2a0000 0x1000
> > -			0x272000 0x1000
> > -			0x271040 0x40>;
> > +		reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
> > +		reg-names = "elbi", "config";
> >  		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > @@ -326,9 +336,9 @@
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> >  		device_type = "pci";
> > -		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000
> /* configuration space */
> > -			  0x81000000 0 0	  0x60001000 0 0x00010000   /*
> downstream I/O */
> > -			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /*
> non-prefetchable memory */
> > +		phys = <&pcie_phy1>;
> > +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
> > +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.

I think so, too.
Please leave the comments

Best regards,
Jingoo Han

> 
> This looks like depending on the changes in the driver, so I will need a
> tag or stable branch from PCIe maintainers.
> 
> Best regards,
> Krzysztof

Re: [PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[Jaehoon Chung]

On 01/05/2017 02:58 AM, Krzysztof Kozlowski wrote:
> On Wed, Jan 04, 2017 at 09:34:35PM +0900, Jaehoon Chung wrote:
>> Add phy-pcie node for using Exynos5440 pcie.
>> And use the reg-names as "elbi" and "config".
> 
> 'and' is only for joining in compound sentences, don't start with it.

Got it.

> 
>> Because the getting configuratioin space address from ranges is old way.
> 
> Spell-check please.

Will do.

> 
>> It also is helpful to distinguish more clearly.
> 
> Distinguish what? Please work on the commit msg, I am not picking 

Will update the commit-msg.

>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - Removes the child-node
>> - Fixes the typo
>> - Removes the unnecessary comments
>>
>>  arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++------------
>>  1 file changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
>> index 2a2e570..feb074d 100644
>> --- a/arch/arm/boot/dts/exynos5440.dtsi
>> +++ b/arch/arm/boot/dts/exynos5440.dtsi
>> @@ -290,11 +290,22 @@
>>  		clock-names = "usbhost";
>>  	};
>>  
>> +	pcie_phy0: pcie-phy@270000 {
>> +		#phy-cells = <0>;
>> +		compatible = "samsung,exynos5440-pcie-phy";
>> +		reg = <0x270000 0x1000>, <0x271000 0x40>;
>> +	};
>> +
>> +	pcie_phy1: pcie-phy@272000 {
>> +		#phy-cells = <0>;
>> +		compatible = "samsung,exynos5440-pcie-phy";
>> +		reg = <0x272000 0x1000>, <0x271040 0x40>;
>> +	};
>> +
>>  	pcie_0: pcie@290000 {
>>  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
>> -		reg = <0x290000 0x1000
>> -			0x270000 0x1000
>> -			0x271000 0x40>;
>> +		reg = <0x290000 0x1000>, <0x40000000 0x1000>;
>> +		reg-names = "elbi", "config";
>>  		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
>>  			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
>>  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -303,9 +314,9 @@
>>  		#address-cells = <3>;
>>  		#size-cells = <2>;
>>  		device_type = "pci";
>> -		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
>> -			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
>> -			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
>> +		phys = <&pcie_phy0>;
>> +		ranges = <0x81000000 0 0	  0x40001000 0 0x00010000
>> +			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.

Ok. will keep.

> 
>>  		#interrupt-cells = <1>;
>>  		interrupt-map-mask = <0 0 0 0>;
>>  		interrupt-map = <0x0 0 &gic 53>;
>> @@ -315,9 +326,8 @@
>>  
>>  	pcie_1: pcie@2a0000 {
>>  		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
>> -		reg = <0x2a0000 0x1000
>> -			0x272000 0x1000
>> -			0x271040 0x40>;
>> +		reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
>> +		reg-names = "elbi", "config";
>>  		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>,
>>  			     <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
>>  			     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -326,9 +336,9 @@
>>  		#address-cells = <3>;
>>  		#size-cells = <2>;
>>  		device_type = "pci";
>> -		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
>> -			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
>> -			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
>> +		phys = <&pcie_phy1>;
>> +		ranges = <0x81000000 0 0	  0x60001000 0 0x00010000
>> +			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>;
> 
> I think the comments were useful. You can leave them.
> 
> This looks like depending on the changes in the driver, so I will need a
> tag or stable branch from PCIe maintainers.

Right..

Best Regards,
Jaehoon Chung

> 
> Best regards,
> Krzysztof
> 
> 
> 

Re: [PATCH V2 5/5] ARM: dts: exynos5440: support the phy-pcie node for pcie

[pankaj.dubey]

Hi Jaehoon,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> Add phy-pcie node for using Exynos5440 pcie.
> And use the reg-names as "elbi" and "config".
> Because the getting configuratioin space address from ranges is old way.
> It also is helpful to distinguish more clearly.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Removes the child-node
> - Fixes the typo
> - Removes the unnecessary comments
> 
>  arch/arm/boot/dts/exynos5440.dtsi | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 

Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>

Thanks,
Pankaj Dubey

Re: [PATCH V2 0/5] PCI: exynos: use the PHY generic framework

[Bjorn Helgaas]

On Wed, Jan 04, 2017 at 09:34:30PM +0900, Jaehoon Chung wrote:
> This patchset is for using PHY generic framework.
> Current pci-exyons doesn't use the phy framework since there haven't been on
> PHY subsystem when Exynos5440 had been upstremed.
> The not using PHY framework make the difficult to upstream the other
> Exynos SoCs.
> 
> Before upstreaming the other Exynos SoCs, it's goal what make to use the PHY framework.
> 
> This patchset has the below modifications:
> 1) Introduces the phy-pcie-pcie
> 2) Handles Phy controller from PHY framework for pci-exynos
> 3) Modifies the dt-binding of pci-exynos
> - The using the getting configuration space address from ranges is old.
> - Deprecated the old way.
> 4) Maintains the backward compatibility
> 
> NOTE: These patches based on below patches:
> 	http://patchwork.ozlabs.org/patch/706998/
> 	http://patchwork.ozlabs.org/patch/706997/
> 	http://patchwork.ozlabs.org/patch/706995/
> 	http://patchwork.ozlabs.org/patch/706994/
> 	http://patchwork.ozlabs.org/patch/703530/
> 	- This patch should be conflicted. so fixes the manually.
> 	http://patchwork.ozlabs.org/patch/708414/
> 
> Changelog on V2:
> - Keep current codes for backward compatibility
> - Fixes some typos
> - Split the patches for removing the dependency
> - Removes the unnecessary codes
> - Change the patch's sequence
> - Based on latest PCI git repository.(next branch)
> 
> Jaehoon Chung (5):
>   Documetation: samsung-phy: add the exynos-pcie-phy binding
>   phy: phy-exynos-pcie: Add support for Exynos PCIe phy
>   Documetation: binding: modify the exynos5440 pcie binding
>   PCI: exynos: support the using PHY generic framework
>   ARM: dts: exynos5440: support the phy-pcie node for pcie

Since you're going to update this, please fix the typos in the above,
e.g., s/Documetation/Documentation/.

In addition, please run "git log --oneline" on the files you're
changing and make your subject lines consistent in style and
capitalization with previous ones, e.g., use "PCIe" consistently.
"Modify the ..." contains no useful information.  Of course a patch
modifies something -- please tell us what the modification is useful
for.

>  .../bindings/pci/samsung,exynos5440-pcie.txt       |  29 +++
>  .../devicetree/bindings/phy/samsung-phy.txt        |  17 ++
>  arch/arm/boot/dts/exynos5440.dtsi                  |  34 ++-
>  drivers/pci/host/pci-exynos.c                      |  61 ++++-
>  drivers/phy/Kconfig                                |   9 +
>  drivers/phy/Makefile                               |   1 +
>  drivers/phy/phy-exynos-pcie.c                      | 280 +++++++++++++++++++++
>  7 files changed, 408 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 
> -- 
> 2.10.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html