Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753068AbbFSGq4 (ORCPT ); Fri, 19 Jun 2015 02:46:56 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:35595 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751517AbbFSGqs (ORCPT ); Fri, 19 Jun 2015 02:46:48 -0400 MIME-Version: 1.0 In-Reply-To: <1434652916.2385.124.camel@x220> References: <1430316005-16480-1-git-send-email-shobhit.kumar@intel.com> <1430316005-16480-7-git-send-email-shobhit.kumar@intel.com> <1430428322.2187.24.camel@x220> <1434652916.2385.124.camel@x220> Date: Fri, 19 Jun 2015 12:16:47 +0530 Message-ID: Subject: Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver From: Shobhit Kumar To: Paul Bolle Cc: Shobhit Kumar , linux-pwm , Jani Nikula , Samuel Ortiz , Alexandre Courbot , David Airlie , Povilas Staniulis , intel-gfx , linux-kernel , dri-devel , linux-gpio , Chih-Wei Huang , Thierry Reding , Daniel Vetter , Lee Jones , Linus Walleij Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2938 Lines: 85 Hi Paul, On Fri, Jun 19, 2015 at 12:11 AM, Paul Bolle wrote: > 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 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). > Causes no harm comment had to be read together with more info about the driver. It causes no harm while providing more info. And as you only said those macros are pre-processed out to really the defaults for built-in drivers. So what is the exact big problem with this ? I can anyway shove out these macros to end the discussion. BTW whether you buy the argument or not, please do treat yourself with ice cream for being able to make such a comment. >> 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. > There are other examples in the kernel. I just gave the one which is related as well. Regards Shobhit >> 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/