From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Bolle Subject: Re: [PATCH v3 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources Date: Tue, 17 Jun 2014 10:53:22 +0200 Message-ID: <1402995202.1984.22.camel@x220> References: <1398879850-9111-1-git-send-email-dianders@chromium.org> <1398879850-9111-6-git-send-email-dianders@chromium.org> <20140520084602.GF24991@lee--X1> <1402483068.3798.82.camel@x220> <1402646902.28881.15.camel@x220> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Doug Anderson Cc: Bill Richardson , Simon Glass , Lee Jones , Stephen Warren , Wolfram Sang , Andrew Bresticker , Dylan Reid , Olof Johansson , Samuel Ortiz , linux-samsung-soc , "linux-tegra@vger.kernel.org" , "linux-kernel@vger.kernel.org" List-Id: linux-tegra@vger.kernel.org Doug, On Fri, 2014-06-13 at 08:22 -0700, Doug Anderson wrote: > On Fri, Jun 13, 2014 at 1:08 AM, Paul Bolle wrote: > > On Wed, 2014-06-11 at 08:11 -0700, Doug Anderson wrote: > >> This is a config option on the ChromeOS EC > >> . Doing a > >> grep there: > >> > >> board/samus/board.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE > >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE > >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE > >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE > >> driver/battery/samus.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE > >> driver/battery/samus.c:#endif /* CONFIG_CHARGER_PROFILE_OVERRIDE */ > >> include/config.h:#undef CONFIG_CHARGER_PROFILE_OVERRIDE > >> include/ec_commands.h: /* Range for CONFIG_CHARGER_PROFILE_OVERRIDE params */ > >> test/test_config.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE > > > > I see. So this is not a Kconfig macro but a general macro with a CONFIG_ > > prefix. There are quite a bit of those in the tree already, but still, > > would another prefix also do? > > Given that it's an entirely separate project and this is a valid > CONFIG option in that project, it seems a lot to ask them not to use > the CONFIG_ prefix. Also: the part you are objecting to is only a > comment, right? So all the hits you quoted above are actually from code that's never going to be included in the kernel tree, right? If so, then yes, we're only discussing a single comment. > We could certainly add extra wording in the comment to make it obvious > that this is a CONFIG option for the EC and not the kernel. Would > that be enough? ...or are you trying to use some scripts to > automatically process files to look for CONFIG options? Yes, I'm using a script to check for Kconfig macros, among other things. It doesn't care about comments (because every now and then mistakes are made in comments too, and some of those can get surprisingly confusing). Anyhow, the CONFIG_ prefix used in the kernel tree is quite generic, but we're stuck with it. Would it be bothersome to drop it in that comment? Mentioning a preprocessor macro from a separate project is a bit confusing to begin with. How is one supposed to know that this is a reference to something out of tree? So, in summary, while we're apparently only discussing a single comment, I would appreciate it if it could be reworded, preferably by dropping that the CONFIG_ prefix. But other people might care very little, as they don't share this particular pet peeve. Thanks, Paul Bolle