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