Subject: Re: [PATCH v3 00/11] Add arm64/signal initial kselftest support

From: Dave Martin <[email-masked]>
Date: Tue, 13 Aug 2019 17:22:54 +0100

On Fri, Aug 02, 2019 at 06:02:49PM +0100, Cristian Marussi wrote:
> Hi
> 
> this patchset aims to add the initial arch-specific arm64 support to
> kselftest starting with signals-related test-cases.
> A common internal test-case layout is proposed which then it is anyway
> wired-up to the toplevel kselftest Makefile, so that it should be possible
> at the end to run it on an arm64 target in the usual way with KSFT.

The tests look like a reasonable base overall and something that we can
extend later as needed.

There are various minor things that need attention -- see my comments on
the individual patches.  Apart for some things that can be factored out,
I don't think any of it involves redesign.

A few general comments:

 * Please wrap all commit messages to <= 75 chars, and follow the other
   guidelines about commit messages in
   Documentation/process/submitting-patches.rst).

 * Remember to run scripts/checkpatch.pl on your patches.  Currently
   various issues are reported: they should mostly be trivial to fix.
   checkpatch does report some false positives, but most of the warnings
   I see look relevant.

 * If you like, you can add an Author: line alongside the copyright
   notice in new files that you create.  (You'll see this elsewhere in
   the kernel if you grep.)

One general stylistic issue (IMHO):

 * Try to avoid inventing names for things that have no established
   name (for example "magic0" to mean "magic number 0").

   The risk is that the reader wastes time grepping for the definition,
   when really the text should be read at face value.  It's best to use
   all caps just for #define names, abbreviations, and other things
   that are customarily capitalised (like "CPU" etc.).  Other words
   containing underscores may resemble variable / function names, and
   may cause confusion of there is no actual variable or function with
   that name.

   I don't think it's worth heavily reworking the patches for this, but
   it's something to bear in mind.

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel