On Fri, Jun 17, 2016 at 05:55:52PM +0200, Thierry Reding wrote: > On Fri, Jun 17, 2016 at 09:06:35AM +0100, Lee Jones wrote: > > On Thu, 16 Jun 2016, Doug Anderson wrote: > > > On Thu, Jun 16, 2016 at 8:38 AM, Lee Jones <lee.jones@xxxxxxxxxx> wrote: [...] > > > > Please use kerneldoc format. [...] > > > 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? I could have sworn that there was an exception for existing code; changing styles just for the sake of changing styles can hurt more than help (e.g., in this instance, in comparing this file with the source of truth -- the Chrome OS EC firmware sources). But I can't find an explicit reference to this right now... > I think usually code that doesn't adhere to kernel coding style ends up > in the staging tree until it's been cleaned up enough. > > The rule doesn't quite apply here because cleaning up isn't the issue. > But I don't know if we have any best practices for this kind of thing. > > One thing that I've seen done in the past is to have this kind of > cross-OS header generated from some sort of definition file (XML, ...) > with an output filter for all supported OSes. Is that something that > could perhaps be done here? I think I can safely say that rewriting this in XML is out of the question for us. > That said, this might not even be worth it in this case. While I see how > it makes sense to avoid work updating this file for various coding > styles, the content in this file defines an ABI, so really the only > changes will be additions, and once they're merged they become set in > stone anyway. The amount of work updating the header, even taking into > account different coding styles should be very low. IMO, there is value in being able to compare the two sources, and there is little (but non-zero) value in making these rather minor suggested changes. Additionally, reformatting this particular change to use kerneldoc, when the rest of the file doesn't, seems inconsistent. How does the following sound instead? (1) Keep the existing patch as-is (perhaps modulo one or two of Lee's non-kerneldoc-related comments) (2) I send a follow-up patch, to help synchronize the out-of-tree EC header with the kernel header, after we've considered converting it to better match kernel styles. BTW, we've filed a bug for this discussion [1], to ensure it doesn't get dropped on the floor. Brian [1] https://bugs.chromium.org/p/chromium/issues/detail?id=621123 -- 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
