[RFC PATCH 0/6] Support the PCIe for TM2(exynos5433)

[RFC PATCH 0/6] Support the PCIe for TM2(exynos5433)

[Jaehoon Chung]

This patchset is for supporting PCIe exynos5433.
TM2(exynos5433) supports the PCIe for WiFi. In driver/pci/host/, there is pci-exynos.c.
But i can't touch anything. The below reasons are why i added the new file.
1. Don't have the exynos5440 TRM
- I can't check anything for exynso5440.
- So i can't touch anything for using PHY generic framework.
2. Can't test the exynos5440 board.
- If used the phy generic framework, can't ensure whether it's working fine or not.
3. There is no maintiain for exynos5440.
- i don't know anywhere pci-exynos5440 is used.

As i know, Bjorn(PCIe Maintainer) agreed about adding the new file.

So i added the new pci-exynos5433 file in driver/pci/host/.
And adds the phy-exynos-pcie.c for using PHY generic framework.
When use the PHY generic framework, controlling pcie is more easier than now.

There are future works,
      - Supporting MSI
      - If possible, combine the one file to pci-exynos.c

This is based on httt://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git (for-next branch)
(Because PCI git repository doesn't snycrhonize yet.)

The below is working messesage

[    0.817081] OF: PCI: host bridge /soc/pcie@15700000 ranges:
[    0.817264] OF: PCI:   No bus range found for /soc/pcie@15700000, using [bus 00-ff]
[    0.821377] OF: PCI:    IO 0x0c001000..0x0c010fff -> 0x00000000
[    0.827270] OF: PCI:   MEM 0x0c011000..0x0ffffffe -> 0x0c011000
[    0.934306] exynos5433-pcie 156b0000.pcie: link up
[    0.934649] exynos5433-pcie 156b0000.pcie: PCI host bridge to bus 0000:00
[    0.934867] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.935045] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    0.935243] pci_bus 0000:00: root bus resource [mem 0x0c011000-0x0ffffffe]
[    0.953719] pci 0000:00:00.0: BAR 8: assigned [mem 0x0c200000-0x0c7fffff]
[    0.953941] pci 0000:01:00.0: BAR 2: assigned [mem 0x0c400000-0x0c7fffff 64bit]
[    0.956672] pci 0000:01:00.0: BAR 0: assigned [mem 0x0c200000-0x0c207fff 64bit]
[    0.963959] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.968368] pci 0000:00:00.0:   bridge window [mem 0x0c200000-0x0c7fffff]
[    0.975241] pcieport 0000:00:00.0: of_irq_parse_pci() failed with rc=-22
[    0.982124] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt

Jaehoon Chung (6):
  phy: exynos-pcie: Add support for Exynos PCIe phy
  Documetation: samsung-phy: add the exynos-pcie-phy binding
  ARM64: dts: exynos5433: add the pcie_phy node for PCIe
  PCI: exynos5433: Add new exynos pci host controller for Exynos5433
  Documentation: pci: add the exynos5433-pcie binding
  ARM64: exynos: add the pcie node for TM2

 .../devicetree/bindings/pci/exynos5433-pcie.txt    |  36 +++
 .../devicetree/bindings/phy/samsung-phy.txt        |  21 ++
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |   7 +
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      |  11 +-
 arch/arm64/boot/dts/exynos/exynos5433.dtsi         |  37 +++
 drivers/pci/host/Kconfig                           |   9 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-exynos5433.c                  | 338 +++++++++++++++++++++
 drivers/phy/Kconfig                                |   9 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-exynos-pcie.c                      | 227 ++++++++++++++
 11 files changed, 695 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
 create mode 100644 drivers/pci/host/pci-exynos5433.c
 create mode 100644 drivers/phy/phy-exynos-pcie.c

-- 
2.10.2

[RFC PATCH 1/6] 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.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/phy/Kconfig           |   9 ++
 drivers/phy/Makefile          |   1 +
 drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/phy/phy-exynos-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index fe00f91..94b0433 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -341,6 +341,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_EXYNOS5433
+	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 a534cf5..586344d 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -38,6 +38,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..0f5eefd
--- /dev/null
+++ b/drivers/phy/phy-exynos-pcie.c
@@ -0,0 +1,227 @@
+/*
+ * 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/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#define PCIE_EXYNOS5433_PMU_PHY_OFFSET		0x730
+#define PCIE_PHY_OFFSET(x)		((x) * 0x4)
+
+/* Sysreg Fsys register offset and bit for Exynos5433 */
+#define PCIE_PHY_MAC_RESET		0x208
+#define PCIE_MAC_RESET_MASK		0xFF
+#define PCIE_MAC_RESET			BIT(4)
+#define PCIE_L1SUB_CM_CON		0x1010
+#define PCIE_REFCLK_GATING_EN		BIT(0)
+#define PCIE_PHY_COMMON_RESET		0x1020
+#define PCIE_PHY_RESET			BIT(0)
+#define PCIE_PHY_GLOBAL_RESET		0x1040
+#define PCIE_GLOBAL_RESET		BIT(0)
+#define PCIE_REFCLK			BIT(1)
+#define PCIE_REFCLK_MASK		0x16
+#define PCIE_APP_REQ_EXIT_L1_MODE	BIT(5)
+
+enum exynos_pcie_phy_data_type {
+	PCIE_PHY_TYPE_EXYNOS5433,
+};
+
+struct exynos_pcie_phy_data {
+	enum exynos_pcie_phy_data_type	ctrl_type;
+	u32	pmureg_offset; /* PMU_REG offset */
+	struct phy_ops	*ops;
+};
+
+/* for Exynos pcie phy */
+struct exynos_pcie_phy {
+	const struct exynos_pcie_phy_data *drv_data;
+	struct regmap *pmureg;
+	struct regmap *fsysreg;
+	void __iomem *phy_base;
+};
+
+static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
+{
+	writel(val, base + offset);
+}
+
+static int exynos_pcie_phy_init(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+
+	if (ep->fsysreg) {
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
+				PCIE_PHY_RESET, 1);
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
+				PCIE_MAC_RESET, 0);
+		/* PHY refclk 24MHz */
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
+				PCIE_REFCLK_MASK, PCIE_REFCLK);
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
+				PCIE_GLOBAL_RESET, 0);
+	}
+
+	exynos_pcie_phy_writel(ep->phy_base, 0x11, PCIE_PHY_OFFSET(0x3));
+
+	/* band gap reference on */
+	exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x20));
+	exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x4b));
+
+	/* jitter tunning */
+	exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x4));
+	exynos_pcie_phy_writel(ep->phy_base, 0x02, PCIE_PHY_OFFSET(0x7));
+	exynos_pcie_phy_writel(ep->phy_base, 0x41, PCIE_PHY_OFFSET(0x21));
+	exynos_pcie_phy_writel(ep->phy_base, 0x7F, PCIE_PHY_OFFSET(0x14));
+	exynos_pcie_phy_writel(ep->phy_base, 0xC0, PCIE_PHY_OFFSET(0x15));
+	exynos_pcie_phy_writel(ep->phy_base, 0x61, PCIE_PHY_OFFSET(0x36));
+
+	/* D0 uninit.. */
+	exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x3D));
+
+	/* 24MHz */
+	exynos_pcie_phy_writel(ep->phy_base, 0x94, PCIE_PHY_OFFSET(0x8));
+	exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x9));
+	exynos_pcie_phy_writel(ep->phy_base, 0x93, PCIE_PHY_OFFSET(0xA));
+	exynos_pcie_phy_writel(ep->phy_base, 0x6B, PCIE_PHY_OFFSET(0xC));
+	exynos_pcie_phy_writel(ep->phy_base, 0xA5, PCIE_PHY_OFFSET(0xF));
+	exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x16));
+	exynos_pcie_phy_writel(ep->phy_base, 0xA3, PCIE_PHY_OFFSET(0x17));
+	exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x1A));
+	exynos_pcie_phy_writel(ep->phy_base, 0x71, PCIE_PHY_OFFSET(0x23));
+	exynos_pcie_phy_writel(ep->phy_base, 0x4C, PCIE_PHY_OFFSET(0x24));
+
+	exynos_pcie_phy_writel(ep->phy_base, 0x0E, PCIE_PHY_OFFSET(0x26));
+	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_OFFSET(0x7));
+	exynos_pcie_phy_writel(ep->phy_base, 0x48, PCIE_PHY_OFFSET(0x43));
+	exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x44));
+	exynos_pcie_phy_writel(ep->phy_base, 0x03, PCIE_PHY_OFFSET(0x45));
+	exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x48));
+	exynos_pcie_phy_writel(ep->phy_base, 0x13, PCIE_PHY_OFFSET(0x54));
+	exynos_pcie_phy_writel(ep->phy_base, 0x04, PCIE_PHY_OFFSET(0x31));
+	exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x32));
+
+	if (ep->fsysreg) {
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
+				PCIE_PHY_RESET, 0);
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
+				PCIE_MAC_RESET_MASK, PCIE_MAC_RESET);
+	}
+
+	return 0;
+}
+
+static int exynos_pcie_phy_power_on(struct phy *phy)
+{
+	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
+
+	if (ep->pmureg) {
+		if (regmap_update_bits(ep->pmureg, ep->drv_data->pmureg_offset,
+					BIT(0), 1))
+			dev_warn(&phy->dev, "Failed to update regmap bit.\n");
+	}
+
+	if (ep->fsysreg) {
+		regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
+				PCIE_APP_REQ_EXIT_L1_MODE, 0);
+		regmap_update_bits(ep->fsysreg, PCIE_L1SUB_CM_CON,
+				PCIE_REFCLK_GATING_EN, 0);
+	}
+
+	return 0;
+}
+
+static struct phy_ops exynos_phy_ops = {
+	.init	= exynos_pcie_phy_init,
+	.power_on = exynos_pcie_phy_power_on,
+};
+
+static const struct exynos_pcie_phy_data exynos5433_pcie_phy_data = {
+	.ctrl_type	= PCIE_PHY_TYPE_EXYNOS5433,
+	.pmureg_offset	= PCIE_EXYNOS5433_PMU_PHY_OFFSET,
+	.ops		= &exynos_phy_ops,
+};
+
+static const struct of_device_id exynos_pcie_phy_match[] = {
+	{
+		.compatible = "samsung,exynos5433-pcie-phy",
+		.data = &exynos5433_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 device_node *np = dev->of_node;
+	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;
+	const struct of_device_id *match;
+
+	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);
+
+	exynos_phy->pmureg = syscon_regmap_lookup_by_phandle(np,
+			"samsung,pmureg-phandle");
+	if (IS_ERR(exynos_phy->pmureg)) {
+		dev_warn(&pdev->dev, "pmureg syscon regmap lookup failed.\n");
+		exynos_phy->pmureg = NULL;
+	}
+
+	match = of_match_node(exynos_pcie_phy_match, pdev->dev.of_node);
+	drv_data = match->data;
+	exynos_phy->drv_data = drv_data;
+
+	exynos_phy->fsysreg = syscon_regmap_lookup_by_phandle(np,
+			"samsung,fsys-sysreg");
+	if (IS_ERR(exynos_phy->fsysreg)) {
+		dev_warn(&pdev->dev, "Fsysreg syscon regmap lookup failed.\n");
+		exynos_phy->fsysreg = NULL;
+	}
+
+	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

[RFC PATCH 2/6] 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>
---
 .../devicetree/bindings/phy/samsung-phy.txt         | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 9872ba8..eb1085e 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -191,3 +191,24 @@ Example:
 		usbdrdphy0 = &usb3_phy0;
 		usbdrdphy1 = &usb3_phy1;
 	};
+
+Samsung Exynos SoC series PCIe PHY controller
+--------------------------------------------------
+Required properties:
+- compatible : Should be set to "samsung,exynos5433-pcie-phy"
+- #phy-cells : must be zero
+- reg : a list of registers usd by phy driver
+
+Optional properites:
+-samsung,pmureg-phandle	- handle to syscon used to control PMU registers
+-samsung,fsys-sysreg	- handle to syscon used to control the system registers
+
+Example:
+	pcie_phy: pcie-phy@15680000 {
+		#phy-cells = <0>;
+		compatible = "samsung,exynos5433-pcie-phy";
+		reg = <0x15680000 0x1000>;
+		samsung,pmureg-phandle = <&pmu_system_controller>;
+		samsung,fsys-sysreg = <&syscon_fsys>;
+		status = "okay";
+	};
-- 
2.10.2

[RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe

[Jaehoon Chung]

To use the generic PHY framework, adds the pcie_phy node.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 64226d5..2a15f18 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -805,6 +805,11 @@
 			reg = <0x145f0000 0x1038>;
 		};
 
+		syscon_fsys: syscon@156f0000 {
+			compatible = "syscon";
+			reg = <0x156f0000 0x1044>;
+		};
+
 		gsc_0: video-scaler@13C00000 {
 			compatible = "samsung,exynos5433-gsc";
 			reg = <0x13c00000 0x1000>;
@@ -1443,6 +1448,15 @@
 				status = "disabled";
 			};
 		};
+
+		pcie_phy: pcie-phy@15680000 {
+			#phy-cells = <0>;
+			compatible = "samsung,exynos5433-pcie-phy";
+			reg = <0x15680000 0x1000>;
+			samsung,pmureg-phandle = <&pmu_system_controller>;
+			samsung,fsys-sysreg = <&syscon_fsys>;
+			status = "okay";
+		};
 	};
 
 	timer: timer {
-- 
2.10.2

[RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433

[Jaehoon Chung]

Exynos5433 supports the PCIe.
This patch adds new pci-exynos5433.c file for Exynos ARM64.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/pci/host/Kconfig          |   9 +
 drivers/pci/host/Makefile         |   1 +
 drivers/pci/host/pci-exynos5433.c | 338 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 348 insertions(+)
 create mode 100644 drivers/pci/host/pci-exynos5433.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index d7e7c0a..3d77d0b 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -60,6 +60,15 @@ config PCI_EXYNOS
 	select PCIEPORTBUS
 	select PCIE_DW
 
+config PCI_EXYNOS5433
+	bool "Samsung Exynos5433 PCIe controller"
+	depends on ARCH_EXYNOS && ARM64
+	depends on PCI_MSI_IRQ_DOMAIN
+	select PCIEPORTBUS
+	select PCIE_DW
+	help
+	  If you want support for Exynos5433 PCIe host controller, say Y.
+
 config PCI_IMX6
 	bool "Freescale i.MX6 PCIe controller"
 	depends on SOC_IMX6Q
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 084cb49..2168de2 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
+obj-$(CONFIG_PCI_EXYNOS5433) += pci-exynos5433.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
diff --git a/drivers/pci/host/pci-exynos5433.c b/drivers/pci/host/pci-exynos5433.c
new file mode 100644
index 0000000..ff254ca
--- /dev/null
+++ b/drivers/pci/host/pci-exynos5433.c
@@ -0,0 +1,338 @@
+/*
+ * PCIe host controller driver for Samsung EXYNOS5433 SoCs
+ *
+ * Copyright (C) 2016 Samsung Electronics Co., Ltd.
+ *		http://www.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/clk.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+#include <linux/mfd/syscon.h>
+#include <linux/phy/phy.h>
+
+#include "pcie-designware.h"
+
+#define to_exynos_pcie(x)	container_of(x, struct exynos_pcie, pp)
+
+/* Pcie structure for Exynos specific data */
+struct exynos_pcie {
+	void __iomem		*elbi_base;
+	struct clk		*clk;
+	struct clk		*bus_clk;
+	struct pcie_port	pp;
+	struct phy		*phy;
+};
+
+/* PCIe ELBI registers */
+#define PCIE_IRQ_PULSE			0x000
+#define IRQ_INTA_ASSERT			BIT(0)
+#define IRQ_INTB_ASSERT			BIT(2)
+#define IRQ_INTC_ASSERT			BIT(4)
+#define IRQ_INTD_ASSERT			BIT(6)
+#define IRQ_INTX_ASSERT	(IRQ_INTA_ASSERT | IRQ_INTB_ASSERT | \
+			IRQ_INTC_ASSERT | IRQ_INTD_ASSERT)
+#define PCIE_IRQ_EN_PULSE		0x00c
+#define PCIE_IRQ_EN_LEVEL		0x010
+#define PCIE_SW_WAKE			0x018
+#define PCIE_BUS_EN			BIT(1)
+#define PCIE_APP_LTSSM_ENABLE		0x02c
+#define PCIE_ELBI_LTSSM_ENABLE		0x1
+#define PCIE_ELBI_DEBUG_L		0x074
+#define PCIE_ELBI_XMLH_LINK_UP		BIT(4)
+#define PCIE_ELBI_SLV_AWMISC		0x11c
+#define PCIE_ELBI_SLV_ARMISC		0x120
+#define PCIE_ELBI_SLV_DBI_ENABLE	BIT(21)
+
+/* DBI register */
+#define PCIE_MISC_CONTROL_1_OFF		0x8BC
+#define DBI_RO_WR_EN			BIT(0)
+
+static inline void exynos_pcie_writel(void __iomem *base, u32 val, u32 offset)
+{
+	writel(val, base + offset);
+}
+
+static inline u32 exynos_pcie_readl(void __iomem *base, u32 offset)
+{
+	return readl(base + offset);
+}
+
+static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
+{
+	u32 val;
+
+	val = exynos_pcie_readl(ep->elbi_base, PCIE_IRQ_PULSE);
+	val &= ~IRQ_INTX_ASSERT;
+	exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_PULSE);
+}
+
+static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
+{
+	exynos_pcie_writel(ep->elbi_base, IRQ_INTX_ASSERT, PCIE_IRQ_EN_PULSE);
+
+	/* Clear PCIE_IRQ_EN_LEVEL register */
+	exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
+}
+
+static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+	struct exynos_pcie *ep = to_exynos_pcie(pp);
+
+	exynos_pcie_clear_irq_pulse(ep);
+
+	return IRQ_HANDLED;
+}
+
+static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
+{
+	u32 val;
+
+	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_AWMISC);
+	if (on)
+		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+	else
+		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
+	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_AWMISC);
+}
+
+static void exynos_pcie_sideband_dbi_r_mode(struct exynos_pcie *ep, bool on)
+{
+	u32 val;
+
+	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_ARMISC);
+	if (on)
+		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+	else
+		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
+	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_ARMISC);
+}
+
+static int exynos_pcie_establish_link(struct exynos_pcie *ep)
+{
+	struct pcie_port *pp = &ep->pp;
+	u32 val;
+
+	if (dw_pcie_link_up(pp)) {
+		dev_info(pp->dev, "Link already up\n");
+		return 0;
+	}
+
+	phy_power_on(ep->phy);
+
+	/* Exynos Pcie assert PHY reset  and init */
+	phy_init(ep->phy);
+
+	val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE);
+	val &= ~PCIE_BUS_EN;
+	exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE);
+
+	/*
+	 * Enable DBI_RO_WR_EN bit.
+	 * - When set to 1, some RO and HWinit bits are wriatble from
+	 *   the local application through the DBI.
+	 */
+	dw_pcie_writel_rc(pp, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
+
+	/* Setup root complex */
+	dw_pcie_setup_rc(pp);
+
+	/* assert LTSSM enable */
+	exynos_pcie_writel(ep->elbi_base, PCIE_ELBI_LTSSM_ENABLE,
+			PCIE_APP_LTSSM_ENABLE);
+
+	return dw_pcie_wait_for_link(pp);
+}
+
+
+static int exynos_pcie_link_up(struct pcie_port *pp)
+{
+	struct exynos_pcie *ep = to_exynos_pcie(pp);
+	u32 val;
+
+	/* Check the Receive Transaction Layer Handler */
+	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_DEBUG_L);
+
+	return (val & PCIE_ELBI_XMLH_LINK_UP);
+}
+
+static void exynos_pcie_host_init(struct pcie_port *pp)
+{
+	struct exynos_pcie *ep = to_exynos_pcie(pp);
+
+	exynos_pcie_enable_irq_pulse(ep);
+	exynos_pcie_establish_link(ep);
+}
+
+static u32 exynos_pcie_readl_rc(struct pcie_port *pp, u32 reg)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	u32 val;
+
+	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
+	val = readl(pp->dbi_base + reg);
+	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
+	return val;
+}
+
+static void exynos_pcie_writel_rc(struct pcie_port *pp, u32 reg, u32 val)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+
+	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
+	writel(val, pp->dbi_base + reg);
+	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
+}
+
+static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
+				u32 *val)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	int ret;
+
+	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
+	ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
+	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
+	return ret;
+}
+
+static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
+				u32 val)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	int ret;
+
+	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
+	ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
+	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
+	return ret;
+}
+
+static struct pcie_host_ops exynos_pcie_host_ops = {
+	.readl_rc = exynos_pcie_readl_rc,
+	.writel_rc = exynos_pcie_writel_rc,
+	.rd_own_conf = exynos_pcie_rd_own_conf,
+	.wr_own_conf = exynos_pcie_wr_own_conf,
+	.host_init = exynos_pcie_host_init,
+	.link_up = exynos_pcie_link_up,
+};
+
+static int __init exynos_pcie_probe(struct platform_device *pdev)
+{
+	struct exynos_pcie *exynos_pcie;
+	struct pcie_port *pp;
+	struct resource *res;
+	int ret;
+
+	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
+			GFP_KERNEL);
+	if (!exynos_pcie)
+		return -ENOMEM;
+
+	pp = &exynos_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
+	if (IS_ERR(exynos_pcie->clk)) {
+		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
+		return PTR_ERR(exynos_pcie->clk);
+	}
+	ret = clk_prepare_enable(exynos_pcie->clk);
+	if (ret)
+		return ret;
+
+	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
+	if (IS_ERR(exynos_pcie->bus_clk)) {
+		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
+		ret = PTR_ERR(exynos_pcie->bus_clk);
+		goto fail_clk;
+	}
+	ret = clk_prepare_enable(exynos_pcie->bus_clk);
+	if (ret)
+		goto fail_clk;
+
+	/* External Local Bus interface(ELBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
+	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(exynos_pcie->elbi_base)) {
+		ret = PTR_ERR(exynos_pcie->elbi_base);
+		goto fail_bus_clk;
+	}
+
+	/* Data Bus Interface(DBI) Register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pp->dbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pp->dbi_base)) {
+		ret = PTR_ERR(pp->dbi_base);
+		goto fail_bus_clk;
+	}
+
+	exynos_pcie->phy = devm_phy_get(&pdev->dev, "pcie-phy");
+	if (IS_ERR(exynos_pcie->phy)) {
+		if (PTR_ERR(exynos_pcie->phy) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Can't find the pcie-phy\n");
+		return PTR_ERR(exynos_pcie->phy);
+	}
+
+	pp->irq = platform_get_irq_byname(pdev, "intr");
+	if (!pp->irq) {
+		dev_err(&pdev->dev, "failed to get irq\n");
+		ret = -ENODEV;
+		goto fail_bus_clk;
+	}
+	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
+				IRQF_SHARED, "exynos-pcie", exynos_pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		goto fail_bus_clk;
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &exynos_pcie_host_ops;
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		goto fail_bus_clk;
+	}
+
+	platform_set_drvdata(pdev, exynos_pcie);
+
+	return 0;
+
+fail_bus_clk:
+	clk_disable_unprepare(exynos_pcie->bus_clk);
+fail_clk:
+	clk_disable_unprepare(exynos_pcie->clk);
+	return ret;
+}
+
+static const struct of_device_id exynos_pcie_of_match[] = {
+	{ .compatible = "samsung,exynos5433-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
+
+static struct platform_driver exynos_pcie_driver = {
+	.probe		= exynos_pcie_probe,
+	.driver		= {
+		.name		= "exynos5433-pcie",
+		.of_match_table = exynos_pcie_of_match,
+	},
+};
+builtin_platform_driver(exynos_pcie_driver);
-- 
2.10.2

[RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding

[Jaehoon Chung]

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 .../devicetree/bindings/pci/exynos5433-pcie.txt    | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/exynos5433-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
new file mode 100644
index 0000000..932a847
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
@@ -0,0 +1,36 @@
+* Samsung Exynos5433 PCIe interface
+
+This PCIe host controller is based on the Synopsis Designware PCIe IP
+and thus inherits all the common properties defined in designware-pcie.txt.
+
+Required properties:
+- compatible: "samsung,exynos5433-pcie"
+- reg: base addresses and lengths of the pcie controller,
+	the phy controller, additional register for the phy controller.
+- reg-names: Must be "elbi", "phy" and "dbi" for each regs
+- interrupt-names: Must be "intr" for legacy interrupt pin.
+
+Other common properites refer to
+	Documentation/devicetree/binding/pci/designware-pcie.txt
+
+Example:
+
+	pcie: pcie@15700000 {
+		compatible ="samsung,exynos5433-pcie", "snps,dw-pcie";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "intr";
+		clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
+		clock-names = "pcie", "pcie_bus";
+		num-lanes = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_bus>;
+		reg = <0x156b0000 0x1000>, <0x15680000 0x1000>,
+		    <0x15700000 0x1000>, <0x0c000000 0x1000>;
+		reg-names = "elbi", "phy", "dbi", "config";
+		ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000
+			  0x82000000 0 0x0c011000 0x0c011000 0 0x3feefff>;
+		status = "disabled";
+	};
-- 
2.10.2

[RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

[Jaehoon Chung]

Add the Exxynos5433 pcie node for TM2.
This pcie device is used for supporting WiFi.

And some gpios are already requested from pinctrl. so it doesn't need to
initialize.
GPJ2-0 is used for supplying to WiFi PCIe chip.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++++++
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 11 +++++++++--
 arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 23 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
index ad71247..3e8b728 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
@@ -183,6 +183,13 @@
 		interrupt-controller;
 		#interrupt-cells = <2>;
 	};
+
+	pcie_wlanen: pcie-wlanen {
+		samsung,pins = "gpj2-0";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <3>;
+		samsung,pin-drv = <3>;
+	};
 };
 
 &pinctrl_finger {
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..c84a2ad 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -737,6 +737,15 @@
 	bus-width = <4>;
 };
 
+&pcie {
+	assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
+			<&cmu_top CLK_MOUT_SCLK_PCIE_100>;
+	assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
+			<&cmu_top CLK_MOUT_BUS_PLL_USER>;
+	assigned-clock-rates = <0>, <100000000>;
+	status = "okay";
+};
+
 &pinctrl_alive {
 	pinctrl-names = "default";
 	pinctrl-0 = <&initial_alive>;
@@ -836,7 +845,6 @@
 	pinctrl-0 = <&initial_ese>;
 
 	initial_ese: initial-state {
-		PIN(IN, gpj2-0, DOWN, LV1);
 		PIN(IN, gpj2-1, DOWN, LV1);
 		PIN(IN, gpj2-2, DOWN, LV1);
 	};
@@ -851,7 +859,6 @@
 		PIN(IN, gpr3-1, DOWN, LV1);
 		PIN(IN, gpr3-2, DOWN, LV1);
 		PIN(IN, gpr3-3, DOWN, LV1);
-		PIN(IN, gpr3-7, NONE, LV1);
 	};
 };
 
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 2a15f18..da287f4 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1457,6 +1457,29 @@
 			samsung,fsys-sysreg = <&syscon_fsys>;
 			status = "okay";
 		};
+
+		pcie: pcie@15700000 {
+			compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "intr";
+			clocks = <&cmu_fsys CLK_PCIE>,
+			       <&cmu_fsys CLK_PCLK_PCIE_PHY>;
+			clock-names = "pcie", "pcie_bus";
+			num-lanes = <1>;
+			pinctrl-names = "default";
+			phys = <&pcie_phy>;
+			phy-names = "pcie-phy";
+			pinctrl-0 = <&pcie_bus &pcie_wlanen>;
+			reg = <0x156b0000 0x1000>, <0x15700000 0x1000>,
+			    <0x0c000000 0x1000>;
+			reg-names = "elbi", "dbi", "config";
+			ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000
+				  0x82000000 0 0x0c011000 0x0c011000 0 0x3feefff>;
+			status = "disabled";
+		};
 	};
 
 	timer: timer {
-- 
2.10.2

Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433

[Joao Pinto]

Hello Jaehoon,

Às 5:20 AM de 12/26/2016, Jaehoon Chung escreveu:
> Exynos5433 supports the PCIe.
> This patch adds new pci-exynos5433.c file for Exynos ARM64.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/pci/host/Kconfig          |   9 +
>  drivers/pci/host/Makefile         |   1 +
>  drivers/pci/host/pci-exynos5433.c | 338 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 348 insertions(+)
>  create mode 100644 drivers/pci/host/pci-exynos5433.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index d7e7c0a..3d77d0b 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -60,6 +60,15 @@ config PCI_EXYNOS
>  	select PCIEPORTBUS
>  	select PCIE_DW
>  
> +config PCI_EXYNOS5433
> +	bool "Samsung Exynos5433 PCIe controller"
> +	depends on ARCH_EXYNOS && ARM64
> +	depends on PCI_MSI_IRQ_DOMAIN
> +	select PCIEPORTBUS
> +	select PCIE_DW
> +	help
> +	  If you want support for Exynos5433 PCIe host controller, say Y.
> +
>  config PCI_IMX6
>  	bool "Freescale i.MX6 PCIe controller"
>  	depends on SOC_IMX6Q
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 084cb49..2168de2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> +obj-$(CONFIG_PCI_EXYNOS5433) += pci-exynos5433.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>  obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
>  obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
> diff --git a/drivers/pci/host/pci-exynos5433.c b/drivers/pci/host/pci-exynos5433.c
> new file mode 100644
> index 0000000..ff254ca
> --- /dev/null
> +++ b/drivers/pci/host/pci-exynos5433.c
> @@ -0,0 +1,338 @@
> +/*
> + * PCIe host controller driver for Samsung EXYNOS5433 SoCs
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
> + *		https://urldefense.proofpoint.com/v2/url?u=http-3A__www.samsung.com&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fUwnZks1U2AwaOxMkLdSnE700sfQXpB3WAg_EJw7NaE&s=IwRYD8maTuXG57Q0qlAmFNh3_TSfUTE27xq8p13FFKI&e= 
> + *
> + * 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/clk.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/resource.h>
> +#include <linux/signal.h>
> +#include <linux/types.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/phy/phy.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_exynos_pcie(x)	container_of(x, struct exynos_pcie, pp)
> +
> +/* Pcie structure for Exynos specific data */
> +struct exynos_pcie {
> +	void __iomem		*elbi_base;
> +	struct clk		*clk;
> +	struct clk		*bus_clk;
> +	struct pcie_port	pp;
> +	struct phy		*phy;
> +};
> +
> +/* PCIe ELBI registers */
> +#define PCIE_IRQ_PULSE			0x000
> +#define IRQ_INTA_ASSERT			BIT(0)
> +#define IRQ_INTB_ASSERT			BIT(2)
> +#define IRQ_INTC_ASSERT			BIT(4)
> +#define IRQ_INTD_ASSERT			BIT(6)
> +#define IRQ_INTX_ASSERT	(IRQ_INTA_ASSERT | IRQ_INTB_ASSERT | \
> +			IRQ_INTC_ASSERT | IRQ_INTD_ASSERT)
> +#define PCIE_IRQ_EN_PULSE		0x00c
> +#define PCIE_IRQ_EN_LEVEL		0x010
> +#define PCIE_SW_WAKE			0x018
> +#define PCIE_BUS_EN			BIT(1)
> +#define PCIE_APP_LTSSM_ENABLE		0x02c
> +#define PCIE_ELBI_LTSSM_ENABLE		0x1
> +#define PCIE_ELBI_DEBUG_L		0x074
> +#define PCIE_ELBI_XMLH_LINK_UP		BIT(4)
> +#define PCIE_ELBI_SLV_AWMISC		0x11c
> +#define PCIE_ELBI_SLV_ARMISC		0x120
> +#define PCIE_ELBI_SLV_DBI_ENABLE	BIT(21)
> +
> +/* DBI register */
> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> +#define DBI_RO_WR_EN			BIT(0)
> +
> +static inline void exynos_pcie_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static inline u32 exynos_pcie_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
> +{
> +	u32 val;
> +
> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_IRQ_PULSE);
> +	val &= ~IRQ_INTX_ASSERT;
> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_PULSE);
> +}
> +
> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> +{
> +	exynos_pcie_writel(ep->elbi_base, IRQ_INTX_ASSERT, PCIE_IRQ_EN_PULSE);
> +
> +	/* Clear PCIE_IRQ_EN_LEVEL register */
> +	exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
> +}
> +
> +static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
> +{
> +	struct pcie_port *pp = arg;
> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
> +
> +	exynos_pcie_clear_irq_pulse(ep);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
> +{
> +	u32 val;
> +
> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_AWMISC);
> +	if (on)
> +		val |= PCIE_ELBI_SLV_DBI_ENABLE;
> +	else
> +		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_AWMISC);
> +}
> +
> +static void exynos_pcie_sideband_dbi_r_mode(struct exynos_pcie *ep, bool on)
> +{
> +	u32 val;
> +
> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_ARMISC);
> +	if (on)
> +		val |= PCIE_ELBI_SLV_DBI_ENABLE;
> +	else
> +		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_ARMISC);
> +}
> +
> +static int exynos_pcie_establish_link(struct exynos_pcie *ep)
> +{
> +	struct pcie_port *pp = &ep->pp;
> +	u32 val;
> +
> +	if (dw_pcie_link_up(pp)) {
> +		dev_info(pp->dev, "Link already up\n");
> +		return 0;
> +	}
> +
> +	phy_power_on(ep->phy);
> +
> +	/* Exynos Pcie assert PHY reset  and init */
> +	phy_init(ep->phy);
> +
> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE);
> +	val &= ~PCIE_BUS_EN;
> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE);
> +
> +	/*
> +	 * Enable DBI_RO_WR_EN bit.
> +	 * - When set to 1, some RO and HWinit bits are wriatble from
> +	 *   the local application through the DBI.
> +	 */
> +	dw_pcie_writel_rc(pp, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
> +
> +	/* Setup root complex */
> +	dw_pcie_setup_rc(pp);
> +
> +	/* assert LTSSM enable */
> +	exynos_pcie_writel(ep->elbi_base, PCIE_ELBI_LTSSM_ENABLE,
> +			PCIE_APP_LTSSM_ENABLE);
> +
> +	return dw_pcie_wait_for_link(pp);
> +}
> +
> +
> +static int exynos_pcie_link_up(struct pcie_port *pp)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
> +	u32 val;
> +
> +	/* Check the Receive Transaction Layer Handler */
> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_DEBUG_L);
> +
> +	return (val & PCIE_ELBI_XMLH_LINK_UP);
> +}
> +
> +static void exynos_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
> +
> +	exynos_pcie_enable_irq_pulse(ep);
> +	exynos_pcie_establish_link(ep);
> +}

Your *_host_init callback should return to pci-designware its error/success
status. There is a recent patch that added this capability to all vendor
specific drivers that use pcie-designware. Please check it out and follow the
same approach:
https://patchwork.kernel.org/patch/9464223/

Thanks,
Joao

> +
> +static u32 exynos_pcie_readl_rc(struct pcie_port *pp, u32 reg)
> +{
> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> +	u32 val;
> +
> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
> +	val = readl(pp->dbi_base + reg);
> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
> +	return val;
> +}
> +
> +static void exynos_pcie_writel_rc(struct pcie_port *pp, u32 reg, u32 val)
> +{
> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> +
> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
> +	writel(val, pp->dbi_base + reg);
> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
> +}
> +
> +static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> +				u32 *val)
> +{
> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> +	int ret;
> +
> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
> +	ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
> +	return ret;
> +}
> +
> +static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> +				u32 val)
> +{
> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> +	int ret;
> +
> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
> +	ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
> +	return ret;
> +}
> +
> +static struct pcie_host_ops exynos_pcie_host_ops = {
> +	.readl_rc = exynos_pcie_readl_rc,
> +	.writel_rc = exynos_pcie_writel_rc,
> +	.rd_own_conf = exynos_pcie_rd_own_conf,
> +	.wr_own_conf = exynos_pcie_wr_own_conf,
> +	.host_init = exynos_pcie_host_init,
> +	.link_up = exynos_pcie_link_up,
> +};
> +
> +static int __init exynos_pcie_probe(struct platform_device *pdev)
> +{
> +	struct exynos_pcie *exynos_pcie;
> +	struct pcie_port *pp;
> +	struct resource *res;
> +	int ret;
> +
> +	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
> +			GFP_KERNEL);
> +	if (!exynos_pcie)
> +		return -ENOMEM;
> +
> +	pp = &exynos_pcie->pp;
> +	pp->dev = &pdev->dev;
> +
> +	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
> +	if (IS_ERR(exynos_pcie->clk)) {
> +		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
> +		return PTR_ERR(exynos_pcie->clk);
> +	}
> +	ret = clk_prepare_enable(exynos_pcie->clk);
> +	if (ret)
> +		return ret;
> +
> +	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
> +	if (IS_ERR(exynos_pcie->bus_clk)) {
> +		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
> +		ret = PTR_ERR(exynos_pcie->bus_clk);
> +		goto fail_clk;
> +	}
> +	ret = clk_prepare_enable(exynos_pcie->bus_clk);
> +	if (ret)
> +		goto fail_clk;
> +
> +	/* External Local Bus interface(ELBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> +	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(exynos_pcie->elbi_base)) {
> +		ret = PTR_ERR(exynos_pcie->elbi_base);
> +		goto fail_bus_clk;
> +	}
> +
> +	/* Data Bus Interface(DBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pp->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(pp->dbi_base)) {
> +		ret = PTR_ERR(pp->dbi_base);
> +		goto fail_bus_clk;
> +	}
> +
> +	exynos_pcie->phy = devm_phy_get(&pdev->dev, "pcie-phy");
> +	if (IS_ERR(exynos_pcie->phy)) {
> +		if (PTR_ERR(exynos_pcie->phy) != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Can't find the pcie-phy\n");
> +		return PTR_ERR(exynos_pcie->phy);
> +	}
> +
> +	pp->irq = platform_get_irq_byname(pdev, "intr");
> +	if (!pp->irq) {
> +		dev_err(&pdev->dev, "failed to get irq\n");
> +		ret = -ENODEV;
> +		goto fail_bus_clk;
> +	}
> +	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
> +				IRQF_SHARED, "exynos-pcie", exynos_pcie);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq\n");
> +		goto fail_bus_clk;
> +	}
> +
> +	pp->root_bus_nr = -1;
> +	pp->ops = &exynos_pcie_host_ops;
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		goto fail_bus_clk;
> +	}
> +
> +	platform_set_drvdata(pdev, exynos_pcie);
> +
> +	return 0;
> +
> +fail_bus_clk:
> +	clk_disable_unprepare(exynos_pcie->bus_clk);
> +fail_clk:
> +	clk_disable_unprepare(exynos_pcie->clk);
> +	return ret;
> +}
> +
> +static const struct of_device_id exynos_pcie_of_match[] = {
> +	{ .compatible = "samsung,exynos5433-pcie", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
> +
> +static struct platform_driver exynos_pcie_driver = {
> +	.probe		= exynos_pcie_probe,
> +	.driver		= {
> +		.name		= "exynos5433-pcie",
> +		.of_match_table = exynos_pcie_of_match,
> +	},
> +};
> +builtin_platform_driver(exynos_pcie_driver);
> 

Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433

[Jaehoon Chung]

On 12/26/2016 08:49 PM, Joao Pinto wrote:
> 
> Hello Jaehoon,
> 
> Às 5:20 AM de 12/26/2016, Jaehoon Chung escreveu:
>> Exynos5433 supports the PCIe.
>> This patch adds new pci-exynos5433.c file for Exynos ARM64.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/pci/host/Kconfig          |   9 +
>>  drivers/pci/host/Makefile         |   1 +
>>  drivers/pci/host/pci-exynos5433.c | 338 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 348 insertions(+)
>>  create mode 100644 drivers/pci/host/pci-exynos5433.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index d7e7c0a..3d77d0b 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -60,6 +60,15 @@ config PCI_EXYNOS
>>  	select PCIEPORTBUS
>>  	select PCIE_DW
>>  
>> +config PCI_EXYNOS5433
>> +	bool "Samsung Exynos5433 PCIe controller"
>> +	depends on ARCH_EXYNOS && ARM64
>> +	depends on PCI_MSI_IRQ_DOMAIN
>> +	select PCIEPORTBUS
>> +	select PCIE_DW
>> +	help
>> +	  If you want support for Exynos5433 PCIe host controller, say Y.
>> +
>>  config PCI_IMX6
>>  	bool "Freescale i.MX6 PCIe controller"
>>  	depends on SOC_IMX6Q
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 084cb49..2168de2 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -2,6 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>> +obj-$(CONFIG_PCI_EXYNOS5433) += pci-exynos5433.o
>>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>  obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
>>  obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
>> diff --git a/drivers/pci/host/pci-exynos5433.c b/drivers/pci/host/pci-exynos5433.c
>> new file mode 100644
>> index 0000000..ff254ca
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-exynos5433.c
>> @@ -0,0 +1,338 @@
>> +/*
>> + * PCIe host controller driver for Samsung EXYNOS5433 SoCs
>> + *
>> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
>> + *		https://urldefense.proofpoint.com/v2/url?u=http-3A__www.samsung.com&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fUwnZks1U2AwaOxMkLdSnE700sfQXpB3WAg_EJw7NaE&s=IwRYD8maTuXG57Q0qlAmFNh3_TSfUTE27xq8p13FFKI&e= 
>> + *
>> + * 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/clk.h>
>> +#include <linux/gpio.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/resource.h>
>> +#include <linux/signal.h>
>> +#include <linux/types.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/phy/phy.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define to_exynos_pcie(x)	container_of(x, struct exynos_pcie, pp)
>> +
>> +/* Pcie structure for Exynos specific data */
>> +struct exynos_pcie {
>> +	void __iomem		*elbi_base;
>> +	struct clk		*clk;
>> +	struct clk		*bus_clk;
>> +	struct pcie_port	pp;
>> +	struct phy		*phy;
>> +};
>> +
>> +/* PCIe ELBI registers */
>> +#define PCIE_IRQ_PULSE			0x000
>> +#define IRQ_INTA_ASSERT			BIT(0)
>> +#define IRQ_INTB_ASSERT			BIT(2)
>> +#define IRQ_INTC_ASSERT			BIT(4)
>> +#define IRQ_INTD_ASSERT			BIT(6)
>> +#define IRQ_INTX_ASSERT	(IRQ_INTA_ASSERT | IRQ_INTB_ASSERT | \
>> +			IRQ_INTC_ASSERT | IRQ_INTD_ASSERT)
>> +#define PCIE_IRQ_EN_PULSE		0x00c
>> +#define PCIE_IRQ_EN_LEVEL		0x010
>> +#define PCIE_SW_WAKE			0x018
>> +#define PCIE_BUS_EN			BIT(1)
>> +#define PCIE_APP_LTSSM_ENABLE		0x02c
>> +#define PCIE_ELBI_LTSSM_ENABLE		0x1
>> +#define PCIE_ELBI_DEBUG_L		0x074
>> +#define PCIE_ELBI_XMLH_LINK_UP		BIT(4)
>> +#define PCIE_ELBI_SLV_AWMISC		0x11c
>> +#define PCIE_ELBI_SLV_ARMISC		0x120
>> +#define PCIE_ELBI_SLV_DBI_ENABLE	BIT(21)
>> +
>> +/* DBI register */
>> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
>> +#define DBI_RO_WR_EN			BIT(0)
>> +
>> +static inline void exynos_pcie_writel(void __iomem *base, u32 val, u32 offset)
>> +{
>> +	writel(val, base + offset);
>> +}
>> +
>> +static inline u32 exynos_pcie_readl(void __iomem *base, u32 offset)
>> +{
>> +	return readl(base + offset);
>> +}
>> +
>> +static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
>> +{
>> +	u32 val;
>> +
>> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_IRQ_PULSE);
>> +	val &= ~IRQ_INTX_ASSERT;
>> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_PULSE);
>> +}
>> +
>> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
>> +{
>> +	exynos_pcie_writel(ep->elbi_base, IRQ_INTX_ASSERT, PCIE_IRQ_EN_PULSE);
>> +
>> +	/* Clear PCIE_IRQ_EN_LEVEL register */
>> +	exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
>> +}
>> +
>> +static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
>> +{
>> +	struct pcie_port *pp = arg;
>> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
>> +
>> +	exynos_pcie_clear_irq_pulse(ep);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
>> +{
>> +	u32 val;
>> +
>> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_AWMISC);
>> +	if (on)
>> +		val |= PCIE_ELBI_SLV_DBI_ENABLE;
>> +	else
>> +		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
>> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_AWMISC);
>> +}
>> +
>> +static void exynos_pcie_sideband_dbi_r_mode(struct exynos_pcie *ep, bool on)
>> +{
>> +	u32 val;
>> +
>> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_ARMISC);
>> +	if (on)
>> +		val |= PCIE_ELBI_SLV_DBI_ENABLE;
>> +	else
>> +		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
>> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_ARMISC);
>> +}
>> +
>> +static int exynos_pcie_establish_link(struct exynos_pcie *ep)
>> +{
>> +	struct pcie_port *pp = &ep->pp;
>> +	u32 val;
>> +
>> +	if (dw_pcie_link_up(pp)) {
>> +		dev_info(pp->dev, "Link already up\n");
>> +		return 0;
>> +	}
>> +
>> +	phy_power_on(ep->phy);
>> +
>> +	/* Exynos Pcie assert PHY reset  and init */
>> +	phy_init(ep->phy);
>> +
>> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE);
>> +	val &= ~PCIE_BUS_EN;
>> +	exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE);
>> +
>> +	/*
>> +	 * Enable DBI_RO_WR_EN bit.
>> +	 * - When set to 1, some RO and HWinit bits are wriatble from
>> +	 *   the local application through the DBI.
>> +	 */
>> +	dw_pcie_writel_rc(pp, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
>> +
>> +	/* Setup root complex */
>> +	dw_pcie_setup_rc(pp);
>> +
>> +	/* assert LTSSM enable */
>> +	exynos_pcie_writel(ep->elbi_base, PCIE_ELBI_LTSSM_ENABLE,
>> +			PCIE_APP_LTSSM_ENABLE);
>> +
>> +	return dw_pcie_wait_for_link(pp);
>> +}
>> +
>> +
>> +static int exynos_pcie_link_up(struct pcie_port *pp)
>> +{
>> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
>> +	u32 val;
>> +
>> +	/* Check the Receive Transaction Layer Handler */
>> +	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_DEBUG_L);
>> +
>> +	return (val & PCIE_ELBI_XMLH_LINK_UP);
>> +}
>> +
>> +static void exynos_pcie_host_init(struct pcie_port *pp)
>> +{
>> +	struct exynos_pcie *ep = to_exynos_pcie(pp);
>> +
>> +	exynos_pcie_enable_irq_pulse(ep);
>> +	exynos_pcie_establish_link(ep);
>> +}
> 
> Your *_host_init callback should return to pci-designware its error/success
> status. There is a recent patch that added this capability to all vendor
> specific drivers that use pcie-designware. Please check it out and follow the
> same approach:
> https://patchwork.kernel.org/patch/9464223/

Ok, I will check it. Thanks for noticing it.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Joao
> 
>> +
>> +static u32 exynos_pcie_readl_rc(struct pcie_port *pp, u32 reg)
>> +{
>> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
>> +	u32 val;
>> +
>> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
>> +	val = readl(pp->dbi_base + reg);
>> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
>> +	return val;
>> +}
>> +
>> +static void exynos_pcie_writel_rc(struct pcie_port *pp, u32 reg, u32 val)
>> +{
>> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
>> +
>> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
>> +	writel(val, pp->dbi_base + reg);
>> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
>> +}
>> +
>> +static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>> +				u32 *val)
>> +{
>> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
>> +	int ret;
>> +
>> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
>> +	ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
>> +	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
>> +	return ret;
>> +}
>> +
>> +static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>> +				u32 val)
>> +{
>> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
>> +	int ret;
>> +
>> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
>> +	ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
>> +	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
>> +	return ret;
>> +}
>> +
>> +static struct pcie_host_ops exynos_pcie_host_ops = {
>> +	.readl_rc = exynos_pcie_readl_rc,
>> +	.writel_rc = exynos_pcie_writel_rc,
>> +	.rd_own_conf = exynos_pcie_rd_own_conf,
>> +	.wr_own_conf = exynos_pcie_wr_own_conf,
>> +	.host_init = exynos_pcie_host_init,
>> +	.link_up = exynos_pcie_link_up,
>> +};
>> +
>> +static int __init exynos_pcie_probe(struct platform_device *pdev)
>> +{
>> +	struct exynos_pcie *exynos_pcie;
>> +	struct pcie_port *pp;
>> +	struct resource *res;
>> +	int ret;
>> +
>> +	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
>> +			GFP_KERNEL);
>> +	if (!exynos_pcie)
>> +		return -ENOMEM;
>> +
>> +	pp = &exynos_pcie->pp;
>> +	pp->dev = &pdev->dev;
>> +
>> +	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
>> +	if (IS_ERR(exynos_pcie->clk)) {
>> +		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
>> +		return PTR_ERR(exynos_pcie->clk);
>> +	}
>> +	ret = clk_prepare_enable(exynos_pcie->clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
>> +	if (IS_ERR(exynos_pcie->bus_clk)) {
>> +		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
>> +		ret = PTR_ERR(exynos_pcie->bus_clk);
>> +		goto fail_clk;
>> +	}
>> +	ret = clk_prepare_enable(exynos_pcie->bus_clk);
>> +	if (ret)
>> +		goto fail_clk;
>> +
>> +	/* External Local Bus interface(ELBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
>> +	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(exynos_pcie->elbi_base)) {
>> +		ret = PTR_ERR(exynos_pcie->elbi_base);
>> +		goto fail_bus_clk;
>> +	}
>> +
>> +	/* Data Bus Interface(DBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>> +	pp->dbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(pp->dbi_base)) {
>> +		ret = PTR_ERR(pp->dbi_base);
>> +		goto fail_bus_clk;
>> +	}
>> +
>> +	exynos_pcie->phy = devm_phy_get(&pdev->dev, "pcie-phy");
>> +	if (IS_ERR(exynos_pcie->phy)) {
>> +		if (PTR_ERR(exynos_pcie->phy) != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev, "Can't find the pcie-phy\n");
>> +		return PTR_ERR(exynos_pcie->phy);
>> +	}
>> +
>> +	pp->irq = platform_get_irq_byname(pdev, "intr");
>> +	if (!pp->irq) {
>> +		dev_err(&pdev->dev, "failed to get irq\n");
>> +		ret = -ENODEV;
>> +		goto fail_bus_clk;
>> +	}
>> +	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
>> +				IRQF_SHARED, "exynos-pcie", exynos_pcie);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to request irq\n");
>> +		goto fail_bus_clk;
>> +	}
>> +
>> +	pp->root_bus_nr = -1;
>> +	pp->ops = &exynos_pcie_host_ops;
>> +
>> +	ret = dw_pcie_host_init(pp);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to initialize host\n");
>> +		goto fail_bus_clk;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, exynos_pcie);
>> +
>> +	return 0;
>> +
>> +fail_bus_clk:
>> +	clk_disable_unprepare(exynos_pcie->bus_clk);
>> +fail_clk:
>> +	clk_disable_unprepare(exynos_pcie->clk);
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id exynos_pcie_of_match[] = {
>> +	{ .compatible = "samsung,exynos5433-pcie", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
>> +
>> +static struct platform_driver exynos_pcie_driver = {
>> +	.probe		= exynos_pcie_probe,
>> +	.driver		= {
>> +		.name		= "exynos5433-pcie",
>> +		.of_match_table = exynos_pcie_of_match,
>> +	},
>> +};
>> +builtin_platform_driver(exynos_pcie_driver);
>>
> 
> --
> 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: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

[Vivek Gautam]

Hi Jaehoon,


On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> 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.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 237 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index fe00f91..94b0433 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -341,6 +341,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"

Is there a reason for this not being 'tristate' ?

> +       depends on ARCH_EXYNOS && OF
> +       depends on PCI_EXYNOS5433
> +       select GENERIC_PHY
> +       help
> +         Enable PCIe PHY support for Exynos SoC series.

If this driver is for Exynos5433, then same should come in this help
text as well.

> +         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 a534cf5..586344d 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -38,6 +38,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..0f5eefd
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,227 @@
> +/*
> + * 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/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>

nit: It's good to have these includes in alphabetical order.

> +
> +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET         0x730
> +#define PCIE_PHY_OFFSET(x)             ((x) * 0x4)
> +
> +/* Sysreg Fsys register offset and bit for Exynos5433 */
> +#define PCIE_PHY_MAC_RESET             0x208
> +#define PCIE_MAC_RESET_MASK            0xFF
> +#define PCIE_MAC_RESET                 BIT(4)
> +#define PCIE_L1SUB_CM_CON              0x1010
> +#define PCIE_REFCLK_GATING_EN          BIT(0)
> +#define PCIE_PHY_COMMON_RESET          0x1020
> +#define PCIE_PHY_RESET                 BIT(0)
> +#define PCIE_PHY_GLOBAL_RESET          0x1040
> +#define PCIE_GLOBAL_RESET              BIT(0)
> +#define PCIE_REFCLK                    BIT(1)
> +#define PCIE_REFCLK_MASK               0x16
> +#define PCIE_APP_REQ_EXIT_L1_MODE      BIT(5)
> +
> +enum exynos_pcie_phy_data_type {
> +       PCIE_PHY_TYPE_EXYNOS5433,
> +};
> +
> +struct exynos_pcie_phy_data {
> +       enum exynos_pcie_phy_data_type  ctrl_type;

Why do we need this controller type ?
If there are changes in the IP between different version,
then you can as well use different compatibles.

> +       u32     pmureg_offset; /* PMU_REG offset */

Please use top comments.

> +       struct phy_ops  *ops;
> +};
> +
> +/* for Exynos pcie phy */
> +struct exynos_pcie_phy {
> +       const struct exynos_pcie_phy_data *drv_data;
> +       struct regmap *pmureg;
> +       struct regmap *fsysreg;
> +       void __iomem *phy_base;

just 'base' ?

> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +       writel(val, base + offset);
> +}
> +
> +static int exynos_pcie_phy_init(struct phy *phy)
> +{
> +       struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +       if (ep->fsysreg) {
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
> +                               PCIE_PHY_RESET, 1);
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
> +                               PCIE_MAC_RESET, 0);
> +               /* PHY refclk 24MHz */
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
> +                               PCIE_REFCLK_MASK, PCIE_REFCLK);
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
> +                               PCIE_GLOBAL_RESET, 0);
> +       }
> +
> +       exynos_pcie_phy_writel(ep->phy_base, 0x11, PCIE_PHY_OFFSET(0x3));
> +
> +       /* band gap reference on */
> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x20));
> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x4b));
> +
> +       /* jitter tunning */
> +       exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x4));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x02, PCIE_PHY_OFFSET(0x7));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x41, PCIE_PHY_OFFSET(0x21));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x7F, PCIE_PHY_OFFSET(0x14));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xC0, PCIE_PHY_OFFSET(0x15));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x61, PCIE_PHY_OFFSET(0x36));
> +
> +       /* D0 uninit.. */
> +       exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x3D));
> +
> +       /* 24MHz */
> +       exynos_pcie_phy_writel(ep->phy_base, 0x94, PCIE_PHY_OFFSET(0x8));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x9));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x93, PCIE_PHY_OFFSET(0xA));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x6B, PCIE_PHY_OFFSET(0xC));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xA5, PCIE_PHY_OFFSET(0xF));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x16));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xA3, PCIE_PHY_OFFSET(0x17));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x1A));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x71, PCIE_PHY_OFFSET(0x23));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x4C, PCIE_PHY_OFFSET(0x24));
> +
> +       exynos_pcie_phy_writel(ep->phy_base, 0x0E, PCIE_PHY_OFFSET(0x26));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_OFFSET(0x7));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x48, PCIE_PHY_OFFSET(0x43));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x44));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x03, PCIE_PHY_OFFSET(0x45));
> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x48));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x13, PCIE_PHY_OFFSET(0x54));
> +       exynos_pcie_phy_writel(ep->phy_base, 0x04, PCIE_PHY_OFFSET(0x31));
> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x32));
> +
> +       if (ep->fsysreg) {
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
> +                               PCIE_PHY_RESET, 0);
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
> +                               PCIE_MAC_RESET_MASK, PCIE_MAC_RESET);
> +       }
> +
> +       return 0;
> +}
> +
> +static int exynos_pcie_phy_power_on(struct phy *phy)
> +{
> +       struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +       if (ep->pmureg) {
> +               if (regmap_update_bits(ep->pmureg, ep->drv_data->pmureg_offset,
> +                                       BIT(0), 1))
> +                       dev_warn(&phy->dev, "Failed to update regmap bit.\n");
> +       }
> +
> +       if (ep->fsysreg) {
> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
> +                               PCIE_APP_REQ_EXIT_L1_MODE, 0);
> +               regmap_update_bits(ep->fsysreg, PCIE_L1SUB_CM_CON,
> +                               PCIE_REFCLK_GATING_EN, 0);
> +       }
> +
> +       return 0;
> +}
> +
> +static struct phy_ops exynos_phy_ops = {

const ?

> +       .init   = exynos_pcie_phy_init,
> +       .power_on = exynos_pcie_phy_power_on,
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5433_pcie_phy_data = {
> +       .ctrl_type      = PCIE_PHY_TYPE_EXYNOS5433,
> +       .pmureg_offset  = PCIE_EXYNOS5433_PMU_PHY_OFFSET,
> +       .ops            = &exynos_phy_ops,
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +       {
> +               .compatible = "samsung,exynos5433-pcie-phy",
> +               .data = &exynos5433_pcie_phy_data,
> +       },
> +       {}

missing comma after braces.

> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       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;
> +       const struct of_device_id *match;
> +
> +       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);
> +
> +       exynos_phy->pmureg = syscon_regmap_lookup_by_phandle(np,
> +                       "samsung,pmureg-phandle");
> +       if (IS_ERR(exynos_phy->pmureg)) {
> +               dev_warn(&pdev->dev, "pmureg syscon regmap lookup failed.\n");
> +               exynos_phy->pmureg = NULL;
> +       }

Is this really optional ? There should be a failure path for IP versions that
require this compulsorily.

> +
> +       match = of_match_node(exynos_pcie_phy_match, pdev->dev.of_node);
> +       drv_data = match->data;

Please use of_device_get_match_data().

> +       exynos_phy->drv_data = drv_data;
> +
> +       exynos_phy->fsysreg = syscon_regmap_lookup_by_phandle(np,
> +                       "samsung,fsys-sysreg");
> +       if (IS_ERR(exynos_phy->fsysreg)) {
> +               dev_warn(&pdev->dev, "Fsysreg syscon regmap lookup failed.\n");
> +               exynos_phy->fsysreg = NULL;
> +       }

Same here, is this optional ?

> +
> +       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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

Re: [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe

[Krzysztof Kozlowski]

On Mon, Dec 26, 2016 at 02:20:26PM +0900, Jaehoon Chung wrote:
> To use the generic PHY framework, adds the pcie_phy node.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 64226d5..2a15f18 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -805,6 +805,11 @@
>  			reg = <0x145f0000 0x1038>;
>  		};
>  
> +		syscon_fsys: syscon@156f0000 {
> +			compatible = "syscon";
> +			reg = <0x156f0000 0x1044>;
> +		};
> +
>  		gsc_0: video-scaler@13C00000 {
>  			compatible = "samsung,exynos5433-gsc";
>  			reg = <0x13c00000 0x1000>;
> @@ -1443,6 +1448,15 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		pcie_phy: pcie-phy@15680000 {
> +			#phy-cells = <0>;
> +			compatible = "samsung,exynos5433-pcie-phy";

Mostly we use the convention of compatible being first property.

> +			reg = <0x15680000 0x1000>;
> +			samsung,pmureg-phandle = <&pmu_system_controller>;
> +			samsung,fsys-sysreg = <&syscon_fsys>;
> +			status = "okay";

Why do you need to put status=okay here?

Best regards,
Krzysztof

> +		};
>  	};
>  
>  	timer: timer {
> -- 
> 2.10.2
> 

Re: [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding

[Krzysztof Kozlowski]

On Mon, Dec 26, 2016 at 02:20:28PM +0900, Jaehoon Chung wrote:
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  .../devicetree/bindings/pci/exynos5433-pcie.txt    | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> new file mode 100644
> index 0000000..932a847
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
> @@ -0,0 +1,36 @@
> +* Samsung Exynos5433 PCIe interface
> +
> +This PCIe host controller is based on the Synopsis Designware PCIe IP

Synopsys.

> +and thus inherits all the common properties defined in designware-pcie.txt.
> +
> +Required properties:
> +- compatible: "samsung,exynos5433-pcie"
> +- reg: base addresses and lengths of the pcie controller,
> +	the phy controller, additional register for the phy controller.

You mentioned three regs but the example contains four of them. Is the
config comming from snps,dw-pcie?

> +- reg-names: Must be "elbi", "phy" and "dbi" for each regs

Again, three here, four in example.

> +- interrupt-names: Must be "intr" for legacy interrupt pin.
> +
> +Other common properites refer to
> +	Documentation/devicetree/binding/pci/designware-pcie.txt
> +
> +Example:
> +
> +	pcie: pcie@15700000 {
> +		compatible ="samsung,exynos5433-pcie", "snps,dw-pcie";
                            ^
			    space needed
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "intr";
> +		clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
> +		clock-names = "pcie", "pcie_bus";
> +		num-lanes = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_bus>;
> +		reg = <0x156b0000 0x1000>, <0x15680000 0x1000>,
> +		    <0x15700000 0x1000>, <0x0c000000 0x1000>;

Indentation here looks wrong. You indented it with spaces after tabs...
but not to align with line before.

Beside that, fine with me:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


> +		reg-names = "elbi", "phy", "dbi", "config";
> +		ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000
> +			  0x82000000 0 0x0c011000 0x0c011000 0 0x3feefff>;
> +		status = "disabled";
> +	};
> -- 
> 2.10.2
> 

Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

[Krzysztof Kozlowski]

On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
> Add the Exxynos5433 pcie node for TM2.
> This pcie device is used for supporting WiFi.
> 
> And some gpios are already requested from pinctrl. so it doesn't need to
> initialize.
> GPJ2-0 is used for supplying to WiFi PCIe chip.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 11 +++++++++--
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 23 ++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> index ad71247..3e8b728 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> @@ -183,6 +183,13 @@
>  		interrupt-controller;
>  		#interrupt-cells = <2>;
>  	};
> +
> +	pcie_wlanen: pcie-wlanen {
> +		samsung,pins = "gpj2-0";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <3>;
> +		samsung,pin-drv = <3>;
> +	};
>  };
>  
>  &pinctrl_finger {
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index f21bdc2..c84a2ad 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -737,6 +737,15 @@
>  	bus-width = <4>;
>  };
>  
> +&pcie {
> +	assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
> +			<&cmu_top CLK_MOUT_SCLK_PCIE_100>;
> +	assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
> +			<&cmu_top CLK_MOUT_BUS_PLL_USER>;
> +	assigned-clock-rates = <0>, <100000000>;
> +	status = "okay";
> +};
> +
>  &pinctrl_alive {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&initial_alive>;
> @@ -836,7 +845,6 @@
>  	pinctrl-0 = <&initial_ese>;
>  
>  	initial_ese: initial-state {
> -		PIN(IN, gpj2-0, DOWN, LV1);
>  		PIN(IN, gpj2-1, DOWN, LV1);
>  		PIN(IN, gpj2-2, DOWN, LV1);
>  	};
> @@ -851,7 +859,6 @@
>  		PIN(IN, gpr3-1, DOWN, LV1);
>  		PIN(IN, gpr3-2, DOWN, LV1);
>  		PIN(IN, gpr3-3, DOWN, LV1);
> -		PIN(IN, gpr3-7, NONE, LV1);
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 2a15f18..da287f4 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -1457,6 +1457,29 @@
>  			samsung,fsys-sysreg = <&syscon_fsys>;
>  			status = "okay";
>  		};
> +
> +		pcie: pcie@15700000 {
> +			compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "intr";
> +			clocks = <&cmu_fsys CLK_PCIE>,
> +			       <&cmu_fsys CLK_PCLK_PCIE_PHY>;

Here and in the 'reg' property - indentation looks weird. Tabs+spaces
but not aligned. Either you use spaces to align... or just don't care
and use tabs. I prefer consistency and below the 'ranges' property is
aligned.

> +			clock-names = "pcie", "pcie_bus";
> +			num-lanes = <1>;
> +			pinctrl-names = "default";
> +			phys = <&pcie_phy>;
> +			phy-names = "pcie-phy";
> +			pinctrl-0 = <&pcie_bus &pcie_wlanen>;
> +			reg = <0x156b0000 0x1000>, <0x15700000 0x1000>,
> +			    <0x0c000000 0x1000>;
> +			reg-names = "elbi", "dbi", "config";

This does not match the bindings documentation.

Best regards,
Krzysztof

Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

[Krzysztof Kozlowski]

On Tue, Dec 27, 2016 at 6:32 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
>> Add the Exxynos5433 pcie node for TM2.
>> This pcie device is used for supporting WiFi.
>>
>> And some gpios are already requested from pinctrl. so it doesn't need to
>> initialize.
>> GPJ2-0 is used for supplying to WiFi PCIe chip.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---


Ahhh, and one more comment - please add missing 'dts' prefix in the subject.

BR,
Krzysztof

Re: [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe

[Jaehoon Chung]

On 12/28/2016 01:11 AM, Krzysztof Kozlowski wrote:
> On Mon, Dec 26, 2016 at 02:20:26PM +0900, Jaehoon Chung wrote:
>> To use the generic PHY framework, adds the pcie_phy node.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> index 64226d5..2a15f18 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> @@ -805,6 +805,11 @@
>>  			reg = <0x145f0000 0x1038>;
>>  		};
>>  
>> +		syscon_fsys: syscon@156f0000 {
>> +			compatible = "syscon";
>> +			reg = <0x156f0000 0x1044>;
>> +		};
>> +
>>  		gsc_0: video-scaler@13C00000 {
>>  			compatible = "samsung,exynos5433-gsc";
>>  			reg = <0x13c00000 0x1000>;
>> @@ -1443,6 +1448,15 @@
>>  				status = "disabled";
>>  			};
>>  		};
>> +
>> +		pcie_phy: pcie-phy@15680000 {
>> +			#phy-cells = <0>;
>> +			compatible = "samsung,exynos5433-pcie-phy";
> 
> Mostly we use the convention of compatible being first property.
> 
>> +			reg = <0x15680000 0x1000>;
>> +			samsung,pmureg-phandle = <&pmu_system_controller>;
>> +			samsung,fsys-sysreg = <&syscon_fsys>;
>> +			status = "okay";
> 
> Why do you need to put status=okay here?

Not need. Will remove.

Best Regards,
Jaehoon Chung

> 
> Best regards,
> Krzysztof
> 
>> +		};
>>  	};
>>  
>>  	timer: timer {
>> -- 
>> 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: [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding

[Jaehoon Chung]

On 12/28/2016 01:19 AM, Krzysztof Kozlowski wrote:
> On Mon, Dec 26, 2016 at 02:20:28PM +0900, Jaehoon Chung wrote:
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  .../devicetree/bindings/pci/exynos5433-pcie.txt    | 36 ++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
>> new file mode 100644
>> index 0000000..932a847
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/exynos5433-pcie.txt
>> @@ -0,0 +1,36 @@
>> +* Samsung Exynos5433 PCIe interface
>> +
>> +This PCIe host controller is based on the Synopsis Designware PCIe IP
> 
> Synopsys.

Will fix.

> 
>> +and thus inherits all the common properties defined in designware-pcie.txt.
>> +
>> +Required properties:
>> +- compatible: "samsung,exynos5433-pcie"
>> +- reg: base addresses and lengths of the pcie controller,
>> +	the phy controller, additional register for the phy controller.
> 
> You mentioned three regs but the example contains four of them. Is the
> config comming from snps,dw-pcie?

Oops..It's my mistake. Just needs to put three reg.
Elbi : External local Bus interface register.
Dbi : Data bus interface register.(Control register.)
Config : for configuration space.

"config" can be removed. Because it's not Exynos specific, synopsys's Required property.

> 
>> +- reg-names: Must be "elbi", "phy" and "dbi" for each regs
> 
> Again, three here, four in example.

Will fix.

> 
>> +- interrupt-names: Must be "intr" for legacy interrupt pin.
>> +
>> +Other common properites refer to
>> +	Documentation/devicetree/binding/pci/designware-pcie.txt
>> +
>> +Example:
>> +
>> +	pcie: pcie@15700000 {
>> +		compatible ="samsung,exynos5433-pcie", "snps,dw-pcie";
>                             ^
> 			    space needed
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +		device_type = "pci";
>> +		interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
>> +		interrupt-names = "intr";
>> +		clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
>> +		clock-names = "pcie", "pcie_bus";
>> +		num-lanes = <1>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pcie_bus>;
>> +		reg = <0x156b0000 0x1000>, <0x15680000 0x1000>,
>> +		    <0x15700000 0x1000>, <0x0c000000 0x1000>;
> 
> Indentation here looks wrong. You indented it with spaces after tabs...
> but not to align with line before.

Will fix.

Best Regards,
Jaehoon Chung

> 
> Beside that, fine with me:
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Best regards,
> Krzysztof
> 
> 
>> +		reg-names = "elbi", "phy", "dbi", "config";
>> +		ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000
>> +			  0x82000000 0 0x0c011000 0x0c011000 0 0x3feefff>;
>> +		status = "disabled";
>> +	};
>> -- 
>> 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: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

[Jaehoon Chung]

On 12/28/2016 01:32 AM, Krzysztof Kozlowski wrote:
> On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
>> Add the Exxynos5433 pcie node for TM2.
>> This pcie device is used for supporting WiFi.
>>
>> And some gpios are already requested from pinctrl. so it doesn't need to
>> initialize.
>> GPJ2-0 is used for supplying to WiFi PCIe chip.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++++++
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 11 +++++++++--
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 23 ++++++++++++++++++++++
>>  3 files changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> index ad71247..3e8b728 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> @@ -183,6 +183,13 @@
>>  		interrupt-controller;
>>  		#interrupt-cells = <2>;
>>  	};
>> +
>> +	pcie_wlanen: pcie-wlanen {
>> +		samsung,pins = "gpj2-0";
>> +		samsung,pin-function = <0>;
>> +		samsung,pin-pud = <3>;
>> +		samsung,pin-drv = <3>;
>> +	};
>>  };
>>  
>>  &pinctrl_finger {
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index f21bdc2..c84a2ad 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -737,6 +737,15 @@
>>  	bus-width = <4>;
>>  };
>>  
>> +&pcie {
>> +	assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
>> +			<&cmu_top CLK_MOUT_SCLK_PCIE_100>;
>> +	assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
>> +			<&cmu_top CLK_MOUT_BUS_PLL_USER>;
>> +	assigned-clock-rates = <0>, <100000000>;
>> +	status = "okay";
>> +};
>> +
>>  &pinctrl_alive {
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&initial_alive>;
>> @@ -836,7 +845,6 @@
>>  	pinctrl-0 = <&initial_ese>;
>>  
>>  	initial_ese: initial-state {
>> -		PIN(IN, gpj2-0, DOWN, LV1);
>>  		PIN(IN, gpj2-1, DOWN, LV1);
>>  		PIN(IN, gpj2-2, DOWN, LV1);
>>  	};
>> @@ -851,7 +859,6 @@
>>  		PIN(IN, gpr3-1, DOWN, LV1);
>>  		PIN(IN, gpr3-2, DOWN, LV1);
>>  		PIN(IN, gpr3-3, DOWN, LV1);
>> -		PIN(IN, gpr3-7, NONE, LV1);
>>  	};
>>  };
>>  
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> index 2a15f18..da287f4 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> @@ -1457,6 +1457,29 @@
>>  			samsung,fsys-sysreg = <&syscon_fsys>;
>>  			status = "okay";
>>  		};
>> +
>> +		pcie: pcie@15700000 {
>> +			compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +			device_type = "pci";
>> +			interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "intr";
>> +			clocks = <&cmu_fsys CLK_PCIE>,
>> +			       <&cmu_fsys CLK_PCLK_PCIE_PHY>;
> 
> Here and in the 'reg' property - indentation looks weird. Tabs+spaces
> but not aligned. Either you use spaces to align... or just don't care
> and use tabs. I prefer consistency and below the 'ranges' property is
> aligned.

Will fix.

> 
>> +			clock-names = "pcie", "pcie_bus";
>> +			num-lanes = <1>;
>> +			pinctrl-names = "default";
>> +			phys = <&pcie_phy>;
>> +			phy-names = "pcie-phy";
>> +			pinctrl-0 = <&pcie_bus &pcie_wlanen>;
>> +			reg = <0x156b0000 0x1000>, <0x15700000 0x1000>,
>> +			    <0x0c000000 0x1000>;
>> +			reg-names = "elbi", "dbi", "config";
> 
> This does not match the bindings documentation.

This is right. Bindings documentation is wrong. :)

