[Nouveau] [PATCH 0/4] Refuse to load if the power cable are not connected

[Rhys Kidd]

On Fri, 12 Jul 2019 at 00:42, Mark Menzynski <mmenzyns at redhat.com> wrote:

Hello Mark,

Thank you for your first kernel patch series to nouveau!
This is a good starting point. However, I would suggest a couple of general
improvements which apply to the whole series:

1. Invest a little bit more time in writing the commit messages.

There is a slightly subjective element to what makes a "good" commit
message, but generally the principle is that it should concisely
convey the "what" of the patch, but more importantly the "why" of the patch.

A future reader (which might be yourself!) can understand *what* the patch
does from the code, to a lesser or so amount of time.
The *why* of a patch really can only be communicated within the commit
message, so a little extra time spent here is well spent.

A helpful approach I suggest is trying to follow the style of the folder,
submodule or driver:

  $ git log -n20 drivers/gpu/drm/nouveau/

There's also a web resource here: https://chris.beams.io/posts/git-commit/
(although it's not Linux kernel specific)

2. Run ./scripts/checkpatch.pl on the final patch series

You may have already done this, as I noticed the current patches report no
errors or warnings. If so, well done!

It's a good habit to re-run this script against the final v2 patch series
again though.

You can also ask questions on freenode #nouveau where many of the nouveau
graphics stack driver developers hang out.

Rhys

trello card:
>
> https://trello.com/c/admzDRvd/50-reduce-performance-refuse-to-boot-if-not-all-the-power-connectors-are-plugged
>
> Mark Menzynski (4):
>   moved gpio so it is sorted by values
>   gpio: checking if gpu power cable is connected
>   gpio: added power check for another GPIO
>   gpio: added power check for another GPIO
>

I'd recommend adding to the first line of the commit message of these two
patches something which distinguishes them
apart. e.g. it appears you've identified that there are different gpio pin
functions for different nvidia gpu families, so perhaps
use the pre- / and post- family names to separate the two patches for a
reader.

Also, take a look at how the prefix before the ":" is usually written in
this area of the code:

  $ git log -n10 --oneline drivers/gpu/drm/nouveau/<relevant subfolders>


>
>  drm/nouveau/include/nvkm/subdev/bios/gpio.h |  5 ++++-
>  drm/nouveau/nvkm/subdev/gpio/base.c         | 25 +++++++++++++++++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20190712/b0a630d1/attachment.html>