Devicetree
 help / color / mirror / Atom feed
* [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170120155225.31905-1-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This adds a new binding for the Freescale i.MX GPC block, which allows
to describe multiple power domains in a more natural way. The driver
will continue to support the old binding for existing DTBs, but new
features like the additional domains present on i.MX6SX will only be
usable with the new binding.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
index 65cc0345747d..e5b660018d63 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
@@ -1,22 +1,41 @@
 Freescale i.MX General Power Controller
 =======================================
 
-The i.MX6Q General Power Control (GPC) block contains DVFS load tracking
-counters and Power Gating Control (PGC) for the CPU and PU (GPU/VPU) power
-domains.
+The i.MX6 General Power Control (GPC) block contains DVFS load tracking
+counters and Power Gating Control (PGC).
 
 Required properties:
 - compatible: Should be "fsl,imx6q-gpc" or "fsl,imx6sl-gpc"
 - reg: should be register base and length as documented in the
   datasheet
-- interrupts: Should contain GPC interrupt request 1
-- pu-supply: Link to the LDO regulator powering the PU power domain
-- clocks: Clock phandles to devices in the PU power domain that need
-	  to be enabled during domain power-up for reset propagation.
-- #power-domain-cells: Should be 1, see below:
+- interrupts: Should contain one interrupt specifier for the GPC interrupt
+- clocks: Must contain an entry for each entry in clock-names.
+  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - ipg
+- pgc: a list of child nodes describing the SoC power domains controlled by the
+  power gating controller.
 
-The gpc node is a power-controller as documented by the generic power domain
-bindings in Documentation/devicetree/bindings/power/power_domain.txt.
+The power domains are generic power domain providers as documented in
+Documentation/devicetree/bindings/power/power_domain.txt. They are described as
+subnodes of the GPC and should contain the following
+
+Required properties:
+- reg: the DOMAIN_INDEX as used by the client devices to refer to this
+  power domain
+  The following DOMAIN_INDEX values are valid for i.MX6Q:
+  ARM_DOMAIN     0
+  PU_DOMAIN      1
+  The following additional DOMAIN_INDEX value is valid for i.MX6SL:
+  DISPLAY_DOMAIN 2
+
+- #power-domain-cells: Should be 0
+
+Optional properties:
+- clocks: a number of phandles to clocks that need to be enabled during domain
+  power-up sequencing to ensure reset propagation into devices located inside
+  this power domain
+- domain-supply: a phandle to the regulator powering this domain
 
 Example:
 
@@ -25,14 +44,29 @@ Example:
 		reg = <0x020dc000 0x4000>;
 		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 90 IRQ_TYPE_LEVEL_HIGH>;
-		pu-supply = <&reg_pu>;
-		clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-			 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-			 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-			 <&clks IMX6QDL_CLK_VPU_AXI>;
-		#power-domain-cells = <1>;
+		clocks = <&clks IMX6QDL_CLK_IPG>;
+		clock-names = "ipg";
+		
+		pgc {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			power-domain@0 {
+				reg = <0>;
+				#power-domain-cells = <0>;
+			};
+			pd_pu: power-domain@1 {
+				reg = <1>;
+				#power-domain-cells = <0>;
+				domain-supply = <&reg_pu>;
+				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+				         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+				         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+				         <&clks IMX6QDL_CLK_VPU_AXI>;
+			};
+		};
 	};
 
 
@@ -40,20 +74,13 @@ Specifying power domain for IP modules
 ======================================
 
 IP cores belonging to a power domain should contain a 'power-domains' property
-that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
-the power domain the device belongs to.
+that is a phandle pointing to the power domain the device belongs to.
 
 Example of a device that is part of the PU power domain:
 
 	vpu: vpu@02040000 {
 		reg = <0x02040000 0x3c000>;
 		/* ... */
-		power-domains = <&gpc 1>;
+		power-domains = <&pd_pu>;
 		/* ... */
 	};
-
-The following DOMAIN_INDEX values are valid for i.MX6Q:
-ARM_DOMAIN     0
-PU_DOMAIN      1
-The following additional DOMAIN_INDEX value is valid for i.MX6SL:
-DISPLAY_DOMAIN 2
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 2/4] ARM: imx6: remove PGC handling from GPC driver
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170120155225.31905-1-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This removes all functionality regarding the power gating controller
from the GPC driver. This will be added back in a re-written driver
outside of the architecture code.

This keeps only the IRQ controller code in the architecture, as this
is closely coupled to the CPU idle implementation.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/gpc.c | 217 ------------------------------------------------
 1 file changed, 217 deletions(-)

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 1dc2a34b9dbd..93f584ba0130 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -10,26 +10,17 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
-#include <linux/clk.h>
-#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-#include <linux/platform_device.h>
-#include <linux/pm_domain.h>
-#include <linux/regulator/consumer.h>
 #include <linux/irqchip/arm-gic.h>
 #include "common.h"
 #include "hardware.h"
 
-#define GPC_CNTR		0x000
 #define GPC_IMR1		0x008
-#define GPC_PGC_GPU_PDN		0x260
-#define GPC_PGC_GPU_PUPSCR	0x264
-#define GPC_PGC_GPU_PDNSCR	0x268
 #define GPC_PGC_CPU_PDN		0x2a0
 #define GPC_PGC_CPU_PUPSCR	0x2a4
 #define GPC_PGC_CPU_PDNSCR	0x2a8
@@ -39,18 +30,6 @@
 #define IMR_NUM			4
 #define GPC_MAX_IRQS		(IMR_NUM * 32)
 
-#define GPU_VPU_PUP_REQ		BIT(1)
-#define GPU_VPU_PDN_REQ		BIT(0)
-
-#define GPC_CLK_MAX		6
-
-struct pu_domain {
-	struct generic_pm_domain base;
-	struct regulator *reg;
-	struct clk *clk[GPC_CLK_MAX];
-	int num_clks;
-};
-
 static void __iomem *gpc_base;
 static u32 gpc_wake_irqs[IMR_NUM];
 static u32 gpc_saved_imrs[IMR_NUM];
@@ -296,199 +275,3 @@ void __init imx_gpc_check_dt(void)
 		gpc_base = of_iomap(np, 0);
 	}
 }
-
-static void _imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	int iso, iso2sw;
-	u32 val;
-
-	/* Read ISO and ISO2SW power down delays */
-	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PDNSCR);
-	iso = val & 0x3f;
-	iso2sw = (val >> 8) & 0x3f;
-
-	/* Gate off PU domain when GPU/VPU when powered down */
-	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
-
-	/* Request GPC to power down GPU/VPU */
-	val = readl_relaxed(gpc_base + GPC_CNTR);
-	val |= GPU_VPU_PDN_REQ;
-	writel_relaxed(val, gpc_base + GPC_CNTR);
-
-	/* Wait ISO + ISO2SW IPG clock cycles */
-	ndelay((iso + iso2sw) * 1000 / 66);
-}
-
-static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
-
-	_imx6q_pm_pu_power_off(genpd);
-
-	if (pu->reg)
-		regulator_disable(pu->reg);
-
-	return 0;
-}
-
-static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
-{
-	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
-	int i, ret, sw, sw2iso;
-	u32 val;
-
-	if (pu->reg)
-		ret = regulator_enable(pu->reg);
-	if (pu->reg && ret) {
-		pr_err("%s: failed to enable regulator: %d\n", __func__, ret);
-		return ret;
-	}
-
-	/* Enable reset clocks for all devices in the PU domain */
-	for (i = 0; i < pu->num_clks; i++)
-		clk_prepare_enable(pu->clk[i]);
-
-	/* Gate off PU domain when GPU/VPU when powered down */
-	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
-
-	/* Read ISO and ISO2SW power down delays */
-	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PUPSCR);
-	sw = val & 0x3f;
-	sw2iso = (val >> 8) & 0x3f;
-
-	/* Request GPC to power up GPU/VPU */
-	val = readl_relaxed(gpc_base + GPC_CNTR);
-	val |= GPU_VPU_PUP_REQ;
-	writel_relaxed(val, gpc_base + GPC_CNTR);
-
-	/* Wait ISO + ISO2SW IPG clock cycles */
-	ndelay((sw + sw2iso) * 1000 / 66);
-
-	/* Disable reset clocks for all devices in the PU domain */
-	for (i = 0; i < pu->num_clks; i++)
-		clk_disable_unprepare(pu->clk[i]);
-
-	return 0;
-}
-
-static struct generic_pm_domain imx6q_arm_domain = {
-	.name = "ARM",
-};
-
-static struct pu_domain imx6q_pu_domain = {
-	.base = {
-		.name = "PU",
-		.power_off = imx6q_pm_pu_power_off,
-		.power_on = imx6q_pm_pu_power_on,
-	},
-};
-
-static struct generic_pm_domain imx6sl_display_domain = {
-	.name = "DISPLAY",
-};
-
-static struct generic_pm_domain *imx_gpc_domains[] = {
-	&imx6q_arm_domain,
-	&imx6q_pu_domain.base,
-	&imx6sl_display_domain,
-};
-
-static struct genpd_onecell_data imx_gpc_onecell_data = {
-	.domains = imx_gpc_domains,
-	.num_domains = ARRAY_SIZE(imx_gpc_domains),
-};
-
-static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
-{
-	struct clk *clk;
-	int i, ret;
-
-	imx6q_pu_domain.reg = pu_reg;
-
-	for (i = 0; ; i++) {
-		clk = of_clk_get(dev->of_node, i);
-		if (IS_ERR(clk))
-			break;
-		if (i >= GPC_CLK_MAX) {
-			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
-			goto clk_err;
-		}
-		imx6q_pu_domain.clk[i] = clk;
-	}
-	imx6q_pu_domain.num_clks = i;
-
-	/* Enable power always in case bootloader disabled it. */
-	imx6q_pm_pu_power_on(&imx6q_pu_domain.base);
-
-	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
-		return 0;
-
-	imx6q_pu_domain.base.states = devm_kzalloc(dev,
-					sizeof(*imx6q_pu_domain.base.states),
-					GFP_KERNEL);
-	if (!imx6q_pu_domain.base.states)
-		return -ENOMEM;
-
-	imx6q_pu_domain.base.states[0].power_off_latency_ns = 25000;
-	imx6q_pu_domain.base.states[0].power_on_latency_ns = 2000000;
-	imx6q_pu_domain.base.state_count = 1;
-
-	for (i = 0; i < ARRAY_SIZE(imx_gpc_domains); i++)
-		pm_genpd_init(imx_gpc_domains[i], NULL, false);
-
-	ret =  of_genpd_add_provider_onecell(dev->of_node,
-					     &imx_gpc_onecell_data);
-	if (ret)
-		goto power_off;
-
-	return 0;
-
-power_off:
-	imx6q_pm_pu_power_off(&imx6q_pu_domain.base);
-clk_err:
-	while (i--)
-		clk_put(imx6q_pu_domain.clk[i]);
-	imx6q_pu_domain.reg = NULL;
-	return -EINVAL;
-}
-
-static int imx_gpc_probe(struct platform_device *pdev)
-{
-	struct regulator *pu_reg;
-	int ret;
-
-	/* bail out if DT too old and doesn't provide the necessary info */
-	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells"))
-		return 0;
-
-	pu_reg = devm_regulator_get_optional(&pdev->dev, "pu");
-	if (PTR_ERR(pu_reg) == -ENODEV)
-		pu_reg = NULL;
-	if (IS_ERR(pu_reg)) {
-		ret = PTR_ERR(pu_reg);
-		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
-		return ret;
-	}
-
-	return imx_gpc_genpd_init(&pdev->dev, pu_reg);
-}
-
-static const struct of_device_id imx_gpc_dt_ids[] = {
-	{ .compatible = "fsl,imx6q-gpc" },
-	{ .compatible = "fsl,imx6sl-gpc" },
-	{ }
-};
-
-static struct platform_driver imx_gpc_driver = {
-	.driver = {
-		.name = "imx-gpc",
-		.of_match_table = imx_gpc_dt_ids,
-	},
-	.probe = imx_gpc_probe,
-};
-
-static int __init imx_pgc_init(void)
-{
-	return platform_driver_register(&imx_gpc_driver);
-}
-subsys_initcall(imx_pgc_init);
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 3/4] soc/imx: add new GPC driver
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170120155225.31905-1-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This is an almost complete re-write of the previous GPC power gating control
code found in the IMX architecture code. It supports both the old and the new
DT binding, allowing more domains to be added later and generally makes the
driver easier to extend, while keeping compatibility with existing DTBs.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/soc/Makefile     |   1 +
 drivers/soc/imx/Makefile |   1 +
 drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 482 insertions(+)
 create mode 100644 drivers/soc/imx/Makefile
 create mode 100644 drivers/soc/imx/gpc.c

diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 50c23d0bd457..42b3fc7687bb 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -6,6 +6,7 @@ obj-y				+= bcm/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-$(CONFIG_ARCH_MXC)		+= imx/
 obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
 obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
new file mode 100644
index 000000000000..35861f5b2802
--- /dev/null
+++ b/drivers/soc/imx/Makefile
@@ -0,0 +1 @@
+obj-y += gpc.o
diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
new file mode 100644
index 000000000000..24bc0c6b72d8
--- /dev/null
+++ b/drivers/soc/imx/gpc.c
@@ -0,0 +1,480 @@
+/*
+ * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ * Copyright 2011-2013 Freescale Semiconductor, Inc.
+ *
+ * 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>
+
+#define GPC_CNTR		0x000
+
+#define GPC_PGC_PDN_OFFS	0x0
+#define GPC_PGC_PUPSCR_OFFS	0x4
+#define GPC_PGC_PDNSCR_OFFS	0x8
+#define GPC_PGC_SW2ISO_SHIFT	0x8
+#define GPC_PGC_SW_SHIFT	0x0
+
+#define GPC_PGC_GPU_PDN		0x260
+#define GPC_PGC_GPU_PUPSCR	0x264
+#define GPC_PGC_GPU_PDNSCR	0x268
+
+#define GPU_VPU_PUP_REQ		BIT(1)
+#define GPU_VPU_PDN_REQ		BIT(0)
+
+#define GPC_CLK_MAX		6
+
+struct imx_pm_domain {
+	struct generic_pm_domain base;
+	struct regmap *regmap;
+	struct regulator *supply;
+	struct clk *clk[GPC_CLK_MAX];
+	int num_clks;
+	unsigned int reg_offs;
+	signed char cntr_pdn_bit;
+	unsigned int ipg_rate_mhz;
+};
+
+static inline struct imx_pm_domain *
+to_imx_pm_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct imx_pm_domain, base);
+}
+
+static int imx6_pm_domain_power_off(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int iso, iso2sw;
+	u32 val;
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	iso = val & 0x3f;
+	iso2sw = (val >> 8) & 0x3f;
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Request GPC to power down domain */
+	val = BIT(pd->cntr_pdn_bit);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(iso + iso2sw, pd->ipg_rate_mhz));
+
+	if (pd->supply)
+		regulator_disable(pd->supply);
+
+	return 0;
+}
+
+static int imx6_pm_domain_power_on(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int i, ret, sw, sw2iso;
+	u32 val;
+
+	if (pd->supply) {
+		ret = regulator_enable(pd->supply);
+		if (ret) {
+			pr_err("%s: failed to enable regulator: %d\n",
+			       __func__, ret);
+			return ret;
+		}
+	}
+
+	/* Enable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_prepare_enable(pd->clk[i]);
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	sw = val & 0x3f;
+	sw2iso = (val >> 8) & 0x3f;
+
+	/* Request GPC to power up domain */
+	val = BIT(pd->cntr_pdn_bit + 1);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(sw + sw2iso, pd->ipg_rate_mhz));
+
+	/* Disable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_disable_unprepare(pd->clk[i]);
+
+	return 0;
+}
+
+static int imx_pgc_get_clocks(struct device *dev, struct imx_pm_domain *domain)
+{
+	int i, ret;
+
+	for (i = 0; ; i++) {
+		struct clk *clk = of_clk_get(dev->of_node, i);
+		if (IS_ERR(clk))
+			break;
+		if (i >= GPC_CLK_MAX) {
+			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
+			ret = -EINVAL;
+			goto clk_err;
+		}
+		domain->clk[i] = clk;
+	}
+	domain->num_clks = i;
+
+	return 0;
+
+clk_err:
+	for (; i >= 0; i--)
+		clk_put(domain->clk[i]);
+
+	return ret;
+}
+
+static void imx_pgc_put_clocks(struct imx_pm_domain *domain)
+{
+	int i;
+
+	for (i = domain->num_clks - 1; i >= 0; i--)
+		clk_put(domain->clk[i]);
+}
+
+static int imx_pgc_parse_dt(struct device *dev, struct imx_pm_domain *domain)
+{
+	/* try to get the domain supply regulator */
+	domain->supply = devm_regulator_get_optional(dev, "domain");
+	if (IS_ERR(domain->supply)) {
+		if (PTR_ERR(domain->supply) == -ENODEV)
+			domain->supply = NULL;
+		else
+			return PTR_ERR(domain->supply);
+	}
+
+	/* try to get all clocks needed for reset propagation */
+	return imx_pgc_get_clocks(dev, domain);
+}
+
+static int imx_pgc_power_domain_probe(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	/* if this PD is associated with a DT node try to parse it */
+	if (dev->of_node) {
+		ret = imx_pgc_parse_dt(dev, domain);
+		if (ret)
+			return ret;
+	}
+
+	/* initially power on the domain */
+	if (domain->base.power_on)
+		domain->base.power_on(&domain->base);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		pm_genpd_init(&domain->base, NULL, false);
+		ret = of_genpd_add_provider_simple(dev->of_node, &domain->base);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&domain->base);
+	imx_pgc_put_clocks(domain);
+
+	return ret;
+}
+
+static int imx_pgc_power_domain_remove(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		of_genpd_del_provider(pdev->dev.of_node);
+		pm_genpd_remove(&domain->base);
+		imx_pgc_put_clocks(domain);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id imx_pgc_power_domain_id[] = {
+	{ "imx-pgc-power-domain"},
+	{ },
+};
+
+static struct platform_driver imx_pgc_power_domain_driver = {
+	.driver = {
+		.name = "imx-pgc-pd",
+	},
+	.probe = imx_pgc_power_domain_probe,
+	.remove = imx_pgc_power_domain_remove,
+	.id_table = imx_pgc_power_domain_id,
+};
+builtin_platform_driver(imx_pgc_power_domain_driver)
+
+static struct genpd_power_state imx6_pm_domain_pu_state = {
+	.power_off_latency_ns = 25000,
+	.power_on_latency_ns = 2000000,
+};
+
+static struct imx_pm_domain imx_gpc_domains[] = {
+	{
+		.base = {
+			.name = "ARM",
+		},
+	}, {
+		.base = {
+			.name = "PU",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+			.states = &imx6_pm_domain_pu_state,
+			.state_count = 1,
+		},
+		.reg_offs = 0x260,
+		.cntr_pdn_bit = 0,
+	}, {
+		.base = {
+			.name = "DISPLAY",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+		},
+		.reg_offs = 0x240,
+		.cntr_pdn_bit = 4,
+	}
+};
+
+struct imx_gpc_dt_data {
+	int num_domains;
+};
+
+static const struct imx_gpc_dt_data imx6q_dt_data = {
+	.num_domains = 2,
+};
+
+static const struct imx_gpc_dt_data imx6sl_dt_data = {
+	.num_domains = 3,
+};
+
+static const struct of_device_id imx_gpc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
+	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
+	{ }
+};
+
+static bool imx_gpc_readable_reg(struct device *dev, unsigned int reg)
+{
+	return (reg % 4 == 0) && (reg <= 0x2ac);
+}
+
+static bool imx_gpc_volatile_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == GPC_CNTR)
+		return true;
+
+	return false;
+}
+
+static const struct regmap_config imx_gpc_regmap_config = {
+	.cache_type = REGCACHE_FLAT,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+
+	.readable_reg = imx_gpc_readable_reg,
+	.volatile_reg = imx_gpc_volatile_reg,
+
+	.max_register = 0x2ac,
+};
+
+static struct generic_pm_domain *imx_gpc_onecell_domains[] = {
+	&imx_gpc_domains[0].base,
+	&imx_gpc_domains[1].base,
+};
+
+static struct genpd_onecell_data imx_gpc_onecell_data = {
+	.domains = imx_gpc_onecell_domains,
+	.num_domains = 2,
+};
+
+static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
+{
+	struct imx_pm_domain *domain;
+	int i, ret;
+
+	for (i = 0; i < 2; i++) {
+		domain = &imx_gpc_domains[i];
+		domain->regmap = regmap;
+		domain->ipg_rate_mhz = 33;
+
+		if (i == 1) {
+			domain->supply = devm_regulator_get(dev, "pu");
+			if (IS_ERR(domain->supply))
+				return PTR_ERR(domain->supply);;
+
+			ret = imx_pgc_get_clocks(dev, domain);
+			if (ret)
+				goto clk_err;
+
+			domain->base.power_on(&domain->base);
+		}
+	}
+
+	for (i = 0; i < 2; i++)
+		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		ret = of_genpd_add_provider_onecell(dev->of_node,
+						    &imx_gpc_onecell_data);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&imx_gpc_domains[1].base);
+	imx_pgc_put_clocks(&imx_gpc_domains[1]);
+clk_err:
+	pm_genpd_remove(&imx_gpc_domains[0].base);
+
+	return ret;
+}
+
+static int imx_gpc_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(imx_gpc_dt_ids, &pdev->dev);
+	const struct imx_gpc_dt_data *of_id_data = of_id->data;
+	struct device_node *pgc_node;
+	struct regmap *regmap;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
+
+	/* bail out if DT too old and doesn't provide the necessary info */
+	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
+	    !pgc_node)
+		return 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
+					   &imx_gpc_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&pdev->dev, "failed to init regmap: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (!pgc_node) {
+		/* old DT layout is only supported for mx6q aka 2 domains */
+		if (of_id_data->num_domains != 2) {
+			dev_err(&pdev->dev, "could not find pgc DT node\n");
+			return -ENODEV;
+		}
+
+		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
+		if (ret)
+			return ret;
+	} else {
+		struct imx_pm_domain *domain;
+		struct platform_device *pd_pdev;
+		struct device_node *np;
+		struct clk *ipg_clk;
+		unsigned int ipg_rate_mhz;
+		int domain_index;
+
+		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(ipg_clk))
+			return PTR_ERR(ipg_clk);
+		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
+
+		for_each_child_of_node(pgc_node, np) {
+			ret = of_property_read_u32(np, "reg", &domain_index);
+			if (ret) {
+				of_node_put(np);
+				return ret;
+			}
+
+			domain = &imx_gpc_domains[domain_index];
+			domain->regmap = regmap;
+			domain->ipg_rate_mhz = ipg_rate_mhz;
+
+			pd_pdev = platform_device_alloc("imx-pgc-power-domain",
+							domain_index);
+			pd_pdev->dev.platform_data = domain;
+			pd_pdev->dev.parent = &pdev->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 int imx_gpc_remove(struct platform_device *pdev)
+{
+	int ret;
+
+	/*
+	 * If the old DT binding is used the toplevel driver needs to
+	 * de-register the power domains
+	 */
+	if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) {
+		of_genpd_del_provider(pdev->dev.of_node);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[1].base);
+		if (ret)
+			return ret;
+		imx_pgc_put_clocks(&imx_gpc_domains[1]);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[0].base);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver imx_gpc_driver = {
+	.driver = {
+		.name = "imx-gpc",
+		.of_match_table = imx_gpc_dt_ids,
+	},
+	.probe = imx_gpc_probe,
+	.remove = imx_gpc_remove,
+};
+builtin_platform_driver(imx_gpc_driver)
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 4/4] ARM: imx6: adopt DT to new GPC binding
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170120155225.31905-1-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Adopt the i.MX6Q/DL DT to the new and more flexible GPC binding.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6q.dtsi   |  2 +-
 arch/arm/boot/dts/imx6qdl.dtsi | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9a5d0b8c7b0..dd33849335b2 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -125,7 +125,7 @@
 			clocks = <&clks IMX6QDL_CLK_OPENVG_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		ipu2: ipu@02800000 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 53e6e63cbb02..ce7069be5852 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1,3 +1,4 @@
+
 /*
  * Copyright 2011 Freescale Semiconductor, Inc.
  * Copyright 2011 Linaro Ltd.
@@ -148,7 +149,7 @@
 				 <&clks IMX6QDL_CLK_GPU3D_CORE>,
 				 <&clks IMX6QDL_CLK_GPU3D_SHADER>;
 			clock-names = "bus", "core", "shader";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		gpu_2d: gpu@00134000 {
@@ -158,7 +159,7 @@
 			clocks = <&clks IMX6QDL_CLK_GPU2D_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		timer@00a00600 {
@@ -425,7 +426,7 @@
 				clocks = <&clks IMX6QDL_CLK_VPU_AXI>,
 					 <&clks IMX6QDL_CLK_MMDC_CH0_AXI>;
 				clock-names = "per", "ahb";
-				power-domains = <&gpc 1>;
+				power-domains = <&pd_pu>;
 				resets = <&src 1>;
 				iram = <&ocram>;
 			};
@@ -788,14 +789,29 @@
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 90 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-parent = <&intc>;
-				pu-supply = <&reg_pu>;
-				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-					 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-					 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-					 <&clks IMX6QDL_CLK_VPU_AXI>;
-				#power-domain-cells = <1>;
+				clocks = <&clks IMX6QDL_CLK_IPG>;
+				clock-names = "ipg";
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					power-domain@0 {
+						reg = <0>;
+						#power-domain-cells = <0>;
+					};
+					pd_pu: power-domain@1 {
+						reg = <1>;
+						#power-domain-cells = <0>;
+						domain-supply = <&reg_pu>;
+						clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+						         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+						         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+						         <&clks IMX6QDL_CLK_VPU_AXI>;
+					};
+				};
 			};
 
 			gpr: iomuxc-gpr@020e0000 {
-- 
2.11.0

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

^ permalink raw reply related

* Re: [PATCH 2/3 v2] clk: ux500: Add device tree bindings for ABx500 clocks
From: Ulf Hansson @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michael Turquette, Stephen Boyd, linux-clk,
	devicetree@vger.kernel.org
In-Reply-To: <20170120132539.3013-1-linus.walleij@linaro.org>

On 20 January 2017 at 14:25, Linus Walleij <linus.walleij@linaro.org> wrote:
> This adds device tree bindings for the ABx500 clocks on the
> ST-Ericsson platforms.
>
> Cc: devicetree@vger.kernel.org
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Stephen, can pick this up once Rob acked it?

Kind regards
Uffe


> ---
> ChangeLog v1->v2:
> - Indicate that the abx500 clocks need to be in a device tree
>   node under an ABx500 MFD device.
> - Rename the example node to clock-controller.
> ---
>  .../devicetree/bindings/clock/stericsson,abx500.txt  | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/stericsson,abx500.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/stericsson,abx500.txt b/Documentation/devicetree/bindings/clock/stericsson,abx500.txt
> new file mode 100644
> index 000000000000..dbaa886b223e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/stericsson,abx500.txt
> @@ -0,0 +1,20 @@
> +Clock bindings for ST-Ericsson ABx500 clocks
> +
> +Required properties :
> +- compatible : shall contain the following:
> +  "stericsson,ab8500-clk"
> +- #clock-cells should be <1>
> +
> +The ABx500 clocks need to be placed as a subnode of an AB8500
> +device node, see mfd/ab8500.txt
> +
> +All available clocks are defined as preprocessor macros in
> +dt-bindings/clock/ste-ab8500.h header and can be used in device
> +tree sources.
> +
> +Example:
> +
> +clock-controller {
> +       compatible = "stericsson,ab8500-clk";
> +       #clock-cells = <1>;
> +};
> --
> 2.9.3
>

^ permalink raw reply

* [PATCH 0/6] rtc: sun6i: Fix the RTC accuracy
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard

Hi,

The RTC used in the A31 and later SoC has an accuracy issue, which is
already significant even after a couple of hours.

This is due to the fact that the oscillator used by default is an internal
and very inaccurate one.

A first attempt at fixing that by switching to the external oscillator was
done in the patch "rtc: sun6i: Switch to the external oscillator". However,
it turned out to be problematic since it was tracked properly in the clock
framework, which might lead to some clocks being disabled, even though
their devices were not notified.

This is a second attempt, this time by making it part of the CCF. It
turned out to be a bit more complicated than one would expect since the mux
found inside the RTC also controls one of the input of the main clock unit.
Therefore, it needs to be probed before the main clock unit driver.

Let me know what you think,
Maxime

Maxime Ripard (6):
  rtc: sun6i: Expose the 32kHz oscillator
  rtc: sun6i: Add some locking
  rtc: sun6i: Disable the build as a module
  rtc: sun6i: Force the mux to the external oscillator
  ARM: sun8i: a23/a33: Enable the real LOSC and use it
  ARM: sun8i: a23/a33: Add the oscillators accuracy

 Documentation/devicetree/bindings/rtc/sun6i-rtc.txt |   8 +-
 arch/arm/boot/dts/sun8i-a23-a33.dtsi                |  15 +-
 drivers/rtc/Kconfig                                 |   2 +-
 drivers/rtc/rtc-sun6i.c                             | 189 +++++++++++--
 4 files changed, 181 insertions(+), 33 deletions(-)

base-commit: 99cef370ac9939df2aeb16c96d07e842b2fa8201
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* [PATCH 1/6] rtc: sun6i: Expose the 32kHz oscillator
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The RTC controls the input source of the main 32kHz oscillator in the
system, feeding it to the clock unit too.

By default, this is using an internal, very inaccurate (+/- 30%)
oscillator with a divider to make it roughly around 32kHz. This is however
quite impractical for the RTC, since our time will not be tracked properly.

Since this oscillator is an input of the main clock unit, and since that
clock unit will be probed using CLK_OF_DECLARE, we have to use it as well,
leading to a two stage probe: one to enable the clock, the other one to
enable the RTC.

There is also a slight change in the binding that is required (and should
have been from the beginning), since we'll need a phandle to the external
oscillator used on that board. We support the old binding by not allowing
to switch to the external oscillator and only using the internal one (which
was the previous behaviour) in the case where we're missing that phandle.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/rtc/sun6i-rtc.txt |   8 +-
 drivers/rtc/rtc-sun6i.c                             | 140 ++++++++++++-
 2 files changed, 139 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
index f007e428a1ab..44d788240162 100644
--- a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
@@ -8,10 +8,18 @@ Required properties:
 		  memory mapped region.
 - interrupts	: IRQ lines for the RTC alarm 0 and alarm 1, in that order.
 
+Required properties for new device trees
+- clocks	: phandle to the 32kHz external oscillator
+- clock-output-names : name of the LOSC clock created
+- #clock-cells  : must be equals to 0
+
 Example:
 
 rtc: rtc@01f00000 {
 	compatible = "allwinner,sun6i-a31-rtc";
 	reg = <0x01f00000 0x54>;
 	interrupts = <0 40 4>, <0 41 4>;
+	clock-output-names = "osc32k";
+	clocks = <&ext_osc32k>;
+	#clock-cells = <0>;
 };
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index c169a2cd4727..408dd512a6ac 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -20,6 +20,8 @@
  * more details.
  */
 
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/fs.h>
@@ -33,15 +35,20 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/rtc.h>
+#include <linux/slab.h>
 #include <linux/types.h>
 
 /* Control register */
 #define SUN6I_LOSC_CTRL				0x0000
+#define SUN6I_LOSC_CTRL_KEY			(0x16aa << 16)
 #define SUN6I_LOSC_CTRL_ALM_DHMS_ACC		BIT(9)
 #define SUN6I_LOSC_CTRL_RTC_HMS_ACC		BIT(8)
 #define SUN6I_LOSC_CTRL_RTC_YMD_ACC		BIT(7)
+#define SUN6I_LOSC_CTRL_EXT_OSC			BIT(0)
 #define SUN6I_LOSC_CTRL_ACC_MASK		GENMASK(9, 7)
 
+#define SUN6I_LOSC_CLK_PRESCAL			0x0008
+
 /* RTC */
 #define SUN6I_RTC_YMD				0x0010
 #define SUN6I_RTC_HMS				0x0014
@@ -114,8 +121,128 @@ struct sun6i_rtc_dev {
 	void __iomem *base;
 	int irq;
 	unsigned long alarm;
+
+	struct clk_hw hw;
+	struct clk_hw *int_osc;
+	struct clk *losc;
 };
 