And I will put the prefix as "dts" on subject, according to your other comment.

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: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

Hi Vivek,

On 12/27/2016 02:53 PM, Vivek Gautam wrote:
> Hi Jaehoon,
> 
> 
> On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> 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.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/phy/Kconfig           |   9 ++
>>  drivers/phy/Makefile          |   1 +
>>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 237 insertions(+)
>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index fe00f91..94b0433 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -341,6 +341,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"
> 
> Is there a reason for this not being 'tristate' ?

Will change.

> 
>> +       depends on ARCH_EXYNOS && OF
>> +       depends on PCI_EXYNOS5433
>> +       select GENERIC_PHY
>> +       help
>> +         Enable PCIe PHY support for Exynos SoC series.
> 
> If this driver is for Exynos5433, then same should come in this help
> text as well.

will support the other exynos series.
I'm working on refactoring exynos5440 with PHY generic Framework.
Then this drive is not for only Exnyos5433. how about?

> 
>> +         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 a534cf5..586344d 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -38,6 +38,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..0f5eefd
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos-pcie.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * 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/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
> 
> nit: It's good to have these includes in alphabetical order.

Will fix.

> 
>> +
>> +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET         0x730
>> +#define PCIE_PHY_OFFSET(x)             ((x) * 0x4)
>> +
>> +/* Sysreg Fsys register offset and bit for Exynos5433 */
>> +#define PCIE_PHY_MAC_RESET             0x208
>> +#define PCIE_MAC_RESET_MASK            0xFF
>> +#define PCIE_MAC_RESET                 BIT(4)
>> +#define PCIE_L1SUB_CM_CON              0x1010
>> +#define PCIE_REFCLK_GATING_EN          BIT(0)
>> +#define PCIE_PHY_COMMON_RESET          0x1020
>> +#define PCIE_PHY_RESET                 BIT(0)
>> +#define PCIE_PHY_GLOBAL_RESET          0x1040
>> +#define PCIE_GLOBAL_RESET              BIT(0)
>> +#define PCIE_REFCLK                    BIT(1)
>> +#define PCIE_REFCLK_MASK               0x16
>> +#define PCIE_APP_REQ_EXIT_L1_MODE      BIT(5)
>> +
>> +enum exynos_pcie_phy_data_type {
>> +       PCIE_PHY_TYPE_EXYNOS5433,
>> +};
>> +
>> +struct exynos_pcie_phy_data {
>> +       enum exynos_pcie_phy_data_type  ctrl_type;
> 
> Why do we need this controller type ?
> If there are changes in the IP between different version,
> then you can as well use different compatibles.

Do you mean is the using "of_device_is_compatible()"?

> 
>> +       u32     pmureg_offset; /* PMU_REG offset */
> 
> Please use top comments.
> 
>> +       struct phy_ops  *ops;
>> +};
>> +
>> +/* for Exynos pcie phy */
>> +struct exynos_pcie_phy {
>> +       const struct exynos_pcie_phy_data *drv_data;
>> +       struct regmap *pmureg;
>> +       struct regmap *fsysreg;
>> +       void __iomem *phy_base;
> 
> just 'base' ?

Will change

> 
>> +};
>> +
>> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
>> +{
>> +       writel(val, base + offset);
>> +}
>> +
>> +static int exynos_pcie_phy_init(struct phy *phy)
>> +{
>> +       struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +       if (ep->fsysreg) {
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
>> +                               PCIE_PHY_RESET, 1);
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
>> +                               PCIE_MAC_RESET, 0);
>> +               /* PHY refclk 24MHz */
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
>> +                               PCIE_REFCLK_MASK, PCIE_REFCLK);
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
>> +                               PCIE_GLOBAL_RESET, 0);
>> +       }
>> +
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x11, PCIE_PHY_OFFSET(0x3));
>> +
>> +       /* band gap reference on */
>> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x20));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x4b));
>> +
>> +       /* jitter tunning */
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x4));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x02, PCIE_PHY_OFFSET(0x7));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x41, PCIE_PHY_OFFSET(0x21));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x7F, PCIE_PHY_OFFSET(0x14));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xC0, PCIE_PHY_OFFSET(0x15));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x61, PCIE_PHY_OFFSET(0x36));
>> +
>> +       /* D0 uninit.. */
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x3D));
>> +
>> +       /* 24MHz */
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x94, PCIE_PHY_OFFSET(0x8));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x9));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x93, PCIE_PHY_OFFSET(0xA));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x6B, PCIE_PHY_OFFSET(0xC));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xA5, PCIE_PHY_OFFSET(0xF));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x34, PCIE_PHY_OFFSET(0x16));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xA3, PCIE_PHY_OFFSET(0x17));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x1A));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x71, PCIE_PHY_OFFSET(0x23));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x4C, PCIE_PHY_OFFSET(0x24));
>> +
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x0E, PCIE_PHY_OFFSET(0x26));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_OFFSET(0x7));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x48, PCIE_PHY_OFFSET(0x43));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x44, PCIE_PHY_OFFSET(0x44));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x03, PCIE_PHY_OFFSET(0x45));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0xA7, PCIE_PHY_OFFSET(0x48));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x13, PCIE_PHY_OFFSET(0x54));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0x04, PCIE_PHY_OFFSET(0x31));
>> +       exynos_pcie_phy_writel(ep->phy_base, 0, PCIE_PHY_OFFSET(0x32));
>> +
>> +       if (ep->fsysreg) {
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET,
>> +                               PCIE_PHY_RESET, 0);
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET,
>> +                               PCIE_MAC_RESET_MASK, PCIE_MAC_RESET);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int exynos_pcie_phy_power_on(struct phy *phy)
>> +{
>> +       struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> +       if (ep->pmureg) {
>> +               if (regmap_update_bits(ep->pmureg, ep->drv_data->pmureg_offset,
>> +                                       BIT(0), 1))
>> +                       dev_warn(&phy->dev, "Failed to update regmap bit.\n");
>> +       }
>> +
>> +       if (ep->fsysreg) {
>> +               regmap_update_bits(ep->fsysreg, PCIE_PHY_GLOBAL_RESET,
>> +                               PCIE_APP_REQ_EXIT_L1_MODE, 0);
>> +               regmap_update_bits(ep->fsysreg, PCIE_L1SUB_CM_CON,
>> +                               PCIE_REFCLK_GATING_EN, 0);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static struct phy_ops exynos_phy_ops = {
> 
> const ?

Yep.

> 
>> +       .init   = exynos_pcie_phy_init,
>> +       .power_on = exynos_pcie_phy_power_on,
>> +};
>> +
>> +static const struct exynos_pcie_phy_data exynos5433_pcie_phy_data = {
>> +       .ctrl_type      = PCIE_PHY_TYPE_EXYNOS5433,
>> +       .pmureg_offset  = PCIE_EXYNOS5433_PMU_PHY_OFFSET,
>> +       .ops            = &exynos_phy_ops,
>> +};
>> +
>> +static const struct of_device_id exynos_pcie_phy_match[] = {
>> +       {
>> +               .compatible = "samsung,exynos5433-pcie-phy",
>> +               .data = &exynos5433_pcie_phy_data,
>> +       },
>> +       {}
> 
> missing comma after braces.

Will fix.

> 
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
>> +
>> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +       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;
>> +       const struct of_device_id *match;
>> +
>> +       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);
>> +
>> +       exynos_phy->pmureg = syscon_regmap_lookup_by_phandle(np,
>> +                       "samsung,pmureg-phandle");
>> +       if (IS_ERR(exynos_phy->pmureg)) {
>> +               dev_warn(&pdev->dev, "pmureg syscon regmap lookup failed.\n");
>> +               exynos_phy->pmureg = NULL;
>> +       }
> 
> Is this really optional ? There should be a failure path for IP versions that
> require this compulsorily.

Agreed. Will fix.

> 
>> +
>> +       match = of_match_node(exynos_pcie_phy_match, pdev->dev.of_node);
>> +       drv_data = match->data;
> 
> Please use of_device_get_match_data().

Ok.

> 
>> +       exynos_phy->drv_data = drv_data;
>> +
>> +       exynos_phy->fsysreg = syscon_regmap_lookup_by_phandle(np,
>> +                       "samsung,fsys-sysreg");
>> +       if (IS_ERR(exynos_phy->fsysreg)) {
>> +               dev_warn(&pdev->dev, "Fsysreg syscon regmap lookup failed.\n");
>> +               exynos_phy->fsysreg = NULL;
>> +       }
> 
> Same here, is this optional ?

Will fix.

Thanks for reviewing.

Best Regards,
Jaehoon Chung

> 
>> +
>> +       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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Thanks
> Vivek
> 

Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

[Vivek Gautam]

Hi Jaehoon,

On Wed, Dec 28, 2016 at 8:19 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Vivek,
>
> On 12/27/2016 02:53 PM, Vivek Gautam wrote:
>> Hi Jaehoon,
>>
>>
>> On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> 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.
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/phy/Kconfig           |   9 ++
>>>  drivers/phy/Makefile          |   1 +
>>>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 237 insertions(+)
>>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index fe00f91..94b0433 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -341,6 +341,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"
>>
>> Is there a reason for this not being 'tristate' ?
>
> Will change.

I notice that PCI_EXYNOS5433 is bool as well.
If the host has to be 'bool' then it makes sense to have phy
also bool as well. But if PCI_EXYNOS5433 can be made
tristate, then this also changes to tristate.

>
>>
>>> +       depends on ARCH_EXYNOS && OF
>>> +       depends on PCI_EXYNOS5433
>>> +       select GENERIC_PHY
>>> +       help
>>> +         Enable PCIe PHY support for Exynos SoC series.
>>
>> If this driver is for Exynos5433, then same should come in this help
>> text as well.
>
> will support the other exynos series.
> I'm working on refactoring exynos5440 with PHY generic Framework.
> Then this drive is not for only Exnyos5433. how about?

Ok, it's good then. My only concern is 'depends on PCI_EXYNOS5433'
makes it look like it is for EXYNOS5433. I am fine if that changes as well.

[...]

>>> +
>>> +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET         0x730
>>> +#define PCIE_PHY_OFFSET(x)             ((x) * 0x4)
>>> +
>>> +/* Sysreg Fsys register offset and bit for Exynos5433 */
>>> +#define PCIE_PHY_MAC_RESET             0x208
>>> +#define PCIE_MAC_RESET_MASK            0xFF
>>> +#define PCIE_MAC_RESET                 BIT(4)
>>> +#define PCIE_L1SUB_CM_CON              0x1010
>>> +#define PCIE_REFCLK_GATING_EN          BIT(0)
>>> +#define PCIE_PHY_COMMON_RESET          0x1020
>>> +#define PCIE_PHY_RESET                 BIT(0)
>>> +#define PCIE_PHY_GLOBAL_RESET          0x1040
>>> +#define PCIE_GLOBAL_RESET              BIT(0)
>>> +#define PCIE_REFCLK                    BIT(1)
>>> +#define PCIE_REFCLK_MASK               0x16
>>> +#define PCIE_APP_REQ_EXIT_L1_MODE      BIT(5)
>>> +
>>> +enum exynos_pcie_phy_data_type {
>>> +       PCIE_PHY_TYPE_EXYNOS5433,
>>> +};
>>> +
>>> +struct exynos_pcie_phy_data {
>>> +       enum exynos_pcie_phy_data_type  ctrl_type;
>>
>> Why do we need this controller type ?
>> If there are changes in the IP between different version,
>> then you can as well use different compatibles.
>
> Do you mean is the using "of_device_is_compatible()"?

I meant that multiple compatible strings can be added based on the
IP versions. And any IP specific data can be put in the .data field
of  'of_device_id' structure.
If there's more to differentiate between the IP versions at runtime,
you can use of_device_is_compatible().

[...]



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

Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

[Jaehoon Chung]

Hi Vivek,

On 12/28/2016 05:58 PM, Vivek Gautam wrote:
> Hi Jaehoon,
> 
> On Wed, Dec 28, 2016 at 8:19 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Vivek,
>>
>> On 12/27/2016 02:53 PM, Vivek Gautam wrote:
>>> Hi Jaehoon,
>>>
>>>
>>> On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> 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.
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>> ---
>>>>  drivers/phy/Kconfig           |   9 ++
>>>>  drivers/phy/Makefile          |   1 +
>>>>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 237 insertions(+)
>>>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>>>
>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>> index fe00f91..94b0433 100644
>>>> --- a/drivers/phy/Kconfig
>>>> +++ b/drivers/phy/Kconfig
>>>> @@ -341,6 +341,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"
>>>
>>> Is there a reason for this not being 'tristate' ?
>>
>> Will change.
> 
> I notice that PCI_EXYNOS5433 is bool as well.
> If the host has to be 'bool' then it makes sense to have phy
> also bool as well. But if PCI_EXYNOS5433 can be made
> tristate, then this also changes to tristate.

Right. I understood what you said.

> 
>>
>>>
>>>> +       depends on ARCH_EXYNOS && OF
>>>> +       depends on PCI_EXYNOS5433
>>>> +       select GENERIC_PHY
>>>> +       help
>>>> +         Enable PCIe PHY support for Exynos SoC series.
>>>
>>> If this driver is for Exynos5433, then same should come in this help
>>> text as well.
>>
>> will support the other exynos series.
>> I'm working on refactoring exynos5440 with PHY generic Framework.
>> Then this drive is not for only Exnyos5433. how about?
> 
> Ok, it's good then. My only concern is 'depends on PCI_EXYNOS5433'
> makes it look like it is for EXYNOS5433. I am fine if that changes as well.

I will not put PCI_EXYNOS5433, just will use the PCI_EXYNOS.
Because it will be supported only one file as pci-exynos.c

> 
> [...]
> 
>>>> +
>>>> +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET         0x730
>>>> +#define PCIE_PHY_OFFSET(x)             ((x) * 0x4)
>>>> +
>>>> +/* Sysreg Fsys register offset and bit for Exynos5433 */
>>>> +#define PCIE_PHY_MAC_RESET             0x208
>>>> +#define PCIE_MAC_RESET_MASK            0xFF
>>>> +#define PCIE_MAC_RESET                 BIT(4)
>>>> +#define PCIE_L1SUB_CM_CON              0x1010
>>>> +#define PCIE_REFCLK_GATING_EN          BIT(0)
>>>> +#define PCIE_PHY_COMMON_RESET          0x1020
>>>> +#define PCIE_PHY_RESET                 BIT(0)
>>>> +#define PCIE_PHY_GLOBAL_RESET          0x1040
>>>> +#define PCIE_GLOBAL_RESET              BIT(0)
>>>> +#define PCIE_REFCLK                    BIT(1)
>>>> +#define PCIE_REFCLK_MASK               0x16
>>>> +#define PCIE_APP_REQ_EXIT_L1_MODE      BIT(5)
>>>> +
>>>> +enum exynos_pcie_phy_data_type {
>>>> +       PCIE_PHY_TYPE_EXYNOS5433,
>>>> +};
>>>> +
>>>> +struct exynos_pcie_phy_data {
>>>> +       enum exynos_pcie_phy_data_type  ctrl_type;
>>>
>>> Why do we need this controller type ?
>>> If there are changes in the IP between different version,
>>> then you can as well use different compatibles.
>>
>> Do you mean is the using "of_device_is_compatible()"?
> 
> I meant that multiple compatible strings can be added based on the
> IP versions. And any IP specific data can be put in the .data field
> of  'of_device_id' structure.
> If there's more to differentiate between the IP versions at runtime,
> you can use of_device_is_compatible().
> 
> [...]
> 
> 
> 

Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

[Vivek Gautam]

On Wed, Dec 28, 2016 at 3:05 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Vivek,
>
> On 12/28/2016 05:58 PM, Vivek Gautam wrote:
>> Hi Jaehoon,
>>
>> On Wed, Dec 28, 2016 at 8:19 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Vivek,
>>>
>>> On 12/27/2016 02:53 PM, Vivek Gautam wrote:
>>>> Hi Jaehoon,
>>>>
>>>>
>>>> On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> 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.
>>>>>
>>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>> ---
>>>>>  drivers/phy/Kconfig           |   9 ++
>>>>>  drivers/phy/Makefile          |   1 +
>>>>>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 237 insertions(+)
>>>>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>>>>
>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>> index fe00f91..94b0433 100644
>>>>> --- a/drivers/phy/Kconfig
>>>>> +++ b/drivers/phy/Kconfig
>>>>> @@ -341,6 +341,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"
>>>>
>>>> Is there a reason for this not being 'tristate' ?
>>>
>>> Will change.
>>
>> I notice that PCI_EXYNOS5433 is bool as well.
>> If the host has to be 'bool' then it makes sense to have phy
>> also bool as well. But if PCI_EXYNOS5433 can be made
>> tristate, then this also changes to tristate.
>
> Right. I understood what you said.
>
>>
>>>
>>>>
>>>>> +       depends on ARCH_EXYNOS && OF
>>>>> +       depends on PCI_EXYNOS5433
>>>>> +       select GENERIC_PHY
>>>>> +       help
>>>>> +         Enable PCIe PHY support for Exynos SoC series.
>>>>
>>>> If this driver is for Exynos5433, then same should come in this help
>>>> text as well.
>>>
>>> will support the other exynos series.
>>> I'm working on refactoring exynos5440 with PHY generic Framework.
>>> Then this drive is not for only Exnyos5433. how about?
>>
>> Ok, it's good then. My only concern is 'depends on PCI_EXYNOS5433'
>> makes it look like it is for EXYNOS5433. I am fine if that changes as well.
>
> I will not put PCI_EXYNOS5433, just will use the PCI_EXYNOS.
> Because it will be supported only one file as pci-exynos.c

cool then.

[...]

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