On Fri, 12 Jul 2019 at 00:42, Mark Menzynski <mmenzyns@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________ Nouveau mailing list Nouveau@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/nouveau
