* [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-27 5:53 ` Vivek Gautam
2016-12-26 5:20 ` [RFC PATCH 2/6] Documetation: samsung-phy: add the exynos-pcie-phy binding Jaehoon Chung
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-26 5:20 ` [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung
@ 2016-12-27 5:53 ` Vivek Gautam
2016-12-28 2:49 ` Jaehoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Vivek Gautam @ 2016-12-27 5:53 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Bjorn Helgaas, robh+dt, Mark Rutland, Kukjin Kim, krzk, javier,
kishon, Will Deacon, catalin.marinas, CPGS
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-27 5:53 ` Vivek Gautam
@ 2016-12-28 2:49 ` Jaehoon Chung
2016-12-28 8:58 ` Vivek Gautam
0 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-28 2:49 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-pci, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Bjorn Helgaas, robh+dt, Mark Rutland, Kukjin Kim, krzk, javier,
kishon, Will Deacon, catalin.marinas, CPGS
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-28 2:49 ` Jaehoon Chung
@ 2016-12-28 8:58 ` Vivek Gautam
2016-12-28 9:35 ` Jaehoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Vivek Gautam @ 2016-12-28 8:58 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Bjorn Helgaas, robh+dt, Mark Rutland, Kukjin Kim, krzk, javier,
kishon, Will Deacon, catalin.marinas, CPGS
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-28 8:58 ` Vivek Gautam
@ 2016-12-28 9:35 ` Jaehoon Chung
2016-12-28 10:12 ` Vivek Gautam
0 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-28 9:35 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-pci, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Bjorn Helgaas, robh+dt, Mark Rutland, Kukjin Kim, krzk, javier,
kishon, Will Deacon, catalin.marinas, CPGS
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().
>
> [...]
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
2016-12-28 9:35 ` Jaehoon Chung
@ 2016-12-28 10:12 ` Vivek Gautam
0 siblings, 0 replies; 21+ messages in thread
From: Vivek Gautam @ 2016-12-28 10:12 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Bjorn Helgaas, robh+dt, Mark Rutland, Kukjin Kim, krzk, javier,
kishon, Will Deacon, catalin.marinas, CPGS
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 2/6] Documetation: samsung-phy: add the exynos-pcie-phy binding
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
2016-12-26 5:20 ` [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-26 5:20 ` [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe Jaehoon Chung
` (3 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
2016-12-26 5:20 ` [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung
2016-12-26 5:20 ` [RFC PATCH 2/6] Documetation: samsung-phy: add the exynos-pcie-phy binding Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-27 16:11 ` Krzysztof Kozlowski
2016-12-26 5:20 ` [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433 Jaehoon Chung
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe
2016-12-26 5:20 ` [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe Jaehoon Chung
@ 2016-12-27 16:11 ` Krzysztof Kozlowski
2016-12-27 22:57 ` Jaehoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-27 16:11 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe
2016-12-27 16:11 ` Krzysztof Kozlowski
@ 2016-12-27 22:57 ` Jaehoon Chung
0 siblings, 0 replies; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-27 22:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
` (2 preceding siblings ...)
2016-12-26 5:20 ` [RFC PATCH 3/6] ARM64: dts: exynos5433: add the pcie_phy node for PCIe Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-26 11:49 ` Joao Pinto
2016-12-26 5:20 ` [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding Jaehoon Chung
2016-12-26 5:20 ` [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2 Jaehoon Chung
5 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433
2016-12-26 5:20 ` [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433 Jaehoon Chung
@ 2016-12-26 11:49 ` Joao Pinto
2016-12-27 1:43 ` Jaehoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Joao Pinto @ 2016-12-26 11:49 UTC (permalink / raw)
To: Jaehoon Chung, linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs
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);
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433
2016-12-26 11:49 ` Joao Pinto
@ 2016-12-27 1:43 ` Jaehoon Chung
0 siblings, 0 replies; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-27 1:43 UTC (permalink / raw)
To: Joao Pinto, linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
` (3 preceding siblings ...)
2016-12-26 5:20 ` [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller for Exynos5433 Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-27 16:19 ` Krzysztof Kozlowski
2016-12-26 5:20 ` [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2 Jaehoon Chung
5 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding
2016-12-26 5:20 ` [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding Jaehoon Chung
@ 2016-12-27 16:19 ` Krzysztof Kozlowski
2016-12-27 23:03 ` Jaehoon Chung
0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-27 16:19 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding
2016-12-27 16:19 ` Krzysztof Kozlowski
@ 2016-12-27 23:03 ` Jaehoon Chung
0 siblings, 0 replies; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-27 23:03 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
2016-12-26 5:20 ` [RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung
` (4 preceding siblings ...)
2016-12-26 5:20 ` [RFC PATCH 5/6] Documentation: pci: add the exynos5433-pcie binding Jaehoon Chung
@ 2016-12-26 5:20 ` Jaehoon Chung
2016-12-27 16:32 ` Krzysztof Kozlowski
5 siblings, 1 reply; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-26 5:20 UTC (permalink / raw)
To: linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
2016-12-26 5:20 ` [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2 Jaehoon Chung
@ 2016-12-27 16:32 ` Krzysztof Kozlowski
2016-12-27 16:33 ` Krzysztof Kozlowski
2016-12-27 23:05 ` Jaehoon Chung
0 siblings, 2 replies; 21+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-27 16:32 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, krzk, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
2016-12-27 16:32 ` Krzysztof Kozlowski
@ 2016-12-27 16:33 ` Krzysztof Kozlowski
2016-12-27 23:05 ` Jaehoon Chung
1 sibling, 0 replies; 21+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-27 16:33 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, Krzysztof Kozlowski,
Javier Martinez Canillas, kishon, will.deacon, catalin.marinas,
cpgs
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2
2016-12-27 16:32 ` Krzysztof Kozlowski
2016-12-27 16:33 ` Krzysztof Kozlowski
@ 2016-12-27 23:05 ` Jaehoon Chung
1 sibling, 0 replies; 21+ messages in thread
From: Jaehoon Chung @ 2016-12-27 23:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
robh+dt, mark.rutland, kgene, javier, kishon, will.deacon,
catalin.marinas, cpgs
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
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread