From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964795AbbFRRyp (ORCPT ); Thu, 18 Jun 2015 13:54:45 -0400 Received: from mail-ig0-f178.google.com ([209.85.213.178]:38765 "EHLO mail-ig0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932767AbbFRRyf (ORCPT ); Thu, 18 Jun 2015 13:54:35 -0400 MIME-Version: 1.0 In-Reply-To: <1430428322.2187.24.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> Date: Thu, 18 Jun 2015 23:24:35 +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 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. Still it is good to keep the module based initialization. Firstly because it causes no harm and even though some of the macros are pre-processed out, gives info about the driver. Secondly there were discussion on why INTEL_SOC_PMIC is bool (note this driver also has module based initialization even when bool). 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. Regards Shobhit > > (If I'm wrong, and that object file can actually be part of a module, > you can stop reading here.) > >> --- /dev/null >> +++ b/drivers/pwm/pwm-crc.c > >> +#include > > Perhaps this include is not needed. > >> +static const struct pwm_ops crc_pwm_ops = { >> + .config = crc_pwm_config, >> + .enable = crc_pwm_enable, >> + .disable = crc_pwm_disable, >> + .owner = THIS_MODULE, > > For built-in only code THIS_MODULE is basically equivalent to NULL (see > include/linux/export.h). So I guess this line can be dropped. > >> +}; > >> +static struct platform_driver crystalcove_pwm_driver = { >> + .probe = crystalcove_pwm_probe, >> + .remove = crystalcove_pwm_remove, >> + .driver = { >> + .name = "crystal_cove_pwm", >> + }, >> +}; >> + >> +module_platform_driver(crystalcove_pwm_driver); > > Speaking from memory: for built-in only code this is equivalent to > calling > platform_driver_register(&crystalcove_pwm_driver); > > from a wrapper, and marking that wrapper with device_initcall(). > >> +MODULE_AUTHOR("Shobhit Kumar "); >> +MODULE_DESCRIPTION("Intel Crystal Cove PWM Driver"); >> +MODULE_LICENSE("GPL v2"); > > These macros will be effectively preprocessed away for built-in only > code. > > > Paul Bolle > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx