Hi Lee, On Mon, Jun 20, 2016 at 09:00:51AM +0100, Lee Jones wrote: > On Fri, 17 Jun 2016, Doug Anderson wrote: > > On Fri, Jun 17, 2016 at 1:06 AM, Lee Jones <lee.jones@xxxxxxxxxx> wrote: > > >> Probably the reason for all of these non-kernel-isms is that this > > >> isn't a kernel file. From the top of the file: > > >> > > >> * NOTE: This file is copied verbatim from the ChromeOS EC Open Source > > >> * project in an attempt to make future updates easy to make. > > >> > > >> So the source of truth for this file is > > >> <https://chromium.googlesource.com/chromiumos/platform/ec/+/master/include/ec_commands.h>. > > >> > > >> Someone could probably submit a CL to that project to make it a little > > >> more kernel-ish and then we'd have to see if the EC team would accept > > >> such changes... > > > > > > Hmmm... that kinda puts me in a difficult position. Do I except > > > non-kernel code, which does not conform to our stands? > > > > What about if Brian made sure to just fully copy the latest version of > > "cros_ec_commands.h" from the EC codebase and changed this commit > > message to say: > > > > Copy the latest version of "cros_ec_commands.h" from the Chrome OS EC > > code base, which is the source of truth for this file. See > > <https://chromium.googlesource.com/chromiumos/platform/ec/+/master/include/ec_commands.h>. > > > > From the commit message it would be clear that this is an external > > file linked into the kernel for convenience. > > > > > > > Naturally I'd be happier if you could try to make the code more > > > 'kernely'. The practices I mention above are still good ones, even if > > > you're not writing kernel specific code. > > > > In general requesting that code from outside the kernel conform to > > "kerneldoc" seems like a bit of a stretch. In general having some > > type of parse-able format for comments is nice, but I could see that > > in the Chrome OS EC codebase it would be a bit overkill. > > It's unfortunate that kerneldoc is named so, since I think it's a nice > way to write structure/function headers regardless of code base and > not overkill at all. It's certainly harder to convince !kernel code > to use the format, or at least easier for others to push back due to > the fact that is has 'kernel' in the name. > > > Also: it would be awfully strange if we suddenly started changing the > > coding convention of this file or we had half the file in one > > convention and half in another. The rest of this file is in EC > > convention and it seems sane to keep it that way... > > Right. It's also a shame we're only catching this now. Really we > should have had this discussion in the first instance. Well, I see your sign-off on 2 of 4 patches to this file :) > Taking into consideration that this file is already in the kernel and > that it's current format is also represented, I'm willing to keep > adding to it. Thanks, that seems quite reasonable. > I would like to see an internal request to adopt > so-called kerneldoc. Not because I am wish to blindly push our > standards to other code-bases, but because I am an advocate of the > format in general. As mentioned in another branch of this thread, such a request has already been made (and it's public, so feel free to follow if you're so inclined): https://bugs.chromium.org/p/chromium/issues/detail?id=621123 It seems as if the firmware authors have already agreed that adopting a more consistent style like kerneldoc would be a net postive. So you should expect to eventually see a patch to update this file. But in the meantime, I think it would make things sanest if we update the file incrementally until that is done. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
