[PATCH v3 0/5] Add support for PWMSS on DRA7

[PATCH v3 0/5] Add support for PWMSS on DRA7

[Franklin S Cooper Jr]

This patch series adds support for PWMSS on DRA7. The IP is same as that
present in AM33XX and AM43XX.

This is an updated version of the patch series originally sent by Vignesh.
The biggest difference is the removal of local clock gating support from
various drivers due to silicon issue.

Links to v2 series:
https://patchwork.ozlabs.org/patch/479933/ (already in mainline)
https://patchwork.ozlabs.org/patch/479932/
https://patchwork.ozlabs.org/patch/479931/
https://patchwork.ozlabs.org/patch/479930/
https://patchwork.ozlabs.org/patch/479929/

Franklin S Cooper Jr (1):
  pwms: pwm-ti*: Remove support for local clock gating

Vignesh R (4):
  ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS
  ARM: dts: DRA7: Add TBCLK for PWMSS
  clk: ti: DRA7: Add tbclk data for ehrpwm
  ARM: dts: DRA7: Add dt nodes for PWMSS

 .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |   8 +
 .../devicetree/bindings/pwm/pwm-tipwmss.txt        |  17 +-
 arch/arm/boot/dts/dra7.dtsi                        |  64 ++++++
 arch/arm/boot/dts/dra7xx-clocks.dtsi               |  24 +++
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          | 239 +++++++++++++++++++++
 drivers/clk/ti/clk-7xx.c                           |   3 +
 drivers/pwm/pwm-tiecap.c                           |  28 ---
 drivers/pwm/pwm-tiehrpwm.c                         |  29 ---
 drivers/pwm/pwm-tipwmss.c                          |  49 -----
 drivers/pwm/pwm-tipwmss.h                          |  39 ----
 10 files changed, 354 insertions(+), 146 deletions(-)
 delete mode 100644 drivers/pwm/pwm-tipwmss.h

-- 
2.7.0

[PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Franklin S Cooper Jr]

The PWMSS local clock gating registers have no real purpose on OMAP ARM
devices. These registers were left over registers from DSP IP where the
PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
don't function properly. TRMs will be update to indicate that these
registers shouldn't be touched.

Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
will be removed by this patch with zero loss of functionality by the ECAP
and EPWM drivers.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 3 changes:
New patch

 drivers/pwm/pwm-tiecap.c   | 28 --------------------------
 drivers/pwm/pwm-tiehrpwm.c | 29 ---------------------------
 drivers/pwm/pwm-tipwmss.c  | 49 ----------------------------------------------
 drivers/pwm/pwm-tipwmss.h  | 39 ------------------------------------
 4 files changed, 145 deletions(-)
 delete mode 100644 drivers/pwm/pwm-tipwmss.h

diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index 616af76..9e0865d7 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -27,8 +27,6 @@
 #include <linux/pwm.h>
 #include <linux/of_device.h>
 
-#include "pwm-tipwmss.h"
-
 /* ECAP registers and bits definitions */
 #define CAP1			0x08
 #define CAP2			0x0C
@@ -206,7 +204,6 @@ static int ecap_pwm_probe(struct platform_device *pdev)
 	struct resource *r;
 	struct clk *clk;
 	struct ecap_pwm_chip *pc;
-	u16 status;
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc)
@@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
-
-	status = pwmss_submodule_state_change(pdev->dev.parent,
-			PWMSS_ECAPCLK_EN);
-	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
-		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
-		ret = -EINVAL;
-		goto pwmss_clk_failure;
-	}
-
-	pm_runtime_put_sync(&pdev->dev);
 
 	platform_set_drvdata(pdev, pc);
 	return 0;
-
-pwmss_clk_failure:
-	pm_runtime_put_sync(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
-	pwmchip_remove(&pc->chip);
-	return ret;
 }
 
 static int ecap_pwm_remove(struct platform_device *pdev)
 {
 	struct ecap_pwm_chip *pc = platform_get_drvdata(pdev);
 
-	pm_runtime_get_sync(&pdev->dev);
-	/*
-	 * Due to hardware misbehaviour, acknowledge of the stop_req
-	 * is missing. Hence checking of the status bit skipped.
-	 */
-	pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ);
-	pm_runtime_put_sync(&pdev->dev);
-
 	pm_runtime_disable(&pdev->dev);
 	return pwmchip_remove(&pc->chip);
 }
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 6a41e66..e09b1f0 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -27,8 +27,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/of_device.h>
 
-#include "pwm-tipwmss.h"
-
 /* EHRPWM registers and bits definitions */
 
 /* Time base module registers */
@@ -437,7 +435,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
 	struct resource *r;
 	struct clk *clk;
 	struct ehrpwm_pwm_chip *pc;
-	u16 status;
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc)
@@ -487,27 +484,9 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
-
-	status = pwmss_submodule_state_change(pdev->dev.parent,
-			PWMSS_EPWMCLK_EN);
-	if (!(status & PWMSS_EPWMCLK_EN_ACK)) {
-		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
-		ret = -EINVAL;
-		goto pwmss_clk_failure;
-	}
-
-	pm_runtime_put_sync(&pdev->dev);
 
 	platform_set_drvdata(pdev, pc);
 	return 0;
-
-pwmss_clk_failure:
-	pm_runtime_put_sync(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
-	pwmchip_remove(&pc->chip);
-	clk_unprepare(pc->tbclk);
-	return ret;
 }
 
 static int ehrpwm_pwm_remove(struct platform_device *pdev)
@@ -516,14 +495,6 @@ static int ehrpwm_pwm_remove(struct platform_device *pdev)
 
 	clk_unprepare(pc->tbclk);
 
-	pm_runtime_get_sync(&pdev->dev);
-	/*
-	 * Due to hardware misbehaviour, acknowledge of the stop_req
-	 * is missing. Hence checking of the status bit skipped.
-	 */
-	pwmss_submodule_state_change(pdev->dev.parent, PWMSS_EPWMCLK_STOP_REQ);
-	pm_runtime_put_sync(&pdev->dev);
-
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return pwmchip_remove(&pc->chip);
diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
index 5cf65a1..829f499 100644
--- a/drivers/pwm/pwm-tipwmss.c
+++ b/drivers/pwm/pwm-tipwmss.c
@@ -22,32 +22,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/of_device.h>
 
-#include "pwm-tipwmss.h"
-
-#define PWMSS_CLKCONFIG		0x8	/* Clock gating reg */
-#define PWMSS_CLKSTATUS		0xc	/* Clock gating status reg */
-
-struct pwmss_info {
-	void __iomem	*mmio_base;
-	struct mutex	pwmss_lock;
-	u16		pwmss_clkconfig;
-};
-
-u16 pwmss_submodule_state_change(struct device *dev, int set)
-{
-	struct pwmss_info *info = dev_get_drvdata(dev);
-	u16 val;
-
-	mutex_lock(&info->pwmss_lock);
-	val = readw(info->mmio_base + PWMSS_CLKCONFIG);
-	val |= set;
-	writew(val , info->mmio_base + PWMSS_CLKCONFIG);
-	mutex_unlock(&info->pwmss_lock);
-
-	return readw(info->mmio_base + PWMSS_CLKSTATUS);
-}
-EXPORT_SYMBOL(pwmss_submodule_state_change);
-
 static const struct of_device_id pwmss_of_match[] = {
 	{ .compatible	= "ti,am33xx-pwmss" },
 	{},
@@ -57,24 +31,10 @@ MODULE_DEVICE_TABLE(of, pwmss_of_match);
 static int pwmss_probe(struct platform_device *pdev)
 {
 	int ret;
-	struct resource *r;
-	struct pwmss_info *info;
 	struct device_node *node = pdev->dev.of_node;
 
-	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
-
-	mutex_init(&info->pwmss_lock);
-
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	info->mmio_base = devm_ioremap_resource(&pdev->dev, r);
-	if (IS_ERR(info->mmio_base))
-		return PTR_ERR(info->mmio_base);
-
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
-	platform_set_drvdata(pdev, info);
 
 	/* Populate all the child nodes here... */
 	ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
@@ -86,30 +46,21 @@ static int pwmss_probe(struct platform_device *pdev)
 
 static int pwmss_remove(struct platform_device *pdev)
 {
-	struct pwmss_info *info = platform_get_drvdata(pdev);
-
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	mutex_destroy(&info->pwmss_lock);
 	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
 static int pwmss_suspend(struct device *dev)
 {
-	struct pwmss_info *info = dev_get_drvdata(dev);
-
-	info->pwmss_clkconfig = readw(info->mmio_base + PWMSS_CLKCONFIG);
 	pm_runtime_put_sync(dev);
 	return 0;
 }
 
 static int pwmss_resume(struct device *dev)
 {
-	struct pwmss_info *info = dev_get_drvdata(dev);
-
 	pm_runtime_get_sync(dev);
-	writew(info->pwmss_clkconfig, info->mmio_base + PWMSS_CLKCONFIG);
 	return 0;
 }
 #endif
diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h
deleted file mode 100644
index 10ad804..0000000
--- a/drivers/pwm/pwm-tipwmss.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * TI PWM Subsystem driver
- *
- * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; 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.
- *
- */
-
-#ifndef __TIPWMSS_H
-#define __TIPWMSS_H
-
-/* PWM substem clock gating */
-#define PWMSS_ECAPCLK_EN	BIT(0)
-#define PWMSS_ECAPCLK_STOP_REQ	BIT(1)
-#define PWMSS_EPWMCLK_EN	BIT(8)
-#define PWMSS_EPWMCLK_STOP_REQ	BIT(9)
-
-#define PWMSS_ECAPCLK_EN_ACK	BIT(0)
-#define PWMSS_EPWMCLK_EN_ACK	BIT(8)
-
-#ifdef CONFIG_PWM_TIPWMSS
-extern u16 pwmss_submodule_state_change(struct device *dev, int set);
-#else
-static inline u16 pwmss_submodule_state_change(struct device *dev, int set)
-{
-	/* return success status value */
-	return 0xFFFF;
-}
-#endif
-#endif	/* __TIPWMSS_H */
-- 
2.7.0

[PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Franklin S Cooper Jr]

From: Vignesh R <vigneshr@ti.com>

Add hwmod entries for the PWMSS on DRA7.

Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
L4PER2_L3_GICLK/2. The TRM does not show the division by 2.

[1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
Version 3 changes:
Switch from SYSC_HAS_RESET_STATUS to SYSC_HAS_SOFTRESET which is the
correct bitfield for that register.

 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 239 ++++++++++++++++++++++++++++++
 1 file changed, 239 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 848356e..4b2d68b 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -383,6 +383,149 @@ static struct omap_hwmod dra7xx_dcan2_hwmod = {
 	},
 };
 
+/* pwmss  */
+static struct omap_hwmod_class_sysconfig dra7xx_epwmss_sysc = {
+	.rev_offs	= 0x0,
+	.sysc_offs	= 0x4,
+	.sysc_flags	= SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET,
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+struct omap_hwmod_class dra7xx_epwmss_hwmod_class = {
+	.name		= "epwmss",
+	.sysc		= &dra7xx_epwmss_sysc,
+};
+
+static struct omap_hwmod_class dra7xx_ecap_hwmod_class = {
+	.name		= "ecap",
+};
+
+static struct omap_hwmod_class dra7xx_eqep_hwmod_class = {
+	.name		= "eqep",
+};
+
+struct omap_hwmod_class dra7xx_ehrpwm_hwmod_class = {
+	.name		= "ehrpwm",
+};
+
+/* epwmss0 */
+struct omap_hwmod dra7xx_epwmss0_hwmod = {
+	.name		= "epwmss0",
+	.class		= &dra7xx_epwmss_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+	.prcm		= {
+		.omap4	= {
+			.modulemode	= MODULEMODE_SWCTRL,
+			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS1_CLKCTRL_OFFSET,
+			.context_offs	= DRA7XX_RM_L4PER2_PWMSS1_CONTEXT_OFFSET,
+		},
+	},
+};
+
+/* ecap0 */
+struct omap_hwmod dra7xx_ecap0_hwmod = {
+	.name		= "ecap0",
+	.class		= &dra7xx_ecap_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* eqep0 */
+struct omap_hwmod dra7xx_eqep0_hwmod = {
+	.name		= "eqep0",
+	.class		= &dra7xx_eqep_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* ehrpwm0 */
+struct omap_hwmod dra7xx_ehrpwm0_hwmod = {
+	.name		= "ehrpwm0",
+	.class		= &dra7xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* epwmss1 */
+struct omap_hwmod dra7xx_epwmss1_hwmod = {
+	.name		= "epwmss1",
+	.class		= &dra7xx_epwmss_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+	.prcm		= {
+		.omap4	= {
+			.modulemode	= MODULEMODE_SWCTRL,
+			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS2_CLKCTRL_OFFSET,
+			.context_offs	= DRA7XX_RM_L4PER2_PWMSS2_CONTEXT_OFFSET,
+		},
+	},
+};
+
+/* ecap1 */
+struct omap_hwmod dra7xx_ecap1_hwmod = {
+	.name		= "ecap1",
+	.class		= &dra7xx_ecap_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* eqep1 */
+struct omap_hwmod dra7xx_eqep1_hwmod = {
+	.name		= "eqep1",
+	.class		= &dra7xx_eqep_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* ehrpwm1 */
+struct omap_hwmod dra7xx_ehrpwm1_hwmod = {
+	.name		= "ehrpwm1",
+	.class		= &dra7xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* epwmss2 */
+struct omap_hwmod dra7xx_epwmss2_hwmod = {
+	.name		= "epwmss2",
+	.class		= &dra7xx_epwmss_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+	.prcm		= {
+		.omap4	= {
+			.modulemode	= MODULEMODE_SWCTRL,
+			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS3_CLKCTRL_OFFSET,
+			.context_offs	= DRA7XX_RM_L4PER2_PWMSS3_CONTEXT_OFFSET,
+		},
+	},
+};
+
+/* ecap2 */
+struct omap_hwmod dra7xx_ecap2_hwmod = {
+	.name		= "ecap2",
+	.class		= &dra7xx_ecap_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* eqep2 */
+struct omap_hwmod dra7xx_eqep2_hwmod = {
+	.name		= "eqep2",
+	.class		= &dra7xx_eqep_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
+/* ehrpwm2 */
+struct omap_hwmod dra7xx_ehrpwm2_hwmod = {
+	.name		= "ehrpwm2",
+	.class		= &dra7xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "l4_root_clk_div",
+};
+
 /*
  * 'dma' class
  *
@@ -2676,6 +2819,90 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio6 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss0 = {
+	.master		= &dra7xx_l4_per2_hwmod,
+	.slave		= &dra7xx_epwmss0_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss0__ecap0 = {
+	.master		= &dra7xx_epwmss0_hwmod,
+	.slave		= &dra7xx_ecap0_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss0__eqep0 = {
+	.master		= &dra7xx_epwmss0_hwmod,
+	.slave		= &dra7xx_eqep0_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss0__ehrpwm0 = {
+	.master		= &dra7xx_epwmss0_hwmod,
+	.slave		= &dra7xx_ehrpwm0_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss1 = {
+	.master		= &dra7xx_l4_per2_hwmod,
+	.slave		= &dra7xx_epwmss1_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss1__ecap1 = {
+	.master		= &dra7xx_epwmss1_hwmod,
+	.slave		= &dra7xx_ecap1_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss1__eqep1 = {
+	.master		= &dra7xx_epwmss1_hwmod,
+	.slave		= &dra7xx_eqep1_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss1__ehrpwm1 = {
+	.master		= &dra7xx_epwmss1_hwmod,
+	.slave		= &dra7xx_ehrpwm1_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = {
+	.master		= &dra7xx_l4_per2_hwmod,
+	.slave		= &dra7xx_epwmss2_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss2__ecap2 = {
+	.master		= &dra7xx_epwmss2_hwmod,
+	.slave		= &dra7xx_ecap2_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss2__eqep2 = {
+	.master		= &dra7xx_epwmss2_hwmod,
+	.slave		= &dra7xx_eqep2_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
+struct omap_hwmod_ocp_if dra7xx_epwmss2__ehrpwm2 = {
+	.master		= &dra7xx_epwmss2_hwmod,
+	.slave		= &dra7xx_ehrpwm2_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU,
+};
+
 /* l4_per1 -> gpio7 */
 static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio7 = {
 	.master		= &dra7xx_l4_per1_hwmod,
@@ -3452,6 +3679,18 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
 	&dra7xx_l3_main_1__vcp2,
 	&dra7xx_l4_per2__vcp2,
 	&dra7xx_l4_wkup__wd_timer2,
+	&dra7xx_l4_per2__epwmss0,
+	&dra7xx_epwmss0__ecap0,
+	&dra7xx_epwmss0__eqep0,
+	&dra7xx_epwmss0__ehrpwm0,
+	&dra7xx_l4_per2__epwmss1,
+	&dra7xx_epwmss1__ecap1,
+	&dra7xx_epwmss1__eqep1,
+	&dra7xx_epwmss1__ehrpwm1,
+	&dra7xx_l4_per2__epwmss2,
+	&dra7xx_epwmss2__ecap2,
+	&dra7xx_epwmss2__eqep2,
+	&dra7xx_epwmss2__ehrpwm2,
 	NULL,
 };
 
-- 
2.7.0

[PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK for PWMSS

[Franklin S Cooper Jr]

From: Vignesh R <vigneshr@ti.com>

tbclk is used by ehrpwm to generate PWM waveform on DRA7 SoC. Add Linux
clock to control ehrpwm tbclk.
The TRM says, tbclk is derived from SYSCLKOUT. SYSCLKOUT is nothing but
ehrpwm functional clock derived from the gateable interface and
functional clock of PWMSS(l4_root_clk_div).
Refer AM57x TRM SPRUHZ6[1], October 2014, Table 29-4 and Section 29.2.2.1,
Table 29-19 and the NOTE at the end of the table.

[1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
Version 3 changes:
None

 arch/arm/boot/dts/dra7xx-clocks.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi
index 357bede..d0bae06 100644
--- a/arch/arm/boot/dts/dra7xx-clocks.dtsi
+++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi
@@ -2146,4 +2146,28 @@
 		ti,bit-shift = <0>;
 		reg = <0x558>;
 	};
+
+       ehrpwm0_tbclk: ehrpwm0_tbclk {
+		#clock-cells = <0>;
+		compatible = "ti,gate-clock";
+		clocks = <&l4_root_clk_div>;
+		ti,bit-shift = <20>;
+		reg = <0x0558>;
+	};
+
+	ehrpwm1_tbclk: ehrpwm1_tbclk {
+		#clock-cells = <0>;
+		compatible = "ti,gate-clock";
+		clocks = <&l4_root_clk_div>;
+		ti,bit-shift = <21>;
+		reg = <0x0558>;
+	};
+
+	ehrpwm2_tbclk: ehrpwm2_tbclk {
+		#clock-cells = <0>;
+		compatible = "ti,gate-clock";
+		clocks = <&l4_root_clk_div>;
+		ti,bit-shift = <22>;
+		reg = <0x0558>;
+	};
 };
-- 
2.7.0

[PATCH v3 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

[Franklin S Cooper Jr]

From: Vignesh R <vigneshr@ti.com>

tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
the required clock information.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Michael Turquette <mturquette@baylibre.com>
---
Version 3 changes:
None

 drivers/clk/ti/clk-7xx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index a911d7d..7fcf818 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -304,6 +304,9 @@ static struct ti_dt_clk dra7xx_clks[] = {
 	DT_CLK("4882a000.timer", "timer_sys_ck", "timer_sys_clk_div"),
 	DT_CLK("4882c000.timer", "timer_sys_ck", "timer_sys_clk_div"),
 	DT_CLK("4882e000.timer", "timer_sys_ck", "timer_sys_clk_div"),
+	DT_CLK("4843e200.ehrpwm", "tbclk", "ehrpwm0_tbclk"),
+	DT_CLK("48440200.ehrpwm", "tbclk", "ehrpwm1_tbclk"),
+	DT_CLK("48442200.ehrpwm", "tbclk", "ehrpwm2_tbclk"),
 	DT_CLK(NULL, "sys_clkin", "sys_clkin1"),
 	DT_CLK(NULL, "dss_deshdcp_clk", "dss_deshdcp_clk"),
 	{ .node_name = NULL },
-- 
2.7.0

[PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Franklin S Cooper Jr]

From: Vignesh R <vigneshr@ti.com>

Add PWMSS device tree nodes for DRA7 SoC family and add documentation
for dt bindings.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
Version 3 changes:
None

 .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |  8 +++
 .../devicetree/bindings/pwm/pwm-tipwmss.txt        | 17 +++++-
 arch/arm/boot/dts/dra7.dtsi                        | 64 ++++++++++++++++++++++
 3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
index 9c100b2..25d91ae 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Must be "ti,<soc>-ehrpwm".
   for am33xx - compatible = "ti,am33xx-ehrpwm";
   for da850  - compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
+  for dra7xx - compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
 - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
   the cells format. The only third cell flag supported by this binding is
   PWM_POLARITY_INVERTED.
@@ -27,3 +28,10 @@ ehrpwm0: ehrpwm@0 { /* EHRPWM on da850 */
 	#pwm-cells = <3>;
 	reg = <0x300000 0x2000>;
 };
+
+ehrpwm0: ehrpwm@0 { /* EHRPWM on dra7xx */
+	compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
+	#pwm-cells = <3>;
+	reg = <0x48440200 0x80>;
+	ti,hwmods = "ehrpwm0";
+};
diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
index f7eae77..9270ce6 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
@@ -1,7 +1,9 @@
 TI SOC based PWM Subsystem
 
 Required properties:
-- compatible: Must be "ti,am33xx-pwmss";
+- compatible: Must be "ti,<soc>-pwmss".
+  for am33xx - compatible = "ti,am33xx-pwmss"
+  for dra7xx - compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss"
 - reg: physical base address and size of the registers map.
 - address-cells: Specify the number of u32 entries needed in child nodes.
 		  Should set to 1.
@@ -29,3 +31,16 @@ pwmss0: pwmss@48300000 {
 
 	/* child nodes go here */
 };
+
+epwmss0: epwmss@4843e000 { /* On DRA7xx */
+	compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+	reg = <0x4843e000 0x30>;
+	ti,hwmods = "epwmss0";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0x4843e100  0x4843e100 0x80	/* ECAP */
+		  0x4843e180  0x4843e180 0x80	/* EQEP */
+		  0x4843e200  0x4843e200 0x80>; /* EHRPWM */
+
+	/* child nodes go here */
+};
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index c4d9175..9143db0 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1597,6 +1597,70 @@
 				clock-names = "fck", "sys_clk";
 			};
 		};
+
+		epwmss0: epwmss@4843e000 {
+			compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+			reg = <0x4843e000 0x30>;
+			ti,hwmods = "epwmss0";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+			ranges = <0x4843e100  0x4843e100 0x80  /* ECAP */
+				  0x4843e180  0x4843e180 0x80  /* EQEP */
+				  0x4843e200  0x4843e200 0x80>;/* EHRPWM */
+
+			ehrpwm0: ehrpwm@4843e200 {
+				compatible = "ti,dra7xx-ehrpwm",
+					     "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x4843e200 0x80>;
+				ti,hwmods = "ehrpwm0";
+				status = "disabled";
+			};
+		};
+
+		epwmss1: epwmss@48440000 {
+			compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+			reg = <0x48440000 0x30>;
+			ti,hwmods = "epwmss1";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+			ranges = <0x48440100  0x48440100  0x80   /* ECAP */
+				  0x48440180  0x48440180  0x80   /* EQEP */
+				  0x48440200  0x48440200  0x80>; /* EHRPWM */
+
+			ehrpwm1: ehrpwm@48440200 {
+				compatible = "ti,dra7xx-ehrpwm",
+					     "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x48440200 0x80>;
+				ti,hwmods = "ehrpwm1";
+				status = "disabled";
+			};
+		};
+
+		epwmss2: epwmss@48442000 {
+			compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+			reg = <0x48442000 0x30>;
+			ti,hwmods = "epwmss2";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+
+			ranges = <0x48442100  0x48442100  0x80   /* ECAP */
+				  0x48442180  0x48442180  0x80   /* EQEP */
+				  0x48442200  0x48442200  0x80>; /* EHRPWM */
+
+			ehrpwm2: ehrpwm@48442200 {
+				compatible = "ti,dra7xx-ehrpwm",
+					     "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x48442200 0x80>;
+				ti,hwmods = "ehrpwm2";
+				status = "disabled";
+			};
+		};
 	};
 
 	thermal_zones: thermal-zones {
-- 
2.7.0

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Sekhar Nori]

+ Thierry, PWM subsystem maintainer.

On Friday 26 February 2016 04:06 AM, Franklin S Cooper Jr wrote:
> The PWMSS local clock gating registers have no real purpose on OMAP ARM
> devices. These registers were left over registers from DSP IP where the
> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
> don't function properly. TRMs will be update to indicate that these
> registers shouldn't be touched.
> 
> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
> will be removed by this patch with zero loss of functionality by the ECAP
> and EPWM drivers.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> Version 3 changes:
> New patch
> 
>  drivers/pwm/pwm-tiecap.c   | 28 --------------------------
>  drivers/pwm/pwm-tiehrpwm.c | 29 ---------------------------
>  drivers/pwm/pwm-tipwmss.c  | 49 ----------------------------------------------
>  drivers/pwm/pwm-tipwmss.h  | 39 ------------------------------------
>  4 files changed, 145 deletions(-)
>  delete mode 100644 drivers/pwm/pwm-tipwmss.h
> 
> diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
> index 616af76..9e0865d7 100644
> --- a/drivers/pwm/pwm-tiecap.c
> +++ b/drivers/pwm/pwm-tiecap.c
> @@ -27,8 +27,6 @@
>  #include <linux/pwm.h>
>  #include <linux/of_device.h>
>  
> -#include "pwm-tipwmss.h"
> -
>  /* ECAP registers and bits definitions */
>  #define CAP1			0x08
>  #define CAP2			0x0C
> @@ -206,7 +204,6 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>  	struct resource *r;
>  	struct clk *clk;
>  	struct ecap_pwm_chip *pc;
> -	u16 status;
>  
>  	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
>  	if (!pc)
> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
> -
> -	status = pwmss_submodule_state_change(pdev->dev.parent,
> -			PWMSS_ECAPCLK_EN);
> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
> -		ret = -EINVAL;
> -		goto pwmss_clk_failure;
> -	}
> -
> -	pm_runtime_put_sync(&pdev->dev);
>  
>  	platform_set_drvdata(pdev, pc);
>  	return 0;
> -
> -pwmss_clk_failure:
> -	pm_runtime_put_sync(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -	pwmchip_remove(&pc->chip);
> -	return ret;
>  }
>  
>  static int ecap_pwm_remove(struct platform_device *pdev)
>  {
>  	struct ecap_pwm_chip *pc = platform_get_drvdata(pdev);
>  
> -	pm_runtime_get_sync(&pdev->dev);
> -	/*
> -	 * Due to hardware misbehaviour, acknowledge of the stop_req
> -	 * is missing. Hence checking of the status bit skipped.
> -	 */
> -	pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ);
> -	pm_runtime_put_sync(&pdev->dev);
> -
>  	pm_runtime_disable(&pdev->dev);
>  	return pwmchip_remove(&pc->chip);
>  }
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 6a41e66..e09b1f0 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -27,8 +27,6 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/of_device.h>
>  
> -#include "pwm-tipwmss.h"
> -
>  /* EHRPWM registers and bits definitions */
>  
>  /* Time base module registers */
> @@ -437,7 +435,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
>  	struct resource *r;
>  	struct clk *clk;
>  	struct ehrpwm_pwm_chip *pc;
> -	u16 status;
>  
>  	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
>  	if (!pc)
> @@ -487,27 +484,9 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
> -
> -	status = pwmss_submodule_state_change(pdev->dev.parent,
> -			PWMSS_EPWMCLK_EN);
> -	if (!(status & PWMSS_EPWMCLK_EN_ACK)) {
> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
> -		ret = -EINVAL;
> -		goto pwmss_clk_failure;
> -	}
> -
> -	pm_runtime_put_sync(&pdev->dev);
>  
>  	platform_set_drvdata(pdev, pc);
>  	return 0;
> -
> -pwmss_clk_failure:
> -	pm_runtime_put_sync(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -	pwmchip_remove(&pc->chip);
> -	clk_unprepare(pc->tbclk);
> -	return ret;
>  }
>  
>  static int ehrpwm_pwm_remove(struct platform_device *pdev)
> @@ -516,14 +495,6 @@ static int ehrpwm_pwm_remove(struct platform_device *pdev)
>  
>  	clk_unprepare(pc->tbclk);
>  
> -	pm_runtime_get_sync(&pdev->dev);
> -	/*
> -	 * Due to hardware misbehaviour, acknowledge of the stop_req
> -	 * is missing. Hence checking of the status bit skipped.
> -	 */
> -	pwmss_submodule_state_change(pdev->dev.parent, PWMSS_EPWMCLK_STOP_REQ);
> -	pm_runtime_put_sync(&pdev->dev);
> -
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	return pwmchip_remove(&pc->chip);
> diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
> index 5cf65a1..829f499 100644
> --- a/drivers/pwm/pwm-tipwmss.c
> +++ b/drivers/pwm/pwm-tipwmss.c
> @@ -22,32 +22,6 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/of_device.h>
>  
> -#include "pwm-tipwmss.h"
> -
> -#define PWMSS_CLKCONFIG		0x8	/* Clock gating reg */
> -#define PWMSS_CLKSTATUS		0xc	/* Clock gating status reg */
> -
> -struct pwmss_info {
> -	void __iomem	*mmio_base;
> -	struct mutex	pwmss_lock;
> -	u16		pwmss_clkconfig;
> -};
> -
> -u16 pwmss_submodule_state_change(struct device *dev, int set)
> -{
> -	struct pwmss_info *info = dev_get_drvdata(dev);
> -	u16 val;
> -
> -	mutex_lock(&info->pwmss_lock);
> -	val = readw(info->mmio_base + PWMSS_CLKCONFIG);
> -	val |= set;
> -	writew(val , info->mmio_base + PWMSS_CLKCONFIG);
> -	mutex_unlock(&info->pwmss_lock);
> -
> -	return readw(info->mmio_base + PWMSS_CLKSTATUS);
> -}
> -EXPORT_SYMBOL(pwmss_submodule_state_change);
> -
>  static const struct of_device_id pwmss_of_match[] = {
>  	{ .compatible	= "ti,am33xx-pwmss" },
>  	{},
> @@ -57,24 +31,10 @@ MODULE_DEVICE_TABLE(of, pwmss_of_match);
>  static int pwmss_probe(struct platform_device *pdev)
>  {
>  	int ret;
> -	struct resource *r;
> -	struct pwmss_info *info;
>  	struct device_node *node = pdev->dev.of_node;
>  
> -	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> -	if (!info)
> -		return -ENOMEM;
> -
> -	mutex_init(&info->pwmss_lock);
> -
> -	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	info->mmio_base = devm_ioremap_resource(&pdev->dev, r);
> -	if (IS_ERR(info->mmio_base))
> -		return PTR_ERR(info->mmio_base);
> -
>  	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_get_sync(&pdev->dev);
> -	platform_set_drvdata(pdev, info);
>  
>  	/* Populate all the child nodes here... */
>  	ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
> @@ -86,30 +46,21 @@ static int pwmss_probe(struct platform_device *pdev)
>  
>  static int pwmss_remove(struct platform_device *pdev)
>  {
> -	struct pwmss_info *info = platform_get_drvdata(pdev);
> -
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> -	mutex_destroy(&info->pwmss_lock);
>  	return 0;
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
>  static int pwmss_suspend(struct device *dev)
>  {
> -	struct pwmss_info *info = dev_get_drvdata(dev);
> -
> -	info->pwmss_clkconfig = readw(info->mmio_base + PWMSS_CLKCONFIG);
>  	pm_runtime_put_sync(dev);
>  	return 0;
>  }
>  
>  static int pwmss_resume(struct device *dev)
>  {
> -	struct pwmss_info *info = dev_get_drvdata(dev);
> -
>  	pm_runtime_get_sync(dev);
> -	writew(info->pwmss_clkconfig, info->mmio_base + PWMSS_CLKCONFIG);
>  	return 0;
>  }
>  #endif
> diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h
> deleted file mode 100644
> index 10ad804..0000000
> --- a/drivers/pwm/pwm-tipwmss.h
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/*
> - * TI PWM Subsystem driver
> - *
> - * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; 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.
> - *
> - */
> -
> -#ifndef __TIPWMSS_H
> -#define __TIPWMSS_H
> -
> -/* PWM substem clock gating */
> -#define PWMSS_ECAPCLK_EN	BIT(0)
> -#define PWMSS_ECAPCLK_STOP_REQ	BIT(1)
> -#define PWMSS_EPWMCLK_EN	BIT(8)
> -#define PWMSS_EPWMCLK_STOP_REQ	BIT(9)
> -
> -#define PWMSS_ECAPCLK_EN_ACK	BIT(0)
> -#define PWMSS_EPWMCLK_EN_ACK	BIT(8)
> -
> -#ifdef CONFIG_PWM_TIPWMSS
> -extern u16 pwmss_submodule_state_change(struct device *dev, int set);
> -#else
> -static inline u16 pwmss_submodule_state_change(struct device *dev, int set)
> -{
> -	/* return success status value */
> -	return 0xFFFF;
> -}
> -#endif
> -#endif	/* __TIPWMSS_H */
> 

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Tony Lindgren]

* Sekhar Nori <nsekhar@ti.com> [160226 02:27]:
> + Thierry, PWM subsystem maintainer.

The driver changes need to be resent to Thierry please.

Regards,

Tony

Re: [PATCH v3 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> From: Vignesh R <vigneshr@ti.com>
> 
> tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
> the required clock information.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> Acked-by: Michael Turquette <mturquette@baylibre.com>

This should be sent to Tero.

Regards,

Tony

> ---
> Version 3 changes:
> None
> 
>  drivers/clk/ti/clk-7xx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> index a911d7d..7fcf818 100644
> --- a/drivers/clk/ti/clk-7xx.c
> +++ b/drivers/clk/ti/clk-7xx.c
> @@ -304,6 +304,9 @@ static struct ti_dt_clk dra7xx_clks[] = {
>  	DT_CLK("4882a000.timer", "timer_sys_ck", "timer_sys_clk_div"),
>  	DT_CLK("4882c000.timer", "timer_sys_ck", "timer_sys_clk_div"),
>  	DT_CLK("4882e000.timer", "timer_sys_ck", "timer_sys_clk_div"),
> +	DT_CLK("4843e200.ehrpwm", "tbclk", "ehrpwm0_tbclk"),
> +	DT_CLK("48440200.ehrpwm", "tbclk", "ehrpwm1_tbclk"),
> +	DT_CLK("48442200.ehrpwm", "tbclk", "ehrpwm2_tbclk"),
>  	DT_CLK(NULL, "sys_clkin", "sys_clkin1"),
>  	DT_CLK(NULL, "dss_deshdcp_clk", "dss_deshdcp_clk"),
>  	{ .node_name = NULL },
> -- 
> 2.7.0
> 

Re: [PATCH v3 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [160226 11:16]:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> > From: Vignesh R <vigneshr@ti.com>
> > 
> > tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
> > the required clock information.
> > 
> > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > Acked-by: Michael Turquette <mturquette@baylibre.com>
> 
> This should be sent to Tero.

Sorry I mean Tero should queue this one as he is already on Cc.

Regards,

Tony

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> From: Vignesh R <vigneshr@ti.com>
> 
> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
> for dt bindings.

Are the dts changes safe to pick separately? I'm mostly worried
about clock related hangs unless the clock changes are merged
first.

Regards,

Tony

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Franklin S Cooper Jr]

Hi Tony

On 02/26/2016 01:18 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> From: Vignesh R <vigneshr@ti.com>
>>
>> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
>> for dt bindings.
> 
> Are the dts changes safe to pick separately? I'm mostly worried
> about clock related hangs unless the clock changes are merged
> first.
> 
> Regards,
> 
> Tony
> 

I just did a boot test where I only had this patch applied and the dra7 board booted successfully. Of course I assume there will be problems if someone actually tried to enable EPWM or ECAP without the other clock patches.

Bootlog: http://pastebin.ubuntu.com/15208985/

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> The PWMSS local clock gating registers have no real purpose on OMAP ARM
> devices. These registers were left over registers from DSP IP where the
> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
> don't function properly. TRMs will be update to indicate that these
> registers shouldn't be touched.
> 
> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
> will be removed by this patch with zero loss of functionality by the ECAP
> and EPWM drivers.
> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
> -
> -	status = pwmss_submodule_state_change(pdev->dev.parent,
> -			PWMSS_ECAPCLK_EN);
> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
> -		ret = -EINVAL;
> -		goto pwmss_clk_failure;
> -	}
> -
> -	pm_runtime_put_sync(&pdev->dev);
>  
>  	platform_set_drvdata(pdev, pc);
>  	return 0;
> -
> -pwmss_clk_failure:
> -	pm_runtime_put_sync(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -	pwmchip_remove(&pc->chip);
> -	return ret;
>  }

Hmm but why are you also removing the pm_runtime calls? Those
actually do take care of gating the clocks via the interconnect
level code that is hwmod in this case.

Regards,

Tony

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Franklin S Cooper Jr.]

Hi Tony,

On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> The PWMSS local clock gating registers have no real purpose on OMAP ARM
>> devices. These registers were left over registers from DSP IP where the
>> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
>> don't function properly. TRMs will be update to indicate that these
>> registers shouldn't be touched.
>>
>> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
>> will be removed by this patch with zero loss of functionality by the ECAP
>> and EPWM drivers.
>> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>> -
>> -	status = pwmss_submodule_state_change(pdev->dev.parent,
>> -			PWMSS_ECAPCLK_EN);
>> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
>> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
>> -		ret = -EINVAL;
>> -		goto pwmss_clk_failure;
>> -	}
>> -
>> -	pm_runtime_put_sync(&pdev->dev);
>>  
>>  	platform_set_drvdata(pdev, pc);
>>  	return 0;
>> -
>> -pwmss_clk_failure:
>> -	pm_runtime_put_sync(&pdev->dev);
>> -	pm_runtime_disable(&pdev->dev);
>> -	pwmchip_remove(&pc->chip);
>> -	return ret;
>>  }
> Hmm but why are you also removing the pm_runtime calls? Those
> actually do take care of gating the clocks via the interconnect
> level code that is hwmod in this case.
I removed all PM runtime calls that revolved around
pwmss_submodule_state_change. Originally the driver would do
a pm_runtime_get_sync then call pwmss_submodule_state_change
and then immediately call pm_runtime_put_sync. Without
pwmss_submodule_state_change those calls would be
meaningless.  I also removed pm_runtime calls in error paths
that no longer existed.

Within ecap and epwm driver pm_runtime_get is done when
pwm_enable is called  and pm_runtime_sync calls are done
when pwm_disable is called. Similar pm_runtime_get and
pm_runtime_sync calls are done in functions that ended up
touching register's within the IP.
>
> Regards,
>
> Tony

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Tony Lindgren]

* Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]:
> On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> > Hmm but why are you also removing the pm_runtime calls? Those
> > actually do take care of gating the clocks via the interconnect
> > level code that is hwmod in this case.
> I removed all PM runtime calls that revolved around
> pwmss_submodule_state_change. Originally the driver would do
> a pm_runtime_get_sync then call pwmss_submodule_state_change
> and then immediately call pm_runtime_put_sync. Without
> pwmss_submodule_state_change those calls would be
> meaningless.  I also removed pm_runtime calls in error paths
> that no longer existed.

Typically the interconnect level code can gate the clkctrl bit
for the module with PM runtime even with no other driver specific
registers. If you remove the pm_runtime calls, that does not
happen.

Also, how do you know this change does not affect the other
SoC variants using the same driver?

Regards,

Tony

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Franklin S Cooper Jr.]

On 02/29/2016 04:55 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]:
>> On 02/29/2016 04:04 PM, Tony Lindgren wrote:
>>> Hmm but why are you also removing the pm_runtime calls? Those
>>> actually do take care of gating the clocks via the interconnect
>>> level code that is hwmod in this case.
>> I removed all PM runtime calls that revolved around
>> pwmss_submodule_state_change. Originally the driver would do
>> a pm_runtime_get_sync then call pwmss_submodule_state_change
>> and then immediately call pm_runtime_put_sync. Without
>> pwmss_submodule_state_change those calls would be
>> meaningless.  I also removed pm_runtime calls in error paths
>> that no longer existed.
> Typically the interconnect level code can gate the clkctrl bit
> for the module with PM runtime even with no other driver specific
> registers. If you remove the pm_runtime calls, that does not
> happen.

So the clocks should be unlocked when ever the IP registers are
being read/written or if the peripheral is being used for
example
the pwm signal is being generated. All these cases are already
being handled.

Using ecap driver as an example.

Pm_runtime_get_sync is called within ecap_pwm_enable when
the pwm signal is to be generated. Pm_runtime_put_sync is called
when the pwm signal is to be stopped.

When either the pwm signal polarity is set or pwm
configuration is made
then a pm_runtime_get_sync and pm_runtime_put_sync are
called within
the same function surrounding calls to the IP's registers.

Probe is calling pm_runtime_enable while remove is calling
pm_runtime_disable.

So the correct pm_runtime calls are being made from what I
can see.
I'm not sure I understand the concern since removing those
calls aren't
creating any kind of imbalance.

If I'm not addressing your concern please give me an example
of where
you see a possible issue.

> Also, how do you know this change does not affect the other
> SoC variants using the same driver?

I've tested these changes on AM335x GP and AM437x GP evms.
AM335x
and AM437x were the only other users of this driver. Sorry 
I should of
documented this in my cover-letter.
> Regards,
>
> Tony

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Tony Lindgren]

* Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]:
> 
> 
> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
> > * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]:
> >> On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> >>> Hmm but why are you also removing the pm_runtime calls? Those
> >>> actually do take care of gating the clocks via the interconnect
> >>> level code that is hwmod in this case.
> >> I removed all PM runtime calls that revolved around
> >> pwmss_submodule_state_change. Originally the driver would do
> >> a pm_runtime_get_sync then call pwmss_submodule_state_change
> >> and then immediately call pm_runtime_put_sync. Without
> >> pwmss_submodule_state_change those calls would be
> >> meaningless.  I also removed pm_runtime calls in error paths
> >> that no longer existed.
> > Typically the interconnect level code can gate the clkctrl bit
> > for the module with PM runtime even with no other driver specific
> > registers. If you remove the pm_runtime calls, that does not
> > happen.
> 
> So the clocks should be unlocked when ever the IP registers are
> being read/written or if the peripheral is being used for
> example
> the pwm signal is being generated. All these cases are already
> being handled.
> 
> Using ecap driver as an example.
> 
> Pm_runtime_get_sync is called within ecap_pwm_enable when
> the pwm signal is to be generated. Pm_runtime_put_sync is called
> when the pwm signal is to be stopped.
> 
> When either the pwm signal polarity is set or pwm
> configuration is made
> then a pm_runtime_get_sync and pm_runtime_put_sync are
> called within
> the same function surrounding calls to the IP's registers.
> 
> Probe is calling pm_runtime_enable while remove is calling
> pm_runtime_disable.

OK good to hear you have considered this. The above answers
my questions then thanks.

> So the correct pm_runtime calls are being made from what I
> can see.
> I'm not sure I understand the concern since removing those
> calls aren't
> creating any kind of imbalance.

OK thanks for checking.

> If I'm not addressing your concern please give me an example
> of where
> you see a possible issue.

No that's fine. I thought you're ripping out all of the the
pm_runtime based on just looking at the patch :)

> > Also, how do you know this change does not affect the other
> > SoC variants using the same driver?
> 
> I've tested these changes on AM335x GP and AM437x GP evms.
> AM335x
> and AM437x were the only other users of this driver. Sorry 
> I should of
> documented this in my cover-letter.

OK good to hear.

Thanks,

Tony

Re: [PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK for PWMSS

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> From: Vignesh R <vigneshr@ti.com>
> 
> tbclk is used by ehrpwm to generate PWM waveform on DRA7 SoC. Add Linux
> clock to control ehrpwm tbclk.
> The TRM says, tbclk is derived from SYSCLKOUT. SYSCLKOUT is nothing but
> ehrpwm functional clock derived from the gateable interface and
> functional clock of PWMSS(l4_root_clk_div).
> Refer AM57x TRM SPRUHZ6[1], October 2014, Table 29-4 and Section 29.2.2.1,
> Table 29-19 and the NOTE at the end of the table.
> 
> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Applying this into omap-for-v4.6/dt thanks.

Note for Tero, let's plan on getting rid of the duplicate
reg entries by using the standard clock output offset within
the clock register. I think we should easily be able to add
a binding for this and then deprecate the overlapping reg
entries.

Regards,

Tony

> --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi
> @@ -2146,4 +2146,28 @@
>  		ti,bit-shift = <0>;
>  		reg = <0x558>;
>  	};
> +
> +       ehrpwm0_tbclk: ehrpwm0_tbclk {
> +		#clock-cells = <0>;
> +		compatible = "ti,gate-clock";
> +		clocks = <&l4_root_clk_div>;
> +		ti,bit-shift = <20>;
> +		reg = <0x0558>;
> +	};
> +
> +	ehrpwm1_tbclk: ehrpwm1_tbclk {
> +		#clock-cells = <0>;
> +		compatible = "ti,gate-clock";
> +		clocks = <&l4_root_clk_div>;
> +		ti,bit-shift = <21>;
> +		reg = <0x0558>;
> +	};
> +
> +	ehrpwm2_tbclk: ehrpwm2_tbclk {
> +		#clock-cells = <0>;
> +		compatible = "ti,gate-clock";
> +		clocks = <&l4_root_clk_div>;
> +		ti,bit-shift = <22>;
> +		reg = <0x0558>;
> +	};
>  };
> -- 
> 2.7.0
> 

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> From: Vignesh R <vigneshr@ti.com>
> 
> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
> for dt bindings.

Seems to be all standard so applying this one into omap-for-v4.6/dt
thanks.

Regards,

Tony

Re: [PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK for PWMSS

[Tero Kristo]

On 03/01/2016 01:23 AM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> From: Vignesh R <vigneshr@ti.com>
>>
>> tbclk is used by ehrpwm to generate PWM waveform on DRA7 SoC. Add Linux
>> clock to control ehrpwm tbclk.
>> The TRM says, tbclk is derived from SYSCLKOUT. SYSCLKOUT is nothing but
>> ehrpwm functional clock derived from the gateable interface and
>> functional clock of PWMSS(l4_root_clk_div).
>> Refer AM57x TRM SPRUHZ6[1], October 2014, Table 29-4 and Section 29.2.2.1,
>> Table 29-19 and the NOTE at the end of the table.
>>
>> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>
> Applying this into omap-for-v4.6/dt thanks.
>
> Note for Tero, let's plan on getting rid of the duplicate
> reg entries by using the standard clock output offset within
> the clock register. I think we should easily be able to add
> a binding for this and then deprecate the overlapping reg
> entries.

Yeah, we have been discussing this offline a bit, but haven't had time 
to look at this. I believe the hwmod clock conversion is of higher 
priority still.

-Tero


>
> Regards,
>
> Tony
>
>> --- a/arch/arm/boot/dts/dra7xx-clocks.dtsi
>> +++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi
>> @@ -2146,4 +2146,28 @@
>>   		ti,bit-shift = <0>;
>>   		reg = <0x558>;
>>   	};
>> +
>> +       ehrpwm0_tbclk: ehrpwm0_tbclk {
>> +		#clock-cells = <0>;
>> +		compatible = "ti,gate-clock";
>> +		clocks = <&l4_root_clk_div>;
>> +		ti,bit-shift = <20>;
>> +		reg = <0x0558>;
>> +	};
>> +
>> +	ehrpwm1_tbclk: ehrpwm1_tbclk {
>> +		#clock-cells = <0>;
>> +		compatible = "ti,gate-clock";
>> +		clocks = <&l4_root_clk_div>;
>> +		ti,bit-shift = <21>;
>> +		reg = <0x0558>;
>> +	};
>> +
>> +	ehrpwm2_tbclk: ehrpwm2_tbclk {
>> +		#clock-cells = <0>;
>> +		compatible = "ti,gate-clock";
>> +		clocks = <&l4_root_clk_div>;
>> +		ti,bit-shift = <22>;
>> +		reg = <0x0558>;
>> +	};
>>   };
>> --
>> 2.7.0
>>

Re: [PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK for PWMSS

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [160301 04:54]:
> On 03/01/2016 01:23 AM, Tony Lindgren wrote:
> >
> >Note for Tero, let's plan on getting rid of the duplicate
> >reg entries by using the standard clock output offset within
> >the clock register. I think we should easily be able to add
> >a binding for this and then deprecate the overlapping reg
> >entries.
> 
> Yeah, we have been discussing this offline a bit, but haven't had time to
> look at this. I believe the hwmod clock conversion is of higher priority
> still.

Yes agreed.

Tony

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Paul Walmsley]

Hi guys

On Thu, 25 Feb 2016, Franklin S Cooper Jr wrote:

> From: Vignesh R <vigneshr@ti.com>
> 
> Add hwmod entries for the PWMSS on DRA7.
> 
> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
> 
> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

So I still don't understand one thing about this patch, and I apologize if 
this has been covered already and I've just forgotten it.  Why are EQEP, 
ECAP, EHRPWM listed as hwmods?  It looks, based on this data, that they 
don't have any of the Highlander integration.  Shouldn't these just be 
listed in a DT 'simple-bus' type of arrangement under epwmss0, epwmss1?  
Or am I missing something?


- Paul 


> ---
> Version 3 changes:
> Switch from SYSC_HAS_RESET_STATUS to SYSC_HAS_SOFTRESET which is the
> correct bitfield for that register.
> 
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 239 ++++++++++++++++++++++++++++++
>  1 file changed, 239 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 848356e..4b2d68b 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -383,6 +383,149 @@ static struct omap_hwmod dra7xx_dcan2_hwmod = {
>  	},
>  };
>  
> +/* pwmss  */
> +static struct omap_hwmod_class_sysconfig dra7xx_epwmss_sysc = {
> +	.rev_offs	= 0x0,
> +	.sysc_offs	= 0x4,
> +	.sysc_flags	= SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET,
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> +	.sysc_fields	= &omap_hwmod_sysc_type2,
> +};
> +
> +struct omap_hwmod_class dra7xx_epwmss_hwmod_class = {
> +	.name		= "epwmss",
> +	.sysc		= &dra7xx_epwmss_sysc,
> +};
> +
> +static struct omap_hwmod_class dra7xx_ecap_hwmod_class = {
> +	.name		= "ecap",
> +};
> +
> +static struct omap_hwmod_class dra7xx_eqep_hwmod_class = {
> +	.name		= "eqep",
> +};
> +
> +struct omap_hwmod_class dra7xx_ehrpwm_hwmod_class = {
> +	.name		= "ehrpwm",
> +};
> +
> +/* epwmss0 */
> +struct omap_hwmod dra7xx_epwmss0_hwmod = {
> +	.name		= "epwmss0",
> +	.class		= &dra7xx_epwmss_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +	.prcm		= {
> +		.omap4	= {
> +			.modulemode	= MODULEMODE_SWCTRL,
> +			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS1_CLKCTRL_OFFSET,
> +			.context_offs	= DRA7XX_RM_L4PER2_PWMSS1_CONTEXT_OFFSET,
> +		},
> +	},
> +};
> +
> +/* ecap0 */
> +struct omap_hwmod dra7xx_ecap0_hwmod = {
> +	.name		= "ecap0",
> +	.class		= &dra7xx_ecap_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* eqep0 */
> +struct omap_hwmod dra7xx_eqep0_hwmod = {
> +	.name		= "eqep0",
> +	.class		= &dra7xx_eqep_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* ehrpwm0 */
> +struct omap_hwmod dra7xx_ehrpwm0_hwmod = {
> +	.name		= "ehrpwm0",
> +	.class		= &dra7xx_ehrpwm_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* epwmss1 */
> +struct omap_hwmod dra7xx_epwmss1_hwmod = {
> +	.name		= "epwmss1",
> +	.class		= &dra7xx_epwmss_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +	.prcm		= {
> +		.omap4	= {
> +			.modulemode	= MODULEMODE_SWCTRL,
> +			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS2_CLKCTRL_OFFSET,
> +			.context_offs	= DRA7XX_RM_L4PER2_PWMSS2_CONTEXT_OFFSET,
> +		},
> +	},
> +};
> +
> +/* ecap1 */
> +struct omap_hwmod dra7xx_ecap1_hwmod = {
> +	.name		= "ecap1",
> +	.class		= &dra7xx_ecap_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* eqep1 */
> +struct omap_hwmod dra7xx_eqep1_hwmod = {
> +	.name		= "eqep1",
> +	.class		= &dra7xx_eqep_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* ehrpwm1 */
> +struct omap_hwmod dra7xx_ehrpwm1_hwmod = {
> +	.name		= "ehrpwm1",
> +	.class		= &dra7xx_ehrpwm_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* epwmss2 */
> +struct omap_hwmod dra7xx_epwmss2_hwmod = {
> +	.name		= "epwmss2",
> +	.class		= &dra7xx_epwmss_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +	.prcm		= {
> +		.omap4	= {
> +			.modulemode	= MODULEMODE_SWCTRL,
> +			.clkctrl_offs	= DRA7XX_CM_L4PER2_PWMSS3_CLKCTRL_OFFSET,
> +			.context_offs	= DRA7XX_RM_L4PER2_PWMSS3_CONTEXT_OFFSET,
> +		},
> +	},
> +};
> +
> +/* ecap2 */
> +struct omap_hwmod dra7xx_ecap2_hwmod = {
> +	.name		= "ecap2",
> +	.class		= &dra7xx_ecap_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* eqep2 */
> +struct omap_hwmod dra7xx_eqep2_hwmod = {
> +	.name		= "eqep2",
> +	.class		= &dra7xx_eqep_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
> +/* ehrpwm2 */
> +struct omap_hwmod dra7xx_ehrpwm2_hwmod = {
> +	.name		= "ehrpwm2",
> +	.class		= &dra7xx_ehrpwm_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "l4_root_clk_div",
> +};
> +
>  /*
>   * 'dma' class
>   *
> @@ -2676,6 +2819,90 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio6 = {
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
> +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss0 = {
> +	.master		= &dra7xx_l4_per2_hwmod,
> +	.slave		= &dra7xx_epwmss0_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss0__ecap0 = {
> +	.master		= &dra7xx_epwmss0_hwmod,
> +	.slave		= &dra7xx_ecap0_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss0__eqep0 = {
> +	.master		= &dra7xx_epwmss0_hwmod,
> +	.slave		= &dra7xx_eqep0_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss0__ehrpwm0 = {
> +	.master		= &dra7xx_epwmss0_hwmod,
> +	.slave		= &dra7xx_ehrpwm0_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss1 = {
> +	.master		= &dra7xx_l4_per2_hwmod,
> +	.slave		= &dra7xx_epwmss1_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss1__ecap1 = {
> +	.master		= &dra7xx_epwmss1_hwmod,
> +	.slave		= &dra7xx_ecap1_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss1__eqep1 = {
> +	.master		= &dra7xx_epwmss1_hwmod,
> +	.slave		= &dra7xx_eqep1_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss1__ehrpwm1 = {
> +	.master		= &dra7xx_epwmss1_hwmod,
> +	.slave		= &dra7xx_ehrpwm1_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = {
> +	.master		= &dra7xx_l4_per2_hwmod,
> +	.slave		= &dra7xx_epwmss2_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss2__ecap2 = {
> +	.master		= &dra7xx_epwmss2_hwmod,
> +	.slave		= &dra7xx_ecap2_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss2__eqep2 = {
> +	.master		= &dra7xx_epwmss2_hwmod,
> +	.slave		= &dra7xx_eqep2_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
> +struct omap_hwmod_ocp_if dra7xx_epwmss2__ehrpwm2 = {
> +	.master		= &dra7xx_epwmss2_hwmod,
> +	.slave		= &dra7xx_ehrpwm2_hwmod,
> +	.clk		= "l4_root_clk_div",
> +	.user		= OCP_USER_MPU,
> +};
> +
>  /* l4_per1 -> gpio7 */
>  static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio7 = {
>  	.master		= &dra7xx_l4_per1_hwmod,
> @@ -3452,6 +3679,18 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>  	&dra7xx_l3_main_1__vcp2,
>  	&dra7xx_l4_per2__vcp2,
>  	&dra7xx_l4_wkup__wd_timer2,
> +	&dra7xx_l4_per2__epwmss0,
> +	&dra7xx_epwmss0__ecap0,
> +	&dra7xx_epwmss0__eqep0,
> +	&dra7xx_epwmss0__ehrpwm0,
> +	&dra7xx_l4_per2__epwmss1,
> +	&dra7xx_epwmss1__ecap1,
> +	&dra7xx_epwmss1__eqep1,
> +	&dra7xx_epwmss1__ehrpwm1,
> +	&dra7xx_l4_per2__epwmss2,
> +	&dra7xx_epwmss2__ecap2,
> +	&dra7xx_epwmss2__eqep2,
> +	&dra7xx_epwmss2__ehrpwm2,
>  	NULL,
>  };
>  
> -- 
> 2.7.0
> 


- Paul

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Tony Lindgren]

* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> From: Vignesh R <vigneshr@ti.com>
> 
> Add hwmod entries for the PWMSS on DRA7.
> 
> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
> 
> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Looks OK to me, assuming Paul will pick this one or ack it.

FYI, the URL above is outdated, looks like there's spruhz7a.pdf
available. Not sure if that's been corrected for the divider?

Regards,

Tony

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Paul Walmsley]

On Tue, 1 Mar 2016, Tony Lindgren wrote:

> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> > From: Vignesh R <vigneshr@ti.com>
> > 
> > Add hwmod entries for the PWMSS on DRA7.
> > 
> > Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
> > equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
> > As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
> > clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
> > L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
> > 
> > [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
> 
> Looks OK to me, assuming Paul will pick this one or ack it.

Well I've already sent comments on it, it doesn't look quite ready for me 
yet.  I would hold off on the whole series because the hwmod comments also 
impact the DT files.

- Paul

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [160301 10:59]:
> On Tue, 1 Mar 2016, Tony Lindgren wrote:
> 
> > * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> > > From: Vignesh R <vigneshr@ti.com>
> > > 
> > > Add hwmod entries for the PWMSS on DRA7.
> > > 
> > > Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
> > > equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
> > > As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
> > > clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
> > > L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
> > > 
> > > [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
> > 
> > Looks OK to me, assuming Paul will pick this one or ack it.
> 
> Well I've already sent comments on it, it doesn't look quite ready for me 
> yet.  I would hold off on the whole series because the hwmod comments also 
> impact the DT files.

OK will drop the dt related patches then.

Regards,

Tony

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [160229 15:25]:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
> > From: Vignesh R <vigneshr@ti.com>
> > 
> > Add PWMSS device tree nodes for DRA7 SoC family and add documentation
> > for dt bindings.
> 
> Seems to be all standard so applying this one into omap-for-v4.6/dt
> thanks.

Except this may still need to change depending on the hwmod
related patch as pointed out by Paul. So I've reverted this
in omap-for-v4.6/dt.

Regards,

Tony

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Franklin S Cooper Jr.]

Hi Paul

On 03/01/2016 02:50 PM, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [160301 10:59]:
>> On Tue, 1 Mar 2016, Tony Lindgren wrote:
>>
>>> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>>>> From: Vignesh R <vigneshr@ti.com>
>>>>
>>>> Add hwmod entries for the PWMSS on DRA7.
>>>>
>>>> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
>>>> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
>>>> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
>>>> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
>>>> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
>>>>
>>>> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>> Looks OK to me, assuming Paul will pick this one or ack it.
>> Well I've already sent comments on it, it doesn't look quite ready for me 
>> yet.  I would hold off on the whole series because the hwmod comments also 
>> impact the DT files.
> OK will drop the dt related patches then.

Sorry you previously asked this question about why hwmod is
used for eCap, ePWM and eQEP before and it wasn't addressed.
I'll take a look at this and I will get back to you.

>
> Regards,
>
> Tony

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Rob Herring]

On Thu, Feb 25, 2016 at 04:36:36PM -0600, Franklin S Cooper Jr wrote:
> From: Vignesh R <vigneshr@ti.com>
> 
> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
> for dt bindings.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
> Version 3 changes:
> None
> 
>  .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |  8 +++
>  .../devicetree/bindings/pwm/pwm-tipwmss.txt        | 17 +++++-
>  arch/arm/boot/dts/dra7.dtsi                        | 64 ++++++++++++++++++++++
>  3 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> index 9c100b2..25d91ae 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Must be "ti,<soc>-ehrpwm".
>    for am33xx - compatible = "ti,am33xx-ehrpwm";
>    for da850  - compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
> +  for dra7xx - compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";

We're starting to push back on wildcards in compatible strings. I guess 
this is okay...

>  - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
>    the cells format. The only third cell flag supported by this binding is
>    PWM_POLARITY_INVERTED.
> @@ -27,3 +28,10 @@ ehrpwm0: ehrpwm@0 { /* EHRPWM on da850 */
>  	#pwm-cells = <3>;
>  	reg = <0x300000 0x2000>;
>  };
> +
> +ehrpwm0: ehrpwm@0 { /* EHRPWM on dra7xx */

Should be pwm@48440200

> +	compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
> +	#pwm-cells = <3>;
> +	reg = <0x48440200 0x80>;
> +	ti,hwmods = "ehrpwm0";
> +};

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Franklin S Cooper Jr.]

On 02/29/2016 05:20 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]:
>>
>> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
>>> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]:
>>>> On 02/29/2016 04:04 PM, Tony Lindgren wrote:
>>>>> Hmm but why are you also removing the pm_runtime calls? Those
>>>>> actually do take care of gating the clocks via the interconnect
>>>>> level code that is hwmod in this case.
>>>> I removed all PM runtime calls that revolved around
>>>> pwmss_submodule_state_change. Originally the driver would do
>>>> a pm_runtime_get_sync then call pwmss_submodule_state_change
>>>> and then immediately call pm_runtime_put_sync. Without
>>>> pwmss_submodule_state_change those calls would be
>>>> meaningless.  I also removed pm_runtime calls in error paths
>>>> that no longer existed.
>>> Typically the interconnect level code can gate the clkctrl bit
>>> for the module with PM runtime even with no other driver specific
>>> registers. If you remove the pm_runtime calls, that does not
>>> happen.
>> So the clocks should be unlocked when ever the IP registers are
>> being read/written or if the peripheral is being used for
>> example
>> the pwm signal is being generated. All these cases are already
>> being handled.
>>
>> Using ecap driver as an example.
>>
>> Pm_runtime_get_sync is called within ecap_pwm_enable when
>> the pwm signal is to be generated. Pm_runtime_put_sync is called
>> when the pwm signal is to be stopped.
>>
>> When either the pwm signal polarity is set or pwm
>> configuration is made
>> then a pm_runtime_get_sync and pm_runtime_put_sync are
>> called within
>> the same function surrounding calls to the IP's registers.
>>
>> Probe is calling pm_runtime_enable while remove is calling
>> pm_runtime_disable.
> OK good to hear you have considered this. The above answers
> my questions then thanks.
>
>> So the correct pm_runtime calls are being made from what I
>> can see.
>> I'm not sure I understand the concern since removing those
>> calls aren't
>> creating any kind of imbalance.
> OK thanks for checking.
>
>> If I'm not addressing your concern please give me an example
>> of where
>> you see a possible issue.
> No that's fine. I thought you're ripping out all of the the
> pm_runtime based on just looking at the patch :)
>
>>> Also, how do you know this change does not affect the other
>>> SoC variants using the same driver?
>> I've tested these changes on AM335x GP and AM437x GP evms.
>> AM335x
>> and AM437x were the only other users of this driver. Sorry 
>> I should of
>> documented this in my cover-letter.
> OK good to hear.
>
> Thanks,
>
> Tony

I know there are some comments regarding other patches in
this patchset but this patch is unrelated and can be pulled
in separately. Any objections to this or should I just
resubmit this patch by itself?

Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Tony Lindgren]

* Franklin S Cooper Jr. <fcooper@ti.com> [160302 11:41]:
> I know there are some comments regarding other patches in
> this patchset but this patch is unrelated and can be pulled
> in separately. Any objections to this or should I just
> resubmit this patch by itself?

No objections from me at least. If something needs to
change at the driver level, it sounds like a different
set of patches then.

Regards,

Tony

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Franklin S Cooper Jr.]

Hi Rob,

On 03/02/2016 12:26 PM, Rob Herring wrote:
> On Thu, Feb 25, 2016 at 04:36:36PM -0600, Franklin S Cooper Jr wrote:
>> From: Vignesh R <vigneshr@ti.com>
>>
>> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
>> for dt bindings.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>> Version 3 changes:
>> None
>>
>>  .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |  8 +++
>>  .../devicetree/bindings/pwm/pwm-tipwmss.txt        | 17 +++++-
>>  arch/arm/boot/dts/dra7.dtsi                        | 64 ++++++++++++++++++++++
>>  3 files changed, 88 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>> index 9c100b2..25d91ae 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>> @@ -4,6 +4,7 @@ Required properties:
>>  - compatible: Must be "ti,<soc>-ehrpwm".
>>    for am33xx - compatible = "ti,am33xx-ehrpwm";
>>    for da850  - compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
>> +  for dra7xx - compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
> We're starting to push back on wildcards in compatible strings. I guess 
> this is okay...
>
>>  - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
>>    the cells format. The only third cell flag supported by this binding is
>>    PWM_POLARITY_INVERTED.
>> @@ -27,3 +28,10 @@ ehrpwm0: ehrpwm@0 { /* EHRPWM on da850 */
>>  	#pwm-cells = <3>;
>>  	reg = <0x300000 0x2000>;
>>  };
>> +
>> +ehrpwm0: ehrpwm@0 { /* EHRPWM on dra7xx */
> Should be pwm@48440200

So the AM335x, AM437x  and DA850 all use ehrpwm0:
ehrpwm@<address>. Also the address of 0 simply follows the
pattern used in the other binding examples in that doc. I
can replace the 0 address in this patch and make another
patch that fixes it for the other examples in that file. But
in terms of switching from ehrpwm0:ehrpwm@<address> to
ehrpwm0:pwm@<address> that would also require making changes
to the various dtsis also. So is it worth making that
change? If so I have no problem doing it.
>
>> +	compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
>> +	#pwm-cells = <3>;
>> +	reg = <0x48440200 0x80>;
>> +	ti,hwmods = "ehrpwm0";
>> +};

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Franklin S Cooper Jr.]

Hi Paul,

On 03/02/2016 10:22 AM, Franklin S Cooper Jr. wrote:
> Hi Paul
>
> On 03/01/2016 02:50 PM, Tony Lindgren wrote:
>> * Paul Walmsley <paul@pwsan.com> [160301 10:59]:
>>> On Tue, 1 Mar 2016, Tony Lindgren wrote:
>>>
>>>> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>>>>> From: Vignesh R <vigneshr@ti.com>
>>>>>
>>>>> Add hwmod entries for the PWMSS on DRA7.
>>>>>
>>>>> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
>>>>> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
>>>>> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
>>>>> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
>>>>> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
>>>>>
>>>>> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>>> Looks OK to me, assuming Paul will pick this one or ack it.
>>> Well I've already sent comments on it, it doesn't look quite ready for me 
>>> yet.  I would hold off on the whole series because the hwmod comments also 
>>> impact the DT files.
>> OK will drop the dt related patches then.
> Sorry you previously asked this question about why hwmod is
> used for eCap, ePWM and eQEP before and it wasn't addressed.
> I'll take a look at this and I will get back to you.

So I looked into this more and verified that the eCAP and
ePWM doesn't have their own unique clock. The PWMSS receives
a clock L4PER2_L3_GICLK/2 which is passed through to its
sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible
for handling its clock internally while the subdevices have
no role in managing this clock. So this explains why we have
hwmod entries for PWMSS and why we are planning on removing
it from the various subdevices.

Since ePWM, eCAP and eQEP are subdevices of PWMSS they
shouldn't have their own concept of their "own" clock. The
ePWM , eCAP and eQEP clocks are all shared and managed by
their parent PWMSS. Once the PWMSS is enabled and has its
clock running then ePWM, eCAP and eQEP from their main clock
perspective have everything they need.

So my plan is to strip all references of clocks (including
hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get
calls made in the ePWM and eCAP will simply point to their
parent's dev (PWMSS). I did a couple of quick test using
this approach and it works. I have more testing to do but if
that checks out are you ok with the above approach?

Also I'm not sure how simple-bus fits in this picture. The
eCAP, eQEP and ePWM are all separate devices. The only thing
that they share is a single clock from their parent. So it
doesn't seem like the right approach. I'm basing this on the
info in this thread
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html
that talks about the usage of simple-bus. So if its outdated
or I"m misinterpreting it incorrectly please let me know.
>
>> Regards,
>>
>> Tony

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Paul Walmsley]

On Thu, 3 Mar 2016, Franklin S Cooper Jr. wrote:

> So I looked into this more and verified that the eCAP and
> ePWM doesn't have their own unique clock. The PWMSS receives
> a clock L4PER2_L3_GICLK/2 which is passed through to its
> sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible
> for handling its clock internally while the subdevices have
> no role in managing this clock. So this explains why we have
> hwmod entries for PWMSS and why we are planning on removing
> it from the various subdevices.

It's not whether they have their own unique clock, but whether the 
submodules have OCP integration registers, speak the idle/standby 
protocols, have direct L3/L4 ports, etc.

