Re: [PATCH] drivers: input: joystick: Add PSX (Play Station 1/2) pad with SPI driver.

From: Antonio Ospite
Date: Wed Apr 26 2017 - 05:46:19 EST


On Tue, 25 Apr 2017 23:44:22 +0900
AZO <typesylph@xxxxxxxxx> wrote:

> PSX pads can be connected directry SPI bus.
^
"directly"

and add "to the" before SPI.

>
> Signed-off-by: AZO <typesylph@xxxxxxxxx>
> ---

Hi,

I haven't looked at the code but I have some general comments.

When submitting another iteration of a patch it's common practice to
mention the version in the Subject (e.g. [PATCH v3] ...), you can use:

git format-patch --subject-prefix='PATCH vX' ...

It is also useful to provide a changelog of the versions, so that
reviewers can see what the changes between the current patch and
the previous versions are. This changelog goes after the '---'
separator and before the diffstat, this way git will ignore it when the
patch is applied, it's meant for the review process and it doesn't need
to go in the commit message of the final version.

Further annotations can also go after the '---' separator.

For more details look at Documentation/SubmittingPatches in the linux
kernel tree.

Also try to use a subject line consistent with the subsystem, by
looking at the history of the files in the same directory.

In this case it could be something like:

Input: psxpad-spi - Add PSX (Play Station 1/2) pad SPI driver

Finally, script/checkpatch.pl suggests some minor issues, I tried with:

./scripts/checkpatch.pl --ignore LONG_LINE,LONG_LINE_COMMENT your.patch

You can ignore some of them, and motivate your decision in an
annotation.

Also try to comment when you do not agree with the reviewer or cannot
comply (for example about using an interrupt instead of polling).

It's fine if your English is not perfect yet, don't let that stop you :)

Ciao ciao,
Antonio

--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?