Re: [PATCH 1/2] misc:ad525x_dpot: Fix missing blank after declaration coding style issue in ad525x_dpot.c

From: Joe Perches
Date: Tue Dec 16 2014 - 13:00:31 EST


On Tue, 2014-12-16 at 23:05 +0530, Mohammad Jamal wrote:
> This is a patch to ad525x_dpot.c file that fixes up a missing blank line after declaration warning found by checkpatch.pl issue

Hello Mohammad.

Thanks for the patch.

Here are some small comments:

Please run your patches through scripts/checkpatch.pl
before sending them.

This submission has an overly long commit message line.
Please limit line lengths to ~70 characters.

The "This is a patch to <foo> that fixes up " is not useful.
The "found by checkpatch.pl issue" is not particularly useful.

A simpler form might have used:

Subject: [PATCH] ad525x_dpot: Add a blank line after declaration

with a commit log of somthing like:

Use a more common kernel style.

Signed-off-by: Mohammad Jamal <md.jamalmohiuddin@xxxxxxxxx>

> Signed-off-by: Mohammad Jamal<md.jamalmohiuddin@xxxxxxxxx>

Add a space between your name and the open angle bracket
in the email address too please.

Sometimes, it's good to do all the similar types of changes
at the same time instead of multiple patches for trivial
bits.

Using:

$ ./scripts/checkpatch.pl -f --strict --types=line_spacing,braces --fix-inplace drivers/misc/ad525x_dpot.c

produces this:

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index a43053d..ddfe38f 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -176,6 +176,7 @@ static s32 dpot_read_i2c(struct dpot_data *dpot, u8 reg)
{
int value;
unsigned ctrl = 0;
+
switch (dpot->uid) {
case DPOT_UID(AD5246_ID):
case DPOT_UID(AD5247_ID):
@@ -427,7 +428,6 @@ static ssize_t sysfs_show_reg(struct device *dev,
test_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask) ?
"enabled" : "disabled");

-
mutex_lock(&data->update_lock);
value = dpot_read(data, reg);
mutex_unlock(&data->update_lock);
@@ -763,7 +763,6 @@ int ad_dpot_remove(struct device *dev)
}
EXPORT_SYMBOL(ad_dpot_remove);

-
MODULE_AUTHOR("Chris Verges <chrisv@xxxxxxxxxxxxxxxxxx>, "
"Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>");
MODULE_DESCRIPTION("Digital potentiometer driver");

This could be submitted with some commit message like:

Subject: [PATCH] ad525x_dpot: Add & remove blank lines

Use a more common kernel style.

Signed-off-by: <you>

cheers, Joe


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/