RE: [PATCH 4/6] ARM: s3c24xx: use new PWM driver

[Kukjin Kim <kgene.kim@samsung.com>]

Arnd Bergmann wrote:
> 
> On Wednesday 08 August 2012, Kukjin Kim wrote:
> 
> > diff --git a/arch/arm/mach-s3c24xx/Kconfig b/arch/arm/mach-
> s3c24xx/Kconfig
> > index e249611..d56b0f7 100644
> > --- a/arch/arm/mach-s3c24xx/Kconfig
> > +++ b/arch/arm/mach-s3c24xx/Kconfig
> > @@ -483,7 +483,7 @@ config MACH_NEO1973_GTA02
> >  	select I2C
> >  	select POWER_SUPPLY
> >  	select MACH_NEO1973
> > -	select S3C2410_PWM
> > +	select S3C24XX_PWM
> >  	select S3C_DEV_USB_HOST
> >  	help
> >  	   Say Y here if you are using the Openmoko GTA02 / Freerunner GSM
> > Phone
> > @@ -493,7 +493,7 @@ config MACH_RX1950
> >  	select S3C24XX_DCLK
> >  	select PM_H1940 if PM
> >  	select I2C
> > -	select S3C2410_PWM
> > +	select S3C24XX_PWM
> >  	select S3C_DEV_NAND
> >  	select S3C2410_IOTIMING if S3C2440_CPUFREQ
> >  	select S3C2440_XTAL_16934400
> > diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-
> samsung/Kconfig
> > index 7aca31c..dcdfb77 100644
> > --- a/arch/arm/plat-samsung/Kconfig
> > +++ b/arch/arm/plat-samsung/Kconfig
> > @@ -403,7 +403,7 @@ config S5P_DEV_USB_EHCI
> >
> >  config S3C24XX_PWM
> >  	bool "PWM device support"
> > -	select HAVE_PWM
> > +	select PWM
> >  	help
> >  	  Support for exporting the PWM timer blocks via the pwm device
> >  	  system
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index 8fc3808..c74d055 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -58,14 +58,12 @@ config PWM_PXA
> >  	  will be called pwm-pxa.
> >
> >  config PWM_SAMSUNG
> > -	tristate "Samsung pwm support"
> > +	bool "Samsung PWM support"
> >  	depends on PLAT_SAMSUNG
> > +	default y
> >  	help
> >  	  Generic PWM framework driver for Samsung.
> >
> > -	  To compile this driver as a module, choose M here: the module
> > -	  will be called pwm-samsung.
> > -
> >  config PWM_TEGRA
> >  	tristate "NVIDIA Tegra PWM support"
> >  	depends on ARCH_TEGRA
> 
> This approach has two disadvantages compared to mine:
> 
> * When building for the samsung platforms other than rx1950 or gta02,
>   you can no longer have the PWM driver as a loadable module, for no
>   good reason.
> 
OK I see, I'm still thinking the PWM driver can be selected in kernel
menuconfig though.

My concept is when PWM is selected on Samsung platforms by defconfig or
manually enabling, PWM_SAMSUNG is selected automatically. In addition,
current Samsung PWM driver cannot support module now because pwm_init() is
called by arch_initcall().

I think, we can remove S3C24XX_PWM and just select PWM (or using HAVE_PWM)
and PWM_SAMSUNG can be selected by enabling PWM in samsung platform next
time :-)

> * It is still possible to manually disable PWM_SAMSUNG, even on
>   rx1950 and gta02, because the Kconfig symbol is visible.
> 
Yeah you're right, makes sense.

> I've added a "Reported-by: Tushar Behera <tushar.behera@linaro.org>"
> line to my version of the patch, but I would prefer to keep that
> approach. I have also changed the patch to use the S3C24XX_PWM
> symbol because that lets everyone migrate the defconfig files.
> 
OK.

> Do you want to give this patch some more testing, or should I keep
> it in arm-soc?
> 
Looks good to me, please keep going on.

> ---
> From 438b0cf94adb2528bdeeb71314f7be16512ea5b1 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Sat, 4 Aug 2012 07:52:19 +0000
> Subject: [PATCH] ARM: s3c24xx: use new PWM driver
> 
> The samsung PWM driver has moved to the new PWM subsystem, which
> changed the Kconfig symbol for that driver, but the rx1950 and
> gta02 boards still uses the old one.
> 
> Without this patch, building s3c2410_defconfig results in:
> 
> arch/arm/mach-s3c24xx/built-in.o: In function `rx1950_lcd_power':
> arch/arm/mach-s3c24xx/mach-rx1950.c:430: undefined reference to
> `pwm_config'
> arch/arm/mach-s3c24xx/mach-rx1950.c:431: undefined reference to
> `pwm_disable'
> arch/arm/mach-s3c24xx/mach-rx1950.c:437: undefined reference to
> `pwm_config'
> arch/arm/mach-s3c24xx/mach-rx1950.c:438: undefined reference to
> `pwm_enable'
> arch/arm/mach-s3c24xx/built-in.o: In function `rx1950_backlight_exit':
> arch/arm/mach-s3c24xx/mach-rx1950.c:504: undefined reference to `pwm_free'
> arch/arm/mach-s3c24xx/built-in.o: In function `rx1950_backlight_init':
> arch/arm/mach-s3c24xx/mach-rx1950.c:487: undefined reference to
> `pwm_request'
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reported-by: Tushar Behera <tushar.behera@linaro.org>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> 
Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> diff --git a/arch/arm/mach-s3c24xx/Kconfig b/arch/arm/mach-s3c24xx/Kconfig
> index e249611..d56b0f7 100644
> --- a/arch/arm/mach-s3c24xx/Kconfig
> +++ b/arch/arm/mach-s3c24xx/Kconfig
> @@ -483,7 +483,7 @@ config MACH_NEO1973_GTA02
>  	select I2C
>  	select POWER_SUPPLY
>  	select MACH_NEO1973
> -	select S3C2410_PWM
> +	select S3C24XX_PWM
>  	select S3C_DEV_USB_HOST
>  	help
>  	   Say Y here if you are using the Openmoko GTA02 / Freerunner GSM
> Phone
> @@ -493,7 +493,7 @@ config MACH_RX1950
>  	select S3C24XX_DCLK
>  	select PM_H1940 if PM
>  	select I2C
> -	select S3C2410_PWM
> +	select S3C24XX_PWM
>  	select S3C_DEV_NAND
>  	select S3C2410_IOTIMING if S3C2440_CPUFREQ
>  	select S3C2440_XTAL_16934400
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index 7aca31c..9c3b90c 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -403,7 +403,8 @@ config S5P_DEV_USB_EHCI
> 
>  config S3C24XX_PWM
>  	bool "PWM device support"
> -	select HAVE_PWM
> +	select PWM
> +	select PWM_SAMSUNG
>  	help
>  	  Support for exporting the PWM timer blocks via the pwm device
>  	  system