Re: [PATCH] ARM: SAMSUNG: Fix compilation error with s3c2410_defconfig

[Thierry Reding <thierry.reding@avionic-design.de>]

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

On Wed, Aug 08, 2012 at 07:25:17PM +0900, Kukjin Kim wrote:
> Thierry Reding wrote:
> > 
> > On Wed, Aug 08, 2012 at 03:30:32PM +0900, Kukjin Kim wrote:
> 
> [...]
> 
> > > Yeah, your pointing out is correct, but in this case, it should be
> > 'bool'
> > > not 'tristate' because the PWM driver cannot support module now.
> > 
> > Is there any reason why that is so?
> 
> I mean, current pwm-samsung.c does not support module, as you know, the
> pwm_init() of pwm-samsung is called by arch_initcall().
> 
> > Looking at the driver it seems like
> > it should be easy to turn it into a module.
> 
> Yeah, I know :)
> 
> > I know that Jingoo (Cc'ed)
> > has been working on the driver and I've asked him the same question
> > already.
> > 
> I didn't know, would be helpful to me if you could add me in Cc for
> discussion of pwm-samsung. And he is my team member, so I will talk to him
> about the plan.

He (she?) sent two patches:

	https://lkml.org/lkml/2012/8/2/108
	https://lkml.org/lkml/2012/8/2/109

And followed-up with the following three:

	https://lkml.org/lkml/2012/8/3/47
	https://lkml.org/lkml/2012/8/3/44
	https://lkml.org/lkml/2012/8/3/45

> > Anyway I don't want to force the issue, I just think you should consider
> > it.
> > 
> Thanks.

I guess it should be fine to go with builtin for now and keep in mind
that it might make sense to make it a proper module eventually.

> > > > solve this problem would be to add a default line, like so:
> > > >
> > > > 	default PLAT_SAMSUNG
> > > >
> > > > I've checked this with a s3c2410_defconfig and this causes PWM_SAMSUNG
> > > > to be selected =y, which I guess is what you want.
> > > >
> > > How do you think following, just adding from original one?
> > >
> > > -	tristate "Samsung pwm support"
> > > +	bool "SAMSUNG PWM support"
> > >
> > > Thanks.
> > 
> > If you convert this to bool anyway, then maybe you can still use
> > def_bool:
> > 
> > 	config PWM_SAMSUNG
> > 		prompt "SAMSUNG PWM support" if PLAT_SAMSUNG
> > 		def_bool PLAT_SAMSUNG
> > 
> > Any particular reason why you want "SAMSUNG" capitalized?
> > 
> No, there is no reason, just because...
> 
> So, how about following? If PWM is selected on Samsung SoCs, the PWM_SAMSUNG
> will be selected automatically. Of course, it can be de-selected in kernel
> menuconfig. Note that, I think, using 'bool <expr>' and 'depends on <expr>'
> is more clear than 'prompt <prompt> ["if" <expr>]'. However if any your
> preference here, please kindly let me know.

I do like that much better as well. I just kept the def_bool because it
was proposed earlier in the patch.

> ---
> 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
> ---

Yes, that looks good to me. Do you want me to take that through the PWM
tree or would you rather take it through yours?

Thierry

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