Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754002AbbDNGpH (ORCPT ); Tue, 14 Apr 2015 02:45:07 -0400 Received: from mail-pd0-f175.google.com ([209.85.192.175]:34964 "EHLO mail-pd0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753825AbbDNGoc (ORCPT ); Tue, 14 Apr 2015 02:44:32 -0400 MIME-Version: 1.0 In-Reply-To: <1428950956.3868.16.camel@x220> References: <1428941512-26411-1-git-send-email-punnaia@xilinx.com> <1428950956.3868.16.camel@x220> Date: Tue, 14 Apr 2015 12:14:32 +0530 X-Google-Sender-Auth: PS7ZsMYhgFf-vZthFfdSOBziVfA Message-ID: Subject: Re: [PATCH v6 2/2] memory: pl353: Add driver for arm pl353 static memory controller From: punnaiah choudary kalluri To: Paul Bolle Cc: Punnaiah Choudary Kalluri , "robh+dt@kernel.org" , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , Kumar Gala , Rob Landley , "michal.simek@xilinx.com" , Grant Likely , "gregkh@linuxfoundation.org" , "jason@lakedaemon.net" , "ezequiel.garcia@free-electrons.com" , Arnd Bergmann , David Woodhouse , Brian Norris , "artem.bityutskiy@linux.intel.com" , "jussi.kivilinna@iki.fi" , Alexandre Courbot , "Khoronzhuk, Ivan" , "joern@logfs.org" , "devicetree@vger.kernel.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mtd@lists.infradead.org" , Punnaiah Choudary 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: 3287 Lines: 105 Hi Paul Bolle On Tue, Apr 14, 2015 at 12:19 AM, Paul Bolle wrote: > On Mon, 2015-04-13 at 21:41 +0530, Punnaiah Choudary Kalluri wrote: >> --- a/drivers/memory/Kconfig >> +++ b/drivers/memory/Kconfig > >> +config PL353_SMC >> + bool "ARM PL353 Static Memory Controller (SMC) driver" >> + depends on ARM >> + help >> + This driver is for the ARM PL353 Static Memory Controller (SMC) >> + module. > > This adds a bool symbol. > >> --- a/drivers/memory/Makefile >> +++ b/drivers/memory/Makefile > >> +obj-$(CONFIG_PL353_SMC) += pl353-smc.o > > Which means pl353-smc.o can never be part of a module, right? > > (If that's not right you can stop reading here.) > >> --- /dev/null >> +++ b/drivers/memory/pl353-smc.c > >> + * This program is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation, either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. > > This states the license is GPL v2 or later. > >> +#include > > I wonder whether this include is needed, since this is built-in only > code. > >> +MODULE_DEVICE_TABLE(of, pl353_smc_of_match); > > According to include/linux/module.h this will be preprocessed away for > built-in code. > >> +static struct platform_driver pl353_smc_driver = { >> + .probe = pl353_smc_probe, >> + .remove = pl353_smc_remove, >> + .driver = { >> + .name = "pl353-smc", >> + .owner = THIS_MODULE, > > THIS_MODULE will be equivalent to NULL for built-in code, according to > include/linux/export.h. > >> + .pm = &pl353_smc_dev_pm_ops, >> + .of_match_table = pl353_smc_of_match, >> + }, >> +}; > >> +module_platform_driver(pl353_smc_driver); > > Speaking from memory: for built-in only code this is equivalent to > having a wrapper that only does > register_platform_driver(&pl353_smc_driver); > > and mark that wrapper with device_initcall(). > >> +MODULE_AUTHOR("Xilinx, Inc."); >> +MODULE_DESCRIPTION("ARM PL353 SMC Driver"); >> +MODULE_LICENSE("GPL v2"); > > For built-in only code these macros will be effectively preprocessed > away. > > (Would you make PL353_SMC a tristate symbol then you should note that > according to include/linux/module.h "GPL" is the license ident that > matches the license stated in the comment at the top of this file.) Ok. I will make PL353_SMC as tristate symbol and also i will change the licence ident to "GPL". Thanks for the review. I will wait some time for further functional comments on this driver before sending the next version of patches Thanks, Punnaiah > > 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/