| Message ID | 0af001cd7550$1af66040$50e320c0$%kim@samsung.com |
|---|---|
| State | New |
| Headers | show |
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
Thierry Reding wrote: > > 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: > > [...] > He (she?) sent two patches: > He :-) > 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 > Thanks for above list-up and if any comments, let you know. [...] > 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? > Thanks. Since the error happens in arch/arm/ part and this patch touches some arch/arm/ files, would be better if this could be sent to upstream via samsung tree. Of course, if your ack, would be nice to me ;-) Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.
On Wed, Aug 08, 2012 at 07:25:17PM +0900, Kukjin Kim wrote: > 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. > > --- > 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 Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
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