diff mbox

[1/3] pwm: davinci: Add Kconfig support for ECAP & EHRPWM devices

Message ID 1363257158-11615-2-git-send-email-avinashphilip@ti.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

avinash philip March 14, 2013, 10:32 a.m. UTC
Add EHRPWM and ECAP support build support for DAVINCI_DA850 platforms.

Also, since DAVINCI platforms doesn't support TI-PWM-Subsystem module,
remove the select option for CONFIG_PWM_TIPWMSS.

Also, update CONFIG_PWM_TIPWMSS compiler directive appropriately in
pwm-tipwmss.h to fix the below compiler error upon removal of
CONFIG_PWM_TIPWMSS for Davinci platforms.

	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_probe':
	drivers/pwm/pwm-tiecap.c:263:4: error: 'PWMSS_ECAPCLK_EN' undeclared
	(first use in this function)
	drivers/pwm/pwm-tiecap.c:263:4: note: each undeclared identifier
	is reported only once for each function it appears in
	drivers/pwm/pwm-tiecap.c:264:17: error: 'PWMSS_ECAPCLK_EN_ACK'
	undeclared (first use in this function)
	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_remove':
	drivers/pwm/pwm-tiecap.c:291:49: error: 'PWMSS_ECAPCLK_STOP_REQ'
	undeclared (first use in this function)
	make[2]: *** [drivers/pwm/pwm-tiecap.o] Error 1
	make[1]: *** [drivers/pwm] Error 2
	make: *** [drivers] Error 2

Signed-off-by: Philip Avinash <avinashphilip@ti.com>
---
:100644 100644 0e0bfa0... 897b53c... M	drivers/pwm/Kconfig
:100644 100644 11f76a1... 10ad804... M	drivers/pwm/pwm-tipwmss.h
 drivers/pwm/Kconfig       |    8 +++-----
 drivers/pwm/pwm-tipwmss.h |    2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Sekhar Nori March 14, 2013, 11:39 a.m. UTC | #1
On 3/14/2013 4:02 PM, Philip Avinash wrote:
> Add EHRPWM and ECAP support build support for DAVINCI_DA850 platforms.
> 
> Also, since DAVINCI platforms doesn't support TI-PWM-Subsystem module,
> remove the select option for CONFIG_PWM_TIPWMSS.
> 
> Also, update CONFIG_PWM_TIPWMSS compiler directive appropriately in
> pwm-tipwmss.h to fix the below compiler error upon removal of
> CONFIG_PWM_TIPWMSS for Davinci platforms.
> 
> 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_probe':
> 	drivers/pwm/pwm-tiecap.c:263:4: error: 'PWMSS_ECAPCLK_EN' undeclared
> 	(first use in this function)
> 	drivers/pwm/pwm-tiecap.c:263:4: note: each undeclared identifier
> 	is reported only once for each function it appears in
> 	drivers/pwm/pwm-tiecap.c:264:17: error: 'PWMSS_ECAPCLK_EN_ACK'
> 	undeclared (first use in this function)
> 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_remove':
> 	drivers/pwm/pwm-tiecap.c:291:49: error: 'PWMSS_ECAPCLK_STOP_REQ'
> 	undeclared (first use in this function)
> 	make[2]: *** [drivers/pwm/pwm-tiecap.o] Error 1
> 	make[1]: *** [drivers/pwm] Error 2
> 	make: *** [drivers] Error 2
> 
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>

>  config  PWM_TIECAP
>  	tristate "ECAP PWM support"
> -	depends on SOC_AM33XX
> -	select PWM_TIPWMSS
> +	depends on SOC_AM33XX || ARCH_DAVINCI_DA850

Having such narrow dependencies is wrong. The same device is present on
DaVinci DA830 too. A depends on should not be required at all since the
driver should build on all architectures. But I have seen resistance to
doing that since users don't like to see configuration options totally
irrelevant for the architecture they are building for. So may be take a
middle path and do 'depends on ARCH_ARM'?

Thanks,
Sekhar
avinash philip March 14, 2013, 12:54 p.m. UTC | #2
On Thu, Mar 14, 2013 at 17:09:04, Nori, Sekhar wrote:
> On 3/14/2013 4:02 PM, Philip Avinash wrote:
> > Add EHRPWM and ECAP support build support for DAVINCI_DA850 platforms.
> > 
> > Also, since DAVINCI platforms doesn't support TI-PWM-Subsystem module,
> > remove the select option for CONFIG_PWM_TIPWMSS.
> > 
> > Also, update CONFIG_PWM_TIPWMSS compiler directive appropriately in
> > pwm-tipwmss.h to fix the below compiler error upon removal of
> > CONFIG_PWM_TIPWMSS for Davinci platforms.
> > 
> > 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_probe':
> > 	drivers/pwm/pwm-tiecap.c:263:4: error: 'PWMSS_ECAPCLK_EN' undeclared
> > 	(first use in this function)
> > 	drivers/pwm/pwm-tiecap.c:263:4: note: each undeclared identifier
> > 	is reported only once for each function it appears in
> > 	drivers/pwm/pwm-tiecap.c:264:17: error: 'PWMSS_ECAPCLK_EN_ACK'
> > 	undeclared (first use in this function)
> > 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_remove':
> > 	drivers/pwm/pwm-tiecap.c:291:49: error: 'PWMSS_ECAPCLK_STOP_REQ'
> > 	undeclared (first use in this function)
> > 	make[2]: *** [drivers/pwm/pwm-tiecap.o] Error 1
> > 	make[1]: *** [drivers/pwm] Error 2
> > 	make: *** [drivers] Error 2
> > 
> > Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> 
> >  config  PWM_TIECAP
> >  	tristate "ECAP PWM support"
> > -	depends on SOC_AM33XX
> > -	select PWM_TIPWMSS
> > +	depends on SOC_AM33XX || ARCH_DAVINCI_DA850
> 
> Having such narrow dependencies is wrong. The same device is present on
> DaVinci DA830 too. A depends on should not be required at all since the
> driver should build on all architectures. But I have seen resistance to
> doing that since users don't like to see configuration options totally
> irrelevant for the architecture they are building for. So may be take a
> middle path and do 'depends on ARCH_ARM'?

I will add dependency only on ARCH_ARM and submit another version.

Thanks
Avinash

> 
> Thanks,
> Sekhar
>
Thierry Reding March 14, 2013, 3:11 p.m. UTC | #3
On Thu, Mar 14, 2013 at 12:54:08PM +0000, Philip, Avinash wrote:
> On Thu, Mar 14, 2013 at 17:09:04, Nori, Sekhar wrote:
> > On 3/14/2013 4:02 PM, Philip Avinash wrote:
> > > Add EHRPWM and ECAP support build support for DAVINCI_DA850 platforms.
> > > 
> > > Also, since DAVINCI platforms doesn't support TI-PWM-Subsystem module,
> > > remove the select option for CONFIG_PWM_TIPWMSS.
> > > 
> > > Also, update CONFIG_PWM_TIPWMSS compiler directive appropriately in
> > > pwm-tipwmss.h to fix the below compiler error upon removal of
> > > CONFIG_PWM_TIPWMSS for Davinci platforms.
> > > 
> > > 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_probe':
> > > 	drivers/pwm/pwm-tiecap.c:263:4: error: 'PWMSS_ECAPCLK_EN' undeclared
> > > 	(first use in this function)
> > > 	drivers/pwm/pwm-tiecap.c:263:4: note: each undeclared identifier
> > > 	is reported only once for each function it appears in
> > > 	drivers/pwm/pwm-tiecap.c:264:17: error: 'PWMSS_ECAPCLK_EN_ACK'
> > > 	undeclared (first use in this function)
> > > 	drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_remove':
> > > 	drivers/pwm/pwm-tiecap.c:291:49: error: 'PWMSS_ECAPCLK_STOP_REQ'
> > > 	undeclared (first use in this function)
> > > 	make[2]: *** [drivers/pwm/pwm-tiecap.o] Error 1
> > > 	make[1]: *** [drivers/pwm] Error 2
> > > 	make: *** [drivers] Error 2
> > > 
> > > Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> > 
> > >  config  PWM_TIECAP
> > >  	tristate "ECAP PWM support"
> > > -	depends on SOC_AM33XX
> > > -	select PWM_TIPWMSS
> > > +	depends on SOC_AM33XX || ARCH_DAVINCI_DA850
> > 
> > Having such narrow dependencies is wrong. The same device is present on
> > DaVinci DA830 too. A depends on should not be required at all since the
> > driver should build on all architectures. But I have seen resistance to
> > doing that since users don't like to see configuration options totally
> > irrelevant for the architecture they are building for. So may be take a
> > middle path and do 'depends on ARCH_ARM'?
> 
> I will add dependency only on ARCH_ARM and submit another version.

Perhaps SOC_AM33XX || ARCH_DAVINCI_DA8XX would be an alternative?

Thierry
diff mbox

Patch

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 0e0bfa0..897b53c 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -147,8 +147,7 @@  config PWM_TEGRA
 
 config  PWM_TIECAP
 	tristate "ECAP PWM support"
-	depends on SOC_AM33XX
-	select PWM_TIPWMSS
+	depends on SOC_AM33XX || ARCH_DAVINCI_DA850
 	help
 	  PWM driver support for the ECAP APWM controller found on AM33XX
 	  TI SOC
@@ -158,8 +157,7 @@  config  PWM_TIECAP
 
 config  PWM_TIEHRPWM
 	tristate "EHRPWM PWM support"
-	depends on SOC_AM33XX
-	select PWM_TIPWMSS
+	depends on SOC_AM33XX || ARCH_DAVINCI_DA850
 	help
 	  PWM driver support for the EHRPWM controller found on AM33XX
 	  TI SOC
@@ -169,7 +167,7 @@  config  PWM_TIEHRPWM
 
 config  PWM_TIPWMSS
 	bool
-	depends on SOC_AM33XX && (PWM_TIEHRPWM || PWM_TIECAP)
+	default y if SOC_AM33XX && (PWM_TIECAP || PWM_TIEHRPWM)
 	help
 	  PWM Subsystem driver support for AM33xx SOC.
 
diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h
index 11f76a1..10ad804 100644
--- a/drivers/pwm/pwm-tipwmss.h
+++ b/drivers/pwm/pwm-tipwmss.h
@@ -18,7 +18,6 @@ 
 #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)
@@ -28,6 +27,7 @@ 
 #define PWMSS_ECAPCLK_EN_ACK	BIT(0)
 #define PWMSS_EPWMCLK_EN_ACK	BIT(8)
 
+#ifdef CONFIG_PWM_TIPWMSS
 extern u16 pwmss_submodule_state_change(struct device *dev, int set);
 #else
 static inline u16 pwmss_submodule_state_change(struct device *dev, int set)