> Since ePWM, eCAP and eQEP are subdevices of PWMSS they
> shouldn't have their own concept of their "own" clock. The
> ePWM , eCAP and eQEP clocks are all shared and managed by
> their parent PWMSS. Once the PWMSS is enabled and has its
> clock running then ePWM, eCAP and eQEP from their main clock
> perspective have everything they need.
> 
> So my plan is to strip all references of clocks (including
> hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get
> calls made in the ePWM and eCAP will simply point to their
> parent's dev (PWMSS). I did a couple of quick test using
> this approach and it works. I have more testing to do but if
> that checks out are you ok with the above approach?

I don't know if that should be done or not.  I haven't stared at the code 
yet, but based on your description, it sounds to me that it probably 
shouldn't be done.  In any case, it's not what I meant...

> Also I'm not sure how simple-bus fits in this picture. The
> eCAP, eQEP and ePWM are all separate devices. The only thing
> that they share is a single clock from their parent. So it
> doesn't seem like the right approach. I'm basing this on the
> info in this thread
> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html
> that talks about the usage of simple-bus. So if its outdated
> or I"m misinterpreting it incorrectly please let me know.

What I meant is that the ECAP*, EQEP*, EHRPWM* devices don't need to be 
registered through the hwmod code, due to the fact that they don't have 
the integration mentioned above.  Instead, I think those three subdevices 
should be listed as child nodes of epwmss* in the DT.

Looking at the DT data from Vignesh, it looks like he's already got 
ehrpwm1 as a child node of the epwmss1:

+               epwmss1: epwmss@48440000 {
+                       compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+                       reg = <0x48440000 0x30>;
+                       ti,hwmods = "epwmss1";
+                       #address-cells = <1>;
+                       #size-cells = <1>;
+                       status = "disabled";
+                       ranges = <0x48440100  0x48440100  0x80   /* ECAP */
+                                 0x48440180  0x48440180  0x80   /* EQEP */
+                                 0x48440200  0x48440200  0x80>; /* EHRPWM */
+
+                       ehrpwm1: ehrpwm@48440200 {
+                               compatible = "ti,dra7xx-ehrpwm",
+                                            "ti,am33xx-ehrpwm";
+                               #pwm-cells = <3>;
+                               reg = <0x48440200 0x80>;
+                               ti,hwmods = "ehrpwm1";

So, drop the above line, since the subdevices don't have corresponding 
hwmods any more.

+                               status = "disabled";
+                       };

Then, here, you'd add nodes similar to ehrpwm1 for ecap1 and eqep1.  I 
can't remember at the moment if adding "simple-bus" to the epwmss1 string 
would be sufficient to register the subdevices after the epwmss1 is 
probed.  If so, maybe that's all you need.

+               };

Then repeat for epwmss0, epwmss2.


- Paul

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Franklin S Cooper Jr.]

Hi Paul,

On 03/04/2016 12:25 AM, Paul Walmsley wrote:
> On Thu, 3 Mar 2016, Franklin S Cooper Jr. wrote:
>
>> So I looked into this more and verified that the eCAP and
>> ePWM doesn't have their own unique clock. The PWMSS receives
>> a clock L4PER2_L3_GICLK/2 which is passed through to its
>> sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible
>> for handling its clock internally while the subdevices have
>> no role in managing this clock. So this explains why we have
>> hwmod entries for PWMSS and why we are planning on removing
>> it from the various subdevices.
> It's not whether they have their own unique clock, but whether the 
> submodules have OCP integration registers, speak the idle/standby 
> protocols, have direct L3/L4 ports, etc.

Sorry from a hwmod perspective your right. I initially
thought my response about the clocks were related to hwmod
but I see why I was off after reading a bit more on hwmod.
>> Since ePWM, eCAP and eQEP are subdevices of PWMSS they
>> shouldn't have their own concept of their "own" clock. The
>> ePWM , eCAP and eQEP clocks are all shared and managed by
>> their parent PWMSS. Once the PWMSS is enabled and has its
>> clock running then ePWM, eCAP and eQEP from their main clock
>> perspective have everything they need.
>>
>> So my plan is to strip all references of clocks (including
>> hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get
>> calls made in the ePWM and eCAP will simply point to their
>> parent's dev (PWMSS). I did a couple of quick test using
>> this approach and it works. I have more testing to do but if
>> that checks out are you ok with the above approach?
> I don't know if that should be done or not.  I haven't stared at the code 
> yet, but based on your description, it sounds to me that it probably 
> shouldn't be done.  In any case, it's not what I meant...

Your right my response did miss your initial point. But what
I'm proposing sounds like exactly what you were suggesting.
I think we both agree that hwmod entries for ePWM, eCAP and
eQEP should be removed. Not just for dra7 but also for
am335x and am437x as separate patches.

My comment about devm_clk_get was based on the fact that I
don't believe eCAP, ePWM and eQEP should have their own
clock defined even within DT. Instead they can just
reference the clk provided by pwmss (its parent) which in
hardware is the clock that is directly passed to the
sub-devices. The only reason those drivers need a reference
to that clk is to get its rate once during probe.

I plan on doing a bit more testing and I can shoot an
updated v4. I believe it will address all of your comments
and you will hopefully get a better understanding of the
minor change I plan on making.
>
>> Also I'm not sure how simple-bus fits in this picture. The
>> eCAP, eQEP and ePWM are all separate devices. The only thing
>> that they share is a single clock from their parent. So it
>> doesn't seem like the right approach. I'm basing this on the
>> info in this thread
>> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html
>> that talks about the usage of simple-bus. So if its outdated
>> or I"m misinterpreting it incorrectly please let me know.
> What I meant is that the ECAP*, EQEP*, EHRPWM* devices don't need to be 
> registered through the hwmod code, due to the fact that they don't have 
> the integration mentioned above.  Instead, I think those three subdevices 
> should be listed as child nodes of epwmss* in the DT.
>
> Looking at the DT data from Vignesh, it looks like he's already got 
> ehrpwm1 as a child node of the epwmss1:
>
> +               epwmss1: epwmss@48440000 {
> +                       compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
> +                       reg = <0x48440000 0x30>;
> +                       ti,hwmods = "epwmss1";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       status = "disabled";
> +                       ranges = <0x48440100  0x48440100  0x80   /* ECAP */
> +                                 0x48440180  0x48440180  0x80   /* EQEP */
> +                                 0x48440200  0x48440200  0x80>; /* EHRPWM */
> +
> +                       ehrpwm1: ehrpwm@48440200 {
> +                               compatible = "ti,dra7xx-ehrpwm",
> +                                            "ti,am33xx-ehrpwm";
> +                               #pwm-cells = <3>;
> +                               reg = <0x48440200 0x80>;
> +                               ti,hwmods = "ehrpwm1";
>
> So, drop the above line, since the subdevices don't have corresponding 
> hwmods any more.

Right
>
> +                               status = "disabled";
> +                       };
>
> Then, here, you'd add nodes similar to ehrpwm1 for ecap1 and eqep1.  I 
> can't remember at the moment if adding "simple-bus" to the epwmss1 string 
> would be sufficient to register the subdevices after the epwmss1 is 
> probed.  If so, maybe that's all you need.
>
> +               };
>
> Then repeat for epwmss0, epwmss2.

As you mentioned pwmss is the parent node in the DT while
ecap and epwm are child nodes. These child nodes must be
probed only after their parent node has been probed and the
parent clock is running. The pwm-tipwmss.c already calls
of_platform_populate(node, NULL, NULL, &pdev->dev); as the
very last step of its probe call. So what you want to happen
is already happening in the driver. So I don't believe
simple-bus is needed.
>
>
> - Paul

Re: [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

[Rob Herring]

On Thu, Mar 3, 2016 at 7:39 PM, Franklin S Cooper Jr. <fcooper@ti.com> wrote:
> Hi Rob,
>
> On 03/02/2016 12:26 PM, Rob Herring wrote:
>> On Thu, Feb 25, 2016 at 04:36:36PM -0600, Franklin S Cooper Jr wrote:
>>> From: Vignesh R <vigneshr@ti.com>
>>>
>>> Add PWMSS device tree nodes for DRA7 SoC family and add documentation
>>> for dt bindings.
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>> ---
>>> Version 3 changes:
>>> None
>>>
>>>  .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |  8 +++
>>>  .../devicetree/bindings/pwm/pwm-tipwmss.txt        | 17 +++++-
>>>  arch/arm/boot/dts/dra7.dtsi                        | 64 ++++++++++++++++++++++
>>>  3 files changed, 88 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>>> index 9c100b2..25d91ae 100644
>>> --- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
>>> @@ -4,6 +4,7 @@ Required properties:
>>>  - compatible: Must be "ti,<soc>-ehrpwm".
>>>    for am33xx - compatible = "ti,am33xx-ehrpwm";
>>>    for da850  - compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
>>> +  for dra7xx - compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
>> We're starting to push back on wildcards in compatible strings. I guess
>> this is okay...
>>
>>>  - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
>>>    the cells format. The only third cell flag supported by this binding is
>>>    PWM_POLARITY_INVERTED.
>>> @@ -27,3 +28,10 @@ ehrpwm0: ehrpwm@0 { /* EHRPWM on da850 */
>>>      #pwm-cells = <3>;
>>>      reg = <0x300000 0x2000>;
>>>  };
>>> +
>>> +ehrpwm0: ehrpwm@0 { /* EHRPWM on dra7xx */
>> Should be pwm@48440200
>
> So the AM335x, AM437x  and DA850 all use ehrpwm0:
> ehrpwm@<address>. Also the address of 0 simply follows the
> pattern used in the other binding examples in that doc. I
> can replace the 0 address in this patch and make another
> patch that fixes it for the other examples in that file. But
> in terms of switching from ehrpwm0:ehrpwm@<address> to
> ehrpwm0:pwm@<address> that would also require making changes
> to the various dtsis also. So is it worth making that
> change? If so I have no problem doing it.

Follow-up patches to fix are fine. Unit-address and reg mismatches are
going to start warning in dtc soon.

Rob

Re: [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

[Paul Walmsley]

On Fri, 4 Mar 2016, Franklin S Cooper Jr. wrote:

> So what you want to happen
> is already happening in the driver. So I don't believe
> simple-bus is needed.

OK great, even better!

- Paul