+static struct sun6i_rtc_dev *sun6i_rtc;
+
+static unsigned long sun6i_rtc_osc_recalc_rate(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	struct sun6i_rtc_dev *rtc = container_of(hw, struct sun6i_rtc_dev, hw);
+	u32 val;
+
+	val = readl(rtc->base + SUN6I_LOSC_CTRL);
+	if (val & SUN6I_LOSC_CTRL_EXT_OSC)
+		return parent_rate;
+
+	val = readl(rtc->base + SUN6I_LOSC_CLK_PRESCAL);
+	val &= GENMASK(4, 0);
+
+	return parent_rate / (val + 1);
+}
+
+static u8 sun6i_rtc_osc_get_parent(struct clk_hw *hw)
+{
+	struct sun6i_rtc_dev *rtc = container_of(hw, struct sun6i_rtc_dev, hw);
+
+	return readl(rtc->base + SUN6I_LOSC_CTRL) & SUN6I_LOSC_CTRL_EXT_OSC;
+}
+
+static int sun6i_rtc_osc_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct sun6i_rtc_dev *rtc = container_of(hw, struct sun6i_rtc_dev, hw);
+	u32 val;
+
+	if (index > 1)
+		return -EINVAL;
+
+	val = readl(rtc->base + SUN6I_LOSC_CTRL);
+	val &= ~SUN6I_LOSC_CTRL_EXT_OSC;
+	val |= SUN6I_LOSC_CTRL_KEY;
+	val |= index ? SUN6I_LOSC_CTRL_EXT_OSC : 0;
+	writel(val, rtc->base + SUN6I_LOSC_CTRL);
+
+	return 0;
+}
+
+static const struct clk_ops sun6i_rtc_osc_ops = {
+	.recalc_rate	= sun6i_rtc_osc_recalc_rate,
+
+	.get_parent	= sun6i_rtc_osc_get_parent,
+	.set_parent	= sun6i_rtc_osc_set_parent,
+};
+
+static void __init sun6i_rtc_clk_init(struct device_node *node)
+{
+	struct sun6i_rtc_dev *rtc;
+	struct clk_init_data init = {
+		.ops		= &sun6i_rtc_osc_ops,
+	};
+	const char *parents[2];
+
+	rtc = kzalloc(sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		pr_crit("Can't allocate RTC structure\n");
+
+	rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (!rtc->base) {
+		pr_crit("Can't map RTC registers");
+		return;
+	}
+
+	rtc->int_osc = clk_hw_register_fixed_rate_with_accuracy(NULL,
+								"rtc-int-osc",
+								NULL, 0,
+								667000,
+								300000000);
+	if (IS_ERR(rtc->int_osc)) {
+		pr_crit("Couldn't register the internal oscillator\n");
+		return;
+	}
+
+	/*
+	 * Due to the missing clocks property we had to express the
+	 * parenthood with the external oscillator that cannot fix (or
+	 * at least expect to have) in the old DTs, we have to be a
+	 * bit smart here.
+	 *
+	 * We deal with that by simply registering the internal
+	 * oscillator as our parent in all cases, and try to get the
+	 * external oscillator from the DT.
+	 *
+	 * In the case where we don't have it, of_clk_get_parent_name
+	 * will return NULL, which is ok, and of_clk_get_parent_count
+	 * will return 0, which is fine too. We will just register a
+	 * single parent, everything works.
+	 */
+	parents[0] = clk_hw_get_name(rtc->int_osc);
+	parents[1] = of_clk_get_parent_name(node, 0);
+
+	rtc->hw.init = &init;
+
+	init.parent_names = parents;
+	init.num_parents = of_clk_get_parent_count(node) + 1;
+	init.name = "rtc-osc";
+	of_property_read_string(node, "clock-output-names", &init.name);
+
+	rtc->losc = clk_register(NULL, &rtc->hw);
+	if (IS_ERR(rtc->losc)) {
+		pr_crit("Couldn't register the LOSC clock\n");
+		return;
+	}
+
+	of_clk_add_hw_provider(node, of_clk_hw_simple_get, &rtc->hw);
+
+	/* Yes, I know, this is ugly. */
+	sun6i_rtc = rtc;
+}
+CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
+		      sun6i_rtc_clk_init);
+
 static irqreturn_t sun6i_rtc_alarmirq(int irq, void *id)
 {
 	struct sun6i_rtc_dev *chip = (struct sun6i_rtc_dev *) id;
@@ -349,22 +476,15 @@ static const struct rtc_class_ops sun6i_rtc_ops = {
 
 static int sun6i_rtc_probe(struct platform_device *pdev)
 {
-	struct sun6i_rtc_dev *chip;
-	struct resource *res;
+	struct sun6i_rtc_dev *chip = sun6i_rtc;
 	int ret;
 
-	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
-		return -ENOMEM;
+		return -ENODEV;
 
 	platform_set_drvdata(pdev, chip);
 	chip->dev = &pdev->dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	chip->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(chip->base))
-		return PTR_ERR(chip->base);
-
 	chip->irq = platform_get_irq(pdev, 0);
 	if (chip->irq < 0) {
 		dev_err(&pdev->dev, "No IRQ resource\n");
@@ -404,6 +524,8 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
 	/* disable alarm wakeup */
 	writel(0, chip->base + SUN6I_ALARM_CONFIG);
 
+	clk_prepare_enable(chip->losc);
+
 	chip->rtc = rtc_device_register("rtc-sun6i", &pdev->dev,
 					&sun6i_rtc_ops, THIS_MODULE);
 	if (IS_ERR(chip->rtc)) {
-- 
git-series 0.8.11
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/6] rtc: sun6i: Add some locking
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Some registers have a read-modify-write access pattern that are not atomic.

Add some locking to prevent from concurrent accesses.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/rtc/rtc-sun6i.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 408dd512a6ac..872d18609183 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -125,6 +125,8 @@ struct sun6i_rtc_dev {
 	struct clk_hw hw;
 	struct clk_hw *int_osc;
 	struct clk *losc;
+
+	spinlock_t lock;
 };
 
 static struct sun6i_rtc_dev *sun6i_rtc;
@@ -155,16 +157,19 @@ static u8 sun6i_rtc_osc_get_parent(struct clk_hw *hw)
 static int sun6i_rtc_osc_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct sun6i_rtc_dev *rtc = container_of(hw, struct sun6i_rtc_dev, hw);
+	unsigned long flags;
 	u32 val;
 
 	if (index > 1)
 		return -EINVAL;
 
+	spin_lock_irqsave(&rtc->lock, flags);
 	val = readl(rtc->base + SUN6I_LOSC_CTRL);
 	val &= ~SUN6I_LOSC_CTRL_EXT_OSC;
 	val |= SUN6I_LOSC_CTRL_KEY;
 	val |= index ? SUN6I_LOSC_CTRL_EXT_OSC : 0;
 	writel(val, rtc->base + SUN6I_LOSC_CTRL);
+	spin_unlock_irqrestore(&rtc->lock, flags);
 
 	return 0;
 }
@@ -187,6 +192,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
 	rtc = kzalloc(sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
 		pr_crit("Can't allocate RTC structure\n");
+	spin_lock_init(&rtc->lock);
 
 	rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
 	if (!rtc->base) {
@@ -246,8 +252,10 @@ CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
 static irqreturn_t sun6i_rtc_alarmirq(int irq, void *id)
 {
 	struct sun6i_rtc_dev *chip = (struct sun6i_rtc_dev *) id;
+	irqreturn_t ret = IRQ_NONE;
 	u32 val;
 
+	spin_lock(&chip->lock);
 	val = readl(chip->base + SUN6I_ALRM_IRQ_STA);
 
 	if (val & SUN6I_ALRM_IRQ_STA_CNT_IRQ_PEND) {
@@ -256,10 +264,11 @@ static irqreturn_t sun6i_rtc_alarmirq(int irq, void *id)
 
 		rtc_update_irq(chip->rtc, 1, RTC_AF | RTC_IRQF);
 
-		return IRQ_HANDLED;
+		ret = IRQ_HANDLED;
 	}
+	spin_unlock(&chip->lock);
 
-	return IRQ_NONE;
+	return ret;
 }
 
 static void sun6i_rtc_setaie(int to, struct sun6i_rtc_dev *chip)
@@ -267,6 +276,7 @@ static void sun6i_rtc_setaie(int to, struct sun6i_rtc_dev *chip)
 	u32 alrm_val = 0;
 	u32 alrm_irq_val = 0;
 	u32 alrm_wake_val = 0;
+	unsigned long flags;
 
 	if (to) {
 		alrm_val = SUN6I_ALRM_EN_CNT_EN;
@@ -277,9 +287,11 @@ static void sun6i_rtc_setaie(int to, struct sun6i_rtc_dev *chip)
 		       chip->base + SUN6I_ALRM_IRQ_STA);
 	}
 
+	spin_lock_irqsave(&chip->lock, flags);
 	writel(alrm_val, chip->base + SUN6I_ALRM_EN);
 	writel(alrm_irq_val, chip->base + SUN6I_ALRM_IRQ_EN);
 	writel(alrm_wake_val, chip->base + SUN6I_ALARM_CONFIG);
+	spin_unlock_irqrestore(&chip->lock, flags);
 }
 
 static int sun6i_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
@@ -318,11 +330,15 @@ static int sun6i_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 static int sun6i_rtc_getalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
 {
 	struct sun6i_rtc_dev *chip = dev_get_drvdata(dev);
+	unsigned long flags;
 	u32 alrm_st;
 	u32 alrm_en;
 
+	spin_lock_irqsave(&chip->lock, flags);
 	alrm_en = readl(chip->base + SUN6I_ALRM_IRQ_EN);
 	alrm_st = readl(chip->base + SUN6I_ALRM_IRQ_STA);
+	spin_unlock_irqrestore(&chip->lock, flags);
+
 	wkalrm->enabled = !!(alrm_en & SUN6I_ALRM_EN_CNT_EN);
 	wkalrm->pending = !!(alrm_st & SUN6I_ALRM_EN_CNT_EN);
 	rtc_time_to_tm(chip->alarm, &wkalrm->time);
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH 3/6] rtc: sun6i: Disable the build as a module
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Since we have to provide the clock very early on, the RTC driver cannot be
built as a module. Make sure that won't happen.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/rtc/Kconfig     |  2 +-
 drivers/rtc/rtc-sun6i.c | 17 +----------------
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index c93c5a8fba32..53e35c138ff3 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1434,7 +1434,7 @@ config RTC_DRV_SUN4V
 	  based RTC on SUN4V systems.
 
 config RTC_DRV_SUN6I
-	tristate "Allwinner A31 RTC"
+	bool "Allwinner A31 RTC"
 	default MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
 	depends on ARCH_SUNXI
 	help
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 872d18609183..edd5627da10f 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -554,15 +554,6 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int sun6i_rtc_remove(struct platform_device *pdev)
-{
-	struct sun6i_rtc_dev *chip = platform_get_drvdata(pdev);
-
-	rtc_device_unregister(chip->rtc);
-
-	return 0;
-}
-
 static const struct of_device_id sun6i_rtc_dt_ids[] = {
 	{ .compatible = "allwinner,sun6i-a31-rtc" },
 	{ /* sentinel */ },
@@ -571,15 +562,9 @@ MODULE_DEVICE_TABLE(of, sun6i_rtc_dt_ids);
 
 static struct platform_driver sun6i_rtc_driver = {
 	.probe		= sun6i_rtc_probe,
-	.remove		= sun6i_rtc_remove,
 	.driver		= {
 		.name		= "sun6i-rtc",
 		.of_match_table = sun6i_rtc_dt_ids,
 	},
 };
-
-module_platform_driver(sun6i_rtc_driver);
-
-MODULE_DESCRIPTION("sun6i RTC driver");
-MODULE_AUTHOR("Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>");
-MODULE_LICENSE("GPL");
+builtin_platform_driver(sun6i_rtc_driver);
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH 4/6] rtc: sun6i: Force the mux to the external oscillator
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The internal oscillator is way too inaccurate to do something useful with
it. Switch to the external oscillator if it is available.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/rtc/rtc-sun6i.c | 12 ++++++++++++
 1 file changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index edd5627da10f..1695fae24cd5 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -493,6 +493,7 @@ static const struct rtc_class_ops sun6i_rtc_ops = {
 static int sun6i_rtc_probe(struct platform_device *pdev)
 {
 	struct sun6i_rtc_dev *chip = sun6i_rtc;
+	struct clk *parent;
 	int ret;
 
 	if (!chip)
@@ -540,6 +541,17 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
 	/* disable alarm wakeup */
 	writel(0, chip->base + SUN6I_ALARM_CONFIG);
 
+	parent = clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(parent)) {
+		ret = clk_set_parent(chip->losc, parent);
+		clk_put(parent);
+
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Failed to reparent the RTC to the external oscillator\n");
+			return ret;
+		}
+	}
 	clk_prepare_enable(chip->losc);
 
 	chip->rtc = rtc_device_register("rtc-sun6i", &pdev->dev,
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH 5/6] ARM: sun8i: a23/a33: Enable the real LOSC and use it
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

So far, the LOSC was generated through the RTC internal oscillator, which
was a pretty poor and inaccurate choice.

Now that the RTC properly exposes its internal mux between its oscillator
and the external oscillator, we can use it were relevant.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 597f2fce6c31..e9d98d803dc6 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -109,11 +109,11 @@
 			clock-output-names = "osc24M";
 		};
 
-		osc32k: osc32k_clk {
+		ext_osc32k: ext_osc32k_clk {
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
 			clock-frequency = <32768>;
-			clock-output-names = "osc32k";
+			clock-output-names = "ext-osc32k";
 		};
 	};
 
@@ -256,7 +256,7 @@
 
 		ccu: clock@01c20000 {
 			reg = <0x01c20000 0x400>;
-			clocks = <&osc24M>, <&osc32k>;
+			clocks = <&osc24M>, <&rtc>;
 			clock-names = "hosc", "losc";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
@@ -266,7 +266,7 @@
 			/* compatible gets set in SoC specific dtsi file */
 			reg = <0x01c20800 0x400>;
 			/* interrupts get set in SoC specific dtsi file */
-			clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&osc32k>;
+			clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&rtc>;
 			clock-names = "apb", "hosc", "losc";
 			gpio-controller;
 			interrupt-controller;
@@ -486,6 +486,9 @@
 			reg = <0x01f00000 0x54>;
 			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+			clock-output-names = "osc32k";
+			clocks = <&ext_osc32k>;
+			#clock-cells = <0>;
 		};
 
 		nmi_intc: interrupt-controller@01f00c0c {
@@ -564,7 +567,7 @@
 			compatible = "allwinner,sun8i-a23-r-pinctrl";
 			reg = <0x01f02c00 0x400>;
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&apb0_gates 0>, <&osc24M>, <&osc32k>;
+			clocks = <&apb0_gates 0>, <&osc24M>, <&rtc>;
 			clock-names = "apb", "hosc", "losc";
 			resets = <&apb0_rst 0>;
 			gpio-controller;
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH 6/6] ARM: sun8i: a23/a33: Add the oscillators accuracy
From: Maxime Ripard @ 2017-01-20 15:56 UTC (permalink / raw)
  To: Alexandre Belloni, Alessandro Zummo, Chen-Yu Tsai
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard
In-Reply-To: <cover.180d5ae6bc36ead9625d208824f5cbd7114cf92e.1484927680.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The datasheet provided by Allwinner requires oscillators with an accuracy
of 50ppm. Add it to our fixed clocks so that we can properly track the
accuracy chain.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 ++
 1 file changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index e9d98d803dc6..f5925a18031c 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -106,6 +106,7 @@
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
 			clock-frequency = <24000000>;
+			clock-accuracy = <50000>;
 			clock-output-names = "osc24M";
 		};
 
@@ -113,6 +114,7 @@
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
 			clock-frequency = <32768>;
+			clock-accuracy = <50000>;
 			clock-output-names = "ext-osc32k";
 		};
 	};
-- 
git-series 0.8.11

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* Re: [PATCH 4/8] clk: renesas: cpg-mssr: Add support for reset control
From: Philipp Zabel @ 2017-01-20 15:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland, devicetree, Magnus Damm, Michael Turquette,
	Stephen Boyd, linux-kernel, Rob Herring, linux-renesas-soc,
	Simon Horman, linux-clk, linux-arm-kernel
In-Reply-To: <1484921306-9967-5-git-send-email-geert+renesas@glider.be>

Hi Geert,

On Fri, 2017-01-20 at 15:08 +0100, Geert Uytterhoeven wrote:
> Add optional support for the Reset Control feature of the Renesas Clock
> Pulse Generator / Module Standby and Software Reset module on R-Car
> Gen2, R-Car Gen3, and RZ/G1 SoCs.

Is there a reason to make this optional?

> This allows to reset SoC devices using the Reset Controller API.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Looks good to me,

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Just a small issue below,

> ---
>  drivers/clk/renesas/renesas-cpg-mssr.c | 122 +++++++++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index f1161a585c57e433..ea4af714ac14603a 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
[...]
> +static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +	unsigned int reg = id / 32;
> +	unsigned int bit = id % 32;
> +	u32 bitmask = BIT(bit);

Here you have a bitmask = BIT(bit) variable.

> +	unsigned long flags;
> +	u32 value;
> +
> +	dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
> +
> +	/* Reset module */
> +	spin_lock_irqsave(&priv->rmw_lock, flags);
> +	value = readl(priv->base + SRCR(reg));
> +	value |= bitmask;

Here you use it.

> +	writel(value, priv->base + SRCR(reg));
> +	spin_unlock_irqrestore(&priv->rmw_lock, flags);
> +
> +	/* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
> +	udelay(35);
> +
> +	/* Release module from reset state */
> +	writel(bitmask, priv->base + SRSTCLR(reg));
> +
> +	return 0;
> +}
> +
> +static int cpg_mssr_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +	unsigned int reg = id / 32;
> +	unsigned int bit = id % 32;

Here you haven't.

> +	unsigned long flags;
> +	u32 value;
> +
> +	dev_dbg(priv->dev, "assert %u%02u\n", reg, bit);
> +
> +	spin_lock_irqsave(&priv->rmw_lock, flags);
> +	value = readl(priv->base + SRCR(reg));
> +	writel(value | BIT(bit), priv->base + SRCR(reg));

Here you don't.

> +	spin_unlock_irqrestore(&priv->rmw_lock, flags);
> +	return 0;
> +}
> +
> +static int cpg_mssr_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +	unsigned int reg = id / 32;
> +	unsigned int bit = id % 32;
> +
> +	dev_dbg(priv->dev, "deassert %u%02u\n", reg, bit);
> +
> +	writel(BIT(bit), priv->base + SRSTCLR(reg));

And here ...

> +	return 0;
> +}
> +
> +static int cpg_mssr_status(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +	unsigned int reg = id / 32;
> +	unsigned int bit = id % 32;
> +
> +	return !!(readl(priv->base + SRCR(reg)) & BIT(bit));

And here neither.

I'd choose one variant over the other for consistency.

regards
Philipp

^ permalink raw reply

* RE: [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings
From: Chris Brandt @ 2017-01-20 16:05 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Simon Horman, Wolfram Sang,
	Geert Uytterhoeven,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <CAPDyKFq9psGMnKUx+SM_QGt6OKoMHgNFop6ANagtLOLwsMTT8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hello Ulf,

Friday, January 20, 2017, Ulf Hansson wrote:
> > +- clocks: Most controllers only have 1 clock source per channel.
> However, some
> > +         have a second clock dedicated to card detection. If 2 clocks
> are
> > +         specified, you must name them as "core" and "cd". If the
> controller
> > +         only has 1 clock, naming is not required.
> 
> Could you please elaborate a bit on the card detection clock?
> 
> I guess that there is some kind of internal card detection logic (native
> card detect) in the SDHI IP, which requires a separate clock for it to
> work? Perhaps you can state that somehow?


The reality is that the chip guys cut up the standard SDHI IP to add a
'cool new feature', but all I want to do is put it back the way it was.

NOTE: The design guys like to reuse IP blocks from previous designs that they
know worked and didn't have bugs. So, there is a good chance you will see this
change in future RZ/A devices (or even other future Renesas SoC devices).
To remove an unwanted feature adds additional design risk of breaking
something else....and given the cost of redoing silicon mask layers...no
engineer wants that mistake on their hands.



> > +Example showing 2 clocks:
> > +       sdhi0: sd@e804e000 {
> > +               compatible = "renesas,sdhi-r7s72100";
> > +               reg = <0xe804e000 0x100>;
> > +               interrupts = <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH
> > +                             GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH
> > +                             GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +               clocks = <&mstp12_clks R7S72100_CLK_SDHI00>,
> > +                        <&mstp12_clks R7S72100_CLK_SDHI01>;
> > +               clock-names = "core", "cd";
> > +               cap-sd-highspeed;
> > +               cap-sdio-irq;
> > +               status = "disabled";
> 
> The last line seems a bit odd to include in an example.

You're right. I'll take that out.

Thanks,
Chris

^ permalink raw reply

* Re: [PATCH v3 2/9] ARM64: dts: meson-gxbb-p200: add the ethernet PHY's reset GPIO
From: Andrew Lunn @ 2017-01-20 16:18 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	narmstrong-rdvid1DuHRBWk0Htik3J/w, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20170120152232.13943-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Fri, Jan 20, 2017 at 04:22:25PM +0100, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state.
> While here also specify the phy-handle of the ethmac node to make the
> PHY configuration similar to the one we have on GXL devices. This will
> allow us to specify OF-properties for the PHY itself.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> index 03e3d76626dd..fa0f84cfeaa9 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> @@ -51,6 +51,30 @@
>  	model = "Amlogic Meson GXBB P200 Development Board";
>  };
>  
> +&ethmac {
> +	status = "okay";
> +	pinctrl-0 = <&eth_rgmii_pins>;
> +	pinctrl-names = "default";
> +	phy-handle = <&eth_phy0>;
> +	phy-mode = "rgmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		eth_phy0: ethernet-phy@0 {

Hi Martin

ethernet-phy@3, because of the reg property.


> +			compatible = "ethernet-phy-id0022.1620",
> +				     "ethernet-phy-ieee802.3-c22";
> +			reg = <3>;

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

^ permalink raw reply

* Re: [PATCH v3 2/9] ARM64: dts: meson-gxbb-p200: add the ethernet PHY's reset GPIO
From: Andrew Lunn @ 2017-01-20 16:21 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	narmstrong-rdvid1DuHRBWk0Htik3J/w, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20170120152232.13943-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Fri, Jan 20, 2017 at 04:22:25PM +0100, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state.
> While here also specify the phy-handle of the ethmac node to make the
> PHY configuration similar to the one we have on GXL devices. This will
> allow us to specify OF-properties for the PHY itself.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> index 03e3d76626dd..fa0f84cfeaa9 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> @@ -51,6 +51,30 @@
>  	model = "Amlogic Meson GXBB P200 Development Board";
>  };
>  
> +&ethmac {
> +	status = "okay";
> +	pinctrl-0 = <&eth_rgmii_pins>;
> +	pinctrl-names = "default";
> +	phy-handle = <&eth_phy0>;
> +	phy-mode = "rgmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		eth_phy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id0022.1620",

Hi Martin

You don't need to specify the PHY ID. You really only need this when
the PHY is reporting no ID, or a wrong ID. In fact, if a new revision
of the board is made, with a different PHY, you might have a problem,
the wrong PHY driver is loaded.

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

^ permalink raw reply

* Re: [PATCH v3 1/9] ARM64: dts: meson-gxbb-odroidc2: add the ethernet PHY's reset GPIO
From: Neil Armstrong @ 2017-01-20 16:23 UTC (permalink / raw)
  To: Martin Blumenstingl,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20170120152232.13943-2-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On 01/20/2017 04:22 PM, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state. While here also explicitly specify the phy-mode instead of
> relying on the default-value from meson-gx.dtsi.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index c59403adb387..6b5579522587 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -152,6 +152,11 @@
>  	pinctrl-0 = <&eth_rgmii_pins>;
>  	pinctrl-names = "default";
>  	phy-handle = <&eth_phy0>;
> +	phy-mode = "rgmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
>  
>  	mdio {
>  		compatible = "snps,dwmac-mdio";
> 

On Odroid-c2 rev 0.2 :
Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 2/9] ARM64: dts: meson-gxbb-p200: add the ethernet PHY's reset GPIO
From: Neil Armstrong @ 2017-01-20 16:23 UTC (permalink / raw)
  To: Martin Blumenstingl,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20170120152232.13943-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On 01/20/2017 04:22 PM, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state.
> While here also specify the phy-handle of the ethmac node to make the
> PHY configuration similar to the one we have on GXL devices. This will
> allow us to specify OF-properties for the PHY itself.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> index 03e3d76626dd..fa0f84cfeaa9 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> @@ -51,6 +51,30 @@
>  	model = "Amlogic Meson GXBB P200 Development Board";
>  };
>  
> +&ethmac {
> +	status = "okay";
> +	pinctrl-0 = <&eth_rgmii_pins>;
> +	pinctrl-names = "default";
> +	phy-handle = <&eth_phy0>;
> +	phy-mode = "rgmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		eth_phy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id0022.1620",
> +				     "ethernet-phy-ieee802.3-c22";
> +			reg = <3>;
> +		};
> +	};
> +};
> +
>  &i2c_B {
>  	status = "okay";
>  	pinctrl-0 = <&i2c_b_pins>;
> 

On Amlogic p200 :
Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 4/9] ARM64: dts: meson-gxbb-nexbox-a95x: add the ethernet PHY's reset GPIO
From: Neil Armstrong @ 2017-01-20 16:23 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, khilman, carlo
  Cc: mark.rutland, devicetree, catalin.marinas, will.deacon, robh+dt,
	linux-arm-kernel, jbrunet
In-Reply-To: <20170120152232.13943-5-martin.blumenstingl@googlemail.com>

On 01/20/2017 04:22 PM, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state.
> While here also specify the phy-handle of the ethmac node to make the
> PHY configuration similar to the one we have on GXL devices. This will
> allow us to specify OF-properties for the PHY itself.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> index 4cbd626a9e88..c7e3ea008566 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> @@ -164,7 +164,24 @@
>  	status = "okay";
>  	pinctrl-0 = <&eth_rmii_pins>;
>  	pinctrl-names = "default";
> +	phy-handle = <&eth_phy0>;
>  	phy-mode = "rmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		eth_phy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id0243.0c54",
> +				     "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +		};
> +	};
>  };
>  
>  &ir {
> 

On Nexbox A95X (S905) :
Tested-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply

* Re: [PATCH v3 6/9] ARM64: dts: meson-gxbb-wetek-play2: add the ethernet PHY's reset GPIO
From: Neil Armstrong @ 2017-01-20 16:23 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, khilman, carlo
  Cc: mark.rutland, devicetree, catalin.marinas, will.deacon, robh+dt,
	linux-arm-kernel, jbrunet
In-Reply-To: <20170120152232.13943-7-martin.blumenstingl@googlemail.com>

On 01/20/2017 04:22 PM, Martin Blumenstingl wrote:
> This resets the ethernet PHY during boot to get the PHY into a "clean"
> state.
> While here also specify the phy-handle of the ethmac node to make the
> PHY configuration similar to the one we have on GXL devices. This will
> allow us to specify OF-properties for the PHY itself.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../boot/dts/amlogic/meson-gxbb-wetek-play2.dts    | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> index ea79fdd2c248..9a8e2f492b97 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> @@ -87,6 +87,30 @@
>  	};
>  };
>  
> +&ethmac {
> +	status = "okay";
> +	pinctrl-0 = <&eth_rgmii_pins>;
> +	pinctrl-names = "default";
> +	phy-handle = <&eth_phy0>;
> +	phy-mode = "rgmii";
> +
> +	snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +	snps,reset-delays-us = <0 10000 1000000>;
> +	snps,reset-active-low;
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		eth_phy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id001c.c916",
> +				     "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +		};
> +	};
> +};
> +
>  &i2c_A {
>  	status = "okay";
>  	pinctrl-0 = <&i2c_a_pins>;
> 

On Wetek Play 2 :
Tested-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply

* Re: [PATCH 2/2] ARM: dts: exynos: Use correct mfc_pd async-bridge clock for Exynos5420
From: Krzysztof Kozlowski @ 2017-01-20 16:28 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Inki Dae, Andi Shyti, Shuah Khan, Marek Szyprowski,
	Andrzej Hajda, devicetree, Kukjin Kim, Russell King,
	linux-samsung-soc, Rob Herring, Mark Rutland, linux-arm-kernel
In-Reply-To: <1484864995-10679-2-git-send-email-javier@osg.samsung.com>

On Thu, Jan 19, 2017 at 07:29:55PM -0300, Javier Martinez Canillas wrote:
> Commit 94aed538e032 ("ARM: dts: exynos: Add async-bridge clock to MFC
> power domain for Exynos5420") fixed an imprecise external abort error
> when the MFC registers were tried to be accessed and the needed clock
> for the asynchronous bridges were gated.
> 
> But according to the Exynos5420 manual the "Gating AXI clock for MFC"
> is not CLK_ACLK333 but CLK_MFC.
> 
> The end effect is the same because CLK_ACLK333 is a parent of CLK_MFC
> but the correct clock should be used instead.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  arch/arm/boot/dts/exynos5420.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Is this still needed?

Best regards,
Krzysztof

> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 906a1a42a7ea..ffb148ea91d6 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -294,7 +294,7 @@
>  			reg = <0x10044060 0x20>;
>  			clocks = <&clock CLK_FIN_PLL>,
>  				 <&clock CLK_MOUT_USER_ACLK333>,
> -				 <&clock CLK_ACLK333>;
> +				 <&clock CLK_MFC>;
>  			clock-names = "oscclk", "clk0","asb0";
>  			#power-domain-cells = <0>;
>  		};
> -- 
> 2.7.4
> 

^ permalink raw reply

* Re: [PATCH v3 00/24] i.MX Media Driver
From: Philipp Zabel @ 2017-01-20 16:31 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: mark.rutland, andrew-ct.chen, minghsiu.tsai, nick, songjun.wu,
	Steve Longerbeam, robert.jarzmik, devel, markus.heiser,
	laurent.pinchart+renesas, linux, geert, Steve Longerbeam,
	linux-media, devicetree, arnd, mchehab, bparrot, robh+dt,
	horms+renesas, tiffany.lin, linux-arm-kernel,
	niklas.soderlund+renesas, gregkh, linux-kernel,
	jean-christophe.trotin, kernel, fabio.estevam, shawnguo,
	sudipm.mukherjee
In-Reply-To: <c6e98327-7e2c-f34a-2d23-af7b236de441@xs4all.nl>

Hi Hans,

On Fri, 2017-01-20 at 14:52 +0100, Hans Verkuil wrote:
> Hi Steve, Philipp,
> 
> On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
> > In version 3:
> > 
> > Changes suggested by Rob Herring <robh@kernel.org>:
> > 
> >   - prepended FIM node properties with vendor prefix "fsl,".
> > 
> >   - make mipi csi-2 receiver compatible string SoC specific:
> >     "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
> > 
> >   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
> > 
> >   - removed board-specific info from the media driver binding doc. These
> >     were all related to sensor bindings, which already are (adv7180)
> >     or will be (ov564x) covered in separate binding docs. All reference
> >     board info not related to DT bindings has been moved to
> >     Documentation/media/v4l-drivers/imx.rst.
> > 
> >   - removed "_mipi" from the OV5640 compatible string.
> > 
> > Changes suggested by Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>:
> > 
> >   Mostly cosmetic/non-functional changes which I won't list here, except
> >   for the following:
> > 
> >   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
> > 
> >   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
> > 
> >   - check/handle return code from required reg property of CSI port nodes.
> > 
> >   - check/handle return code from clk_prepare_enable().
> > 
> > Changes suggested by Fabio Estevam <festevam@gmail.com>:
> > 
> >   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
> > 
> >   - finally got around to passing valid IOMUX pin config values to the
> >     pin groups.
> > 
> > Other changes:
> > 
> >   - removed the FIM properties that overrided the v4l2 FIM control defaults
> >     values. This was left-over from a requirement of a customer and is not
> >     necessary here.
> > 
> >   - The FIM must be explicitly enabled in the fim child node under the CSI
> >     port nodes, using the status property. If not enabled, FIM v4l2 controls
> >     will not appear in the video capture driver.
> > 
> >   - brought in additional media types patch from Philipp Zabel. Use new
> >     MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
> > 
> >   - brought in latest platform generic video multiplexer subdevice driver
> >     from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
> > 
> >   - removed imx-media-of.h, moved those prototypes into imx-media.h.
> 
> Based on the discussion on the mailinglist it seems everyone agrees that this
> is the preferred driver, correct?

No. I have some major reservations against the custom mem2mem framework
embedded in Steve's driver.
I think it is a misuse of the media entity links (which should describe
hardware connections) for something that should be done at the vb2 level
(letting one device's capture EOF interrupt trigger the next device's
m2m device_run without going through userspace).
Steve and I disagree on that point, so we'd appreciate if we could get
some more eyes on the above issue.

regards
Philipp

^ permalink raw reply

* Re: [PATCH v3 8/9] ARM64: dts: amlogic: add the ethernet TX delay configuration
From: Neil Armstrong @ 2017-01-20 16:34 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, khilman, carlo
  Cc: mark.rutland, devicetree, catalin.marinas, will.deacon, robh+dt,
	linux-arm-kernel, jbrunet
In-Reply-To: <20170120152232.13943-9-martin.blumenstingl@googlemail.com>

On 01/20/2017 04:22 PM, Martin Blumenstingl wrote:
> This adds the amlogic,tx-delay-ns property with the old (hardcoded)
> default value of 2ns to all boards which are using an RGMII ethernet
> PHY.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts    | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts        | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi   | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts   | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts   | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts    | 2 ++
>  arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts         | 2 ++
>  8 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index 6b5579522587..e63ed98d6c25 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -154,6 +154,8 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rgmii";
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> index fa0f84cfeaa9..6c7e914e9bd9 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
> @@ -58,6 +58,8 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rgmii";
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
> index 358ab5159bd5..4301ad860a76 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
> @@ -131,6 +131,8 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rgmii";
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
> index 47fe4e444f33..f6d8c70a26a2 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
> @@ -72,6 +72,8 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rgmii";
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> index 9a8e2f492b97..1a222ce60a30 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
> @@ -94,6 +94,8 @@
>  	phy-handle = <&eth_phy0>;
>  	phy-mode = "rgmii";
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
> index f66939cacd37..7663d6775823 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
> @@ -59,6 +59,8 @@
>  	/* Select external PHY by default */
>  	phy-handle = <&external_phy>;
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	/* External PHY reset is shared with internal PHY Led signals */
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> index 5a337d339df1..a0bc746adb42 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> @@ -162,6 +162,8 @@
>  	/* Select external PHY by default */
>  	phy-handle = <&external_phy>;
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
>  	snps,reset-active-low;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts
> index 5dbc66088355..e6ac39b712b7 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts
> @@ -59,6 +59,8 @@
>  	/* Select external PHY by default */
>  	phy-handle = <&external_phy>;
>  
> +	amlogic,tx-delay-ns = <2>;
> +
>  	/* External PHY reset is shared with internal PHY Led signals */
>  	snps,reset-gpio = <&gpio GPIOZ_14 0>;
>  	snps,reset-delays-us = <0 10000 1000000>;
> 

On :
- Odroid-C2 rev 0.2
- P200
- Wetek Play 2
- Amlogic P230
- Nexbox A1
Tested-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply

* Re: [PATCH 3/3] dts: hisi: add dts files for Hi3516CV300 demo board
From: Wei Xu @ 2017-01-20 16:40 UTC (permalink / raw)
  To: Pan Wen, mturquette-rdvid1DuHRBWk0Htik3J/w,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw
  Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	howell.yang-C8/M+/jPZTeaMJb+Lgu22Q,
	xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
	jalen.hsu-C8/M+/jPZTeaMJb+Lgu22Q,
	lvkuanliang-C8/M+/jPZTeaMJb+Lgu22Q,
	suwenping-C8/M+/jPZTeaMJb+Lgu22Q, raojun-C8/M+/jPZTeaMJb+Lgu22Q,
	kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161017120705.3726-4-wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

Hi Pan Wen,

On 2016/10/17 13:07, Pan Wen wrote:
> Add dts files for Hi3516CV300 demo board.
> 
> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile             |   1 +
>  arch/arm/boot/dts/hi3516cv300-demb.dts | 148 ++++++++++++
>  arch/arm/boot/dts/hi3516cv300.dtsi     | 397 +++++++++++++++++++++++++++++++++
>  3 files changed, 546 insertions(+)
>  create mode 100644 arch/arm/boot/dts/hi3516cv300-demb.dts
>  create mode 100644 arch/arm/boot/dts/hi3516cv300.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..1f25530 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_HIP01) += \
>  dtb-$(CONFIG_ARCH_HIP04) += \
>  	hip04-d01.dtb
>  dtb-$(CONFIG_ARCH_HISI) += \
> +	hi3516cv300-demb.dtb \
>  	hi3519-demb.dtb
>  dtb-$(CONFIG_ARCH_HIX5HD2) += \
>  	hisi-x5hd2-dkb.dtb
> diff --git a/arch/arm/boot/dts/hi3516cv300-demb.dts b/arch/arm/boot/dts/hi3516cv300-demb.dts
> new file mode 100644
> index 0000000..6a75cd6
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300-demb.dts
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +
> +/dts-v1/;
> +#include "hi3516cv300.dtsi"
> +
> +/ {
> +	model = "Hisilicon Hi3516CV300 DEMO Board";
> +	compatible = "hisilicon,hi3516cv300";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		i2c0 = &i2c_bus0;
> +		i2c1 = &i2c_bus1;
> +		spi0 = &spi_bus0;
> +		spi1 = &spi_bus1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x80000000 0x10000000>;
> +	};
> +};
> +
> +&dual_timer0 {
> +	status = "okay";
> +};
> +
> +&watchdog {
> +	status = "okay";
> +};
> +
> +&pwm {
> +	status = "okay";
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&i2c_bus0 {
> +	status = "okay";
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0_pmux>;
> +};
> +
> +&i2c_bus1 {
> +	status = "okay";
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pmux>;
> +};
> +
> +&spi_bus0{
> +	status = "disabled";
> +	num-cs = <1>;
> +	cs-gpios = <&gpio_chip0 6 0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_pmux>;
> +};
> +
> +&spi_bus1{
> +	status = "okay";
> +	num-cs = <2>;
> +	cs-gpios = <&gpio_chip5 3 0>, <&gpio_chip5 4 0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi1_pmux>;
> +};
> +
> +&fmc {
> +	spi-nor@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <160000000>;
> +		m25p,fast-read;
> +	};
> +};
> +
> +&mdio {
> +	phy0: phy@1 {
> +		reg = <1>;
> +	};
> +};
> +
> +&hisi_femac {
> +	mac-address = [00 00 00 00 00 00];
> +	phy-mode = "rmii";
> +	phy-handle = <&phy0>;
> +	hisilicon,phy-reset-delays-us = <10000 10000 150000>;
> +};
> +
> +&dmac {
> +	status = "okay";
> +};
> +
> +&pmux {
> +	i2c0_pmux: i2c0_pmux {
> +		pinctrl-single,pins = <
> +			0x2c 0x3
> +			0x30 0x3>;
> +	};
> +
> +	i2c1_pmux: i2c1_pmux {
> +		pinctrl-single,pins = <
> +			0x20 0x1
> +			0x24 0x1>;
> +	};
> +
> +	spi0_pmux: spi0_pmux {
> +		pinctrl-single,pins = <
> +			0x28 0x1
> +			0x2c 0x1
> +			0x30 0x1
> +			0x34 0x1>;
> +	};
> +
> +	spi1_pmux: spi1_pmux {
> +		pinctrl-single,pins = <
> +			0xc4 0x1
> +			0xc8 0x1
> +			0xcc 0x1
> +			0xd0 0x1
> +			0xd4 0x1>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/hi3516cv300.dtsi b/arch/arm/boot/dts/hi3516cv300.dtsi
> new file mode 100644
> index 0000000..1da41ab
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300.dtsi
> @@ -0,0 +1,397 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "skeleton.dtsi"
> +#include <dt-bindings/clock/hi3516cv300-clock.h>
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,arm926ej-s";
> +			reg = <0>;
> +		};
> +	};
> +
> +	vic: interrupt-controller@10040000 {
> +		compatible = "arm,pl190-vic";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		reg = <0x10040000 0x1000>;
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&vic>;
> +		ranges;
> +
> +		clk_3m: clk_3m {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <3000000>;
> +		};
> +
> +		clk_apb: clk_apb {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <50000000>;
> +		};
> +
> +		crg: clock-reset-controller@12010000 {
> +			compatible = "hisilicon,hi3516cv300-crg";
> +			reg = <0x12010000 0x1000>;
> +			#clock-cells = <1>;
> +			#reset-cells = <2>;
> +		};
> +
> +		sysctrl: system-controller@12020000 {
> +			compatible = "hisilicon,hi3516cv300-sysctrl", "syscon";
> +			reg = <0x12020000 0x1000>;
> +			#clock-cells = <1>;
> +		};
> +
> +		reboot {
> +			compatible = "syscon-reboot";
> +			regmap = <&sysctrl>;
> +			offset = <0x4>;
> +			mask = <0xdeadbeef>;
> +		};
> +
> +		dual_timer0: dual_timer@12000000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			reg = <0x12000000 0x1000>;
> +			interrupts = <3>;
> +			clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> +			clock-names = "timer0", "timer1", "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		dual_timer1: dual_timer@12001000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			reg = <0x12001000 0x1000>;
> +			interrupts = <4>;
> +			clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> +			clock-names = "timer0", "timer1", "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		watchdog: watchdog@12080000 {
> +			compatible = "arm,sp805-wdt", "arm,primecell";
> +			arm,primecell-periphid = <0x00141805>;
> +			reg = <0x12080000 0x1000>;
> +			clocks = <&sysctrl HI3516CV300_WDT_CLK>,
> +				<&crg HI3516CV300_APB_CLK>;
> +			clock-names = "wdog_clk", "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		pwm: pwm@12130000 {
> +			compatible = "hisilicon,hi3516cv300-pwm",
> +				"hisilicon,hibvt-pwm";
> +			reg = <0x12130000 0x10000>;
> +			clocks = <&crg HI3516CV300_PWM_CLK>;
> +			resets = <&crg 0x38 0>;
> +			#pwm-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		uart0: uart@12100000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x12100000 0x1000>;
> +			interrupts = <5>;
> +			clocks = <&crg HI3516CV300_UART0_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		uart1: uart@12101000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x12101000 0x1000>;
> +			interrupts = <30>;
> +			clocks = <&crg HI3516CV300_UART1_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		uart2: uart@12102000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x12102000 0x1000>;
> +			interrupts = <25>;
> +			clocks = <&crg HI3516CV300_UART2_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		i2c_bus0: i2c@12110000 {
> +			compatible = "hisilicon,hi3516cv300-i2c",
> +				"hisilicon,hibvt-i2c";

Since this driver is not in the mainline yet,
is it OK to delete this node firstly?

> +			reg = <0x12110000 0x1000>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			status = "disabled";
> +		};
> +
> +		i2c_bus1: i2c@12112000 {
> +			compatible = "hisilicon,hi3516cv300-i2c",
> +				"hisilicon,hibvt-i2c";

ditto.
Other is fine to me.

Best Regards,
Wei

> +			reg = <0x12112000 0x1000>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			status = "disabled";
> +		};
> +
> +		spi_bus0: spi@12120000 {
> +			compatible = "arm,pl022", "arm,primecell";
> +			reg = <0x12120000 0x1000>;
> +			interrupts = <6>;
> +			clocks = <&crg HI3516CV300_SPI0_CLK>;
> +			clock-names = "apb_pclk";
> +			dmas = <&dmac 12 1>, <&dmac 13 2>;
> +			dma-names = "rx", "tx";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		spi_bus1: spi@12121000 {
> +			compatible = "arm,pl022", "arm,primecell";
> +			reg = <0x12121000 0x1000>, <0x12030000 0x4>;
> +			interrupts = <7>;
> +			clocks = <&crg HI3516CV300_SPI1_CLK>;
> +			clock-names = "apb_pclk";
> +			dmas = <&dmac 14 1>, <&dmac 15 2>;
> +			dma-names = "rx", "tx";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		fmc: spi-nor-controller@10000000 {
> +			compatible = "hisilicon,fmc-spi-nor";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x10000000 0x1000>, <0x14000000 0x1000000>;
> +			reg-names = "control", "memory";
> +			clocks = <&crg HI3516CV300_FMC_CLK>;
> +			assigned-clocks = <&crg HI3516CV300_FMC_CLK>;
> +			assigned-clock-rates = <24000000>;
> +		};
> +
> +		mdio: mdio@10051100 {
> +			compatible = "hisilicon,hisi-femac-mdio";
> +			reg = <0x10051100 0x10>;
> +			clocks = <&crg HI3516CV300_ETH_CLK>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		hisi_femac: ethernet@10090000 {
> +			compatible = "hisilicon,hi3516cv300-femac",
> +				"hisilicon,hisi-femac-v2";
> +			reg = <0x10050000 0x1000>,<0x10051300 0x200>;
> +			interrupts = <12>;
> +			clocks = <&crg HI3516CV300_ETH_CLK>;
> +			resets = <&crg 0xec 0>, <&crg 0xec 3>;
> +			reset-names = "mac","phy";
> +		};
> +
> +		dmac: dma-controller@10030000 {
> +			compatible = "arm,pl080", "arm,primecell";
> +			reg = <0x10030000 0x1000>;
> +			interrupts = <14>;
> +			clocks = <&crg HI3516CV300_DMAC_CLK>;
> +			clock-names = "apb_pclk";
> +			lli-bus-interface-ahb1;
> +			lli-bus-interface-ahb2;
> +			mem-bus-interface-ahb1;
> +			mem-bus-interface-ahb2;
> +			memcpy-burst-size = <256>;
> +			memcpy-bus-width = <32>;
> +			#dma-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip0: gpio@12140000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12140000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 61 2>,
> +				<&pmux 4 11 1>,
> +				<&pmux 5 10 1>,
> +				<&pmux 6 13 2>;
> +
> +			status = "disabled";
> +		};
> +
> +		gpio_chip1: gpio@12141000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12141000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 16 7>,
> +				<&pmux 7 0 1>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip2: gpio@12142000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12142000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 46 1>,
> +				<&pmux 1 45 1>,
> +				<&pmux 2 44 1>,
> +				<&pmux 3 43 1>,
> +				<&pmux 4 39 1>,
> +				<&pmux 5 38 1>,
> +				<&pmux 6 40 1>,
> +				<&pmux 7 48 1>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip3: gpio@12143000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12143000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 37 1>,
> +				<&pmux 1 36 1>,
> +				<&pmux 2 35 1>,
> +				<&pmux 3 34 1>,
> +				<&pmux 4 23 2>,
> +				<&pmux 6 8 2>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip4: gpio@12144000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12144000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 27 1>,
> +				<&pmux 1 26 1>,
> +				<&pmux 2 31 1>,
> +				<&pmux 3 30 1>,
> +				<&pmux 4 28 2>,
> +				<&pmux 6 33 1>,
> +				<&pmux 7 32 1>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip5: gpio@12145000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12145000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 53 1>,
> +				<&pmux 1 51 2>,
> +				<&pmux 3 50 1>,
> +				<&pmux 4 49 1>,
> +				<&pmux 5 47 1>,
> +				<&pmux 6 40 2>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip6: gpio@12146000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12146000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 7 1>,
> +				<&pmux 1 6 1>,
> +				<&pmux 2 4 1>,
> +				<&pmux 3 5 1>,
> +				<&pmux 4 15 1>,
> +				<&pmux 5 1 3>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip7: gpio@12147000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12147000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 1 55 6>,
> +				<&pmux 7 25 1>;
> +			status = "disabled";
> +		};
> +
> +		gpio_chip8: gpio@12148000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12148000 0x1000>;
> +			interrupts = <31>;
> +			clocks = <&crg HI3516CV300_APB_CLK>;
> +			clock-names = "apb_pclk";
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pmux 0 63 3>,
> +				<&pmux 3 12 1>;
> +			status = "disabled";
> +		};
> +
> +		pmux: pinmux@12040000 {
> +			compatible = "pinctrl-single";
> +			reg = <0x12040000 0x108>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			#gpio-range-cells = <3>;
> +			ranges;
> +
> +			pinctrl-single,register-width = <32>;
> +			pinctrl-single,function-mask = <7>;
> +			/* pin base, nr pins & gpio function */
> +			pinctrl-single,gpio-range = <&range 0 54 0
> +				&range 55 6 1 &range 61 5 0>;
> +
> +			range: gpio-range {
> +				#pinctrl-single,gpio-range-cells = <3>;
> +			};
> +		};
> +
> +		pconf: pinconf@12040800 {
> +			compatible = "pinconf-single";
> +			reg = <0x12040800 0x130>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			pinctrl-single,register-width = <32>;
> +		};
> +	};
> +};
> 

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

^ permalink raw reply

* Re: [PATCH v13 2/5] tee: generic TEE subsystem
From: Arnd Bergmann @ 2017-01-20 16:47 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: valentin.manea, devicetree, javier, emmanuel.michel,
	Greg Kroah-Hartman, Mark Rutland, Will Deacon, linux-kernel,
	Wei Xu, Nishanth Menon, Jason Gunthorpe, Rob Herring, broonie,
	Al Viro, Andrew F. Davis, Olof Johansson, Andrew Morton,
	jean-michel.delorme, Michal Simek, linux-arm-kernel
In-Reply-To: <20170119164541.GA22094@jax>

On Thursday, January 19, 2017 5:45:43 PM CET Jens Wiklander wrote:
> 
> On Wed, Jan 18, 2017 at 09:19:25PM +0100, Arnd Bergmann wrote:
> > On Friday, November 18, 2016 3:51:37 PM CET Jens Wiklander wrote:
> > > Initial patch for generic TEE subsystem.
> > > This subsystem provides:
> > > * Registration/un-registration of TEE drivers.
> > > * Shared memory between normal world and secure world.
> > > * Ioctl interface for interaction with user space.
> > > * Sysfs implementation_id of TEE driver
> > > 
> > > A TEE (Trusted Execution Environment) driver is a driver that interfaces
> > > with a trusted OS running in some secure environment, for example,
> > > TrustZone on ARM cpus, or a separate secure co-processor etc.
> > > 
> > > The TEE subsystem can serve a TEE driver for a Global Platform compliant
> > > TEE, but it's not limited to only Global Platform TEEs.
> > > 
> > > This patch builds on other similar implementations trying to solve
> > > the same problem:
> > > * "optee_linuxdriver" by among others
> > >   Jean-michel DELORME<jean-michel.delorme@st.com> and
> > >   Emmanuel MICHEL <emmanuel.michel@st.com>
> > > * "Generic TrustZone Driver" by Javier González <javier@javigon.com>
> > 
> > Can you give an example for a system that would contain more than one
> > TEE? I see that you support dynamic registration, and it's clear that
> > there can be more than one type of TEE, but why would one have more
> > than one at a time, and why not more than 32?
> 
> I know that ST has systems where there's one TEE in TrustZone and
> another TEE on a separate secure co-processor. If you have several TEEs
> it's probably because they have different capabilities (performance
> versus level of security). Just going beyond two or three different
> levels of security with different TEEs sounds a bit extreme, so a
> maximum of 32 or 16 should be fairly safe. If it turns out I'm wrong in
> this assumption it's not that hard to correct it.

Ok

> > 
> > > +	if (copy_from_user(&arg, uarg, sizeof(arg)))
> > > +		return -EFAULT;
> > > +
> > > +	if (sizeof(arg) + TEE_IOCTL_PARAM_SIZE(arg.num_params) != buf.buf_len)
> > > +		return -EINVAL;
> > > +
> > > +	if (arg.num_params) {
> > > +		params = kcalloc(arg.num_params, sizeof(struct tee_param),
> > > +				 GFP_KERNEL);
> > > +		if (!params)
> > > +			return -ENOMEM;
> > 
> > It would be good to have an upper bound on the number of parameters
> > to limit the size of the memory allocation here.
> 
> This is already limited due to:
> 
> The test with: buf.buf_len > TEE_MAX_ARG_SIZE
> 
> And then another test that the number of parameters matches the buffer size
> with: sizeof(arg) + TEE_IOCTL_PARAM_SIZE(arg.num_params) != buf.buf_len

Ok, makes sense.

> > 
> > > +/**
> > > + * struct tee_ioctl_param - parameter
> > > + * @attr: attributes
> > > + * @memref: a memory reference
> > > + * @value: a value
> > > + *
> > > + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> > > + * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> > > + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> > > + * indicates that none of the members are used.
> > > + */
> > > +struct tee_ioctl_param {
> > > +	__u64 attr;
> > > +	union {
> > > +		struct tee_ioctl_param_memref memref;
> > > +		struct tee_ioctl_param_value value;
> > > +	} u;
> > > +};
> > > +
> > > +#define TEE_IOCTL_UUID_LEN		16
> > > +
> > 
> > Having a union in an ioctl argument seems odd. Have you considered
> > using two different ioctl command numbers depending on the type?
> 
> struct tee_ioctl_param is used as an array and some parameters can be
> memrefs while other are values.

Got it. I still think it's a bit awkward on the user ABI side.
I also see that (unlike the in-kernel interface) tee_ioctl_param_memref
and tee_ioctl_param_value are both defined in terms of three __u64
members.

How about simply using one format here and making this

struct tee_ioctl_param {
   __u64 attr;
   __u64 a;
   __u64 b;
   __u64 c;
};

Given that you need a wrapper to set the pointer in memref anyway?

Having an ioctl with a variable number of variable type arguments
is really a weakness of the ABI, but I don't see a good way around
it either, the above would just make it slightly more direct.

> > > +/**
> > > + * struct tee_iocl_supp_send_arg - Send a response to a received request
> > > + * @ret:	[out] return value
> > > + * @num_params	[in] number of parameters following this struct
> > > + */
> > > +struct tee_iocl_supp_send_arg {
> > > +	__u32 ret;
> > > +	__u32 num_params;
> > > +	/*
> > > +	 * this struct is 8 byte aligned since the 'struct tee_ioctl_param'
> > > +	 * which follows requires 8 byte alignment.
> > > +	 *
> > > +	 * Commented out element used to visualize the layout dynamic part
> > > +	 * of the struct. This field is not available at all if
> > > +	 * num_params == 0.
> > > +	 *
> > > +	 * struct tee_ioctl_param params[num_params];
> > > +	 */
> > > +} __aligned(8);
> > 
> > I'd make that 
> > 
> > 	struct tee_ioctl_param params[0];
> > 
> > as wel here, as I also commented in patch 3 that has a similar structure.
> 
> I'm concerned that this may cause warnings when compiling for user space
> depending on compiler and options. Am I too cautious here?

See https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

I actually misremembered it and the syntax I listed is GCC specific,
but C99 allows "flexible arrays". I think there is no problem relying
on C99 here, we already rely on C99 features elsewhere in headers.

	Arnd

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox