2018-08-17 12:34:48

by Alan Douglas

[permalink] [raw]
Subject: [RFC PATCH 2/2] phy: cadence: Add driver for Sierra PHY

Add a Sierra PHY driver with PCIe and USB support.
There are two resets to the PHY, one to enable
the APB interface for programming registers, and
another to enable the PHY itself.

The sequence of operation on startup is to enable
the APB bus, write the PHY registers and then
enable the PHY.

On power-down we simply put the PHY and APB bus in
reset.

Signed-off-by: Alan Douglas <[email protected]>
---
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/cadence/Kconfig | 9 ++
drivers/phy/cadence/Makefile | 3 +
drivers/phy/cadence/cdns-sierra.c | 326 ++++++++++++++++++++++++++++++++++++++
5 files changed, 340 insertions(+)
create mode 100644 drivers/phy/cadence/Kconfig
create mode 100644 drivers/phy/cadence/Makefile
create mode 100644 drivers/phy/cadence/cdns-sierra.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5c8d452..cc47f85 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -43,6 +43,7 @@ config PHY_XGENE
source "drivers/phy/allwinner/Kconfig"
source "drivers/phy/amlogic/Kconfig"
source "drivers/phy/broadcom/Kconfig"
+source "drivers/phy/cadence/Kconfig"
source "drivers/phy/hisilicon/Kconfig"
source "drivers/phy/lantiq/Kconfig"
source "drivers/phy/marvell/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 84e3bd9..ba48acd 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_RENESAS) += renesas/
obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-y += broadcom/ \
+ cadence/ \
hisilicon/ \
marvell/ \
motorola/ \
diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
new file mode 100644
index 0000000..6bbe5a2
--- /dev/null
+++ b/drivers/phy/cadence/Kconfig
@@ -0,0 +1,9 @@
+#
+# Phy drivers for cadence devices
+#
+config CDNS_SIERRA_PHY
+ tristate "CDNS Sierra PHY Driver"
+ depends on OF && HAS_IOMEM
+ select GENERIC_PHY
+ help
+ Enable this to support the Cadence Sierra PHY driver
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
new file mode 100644
index 0000000..b384cb3
--- /dev/null
+++ b/drivers/phy/cadence/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_CDNS_SIERRA_PHY) += cdns-sierra.o
+
diff --git a/drivers/phy/cadence/cdns-sierra.c b/drivers/phy/cadence/cdns-sierra.c
new file mode 100644
index 0000000..0581516
--- /dev/null
+++ b/drivers/phy/cadence/cdns-sierra.c
@@ -0,0 +1,326 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Cadence Sierra PHY Driver
+ *
+ * Copyright (c) 2018 Cadence Design Systems
+ * Author: Alan Douglas <[email protected]>
+ *
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <dt-bindings/phy/phy.h>
+
+#define CDNS_PHY_PLL_CFG (0xc00e << 2)
+#define CDNS_DET_STANDEC_A (0x4000 << 2)
+#define CDNS_DET_STANDEC_B (0x4001 << 2)
+#define CDNS_DET_STANDEC_C (0x4002 << 2)
+#define CDNS_DET_STANDEC_D (0x4003 << 2)
+#define CDNS_DET_STANDEC_E (0x4004 << 2)
+#define CDNS_PSM_LANECAL (0x4008 << 2)
+#define CDNS_PSM_DIAG (0x4015 << 2)
+#define CDNS_PSC_TX_A0 (0x4028 << 2)
+#define CDNS_PSC_TX_A1 (0x4029 << 2)
+#define CDNS_PSC_TX_A2 (0x402A << 2)
+#define CDNS_PSC_TX_A3 (0x402B << 2)
+#define CDNS_PSC_RX_A0 (0x4030 << 2)
+#define CDNS_PSC_RX_A1 (0x4031 << 2)
+#define CDNS_PSC_RX_A2 (0x4032 << 2)
+#define CDNS_PSC_RX_A3 (0x4033 << 2)
+#define CDNS_PLLCTRL_SUBRATE (0x403A << 2)
+#define CDNS_PLLCTRL_GEN_D (0x403E << 2)
+#define CDNS_DRVCTRL_ATTEN (0x406A << 2)
+#define CDNS_CLKPATHCTRL_TMR (0x4081 << 2)
+#define CDNS_RX_CREQ_FLTR_A_MODE1 (0x4087 << 2)
+#define CDNS_RX_CREQ_FLTR_A_MODE0 (0x4088 << 2)
+#define CDNS_CREQ_CCLKDET_MODE01 (0x408E << 2)
+#define CDNS_RX_CTLE_MAINTENANCE (0x4091 << 2)
+#define CDNS_CREQ_FSMCLK_SEL (0x4092 << 2)
+#define CDNS_CTLELUT_CTRL (0x4098 << 2)
+#define CDNS_DFE_ECMP_RATESEL (0x40C0 << 2)
+#define CDNS_DFE_SMP_RATESEL (0x40C1 << 2)
+#define CDNS_DEQ_VGATUNE_CTRL (0x40E1 << 2)
+#define CDNS_TMRVAL_MODE3 (0x416E << 2)
+#define CDNS_TMRVAL_MODE2 (0x416F << 2)
+#define CDNS_TMRVAL_MODE1 (0x4170 << 2)
+#define CDNS_TMRVAL_MODE0 (0x4171 << 2)
+#define CDNS_PICNT_MODE1 (0x4174 << 2)
+#define CDNS_CPI_OUTBUF_RATESEL (0x417C << 2)
+#define CDNS_LFPSFILT_NS (0x418A << 2)
+#define CDNS_LFPSFILT_RD (0x418B << 2)
+#define CDNS_LFPSFILT_MP (0x418C << 2)
+#define CDNS_SDFILT_H2L_A (0x4191 << 2)
+
+#define SIERRA_PHYS_NUM 2
+
+struct cdns_phy_instance {
+ struct phy *phy;
+ u32 phy_type;
+ u32 nlanes;
+ void __iomem *base;
+};
+
+struct cdns_sierra_phy {
+ struct device *dev;
+ void __iomem *base;
+ u32 *init_data;
+ struct cdns_phy_instance phys[SIERRA_PHYS_NUM];
+ struct reset_control *reset;
+ struct reset_control *apb_reset;
+ struct clk *clk;
+ struct phy *pcie_phy;
+};
+
+static void cdns_sierra_pcie_on(struct cdns_phy_instance *ins)
+{
+ int i;
+
+ for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
+ writel(0x891f, ins->base + CDNS_DET_STANDEC_D + i);
+ writel(0x0053, ins->base + CDNS_DET_STANDEC_E + i);
+ writel(0x0400, ins->base + CDNS_TMRVAL_MODE2 + i);
+ writel(0x0200, ins->base + CDNS_TMRVAL_MODE3 + i);
+ }
+}
+
+static void cdns_sierra_usb_on(struct cdns_phy_instance *ins)
+{
+ int i;
+
+ writel(0x0000, ins->base + CDNS_PHY_PLL_CFG);
+ for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
+ writel(0xFE0A, ins->base + CDNS_DET_STANDEC_A + i);
+ writel(0x000F, ins->base + CDNS_DET_STANDEC_B + i);
+ writel(0x55A5, ins->base + CDNS_DET_STANDEC_C + i);
+ writel(0x69AD, ins->base + CDNS_DET_STANDEC_D + i);
+ writel(0x0241, ins->base + CDNS_DET_STANDEC_E + i);
+ writel(0x0110, ins->base + CDNS_PSM_LANECAL + i);
+ writel(0xCF00, ins->base + CDNS_PSM_DIAG + i);
+ writel(0x001F, ins->base + CDNS_PSC_TX_A0 + i);
+ writel(0x0007, ins->base + CDNS_PSC_TX_A1 + i);
+ writel(0x0003, ins->base + CDNS_PSC_TX_A2 + i);
+ writel(0x0003, ins->base + CDNS_PSC_TX_A3 + i);
+ writel(0x0FFF, ins->base + CDNS_PSC_RX_A0 + i);
+ writel(0x0003, ins->base + CDNS_PSC_RX_A1 + i);
+ writel(0x0003, ins->base + CDNS_PSC_RX_A2 + i);
+ writel(0x0001, ins->base + CDNS_PSC_RX_A3 + i);
+ writel(0x0001, ins->base + CDNS_PLLCTRL_SUBRATE + i);
+ writel(0x0406, ins->base + CDNS_PLLCTRL_GEN_D + i);
+ writel(0x0000, ins->base + CDNS_DRVCTRL_ATTEN + i);
+ writel(0x823E, ins->base + CDNS_CLKPATHCTRL_TMR + i);
+ writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE1 + i);
+ writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE0 + i);
+ writel(0x7B3C, ins->base + CDNS_CREQ_CCLKDET_MODE01 + i);
+ writel(0x023C, ins->base + CDNS_RX_CTLE_MAINTENANCE + i);
+ writel(0x3232, ins->base + CDNS_CREQ_FSMCLK_SEL + i);
+ writel(0x8452, ins->base + CDNS_CTLELUT_CTRL + i);
+ writel(0x4121, ins->base + CDNS_DFE_ECMP_RATESEL + i);
+ writel(0x4121, ins->base + CDNS_DFE_SMP_RATESEL + i);
+ writel(0x9999, ins->base + CDNS_DEQ_VGATUNE_CTRL + i);
+ writel(0x0330, ins->base + CDNS_TMRVAL_MODE0 + i);
+ writel(0x01FF, ins->base + CDNS_PICNT_MODE1 + i);
+ writel(0x0009, ins->base + CDNS_CPI_OUTBUF_RATESEL + i);
+ writel(0x000F, ins->base + CDNS_LFPSFILT_NS + i);
+ writel(0x0009, ins->base + CDNS_LFPSFILT_RD + i);
+ writel(0x0001, ins->base + CDNS_LFPSFILT_MP + i);
+ writel(0x8013, ins->base + CDNS_SDFILT_H2L_A + i);
+ writel(0x0400, ins->base + CDNS_TMRVAL_MODE1 + i);
+ }
+}
+
+static int cdns_sierra_phy_on(struct phy *x)
+{
+ struct cdns_phy_instance *ins = phy_get_drvdata(x);
+ struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
+ int ret;
+
+ ret = clk_prepare_enable(phy->clk);
+ if (ret)
+ return ret;
+
+ /* Ensure the PHY is in reset */
+ reset_control_assert(phy->reset);
+
+ /* Enable APB */
+ reset_control_deassert(phy->apb_reset);
+
+ /* Program the PHY registers*/
+ if (ins->phy_type == PHY_TYPE_PCIE)
+ cdns_sierra_pcie_on(ins);
+ else
+ cdns_sierra_usb_on(ins);
+
+ /* Take the PHY out of reset */
+ ret = reset_control_assert(phy->reset);
+
+ if (ret) {
+ clk_disable_unprepare(phy->clk);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int cdns_sierra_phy_off(struct phy *x)
+{
+ struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
+ int ret = 0;
+
+ reset_control_assert(phy->reset);
+ reset_control_assert(phy->apb_reset);
+ clk_disable_unprepare(phy->clk);
+ dev_info(phy->dev, "sierra PHY OFF\n");
+
+ return ret;
+}
+
+static const struct phy_ops ops = {
+ .power_on = cdns_sierra_phy_on,
+ .power_off = cdns_sierra_phy_off,
+ .owner = THIS_MODULE,
+};
+
+static const struct of_device_id cdns_sierra_id_table[];
+static struct phy *cdns_sierra_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct cdns_sierra_phy *sphy = dev_get_drvdata(dev);
+ struct cdns_phy_instance *ins = NULL;
+ int i;
+
+ if (args->args_count != 2) {
+ dev_err(dev, "invalid number of cells in 'phy' property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (WARN_ON(args->args[1] == 0 || args->args[1] > 4))
+ return ERR_PTR(-ENODEV);
+
+ for (i = 0; i < SIERRA_PHYS_NUM; i++) {
+ if (sphy->phys[i].phy_type == args->args[0])
+ ins = &sphy->phys[i];
+ }
+
+ if (!ins) {
+ dev_err(dev, "failed to find appropriate phy\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ ins->nlanes = args->args[1];
+
+ return ins->phy;
+}
+
+static int cdns_sierra_phy_probe(struct platform_device *pdev)
+{
+ struct cdns_sierra_phy *sphy;
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ const struct of_device_id *match;
+ struct resource *res;
+ int i;
+
+ sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
+ if (!sphy)
+ return -ENOMEM;
+
+ sphy->dev = dev;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
+ sphy->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(sphy->base)) {
+ dev_err(dev, "missing \"reg\"\n");
+ return PTR_ERR(sphy->base);
+ }
+
+ /* Get init data for this PHY */
+ match = of_match_device(cdns_sierra_id_table, dev);
+ if (!match)
+ return -EINVAL;
+ sphy->init_data = (u32 *)match->data;
+
+ /* Check that PHY is present */
+ if (sphy->init_data[0] != readl(sphy->base))
+ return -EINVAL;
+
+ platform_set_drvdata(pdev, sphy);
+
+ sphy->clk = devm_clk_get(dev, "phy_clk");
+ if (IS_ERR(sphy->clk)) {
+ dev_err(dev, "failed to get clock phy_clk\n");
+ return PTR_ERR(sphy->clk);
+ }
+
+ sphy->reset = devm_reset_control_get(dev, "sierra_reset");
+ if (IS_ERR(sphy->reset)) {
+ dev_err(dev, "failed to get reset\n");
+ return PTR_ERR(sphy->reset);
+ }
+
+ sphy->apb_reset = devm_reset_control_get(dev, "sierra_apb");
+ if (IS_ERR(sphy->apb_reset)) {
+ dev_err(dev, "failed to get apb reset\n");
+ return PTR_ERR(sphy->apb_reset);
+ }
+
+ sphy->phys[0].phy_type = PHY_TYPE_PCIE;
+ sphy->phys[1].phy_type = PHY_TYPE_USB3;
+ for (i = 0; i < SIERRA_PHYS_NUM; i++) {
+ struct phy *gphy = devm_phy_create(dev, NULL, &ops);
+
+ if (IS_ERR(gphy))
+ return PTR_ERR(gphy);
+ sphy->phys[i].phy = gphy;
+ sphy->phys[i].base = sphy->base;
+ phy_set_drvdata(gphy, &sphy->phys[i]);
+ }
+
+ pm_runtime_enable(dev);
+ phy_provider = devm_of_phy_provider_register(dev, cdns_sierra_xlate);
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int cdns_sierra_phy_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static int cdns_map_sierra[] = {
+ 0x00007364,
+};
+
+static const struct of_device_id cdns_sierra_id_table[] = {
+ {
+ .compatible = "cdns,sierra-phy",
+ .data = cdns_map_sierra,
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, cdns_sierra_id_table);
+
+static struct platform_driver cdns_sierra_driver = {
+ .probe = cdns_sierra_phy_probe,
+ .remove = cdns_sierra_phy_remove,
+ .driver = {
+ .name = "cdns-sierra-phy",
+ .of_match_table = cdns_sierra_id_table,
+ },
+};
+module_platform_driver(cdns_sierra_driver);
+
+MODULE_ALIAS("platform:cdns_sierra");
+MODULE_AUTHOR("Cadence Design Systems");
+MODULE_DESCRIPTION("CDNS sierra phy driver");
+MODULE_LICENSE("GPL v2");
--
1.9.0



2018-08-20 13:12:03

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] phy: cadence: Add driver for Sierra PHY

Hi,

On Friday 17 August 2018 06:01 PM, Alan Douglas wrote:
> Add a Sierra PHY driver with PCIe and USB support.
> There are two resets to the PHY, one to enable
> the APB interface for programming registers, and
> another to enable the PHY itself.
>
> The sequence of operation on startup is to enable
> the APB bus, write the PHY registers and then
> enable the PHY.
>
> On power-down we simply put the PHY and APB bus in
> reset.
>
> Signed-off-by: Alan Douglas <[email protected]>
> ---
> drivers/phy/Kconfig | 1 +
> drivers/phy/Makefile | 1 +
> drivers/phy/cadence/Kconfig | 9 ++
> drivers/phy/cadence/Makefile | 3 +
> drivers/phy/cadence/cdns-sierra.c | 326 ++++++++++++++++++++++++++++++++++++++
> 5 files changed, 340 insertions(+)
> create mode 100644 drivers/phy/cadence/Kconfig
> create mode 100644 drivers/phy/cadence/Makefile
> create mode 100644 drivers/phy/cadence/cdns-sierra.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 5c8d452..cc47f85 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -43,6 +43,7 @@ config PHY_XGENE
> source "drivers/phy/allwinner/Kconfig"
> source "drivers/phy/amlogic/Kconfig"
> source "drivers/phy/broadcom/Kconfig"
> +source "drivers/phy/cadence/Kconfig"
> source "drivers/phy/hisilicon/Kconfig"
> source "drivers/phy/lantiq/Kconfig"
> source "drivers/phy/marvell/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 84e3bd9..ba48acd 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_RENESAS) += renesas/
> obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
> obj-$(CONFIG_ARCH_TEGRA) += tegra/
> obj-y += broadcom/ \
> + cadence/ \
> hisilicon/ \
> marvell/ \
> motorola/ \
> diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> new file mode 100644
> index 0000000..6bbe5a2
> --- /dev/null
> +++ b/drivers/phy/cadence/Kconfig
> @@ -0,0 +1,9 @@
> +#
> +# Phy drivers for cadence devices
> +#
> +config CDNS_SIERRA_PHY
> + tristate "CDNS Sierra PHY Driver"
> + depends on OF && HAS_IOMEM

depends on RESET_CONTROLLER?
> + select GENERIC_PHY
> + help
> + Enable this to support the Cadence Sierra PHY driver
> diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> new file mode 100644
> index 0000000..b384cb3
> --- /dev/null
> +++ b/drivers/phy/cadence/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_CDNS_SIERRA_PHY) += cdns-sierra.o
> +
> diff --git a/drivers/phy/cadence/cdns-sierra.c b/drivers/phy/cadence/cdns-sierra.c
> new file mode 100644
> index 0000000..0581516
> --- /dev/null
> +++ b/drivers/phy/cadence/cdns-sierra.c
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence Sierra PHY Driver
> + *
> + * Copyright (c) 2018 Cadence Design Systems
> + * Author: Alan Douglas <[email protected]>
> + *
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <dt-bindings/phy/phy.h>
> +
> +#define CDNS_PHY_PLL_CFG (0xc00e << 2)
> +#define CDNS_DET_STANDEC_A (0x4000 << 2)
> +#define CDNS_DET_STANDEC_B (0x4001 << 2)
> +#define CDNS_DET_STANDEC_C (0x4002 << 2)
> +#define CDNS_DET_STANDEC_D (0x4003 << 2)
> +#define CDNS_DET_STANDEC_E (0x4004 << 2)
> +#define CDNS_PSM_LANECAL (0x4008 << 2)
> +#define CDNS_PSM_DIAG (0x4015 << 2)
> +#define CDNS_PSC_TX_A0 (0x4028 << 2)
> +#define CDNS_PSC_TX_A1 (0x4029 << 2)
> +#define CDNS_PSC_TX_A2 (0x402A << 2)
> +#define CDNS_PSC_TX_A3 (0x402B << 2)
> +#define CDNS_PSC_RX_A0 (0x4030 << 2)
> +#define CDNS_PSC_RX_A1 (0x4031 << 2)
> +#define CDNS_PSC_RX_A2 (0x4032 << 2)
> +#define CDNS_PSC_RX_A3 (0x4033 << 2)
> +#define CDNS_PLLCTRL_SUBRATE (0x403A << 2)
> +#define CDNS_PLLCTRL_GEN_D (0x403E << 2)
> +#define CDNS_DRVCTRL_ATTEN (0x406A << 2)
> +#define CDNS_CLKPATHCTRL_TMR (0x4081 << 2)
> +#define CDNS_RX_CREQ_FLTR_A_MODE1 (0x4087 << 2)
> +#define CDNS_RX_CREQ_FLTR_A_MODE0 (0x4088 << 2)
> +#define CDNS_CREQ_CCLKDET_MODE01 (0x408E << 2)
> +#define CDNS_RX_CTLE_MAINTENANCE (0x4091 << 2)
> +#define CDNS_CREQ_FSMCLK_SEL (0x4092 << 2)
> +#define CDNS_CTLELUT_CTRL (0x4098 << 2)
> +#define CDNS_DFE_ECMP_RATESEL (0x40C0 << 2)
> +#define CDNS_DFE_SMP_RATESEL (0x40C1 << 2)
> +#define CDNS_DEQ_VGATUNE_CTRL (0x40E1 << 2)
> +#define CDNS_TMRVAL_MODE3 (0x416E << 2)
> +#define CDNS_TMRVAL_MODE2 (0x416F << 2)
> +#define CDNS_TMRVAL_MODE1 (0x4170 << 2)
> +#define CDNS_TMRVAL_MODE0 (0x4171 << 2)
> +#define CDNS_PICNT_MODE1 (0x4174 << 2)
> +#define CDNS_CPI_OUTBUF_RATESEL (0x417C << 2)
> +#define CDNS_LFPSFILT_NS (0x418A << 2)
> +#define CDNS_LFPSFILT_RD (0x418B << 2)
> +#define CDNS_LFPSFILT_MP (0x418C << 2)
> +#define CDNS_SDFILT_H2L_A (0x4191 << 2)
> +
> +#define SIERRA_PHYS_NUM 2
> +
> +struct cdns_phy_instance {
> + struct phy *phy;
> + u32 phy_type;
> + u32 nlanes;
> + void __iomem *base;
> +};
> +
> +struct cdns_sierra_phy {
> + struct device *dev;
> + void __iomem *base;
> + u32 *init_data;
> + struct cdns_phy_instance phys[SIERRA_PHYS_NUM];
> + struct reset_control *reset;
> + struct reset_control *apb_reset;
> + struct clk *clk;
> + struct phy *pcie_phy;
> +};
> +
> +static void cdns_sierra_pcie_on(struct cdns_phy_instance *ins)
> +{
> + int i;
> +
> + for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
> + writel(0x891f, ins->base + CDNS_DET_STANDEC_D + i);
> + writel(0x0053, ins->base + CDNS_DET_STANDEC_E + i);
> + writel(0x0400, ins->base + CDNS_TMRVAL_MODE2 + i);
> + writel(0x0200, ins->base + CDNS_TMRVAL_MODE3 + i);
> + }
> +}
> +
> +static void cdns_sierra_usb_on(struct cdns_phy_instance *ins)
> +{
> + int i;
> +
> + writel(0x0000, ins->base + CDNS_PHY_PLL_CFG);
> + for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
> + writel(0xFE0A, ins->base + CDNS_DET_STANDEC_A + i);
> + writel(0x000F, ins->base + CDNS_DET_STANDEC_B + i);
> + writel(0x55A5, ins->base + CDNS_DET_STANDEC_C + i);
> + writel(0x69AD, ins->base + CDNS_DET_STANDEC_D + i);
> + writel(0x0241, ins->base + CDNS_DET_STANDEC_E + i);
> + writel(0x0110, ins->base + CDNS_PSM_LANECAL + i);
> + writel(0xCF00, ins->base + CDNS_PSM_DIAG + i);
> + writel(0x001F, ins->base + CDNS_PSC_TX_A0 + i);
> + writel(0x0007, ins->base + CDNS_PSC_TX_A1 + i);
> + writel(0x0003, ins->base + CDNS_PSC_TX_A2 + i);
> + writel(0x0003, ins->base + CDNS_PSC_TX_A3 + i);
> + writel(0x0FFF, ins->base + CDNS_PSC_RX_A0 + i);
> + writel(0x0003, ins->base + CDNS_PSC_RX_A1 + i);
> + writel(0x0003, ins->base + CDNS_PSC_RX_A2 + i);
> + writel(0x0001, ins->base + CDNS_PSC_RX_A3 + i);
> + writel(0x0001, ins->base + CDNS_PLLCTRL_SUBRATE + i);
> + writel(0x0406, ins->base + CDNS_PLLCTRL_GEN_D + i);
> + writel(0x0000, ins->base + CDNS_DRVCTRL_ATTEN + i);
> + writel(0x823E, ins->base + CDNS_CLKPATHCTRL_TMR + i);
> + writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE1 + i);
> + writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE0 + i);
> + writel(0x7B3C, ins->base + CDNS_CREQ_CCLKDET_MODE01 + i);
> + writel(0x023C, ins->base + CDNS_RX_CTLE_MAINTENANCE + i);
> + writel(0x3232, ins->base + CDNS_CREQ_FSMCLK_SEL + i);
> + writel(0x8452, ins->base + CDNS_CTLELUT_CTRL + i);
> + writel(0x4121, ins->base + CDNS_DFE_ECMP_RATESEL + i);
> + writel(0x4121, ins->base + CDNS_DFE_SMP_RATESEL + i);
> + writel(0x9999, ins->base + CDNS_DEQ_VGATUNE_CTRL + i);
> + writel(0x0330, ins->base + CDNS_TMRVAL_MODE0 + i);
> + writel(0x01FF, ins->base + CDNS_PICNT_MODE1 + i);
> + writel(0x0009, ins->base + CDNS_CPI_OUTBUF_RATESEL + i);
> + writel(0x000F, ins->base + CDNS_LFPSFILT_NS + i);
> + writel(0x0009, ins->base + CDNS_LFPSFILT_RD + i);
> + writel(0x0001, ins->base + CDNS_LFPSFILT_MP + i);
> + writel(0x8013, ins->base + CDNS_SDFILT_H2L_A + i);
> + writel(0x0400, ins->base + CDNS_TMRVAL_MODE1 + i);

Are these tuning parameters? Don't they change for different platforms/vendors?
> + }
> +}
> +
> +static int cdns_sierra_phy_on(struct phy *x)
> +{
> + struct cdns_phy_instance *ins = phy_get_drvdata(x);
> + struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
> + int ret;
> +
> + ret = clk_prepare_enable(phy->clk);
> + if (ret)
> + return ret;
> +
> + /* Ensure the PHY is in reset */
> + reset_control_assert(phy->reset);
> +
> + /* Enable APB */
> + reset_control_deassert(phy->apb_reset);
> +
> + /* Program the PHY registers*/
> + if (ins->phy_type == PHY_TYPE_PCIE)
> + cdns_sierra_pcie_on(ins);
> + else
> + cdns_sierra_usb_on(ins);
> +
> + /* Take the PHY out of reset */
> + ret = reset_control_assert(phy->reset);
> +
> + if (ret) {
> + clk_disable_unprepare(phy->clk);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int cdns_sierra_phy_off(struct phy *x)
> +{
> + struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
> + int ret = 0;
> +
> + reset_control_assert(phy->reset);
> + reset_control_assert(phy->apb_reset);
> + clk_disable_unprepare(phy->clk);
> + dev_info(phy->dev, "sierra PHY OFF\n");

dev_vdbg?
> +
> + return ret;
> +}
> +
> +static const struct phy_ops ops = {
> + .power_on = cdns_sierra_phy_on,
> + .power_off = cdns_sierra_phy_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static const struct of_device_id cdns_sierra_id_table[];
> +static struct phy *cdns_sierra_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct cdns_sierra_phy *sphy = dev_get_drvdata(dev);
> + struct cdns_phy_instance *ins = NULL;
> + int i;
> +
> + if (args->args_count != 2) {
> + dev_err(dev, "invalid number of cells in 'phy' property\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (WARN_ON(args->args[1] == 0 || args->args[1] > 4))
> + return ERR_PTR(-ENODEV);
> +
> + for (i = 0; i < SIERRA_PHYS_NUM; i++) {
> + if (sphy->phys[i].phy_type == args->args[0])
> + ins = &sphy->phys[i];
> + }
> +
> + if (!ins) {
> + dev_err(dev, "failed to find appropriate phy\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + ins->nlanes = args->args[1];
> +
> + return ins->phy;
> +}
> +
> +static int cdns_sierra_phy_probe(struct platform_device *pdev)
> +{
> + struct cdns_sierra_phy *sphy;
> + struct phy_provider *phy_provider;
> + struct device *dev = &pdev->dev;
> + const struct of_device_id *match;
> + struct resource *res;
> + int i;
> +
> + sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
> + if (!sphy)
> + return -ENOMEM;
> +
> + sphy->dev = dev;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> + sphy->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(sphy->base)) {
> + dev_err(dev, "missing \"reg\"\n");
> + return PTR_ERR(sphy->base);
> + }
> +
> + /* Get init data for this PHY */
> + match = of_match_device(cdns_sierra_id_table, dev);
> + if (!match)
> + return -EINVAL;
> + sphy->init_data = (u32 *)match->data;
> +
> + /* Check that PHY is present */
> + if (sphy->init_data[0] != readl(sphy->base))
> + return -EINVAL;
> +
> + platform_set_drvdata(pdev, sphy);
> +
> + sphy->clk = devm_clk_get(dev, "phy_clk");
> + if (IS_ERR(sphy->clk)) {
> + dev_err(dev, "failed to get clock phy_clk\n");
> + return PTR_ERR(sphy->clk);
> + }
> +
> + sphy->reset = devm_reset_control_get(dev, "sierra_reset");
> + if (IS_ERR(sphy->reset)) {
> + dev_err(dev, "failed to get reset\n");
> + return PTR_ERR(sphy->reset);
> + }
> +
> + sphy->apb_reset = devm_reset_control_get(dev, "sierra_apb");
> + if (IS_ERR(sphy->apb_reset)) {
> + dev_err(dev, "failed to get apb reset\n");
> + return PTR_ERR(sphy->apb_reset);
> + }
> +
> + sphy->phys[0].phy_type = PHY_TYPE_PCIE;
> + sphy->phys[1].phy_type = PHY_TYPE_USB3;

We can have two subnodes, one for PCIe PHY and one for USB3 PHY. That way we
don't create two PHY's if a vendor hasn't exposed one type of PHY. We also
don't need a custom xlate then.

Thanks
Kishon

2018-08-30 14:45:07

by Alan Douglas

[permalink] [raw]
Subject: RE: [RFC PATCH 2/2] phy: cadence: Add driver for Sierra PHY

Hi,

Thanks for your comments.

On 20 August 2018 14:07, Kishon Vijay Abraham wrote:
> Hi,
>
> On Friday 17 August 2018 06:01 PM, Alan Douglas wrote:
> > Add a Sierra PHY driver with PCIe and USB support.
> > There are two resets to the PHY, one to enable
> > the APB interface for programming registers, and
> > another to enable the PHY itself.
> >
> > The sequence of operation on startup is to enable
> > the APB bus, write the PHY registers and then
> > enable the PHY.
> >
> > On power-down we simply put the PHY and APB bus in
> > reset.
> >
> > Signed-off-by: Alan Douglas <[email protected]>
> > ---
> > drivers/phy/Kconfig | 1 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/cadence/Kconfig | 9 ++
> > drivers/phy/cadence/Makefile | 3 +
> > drivers/phy/cadence/cdns-sierra.c | 326 ++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 340 insertions(+)
> > create mode 100644 drivers/phy/cadence/Kconfig
> > create mode 100644 drivers/phy/cadence/Makefile
> > create mode 100644 drivers/phy/cadence/cdns-sierra.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 5c8d452..cc47f85 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -43,6 +43,7 @@ config PHY_XGENE
> > source "drivers/phy/allwinner/Kconfig"
> > source "drivers/phy/amlogic/Kconfig"
> > source "drivers/phy/broadcom/Kconfig"
> > +source "drivers/phy/cadence/Kconfig"
> > source "drivers/phy/hisilicon/Kconfig"
> > source "drivers/phy/lantiq/Kconfig"
> > source "drivers/phy/marvell/Kconfig"
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 84e3bd9..ba48acd 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_RENESAS) += renesas/
> > obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
> > obj-$(CONFIG_ARCH_TEGRA) += tegra/
> > obj-y += broadcom/ \
> > + cadence/ \
> > hisilicon/ \
> > marvell/ \
> > motorola/ \
> > diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> > new file mode 100644
> > index 0000000..6bbe5a2
> > --- /dev/null
> > +++ b/drivers/phy/cadence/Kconfig
> > @@ -0,0 +1,9 @@
> > +#
> > +# Phy drivers for cadence devices
> > +#
> > +config CDNS_SIERRA_PHY
> > + tristate "CDNS Sierra PHY Driver"
> > + depends on OF && HAS_IOMEM
>
> depends on RESET_CONTROLLER?
Yes, I will add this.

> > + select GENERIC_PHY
> > + help
> > + Enable this to support the Cadence Sierra PHY driver
> > diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> > new file mode 100644
> > index 0000000..b384cb3
> > --- /dev/null
> > +++ b/drivers/phy/cadence/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_CDNS_SIERRA_PHY) += cdns-sierra.o
> > +
> > diff --git a/drivers/phy/cadence/cdns-sierra.c b/drivers/phy/cadence/cdns-sierra.c
> > new file mode 100644
> > index 0000000..0581516
> > --- /dev/null
> > +++ b/drivers/phy/cadence/cdns-sierra.c
> > @@ -0,0 +1,326 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Cadence Sierra PHY Driver
> > + *
> > + * Copyright (c) 2018 Cadence Design Systems
> > + * Author: Alan Douglas <[email protected]>
> > + *
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> > +#include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <dt-bindings/phy/phy.h>
> > +
> > +#define CDNS_PHY_PLL_CFG (0xc00e << 2)
> > +#define CDNS_DET_STANDEC_A (0x4000 << 2)
> > +#define CDNS_DET_STANDEC_B (0x4001 << 2)
> > +#define CDNS_DET_STANDEC_C (0x4002 << 2)
> > +#define CDNS_DET_STANDEC_D (0x4003 << 2)
> > +#define CDNS_DET_STANDEC_E (0x4004 << 2)
> > +#define CDNS_PSM_LANECAL (0x4008 << 2)
> > +#define CDNS_PSM_DIAG (0x4015 << 2)
> > +#define CDNS_PSC_TX_A0 (0x4028 << 2)
> > +#define CDNS_PSC_TX_A1 (0x4029 << 2)
> > +#define CDNS_PSC_TX_A2 (0x402A << 2)
> > +#define CDNS_PSC_TX_A3 (0x402B << 2)
> > +#define CDNS_PSC_RX_A0 (0x4030 << 2)
> > +#define CDNS_PSC_RX_A1 (0x4031 << 2)
> > +#define CDNS_PSC_RX_A2 (0x4032 << 2)
> > +#define CDNS_PSC_RX_A3 (0x4033 << 2)
> > +#define CDNS_PLLCTRL_SUBRATE (0x403A << 2)
> > +#define CDNS_PLLCTRL_GEN_D (0x403E << 2)
> > +#define CDNS_DRVCTRL_ATTEN (0x406A << 2)
> > +#define CDNS_CLKPATHCTRL_TMR (0x4081 << 2)
> > +#define CDNS_RX_CREQ_FLTR_A_MODE1 (0x4087 << 2)
> > +#define CDNS_RX_CREQ_FLTR_A_MODE0 (0x4088 << 2)
> > +#define CDNS_CREQ_CCLKDET_MODE01 (0x408E << 2)
> > +#define CDNS_RX_CTLE_MAINTENANCE (0x4091 << 2)
> > +#define CDNS_CREQ_FSMCLK_SEL (0x4092 << 2)
> > +#define CDNS_CTLELUT_CTRL (0x4098 << 2)
> > +#define CDNS_DFE_ECMP_RATESEL (0x40C0 << 2)
> > +#define CDNS_DFE_SMP_RATESEL (0x40C1 << 2)
> > +#define CDNS_DEQ_VGATUNE_CTRL (0x40E1 << 2)
> > +#define CDNS_TMRVAL_MODE3 (0x416E << 2)
> > +#define CDNS_TMRVAL_MODE2 (0x416F << 2)
> > +#define CDNS_TMRVAL_MODE1 (0x4170 << 2)
> > +#define CDNS_TMRVAL_MODE0 (0x4171 << 2)
> > +#define CDNS_PICNT_MODE1 (0x4174 << 2)
> > +#define CDNS_CPI_OUTBUF_RATESEL (0x417C << 2)
> > +#define CDNS_LFPSFILT_NS (0x418A << 2)
> > +#define CDNS_LFPSFILT_RD (0x418B << 2)
> > +#define CDNS_LFPSFILT_MP (0x418C << 2)
> > +#define CDNS_SDFILT_H2L_A (0x4191 << 2)
> > +
> > +#define SIERRA_PHYS_NUM 2
> > +
> > +struct cdns_phy_instance {
> > + struct phy *phy;
> > + u32 phy_type;
> > + u32 nlanes;
> > + void __iomem *base;
> > +};
> > +
> > +struct cdns_sierra_phy {
> > + struct device *dev;
> > + void __iomem *base;
> > + u32 *init_data;
> > + struct cdns_phy_instance phys[SIERRA_PHYS_NUM];
> > + struct reset_control *reset;
> > + struct reset_control *apb_reset;
> > + struct clk *clk;
> > + struct phy *pcie_phy;
> > +};
> > +
> > +static void cdns_sierra_pcie_on(struct cdns_phy_instance *ins)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
> > + writel(0x891f, ins->base + CDNS_DET_STANDEC_D + i);
> > + writel(0x0053, ins->base + CDNS_DET_STANDEC_E + i);
> > + writel(0x0400, ins->base + CDNS_TMRVAL_MODE2 + i);
> > + writel(0x0200, ins->base + CDNS_TMRVAL_MODE3 + i);
> > + }
> > +}
> > +
> > +static void cdns_sierra_usb_on(struct cdns_phy_instance *ins)
> > +{
> > + int i;
> > +
> > + writel(0x0000, ins->base + CDNS_PHY_PLL_CFG);
> > + for (i = 0; i < (ins->nlanes << 11); i += (1 << 11)) {
> > + writel(0xFE0A, ins->base + CDNS_DET_STANDEC_A + i);
> > + writel(0x000F, ins->base + CDNS_DET_STANDEC_B + i);
> > + writel(0x55A5, ins->base + CDNS_DET_STANDEC_C + i);
> > + writel(0x69AD, ins->base + CDNS_DET_STANDEC_D + i);
> > + writel(0x0241, ins->base + CDNS_DET_STANDEC_E + i);
> > + writel(0x0110, ins->base + CDNS_PSM_LANECAL + i);
> > + writel(0xCF00, ins->base + CDNS_PSM_DIAG + i);
> > + writel(0x001F, ins->base + CDNS_PSC_TX_A0 + i);
> > + writel(0x0007, ins->base + CDNS_PSC_TX_A1 + i);
> > + writel(0x0003, ins->base + CDNS_PSC_TX_A2 + i);
> > + writel(0x0003, ins->base + CDNS_PSC_TX_A3 + i);
> > + writel(0x0FFF, ins->base + CDNS_PSC_RX_A0 + i);
> > + writel(0x0003, ins->base + CDNS_PSC_RX_A1 + i);
> > + writel(0x0003, ins->base + CDNS_PSC_RX_A2 + i);
> > + writel(0x0001, ins->base + CDNS_PSC_RX_A3 + i);
> > + writel(0x0001, ins->base + CDNS_PLLCTRL_SUBRATE + i);
> > + writel(0x0406, ins->base + CDNS_PLLCTRL_GEN_D + i);
> > + writel(0x0000, ins->base + CDNS_DRVCTRL_ATTEN + i);
> > + writel(0x823E, ins->base + CDNS_CLKPATHCTRL_TMR + i);
> > + writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE1 + i);
> > + writel(0x078F, ins->base + CDNS_RX_CREQ_FLTR_A_MODE0 + i);
> > + writel(0x7B3C, ins->base + CDNS_CREQ_CCLKDET_MODE01 + i);
> > + writel(0x023C, ins->base + CDNS_RX_CTLE_MAINTENANCE + i);
> > + writel(0x3232, ins->base + CDNS_CREQ_FSMCLK_SEL + i);
> > + writel(0x8452, ins->base + CDNS_CTLELUT_CTRL + i);
> > + writel(0x4121, ins->base + CDNS_DFE_ECMP_RATESEL + i);
> > + writel(0x4121, ins->base + CDNS_DFE_SMP_RATESEL + i);
> > + writel(0x9999, ins->base + CDNS_DEQ_VGATUNE_CTRL + i);
> > + writel(0x0330, ins->base + CDNS_TMRVAL_MODE0 + i);
> > + writel(0x01FF, ins->base + CDNS_PICNT_MODE1 + i);
> > + writel(0x0009, ins->base + CDNS_CPI_OUTBUF_RATESEL + i);
> > + writel(0x000F, ins->base + CDNS_LFPSFILT_NS + i);
> > + writel(0x0009, ins->base + CDNS_LFPSFILT_RD + i);
> > + writel(0x0001, ins->base + CDNS_LFPSFILT_MP + i);
> > + writel(0x8013, ins->base + CDNS_SDFILT_H2L_A + i);
> > + writel(0x0400, ins->base + CDNS_TMRVAL_MODE1 + i);
>
> Are these tuning parameters? Don't they change for different platforms/vendors?
These parameters may vary depending on the actual platform. I need to rethink how these are included.

> > + }
> > +}
> > +
> > +static int cdns_sierra_phy_on(struct phy *x)
> > +{
> > + struct cdns_phy_instance *ins = phy_get_drvdata(x);
> > + struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(phy->clk);
> > + if (ret)
> > + return ret;
> > +
> > + /* Ensure the PHY is in reset */
> > + reset_control_assert(phy->reset);
> > +
> > + /* Enable APB */
> > + reset_control_deassert(phy->apb_reset);
> > +
> > + /* Program the PHY registers*/
> > + if (ins->phy_type == PHY_TYPE_PCIE)
> > + cdns_sierra_pcie_on(ins);
> > + else
> > + cdns_sierra_usb_on(ins);
> > +
> > + /* Take the PHY out of reset */
> > + ret = reset_control_assert(phy->reset);
> > +
> > + if (ret) {
> > + clk_disable_unprepare(phy->clk);
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int cdns_sierra_phy_off(struct phy *x)
> > +{
> > + struct cdns_sierra_phy *phy = dev_get_drvdata(x->dev.parent);
> > + int ret = 0;
> > +
> > + reset_control_assert(phy->reset);
> > + reset_control_assert(phy->apb_reset);
> > + clk_disable_unprepare(phy->clk);
> > + dev_info(phy->dev, "sierra PHY OFF\n");
>
> dev_vdbg?
I will remove, it was for debug and shouldn't be required.

> > +
> > + return ret;
> > +}
> > +
> > +static const struct phy_ops ops = {
> > + .power_on = cdns_sierra_phy_on,
> > + .power_off = cdns_sierra_phy_off,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static const struct of_device_id cdns_sierra_id_table[];
> > +static struct phy *cdns_sierra_xlate(struct device *dev,
> > + struct of_phandle_args *args)
> > +{
> > + struct cdns_sierra_phy *sphy = dev_get_drvdata(dev);
> > + struct cdns_phy_instance *ins = NULL;
> > + int i;
> > +
> > + if (args->args_count != 2) {
> > + dev_err(dev, "invalid number of cells in 'phy' property\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + if (WARN_ON(args->args[1] == 0 || args->args[1] > 4))
> > + return ERR_PTR(-ENODEV);
> > +
> > + for (i = 0; i < SIERRA_PHYS_NUM; i++) {
> > + if (sphy->phys[i].phy_type == args->args[0])
> > + ins = &sphy->phys[i];
> > + }
> > +
> > + if (!ins) {
> > + dev_err(dev, "failed to find appropriate phy\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + ins->nlanes = args->args[1];
> > +
> > + return ins->phy;
> > +}
> > +
> > +static int cdns_sierra_phy_probe(struct platform_device *pdev)
> > +{
> > + struct cdns_sierra_phy *sphy;
> > + struct phy_provider *phy_provider;
> > + struct device *dev = &pdev->dev;
> > + const struct of_device_id *match;
> > + struct resource *res;
> > + int i;
> > +
> > + sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
> > + if (!sphy)
> > + return -ENOMEM;
> > +
> > + sphy->dev = dev;
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> > + sphy->base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(sphy->base)) {
> > + dev_err(dev, "missing \"reg\"\n");
> > + return PTR_ERR(sphy->base);
> > + }
> > +
> > + /* Get init data for this PHY */
> > + match = of_match_device(cdns_sierra_id_table, dev);
> > + if (!match)
> > + return -EINVAL;
> > + sphy->init_data = (u32 *)match->data;
> > +
> > + /* Check that PHY is present */
> > + if (sphy->init_data[0] != readl(sphy->base))
> > + return -EINVAL;
> > +
> > + platform_set_drvdata(pdev, sphy);
> > +
> > + sphy->clk = devm_clk_get(dev, "phy_clk");
> > + if (IS_ERR(sphy->clk)) {
> > + dev_err(dev, "failed to get clock phy_clk\n");
> > + return PTR_ERR(sphy->clk);
> > + }
> > +
> > + sphy->reset = devm_reset_control_get(dev, "sierra_reset");
> > + if (IS_ERR(sphy->reset)) {
> > + dev_err(dev, "failed to get reset\n");
> > + return PTR_ERR(sphy->reset);
> > + }
> > +
> > + sphy->apb_reset = devm_reset_control_get(dev, "sierra_apb");
> > + if (IS_ERR(sphy->apb_reset)) {
> > + dev_err(dev, "failed to get apb reset\n");
> > + return PTR_ERR(sphy->apb_reset);
> > + }
> > +
> > + sphy->phys[0].phy_type = PHY_TYPE_PCIE;
> > + sphy->phys[1].phy_type = PHY_TYPE_USB3;
>
> We can have two subnodes, one for PCIe PHY and one for USB3 PHY. That way we
> don't create two PHY's if a vendor hasn't exposed one type of PHY. We also
> don't need a custom xlate then.
Makes sense. I will make this change and issue a new patch.
Thanks,
Alan
>
> Thanks
> Kishon