[PATCH v2 0/3] Platform support for EHRPWM & ECAP devices in DAVINCI.

[PATCH v2 0/3] Platform support for EHRPWM & ECAP devices in DAVINCI.

[Philip Avinash]

Add platform support for EHRPWM and ECAP by providing clock nodes and
device tree nodes.
This series depends on [1] and [2] and is available for testing at [3].
Tested for backlight support in da850 EVM with EHRPWM PWM device.

[1] http://gitorious.org/linux-davinci/linux-davinci/trees/davinci-for-v3.9/dt-2
[2] https://gitorious.org/linux-pwm/linux-pwm/trees/for-next
[3] https://github.com/avinashphilip/am335x_linux/tree/davinci-for-v3.9_soc_pwm

Note:
DT support for EHRPWM backlight has not been added in da850-evm.dts as there is
conflicting pin-mux requirement with SPI flash.

Philip Avinash (3):
  ARM: davinci: clk framework support for enable/disable functionality
  arm: davinci: clock node support for ECAP & EHRPWM
  ARM: davinci: da850: add EHRPWM & ECAP DT node

 arch/arm/boot/dts/da850.dtsi               |   30 ++++++++++++++++++
 arch/arm/mach-davinci/clock.c              |    4 +++
 arch/arm/mach-davinci/clock.h              |    2 ++
 arch/arm/mach-davinci/da850.c              |   46 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/da8xx-dt.c           |    5 +++
 arch/arm/mach-davinci/include/mach/da8xx.h |    1 +
 6 files changed, 88 insertions(+)

-- 
1.7.9.5

[PATCH v2 1/3] ARM: davinci: clk framework support for enable/disable functionality

[Philip Avinash]

DAVINCI clock framework currently not supporting clock enable/disable
functionality on clock nodes. In DAVINCI platform EHRPWM module requires
support for clock enable/disable for TBCLK support. Hence this patch
adds support for enabling/disabling clocks depends on the availability
of the functionality.

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
Changes since v1:
	- Add support for clock enable/disable functionality.

:100644 100644 d458558... aa89e5e... M	arch/arm/mach-davinci/clock.c
:100644 100644 8694b39... 1e4e836... M	arch/arm/mach-davinci/clock.h
 arch/arm/mach-davinci/clock.c |    4 ++++
 arch/arm/mach-davinci/clock.h |    2 ++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
index d458558..aa89e5e 100644
--- a/arch/arm/mach-davinci/clock.c
+++ b/arch/arm/mach-davinci/clock.c
@@ -35,6 +35,8 @@ static void __clk_enable(struct clk *clk)
 {
 	if (clk->parent)
 		__clk_enable(clk->parent);
+	if (clk->clk_enable)
+		clk->clk_enable(clk);
 	if (clk->usecount++ == 0 && (clk->flags & CLK_PSC))
 		davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
 				true, clk->flags);
@@ -44,6 +46,8 @@ static void __clk_disable(struct clk *clk)
 {
 	if (WARN_ON(clk->usecount == 0))
 		return;
+	if (clk->clk_disable)
+		clk->clk_disable(clk);
 	if (--clk->usecount == 0 && !(clk->flags & CLK_PLL) &&
 	    (clk->flags & CLK_PSC))
 		davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
index 8694b39..1e4e836 100644
--- a/arch/arm/mach-davinci/clock.h
+++ b/arch/arm/mach-davinci/clock.h
@@ -104,6 +104,8 @@ struct clk {
 	int (*set_rate) (struct clk *clk, unsigned long rate);
 	int (*round_rate) (struct clk *clk, unsigned long rate);
 	int (*reset) (struct clk *clk, bool reset);
+	void (*clk_enable) (struct clk *clk);
+	void (*clk_disable) (struct clk *clk);
 };
 
 /* Clock flags: SoC-specific flags start at BIT(16) */
-- 
1.7.9.5

[PATCH v2 2/3] arm: davinci: clock node support for ECAP & EHRPWM

[Philip Avinash]

Add clock node support for ECAP and EHRPWM modules.
Also adds TBCLK for EHRWPM TBCLK to comply with pwm-tiehrpwm
driver.

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
Changes Since v1:
	- TBCLK make it as actual clock with enable/disable feature.

:100644 100644 0c4a26d... dbed75c... M	arch/arm/mach-davinci/da850.c
:100644 100644 de439b7... be77ce2... M	arch/arm/mach-davinci/include/mach/da8xx.h
 arch/arm/mach-davinci/da850.c              |   46 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |    1 +
 2 files changed, 47 insertions(+)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 0c4a26d..dbed75c 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -383,6 +383,49 @@ static struct clk dsp_clk = {
 	.flags		= PSC_LRST | PSC_FORCE,
 };
 
+static struct clk ehrpwm_clk = {
+	.name		= "ehrpwm",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC1_PWM,
+	.gpsc		= 1,
+	.flags		= DA850_CLK_ASYNC3,
+};
+
+#define DA8XX_EHRPWM_TBCLKSYNC	BIT(12)
+
+void tblck_enable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
+	val |= DA8XX_EHRPWM_TBCLKSYNC;
+	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
+}
+
+void tblck_disable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
+	val &= ~DA8XX_EHRPWM_TBCLKSYNC;
+	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
+}
+
+static struct clk ehrpwm_tbclk = {
+	.name		= "ehrpwm_tbclk",
+	.parent		= &ehrpwm_clk,
+	.clk_enable	= tblck_enable,
+	.clk_disable	= tblck_disable,
+};
+
+static struct clk ecap_clk = {
+	.name		= "ecap",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC1_ECAP,
+	.gpsc		= 1,
+	.flags		= DA850_CLK_ASYNC3,
+};
+
 static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"ref",		&ref_clk),
 	CLK(NULL,		"pll0",		&pll0_clk),
@@ -430,6 +473,9 @@ static struct clk_lookup da850_clks[] = {
 	CLK("vpif",		NULL,		&vpif_clk),
 	CLK("ahci",		NULL,		&sata_clk),
 	CLK("davinci-rproc.0",	NULL,		&dsp_clk),
+	CLK("ehrpwm",		"fck",		&ehrpwm_clk),
+	CLK("ehrpwm",		"tbclk",	&ehrpwm_tbclk),
+	CLK("ecap",		"fck",		&ecap_clk),
 	CLK(NULL,		NULL,		NULL),
 };
 
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index de439b7..be77ce2 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -55,6 +55,7 @@ extern unsigned int da850_max_speed;
 #define DA8XX_SYSCFG0_VIRT(x)	(da8xx_syscfg0_base + (x))
 #define DA8XX_JTAG_ID_REG	0x18
 #define DA8XX_CFGCHIP0_REG	0x17c
+#define DA8XX_CFGCHIP1_REG	0x180
 #define DA8XX_CFGCHIP2_REG	0x184
 #define DA8XX_CFGCHIP3_REG	0x188
 
-- 
1.7.9.5

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Philip Avinash]

Add da850 EHRPWM & ECAP DT node.
Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
clock.

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
Changes since v1:
	- Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
	  same with am33xx platform and da850 has no platform specific
	  dependency.

:100644 100644 3ec1bda... 62fd2d4... M	arch/arm/boot/dts/da850.dtsi
:100644 100644 6b7a0a2... 89ee974... M	arch/arm/mach-davinci/da8xx-dt.c
 arch/arm/boot/dts/da850.dtsi     |   30 ++++++++++++++++++++++++++++++
 arch/arm/mach-davinci/da8xx-dt.c |    5 +++++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 3ec1bda..62fd2d4 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -107,6 +107,36 @@
 			reg = <0x21000 0x1000>;
 			status = "disabled";
 		};
+		ehrpwm0: ehrpwm at 01f00000 {
+			compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
+			#pwm-cells = <3>;
+			reg = <0x300000 0x2000>;
+			status = "disabled";
+		};
+		ehrpwm1: ehrpwm at 01f02000 {
+			compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
+			#pwm-cells = <3>;
+			reg = <0x302000 0x2000>;
+			status = "disabled";
+		};
+		ecap0: ecap at 01f06000 {
+			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
+			#pwm-cells = <3>;
+			reg = <0x306000 0x80>;
+			status = "disabled";
+		};
+		ecap1: ecap at 01f07000 {
+			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
+			#pwm-cells = <3>;
+			reg = <0x307000 0x80>;
+			status = "disabled";
+		};
+		ecap2: ecap at 01f08000 {
+			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
+			#pwm-cells = <3>;
+			reg = <0x308000 0x80>;
+			status = "disabled";
+		};
 	};
 	nand_cs3 at 62000000 {
 		compatible = "ti,davinci-nand";
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 6b7a0a2..89ee974 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -40,6 +40,11 @@ static void __init da8xx_init_irq(void)
 struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
 	OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "watchdog", NULL),
+	OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm", NULL),
+	OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm", NULL),
+	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap", NULL),
+	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL),
+	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
 	{}
 };
 
-- 
1.7.9.5

[PATCH v2 1/3] ARM: davinci: clk framework support for enable/disable functionality

[Sekhar Nori]

On 3/20/2013 12:11 PM, Philip Avinash wrote:
> DAVINCI clock framework currently not supporting clock enable/disable
> functionality on clock nodes. In DAVINCI platform EHRPWM module requires

Wrong. clock enable/disable is supported in the DaVinci clock
implementation, but just for PSC clocks.

> support for clock enable/disable for TBCLK support. Hence this patch
> adds support for enabling/disabling clocks depends on the availability
> of the functionality.
> 
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> ---
> Changes since v1:
> 	- Add support for clock enable/disable functionality.
> 
> :100644 100644 d458558... aa89e5e... M	arch/arm/mach-davinci/clock.c
> :100644 100644 8694b39... 1e4e836... M	arch/arm/mach-davinci/clock.h
>  arch/arm/mach-davinci/clock.c |    4 ++++
>  arch/arm/mach-davinci/clock.h |    2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index d458558..aa89e5e 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -35,6 +35,8 @@ static void __clk_enable(struct clk *clk)
>  {
>  	if (clk->parent)
>  		__clk_enable(clk->parent);
> +	if (clk->clk_enable)
> +		clk->clk_enable(clk);

Why ignore usecount in this case?

>  	if (clk->usecount++ == 0 && (clk->flags & CLK_PSC))

if clk->enable is available, no need to check for 'clk->flags & CLK_PSC'

Thanks,
Sekhar

[PATCH v2 2/3] arm: davinci: clock node support for ECAP & EHRPWM

[Sekhar Nori]

On 3/20/2013 12:11 PM, Philip Avinash wrote:
> Add clock node support for ECAP and EHRPWM modules.
> Also adds TBCLK for EHRWPM TBCLK to comply with pwm-tiehrpwm
> driver.
> 
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> ---
> Changes Since v1:
> 	- TBCLK make it as actual clock with enable/disable feature.
> 
> :100644 100644 0c4a26d... dbed75c... M	arch/arm/mach-davinci/da850.c
> :100644 100644 de439b7... be77ce2... M	arch/arm/mach-davinci/include/mach/da8xx.h
>  arch/arm/mach-davinci/da850.c              |   46 ++++++++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/da8xx.h |    1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 0c4a26d..dbed75c 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -383,6 +383,49 @@ static struct clk dsp_clk = {
>  	.flags		= PSC_LRST | PSC_FORCE,
>  };
>  
> +static struct clk ehrpwm_clk = {
> +	.name		= "ehrpwm",
> +	.parent		= &pll0_sysclk2,
> +	.lpsc		= DA8XX_LPSC1_PWM,
> +	.gpsc		= 1,
> +	.flags		= DA850_CLK_ASYNC3,
> +};
> +
> +#define DA8XX_EHRPWM_TBCLKSYNC	BIT(12)
> +
> +void tblck_enable(struct clk *clk)

This should be static. Also, since tbclk is associated with ehrpwm,
please call the function ehrpwm_tbclk_enable().

> +{
> +	u32 val;
> +
> +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
> +	val |= DA8XX_EHRPWM_TBCLKSYNC;
> +	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
> +}
> +
> +void tblck_disable(struct clk *clk)

Same comment as above applies.

Thanks,
Sekhar

[PATCH v2 1/3] ARM: davinci: clk framework support for enable/disable functionality

[Philip, Avinash]

On Wed, Mar 20, 2013 at 16:49:55, Nori, Sekhar wrote:
> On 3/20/2013 12:11 PM, Philip Avinash wrote:
> > DAVINCI clock framework currently not supporting clock enable/disable
> > functionality on clock nodes. In DAVINCI platform EHRPWM module requires
> 
> Wrong. clock enable/disable is supported in the DaVinci clock
> implementation, but just for PSC clocks.
> 
> > support for clock enable/disable for TBCLK support. Hence this patch
> > adds support for enabling/disabling clocks depends on the availability
> > of the functionality.
> > 
> > Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> > ---
> > Changes since v1:
> > 	- Add support for clock enable/disable functionality.
> > 
> > :100644 100644 d458558... aa89e5e... M	arch/arm/mach-davinci/clock.c
> > :100644 100644 8694b39... 1e4e836... M	arch/arm/mach-davinci/clock.h
> >  arch/arm/mach-davinci/clock.c |    4 ++++
> >  arch/arm/mach-davinci/clock.h |    2 ++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> > index d458558..aa89e5e 100644
> > --- a/arch/arm/mach-davinci/clock.c
> > +++ b/arch/arm/mach-davinci/clock.c
> > @@ -35,6 +35,8 @@ static void __clk_enable(struct clk *clk)
> >  {
> >  	if (clk->parent)
> >  		__clk_enable(clk->parent);
> > +	if (clk->clk_enable)
> > +		clk->clk_enable(clk);
> 
> Why ignore usecount in this case?

Will check usecount & do enable. 

> 
> >  	if (clk->usecount++ == 0 && (clk->flags & CLK_PSC))
> 
> if clk->enable is available, no need to check for 'clk->flags & CLK_PSC'

I will correct it.

Thanks
Avinash

> 
> Thanks,
> Sekhar
> 

[PATCH v2 2/3] arm: davinci: clock node support for ECAP & EHRPWM

[Philip, Avinash]

On Wed, Mar 20, 2013 at 16:54:34, Nori, Sekhar wrote:
> On 3/20/2013 12:11 PM, Philip Avinash wrote:
> > Add clock node support for ECAP and EHRPWM modules.
> > Also adds TBCLK for EHRWPM TBCLK to comply with pwm-tiehrpwm
> > driver.
> > 
> > Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> > ---
> > Changes Since v1:
> > 	- TBCLK make it as actual clock with enable/disable feature.
> > 
> > :100644 100644 0c4a26d... dbed75c... M	arch/arm/mach-davinci/da850.c
> > :100644 100644 de439b7... be77ce2... M	arch/arm/mach-davinci/include/mach/da8xx.h
> >  arch/arm/mach-davinci/da850.c              |   46 ++++++++++++++++++++++++++++
> >  arch/arm/mach-davinci/include/mach/da8xx.h |    1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> > index 0c4a26d..dbed75c 100644
> > --- a/arch/arm/mach-davinci/da850.c
> > +++ b/arch/arm/mach-davinci/da850.c
> > @@ -383,6 +383,49 @@ static struct clk dsp_clk = {
> >  	.flags		= PSC_LRST | PSC_FORCE,
> >  };
> >  
> > +static struct clk ehrpwm_clk = {
> > +	.name		= "ehrpwm",
> > +	.parent		= &pll0_sysclk2,
> > +	.lpsc		= DA8XX_LPSC1_PWM,
> > +	.gpsc		= 1,
> > +	.flags		= DA850_CLK_ASYNC3,
> > +};
> > +
> > +#define DA8XX_EHRPWM_TBCLKSYNC	BIT(12)
> > +
> > +void tblck_enable(struct clk *clk)
> 
> This should be static. Also, since tbclk is associated with ehrpwm,
> please call the function ehrpwm_tbclk_enable().

Ok I will correct it.

Thanks
Avinash
> 
> > +{
> > +	u32 val;
> > +
> > +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
> > +	val |= DA8XX_EHRPWM_TBCLKSYNC;
> > +	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP1_REG));
> > +}
> > +
> > +void tblck_disable(struct clk *clk)
> 
> Same comment as above applies.
> 
> Thanks,
> Sekhar
> 

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Sekhar Nori]

On 3/20/2013 12:11 PM, Philip Avinash wrote:
> Add da850 EHRPWM & ECAP DT node.
> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
> clock.
> 
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> ---
> Changes since v1:
> 	- Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
> 	  same with am33xx platform and da850 has no platform specific
> 	  dependency.

Which is fine, but I think the binding documentation still needs to be
updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
always a good idea to CC folks who reviewed your patch last time
around). Also, please Cc the DT folks and devicetree-discuss list too
for their opinion.

Thanks,
Sekhar

> 
> :100644 100644 3ec1bda... 62fd2d4... M	arch/arm/boot/dts/da850.dtsi
> :100644 100644 6b7a0a2... 89ee974... M	arch/arm/mach-davinci/da8xx-dt.c
>  arch/arm/boot/dts/da850.dtsi     |   30 ++++++++++++++++++++++++++++++
>  arch/arm/mach-davinci/da8xx-dt.c |    5 +++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 3ec1bda..62fd2d4 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -107,6 +107,36 @@
>  			reg = <0x21000 0x1000>;
>  			status = "disabled";
>  		};
> +		ehrpwm0: ehrpwm at 01f00000 {
> +			compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
> +			#pwm-cells = <3>;
> +			reg = <0x300000 0x2000>;
> +			status = "disabled";
> +		};
> +		ehrpwm1: ehrpwm at 01f02000 {
> +			compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
> +			#pwm-cells = <3>;
> +			reg = <0x302000 0x2000>;
> +			status = "disabled";
> +		};
> +		ecap0: ecap at 01f06000 {
> +			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
> +			#pwm-cells = <3>;
> +			reg = <0x306000 0x80>;
> +			status = "disabled";
> +		};
> +		ecap1: ecap at 01f07000 {
> +			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
> +			#pwm-cells = <3>;
> +			reg = <0x307000 0x80>;
> +			status = "disabled";
> +		};
> +		ecap2: ecap at 01f08000 {
> +			compatible = "ti,da850-ecap", "ti,am33xx-ecap";
> +			#pwm-cells = <3>;
> +			reg = <0x308000 0x80>;
> +			status = "disabled";
> +		};
>  	};
>  	nand_cs3 at 62000000 {
>  		compatible = "ti,davinci-nand";
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index 6b7a0a2..89ee974 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -40,6 +40,11 @@ static void __init da8xx_init_irq(void)
>  struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>  	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
>  	OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "watchdog", NULL),
> +	OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm", NULL),
> +	OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm", NULL),
> +	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap", NULL),
> +	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL),
> +	OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
>  	{}
>  };
>  
> 

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Peter Korsgaard]

>>>>> "Sekhar" == Sekhar Nori <nsekhar@ti.com> writes:

 Sekhar> On 3/20/2013 12:11 PM, Philip Avinash wrote:
 >> Add da850 EHRPWM & ECAP DT node.
 >> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
 >> clock.
 >> 
 >> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
 >> ---
 >> Changes since v1:
 >> - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
 >> same with am33xx platform and da850 has no platform specific
 >> dependency.

 Sekhar> Which is fine, but I think the binding documentation still needs to be
 Sekhar> updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
 Sekhar> always a good idea to CC folks who reviewed your patch last time
 Sekhar> around). Also, please Cc the DT folks and devicetree-discuss list too
 Sekhar> for their opinion.

Yes, thanks for CC'ing me. I also think da850-* should be
documented. See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for
an similar (old) example.

-- 
Bye, Peter Korsgaard

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Philip, Avinash]

On Wed, Mar 20, 2013 at 18:17:59, Peter Korsgaard wrote:
> >>>>> "Sekhar" == Sekhar Nori <nsekhar@ti.com> writes:
> 
>  Sekhar> On 3/20/2013 12:11 PM, Philip Avinash wrote:
>  >> Add da850 EHRPWM & ECAP DT node.
>  >> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
>  >> clock.
>  >> 
>  >> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>  >> ---
>  >> Changes since v1:
>  >> - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
>  >> same with am33xx platform and da850 has no platform specific
>  >> dependency.
> 
>  Sekhar> Which is fine, but I think the binding documentation still needs to be
>  Sekhar> updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
>  Sekhar> always a good idea to CC folks who reviewed your patch last time
>  Sekhar> around). Also, please Cc the DT folks and devicetree-discuss list too
>  Sekhar> for their opinion.
> 
> Yes, thanks for CC'ing me. I also think da850-* should be
> documented. See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for
> an similar (old) example.

Peter,

In this binding file also, only initial compatible field is updated. Later on many
compatible were added in driver. But not update back to binding doc.

<Documentation/devicetree/bindings/gpio/8xxx_gpio.txt>
---
	followed by "fsl,mpc8349-gpio" for 83xx, "fsl,mpc8572-gpio" for 85xx and
	"fsl,mpc8610-gpio" for 86xx.
---
<drivers/gpio/gpio-mpc8xxx.c>
---
static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
        { .compatible = "fsl,mpc8349-gpio", },
        { .compatible = "fsl,mpc8572-gpio", },
        { .compatible = "fsl,mpc8610-gpio", },
        { .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
        { .compatible = "fsl,pq3-gpio",     },
        { .compatible = "fsl,qoriq-gpio",   },
        {}
};
---

Grant/Rob,
With respect peters explanation (below), what is your opinion on adding information to 
binding doc for da850 support?

Peter --> if the hardware block is identical the dts should simply list:
Peter --> compatible = "ti,da850-ecap", "ti,am33xx-ecap"
Peter --> And the driver only bind to ti,am33xx-ecap (unless there ever needs to
Peter --> be a da850 specific workaround.

Or
Should I update both binding doc & the driver to use new compatible option "ti,da830-ecap"
(as da830 platform has initial IP support)?
Even with this, platforms da830, da850 and am33xx can reuse "ti,da830-ecap" compatible field.

Thanks
Avinash

> 
> -- 
> Bye, Peter Korsgaard
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Sekhar Nori]

On 3/21/2013 1:31 PM, Philip, Avinash wrote:
> On Wed, Mar 20, 2013 at 18:17:59, Peter Korsgaard wrote:
>>>>>>> "Sekhar" == Sekhar Nori <nsekhar@ti.com> writes:
>>
>>  Sekhar> On 3/20/2013 12:11 PM, Philip Avinash wrote:
>>  >> Add da850 EHRPWM & ECAP DT node.
>>  >> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
>>  >> clock.
>>  >> 
>>  >> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>  >> ---
>>  >> Changes since v1:
>>  >> - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
>>  >> same with am33xx platform and da850 has no platform specific
>>  >> dependency.
>>
>>  Sekhar> Which is fine, but I think the binding documentation still needs to be
>>  Sekhar> updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
>>  Sekhar> always a good idea to CC folks who reviewed your patch last time
>>  Sekhar> around). Also, please Cc the DT folks and devicetree-discuss list too
>>  Sekhar> for their opinion.
>>
>> Yes, thanks for CC'ing me. I also think da850-* should be
>> documented. See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for
>> an similar (old) example.
> 
> Peter,
> 
> In this binding file also, only initial compatible field is updated. Later on many
> compatible were added in driver. But not update back to binding doc.

Probably someone forgot to keep updating the binding doc after a point.

> 
> <Documentation/devicetree/bindings/gpio/8xxx_gpio.txt>
> ---
> 	followed by "fsl,mpc8349-gpio" for 83xx, "fsl,mpc8572-gpio" for 85xx and
> 	"fsl,mpc8610-gpio" for 86xx.
> ---
> <drivers/gpio/gpio-mpc8xxx.c>
> ---
> static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
>         { .compatible = "fsl,mpc8349-gpio", },
>         { .compatible = "fsl,mpc8572-gpio", },
>         { .compatible = "fsl,mpc8610-gpio", },
>         { .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
>         { .compatible = "fsl,pq3-gpio",     },
>         { .compatible = "fsl,qoriq-gpio",   },
>         {}
> };
> ---
> 
> Grant/Rob,
> With respect peters explanation (below), what is your opinion on adding information to 
> binding doc for da850 support?
> 
> Peter --> if the hardware block is identical the dts should simply list:
> Peter --> compatible = "ti,da850-ecap", "ti,am33xx-ecap"
> Peter --> And the driver only bind to ti,am33xx-ecap (unless there ever needs to
> Peter --> be a da850 specific workaround.
> 
> Or
> Should I update both binding doc & the driver to use new compatible option "ti,da830-ecap"
> (as da830 platform has initial IP support)?
> Even with this, platforms da830, da850 and am33xx can reuse "ti,da830-ecap" compatible field.

To me updating the driver to keep adding a .compatible even when not
using it elsewhere in code is not required. Adding the new binding in
.dts file is must since you may need to do something specific to da830
at a later time (and the .dtb should be considered ROM'ed so you wont be
able to change it then). Adding documentation for the binding is also
required to help anyone wanting to know more about the binding after
reading the .dts file.

Thanks,
Sekhar

[PATCH v2 3/3] ARM: davinci: da850: add EHRPWM & ECAP DT node

[Philip, Avinash]

On Fri, Mar 22, 2013 at 11:23:32, Nori, Sekhar wrote:
> On 3/21/2013 1:31 PM, Philip, Avinash wrote:
> > On Wed, Mar 20, 2013 at 18:17:59, Peter Korsgaard wrote:
> >>>>>>> "Sekhar" == Sekhar Nori <nsekhar@ti.com> writes:
> >>
> >>  Sekhar> On 3/20/2013 12:11 PM, Philip Avinash wrote:
> >>  >> Add da850 EHRPWM & ECAP DT node.
> >>  >> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
> >>  >> clock.
> >>  >> 
> >>  >> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> >>  >> ---
> >>  >> Changes since v1:
> >>  >> - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
> >>  >> same with am33xx platform and da850 has no platform specific
> >>  >> dependency.
> >>
> >>  Sekhar> Which is fine, but I think the binding documentation still needs to be
> >>  Sekhar> updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
> >>  Sekhar> always a good idea to CC folks who reviewed your patch last time
> >>  Sekhar> around). Also, please Cc the DT folks and devicetree-discuss list too
> >>  Sekhar> for their opinion.
> >>
> >> Yes, thanks for CC'ing me. I also think da850-* should be
> >> documented. See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for
> >> an similar (old) example.
> > 
> > Peter,
> > 
> > In this binding file also, only initial compatible field is updated. Later on many
> > compatible were added in driver. But not update back to binding doc.
> 
> Probably someone forgot to keep updating the binding doc after a point.
> 
> > 
> > <Documentation/devicetree/bindings/gpio/8xxx_gpio.txt>
> > ---
> > 	followed by "fsl,mpc8349-gpio" for 83xx, "fsl,mpc8572-gpio" for 85xx and
> > 	"fsl,mpc8610-gpio" for 86xx.
> > ---
> > <drivers/gpio/gpio-mpc8xxx.c>
> > ---
> > static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
> >         { .compatible = "fsl,mpc8349-gpio", },
> >         { .compatible = "fsl,mpc8572-gpio", },
> >         { .compatible = "fsl,mpc8610-gpio", },
> >         { .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
> >         { .compatible = "fsl,pq3-gpio",     },
> >         { .compatible = "fsl,qoriq-gpio",   },
> >         {}
> > };
> > ---
> > 
> > Grant/Rob,
> > With respect peters explanation (below), what is your opinion on adding information to 
> > binding doc for da850 support?
> > 
> > Peter --> if the hardware block is identical the dts should simply list:
> > Peter --> compatible = "ti,da850-ecap", "ti,am33xx-ecap"
> > Peter --> And the driver only bind to ti,am33xx-ecap (unless there ever needs to
> > Peter --> be a da850 specific workaround.
> > 
> > Or
> > Should I update both binding doc & the driver to use new compatible option "ti,da830-ecap"
> > (as da830 platform has initial IP support)?
> > Even with this, platforms da830, da850 and am33xx can reuse "ti,da830-ecap" compatible field.
> 
> To me updating the driver to keep adding a .compatible even when not
> using it elsewhere in code is not required. Adding the new binding in
> .dts file is must since you may need to do something specific to da830
> at a later time (and the .dtb should be considered ROM'ed so you wont be
> able to change it then).

Thanks for the explanation.
I will add "ti,da850-ecap" for da850.dtsi along with "ti,am33xx-ecap" as
compatible fields.

> Adding documentation for the binding is also
> required to help anyone wanting to know more about the binding after
> reading the .dts file.

I will adding documentation part for "ti,da850-ecap".

Thanks
Avinash

> 
> Thanks,
> Sekhar
>