[PATCH 0/6] staging: goldfish: fix coding style warnings

[PATCH 0/6] staging: goldfish: fix coding style warnings

[Loic Pefferkorn]

Hi,

This patchset (for next-20140902) fixes several coding style warnings found by checkpatch.pl:
-CHECK: Please don't use multiple blank lines
-CHECK: No space is necessary after a cast
-CHECK: spinlock_t definition without comment
-CHECK: Alignment should match open parenthesis
-CHECK: struct mutex definition without comment


Loic Pefferkorn (6):
  staging: goldfish: suppress consecutive blank lines
  staging: goldfish: remove useless space after a cast
  staging: goldfish: document spinlock usage
  staging: goldfish: fix alignment to match open parenthesis
  staging: goldfish: document mutex usage
  staging: goldfish: avoid multiple assignments

 drivers/staging/goldfish/goldfish_audio.c | 25 ++++++++----------
 drivers/staging/goldfish/goldfish_nand.c  | 43 ++++++++++++++++---------------
 2 files changed, 33 insertions(+), 35 deletions(-)

-- 
2.0.4

[PATCH 1/6] staging: goldfish: suppress consecutive blank lines

[Loic Pefferkorn]

Coding style: suppress consecutive blank lines

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_audio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/goldfish/goldfish_audio.c b/drivers/staging/goldfish/goldfish_audio.c
index c89d0b8..23a206d 100644
--- a/drivers/staging/goldfish/goldfish_audio.c
+++ b/drivers/staging/goldfish/goldfish_audio.c
@@ -108,10 +108,8 @@ enum {
 					  AUDIO_INT_READ_BUFFER_FULL,
 };
 
-
 static atomic_t open_count = ATOMIC_INIT(0);
 
-
 static ssize_t goldfish_audio_read(struct file *fp, char __user *buf,
 						size_t count, loff_t *pos)
 {
-- 
2.0.4

[PATCH 2/6] staging: goldfish: remove useless space after a cast

[Loic Pefferkorn]

Coding style: remove useless space after a cast

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c
index 092604c..aac8e20 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -340,7 +340,7 @@ static int goldfish_nand_init_device(struct platform_device *pdev,
 			       result, name_len);
 		return -ENODEV;
 	}
-	((char *) mtd->name)[name_len] = '\0';
+	((char *)mtd->name)[name_len] = '\0';
 
 	/* Setup the MTD structure */
 	mtd->type = MTD_NANDFLASH;
-- 
2.0.4

[PATCH 3/6] staging: goldfish: document spinlock usage

[Loic Pefferkorn]

Coding style: document spinlock usage

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/goldfish/goldfish_audio.c b/drivers/staging/goldfish/goldfish_audio.c
index 23a206d..ab723ab 100644
--- a/drivers/staging/goldfish/goldfish_audio.c
+++ b/drivers/staging/goldfish/goldfish_audio.c
@@ -36,7 +36,7 @@ MODULE_VERSION("1.0");
 struct goldfish_audio {
 	char __iomem *reg_base;
 	int irq;
-	spinlock_t lock;
+	spinlock_t lock;                /* Serialize access to device */
 	wait_queue_head_t wait;
 
 	char __iomem *buffer_virt;      /* combined buffer virtual address */
-- 
2.0.4

[PATCH 4/6] staging: goldfish: fix alignment to match open parenthesis

[Loic Pefferkorn]

Coding style: fix alignment to match open parenthesis

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_audio.c | 21 +++++++++---------
 drivers/staging/goldfish/goldfish_nand.c  | 36 +++++++++++++++----------------
 2 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/goldfish/goldfish_audio.c b/drivers/staging/goldfish/goldfish_audio.c
index ab723ab..c858876 100644
--- a/drivers/staging/goldfish/goldfish_audio.c
+++ b/drivers/staging/goldfish/goldfish_audio.c
@@ -111,7 +111,7 @@ enum {
 static atomic_t open_count = ATOMIC_INIT(0);
 
 static ssize_t goldfish_audio_read(struct file *fp, char __user *buf,
-						size_t count, loff_t *pos)
+				   size_t count, loff_t *pos)
 {
 	struct goldfish_audio *data = fp->private_data;
 	int length;
@@ -124,11 +124,10 @@ static ssize_t goldfish_audio_read(struct file *fp, char __user *buf,
 		length = (count > READ_BUFFER_SIZE ? READ_BUFFER_SIZE : count);
 		AUDIO_WRITE(data, AUDIO_START_READ, length);
 
-		wait_event_interruptible(data->wait,
-			(data->buffer_status & AUDIO_INT_READ_BUFFER_FULL));
+		wait_event_interruptible(data->wait, (data->buffer_status &
+					 AUDIO_INT_READ_BUFFER_FULL));
 
-		length = AUDIO_READ(data,
-						AUDIO_READ_BUFFER_AVAILABLE);
+		length = AUDIO_READ(data, AUDIO_READ_BUFFER_AVAILABLE);
 
 		/* copy data to user space */
 		if (copy_to_user(buf, data->read_buffer, length))
@@ -142,7 +141,7 @@ static ssize_t goldfish_audio_read(struct file *fp, char __user *buf,
 }
 
 static ssize_t goldfish_audio_write(struct file *fp, const char __user *buf,
-						 size_t count, loff_t *pos)
+				    size_t count, loff_t *pos)
 {
 	struct goldfish_audio *data = fp->private_data;
 	unsigned long irq_flags;
@@ -216,7 +215,7 @@ static int goldfish_audio_release(struct inode *ip, struct file *fp)
 }
 
 static long goldfish_audio_ioctl(struct file *fp, unsigned int cmd,
-							unsigned long arg)
+				 unsigned long arg)
 {
 	/* temporary workaround, until we switch to the ALSA API */
 	if (cmd == 315)
@@ -305,7 +304,7 @@ static int goldfish_audio_probe(struct platform_device *pdev)
 	data->read_buffer = data->buffer_virt + 2 * WRITE_BUFFER_SIZE;
 
 	ret = devm_request_irq(&pdev->dev, data->irq, goldfish_audio_interrupt,
-					IRQF_SHARED, pdev->name, data);
+			       IRQF_SHARED, pdev->name, data);
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq failed\n");
 		return ret;
@@ -320,18 +319,18 @@ static int goldfish_audio_probe(struct platform_device *pdev)
 	}
 
 	AUDIO_WRITE64(data, AUDIO_SET_WRITE_BUFFER_1,
-				AUDIO_SET_WRITE_BUFFER_1_HIGH, buf_addr);
+		      AUDIO_SET_WRITE_BUFFER_1_HIGH, buf_addr);
 	buf_addr += WRITE_BUFFER_SIZE;
 
 	AUDIO_WRITE64(data, AUDIO_SET_WRITE_BUFFER_2,
-				AUDIO_SET_WRITE_BUFFER_2_HIGH, buf_addr);
+		      AUDIO_SET_WRITE_BUFFER_2_HIGH, buf_addr);
 
 	buf_addr += WRITE_BUFFER_SIZE;
 
 	data->read_supported = AUDIO_READ(data, AUDIO_READ_SUPPORTED);
 	if (data->read_supported)
 		AUDIO_WRITE64(data, AUDIO_SET_READ_BUFFER,
-				AUDIO_SET_READ_BUFFER_HIGH, buf_addr);
+			      AUDIO_SET_READ_BUFFER_HIGH, buf_addr);
 
 	audio_data = data;
 	return 0;
diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c
index aac8e20..3f13ef0 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -39,8 +39,8 @@ struct goldfish_nand {
 };
 
 static u32 goldfish_nand_cmd_with_params(struct mtd_info *mtd,
-			enum nand_cmd cmd, u64 addr, u32 len,
-			void *ptr, u32 *rv)
+					 enum nand_cmd cmd, u64 addr, u32 len,
+					 void *ptr, u32 *rv)
 {
 	u32 cmdp;
 	struct goldfish_nand *nand = mtd->priv;
@@ -74,7 +74,7 @@ static u32 goldfish_nand_cmd_with_params(struct mtd_info *mtd,
 }
 
 static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
-				u64 addr, u32 len, void *ptr)
+			     u64 addr, u32 len, void *ptr)
 {
 	struct goldfish_nand *nand = mtd->priv;
 	u32 rv;
@@ -113,7 +113,7 @@ static int goldfish_nand_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 	if (goldfish_nand_cmd(mtd, NAND_CMD_ERASE, ofs, len, NULL) != len) {
 		pr_err("goldfish_nand_erase: erase failed, start %llx, len %x, dev_size %llx, erase_size %x\n",
-			ofs, len, mtd->size, mtd->erasesize);
+		       ofs, len, mtd->size, mtd->erasesize);
 		return -EIO;
 	}
 
@@ -124,12 +124,12 @@ static int goldfish_nand_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 invalid_arg:
 	pr_err("goldfish_nand_erase: invalid erase, start %llx, len %x, dev_size %llx, erase_size %x\n",
-		ofs, len, mtd->size, mtd->erasesize);
+	       ofs, len, mtd->size, mtd->erasesize);
 	return -EINVAL;
 }
 
 static int goldfish_nand_read_oob(struct mtd_info *mtd, loff_t ofs,
-				struct mtd_oob_ops *ops)
+				  struct mtd_oob_ops *ops)
 {
 	u32 rem;
 
@@ -156,12 +156,12 @@ static int goldfish_nand_read_oob(struct mtd_info *mtd, loff_t ofs,
 
 invalid_arg:
 	pr_err("goldfish_nand_read_oob: invalid read, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n",
-		ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize);
+	       ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
 static int goldfish_nand_write_oob(struct mtd_info *mtd, loff_t ofs,
-				struct mtd_oob_ops *ops)
+				   struct mtd_oob_ops *ops)
 {
 	u32 rem;
 
@@ -188,12 +188,12 @@ static int goldfish_nand_write_oob(struct mtd_info *mtd, loff_t ofs,
 
 invalid_arg:
 	pr_err("goldfish_nand_write_oob: invalid write, start %llx, len %zx, ooblen %zx, dev_size %llx, write_size %x\n",
-		ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize);
+	       ofs, ops->len, ops->ooblen, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
 static int goldfish_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
-				size_t *retlen, u_char *buf)
+			      size_t *retlen, u_char *buf)
 {
 	u32 rem;
 
@@ -210,12 +210,12 @@ static int goldfish_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
 
 invalid_arg:
 	pr_err("goldfish_nand_read: invalid read, start %llx, len %zx, dev_size %llx, write_size %x\n",
-		from, len, mtd->size, mtd->writesize);
+	       from, len, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
 static int goldfish_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
-				size_t *retlen, const u_char *buf)
+			       size_t *retlen, const u_char *buf)
 {
 	u32 rem;
 
@@ -232,7 +232,7 @@ static int goldfish_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 invalid_arg:
 	pr_err("goldfish_nand_write: invalid write, start %llx, len %zx, dev_size %llx, write_size %x\n",
-		to, len, mtd->size, mtd->writesize);
+	       to, len, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
@@ -253,7 +253,7 @@ static int goldfish_nand_block_isbad(struct mtd_info *mtd, loff_t ofs)
 
 invalid_arg:
 	pr_err("goldfish_nand_block_isbad: invalid arg, ofs %llx, dev_size %llx, write_size %x\n",
-		ofs, mtd->size, mtd->writesize);
+	       ofs, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
@@ -276,12 +276,12 @@ static int goldfish_nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 invalid_arg:
 	pr_err("goldfish_nand_block_markbad: invalid arg, ofs %llx, dev_size %llx, write_size %x\n",
-		ofs, mtd->size, mtd->writesize);
+	       ofs, mtd->size, mtd->writesize);
 	return -EINVAL;
 }
 
 static int nand_setup_cmd_params(struct platform_device *pdev,
-						struct goldfish_nand *nand)
+				 struct goldfish_nand *nand)
 {
 	u64 paddr;
 	unsigned char __iomem  *base = nand->base;
@@ -298,7 +298,7 @@ static int nand_setup_cmd_params(struct platform_device *pdev,
 }
 
 static int goldfish_nand_init_device(struct platform_device *pdev,
-					struct goldfish_nand *nand, int id)
+				     struct goldfish_nand *nand, int id)
 {
 	u32 name_len;
 	u32 result;
@@ -333,7 +333,7 @@ static int goldfish_nand_init_device(struct platform_device *pdev,
 		return -ENOMEM;
 
 	result = goldfish_nand_cmd(mtd, NAND_CMD_GET_DEV_NAME, 0, name_len,
-									name);
+				   name);
 	if (result != name_len) {
 		dev_err(&pdev->dev,
 			"goldfish_nand_init_device failed to get dev name %d != %d\n",
-- 
2.0.4

[PATCH 5/6] staging: goldfish: document mutex usage

[Loic Pefferkorn]

Coding style: document mutex usage

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c
index 3f13ef0..f177911 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -31,7 +31,7 @@
 #include "goldfish_nand_reg.h"
 
 struct goldfish_nand {
-	struct mutex            lock;
+	struct mutex            lock;	      /* Serialize device operations */
 	unsigned char __iomem  *base;
 	struct cmd_params       *cmd_params;
 	size_t                  mtd_count;
-- 
2.0.4

[PATCH 6/6] staging: goldfish: avoid multiple assignments

[Loic Pefferkorn]

Coding style: avoid multiple assignments

Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
---
 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c
index f177911..17cd439 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -328,9 +328,10 @@ static int goldfish_nand_init_device(struct platform_device *pdev,
 
 	mtd->priv = nand;
 
-	mtd->name = name = devm_kzalloc(&pdev->dev, name_len + 1, GFP_KERNEL);
+	name = devm_kzalloc(&pdev->dev, name_len + 1, GFP_KERNEL);
 	if (name == NULL)
 		return -ENOMEM;
+	mtd->name = name;
 
 	result = goldfish_nand_cmd(mtd, NAND_CMD_GET_DEV_NAME, 0, name_len,
 				   name);
-- 
2.0.4

Re: [PATCH 3/6] staging: goldfish: document spinlock usage

[One Thousand Gnomes]

On Wed,  3 Sep 2014 13:13:44 +0200
Loic Pefferkorn <loic@loicp.eu> wrote:

> Coding style: document spinlock usage
> 
> Signed-off-by: Loic Pefferkorn <loic@loicp.eu>
> ---
>  drivers/staging/goldfish/goldfish_audio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/goldfish/goldfish_audio.c b/drivers/staging/goldfish/goldfish_audio.c
> index 23a206d..ab723ab 100644
> --- a/drivers/staging/goldfish/goldfish_audio.c
> +++ b/drivers/staging/goldfish/goldfish_audio.c
> @@ -36,7 +36,7 @@ MODULE_VERSION("1.0");
>  struct goldfish_audio {
>  	char __iomem *reg_base;
>  	int irq;
> -	spinlock_t lock;
> +	spinlock_t lock;                /* Serialize access to device */

This tells the reader nothing. It's good to document locking models but

- you lock data not code (which is a detail a lot of programmers get wrong
			  in th design stage too)
- you need to document what objects are protected by the lock


So it should tell the reader what the lock must be held to do

If you look at the audio code it actually protects the status field and
status register of the "hardware"

Re: [PATCH 3/6] staging: goldfish: document spinlock usage

[Loic Pefferkorn]

On Wed, Sep 03, 2014 at 01:13:19PM +0100, One Thousand Gnomes wrote:
> 
> This tells the reader nothing. It's good to document locking models but
> 
> - you lock data not code (which is a detail a lot of programmers get wrong
> 			  in th design stage too)
> - you need to document what objects are protected by the lock
> 
> 
> So it should tell the reader what the lock must be held to do
> 
> If you look at the audio code it actually protects the status field and
> status register of the "hardware"

Hello,

Thank you for the feedback, I will send a v2.

-- 
Cheers,
Loïc