Messages in this thread | | | Subject | Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver | From | Paul Bolle <> | Date | Thu, 18 Jun 2015 20:41:56 +0200 |
| |
Hi Shobhit,
On Thu, 2015-06-18 at 23:24 +0530, Shobhit Kumar wrote: > On Fri, May 1, 2015 at 2:42 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > > On Wed, 2015-04-29 at 19:30 +0530, Shobhit Kumar wrote: > >> --- a/drivers/pwm/Kconfig > >> +++ b/drivers/pwm/Kconfig > > > >> +config PWM_CRC > >> + bool "Intel Crystalcove (CRC) PWM support" > >> + depends on X86 && INTEL_SOC_PMIC > >> + help > >> + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM > >> + control. > > > >> --- a/drivers/pwm/Makefile > >> +++ b/drivers/pwm/Makefile > > > >> +obj-$(CONFIG_PWM_CRC) += pwm-crc.o > > > > PWM_CRC is a bool symbol. So pwm-crc.o can never be part of a module. > > I actually started this as a module but later decided to make it as > bool because INTEL_SOC_PMIC on which this depends is itself a bool as > well.
As does GPIO_CRYSTAL_COVE and that's a tristate. So?
> Still it is good to keep the module based initialization. > Firstly because it causes no harm
If I got a dime for every time people used an argument like that I ... I could treat myself to an ice cream. A really big ice cream. Hmm, that doesn't sound too impressive. But still, "causes no harm" is below the bar for kernel code. Kernel code needs to add value.
> and even though some of the macros > are pre-processed out, gives info about the driver.
None of which can't be gotten elsewhere (ie, the commit message, or the file these macro reside in).
> Secondly there > were discussion on why INTEL_SOC_PMIC is bool (note this driver also > has module based initialization even when bool).
Yes, there's copy and paste going on even in kernel development.
> I am guessing because > of some tricky module load order dependencies. If ever that becomes a > module, this can mostly be unchanged to be loaded as a module.
You put in a macro, or any other bit of code, when it's needed, not beforehand, "just in case". That's silly.
Thanks,
Paul Bolle
| |