[PATCH v2] reset: Add i.MX7 SRC reset driver

[PATCH v2] reset: Add i.MX7 SRC reset driver

[Andrey Smirnov]

This driver exposes various reset faculties, impelented by System Reset
Controller IP block, as a reset driver. Currently only PCIE related
reset lines are implemented.

Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v1 (see [v1]):

	- Various small DT bindings description fixes as per feedback
          from Rob Herring


[v1] https://lkml.org/lkml/2017/2/6/554

 .../devicetree/bindings/reset/fsl,imx7-src.txt     |  47 +++++++
 drivers/reset/Kconfig                              |   8 ++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-imx7.c                         | 149 +++++++++++++++++++++
 include/dt-bindings/reset/imx7-reset.h             |  28 ++++
 5 files changed, 233 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
 create mode 100644 drivers/reset/reset-imx7.c
 create mode 100644 include/dt-bindings/reset/imx7-reset.h

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
new file mode 100644
index 0000000..c38f7f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
@@ -0,0 +1,47 @@
+Freescale i.MX7 System Reset Controller
+======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be "fsl,imx7-src", "syscon"
+- reg: should be register base and length as documented in the
+  datasheet
+- interrupts: Should contain SRC interrupt
+- #reset-cells: 1, see below
+
+example:
+
+src: reset-controller@30390000 {
+     compatible = "fsl,imx7d-src", "syscon";
+     reg = <0x30390000 0x2000>;
+     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+     #reset-cells = <1>;
+};
+
+
+Specifying reset lines connected to IP modules
+==============================================
+
+The system reset controller can be used to reset various set of
+peripherals. Device nodes that need access to reset lines should
+specify them as a reset phandle in their corresponding node as
+specified in reset.txt.
+
+Example:
+
+	pcie: pcie@33800000 {
+
+		...
+
+		resets = <&src IMX7_RESET_PCIEPHY>,
+			 <&src IMX7_RESET_PCIE_CTRL_APPS>;
+		reset-names = "pciephy", "apps";
+
+		...
+        };
+
+
+For list of all valid reset indicies see
+<dt-bindings/reset/imx7-reset.h>
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 172dc96..664a88b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -92,6 +92,14 @@ config RESET_ZYNQ
 	help
 	  This enables the reset controller driver for Xilinx Zynq SoCs.
 
+config RESET_IMX7
+	bool "i.MX7 Reset Driver"
+	depends on SOC_IMX7D || COMPILE_TEST
+	select MFD_SYSCON
+	default SOC_IMX7D
+	help
+	  This enables the reset controller driver for i.MX7 SoCs.
+
 source "drivers/reset/sti/Kconfig"
 source "drivers/reset/hisilicon/Kconfig"
 source "drivers/reset/tegra/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 13b346e..417c9d0 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
 obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
+obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
new file mode 100644
index 0000000..6d90e57
--- /dev/null
+++ b/drivers/reset/reset-imx7.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright (c) 2017, Impinj, Inc.
+ *
+ * i.MX7 System Reset Controller (SRC) driver
+ *
+ * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <dt-bindings/reset/imx7-reset.h>
+
+struct imx7_src {
+	struct reset_controller_dev rcdev;
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+enum imx7_src_registers {
+	SRC_PCIEPHY_RCR = 0x002c,
+	SRC_PCIEPHY_RCR_PCIEPHY_G_RST     = BIT(1),
+	SRC_PCIEPHY_RCR_PCIEPHY_BTN       = BIT(2),
+	SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN = BIT(6),
+};
+
+static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct imx7_src, rcdev);
+}
+
+static int imx7_reset_assert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	struct imx7_src *imx7src = to_imx7_src(rcdev);
+
+	switch (id) {
+	case IMX7_RESET_PCIE_CTRL_APPS:
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN, 0);
+		return 0;
+
+	case IMX7_RESET_PCIEPHY:
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST,
+				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST);
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIEPHY_BTN,
+				   SRC_PCIEPHY_RCR_PCIEPHY_BTN);
+		return 0;
+	default:
+		dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int imx7_reset_deassert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct imx7_src *imx7src = to_imx7_src(rcdev);
+
+	switch (id) {
+	case IMX7_RESET_PCIE_CTRL_APPS:
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN,
+				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN);
+		return 0;
+
+	case IMX7_RESET_PCIEPHY:
+		/* wait for more than 10us to release phy g_rst and btnrst */
+		udelay(10);
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST, 0);
+		regmap_update_bits(imx7src->regmap,
+				   SRC_PCIEPHY_RCR,
+				   SRC_PCIEPHY_RCR_PCIEPHY_BTN, 0);
+		return 0;
+	default:
+		dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
+		break;
+	};
+
+	return -EINVAL;
+}
+
+static const struct reset_control_ops imx7_reset_ops = {
+	.assert		= imx7_reset_assert,
+	.deassert	= imx7_reset_deassert,
+};
+
+static int imx7_reset_probe(struct platform_device *pdev)
+{
+	struct imx7_src *imx7src;
+	struct device *dev = &pdev->dev;
+
+	imx7src = devm_kzalloc(dev, sizeof(*imx7src), GFP_KERNEL);
+	if (!imx7src)
+		return -ENOMEM;
+
+	imx7src->dev = dev;
+	imx7src->regmap = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(imx7src->regmap)) {
+		dev_err(dev, "Unable to get imx7-src regmap");
+		return PTR_ERR(imx7src->regmap);
+	}
+
+	imx7src->rcdev.owner     = THIS_MODULE;
+	imx7src->rcdev.nr_resets = IMX7_RESET_NUM;
+	imx7src->rcdev.ops       = &imx7_reset_ops;
+	imx7src->rcdev.of_node   = dev->of_node;
+
+	return devm_reset_controller_register(dev, &imx7src->rcdev);
+}
+
+static const struct of_device_id imx7_reset_dt_ids[] = {
+	{ .compatible = "fsl,imx7d-src", },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver imx7_reset_driver = {
+	.probe	= imx7_reset_probe,
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= imx7_reset_dt_ids,
+	},
+};
+builtin_platform_driver(imx7_reset_driver);
diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
new file mode 100644
index 0000000..e20f2db
--- /dev/null
+++ b/include/dt-bindings/reset/imx7-reset.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2017 Impinj, Inc.
+ *
+ * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef DT_BINDING_RESET_IMX7_H
+#define DT_BINDING_RESET_IMX7_H
+
+#define IMX7_RESET_PCIE_CTRL_APPS	0
+#define IMX7_RESET_PCIEPHY		1
+
+#define IMX7_RESET_NUM			2
+
+#endif
+
-- 
2.9.3

[PATCH v2] soc/imx: Add GPCv2 power gating driver

[Andrey Smirnov]

Add code allowing for control of various power domains managed by GPCv2
IP block found in i.MX7 series of SoCs. Power domains covered by this
patch are:

    - PCIE PHY
    - MIPI PHY
    - USB HSIC PHY
    - USB OTG1/2 PHY

Support for any other power domain controlled by GPC is not present, and
can be added at some later point.

Testing of this code was done against a PCIe driver.

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v1 (see [v1]):

	- Various DT bindings description fixes as per feedback from
          Rob Herring

[v1] https://lkml.org/lkml/2017/2/6/551

 .../devicetree/bindings/power/fsl,imx-gpcv2.txt    |  71 ++++
 drivers/soc/imx/Makefile                           |   2 +-
 drivers/soc/imx/gpcv2.c                            | 380 +++++++++++++++++++++
 include/dt-bindings/power/imx7-power.h             |  18 +
 4 files changed, 470 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
 create mode 100644 drivers/soc/imx/gpcv2.c
 create mode 100644 include/dt-bindings/power/imx7-power.h

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
new file mode 100644
index 0000000..393f68a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
@@ -0,0 +1,71 @@
+Freescale i.MX General Power Controller v2
+==========================================
+
+The i.MX7S/D General Power Control (GPC) block contains Power Gating
+Control (PGC) for various power domains.
+
+Required properties:
+
+- compatible: Should be "fsl,imx7d-gpc"
+
+- reg: should be register base and length as documented in the
+  datasheet
+
+- interrupts: Should contain GPC interrupt request 1
+
+Power domains contained within GPC node are generic power domain
+providers, documented in
+Documentation/devicetree/bindings/power/power_domain.txt, which are
+described as subnodes of the power gating controller 'pgc' node,
+which, in turn, is expected to contain the following:
+
+Required properties:
+
+- reg: Power domain index. Valid values are defined in
+  include/dt-bindings/power/imx7-power.h
+
+- #power-domain-cells: Should be 0
+
+Optional properties:
+
+- power-supply: Power supply used to power the domain
+
+Example:
+
+	gpc: gpc@303a0000 {
+		compatible = "fsl,imx7d-gpc";
+		reg = <0x303a0000 0x1000>;
+		interrupt-controller;
+		interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
+
+		pgc {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pgc_pcie_phy: pgc-pcie-phy-domain {
+				#power-domain-cells = <0>;
+
+				reg = <IMX7_POWER_DOMAIN_PCIE_PHY>;
+				power-supply = <&reg_1p0d>;
+			};
+		};
+	};
+
+
+Specifying power domain for IP modules
+======================================
+
+IP cores belonging to a power domain should contain a 'power-domains'
+property that is a phandle for PGC node representing the domain.
+
+Example of a device that is part of the PCIE_PHY power domain:
+
+	pcie: pcie@0x33800000 {
+	      reg = <0x33800000 0x4000>,
+	            <0x4ff00000 0x80000>;
+		/* ... */
+		power-domains = <&pgc_pcie_phy>;
+		/* ... */
+	};
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 35861f5..7bcc933 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -1 +1 @@
-obj-y += gpc.o
+obj-y += gpc.o gpcv2.o
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
new file mode 100644
index 0000000..e4a5d16
--- /dev/null
+++ b/drivers/soc/imx/gpcv2.c
@@ -0,0 +1,380 @@
+/*
+ * Copyright 2017 Impinj, Inc
+ *
+ * Based on the code of analogus driver:
+ *
+ * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <dt-bindings/power/imx7-power.h>
+
+#define GPC_PGC_CPU_MAPPING	0xec
+#define USB_HSIC_PHY_A7_DOMAIN	BIT(6)
+#define USB_OTG2_PHY_A7_DOMAIN	BIT(5)
+#define USB_OTG1_PHY_A7_DOMAIN	BIT(4)
+#define PCIE_PHY_A7_DOMAIN	BIT(3)
+#define MIPI_PHY_A7_DOMAIN	BIT(2)
+
+#define GPC_PU_PGC_SW_PUP_REQ	0xf8
+#define GPC_PU_PGC_SW_PDN_REQ	0x104
+#define USB_HSIC_PHY_SW_Pxx_REQ	BIT(4)
+#define USB_OTG2_PHY_SW_Pxx_REQ	BIT(3)
+#define USB_OTG1_PHY_SW_Pxx_REQ	BIT(2)
+#define PCIE_PHY_SW_Pxx_REQ	BIT(1)
+#define MIPI_PHY_SW_Pxx_REQ	BIT(0)
+
+#define GPC_MAX_REGISTER	GPC_PU_PGC_SW_PDN_REQ
+
+struct imx7_pgc_domain {
+	struct generic_pm_domain genpd;
+	struct regmap *regmap;
+	struct regulator *regulator;
+
+	const struct {
+		u32 pxx;
+		u32 map;
+	} bits;
+
+	const int voltage;
+
+	struct device *dev;
+};
+
+static int imx7_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
+				      bool on)
+{
+	int ret = 0;
+	unsigned long deadline;
+	struct imx7_pgc_domain *domain = container_of(genpd,
+						      struct imx7_pgc_domain,
+						      genpd);
+	unsigned int offset = (on) ?
+		GPC_PU_PGC_SW_PUP_REQ : GPC_PU_PGC_SW_PDN_REQ;
+
+	const bool has_regulator = !IS_ERR(domain->regulator);
+
+	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+			   domain->bits.map, domain->bits.map);
+	if (has_regulator && on) {
+		ret = regulator_enable(domain->regulator);
+		if (ret) {
+			dev_err(domain->dev, "failed to enable regulator\n");
+			goto unmap;
+		}
+	}
+
+	regmap_update_bits(domain->regmap, offset,
+			   domain->bits.pxx, domain->bits.pxx);
+	/*
+	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+	 * for PUP_REQ/PDN_REQ bit to be cleared
+	 */
+	deadline = jiffies + msecs_to_jiffies(1);
+	while (true) {
+		u32 pxx_req;
+
+		regmap_read(domain->regmap, offset, &pxx_req);
+
+		if (pxx_req & domain->bits.pxx)
+			break;
+
+		if (time_after(jiffies, deadline)) {
+			dev_err(domain->dev, "falied to command PGC\n");
+			ret = -ETIMEDOUT;
+			/*
+			 * If we were in a process of enabling a
+			 * domain and failed we might as well disable
+			 * the regulator we just enabled. And if it
+			 * was the opposite situation and we failed to
+			 * power down -- keep the regulator on
+			 */
+			on  = !on;
+			break;
+		}
+
+		cpu_relax();
+	}
+
+	if (has_regulator && !on) {
+		int err;
+
+		err = regulator_disable(domain->regulator);
+		if (err)
+			dev_err(domain->dev,
+				"failed to disable regulator: %d\n", ret);
+		/*
+		 * Preserve earlier error code
+		 */
+		ret = ret ?: err;
+	}
+unmap:
+	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+			   domain->bits.map, 0);
+	return ret;
+}
+
+static int imx7_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain *genpd)
+{
+	return imx7_gpc_pu_pgc_sw_pxx_req(genpd, true);
+}
+
+static int imx7_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain *genpd)
+{
+	return imx7_gpc_pu_pgc_sw_pxx_req(genpd, false);
+}
+
+static struct imx7_pgc_domain imx7_pgc_domains[] = {
+	[IMX7_POWER_DOMAIN_USB_HSIC_PHY] = {
+		.genpd = {
+			.name      = "usb-hsic-phy",
+			.power_on  = imx7_gpc_pu_pgc_sw_pup_req,
+			.power_off = imx7_gpc_pu_pgc_sw_pdn_req,
+		},
+		.bits  = {
+			.pxx = USB_HSIC_PHY_SW_Pxx_REQ,
+			.map = USB_HSIC_PHY_A7_DOMAIN,
+		},
+		.voltage = 1200000,
+	},
+
+	[IMX7_POWER_DOMAIN_USB_OTG2_PHY] = {
+		.genpd = {
+			.name      = "usb-otg2-phy",
+			.power_on  = imx7_gpc_pu_pgc_sw_pup_req,
+			.power_off = imx7_gpc_pu_pgc_sw_pdn_req,
+		},
+		.bits  = {
+			.pxx = USB_OTG2_PHY_SW_Pxx_REQ,
+			.map = USB_OTG2_PHY_A7_DOMAIN,
+		},
+	},
+
+	[IMX7_POWER_DOMAIN_USB_OTG1_PHY] = {
+		.genpd = {
+			.name      = "usb-otg1-phy",
+			.power_on  = imx7_gpc_pu_pgc_sw_pup_req,
+			.power_off = imx7_gpc_pu_pgc_sw_pdn_req,
+		},
+		.bits  = {
+			.pxx = USB_OTG1_PHY_SW_Pxx_REQ,
+			.map = USB_OTG1_PHY_A7_DOMAIN,
+		},
+	},
+
+	[IMX7_POWER_DOMAIN_PCIE_PHY] = {
+		.genpd = {
+			.name      = "pcie-phy",
+			.power_on  = imx7_gpc_pu_pgc_sw_pup_req,
+			.power_off = imx7_gpc_pu_pgc_sw_pdn_req,
+		},
+		.bits  = {
+			.pxx = PCIE_PHY_SW_Pxx_REQ,
+			.map = PCIE_PHY_A7_DOMAIN,
+		},
+		.voltage = 1000000,
+	},
+
+	[IMX7_POWER_DOMAIN_MIPI_PHY] = {
+		.genpd = {
+			.name      = "mipi-phy",
+			.power_on  = imx7_gpc_pu_pgc_sw_pup_req,
+			.power_off = imx7_gpc_pu_pgc_sw_pdn_req,
+		},
+		.bits  = {
+			.pxx = MIPI_PHY_SW_Pxx_REQ,
+			.map = MIPI_PHY_A7_DOMAIN,
+		},
+		.voltage = 1000000,
+	},
+};
+
+static int imx7_pgc_domain_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct imx7_pgc_domain *domain = pdev->dev.platform_data;
+
+	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
+		return 0;
+
+	domain->dev = &pdev->dev;
+
+	ret = pm_genpd_init(&domain->genpd, NULL, true);
+	if (ret) {
+		dev_err(domain->dev, "Failed to init power domain\n");
+		return ret;
+	}
+
+	domain->regulator = devm_regulator_get_optional(domain->dev, "power");
+	if (IS_ERR(domain->regulator)  &&
+	    PTR_ERR(domain->regulator) != -ENODEV) {
+		dev_err(domain->dev, "Failed to get domain's regulator\n");
+		return PTR_ERR(domain->regulator);
+	}
+
+	if (!IS_ERR(domain->regulator)) {
+		WARN_ON(!domain->voltage);
+		regulator_set_voltage(domain->regulator,
+				      domain->voltage, domain->voltage);
+	}
+
+	ret = of_genpd_add_provider_simple(domain->dev->of_node,
+					   &domain->genpd);
+	if (ret) {
+		dev_err(domain->dev, "Failed to add genpd provider\n");
+		pm_genpd_remove(&domain->genpd);
+	}
+
+	return ret;
+}
+
+static int imx7_pgc_domain_remove(struct platform_device *pdev)
+{
+	struct imx7_pgc_domain *domain = pdev->dev.platform_data;
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		of_genpd_del_provider(domain->dev->of_node);
+		pm_genpd_remove(&domain->genpd);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id imx7_pgc_domain_id[] = {
+	{ "imx7-pgc-domain", },
+	{ },
+};
+
+static struct platform_driver imx7_pgc_domain_driver = {
+	.driver = {
+		.name = "imx7-pgc",
+	},
+	.probe    = imx7_pgc_domain_probe,
+	.remove   = imx7_pgc_domain_remove,
+	.id_table = imx7_pgc_domain_id,
+};
+builtin_platform_driver(imx7_pgc_domain_driver)
+
+static bool imx_gpcv2_readable_reg(struct device *dev, unsigned int reg)
+{
+	return reg % 4 == 0 &&
+	       reg <= GPC_MAX_REGISTER;
+}
+
+static bool imx_gpcv2_volatile_reg(struct device *dev, unsigned int reg)
+{
+	return reg == GPC_PU_PGC_SW_PUP_REQ ||
+	       reg == GPC_PU_PGC_SW_PDN_REQ;
+}
+
+static int imx_gpcv2_probe(struct platform_device *pdev)
+{
+	int ret;
+	void __iomem *base;
+	struct resource *res;
+	struct regmap *regmap;
+	struct device *dev = &pdev->dev;
+	struct device_node *pgc_np, *np;
+
+	static const struct regmap_config regmap_config = {
+		.cache_type	= REGCACHE_FLAT,
+		.reg_bits	= 32,
+		.val_bits	= 32,
+		.reg_stride	= 4,
+
+		.readable_reg	= imx_gpcv2_readable_reg,
+		.volatile_reg	= imx_gpcv2_volatile_reg,
+
+		.max_register	= GPC_MAX_REGISTER,
+	};
+
+	pgc_np = of_get_child_by_name(dev->of_node, "pgc");
+	if (!pgc_np) {
+		dev_err(dev, "No power domains specified in DT\n");
+		return -EINVAL;
+	}
+
+	res  = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio_clk(dev, NULL, base, &regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(dev, "failed to init regmap\n");
+		return ret;
+	}
+
+	for_each_child_of_node(pgc_np, np) {
+		u32 domain_index;
+		struct platform_device *pd_pdev;
+		struct imx7_pgc_domain *domain;
+
+		ret = of_property_read_u32(np, "reg", &domain_index);
+		if (ret) {
+			dev_err(dev, "Failed to read 'reg' property\n");
+			of_node_put(np);
+			return ret;
+		}
+
+		if (domain_index >= ARRAY_SIZE(imx7_pgc_domains)) {
+			dev_warn(dev,
+				 "Domain index %d is out of bounds\n",
+				 domain_index);
+			continue;
+		}
+
+		domain = &imx7_pgc_domains[domain_index];
+		domain->regmap = regmap;
+
+		pd_pdev = platform_device_alloc("imx7-pgc-domain",
+						domain_index);
+		if (!pd_pdev) {
+			dev_err(dev, "Failed to allocate platform device\n");
+			of_node_put(np);
+			return -ENOMEM;
+		}
+
+		pd_pdev->dev.platform_data = domain;
+		pd_pdev->dev.parent = dev;
+		pd_pdev->dev.of_node = np;
+
+		ret = platform_device_add(pd_pdev);
+		if (ret) {
+			platform_device_put(pd_pdev);
+			of_node_put(np);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id imx_gpcv2_dt_ids[] = {
+	{ .compatible = "fsl,imx7d-gpc" },
+	{ }
+};
+
+static struct platform_driver imx_gpc_driver = {
+	.driver = {
+		.name = "imx-gpcv2",
+		.of_match_table = imx_gpcv2_dt_ids,
+	},
+	.probe = imx_gpcv2_probe,
+};
+builtin_platform_driver(imx_gpc_driver)
diff --git a/include/dt-bindings/power/imx7-power.h b/include/dt-bindings/power/imx7-power.h
new file mode 100644
index 0000000..24dde62
--- /dev/null
+++ b/include/dt-bindings/power/imx7-power.h
@@ -0,0 +1,18 @@
+/*
+ *  Copyright © 2017 Impinj
+ *
+ * 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.
+ */
+
+#ifndef __DT_BINDINGS_ARM_IMX7_POWER_H__
+#define __DT_BINDINGS_ARM_IMX7_POWER_H__
+
+#define IMX7_POWER_DOMAIN_USB_HSIC_PHY	0
+#define IMX7_POWER_DOMAIN_USB_OTG2_PHY	1
+#define IMX7_POWER_DOMAIN_USB_OTG1_PHY	2
+#define IMX7_POWER_DOMAIN_PCIE_PHY	3
+#define IMX7_POWER_DOMAIN_MIPI_PHY	4
+
+#endif
-- 
2.9.3

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Philipp Zabel]

Hi Andrey,

thank you for the driver, I have a few comments below.

On Mon, 2017-02-13 at 07:33 -0800, Andrey Smirnov wrote:
> This driver exposes various reset faculties, impelented by System Reset

s/impelented/implemented/

> Controller IP block, as a reset driver. Currently only PCIE related
> reset lines are implemented.
> 
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> Changes since v1 (see [v1]):
> 
> 	- Various small DT bindings description fixes as per feedback
>           from Rob Herring
> 
> 
> [v1] https://lkml.org/lkml/2017/2/6/554
> 
>  .../devicetree/bindings/reset/fsl,imx7-src.txt     |  47 +++++++
>  drivers/reset/Kconfig                              |   8 ++
>  drivers/reset/Makefile                             |   1 +
>  drivers/reset/reset-imx7.c                         | 149 +++++++++++++++++++++
>  include/dt-bindings/reset/imx7-reset.h             |  28 ++++
>  5 files changed, 233 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
>  create mode 100644 drivers/reset/reset-imx7.c
>  create mode 100644 include/dt-bindings/reset/imx7-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> new file mode 100644
> index 0000000..c38f7f0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> @@ -0,0 +1,47 @@
> +Freescale i.MX7 System Reset Controller
> +======================================
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible: Should be "fsl,imx7-src", "syscon"
> +- reg: should be register base and length as documented in the
> +  datasheet
> +- interrupts: Should contain SRC interrupt
> +- #reset-cells: 1, see below
> +
> +example:
> +
> +src: reset-controller@30390000 {
> +     compatible = "fsl,imx7d-src", "syscon";
> +     reg = <0x30390000 0x2000>;
> +     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> +     #reset-cells = <1>;
> +};
> +
> +
> +Specifying reset lines connected to IP modules
> +==============================================
> +
> +The system reset controller can be used to reset various set of
> +peripherals. Device nodes that need access to reset lines should
> +specify them as a reset phandle in their corresponding node as
> +specified in reset.txt.
> +
> +Example:
> +
> +	pcie: pcie@33800000 {
> +
> +		...
> +
> +		resets = <&src IMX7_RESET_PCIEPHY>,
> +			 <&src IMX7_RESET_PCIE_CTRL_APPS>;
> +		reset-names = "pciephy", "apps";
> +
> +		...
> +        };
> +
> +
> +For list of all valid reset indicies see
> +<dt-bindings/reset/imx7-reset.h>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 172dc96..664a88b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -92,6 +92,14 @@ config RESET_ZYNQ
>  	help
>  	  This enables the reset controller driver for Xilinx Zynq SoCs.
>  
> +config RESET_IMX7
> +	bool "i.MX7 Reset Driver"
> +	depends on SOC_IMX7D || COMPILE_TEST
> +	select MFD_SYSCON
> +	default SOC_IMX7D
> +	help
> +	  This enables the reset controller driver for i.MX7 SoCs.
> +

Alphabetical order, please.

>  source "drivers/reset/sti/Kconfig"
>  source "drivers/reset/hisilicon/Kconfig"
>  source "drivers/reset/tegra/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 13b346e..417c9d0 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> +obj-$(CONFIG_RESET_IMX7) += reset-imx7.o

See above, those are also in alphabetical order.

> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> new file mode 100644
> index 0000000..6d90e57
> --- /dev/null
> +++ b/drivers/reset/reset-imx7.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright (c) 2017, Impinj, Inc.
> + *
> + * i.MX7 System Reset Controller (SRC) driver
> + *
> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>

I think this is not be necessary because register access is done via
regmap. Please cull unnecessary headers.

> +#include <linux/init.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +#include <dt-bindings/reset/imx7-reset.h>
> +
> +struct imx7_src {
> +	struct reset_controller_dev rcdev;
> +	struct device *dev;
> +	struct regmap *regmap;
> +};
> +
> +enum imx7_src_registers {
> +	SRC_PCIEPHY_RCR = 0x002c,
> +	SRC_PCIEPHY_RCR_PCIEPHY_G_RST     = BIT(1),

What about the others resets? There's at least HSICPHY, MIPIPHY and
USBOPHY registers before the PCIEPHY register.

> +	SRC_PCIEPHY_RCR_PCIEPHY_BTN       = BIT(2),
> +	SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN = BIT(6),

Are those really resets? At least the PCIE_CTRL_APPS_EN has a bit called
PCIE_CTRL_APPS_RST right next to it, so this warrants some explanation.

> +};
> +
> +static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct imx7_src, rcdev);
> +}
> +
> +static int imx7_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct imx7_src *imx7src = to_imx7_src(rcdev);
> +
> +	switch (id) {
> +	case IMX7_RESET_PCIE_CTRL_APPS:
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN, 0);
> +		return 0;
> +
> +	case IMX7_RESET_PCIEPHY:
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST);
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_BTN,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_BTN);

What is the PCIE PHY button, and why does it have to be set with the
reset bit?

> +		return 0;
> +	default:
> +		dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int imx7_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct imx7_src *imx7src = to_imx7_src(rcdev);
> +
> +	switch (id) {
> +	case IMX7_RESET_PCIE_CTRL_APPS:
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN,
> +				   SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN);
> +		return 0;
> +
> +	case IMX7_RESET_PCIEPHY:
> +		/* wait for more than 10us to release phy g_rst and btnrst */
> +		udelay(10);
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_G_RST, 0);
> +		regmap_update_bits(imx7src->regmap,
> +				   SRC_PCIEPHY_RCR,
> +				   SRC_PCIEPHY_RCR_PCIEPHY_BTN, 0);
> +		return 0;
> +	default:
> +		dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
> +		break;
> +	};
> +
> +	return -EINVAL;
> +}
> +
> +static const struct reset_control_ops imx7_reset_ops = {
> +	.assert		= imx7_reset_assert,
> +	.deassert	= imx7_reset_deassert,
> +};
> +
> +static int imx7_reset_probe(struct platform_device *pdev)
> +{
> +	struct imx7_src *imx7src;
> +	struct device *dev = &pdev->dev;
> +
> +	imx7src = devm_kzalloc(dev, sizeof(*imx7src), GFP_KERNEL);
> +	if (!imx7src)
> +		return -ENOMEM;
> +
> +	imx7src->dev = dev;
> +	imx7src->regmap = syscon_node_to_regmap(dev->of_node);
> +	if (IS_ERR(imx7src->regmap)) {
> +		dev_err(dev, "Unable to get imx7-src regmap");
> +		return PTR_ERR(imx7src->regmap);
> +	}

You could use regmap_attach_dev to attach the device to the regmap. This
would improve error messages, regmap debugfs structure, and make
imx7src->dev unnecessary.

> +
> +	imx7src->rcdev.owner     = THIS_MODULE;
> +	imx7src->rcdev.nr_resets = IMX7_RESET_NUM;
> +	imx7src->rcdev.ops       = &imx7_reset_ops;
> +	imx7src->rcdev.of_node   = dev->of_node;
> +
> +	return devm_reset_controller_register(dev, &imx7src->rcdev);
> +}
> +
> +static const struct of_device_id imx7_reset_dt_ids[] = {
> +	{ .compatible = "fsl,imx7d-src", },
> +	{ /* sentinel */ },
> +};
> +
> +static struct platform_driver imx7_reset_driver = {
> +	.probe	= imx7_reset_probe,
> +	.driver = {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table	= imx7_reset_dt_ids,
> +	},
> +};
> +builtin_platform_driver(imx7_reset_driver);
> diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
> new file mode 100644
> index 0000000..e20f2db
> --- /dev/null
> +++ b/include/dt-bindings/reset/imx7-reset.h
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright (C) 2017 Impinj, Inc.
> + *
> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef DT_BINDING_RESET_IMX7_H
> +#define DT_BINDING_RESET_IMX7_H
> +
> +#define IMX7_RESET_PCIE_CTRL_APPS	0
> +#define IMX7_RESET_PCIEPHY		1

It would expect these to be numbered in the order they appear in the
register map, not starting from the end. Could you add all available
peripheral resets to this list, in the correct order?

> +#define IMX7_RESET_NUM			2
> +
> +#endif
> +

regards
Philipp

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Andrey Smirnov]

On Mon, Feb 13, 2017 at 8:50 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Andrey,
>
> thank you for the driver, I have a few comments below.
>
> On Mon, 2017-02-13 at 07:33 -0800, Andrey Smirnov wrote:
>> This driver exposes various reset faculties, impelented by System Reset
>
> s/impelented/implemented/

Thanks for noticing, will fix in v3.

>
>> Controller IP block, as a reset driver. Currently only PCIE related
>> reset lines are implemented.
>>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>
>> Changes since v1 (see [v1]):
>>
>>       - Various small DT bindings description fixes as per feedback
>>           from Rob Herring
>>
>>
>> [v1] https://lkml.org/lkml/2017/2/6/554
>>
>>  .../devicetree/bindings/reset/fsl,imx7-src.txt     |  47 +++++++
>>  drivers/reset/Kconfig                              |   8 ++
>>  drivers/reset/Makefile                             |   1 +
>>  drivers/reset/reset-imx7.c                         | 149 +++++++++++++++++++++
>>  include/dt-bindings/reset/imx7-reset.h             |  28 ++++
>>  5 files changed, 233 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
>>  create mode 100644 drivers/reset/reset-imx7.c
>>  create mode 100644 include/dt-bindings/reset/imx7-reset.h
>>
>> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
>> new file mode 100644
>> index 0000000..c38f7f0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
>> @@ -0,0 +1,47 @@
>> +Freescale i.MX7 System Reset Controller
>> +======================================
>> +
>> +Please also refer to reset.txt in this directory for common reset
>> +controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be "fsl,imx7-src", "syscon"
>> +- reg: should be register base and length as documented in the
>> +  datasheet
>> +- interrupts: Should contain SRC interrupt
>> +- #reset-cells: 1, see below
>> +
>> +example:
>> +
>> +src: reset-controller@30390000 {
>> +     compatible = "fsl,imx7d-src", "syscon";
>> +     reg = <0x30390000 0x2000>;
>> +     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>> +     #reset-cells = <1>;
>> +};
>> +
>> +
>> +Specifying reset lines connected to IP modules
>> +==============================================
>> +
>> +The system reset controller can be used to reset various set of
>> +peripherals. Device nodes that need access to reset lines should
>> +specify them as a reset phandle in their corresponding node as
>> +specified in reset.txt.
>> +
>> +Example:
>> +
>> +     pcie: pcie@33800000 {
>> +
>> +             ...
>> +
>> +             resets = <&src IMX7_RESET_PCIEPHY>,
>> +                      <&src IMX7_RESET_PCIE_CTRL_APPS>;
>> +             reset-names = "pciephy", "apps";
>> +
>> +             ...
>> +        };
>> +
>> +
>> +For list of all valid reset indicies see
>> +<dt-bindings/reset/imx7-reset.h>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 172dc96..664a88b 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -92,6 +92,14 @@ config RESET_ZYNQ
>>       help
>>         This enables the reset controller driver for Xilinx Zynq SoCs.
>>
>> +config RESET_IMX7
>> +     bool "i.MX7 Reset Driver"
>> +     depends on SOC_IMX7D || COMPILE_TEST
>> +     select MFD_SYSCON
>> +     default SOC_IMX7D
>> +     help
>> +       This enables the reset controller driver for i.MX7 SoCs.
>> +
>
> Alphabetical order, please.

OK, will fix in v3.

>
>>  source "drivers/reset/sti/Kconfig"
>>  source "drivers/reset/hisilicon/Kconfig"
>>  source "drivers/reset/tegra/Kconfig"
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index 13b346e..417c9d0 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -14,3 +14,4 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>>  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>>  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>> +obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
>
> See above, those are also in alphabetical order.

Ditto.

>
>> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
>> new file mode 100644
>> index 0000000..6d90e57
>> --- /dev/null
>> +++ b/drivers/reset/reset-imx7.c
>> @@ -0,0 +1,149 @@
>> +/*
>> + * Copyright (c) 2017, Impinj, Inc.
>> + *
>> + * i.MX7 System Reset Controller (SRC) driver
>> + *
>> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>
> I think this is not be necessary because register access is done via
> regmap. Please cull unnecessary headers.

OK, will do for v3.

>
>> +#include <linux/init.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/regmap.h>
>> +#include <linux/types.h>
>> +#include <dt-bindings/reset/imx7-reset.h>
>> +
>> +struct imx7_src {
>> +     struct reset_controller_dev rcdev;
>> +     struct device *dev;
>> +     struct regmap *regmap;
>> +};
>> +
>> +enum imx7_src_registers {
>> +     SRC_PCIEPHY_RCR = 0x002c,
>> +     SRC_PCIEPHY_RCR_PCIEPHY_G_RST     = BIT(1),
>
> What about the others resets? There's at least HSICPHY, MIPIPHY and
> USBOPHY registers before the PCIEPHY register.

I don't really have any code using anything but PCI reset related
functionality, so I can't test any of the resets you mention. In light
of that I didn't want to add any of the definitions that are not going
to be used anywhere in the code.

>
>> +     SRC_PCIEPHY_RCR_PCIEPHY_BTN       = BIT(2),
>> +     SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN = BIT(6),
>
> Are those really resets? At least the PCIE_CTRL_APPS_EN has a bit called
> PCIE_CTRL_APPS_RST right next to it, so this warrants some explanation.
>

Public documentation for that aspect of i.MX7 is nonexistent and,
unfortunately, that is my only source of information. Given that, I
can't really tell you what the difference between PCIE_CTRL_APPS_EN
and PCIE_CTRL_APPS_RST besides that the former is what downstream PCIe
driver uses to inhibit LTSSM and the latter is not referenced or used
by any code (as far as I am aware).

>> +};
>> +
>> +static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
>> +{
>> +     return container_of(rcdev, struct imx7_src, rcdev);
>> +}
>> +
>> +static int imx7_reset_assert(struct reset_controller_dev *rcdev,
>> +                          unsigned long id)
>> +{
>> +     struct imx7_src *imx7src = to_imx7_src(rcdev);
>> +
>> +     switch (id) {
>> +     case IMX7_RESET_PCIE_CTRL_APPS:
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN, 0);
>> +             return 0;
>> +
>> +     case IMX7_RESET_PCIEPHY:
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_G_RST,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_G_RST);
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN);
>
> What is the PCIE PHY button, and why does it have to be set with the
> reset bit?

I am sorry, but just as above, I wouldn't be able to enlighten you any
more about the subject. I have no knowledge about the details of G_RST
and BTN signal (or even if BTN stands for "button") behavior beyond
the fact that that is how downstream driver performs PCIEPHY reset.

>
>> +             return 0;
>> +     default:
>> +             dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
>> +             break;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int imx7_reset_deassert(struct reset_controller_dev *rcdev,
>> +                            unsigned long id)
>> +{
>> +     struct imx7_src *imx7src = to_imx7_src(rcdev);
>> +
>> +     switch (id) {
>> +     case IMX7_RESET_PCIE_CTRL_APPS:
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN,
>> +                                SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN);
>> +             return 0;
>> +
>> +     case IMX7_RESET_PCIEPHY:
>> +             /* wait for more than 10us to release phy g_rst and btnrst */
>> +             udelay(10);
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_G_RST, 0);
>> +             regmap_update_bits(imx7src->regmap,
>> +                                SRC_PCIEPHY_RCR,
>> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN, 0);
>> +             return 0;
>> +     default:
>> +             dev_err(imx7src->dev, "Unknown reset ID %lu\n", id);
>> +             break;
>> +     };
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static const struct reset_control_ops imx7_reset_ops = {
>> +     .assert         = imx7_reset_assert,
>> +     .deassert       = imx7_reset_deassert,
>> +};
>> +
>> +static int imx7_reset_probe(struct platform_device *pdev)
>> +{
>> +     struct imx7_src *imx7src;
>> +     struct device *dev = &pdev->dev;
>> +
>> +     imx7src = devm_kzalloc(dev, sizeof(*imx7src), GFP_KERNEL);
>> +     if (!imx7src)
>> +             return -ENOMEM;
>> +
>> +     imx7src->dev = dev;
>> +     imx7src->regmap = syscon_node_to_regmap(dev->of_node);
>> +     if (IS_ERR(imx7src->regmap)) {
>> +             dev_err(dev, "Unable to get imx7-src regmap");
>> +             return PTR_ERR(imx7src->regmap);
>> +     }
>
> You could use regmap_attach_dev to attach the device to the regmap. This
> would improve error messages, regmap debugfs structure, and make
> imx7src->dev unnecessary.

OK, will do that in v3.

>
>> +
>> +     imx7src->rcdev.owner     = THIS_MODULE;
>> +     imx7src->rcdev.nr_resets = IMX7_RESET_NUM;
>> +     imx7src->rcdev.ops       = &imx7_reset_ops;
>> +     imx7src->rcdev.of_node   = dev->of_node;
>> +
>> +     return devm_reset_controller_register(dev, &imx7src->rcdev);
>> +}
>> +
>> +static const struct of_device_id imx7_reset_dt_ids[] = {
>> +     { .compatible = "fsl,imx7d-src", },
>> +     { /* sentinel */ },
>> +};
>> +
>> +static struct platform_driver imx7_reset_driver = {
>> +     .probe  = imx7_reset_probe,
>> +     .driver = {
>> +             .name           = KBUILD_MODNAME,
>> +             .of_match_table = imx7_reset_dt_ids,
>> +     },
>> +};
>> +builtin_platform_driver(imx7_reset_driver);
>> diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
>> new file mode 100644
>> index 0000000..e20f2db
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/imx7-reset.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Copyright (C) 2017 Impinj, Inc.
>> + *
>> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef DT_BINDING_RESET_IMX7_H
>> +#define DT_BINDING_RESET_IMX7_H
>> +
>> +#define IMX7_RESET_PCIE_CTRL_APPS    0
>> +#define IMX7_RESET_PCIEPHY           1
>
> It would expect these to be numbered in the order they appear in the
> register map, not starting from the end. Could you add all available
> peripheral resets to this list, in the correct order?

The numbering is just a consequence of me adding only resets I could
exercise with my code and numbering then starting from zero. I also
was hesitant adding more sources since it seemed to me that some of
less trivial registers in that IP block might be best represented as a
single reset source doing something more sophisticated that just
setting a bit under the hood.

Thanks,
Andrey Smirnov

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Philipp Zabel]

On Tue, 2017-02-14 at 07:46 -0800, Andrey Smirnov wrote:
[...]
> >> +enum imx7_src_registers {
> >> +     SRC_PCIEPHY_RCR = 0x002c,
> >> +     SRC_PCIEPHY_RCR_PCIEPHY_G_RST     = BIT(1),
> >
> > What about the others resets? There's at least HSICPHY, MIPIPHY and
> > USBOPHY registers before the PCIEPHY register.
> 
> I don't really have any code using anything but PCI reset related
> functionality, so I can't test any of the resets you mention. In light
> of that I didn't want to add any of the definitions that are not going
> to be used anywhere in the code.

Since the device tree bindings are supposed to be descriptive of the
hardware, not of the development history of the linux driver, I'd like
the reset controls to be in a sensible order. From a quick scan of the
chapter in the reference manual, this is the complete list of resets:

A7_CORE_POR_RESET0
A7_CORE_POR_RESET1
A7_CORE_RESET0
A7_CORE_RESET1
A7_DBG_RESET0
A7_DBG_RESET1
A7_ETM_RESET0
A7_ETM_RESET1
A7_SOC_DBG_RESET
A7_L2RESET
SW_M4C_RST
SW_M4P_RST
EIM_RST
HSICPHY_PORT_RST
HSIC_PHY_POR (or Reserved, the manual is conflicted about this)
USBPHY1_POR
USBPHY1_PORT_RST
USBPHY2_POR
USBPHY2_PORT_RST
MIPI_PHY_MRST
MIPI_PHY_SRST
PCIEPHY_G_RST
PCIEPHY_BTN
PCIEPHY_PERST
PCIE_CTRL_APPS_RST/EN

Arguably, the A7 resets should not be handled by the peripheral reset
controller, but at least for the others I see no reason not to leave
space for them in the index table. In fact, since unused reset controls
don't use space, why not number them all?

> >
> >> +     SRC_PCIEPHY_RCR_PCIEPHY_BTN       = BIT(2),
> >> +     SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN = BIT(6),
> >
> > Are those really resets? At least the PCIE_CTRL_APPS_EN has a bit called
> > PCIE_CTRL_APPS_RST right next to it, so this warrants some explanation.
>
> Public documentation for that aspect of i.MX7 is nonexistent and,
> unfortunately, that is my only source of information. Given that, I
> can't really tell you what the difference between PCIE_CTRL_APPS_EN
> and PCIE_CTRL_APPS_RST besides that the former is what downstream PCIe
> driver uses to inhibit LTSSM and the latter is not referenced or used
> by any code (as far as I am aware).

Ok. That's unfortunate, but nothing we can do about that.
After discussing with Lucas, I've come to the belief that APPS_EN stops
the LTSSM, whereas APPS_RST might reset it into the initial state. Since
the latter doesn't exist at all on i.MX6, and we can obviously live
without it, I'm fine with just calling the enable bit an active low
reset to hold it in place. It doesn't fit perfectly, but probably better
to put it here than building i.MX7 specific fabric code around the DW
PCIe driver.

[...]
> >> +             regmap_update_bits(imx7src->regmap,
> >> +                                SRC_PCIEPHY_RCR,
> >> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN,
> >> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN);
> >
> > What is the PCIE PHY button, and why does it have to be set with the
> > reset bit?
> 
> I am sorry, but just as above, I wouldn't be able to enlighten you any
> more about the subject. I have no knowledge about the details of G_RST
> and BTN signal (or even if BTN stands for "button") behavior beyond
> the fact that that is how downstream driver performs PCIEPHY reset.

The SRC_PCIEPHY_RCR register documentation in the i.MX 7Dual
Applications Processor Reference Manual v0.1 describes this bit as "PCIE
PHY button".

[...]
> >> +#define IMX7_RESET_PCIE_CTRL_APPS    0
> >> +#define IMX7_RESET_PCIEPHY           1
> >
> > It would expect these to be numbered in the order they appear in the
> > register map, not starting from the end. Could you add all available
> > peripheral resets to this list, in the correct order?
> 
> The numbering is just a consequence of me adding only resets I could
> exercise with my code and numbering then starting from zero. I also
> was hesitant adding more sources since it seemed to me that some of
> less trivial registers in that IP block might be best represented as a
> single reset source doing something more sophisticated that just
> setting a bit under the hood.

Any in particular?

regards
Philipp

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Andrey Smirnov]

On Tue, Feb 14, 2017 at 8:31 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Tue, 2017-02-14 at 07:46 -0800, Andrey Smirnov wrote:
> [...]
>> >> +enum imx7_src_registers {
>> >> +     SRC_PCIEPHY_RCR = 0x002c,
>> >> +     SRC_PCIEPHY_RCR_PCIEPHY_G_RST     = BIT(1),
>> >
>> > What about the others resets? There's at least HSICPHY, MIPIPHY and
>> > USBOPHY registers before the PCIEPHY register.
>>
>> I don't really have any code using anything but PCI reset related
>> functionality, so I can't test any of the resets you mention. In light
>> of that I didn't want to add any of the definitions that are not going
>> to be used anywhere in the code.
>
> Since the device tree bindings are supposed to be descriptive of the
> hardware, not of the development history of the linux driver, I'd like
> the reset controls to be in a sensible order. From a quick scan of the
> chapter in the reference manual, this is the complete list of resets:
>
> A7_CORE_POR_RESET0
> A7_CORE_POR_RESET1
> A7_CORE_RESET0
> A7_CORE_RESET1
> A7_DBG_RESET0
> A7_DBG_RESET1
> A7_ETM_RESET0
> A7_ETM_RESET1
> A7_SOC_DBG_RESET
> A7_L2RESET
> SW_M4C_RST
> SW_M4P_RST
> EIM_RST
> HSICPHY_PORT_RST
> HSIC_PHY_POR (or Reserved, the manual is conflicted about this)
> USBPHY1_POR
> USBPHY1_PORT_RST
> USBPHY2_POR
> USBPHY2_PORT_RST
> MIPI_PHY_MRST
> MIPI_PHY_SRST
> PCIEPHY_G_RST
> PCIEPHY_BTN
> PCIEPHY_PERST
> PCIE_CTRL_APPS_RST/EN
>
> Arguably, the A7 resets should not be handled by the peripheral reset
> controller, but at least for the others I see no reason not to leave
> space for them in the index table.

If for some bizarre reason one was to run Linux on M4 and use A7 as
applications processor, resetting A7 might be useful. But that's a
very unlikely use-case.

> In fact, since unused reset controls
> don't use space, why not number them all?

IMHO because it is unused code and because those numbers constitute an
interface which once set will be hard to change if need be.

But that's not that important and I don't feel particularly strong
about that point, so I'll add those sources in v3.

Do you insist that I split what I call IMX7_RESET_PCIEPHY into
PCIEPHY_G_RST and PCIEPHY_BTN or can I keep it as a single logical
reset?

>
>> >
>> >> +     SRC_PCIEPHY_RCR_PCIEPHY_BTN       = BIT(2),
>> >> +     SRC_PCIEPHY_RCR_PCIE_CTRL_APPS_EN = BIT(6),
>> >
>> > Are those really resets? At least the PCIE_CTRL_APPS_EN has a bit called
>> > PCIE_CTRL_APPS_RST right next to it, so this warrants some explanation.
>>
>> Public documentation for that aspect of i.MX7 is nonexistent and,
>> unfortunately, that is my only source of information. Given that, I
>> can't really tell you what the difference between PCIE_CTRL_APPS_EN
>> and PCIE_CTRL_APPS_RST besides that the former is what downstream PCIe
>> driver uses to inhibit LTSSM and the latter is not referenced or used
>> by any code (as far as I am aware).
>
> Ok. That's unfortunate, but nothing we can do about that.
> After discussing with Lucas, I've come to the belief that APPS_EN stops
> the LTSSM, whereas APPS_RST might reset it into the initial state. Since
> the latter doesn't exist at all on i.MX6, and we can obviously live
> without it, I'm fine with just calling the enable bit an active low
> reset to hold it in place. It doesn't fit perfectly, but probably better
> to put it here than building i.MX7 specific fabric code around the DW
> PCIe driver.
>
> [...]
>> >> +             regmap_update_bits(imx7src->regmap,
>> >> +                                SRC_PCIEPHY_RCR,
>> >> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN,
>> >> +                                SRC_PCIEPHY_RCR_PCIEPHY_BTN);
>> >
>> > What is the PCIE PHY button, and why does it have to be set with the
>> > reset bit?
>>
>> I am sorry, but just as above, I wouldn't be able to enlighten you any
>> more about the subject. I have no knowledge about the details of G_RST
>> and BTN signal (or even if BTN stands for "button") behavior beyond
>> the fact that that is how downstream driver performs PCIEPHY reset.
>
> The SRC_PCIEPHY_RCR register documentation in the i.MX 7Dual
> Applications Processor Reference Manual v0.1 describes this bit as "PCIE
> PHY button".

Oh, yes, that's right. Missed that the datasheet spells "button" explicitly.

>
> [...]
>> >> +#define IMX7_RESET_PCIE_CTRL_APPS    0
>> >> +#define IMX7_RESET_PCIEPHY           1
>> >
>> > It would expect these to be numbered in the order they appear in the
>> > register map, not starting from the end. Could you add all available
>> > peripheral resets to this list, in the correct order?
>>
>> The numbering is just a consequence of me adding only resets I could
>> exercise with my code and numbering then starting from zero. I also
>> was hesitant adding more sources since it seemed to me that some of
>> less trivial registers in that IP block might be best represented as a
>> single reset source doing something more sophisticated that just
>> setting a bit under the hood.
>
> Any in particular?

USBPHY1/2 and maybe MIPI resets? But that's no more than a gut feeling.

Thanks,
Andrey Smirnov

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Philipp Zabel]

On Tue, 2017-02-14 at 12:11 -0800, Andrey Smirnov wrote:
[...]
> > Arguably, the A7 resets should not be handled by the peripheral reset
> > controller, but at least for the others I see no reason not to leave
> > space for them in the index table.
> 
> If for some bizarre reason one was to run Linux on M4 and use A7 as
> applications processor, resetting A7 might be useful. But that's a
> very unlikely use-case.
>
> > In fact, since unused reset controls
> > don't use space, why not number them all?
> 
> IMHO because it is unused code and because those numbers constitute an
> interface which once set will be hard to change if need be.
> 
> But that's not that important and I don't feel particularly strong
> about that point, so I'll add those sources in v3.

Thanks.

> Do you insist that I split what I call IMX7_RESET_PCIEPHY into
> PCIEPHY_G_RST and PCIEPHY_BTN or can I keep it as a single logical
> reset?

No, I say keep it as is. For now I'll assume this is not a reset, but
some other interface signal that just happens to be routed out to the
SRC and just happens to be toggled around the same time in the enable
sequence.

[...]
> >> >> +#define IMX7_RESET_PCIE_CTRL_APPS    0
> >> >> +#define IMX7_RESET_PCIEPHY           1
> >> >
> >> > It would expect these to be numbered in the order they appear in the
> >> > register map, not starting from the end. Could you add all available
> >> > peripheral resets to this list, in the correct order?
> >>
> >> The numbering is just a consequence of me adding only resets I could
> >> exercise with my code and numbering then starting from zero. I also
> >> was hesitant adding more sources since it seemed to me that some of
> >> less trivial registers in that IP block might be best represented as a
> >> single reset source doing something more sophisticated that just
> >> setting a bit under the hood.
> >
> > Any in particular?
> 
> USBPHY1/2 and maybe MIPI resets? But that's no more than a gut feeling.

Is there any downstream code that already handles these resets? At least
for the USB PHYs I'd expect there has to be something we could look at.

regards
Philipp

Re: [PATCH v2] reset: Add i.MX7 SRC reset driver

[Andrey Smirnov]

On Wed, Feb 15, 2017 at 1:15 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Tue, 2017-02-14 at 12:11 -0800, Andrey Smirnov wrote:
> [...]
>> > Arguably, the A7 resets should not be handled by the peripheral reset
>> > controller, but at least for the others I see no reason not to leave
>> > space for them in the index table.
>>
>> If for some bizarre reason one was to run Linux on M4 and use A7 as
>> applications processor, resetting A7 might be useful. But that's a
>> very unlikely use-case.
>>
>> > In fact, since unused reset controls
>> > don't use space, why not number them all?
>>
>> IMHO because it is unused code and because those numbers constitute an
>> interface which once set will be hard to change if need be.
>>
>> But that's not that important and I don't feel particularly strong
>> about that point, so I'll add those sources in v3.
>
> Thanks.
>
>> Do you insist that I split what I call IMX7_RESET_PCIEPHY into
>> PCIEPHY_G_RST and PCIEPHY_BTN or can I keep it as a single logical
>> reset?
>
> No, I say keep it as is. For now I'll assume this is not a reset, but
> some other interface signal that just happens to be routed out to the
> SRC and just happens to be toggled around the same time in the enable
> sequence.
>
> [...]
>> >> >> +#define IMX7_RESET_PCIE_CTRL_APPS    0
>> >> >> +#define IMX7_RESET_PCIEPHY           1
>> >> >
>> >> > It would expect these to be numbered in the order they appear in the
>> >> > register map, not starting from the end. Could you add all available
>> >> > peripheral resets to this list, in the correct order?
>> >>
>> >> The numbering is just a consequence of me adding only resets I could
>> >> exercise with my code and numbering then starting from zero. I also
>> >> was hesitant adding more sources since it seemed to me that some of
>> >> less trivial registers in that IP block might be best represented as a
>> >> single reset source doing something more sophisticated that just
>> >> setting a bit under the hood.
>> >
>> > Any in particular?
>>
>> USBPHY1/2 and maybe MIPI resets? But that's no more than a gut feeling.
>
> Is there any downstream code that already handles these resets? At least
> for the USB PHYs I'd expect there has to be something we could look at.

Just went and double checked. Of the three that I mentioned, couldn't
find anything besides MIPI and for that only one of the signals was
being used, nothing fancy. I think I'll just expose all of the signals
as discussed above and not worry about it in v3.

Thanks,
Andrey Smirnov