[PATCH v4 00/11] Support for AM33xx PWM Subsystem

[PATCH v4 00/11] Support for AM33xx PWM Subsystem

[Philip, Avinash]

In AM33xx PWM sub modules like ECAP, EHRPWM & EQEP are  integrated to
PWM subsystem. All these submodules shares the resources (clock) & has
a clock gating register in PWM Subsystem. This patch series creates a
parent PWM Subsystem driver to handle access synchronization of shared
resources & clock gating from PWM Subsystem configuration space.
Also Device tree nodes populated to support parent child relation
between PWMSS, ECAP & EHRPWM submodules.
In addition EHRPWM module requires explicit clock gating from control
module & is handled by patch #2 & 8.

As suggested by  Thierry for handling clock gating for PWM submodules
should handle with a global function. This requires config space
handling done independent from driver and is done at parent driver.

So the parent<->child relation adopted to handle
1. pm runtime synchronization
2. PWM subsystem common config space clock gating for PWM submodules.

Patches supports
- Driver support for parent child relation handled patch #1
- Optional EHRPWM tb clock in patch #2
- Parent child in HWMOD handled at patch #3
- Device tree binding support handled in patch #4, 6 &8
- pinctrl support in patch #5 & 7.
- DT node populated in patch #9 ,10 & 11.

This patch series based on omap_dt/for_3.8/dts_part2 and tested
on am335x-evm & am335x-evmsk.

It depends on [1]

1. https://lkml.org/lkml/2012/11/21/70
pwm: Device tree support for PWM polarity

Changes since v3:
	- Rebased on top of omap_dt/for_3.8/dts_part2
	- Add pwm backlight for am335xevm_sk
	- Moved tipwmss.h to pwm-tipwmss.h

Philip, Avinash (11):
  PWMSS: Add PWM Subsystem driver for parent<->child relationship
  ARM: am33xx: clk: Add optional clock for EHRPWM
  ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM
    subsystem
  pwm: pwm-tiecap: Add device-tree binding support for APWM driver
  pwm: pwm-tiecap: pinctrl support
  pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
  pwm: pwm-tiehrpwm: pinctrl support
  pwm: pwm-tiehrpwm: Adding TBCLK gating support.
  ARM: dts: AM33XX: Add PWMSS device tree nodes
  ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
  ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk

 .../devicetree/bindings/pwm/pwm-tiecap.txt         |   23 ++
 .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |   23 ++
 .../devicetree/bindings/pwm/pwm-tipwmss.txt        |   31 ++
 arch/arm/boot/dts/am335x-evm.dts                   |   23 ++
 arch/arm/boot/dts/am335x-evmsk.dts                 |   23 ++
 arch/arm/boot/dts/am33xx.dtsi                      |   84 ++++
 arch/arm/mach-omap2/clock33xx_data.c               |   37 ++
 arch/arm/mach-omap2/control.h                      |    8 +
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |  419 +++++++++++++-------
 drivers/pwm/Kconfig                                |   11 +
 drivers/pwm/Makefile                               |    1 +
 drivers/pwm/pwm-tiecap.c                           |   50 +++-
 drivers/pwm/pwm-tiehrpwm.c                         |   65 +++-
 drivers/pwm/pwm-tipwmss.c                          |  143 +++++++
 drivers/pwm/pwm-tipwmss.h                          |   39 ++
 15 files changed, 835 insertions(+), 145 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
 create mode 100644 drivers/pwm/pwm-tipwmss.c
 create mode 100644 drivers/pwm/pwm-tipwmss.h

[PATCH v4 01/11] PWMSS: Add PWM Subsystem driver for parent<->child relationship

[Philip, Avinash]

In some platforms (like am33xx), PWM sub modules (ECAP, EHRPWM, EQEP)
are integrated to PWM subsystem. These PWM submodules has resources
shared and only one register bit-field is provided to control
module/clock enable/disable, makes it difficult to handle common
resources from independent PWMSS submodule drivers.

So the solution here implemented in this patch is, to create driver for
PWMSS and take the role of parent driver for PWM submodules. PWMSS
parent driver enumerates all the child nodes under PWMSS module. Also
symbol "pwmss_submodule_state_change" exported to enable clock gating
for individual PWMSS submodules, and submodule drivers has to enable
clock gating from their drivers.

As this is only supported during DT boot, the parent<->child relationship
is created and populated in DT execution flow. The only required change
is inside DTS file, making EHRPWM & ECAP as a child to PWMSS node.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
Changes since v3:
	- Corrected the usage of ranges property in documentation.
	- Add pwm prefix to tipwmss.h file

Changes since v2:
	- Set build attribute to bool type.
	- Add pwm prefix to tipwmss.c file
	- Span mutex protection for read modify approach.
	- Use SET_SYSTEM_SLEEP_PM_OPS
	- Add PWM subsystem bit fields to tipwmss.h
	- Remove devinit & devexit attributes.
	- Left out adding of child node's DT property in Documentation
	  as those definitions are part of later patches.

Changes since v1:
	- Add conditional check for PWM subsystem clock enabling.
	- Add context save/restore for PWM subsystem clock config register.

:000000 100644 0000000... b067ea7... A	Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
:100644 100644 ed81720... 1dc32a8... M	drivers/pwm/Kconfig
:100644 100644 acfe482... e12b722... M	drivers/pwm/Makefile
:000000 100644 0000000... 30d3674... A	drivers/pwm/pwm-tipwmss.c
:000000 100644 0000000... 11f76a1... A	drivers/pwm/pwm-tipwmss.h
 .../devicetree/bindings/pwm/pwm-tipwmss.txt        |   31 +++++
 drivers/pwm/Kconfig                                |   11 ++
 drivers/pwm/Makefile                               |    1 +
 drivers/pwm/pwm-tipwmss.c                          |  143 ++++++++++++++++++++
 drivers/pwm/pwm-tipwmss.h                          |   39 ++++++
 5 files changed, 225 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
new file mode 100644
index 0000000..b067ea7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
@@ -0,0 +1,31 @@
+TI SOC based PWM Subsystem
+
+Required properties:
+- compatible: Must be "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.
+- size-cells: specify number of u32 entries needed to specify child nodes size
+		in reg property. Should set to 1.
+- ranges: describes the address mapping of a memory-mapped bus. Should set to
+	   physical address map of child's base address, physical address within
+	   parent's address  space and length of the address map. For am33xx,
+	   3 set of child register maps present, ECAP register space, EQEP
+	   register space, EHRPWM register space.
+
+Also child nodes should also populated under PWMSS DT node.
+
+Example:
+pwmss0: pwmss@48300000 {
+	compatible = "ti,am33xx-pwmss";
+	reg = <0x48300000 0x10>;
+	ti,hwmods = "epwmss0";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	status = "disabled";
+	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
+		  0x48300180 0x48300180 0x80   /* EQEP */
+		  0x48300200 0x48300200 0x80>; /* EHRPWM */
+
+	/* child nodes go here */
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..1dc32a8 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -125,6 +125,7 @@ config PWM_TEGRA
 config  PWM_TIECAP
 	tristate "ECAP PWM support"
 	depends on SOC_AM33XX
+	select PWM_TIPWMSS
 	help
 	  PWM driver support for the ECAP APWM controller found on AM33XX
 	  TI SOC
@@ -135,6 +136,7 @@ config  PWM_TIECAP
 config  PWM_TIEHRPWM
 	tristate "EHRPWM PWM support"
 	depends on SOC_AM33XX
+	select PWM_TIPWMSS
 	help
 	  PWM driver support for the EHRPWM controller found on AM33XX
 	  TI SOC
@@ -142,6 +144,15 @@ config  PWM_TIEHRPWM
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-tiehrpwm.
 
+config  PWM_TIPWMSS
+	bool
+	depends on SOC_AM33XX && (PWM_TIEHRPWM || PWM_TIECAP)
+	help
+	  PWM Subsystem driver support for AM33xx SOC.
+
+	  PWM submodules require PWM config space access from submodule
+	  drivers and require common parent driver support.
+
 config PWM_TWL6030
 	tristate "TWL6030 PWM support"
 	depends on TWL4030_CORE
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..e12b722 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -11,5 +11,6 @@ obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
 obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
 obj-$(CONFIG_PWM_TIECAP)	+= pwm-tiecap.o
 obj-$(CONFIG_PWM_TIEHRPWM)	+= pwm-tiehrpwm.o
+obj-$(CONFIG_PWM_TIPWMSS)	+= pwm-tipwmss.o
 obj-$(CONFIG_PWM_TWL6030)	+= pwm-twl6030.o
 obj-$(CONFIG_PWM_VT8500)	+= pwm-vt8500.o
diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
new file mode 100644
index 0000000..30d3674
--- /dev/null
+++ b/drivers/pwm/pwm-tipwmss.c
@@ -0,0 +1,143 @@
+/*
+ * 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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#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",
+	},
+	{},
+};
+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) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	mutex_init(&info->pwmss_lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "no memory resource defined\n");
+		return -ENODEV;
+	}
+
+	info->mmio_base = devm_request_and_ioremap(&pdev->dev, r);
+	if (!info->mmio_base)
+		return -EADDRNOTAVAIL;
+
+	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);
+	if (ret)
+		dev_err(&pdev->dev, "no child node found\n");
+
+	return ret;
+}
+
+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;
+}
+
+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;
+}
+
+static const struct dev_pm_ops pwmss_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pwmss_suspend, pwmss_resume)
+};
+
+static struct platform_driver pwmss_driver = {
+	.driver	= {
+		.name	= "pwmss",
+		.owner	= THIS_MODULE,
+		.pm	= &pwmss_pm_ops,
+		.of_match_table	= pwmss_of_match,
+	},
+	.probe	= pwmss_probe,
+	.remove	= pwmss_remove,
+};
+
+module_platform_driver(pwmss_driver);
+
+MODULE_DESCRIPTION("PWM Subsystem driver");
+MODULE_AUTHOR("Texas Instruments");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h
new file mode 100644
index 0000000..11f76a1
--- /dev/null
+++ b/drivers/pwm/pwm-tipwmss.h
@@ -0,0 +1,39 @@
+/*
+ * 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
+
+#ifdef CONFIG_PWM_TIPWMSS
+/* 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)
+
+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 */
-- 
1.7.0.4

Re: [PATCH v4 01/11] PWMSS: Add PWM Subsystem driver for parent<->child relationship

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

On Wed, Nov 21, 2012 at 06:40:58PM +0530, Philip, Avinash wrote:
[...]
> +static const struct of_device_id pwmss_of_match[] = {
> +	{
> +		.compatible	= "ti,am33xx-pwmss",
> +	},

For consistency with other drivers this should be all on one line.

> +static const struct dev_pm_ops pwmss_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pwmss_suspend, pwmss_resume)
> +};

This could be even shorter:

static SIMPLE_DEV_PM_OPS(pwmss_pm_ops, pwmss_suspend, pwmss_resume);

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 01/11] PWMSS: Add PWM Subsystem driver for parent<->child relationship

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:17:04, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:40:58PM +0530, Philip, Avinash wrote:
> [...]
> > +static const struct of_device_id pwmss_of_match[] = {
> > +	{
> > +		.compatible	= "ti,am33xx-pwmss",
> > +	},
> 
> For consistency with other drivers this should be all on one line.

Ok I will correct it.

> 
> > +static const struct dev_pm_ops pwmss_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(pwmss_suspend, pwmss_resume)
> > +};
> 
> This could be even shorter:
> 
> static SIMPLE_DEV_PM_OPS(pwmss_pm_ops, pwmss_suspend, pwmss_resume);

Ok I will correct it.

Thanks
Avinash

> 
> Thierry
> 

[PATCH v4 02/11] ARM: am33xx: clk: Add optional clock for EHRPWM

[Philip, Avinash]

EHRPWM module requires explicit clock gating from control module.
Hence add clock node in clock tree for EHRPWM modules.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
:100644 100644 1a45d6b... 08b21d2... M	arch/arm/mach-omap2/clock33xx_data.c
:100644 100644 a89e825... c0e34e6... M	arch/arm/mach-omap2/control.h
 arch/arm/mach-omap2/clock33xx_data.c |   37 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/control.h        |    8 +++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
index 1a45d6b..08b21d2 100644
--- a/arch/arm/mach-omap2/clock33xx_data.c
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -996,6 +996,40 @@ static struct clk wdt1_fck = {
 };
 
 /*
+ * PWMSS Time based module clock node. This node is
+ * requred to enable clock gating for EHRPWM TBCLK.
+ */
+static struct clk ehrpwm0_tbclk = {
+	.name		= "ehrpwm0_tbclk",
+	.clkdm_name	= "l4ls_clkdm",
+	.enable_reg	= AM33XX_CTRL_REGADDR(AM33XX_PWMSS_TBCLK_CLKCTRL),
+	.enable_bit	= AM33XX_PWMSS0_TBCLKEN_SHIFT,
+	.ops		= &clkops_omap2_dflt,
+	.parent		= &l4ls_gclk,
+	.recalc		= &followparent_recalc,
+};
+
+static struct clk ehrpwm1_tbclk = {
+	.name		= "ehrpwm1_tbclk",
+	.clkdm_name	= "l4ls_clkdm",
+	.enable_reg	= AM33XX_CTRL_REGADDR(AM33XX_PWMSS_TBCLK_CLKCTRL),
+	.enable_bit	= AM33XX_PWMSS1_TBCLKEN_SHIFT,
+	.ops		= &clkops_omap2_dflt,
+	.parent		= &l4ls_gclk,
+	.recalc		= &followparent_recalc,
+};
+
+static struct clk ehrpwm2_tbclk = {
+	.name		= "ehrpwm2_tbclk",
+	.clkdm_name	= "l4ls_clkdm",
+	.enable_reg	= AM33XX_CTRL_REGADDR(AM33XX_PWMSS_TBCLK_CLKCTRL),
+	.enable_bit	= AM33XX_PWMSS2_TBCLKEN_SHIFT,
+	.ops		= &clkops_omap2_dflt,
+	.parent		= &l4ls_gclk,
+	.recalc		= &followparent_recalc,
+};
+
+/*
  * clkdev
  */
 static struct omap_clk am33xx_clks[] = {
@@ -1075,6 +1109,9 @@ static struct omap_clk am33xx_clks[] = {
 	CLK(NULL,	"clkout2_ck",		&clkout2_ck,	CK_AM33XX),
 	CLK(NULL,	"timer_32k_ck",		&clkdiv32k_ick,	CK_AM33XX),
 	CLK(NULL,	"timer_sys_ck",		&sys_clkin_ck,	CK_AM33XX),
+	CLK(NULL,	"ehrpwm0_tbclk",	&ehrpwm0_tbclk,	CK_AM33XX),
+	CLK(NULL,	"ehrpwm1_tbclk",	&ehrpwm1_tbclk,	CK_AM33XX),
+	CLK(NULL,	"ehrpwm2_tbclk",	&ehrpwm2_tbclk,	CK_AM33XX),
 };
 
 int __init am33xx_clk_init(void)
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index a89e825..c0e34e6 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -357,6 +357,14 @@
 #define AM33XX_CONTROL_STATUS_SYSBOOT1_WIDTH		0x2
 #define AM33XX_CONTROL_STATUS_SYSBOOT1_MASK		(0x3 << 22)
 
+/* AM33XX PWMSS Control register */
+#define AM33XX_PWMSS_TBCLK_CLKCTRL			0x664
+
+/* AM33XX  PWMSS Control bitfields */
+#define AM33XX_PWMSS0_TBCLKEN_SHIFT			0
+#define AM33XX_PWMSS1_TBCLKEN_SHIFT			1
+#define AM33XX_PWMSS2_TBCLKEN_SHIFT			2
+
 /* CONTROL OMAP STATUS register to identify OMAP3 features */
 #define OMAP3_CONTROL_OMAP_STATUS	0x044c
 
-- 
1.7.0.4

[PATCH v4 06/11] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver

[Philip, Avinash]

This patch
1. Add support for device-tree binding for EHRWPM driver.
2. Set size of pwm-cells set to 3 to support PWM channel number, PWM
   period & polarity configuration from device tree.
3. Add enable/disable clock gating in PWM subsystem common config space.
4. When here set .owner member in platform_driver structure to
   THIS_MODULE.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Rob Landley <rob@landley.net>
---
Changes since v3:
	- Replace pwmss.h with pwm-tipwmss.h

Changes since v2:
	- Add of_pwm_xlate_with_flags function support

Changes since v1:
	- Add separate patch for pinctrl support
	- Add conditional check for PWM subsystem clock enable.
	- Combined with HWMOD changes & DT bindings.
	- Remove the custom of_xlate support.

:000000 100644 0000000... 4fc7079... A	Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
:100644 100644 d3c1dff... 34f9378... M	drivers/pwm/pwm-tiehrpwm.c
 .../devicetree/bindings/pwm/pwm-tiehrpwm.txt       |   23 ++++++++++
 drivers/pwm/pwm-tiehrpwm.c                         |   45 +++++++++++++++++++-
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
new file mode 100644
index 0000000..4fc7079
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
@@ -0,0 +1,23 @@
+TI SOC EHRPWM based PWM controller
+
+Required properties:
+- compatible : Must be "ti,am33xx-ehrpwm"
+- #pwm-cells: Should be 3. Number of cells being used to specify PWM property.
+  First cell specifies the per-chip index of the PWM to use, the second
+  cell is the period in nanoseconds and bit 0 in the third cell is used to
+  encode the polarity of PWM output. Set bit 0 of the third in PWM specifier
+  to 1 for inverse polarity & set to 0 for normal polarity.
+- reg: physical base address and size of the registers map.
+
+Optional properties:
+- ti,hwmods: Name of the hwmod associated to the EHRPWM:
+  "ehrpwm<x>", <x> being the 0-based instance number from the HW spec
+
+Example:
+
+ehrpwm0: ehrpwm@0 {
+	compatible = "ti,am33xx-ehrpwm";
+	#pwm-cells = <3>;
+	reg = <0x48300200 0x100>;
+	ti,hwmods = "ehrpwm0";
+};
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index d3c1dff..34f9378 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -25,6 +25,9 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
+#include <linux/of_device.h>
+
+#include "pwm-tipwmss.h"
 
 /* EHRPWM registers and bits definitions */
 
@@ -392,12 +395,21 @@ static const struct pwm_ops ehrpwm_pwm_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static const struct of_device_id ehrpwm_of_match[] = {
+	{
+		.compatible	= "ti,am33xx-ehrpwm",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ehrpwm_of_match);
+
 static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct resource *r;
 	struct clk *clk;
 	struct ehrpwm_pwm_chip *pc;
+	u16 status;
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc) {
@@ -419,6 +431,8 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
 
 	pc->chip.dev = &pdev->dev;
 	pc->chip.ops = &ehrpwm_pwm_ops;
+	pc->chip.of_xlate = of_pwm_xlate_with_flags;
+	pc->chip.of_pwm_n_cells = 3;
 	pc->chip.base = -1;
 	pc->chip.npwm = NUM_PWM_CHANNEL;
 
@@ -437,16 +451,41 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
 		return ret;
 	}
-
 	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);
+	return ret;
 }
 
 static int __devexit ehrpwm_pwm_remove(struct platform_device *pdev)
 {
 	struct ehrpwm_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_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);
@@ -454,7 +493,9 @@ static int __devexit ehrpwm_pwm_remove(struct platform_device *pdev)
 
 static struct platform_driver ehrpwm_pwm_driver = {
 	.driver = {
-		.name = "ehrpwm",
+		.name	= "ehrpwm",
+		.owner	= THIS_MODULE,
+		.of_match_table = ehrpwm_of_match,
 	},
 	.probe = ehrpwm_pwm_probe,
 	.remove = __devexit_p(ehrpwm_pwm_remove),
-- 
1.7.0.4

Re: [PATCH v4 06/11] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 475 bytes --]

On Wed, Nov 21, 2012 at 06:41:03PM +0530, Philip, Avinash wrote:
[...]
> +static const struct of_device_id ehrpwm_of_match[] = {
> +	{
> +		.compatible	= "ti,am33xx-ehrpwm",
> +	},

Same comment as for patch 4.

> @@ -437,16 +451,41 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
>  		return ret;
>  	}
> -

I think this blank line actually improves readability so it can stay in.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 06/11] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:33:47, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:03PM +0530, Philip, Avinash wrote:
> [...]
> > +static const struct of_device_id ehrpwm_of_match[] = {
> > +	{
> > +		.compatible	= "ti,am33xx-ehrpwm",
> > +	},
> 
> Same comment as for patch 4.

Ok I will correct it.

> 
> > @@ -437,16 +451,41 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
> >  		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
> >  		return ret;
> >  	}
> > -
> 
> I think this blank line actually improves readability so it can stay in.

I will add.

Thanks
Avinash

> 
> Thierry
> 

[PATCH v4 08/11] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

[Philip, Avinash]

Some platforms (like AM33XX) requires clock gating from control module
explicitly for TBCLK. Enabling of this clock required for the
functioning of the time base sub module in EHRPWM module. So adding
optional TBCLK handling if DT node populated with tbclkgating. This
helps the driver can coexist for Davinci platforms.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
Changes since v2:
	- Remove DT property for tbclkgating
	- Use devm_clk_get instead of clk_get

Changes since v1:
	- Moved TBCLK enable from probe to .pwm_enable & disable from
	  remove to .pwm_disable

:100644 100644 23fd3c3... 6e90829... M	drivers/pwm/pwm-tiehrpwm.c
 drivers/pwm/pwm-tiehrpwm.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 23fd3c3..6e90829 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -119,6 +119,7 @@ struct ehrpwm_pwm_chip {
 	void __iomem	*mmio_base;
 	unsigned long period_cycles[NUM_PWM_CHANNEL];
 	enum pwm_polarity polarity[NUM_PWM_CHANNEL];
+	struct	clk	*tbclk;
 };
 
 static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
@@ -339,6 +340,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	/* Channels polarity can be configured from action qualifier module */
 	configure_polarity(pc, pwm->hwpwm);
 
+	/*
+	 * Platforms require explicit clock enabling of TBCLK has
+	 * to enable TBCLK explicitly before enabling PWM device
+	 */
+	if (pc->tbclk)
+		clk_enable(pc->tbclk);
+
 	/* Enable time counter for free_run */
 	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
 	return 0;
@@ -367,6 +375,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	ehrpwm_modify(pc->mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
 
+	/* Disabling TBCLK on PWM disable */
+	if (pc->tbclk)
+		clk_disable(pc->tbclk);
+
 	/* Stop Time base counter */
 	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
 
@@ -457,6 +469,10 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
 		return ret;
 	}
+
+	/* Populate tbclk entry for platforms require explicit tbclk gating */
+	pc->tbclk = devm_clk_get(&pdev->dev, "tbclk");
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-- 
1.7.0.4

Re: [PATCH v4 08/11] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

On Wed, Nov 21, 2012 at 06:41:05PM +0530, Philip, Avinash wrote:
> Some platforms (like AM33XX) requires clock gating from control module
> explicitly for TBCLK. Enabling of this clock required for the
> functioning of the time base sub module in EHRPWM module. So adding
> optional TBCLK handling if DT node populated with tbclkgating. This
> helps the driver can coexist for Davinci platforms.
> 
> Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> ---
> Changes since v2:
> 	- Remove DT property for tbclkgating
> 	- Use devm_clk_get instead of clk_get
> 
> Changes since v1:
> 	- Moved TBCLK enable from probe to .pwm_enable & disable from
> 	  remove to .pwm_disable
> 
> :100644 100644 23fd3c3... 6e90829... M	drivers/pwm/pwm-tiehrpwm.c
>  drivers/pwm/pwm-tiehrpwm.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)

Looks good.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v4 09/11] ARM: dts: AM33XX: Add PWMSS device tree nodes

[Philip, Avinash]

Add PWMSS device tree nodes in relation with ECAP & EHRPWM DT nodes to
AM33XX SoC family. Also populates device tree nodes for ECAP & EHRPWM by
adding necessary properties like pwm-cells, base reg & set disabled as
status.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
Changes since v2:
	- ranges property populated to handle child devices address range

:100644 100644 20a3f29... 4393161... M	arch/arm/boot/dts/am33xx.dtsi
 arch/arm/boot/dts/am33xx.dtsi |   84 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 20a3f29..4393161 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -338,5 +338,89 @@
 			power = <250>;
 			ti,hwmods = "usb_otg_hs";
 		};
+
+		epwmss0: epwmss@48300000 {
+			compatible = "ti,am33xx-pwmss";
+			reg = <0x48300000 0x10>;
+			ti,hwmods = "epwmss0";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+			ranges = <0x48300100 0x48300100 0x80   /* ECAP */
+				  0x48300180 0x48300180 0x80   /* EQEP */
+				  0x48300200 0x48300200 0x80>; /* EHRPWM */
+
+			ecap0: ecap@48300100 {
+				compatible = "ti,am33xx-ecap";
+				#pwm-cells = <3>;
+				reg = <0x48300100 0x80>;
+				ti,hwmods = "ecap0";
+				status = "disabled";
+			};
+
+			ehrpwm0: ehrpwm@48300200 {
+				compatible = "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x48300200 0x80>;
+				ti,hwmods = "ehrpwm0";
+				status = "disabled";
+			};
+		};
+
+		epwmss1: epwmss@48302000 {
+			compatible = "ti,am33xx-pwmss";
+			reg = <0x48302000 0x10>;
+			ti,hwmods = "epwmss1";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+			ranges = <0x48302100 0x48302100 0x80   /* ECAP */
+				  0x48302180 0x48302180 0x80   /* EQEP */
+				  0x48302200 0x48302200 0x80>; /* EHRPWM */
+
+			ecap1: ecap@48302100 {
+				compatible = "ti,am33xx-ecap";
+				#pwm-cells = <3>;
+				reg = <0x48302100 0x80>;
+				ti,hwmods = "ecap1";
+				status = "disabled";
+			};
+
+			ehrpwm1: ehrpwm@48302200 {
+				compatible = "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x48302200 0x80>;
+				ti,hwmods = "ehrpwm1";
+				status = "disabled";
+			};
+		};
+
+		epwmss2: epwmss@48304000 {
+			compatible = "ti,am33xx-pwmss";
+			reg = <0x48304000 0x10>;
+			ti,hwmods = "epwmss2";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "disabled";
+			ranges = <0x48304100 0x48304100 0x80   /* ECAP */
+				  0x48304180 0x48304180 0x80   /* EQEP */
+				  0x48304200 0x48304200 0x80>; /* EHRPWM */
+
+			ecap2: ecap@48304100 {
+				compatible = "ti,am33xx-ecap";
+				#pwm-cells = <3>;
+				reg = <0x48304100 0x80>;
+				ti,hwmods = "ecap2";
+				status = "disabled";
+			};
+
+			ehrpwm2: ehrpwm@48304200 {
+				compatible = "ti,am33xx-ehrpwm";
+				#pwm-cells = <3>;
+				reg = <0x48304200 0x80>;
+				ti,hwmods = "ehrpwm2";
+				status = "disabled";
+			};
+		};
 	};
 };
-- 
1.7.0.4

Re: [PATCH v4 09/11] ARM: dts: AM33XX: Add PWMSS device tree nodes

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 814 bytes --]

On Wed, Nov 21, 2012 at 06:41:06PM +0530, Philip, Avinash wrote:
> Add PWMSS device tree nodes in relation with ECAP & EHRPWM DT nodes to
> AM33XX SoC family. Also populates device tree nodes for ECAP & EHRPWM by
> adding necessary properties like pwm-cells, base reg & set disabled as
> status.
> 
> Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
> ---
> Changes since v2:
> 	- ranges property populated to handle child devices address range
> 
> :100644 100644 20a3f29... 4393161... M	arch/arm/boot/dts/am33xx.dtsi
>  arch/arm/boot/dts/am33xx.dtsi |   84 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 84 insertions(+), 0 deletions(-)

Looks good to me,

Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 09/11] ARM: dts: AM33XX: Add PWMSS device tree nodes

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:38:57, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:06PM +0530, Philip, Avinash wrote:
> > Add PWMSS device tree nodes in relation with ECAP & EHRPWM DT nodes to
> > AM33XX SoC family. Also populates device tree nodes for ECAP & EHRPWM by
> > adding necessary properties like pwm-cells, base reg & set disabled as
> > status.
> > 
> > Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
> > ---
> > Changes since v2:
> > 	- ranges property populated to handle child devices address range
> > 
> > :100644 100644 20a3f29... 4393161... M	arch/arm/boot/dts/am33xx.dtsi
> >  arch/arm/boot/dts/am33xx.dtsi |   84 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 84 insertions(+), 0 deletions(-)
> 
> Looks good to me,

Thanks
Avinash

> 
> Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> 

[PATCH v4 10/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm

[Philip, Avinash]

PWM output from ecap0 uses as backlight source. Also adds low threshold
value to have a uniform divisions in brightness-levels scales.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
Changes since v3:
	- Add epwmss parent status="okay" field.

:100644 100644 9f65f17... 4178ba4c.. M	arch/arm/boot/dts/am335x-evm.dts
 arch/arm/boot/dts/am335x-evm.dts |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 9f65f17..4178ba4 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -44,6 +44,12 @@
 				0x154 0x27	/* spi0_d0.gpio0_3, INPUT | MODE7 */
 			>;
 		};
+
+		ecap0_pins: backlight_pins {
+			pinctrl-single,pins = <
+				0x164 0x0	/* eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
+			>;
+		};
 	};
 
 	ocp {
@@ -102,6 +108,16 @@
 				reg = <0x48>;
 			};
 		};
+
+		epwmss0: epwmss@48300000 {
+			status = "okay";
+
+			ecap0: ecap@48300100 {
+				status = "okay";
+				pinctrl-names = "default";
+				pinctrl-0 = <&ecap0_pins>;
+			};
+		};
 	};
 
 	vbat: fixedregulator@0 {
@@ -158,6 +174,13 @@
 			gpio-key,wakeup;
 		};
 	};
+
+	backlight {
+		compatible	= "pwm-backlight";
+		pwms = <&ecap0 0 50000 0>;
+		brightness-levels = <0 51 53 56 62 75 101 152 255>;
+		default-brightness-level = <8>;
+	};
 };
 
 /include/ "tps65910.dtsi"
-- 
1.7.0.4

Re: [PATCH v4 10/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 930 bytes --]

On Wed, Nov 21, 2012 at 06:41:07PM +0530, Philip, Avinash wrote:
[...]
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index 9f65f17..4178ba4 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -44,6 +44,12 @@
>  				0x154 0x27	/* spi0_d0.gpio0_3, INPUT | MODE7 */
>  			>;
>  		};
> +
> +		ecap0_pins: backlight_pins {
> +			pinctrl-single,pins = <
> +				0x164 0x0	/* eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
> +			>;

This looks kind of funky, but I see the rest of the file uses similar
formatting, so I guess it's okay.

> @@ -158,6 +174,13 @@
>  			gpio-key,wakeup;
>  		};
>  	};
> +
> +	backlight {
> +		compatible	= "pwm-backlight";

Maybe you don't want a tab between "compatible" and "=", but a space
instead.

Other than that, looks good:

Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 10/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:41:47, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:07PM +0530, Philip, Avinash wrote:
> [...]
> > diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> > index 9f65f17..4178ba4 100644
> > --- a/arch/arm/boot/dts/am335x-evm.dts
> > +++ b/arch/arm/boot/dts/am335x-evm.dts
> > @@ -44,6 +44,12 @@
> >  				0x154 0x27	/* spi0_d0.gpio0_3, INPUT | MODE7 */
> >  			>;
> >  		};
> > +
> > +		ecap0_pins: backlight_pins {
> > +			pinctrl-single,pins = <
> > +				0x164 0x0	/* eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
> > +			>;
> 
> This looks kind of funky, but I see the rest of the file uses similar
> formatting, so I guess it's okay.
> 
> > @@ -158,6 +174,13 @@
> >  			gpio-key,wakeup;
> >  		};
> >  	};
> > +
> > +	backlight {
> > +		compatible	= "pwm-backlight";
> 
> Maybe you don't want a tab between "compatible" and "=", but a space
> instead.

I will correct it.

Thanks
Avinash

> 
> Other than that, looks good:
> 
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> 

[PATCH v4 11/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk

[Philip, Avinash]

PWM output from ecap2 uses as backlight source. Also adds low threshold
value to have a uniform divisions in brightness-levels scales with
inverse polarity.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
---
:100644 100644 f5a6162... 6f3de83... M	arch/arm/boot/dts/am335x-evmsk.dts
 arch/arm/boot/dts/am335x-evmsk.dts |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index f5a6162..6f3de83 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -51,6 +51,12 @@
 				0x9c 0x27	/* gpmc_ben0_cle.gpio2_5, INPUT | MODE7 */
 			>;
 		};
+
+		ecap2_pins: backlight_pins {
+			pinctrl-single,pins = <
+				0x19c 0x4	/* mcasp0_ahclkr.ecap2_in_pwm2_out MODE4 */
+			>;
+		};
 	};
 
 	ocp {
@@ -94,6 +100,16 @@
 				st,max-limit-z = <750>;
 			};
 		};
+
+		epwmss2: epwmss@48304000 {
+			status = "okay";
+
+			ecap2: ecap@48304100 {
+				status = "okay";
+				pinctrl-names = "default";
+				pinctrl-0 = <&ecap2_pins>;
+			};
+		};
 	};
 
 	vbat: fixedregulator@0 {
@@ -170,6 +186,13 @@
 			gpios = <&gpio3 5 0>;
 		};
 	};
+
+	backlight {
+		compatible	= "pwm-backlight";
+		pwms = <&ecap2 0 50000 1>;
+		brightness-levels = <0 58 61 66 75 90 125 170 255>;
+		default-brightness-level = <8>;
+	};
 };
 
 /include/ "tps65910.dtsi"
-- 
1.7.0.4

Re: [PATCH v4 11/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On Wed, Nov 21, 2012 at 06:41:08PM +0530, Philip, Avinash wrote:
> PWM output from ecap2 uses as backlight source. Also adds low threshold
> value to have a uniform divisions in brightness-levels scales with
> inverse polarity.
> 
> Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> ---
> :100644 100644 f5a6162... 6f3de83... M	arch/arm/boot/dts/am335x-evmsk.dts
>  arch/arm/boot/dts/am335x-evmsk.dts |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)

Same comments as for the previous patch, but otherwise:

Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 11/11] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:42:45, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:08PM +0530, Philip, Avinash wrote:
> > PWM output from ecap2 uses as backlight source. Also adds low threshold
> > value to have a uniform divisions in brightness-levels scales with
> > inverse polarity.
> > 
> > Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
> > ---
> > :100644 100644 f5a6162... 6f3de83... M	arch/arm/boot/dts/am335x-evmsk.dts
> >  arch/arm/boot/dts/am335x-evmsk.dts |   23 +++++++++++++++++++++++
> >  1 files changed, 23 insertions(+), 0 deletions(-)
> 
> Same comments as for the previous patch, but otherwise:

I will correct it

Thanks
Avinash
> 
> Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> 

[PATCH v4 03/11] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem

[Philip, Avinash]

As part of PWM subsystem integration, PWM subsystem are sharing
resources like clock across submodules (ECAP, EQEP & EHRPWM).
To handle resource sharing & IP integration
1. Rework on parent child relation between PWMSS and
   ECAP, EQEP & EHRPWM child devices to support runtime PM.
2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional
   clock gating from control module.
3. Add HWMOD entries for EQEP PWM submodule.

Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
---
Changes since v1:
	- Remove ADDR_TYPE_RT for PWM sub module register entries.

:100644 100644 59d5c1c... b45e6d3... M	arch/arm/mach-omap2/omap_hwmod_33xx_data.c
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |  419 ++++++++++++++++++----------
 1 files changed, 276 insertions(+), 143 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
index 59d5c1c..b45e6d3 100644
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -768,9 +768,7 @@ static struct omap_hwmod am33xx_elm_hwmod = {
 	},
 };
 
-/*
- * 'epwmss' class: ecap0,1,2,  ehrpwm0,1,2
- */
+/* pwmss */
 static struct omap_hwmod_class_sysconfig am33xx_epwmss_sysc = {
 	.rev_offs	= 0x0,
 	.sysc_offs	= 0x4,
@@ -786,18 +784,23 @@ static struct omap_hwmod_class am33xx_epwmss_hwmod_class = {
 	.sysc		= &am33xx_epwmss_sysc,
 };
 
-/* ehrpwm0 */
-static struct omap_hwmod_irq_info am33xx_ehrpwm0_irqs[] = {
-	{ .name = "int", .irq = 86 + OMAP_INTC_START, },
-	{ .name = "tzint", .irq = 58 + OMAP_INTC_START, },
-	{ .irq = -1 },
+
+static struct omap_hwmod_class am33xx_ecap_hwmod_class = {
+	.name		= "ecap",
 };
 
-static struct omap_hwmod am33xx_ehrpwm0_hwmod = {
-	.name		= "ehrpwm0",
+static struct omap_hwmod_class am33xx_eqep_hwmod_class = {
+	.name		= "eqep",
+};
+
+static struct omap_hwmod_class am33xx_ehrpwm_hwmod_class = {
+	.name		= "ehrpwm",
+};
+/* epwmss0 */
+static struct omap_hwmod am33xx_epwmss0_hwmod = {
+	.name		= "epwmss0",
 	.class		= &am33xx_epwmss_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
-	.mpu_irqs	= am33xx_ehrpwm0_irqs,
 	.main_clk	= "l4ls_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -807,63 +810,68 @@ static struct omap_hwmod am33xx_ehrpwm0_hwmod = {
 	},
 };
 
-/* ehrpwm1 */
-static struct omap_hwmod_irq_info am33xx_ehrpwm1_irqs[] = {
-	{ .name = "int", .irq = 87 + OMAP_INTC_START, },
-	{ .name = "tzint", .irq = 59 + OMAP_INTC_START, },
+/* ecap0 */
+static struct omap_hwmod_irq_info am33xx_ecap0_irqs[] = {
+	{ .irq = 31 + OMAP_INTC_START, },
 	{ .irq = -1 },
 };
 
-static struct omap_hwmod am33xx_ehrpwm1_hwmod = {
-	.name		= "ehrpwm1",
-	.class		= &am33xx_epwmss_hwmod_class,
+static struct omap_hwmod am33xx_ecap0_hwmod = {
+	.name		= "ecap0",
+	.class		= &am33xx_ecap_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
-	.mpu_irqs	= am33xx_ehrpwm1_irqs,
+	.mpu_irqs	= am33xx_ecap0_irqs,
 	.main_clk	= "l4ls_gclk",
-	.prcm		= {
-		.omap4	= {
-			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS1_CLKCTRL_OFFSET,
-			.modulemode	= MODULEMODE_SWCTRL,
-		},
-	},
 };
 
-/* ehrpwm2 */
-static struct omap_hwmod_irq_info am33xx_ehrpwm2_irqs[] = {
-	{ .name = "int", .irq = 39 + OMAP_INTC_START, },
-	{ .name = "tzint", .irq = 60 + OMAP_INTC_START, },
+/* eqep0 */
+static struct omap_hwmod_irq_info am33xx_eqep0_irqs[] = {
+	{ .irq = 79 + OMAP_INTC_START, },
 	{ .irq = -1 },
 };
 
-static struct omap_hwmod am33xx_ehrpwm2_hwmod = {
-	.name		= "ehrpwm2",
-	.class		= &am33xx_epwmss_hwmod_class,
+static struct omap_hwmod am33xx_eqep0_hwmod = {
+	.name		= "eqep0",
+	.class		= &am33xx_eqep_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
-	.mpu_irqs	= am33xx_ehrpwm2_irqs,
+	.mpu_irqs	= am33xx_eqep0_irqs,
 	.main_clk	= "l4ls_gclk",
-	.prcm		= {
-		.omap4	= {
-			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS2_CLKCTRL_OFFSET,
-			.modulemode	= MODULEMODE_SWCTRL,
-		},
-	},
 };
 
-/* ecap0 */
-static struct omap_hwmod_irq_info am33xx_ecap0_irqs[] = {
-	{ .irq = 31 + OMAP_INTC_START, },
+/* ehrpwm0 */
+static struct omap_hwmod_irq_info am33xx_ehrpwm0_irqs[] = {
+	{ .name = "int", .irq = 86 + OMAP_INTC_START, },
+	{ .name = "tzint", .irq = 58 + OMAP_INTC_START, },
 	{ .irq = -1 },
 };
 
-static struct omap_hwmod am33xx_ecap0_hwmod = {
-	.name		= "ecap0",
+/*
+ * Optional clock entry is provided to support additional clock
+ * gating for EHRPWM module functional from control module.
+ */
+static struct omap_hwmod_opt_clk ehrpwm0_opt_clks[] = {
+	{ .role = "tbclk", .clk = "ehrpwm0_tbclk" },
+};
+
+static struct omap_hwmod am33xx_ehrpwm0_hwmod = {
+	.name		= "ehrpwm0",
+	.class		= &am33xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
+	.mpu_irqs	= am33xx_ehrpwm0_irqs,
+	.main_clk	= "l4ls_gclk",
+	.opt_clks	= ehrpwm0_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(ehrpwm0_opt_clks),
+};
+
+/* epwmss1 */
+static struct omap_hwmod am33xx_epwmss1_hwmod = {
+	.name		= "epwmss1",
 	.class		= &am33xx_epwmss_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
-	.mpu_irqs	= am33xx_ecap0_irqs,
 	.main_clk	= "l4ls_gclk",
 	.prcm		= {
 		.omap4	= {
-			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS0_CLKCTRL_OFFSET,
+			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS1_CLKCTRL_OFFSET,
 			.modulemode	= MODULEMODE_SWCTRL,
 		},
 	},
@@ -877,13 +885,60 @@ static struct omap_hwmod_irq_info am33xx_ecap1_irqs[] = {
 
 static struct omap_hwmod am33xx_ecap1_hwmod = {
 	.name		= "ecap1",
-	.class		= &am33xx_epwmss_hwmod_class,
+	.class		= &am33xx_ecap_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
 	.mpu_irqs	= am33xx_ecap1_irqs,
 	.main_clk	= "l4ls_gclk",
+};
+
+/* eqep1 */
+static struct omap_hwmod_irq_info am33xx_eqep1_irqs[] = {
+	{ .irq = 88 + OMAP_INTC_START, },
+	{ .irq = -1 },
+};
+
+static struct omap_hwmod am33xx_eqep1_hwmod = {
+	.name		= "eqep1",
+	.class		= &am33xx_eqep_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
+	.mpu_irqs	= am33xx_eqep1_irqs,
+	.main_clk	= "l4ls_gclk",
+};
+
+/* ehrpwm1 */
+static struct omap_hwmod_irq_info am33xx_ehrpwm1_irqs[] = {
+	{ .name = "int", .irq = 87 + OMAP_INTC_START, },
+	{ .name = "tzint", .irq = 59 + OMAP_INTC_START, },
+	{ .irq = -1 },
+};
+
+/*
+ * Optional clock entry is provided to support additional clock
+ * gating for EHRPWM module functional from control module.
+ */
+static struct omap_hwmod_opt_clk ehrpwm1_opt_clks[] = {
+	{ .role = "tbclk", .clk = "ehrpwm1_tbclk" },
+};
+
+static struct omap_hwmod am33xx_ehrpwm1_hwmod = {
+	.name		= "ehrpwm1",
+	.class		= &am33xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
+	.mpu_irqs	= am33xx_ehrpwm1_irqs,
+	.main_clk	= "l4ls_gclk",
+	.opt_clks	= ehrpwm1_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(ehrpwm1_opt_clks),
+};
+
+/* epwmss2 */
+static struct omap_hwmod am33xx_epwmss2_hwmod = {
+	.name		= "epwmss2",
+	.class		= &am33xx_epwmss_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
+	.main_clk	= "l4ls_gclk",
 	.prcm		= {
 		.omap4	= {
-			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS1_CLKCTRL_OFFSET,
+			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS2_CLKCTRL_OFFSET,
 			.modulemode	= MODULEMODE_SWCTRL,
 		},
 	},
@@ -897,16 +952,49 @@ static struct omap_hwmod_irq_info am33xx_ecap2_irqs[] = {
 
 static struct omap_hwmod am33xx_ecap2_hwmod = {
 	.name		= "ecap2",
+	.class		= &am33xx_ecap_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
 	.mpu_irqs	= am33xx_ecap2_irqs,
-	.class		= &am33xx_epwmss_hwmod_class,
+	.main_clk	= "l4ls_gclk",
+};
+
+/* eqep2 */
+static struct omap_hwmod_irq_info am33xx_eqep2_irqs[] = {
+	{ .irq = 89 + OMAP_INTC_START, },
+	{ .irq = -1 },
+};
+
+static struct omap_hwmod am33xx_eqep2_hwmod = {
+	.name		= "eqep2",
+	.class		= &am33xx_eqep_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
+	.mpu_irqs	= am33xx_eqep2_irqs,
 	.main_clk	= "l4ls_gclk",
-	.prcm		= {
-		.omap4	= {
-			.clkctrl_offs	= AM33XX_CM_PER_EPWMSS2_CLKCTRL_OFFSET,
-			.modulemode	= MODULEMODE_SWCTRL,
-		},
-	},
+};
+
+/* ehrpwm2 */
+static struct omap_hwmod_irq_info am33xx_ehrpwm2_irqs[] = {
+	{ .name = "int", .irq = 39 + OMAP_INTC_START, },
+	{ .name = "tzint", .irq = 60 + OMAP_INTC_START, },
+	{ .irq = -1 },
+};
+
+/*
+ * Optional clock entry is provided to support additional clock
+ * gating for EHRPWM module functional from control module.
+ */
+static struct omap_hwmod_opt_clk ehrpwm2_opt_clks[] = {
+	{ .role = "tbclk", .clk = "ehrpwm2_tbclk" },
+};
+
+static struct omap_hwmod am33xx_ehrpwm2_hwmod = {
+	.name		= "ehrpwm2",
+	.class		= &am33xx_ehrpwm_hwmod_class,
+	.clkdm_name	= "l4ls_clkdm",
+	.mpu_irqs	= am33xx_ehrpwm2_irqs,
+	.main_clk	= "l4ls_gclk",
+	.opt_clks	= ehrpwm2_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(ehrpwm2_opt_clks),
 };
 
 /*
@@ -2518,162 +2606,201 @@ static struct omap_hwmod_ocp_if am33xx_l4_ls__elm = {
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ehrpwm0_addr_space[] = {
+static struct omap_hwmod_addr_space am33xx_epwmss0_addr_space[] = {
 	{
 		.pa_start	= 0x48300000,
 		.pa_end		= 0x48300000 + SZ_16 - 1,
 		.flags		= ADDR_TYPE_RT
 	},
-	{
-		.pa_start	= 0x48300200,
-		.pa_end		= 0x48300200 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
-	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ehrpwm0 = {
+static struct omap_hwmod_ocp_if am33xx_l4_ls__epwmss0 = {
 	.master		= &am33xx_l4_ls_hwmod,
-	.slave		= &am33xx_ehrpwm0_hwmod,
+	.slave		= &am33xx_epwmss0_hwmod,
 	.clk		= "l4ls_gclk",
-	.addr		= am33xx_ehrpwm0_addr_space,
+	.addr		= am33xx_epwmss0_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ehrpwm1_addr_space[] = {
-	{
-		.pa_start	= 0x48302000,
-		.pa_end		= 0x48302000 + SZ_16 - 1,
-		.flags		= ADDR_TYPE_RT
-	},
+static struct omap_hwmod_addr_space am33xx_ecap0_addr_space[] = {
 	{
-		.pa_start	= 0x48302200,
-		.pa_end		= 0x48302200 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
+		.pa_start	= 0x48300100,
+		.pa_end		= 0x48300100 + SZ_128 - 1,
 	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ehrpwm1 = {
-	.master		= &am33xx_l4_ls_hwmod,
-	.slave		= &am33xx_ehrpwm1_hwmod,
+static struct omap_hwmod_ocp_if am33xx_epwmss0__ecap0 = {
+	.master		= &am33xx_epwmss0_hwmod,
+	.slave		= &am33xx_ecap0_hwmod,
 	.clk		= "l4ls_gclk",
-	.addr		= am33xx_ehrpwm1_addr_space,
+	.addr		= am33xx_ecap0_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ehrpwm2_addr_space[] = {
+static struct omap_hwmod_addr_space am33xx_eqep0_addr_space[] = {
 	{
-		.pa_start	= 0x48304000,
-		.pa_end		= 0x48304000 + SZ_16 - 1,
-		.flags		= ADDR_TYPE_RT
-	},
-	{
-		.pa_start	= 0x48304200,
-		.pa_end		= 0x48304200 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
+		.pa_start	= 0x48300180,
+		.pa_end		= 0x48300180 + SZ_128 - 1,
 	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ehrpwm2 = {
-	.master		= &am33xx_l4_ls_hwmod,
-	.slave		= &am33xx_ehrpwm2_hwmod,
+static struct omap_hwmod_ocp_if am33xx_epwmss0__eqep0 = {
+	.master		= &am33xx_epwmss0_hwmod,
+	.slave		= &am33xx_eqep0_hwmod,
 	.clk		= "l4ls_gclk",
-	.addr		= am33xx_ehrpwm2_addr_space,
+	.addr		= am33xx_eqep0_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ecap0_addr_space[] = {
-	{
-		.pa_start	= 0x48300000,
-		.pa_end		= 0x48300000 + SZ_16 - 1,
-		.flags		= ADDR_TYPE_RT
-	},
+static struct omap_hwmod_addr_space am33xx_ehrpwm0_addr_space[] = {
 	{
-		.pa_start	= 0x48300100,
-		.pa_end		= 0x48300100 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
+		.pa_start	= 0x48300200,
+		.pa_end		= 0x48300200 + SZ_128 - 1,
 	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ecap0 = {
-	.master		= &am33xx_l4_ls_hwmod,
-	.slave		= &am33xx_ecap0_hwmod,
+static struct omap_hwmod_ocp_if am33xx_epwmss0__ehrpwm0 = {
+	.master		= &am33xx_epwmss0_hwmod,
+	.slave		= &am33xx_ehrpwm0_hwmod,
 	.clk		= "l4ls_gclk",
-	.addr		= am33xx_ecap0_addr_space,
+	.addr		= am33xx_ehrpwm0_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ecap1_addr_space[] = {
+static struct omap_hwmod_addr_space am33xx_epwmss1_addr_space[] = {
 	{
 		.pa_start	= 0x48302000,
 		.pa_end		= 0x48302000 + SZ_16 - 1,
 		.flags		= ADDR_TYPE_RT
 	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_l4_ls__epwmss1 = {
+	.master		= &am33xx_l4_ls_hwmod,
+	.slave		= &am33xx_epwmss1_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_epwmss1_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am33xx_ecap1_addr_space[] = {
 	{
 		.pa_start	= 0x48302100,
-		.pa_end		= 0x48302100 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
+		.pa_end		= 0x48302100 + SZ_128 - 1,
 	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ecap1 = {
-	.master		= &am33xx_l4_ls_hwmod,
+static struct omap_hwmod_ocp_if am33xx_epwmss1__ecap1 = {
+	.master		= &am33xx_epwmss1_hwmod,
 	.slave		= &am33xx_ecap1_hwmod,
 	.clk		= "l4ls_gclk",
 	.addr		= am33xx_ecap1_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
-/*
- * Splitting the resources to handle access of PWMSS config space
- * and module specific part independently
- */
-static struct omap_hwmod_addr_space am33xx_ecap2_addr_space[] = {
+static struct omap_hwmod_addr_space am33xx_eqep1_addr_space[] = {
+	{
+		.pa_start	= 0x48302180,
+		.pa_end		= 0x48302180 + SZ_128 - 1,
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_epwmss1__eqep1 = {
+	.master		= &am33xx_epwmss1_hwmod,
+	.slave		= &am33xx_eqep1_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_eqep1_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am33xx_ehrpwm1_addr_space[] = {
+	{
+		.pa_start	= 0x48302200,
+		.pa_end		= 0x48302200 + SZ_128 - 1,
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_epwmss1__ehrpwm1 = {
+	.master		= &am33xx_epwmss1_hwmod,
+	.slave		= &am33xx_ehrpwm1_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_ehrpwm1_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am33xx_epwmss2_addr_space[] = {
 	{
 		.pa_start	= 0x48304000,
 		.pa_end		= 0x48304000 + SZ_16 - 1,
 		.flags		= ADDR_TYPE_RT
 	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_l4_ls__epwmss2 = {
+	.master		= &am33xx_l4_ls_hwmod,
+	.slave		= &am33xx_epwmss2_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_epwmss2_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am33xx_ecap2_addr_space[] = {
 	{
 		.pa_start	= 0x48304100,
-		.pa_end		= 0x48304100 + SZ_256 - 1,
-		.flags		= ADDR_TYPE_RT
+		.pa_end		= 0x48304100 + SZ_128 - 1,
 	},
 	{ }
 };
 
-static struct omap_hwmod_ocp_if am33xx_l4_ls__ecap2 = {
-	.master		= &am33xx_l4_ls_hwmod,
+static struct omap_hwmod_ocp_if am33xx_epwmss2__ecap2 = {
+	.master		= &am33xx_epwmss2_hwmod,
 	.slave		= &am33xx_ecap2_hwmod,
 	.clk		= "l4ls_gclk",
 	.addr		= am33xx_ecap2_addr_space,
 	.user		= OCP_USER_MPU,
 };
 
+static struct omap_hwmod_addr_space am33xx_eqep2_addr_space[] = {
+	{
+		.pa_start	= 0x48304180,
+		.pa_end		= 0x48304180 + SZ_128 - 1,
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_epwmss2__eqep2 = {
+	.master		= &am33xx_epwmss2_hwmod,
+	.slave		= &am33xx_eqep2_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_eqep2_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am33xx_ehrpwm2_addr_space[] = {
+	{
+		.pa_start	= 0x48304200,
+		.pa_end		= 0x48304200 + SZ_128 - 1,
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if am33xx_epwmss2__ehrpwm2 = {
+	.master		= &am33xx_epwmss2_hwmod,
+	.slave		= &am33xx_ehrpwm2_hwmod,
+	.clk		= "l4ls_gclk",
+	.addr		= am33xx_ehrpwm2_addr_space,
+	.user		= OCP_USER_MPU,
+};
+
 /* l3s cfg -> gpmc */
 static struct omap_hwmod_addr_space am33xx_gpmc_addr_space[] = {
 	{
@@ -3356,12 +3483,18 @@ static struct omap_hwmod_ocp_if *am33xx_hwmod_ocp_ifs[] __initdata = {
 	&am33xx_l4_ls__uart6,
 	&am33xx_l4_ls__spinlock,
 	&am33xx_l4_ls__elm,
-	&am33xx_l4_ls__ehrpwm0,
-	&am33xx_l4_ls__ehrpwm1,
-	&am33xx_l4_ls__ehrpwm2,
-	&am33xx_l4_ls__ecap0,
-	&am33xx_l4_ls__ecap1,
-	&am33xx_l4_ls__ecap2,
+	&am33xx_l4_ls__epwmss0,
+	&am33xx_epwmss0__ecap0,
+	&am33xx_epwmss0__eqep0,
+	&am33xx_epwmss0__ehrpwm0,
+	&am33xx_l4_ls__epwmss1,
+	&am33xx_epwmss1__ecap1,
+	&am33xx_epwmss1__eqep1,
+	&am33xx_epwmss1__ehrpwm1,
+	&am33xx_l4_ls__epwmss2,
+	&am33xx_epwmss2__ecap2,
+	&am33xx_epwmss2__eqep2,
+	&am33xx_epwmss2__ehrpwm2,
 	&am33xx_l3_s__gpmc,
 	&am33xx_l3_main__lcdc,
 	&am33xx_l4_ls__mcspi0,
-- 
1.7.0.4

[PATCH v4 04/11] pwm: pwm-tiecap: Add device-tree binding support for APWM driver

[Philip, Avinash]

This patch
1. Add support for device-tree binding for ECAP APWM driver.
2. Set size of pwm-cells set to 3 to support PWM channel number, PWM
   period & polarity configuration from device tree.
3. Add enable/disable clock gating in PWM subsystem common config space.
4. When here set .owner member in platform_driver structure to
   THIS_MODULE.

Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Rob Landley <rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org>
---
Changes since v3:
	- Replace pwmss.h with pwm-tipwmss.h

Changes since v2:
	- Add of_pwm_xlate_with_flags function support

Changes since v1:
	- Add separate patch for pinctrl support
	- Add conditional check for PWM subsystem clock enable.
	- Combined with HWMOD changes & DT bindings.
	- Remove the custom of_xlate support.

:000000 100644 0000000... 131e8c1... A	Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
:100644 100644 d6d4cf0... e0bcc85... M	drivers/pwm/pwm-tiecap.c
 .../devicetree/bindings/pwm/pwm-tiecap.txt         |   23 ++++++++++
 drivers/pwm/pwm-tiecap.c                           |   44 +++++++++++++++++++-
 2 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
new file mode 100644
index 0000000..131e8c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
@@ -0,0 +1,23 @@
+TI SOC ECAP based APWM controller
+
+Required properties:
+- compatible: Must be "ti,am33xx-ecap"
+- #pwm-cells: Should be 3. Number of cells being used to specify PWM property.
+  First cell specifies the per-chip index of the PWM to use, the second
+  cell is the period in nanoseconds and bit 0 in the third cell is used to
+  encode the polarity of PWM output. Set bit 0 of the third in PWM specifier
+  to 1 for inverse polarity & set to 0 for normal polarity.
+- reg: physical base address and size of the registers map.
+
+Optional properties:
+- ti,hwmods: Name of the hwmod associated to the ECAP:
+  "ecap<x>", <x> being the 0-based instance number from the HW spec
+
+Example:
+
+ecap0: ecap@0 {
+	compatible = "ti,am33xx-ecap";
+	#pwm-cells = <3>;
+	reg = <0x48300100 0x80>;
+	ti,hwmods = "ecap0";
+};
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index d6d4cf0..e0bcc85 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -25,6 +25,9 @@
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/pwm.h>
+#include <linux/of_device.h>
+
+#include "pwm-tipwmss.h"
 
 /* ECAP registers and bits definitions */
 #define CAP1			0x08
@@ -184,12 +187,21 @@ static const struct pwm_ops ecap_pwm_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static const struct of_device_id ecap_of_match[] = {
+	{
+		.compatible	= "ti,am33xx-ecap",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ecap_of_match);
+
 static int __devinit ecap_pwm_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct resource *r;
 	struct clk *clk;
 	struct ecap_pwm_chip *pc;
+	u16 status;
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc) {
@@ -211,6 +223,8 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
 
 	pc->chip.dev = &pdev->dev;
 	pc->chip.ops = &ecap_pwm_ops;
+	pc->chip.of_xlate = of_pwm_xlate_with_flags;
+	pc->chip.of_pwm_n_cells = 3;
 	pc->chip.base = -1;
 	pc->chip.npwm = 1;
 
@@ -231,14 +245,40 @@ static int __devinit 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 __devexit 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_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return pwmchip_remove(&pc->chip);
@@ -246,7 +286,9 @@ static int __devexit ecap_pwm_remove(struct platform_device *pdev)
 
 static struct platform_driver ecap_pwm_driver = {
 	.driver = {
-		.name = "ecap",
+		.name	= "ecap",
+		.owner	= THIS_MODULE,
+		.of_match_table = ecap_of_match,
 	},
 	.probe = ecap_pwm_probe,
 	.remove = __devexit_p(ecap_pwm_remove),
-- 
1.7.0.4

Re: [PATCH v4 04/11] pwm: pwm-tiecap: Add device-tree binding support for APWM driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

On Wed, Nov 21, 2012 at 06:41:01PM +0530, Philip, Avinash wrote:
[...]
> +static const struct of_device_id ecap_of_match[] = {
> +	{
> +		.compatible	= "ti,am33xx-ecap",
> +	},

Same here, can be shorter by putting it on one line instead of three.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 04/11] pwm: pwm-tiecap: Add device-tree binding support for APWM driver

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:27:33, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:01PM +0530, Philip, Avinash wrote:
> [...]
> > +static const struct of_device_id ecap_of_match[] = {
> > +	{
> > +		.compatible	= "ti,am33xx-ecap",
> > +	},
> 
> Same here, can be shorter by putting it on one line instead of three.

I will correct it

Thanks
Avinash
> 
> Thierry
> 

Re: [PATCH v4 04/11] pwm: pwm-tiecap: Add device-tree binding support for APWM driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 139 bytes --]

I forgot, the subject should be:

	pwm: tiecap: Add device-tree binding

for consistency. Same applies for patches 5, 6, 7 and 8.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Philip, Avinash]

Enable pinctrl for pwm-tiecap

Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
---
:100644 100644 e0bcc85... 646f8b4... M	drivers/pwm/pwm-tiecap.c
 drivers/pwm/pwm-tiecap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index e0bcc85..646f8b4 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -26,6 +26,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/pwm.h>
 #include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "pwm-tipwmss.h"
 
@@ -202,6 +203,11 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
 	struct clk *clk;
 	struct ecap_pwm_chip *pc;
 	u16 status;
+	struct pinctrl *pinctrl;
+
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc) {
-- 
1.7.0.4

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 306 bytes --]

On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
[...]
> +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +	if (IS_ERR(pinctrl))
> +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");

I think we already discussed this, but shouldn't this be a fatal error?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:29:44, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
> [...]
> > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > +	if (IS_ERR(pinctrl))
> > +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
> 
> I think we already discussed this, but shouldn't this be a fatal error?

I had checked relevant discussion and found this can be warning message.
This is because most boards don't have pinctrl implemented at this point,
or may never have.

https://lkml.org/lkml/2012/9/11/369

Any way I will change to
dev_warn(&pdev->dev, "unable to select pin group\n");
as in other drivers.

Thanks
Avinash

> 
> Thierry
> 

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 929 bytes --]

On Fri, Nov 23, 2012 at 10:34:02AM +0000, Philip, Avinash wrote:
> On Fri, Nov 23, 2012 at 02:29:44, Thierry Reding wrote:
> > On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
> > [...]
> > > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > > +	if (IS_ERR(pinctrl))
> > > +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
> > 
> > I think we already discussed this, but shouldn't this be a fatal error?
> 
> I had checked relevant discussion and found this can be warning message.
> This is because most boards don't have pinctrl implemented at this point,
> or may never have.
> 
> https://lkml.org/lkml/2012/9/11/369
> 
> Any way I will change to
> dev_warn(&pdev->dev, "unable to select pin group\n");
> as in other drivers.

But in case pinctrl isn't implemented, shouldn't the implementation of
devm_pinctrl_get_select_default() just be a no-op?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Philip, Avinash]

On Fri, Nov 23, 2012 at 16:21:10, Thierry Reding wrote:
> On Fri, Nov 23, 2012 at 10:34:02AM +0000, Philip, Avinash wrote:
> > On Fri, Nov 23, 2012 at 02:29:44, Thierry Reding wrote:
> > > On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
> > > [...]
> > > > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > > > +	if (IS_ERR(pinctrl))
> > > > +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
> > > 
> > > I think we already discussed this, but shouldn't this be a fatal error?
> > 
> > I had checked relevant discussion and found this can be warning message.
> > This is because most boards don't have pinctrl implemented at this point,
> > or may never have.
> > 
> > https://lkml.org/lkml/2012/9/11/369
> > 
> > Any way I will change to
> > dev_warn(&pdev->dev, "unable to select pin group\n");
> > as in other drivers.
> 
> But in case pinctrl isn't implemented, shouldn't the implementation of
> devm_pinctrl_get_select_default() just be a no-op?

So driver can still exist but pin mux functionality missing. Then warning
message will give hints.
I am also ok with error message & failure of driver with EPROBE_DEFER,
if you want?

Thanks
Avinash

> 
> Thierry
> 

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

On Fri, Nov 23, 2012 at 11:12:15AM +0000, Philip, Avinash wrote:
> On Fri, Nov 23, 2012 at 16:21:10, Thierry Reding wrote:
> > On Fri, Nov 23, 2012 at 10:34:02AM +0000, Philip, Avinash wrote:
> > > On Fri, Nov 23, 2012 at 02:29:44, Thierry Reding wrote:
> > > > On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
> > > > [...]
> > > > > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > > > > +	if (IS_ERR(pinctrl))
> > > > > +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
> > > > 
> > > > I think we already discussed this, but shouldn't this be a fatal error?
> > > 
> > > I had checked relevant discussion and found this can be warning message.
> > > This is because most boards don't have pinctrl implemented at this point,
> > > or may never have.
> > > 
> > > https://lkml.org/lkml/2012/9/11/369
> > > 
> > > Any way I will change to
> > > dev_warn(&pdev->dev, "unable to select pin group\n");
> > > as in other drivers.
> > 
> > But in case pinctrl isn't implemented, shouldn't the implementation of
> > devm_pinctrl_get_select_default() just be a no-op?
> 
> So driver can still exist but pin mux functionality missing. Then warning
> message will give hints.
> I am also ok with error message & failure of driver with EPROBE_DEFER,
> if you want?

Everybody seems to be doing it with a warning, so I guess that's fine
for now. I just find it strange that if you request the default pin
group to be selected when in fact the hardware doesn't support pinctrl
at all you shouldn't be getting an error either.

I'm adding LinusW on Cc, perhaps he can shed some light on it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Peter Korsgaard]

>>>>> "Thierry" == Thierry Reding <thierry.reding@avionic-design.de> writes:

Hi,

 Thierry> Everybody seems to be doing it with a warning, so I guess
 Thierry> that's fine for now. I just find it strange that if you
 Thierry> request the default pin group to be selected when in fact the
 Thierry> hardware doesn't support pinctrl at all you shouldn't be
 Thierry> getting an error either.

There's several different situations:

- Platform without pinctrl support
- Platform with pinctrl support but no pinmux specified in dt for device
  (E.G. pinmux setup in bootloader)
- Pinmux specified in dt
- Some kind of misconfiguration in dt

You could argue that devm_pinctrl_get_select_default() shouldn't return
an error for the first situation, but how should it be able to know the
difference between 2 and 4?

-- 
Bye, Peter Korsgaard

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1707 bytes --]

On Fri, Nov 23, 2012 at 01:48:51PM +0100, Peter Korsgaard wrote:
> >>>>> "Thierry" == Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> writes:
> 
> Hi,
> 
>  Thierry> Everybody seems to be doing it with a warning, so I guess
>  Thierry> that's fine for now. I just find it strange that if you
>  Thierry> request the default pin group to be selected when in fact the
>  Thierry> hardware doesn't support pinctrl at all you shouldn't be
>  Thierry> getting an error either.
> 
> There's several different situations:
> 
> - Platform without pinctrl support
> - Platform with pinctrl support but no pinmux specified in dt for device
>   (E.G. pinmux setup in bootloader)
> - Pinmux specified in dt
> - Some kind of misconfiguration in dt
> 
> You could argue that devm_pinctrl_get_select_default() shouldn't return
> an error for the first situation, but how should it be able to know the
> difference between 2 and 4?

In case where the platform supports pinctrl but no pinmux is specified
for the device it should just assume that no pinmuxing is needed. That
sounds like the most logical behaviour to me. In those cases pinctrl
could just assume that the default has already been selected and not
return an error.

But you can't reasonably expect to cope with misconfigured DT content.
Heck, there's no way for you to even know if it is misconfigured.

That said, I'm not sure how much of an issue this really is. Pinmuxing
is only used for functions local to a given chip, right? So if an SoC
supports pinctrl and a given peripheral needs pinmuxing then we can
reasonably assume that your second case can't happen, can't we?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Peter Korsgaard]

>>>>> "Thierry" == Thierry Reding <thierry.reding@avionic-design.de> writes:

Hi,

 >> There's several different situations:
 >> 
 >> - Platform without pinctrl support
 >> - Platform with pinctrl support but no pinmux specified in dt for device
 >> (E.G. pinmux setup in bootloader)
 >> - Pinmux specified in dt
 >> - Some kind of misconfiguration in dt
 >> 
 >> You could argue that devm_pinctrl_get_select_default() shouldn't return
 >> an error for the first situation, but how should it be able to know the
 >> difference between 2 and 4?

 Thierry> In case where the platform supports pinctrl but no pinmux is specified
 Thierry> for the device it should just assume that no pinmuxing is needed. That
 Thierry> sounds like the most logical behaviour to me. In those cases pinctrl
 Thierry> could just assume that the default has already been selected and not
 Thierry> return an error.

Agreed.

 Thierry> But you can't reasonably expect to cope with misconfigured DT
 Thierry> content.  Heck, there's no way for you to even know if it is
 Thierry> misconfigured.

With 'misconfigured DT' I just mean any kind of real error.


 Thierry> That said, I'm not sure how much of an issue this really
 Thierry> is. Pinmuxing is only used for functions local to a given
 Thierry> chip, right? So if an SoC supports pinctrl and a given
 Thierry> peripheral needs pinmuxing then we can reasonably assume that
 Thierry> your second case can't happen, can't we?

Well, peripherals only need pinmuxing if the bootloader didn't already
set it up in advance. We could naturally enforce people to add
"redundant" pinmux info to their .dts files, but I'm not sure I like
that idea.

-- 
Bye, Peter Korsgaard

Re: [PATCH v4 05/11] pwm: pwm-tiecap: pinctrl support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 193 bytes --]

On Wed, Nov 21, 2012 at 06:41:02PM +0530, Philip, Avinash wrote:
> Enable pinctrl for pwm-tiecap

This could probably be more verbose. Maybe explain some more what
exactly this means.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH v4 07/11] pwm: pwm-tiehrpwm: pinctrl support

[Philip, Avinash]

Enable pinctrl for pwm-tiehrpwm

Signed-off-by: Philip, Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
---
:100644 100644 34f9378... 23fd3c3... M	drivers/pwm/pwm-tiehrpwm.c
 drivers/pwm/pwm-tiehrpwm.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 34f9378..23fd3c3 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -26,6 +26,7 @@
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "pwm-tipwmss.h"
 
@@ -410,6 +411,11 @@ static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
 	struct clk *clk;
 	struct ehrpwm_pwm_chip *pc;
 	u16 status;
+	struct pinctrl *pinctrl;
+
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev, "failed to configure pins from driver\n");
 
 	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
 	if (!pc) {
-- 
1.7.0.4

Re: [PATCH v4 07/11] pwm: pwm-tiehrpwm: pinctrl support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

On Wed, Nov 21, 2012 at 06:41:04PM +0530, Philip, Avinash wrote:
[...]
> +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +	if (IS_ERR(pinctrl))
> +		dev_warn(&pdev->dev, "failed to configure pins from driver\n");

Same comment as for patch 5.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 00/11] Support for AM33xx PWM Subsystem

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2694 bytes --]

On Wed, Nov 21, 2012 at 06:40:57PM +0530, Philip, Avinash wrote:
> In AM33xx PWM sub modules like ECAP, EHRPWM & EQEP are  integrated to
> PWM subsystem. All these submodules shares the resources (clock) & has
> a clock gating register in PWM Subsystem. This patch series creates a
> parent PWM Subsystem driver to handle access synchronization of shared
> resources & clock gating from PWM Subsystem configuration space.
> Also Device tree nodes populated to support parent child relation
> between PWMSS, ECAP & EHRPWM submodules.
> In addition EHRPWM module requires explicit clock gating from control
> module & is handled by patch #2 & 8.
> 
> As suggested by  Thierry for handling clock gating for PWM submodules
> should handle with a global function. This requires config space
> handling done independent from driver and is done at parent driver.
> 
> So the parent<->child relation adopted to handle
> 1. pm runtime synchronization
> 2. PWM subsystem common config space clock gating for PWM submodules.
> 
> Patches supports
> - Driver support for parent child relation handled patch #1
> - Optional EHRPWM tb clock in patch #2
> - Parent child in HWMOD handled at patch #3
> - Device tree binding support handled in patch #4, 6 &8
> - pinctrl support in patch #5 & 7.
> - DT node populated in patch #9 ,10 & 11.
> 
> This patch series based on omap_dt/for_3.8/dts_part2 and tested
> on am335x-evm & am335x-evmsk.
> 
> It depends on [1]
> 
> 1. https://lkml.org/lkml/2012/11/21/70
> pwm: Device tree support for PWM polarity
> 
> Changes since v3:
> 	- Rebased on top of omap_dt/for_3.8/dts_part2
> 	- Add pwm backlight for am335xevm_sk
> 	- Moved tipwmss.h to pwm-tipwmss.h
> 
> Philip, Avinash (11):
>   PWMSS: Add PWM Subsystem driver for parent<->child relationship
>   ARM: am33xx: clk: Add optional clock for EHRPWM
>   ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM
>     subsystem
>   pwm: pwm-tiecap: Add device-tree binding support for APWM driver
>   pwm: pwm-tiecap: pinctrl support
>   pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
>   pwm: pwm-tiehrpwm: pinctrl support
>   pwm: pwm-tiehrpwm: Adding TBCLK gating support.
>   ARM: dts: AM33XX: Add PWMSS device tree nodes
>   ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
>   ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk

Hi,

So how do you want to get this merged? Should I take patches 1, 4, 5, 6,
7 and 8 while the rest go through some ARM tree? Can we still merge this
for 3.8? The patches don't seem to have build dependencies on each other
but how about runtime dependencies?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH v4 00/11] Support for AM33xx PWM Subsystem

[Philip, Avinash]

On Fri, Nov 23, 2012 at 02:46:16, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 06:40:57PM +0530, Philip, Avinash wrote:
> > In AM33xx PWM sub modules like ECAP, EHRPWM & EQEP are  integrated to
> > PWM subsystem. All these submodules shares the resources (clock) & has
> > a clock gating register in PWM Subsystem. This patch series creates a
> > parent PWM Subsystem driver to handle access synchronization of shared
> > resources & clock gating from PWM Subsystem configuration space.
> > Also Device tree nodes populated to support parent child relation
> > between PWMSS, ECAP & EHRPWM submodules.
> > In addition EHRPWM module requires explicit clock gating from control
> > module & is handled by patch #2 & 8.
> > 
> > As suggested by  Thierry for handling clock gating for PWM submodules
> > should handle with a global function. This requires config space
> > handling done independent from driver and is done at parent driver.
> > 
> > So the parent<->child relation adopted to handle
> > 1. pm runtime synchronization
> > 2. PWM subsystem common config space clock gating for PWM submodules.
> > 
> > Patches supports
> > - Driver support for parent child relation handled patch #1
> > - Optional EHRPWM tb clock in patch #2
> > - Parent child in HWMOD handled at patch #3
> > - Device tree binding support handled in patch #4, 6 &8
> > - pinctrl support in patch #5 & 7.
> > - DT node populated in patch #9 ,10 & 11.
> > 
> > This patch series based on omap_dt/for_3.8/dts_part2 and tested
> > on am335x-evm & am335x-evmsk.
> > 
> > It depends on [1]
> > 
> > 1. https://lkml.org/lkml/2012/11/21/70
> > pwm: Device tree support for PWM polarity
> > 
> > Changes since v3:
> > 	- Rebased on top of omap_dt/for_3.8/dts_part2
> > 	- Add pwm backlight for am335xevm_sk
> > 	- Moved tipwmss.h to pwm-tipwmss.h
> > 
> > Philip, Avinash (11):
> >   PWMSS: Add PWM Subsystem driver for parent<->child relationship
> >   ARM: am33xx: clk: Add optional clock for EHRPWM
> >   ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM
> >     subsystem
> >   pwm: pwm-tiecap: Add device-tree binding support for APWM driver
> >   pwm: pwm-tiecap: pinctrl support
> >   pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
> >   pwm: pwm-tiehrpwm: pinctrl support
> >   pwm: pwm-tiehrpwm: Adding TBCLK gating support.
> >   ARM: dts: AM33XX: Add PWMSS device tree nodes
> >   ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
> >   ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evmsk
> 
> Hi,
> 
> So how do you want to get this merged? Should I take patches 1, 4, 5, 6,
> 7 and 8 while the rest go through some ARM tree? Can we still merge this
> for 3.8?

These patches can go on, as DT node is not populated. Also without patch #3
PWM Subsystem driver won't come and hence the platform devices for ECAP 
& EHRPWM. So there is no issue in getting this merged in PWM subsystem.

> The patches don't seem to have build dependencies on each other
> but how about runtime dependencies?

Patch #3 required for PWM subsystem driver to populate PWM subsystem HWMOD
Entries and DT entries. 
Patch #2 required for EHRPWM to have TBCLK handling. This patch required for
EHRPWM driver functionality. But currently DT node is disabled for EHRPWM.

So the dependency on the entire series is patch #3, for backlight through PWM
ecap.

I will follow up with OMAP subsystem community for getting reviews/ack

Thanks
Avinash

> 
> Thierry
>