public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: Cristian Marussi <cristian.marussi@arm•com>
To: Dave Martin <Dave.Martin@arm•com>
Cc: andreyknvl@google•com, shuah@kernel•org,
	linux-arm-kernel@lists•infradead.org,
	linux-kselftest@vger•kernel.org
Subject: Re: [PATCH v3 00/11] Add arm64/signal initial kselftest support
Date: Fri, 30 Aug 2019 17:40:42 +0100	[thread overview]
Message-ID: <b4c9cb22-6e08-8096-addb-4ac267fc0a84@arm.com> (raw)
In-Reply-To: <20190813162254.GX10425@arm.com>

Hi

On 13/08/2019 17:22, Dave Martin wrote:
> 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.
> 

Thanks for the review. I addressed latest issues in V4, published now.

I kept tests verbose (outputting to stderr) as of now.
Removed as a whole standalone build/run.

Thanks

Cristian

>  * 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
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-08-30 16:40 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02 17:02 [PATCH v3 00/11] Add arm64/signal initial kselftest support Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 01/11] kselftest: arm64: introduce new boilerplate code Cristian Marussi
2019-08-13 16:23   ` Dave Martin
2019-08-27 12:14     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 02/11] kselftest: arm64: adds first test and common utils Cristian Marussi
2019-08-06 15:50   ` Cristian Marussi
2019-08-07 15:42   ` Cristian Marussi
2019-08-09 10:54   ` Cristian Marussi
2019-08-09 11:16     ` Dave Martin
2019-08-09 12:20       ` Cristian Marussi
2019-08-09 12:32         ` Dave Martin
2019-08-12 12:43   ` Amit Kachhap
2019-08-13 13:22     ` Cristian Marussi
2019-08-14 10:22       ` Amit Kachhap
2019-08-27 14:24         ` Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-28 17:34     ` Cristian Marussi
2019-09-03 15:34       ` Dave Martin
2019-09-03 16:08         ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 03/11] kselftest: arm64: mangle_pstate_invalid_daif_bits Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-29 10:19     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 04/11] kselftest: arm64: mangle_pstate_invalid_mode_el Cristian Marussi
2019-08-13 16:24   ` Dave Martin
2019-08-29 11:50     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 05/11] kselftest: arm64: mangle_pstate_ssbs_regs Cristian Marussi
2019-08-13 16:25   ` Dave Martin
2019-08-29 15:35     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic Cristian Marussi
2019-08-13 16:25   ` Dave Martin
2019-08-30 14:29     ` Cristian Marussi
2019-09-04 10:05       ` Dave Martin
2019-09-04 10:37         ` Cristian Marussi
2019-09-04 10:47           ` Dave Martin
2019-08-02 17:02 ` [PATCH v3 07/11] kselftest: arm64: fake_sigreturn_bad_size_for_magic0 Cristian Marussi
2019-08-13 16:25   ` Dave Martin
2019-08-30 14:49     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 08/11] kselftest: arm64: fake_sigreturn_missing_fpsimd Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 14:55     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 15:11     ` Cristian Marussi
2019-08-02 17:02 ` [PATCH v3 10/11] kselftest: arm64: fake_sigreturn_bad_size Cristian Marussi
2019-08-13 16:26   ` Dave Martin
2019-08-30 15:21     ` Cristian Marussi
2019-08-02 17:03 ` [PATCH v3 11/11] kselftest: arm64: fake_sigreturn_misaligned_sp Cristian Marussi
2019-08-07 16:04   ` Cristian Marussi
2019-08-13 16:28     ` Dave Martin
2019-08-30 15:22       ` Cristian Marussi
2019-08-13 16:27   ` Dave Martin
2019-08-30 16:33     ` Cristian Marussi
2019-08-13 16:22 ` [PATCH v3 00/11] Add arm64/signal initial kselftest support Dave Martin
2019-08-30 16:40   ` Cristian Marussi [this message]
2019-09-02 10:53     ` Dave Martin
2019-09-02 11:30       ` Cristian Marussi

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=b4c9cb22-6e08-8096-addb-4ac267fc0a84@arm.com \
    --to=cristian.marussi@arm$(echo .)com \
    --cc=Dave.Martin@arm$(echo .)com \
    --cc=andreyknvl@google$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-kselftest@vger$(echo .)kernel.org \
    --cc=shuah@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox