lkml.org 
[lkml]   [2012]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ARM: SAMSUNG: Fix compilation error with s3c2410_defconfig
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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2012-08-08 13:42    [W:0.060 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site