From: Lee Jones <lee.jones@linaro.org> To: Doug Anderson <dianders@chromium.org> Cc: Brian Norris <briannorris@chromium.org>, Thierry Reding <thierry.reding@gmail.com>, Olof Johansson <olof@lixom.net>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Brian Norris <computersforpeace@gmail.com>, linux-pwm <linux-pwm@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, Boris Brezillon <boris.brezillon@free-electrons.com>, Stephen Barber <smbarber@chromium.org>, Javier Martinez Canillas <javier@osg.samsung.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo <enric.balletbo@collabora.co.uk>, Randall Spangler <rspangler@chromium.org>, Shawn Nematbakhsh <shawnn@chromium.org>, Dmitry Torokhov <dmitry.torokhov@gmail.com>, Todd Broch <tbroch@chromium.org>, Gwendal Grignou <gwendal@chromium.org>, Tomeu Vizoso <tomeu.vizoso@collabora.com> Subject: Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Date: Mon, 20 Jun 2016 09:00:51 +0100 [thread overview] Message-ID: <20160620080051.GJ21702@dell> (raw) In-Reply-To: <CAD=FV=UUJt4vBwHJrUYrKN56dv2YqdW3R7wah7-_PqyQ=YYr7Q@mail.gmail.com> On Fri, 17 Jun 2016, Doug Anderson wrote: > Lee, > > On Fri, Jun 17, 2016 at 1:06 AM, Lee Jones <lee.jones@linaro.org> 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. 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. 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. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> To: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Cc: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, linux-pwm <linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>, Stephen Barber <smbarber-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>, Benson Leung <bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Enric Balletbo <enric.balletbo-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>, Randall Spangler <rspangler-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Shawn Nematbakhsh <shawnn-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Gwendal Grignou <gwendal-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Tomeu Vizoso <tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> Subject: Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Date: Mon, 20 Jun 2016 09:00:51 +0100 [thread overview] Message-ID: <20160620080051.GJ21702@dell> (raw) In-Reply-To: <CAD=FV=UUJt4vBwHJrUYrKN56dv2YqdW3R7wah7-_PqyQ=YYr7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> On Fri, 17 Jun 2016, Doug Anderson wrote: > Lee, > > On Fri, Jun 17, 2016 at 1:06 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 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. 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. 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. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-06-20 8:01 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-03 1:21 [PATCH v2 0/4] pwm: add support for ChromeOS EC PWM Brian Norris 2016-06-03 1:21 ` Brian Norris 2016-06-03 1:21 ` [PATCH v2 1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper Brian Norris 2016-06-03 1:21 ` Brian Norris 2016-06-16 15:30 ` Lee Jones 2016-06-16 15:38 ` Enric Balletbo i Serra 2016-06-03 1:21 ` [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions Brian Norris 2016-06-03 1:21 ` Brian Norris 2016-06-16 15:38 ` Lee Jones 2016-06-16 15:51 ` Doug Anderson 2016-06-17 8:06 ` Lee Jones 2016-06-17 15:28 ` Doug Anderson 2016-06-17 15:28 ` Doug Anderson 2016-06-20 8:00 ` Lee Jones [this message] 2016-06-20 8:00 ` Lee Jones 2016-06-20 17:56 ` Brian Norris 2016-06-20 17:56 ` Brian Norris 2016-06-17 15:55 ` Thierry Reding 2016-06-17 19:13 ` Brian Norris 2016-06-17 19:21 ` Brian Norris 2016-06-20 7:47 ` Lee Jones 2016-06-03 1:21 ` [PATCH v2 3/4] doc: dt: pwm: add binding for ChromeOS EC PWM Brian Norris 2016-06-03 1:21 ` Brian Norris 2016-06-06 13:36 ` Rob Herring 2016-06-03 1:21 ` [PATCH v2 4/4] pwm: add ChromeOS EC PWM driver Brian Norris 2016-06-03 1:21 ` Brian Norris
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160620080051.GJ21702@dell \ --to=lee.jones@linaro.org \ --cc=bleung@chromium.org \ --cc=boris.brezillon@free-electrons.com \ --cc=briannorris@chromium.org \ --cc=computersforpeace@gmail.com \ --cc=devicetree@vger.kernel.org \ --cc=dianders@chromium.org \ --cc=dmitry.torokhov@gmail.com \ --cc=enric.balletbo@collabora.co.uk \ --cc=gwendal@chromium.org \ --cc=javier@osg.samsung.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pwm@vger.kernel.org \ --cc=olof@lixom.net \ --cc=rspangler@chromium.org \ --cc=shawnn@chromium.org \ --cc=smbarber@chromium.org \ --cc=tbroch@chromium.org \ --cc=thierry.reding@gmail.com \ --cc=tomeu.vizoso@collabora.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.