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
pgp8Zpkj7U8ML.pgp
Description: PGP signature
