staging: iio: cleanup ring_sw.c

PATCH In review Cong Ding 3 participants 4 messages Dec 18, 2012 – Dec 19, 2012 1 message not in the archive

Subsystems (3)

Staging IIO Staging - industrial io

lore

Share & tools

lore

Cong Ding

author

2012-12-18 23:44 UTC #0 lore raw

PATCH 1/1 staging: iio: cleanup ring_sw.c

clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line issue.

Signed-off-by Cong Ding <[email protected]>

---
 drivers/staging/iio/ring_sw.c |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

drivers/staging/iio/ring_sw.c +14 ~~-11~~

index 3a45f9a..4db1cc3 100644
@@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 		 * as long as the read pointer is valid before this
 		 * passes it - guaranteed as set later in this function.
 		 */
-		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
+		ring->half_p = ring->data -
+			ring->buf.length*ring->buf.bytes_per_datum/2;
 	}
 	/* Copy data to where ever the current write pointer says */
 	memcpy(ring->write_p, data, ring->buf.bytes_per_datum);

@@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	else if (ring->write_p == ring->read_p) {
 		change_test_ptr = ring->read_p;
 		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
-		if (temp_ptr
-		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
+		if (temp_ptr == ring->data +
+		    ring->buf.length*ring->buf.bytes_per_datum) {
 			temp_ptr = ring->data;
 		}
 		/* We are moving pointer on one because the ring is full.  Any

@@ -127,7 +128,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	 * simultaneous read */
 	/* Also need to use loop count to ensure this only happens once */
 	ring->half_p += ring->buf.bytes_per_datum;
-	if (ring->half_p == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
+	if (ring->half_p == ring->data +
+			ring->buf.length*ring->buf.bytes_per_datum)
 		ring->half_p = ring->data;
 	if (ring->half_p == ring->read_p) {
 		ring->buf.stufftoread = true;

@@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (n % ring->buf.bytes_per_datum) {
 		ret = -EINVAL;
 		printk(KERN_INFO "Ring buffer read request not whole number of"
-		       "samples: Request bytes %zd, Current bytes per datum %d\n",
+		       "samples: Request bytes %zd, Current bytes per"
+		       "datum %d\n",
 		       n, ring->buf.bytes_per_datum);
 		goto error_ret;
 	}

@@ -193,7 +196,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_write_p >= initial_read_p)
 		data_available = initial_write_p - initial_read_p;
 	else
-		data_available = buffer_size - (initial_read_p - initial_write_p);
+		data_available = buffer_size -
+			(initial_read_p - initial_write_p);
 
 	if (data_available < bytes_to_rip)
 		bytes_to_rip = data_available;

@@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
 		max_copied = ring->data + buffer_size - initial_read_p;
 		memcpy(data, initial_read_p, max_copied);
-		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
+		memcpy(data + max_copied,
+				ring->data, bytes_to_rip - max_copied);
 		end_read_p = ring->data + bytes_to_rip - max_copied;
 	} else {
 		memcpy(data, initial_read_p, bytes_to_rip);

@@ -251,12 +256,10 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 error_free_data_cpy:
 	kfree(data);
 error_ret:
-
 	return ret;
 }
 
-static int iio_store_to_sw_rb(struct iio_buffer *r,
-			      u8 *data)
+static int iio_store_to_sw_rb(struct iio_buffer *r, u8 *data)
 {
 	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
 	return iio_store_to_sw_ring(ring, data);

@@ -343,7 +346,7 @@ struct iio_buffer *iio_sw_rb_allocate(struct iio_dev *indio_dev)
 	struct iio_buffer *buf;
 	struct iio_sw_ring_buffer *ring;
 
-	ring = kzalloc(sizeof *ring, GFP_KERNEL);
+	ring = kzalloc(sizeof(*ring), GFP_KERNEL);
 	if (!ring)
 		return NULL;
 	ring->update_needed = true;

Signature · 1 line

1.7.10.4

Dan Carpenter

2012-12-19 06:42 UTC #1 lore raw

Quoted 13 lines · Cong Ding

clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line issue.
Signed-off-by Cong Ding <[email protected]>
---
 drivers/staging/iio/ring_sw.c |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/staging/iio/ring_sw.c +0 -0
index 3a45f9a..4db1cc3 100644

drivers/staging/iio/ring_sw.c static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring, · line 81

Quoted 6 lines

drivers/staging/iio/ring_sw.c +2 -1

@@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 		 * as long as the read pointer is valid before this
 		 * passes it - guaranteed as set later in this function.
 		 */
-		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
+		ring->half_p = ring->data -
+			ring->buf.length*ring->buf.bytes_per_datum/2;

Please put spaces around the '*' and '/' characters.

			ring->buf.length * ring->buf.bytes_per_datum / 2;

drivers/staging/iio/ring_sw.c static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring, · line 117

Quoted 11 lines

drivers/staging/iio/ring_sw.c +2 -2

 	}
 	/* Copy data to where ever the current write pointer says */
 	memcpy(ring->write_p, data, ring->buf.bytes_per_datum);

@@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	else if (ring->write_p == ring->read_p) {
 		change_test_ptr = ring->read_p;
 		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
-		if (temp_ptr
-		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
+		if (temp_ptr == ring->data +
+		    ring->buf.length*ring->buf.bytes_per_datum) {

This needs spaces as well. It might look cleaner if you broke it up like this:

		if (temp_ptr == ring->data + ring->buf.length *
				ring->buf.bytes_per_datum) {

drivers/staging/iio/ring_sw.c static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring, · line 132

Quoted 10 lines

drivers/staging/iio/ring_sw.c +2 -1

 			temp_ptr = ring->data;
 		}
 		/* We are moving pointer on one because the ring is full.  Any

@@ -127,7 +128,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	 * simultaneous read */
 	/* Also need to use loop count to ensure this only happens once */
 	ring->half_p += ring->buf.bytes_per_datum;
-	if (ring->half_p == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
+	if (ring->half_p == ring->data +
+			ring->buf.length*ring->buf.bytes_per_datum)

Same.

drivers/staging/iio/ring_sw.c static int iio_read_first_n_sw_rb(struct iio_buffer *r, · line 159

Quoted 10 lines

drivers/staging/iio/ring_sw.c +2 -1

 		ring->half_p = ring->data;
 	if (ring->half_p == ring->read_p) {
 		ring->buf.stufftoread = true;

@@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (n % ring->buf.bytes_per_datum) {
 		ret = -EINVAL;
 		printk(KERN_INFO "Ring buffer read request not whole number of"
-		       "samples: Request bytes %zd, Current bytes per datum %d\n",
+		       "samples: Request bytes %zd, Current bytes per"
+		       "datum %d\n",

No space between "of" and "samples" and also "per" and "datum".

The warning here is that the print should be on one line instead of two. But actually that is a decision for the maintainer. Checkpatch.pl is not the king of us, we do not have to do what it says.

Quoted 14 lines

drivers/staging/iio/ring_sw.c +2 -1

 		       n, ring->buf.bytes_per_datum);
 		goto error_ret;
 	}

@@ -193,7 +196,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_write_p >= initial_read_p)
 		data_available = initial_write_p - initial_read_p;
 	else
-		data_available = buffer_size - (initial_read_p - initial_write_p);
+		data_available = buffer_size -
+			(initial_read_p - initial_write_p);
 
 	if (data_available < bytes_to_rip)
 		bytes_to_rip = data_available;

drivers/staging/iio/ring_sw.c static int iio_read_first_n_sw_rb(struct iio_buffer *r, · line 209

Quoted 6 lines

drivers/staging/iio/ring_sw.c +2 -1

@@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
 		max_copied = ring->data + buffer_size - initial_read_p;
 		memcpy(data, initial_read_p, max_copied);
-		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
+		memcpy(data + max_copied,
+				ring->data, bytes_to_rip - max_copied);

Line it up like this:

		memcpy(data + max_copied, ring->data,
		       bytes_to_rip - max_copied);

[tab][tab][space][space][space][space][space][space][space]bytes_

regards,
dan carpenter

Joe Perches

2012-12-19 07:02 UTC #2 lore raw

Quoted 3 lines · Dan Carpenter

Quoted 2 lines · Cong Ding
clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line issue.

[]

Quoted 1 line

drivers/staging/iio/ring_sw.c
Quoted 1 line
drivers/staging/iio/ring_sw.c +0 -0

[]

Quoted 10 lines

drivers/staging/iio/ring_sw.c static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring, · line 81
Quoted 7 lines
drivers/staging/iio/ring_sw.c +2 -1
@@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 		 * as long as the read pointer is valid before this
 		 * passes it - guaranteed as set later in this function.
 		 */
-		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
+		ring->half_p = ring->data -
+			ring->buf.length*ring->buf.bytes_per_datum/2;
Please put spaces around the '*' and '/' characters.
			ring->buf.length * ring->buf.bytes_per_datum / 2;

or maybe add a helper like ring_datum_size(ring):

size_t ring_datum_size(const struct iio_sw_ring_buffer *ring)

{
	return ring->buf.length * ring->buf.bytes_per_datum;
}

so this becomes

	ring->half_p = ring->data - ring_datum_size(ring) / 2;

Quoted 12 lines

drivers/staging/iio/ring_sw.c static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring, · line 117
Quoted 8 lines
drivers/staging/iio/ring_sw.c +2 -2
@@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	else if (ring->write_p == ring->read_p) {
 		change_test_ptr = ring->read_p;
 		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
-		if (temp_ptr
-		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
+		if (temp_ptr == ring->data +
+		    ring->buf.length*ring->buf.bytes_per_datum) {
This needs spaces as well. It might look cleaner if you broke it up like this:

Quoted 2 lines

		if (temp_ptr == ring->data + ring->buf.length *
				ring->buf.bytes_per_datum) {

		if (temp_ptr == ring->data + ring_datum_size(ring)) {

etc...

Quoted 10 lines

drivers/staging/iio/ring_sw.c static int iio_read_first_n_sw_rb(struct iio_buffer *r, · line 159
Quoted 7 lines
drivers/staging/iio/ring_sw.c +2 -1
@@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (n % ring->buf.bytes_per_datum) {
 		ret = -EINVAL;
 		printk(KERN_INFO "Ring buffer read request not whole number of"
-		       "samples: Request bytes %zd, Current bytes per datum %d\n",
+		       "samples: Request bytes %zd, Current bytes per"
+		       "datum %d\n",
No space between "of" and "samples" and also "per" and "datum".

Quoted 4 lines

The warning here is that the print should be on one line instead of two. But actually that is a decision for the maintainer. Checkpatch.pl is not the king of us, we do not have to do what it says.

Yup.

It is nicer though to coalesce formats so that it's easier to grep for strings and substrings.

Quoted 9 lines

drivers/staging/iio/ring_sw.c static int iio_read_first_n_sw_rb(struct iio_buffer *r, · line 209

Quoted 7 lines

drivers/staging/iio/ring_sw.c +2 -1

@@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
 		max_copied = ring->data + buffer_size - initial_read_p;
 		memcpy(data, initial_read_p, max_copied);
-		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
+		memcpy(data + max_copied,
+				ring->data, bytes_to_rip - max_copied);

Line it up like this:

Quoted 5 lines

		memcpy(data + max_copied, ring->data,
		       bytes_to_rip - max_copied);
[tab][tab][space][space][space][space][space][space][space]bytes_

checkpatch --strict emits alignment messages

or not worry about 80 columns in this case.

A message that is not in the archive (sent off-list or not yet received): [email protected]

Joe Perches

2012-12-19 07:16 UTC #4 lore raw

Quoted 2 lines · Jonathan Cameron

Err. This whole file is going away in the coming cycle anyway so I am not going to take cleanup patches for it.

That's the best kind of staging cleanup.