Date:   Mon,  4 Dec 2017 21:40:10 +0100
From:   Tomas Marek <marek_tomas@...trum.cz>
To:     gregkh@...uxfoundation.org
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        joe@...ches.com, dan.carpenter@...cle.com, marek_tomas@...trum.cz
Subject: [PATCH v2] Staging: pi433: fix brace coding style issues in pi433_if.c

This patch fix several brace on next line, braces not necessary, space
around =/<, and space before/after open/close parenthesis coding style
errors find by checkpatch in pi433_if.c.

In addition, the interrupt routine DIO0_irq_handler logic is updated:
- use 'switch' statement instead of 'if/else if' combination for the
  sake of readability, and
- use dev_dbg_ratelimited instead of dev_dbg to avoid message flooding.

Signed-off-by: Tomas Marek <marek_tomas@...trum.cz>
---
Changes in v2:
  - DIO0_irq_handler updated - 'if/else if' replaced by 'switch' and
    'dev_dbg_ratelimited' used instead of 'dev_dbg' according to Joe
    Perches suggestion.
  - The removal of braces around SET_CHECKED() reverted - caused syntax
    error and is addressed by another patch
    "[PATCHv2] staging: pi433: pi433_if.c remove SET_CHECKED macro".
---
 drivers/staging/pi433/pi433_if.c | 135 ++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 81 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2a205c6..3a333b8 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -132,23 +132,29 @@ struct pi433_instance {
 static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
 {
 	struct pi433_device *device = dev_id;
+	wait_queue_head_t *wq;
+	const char *msg = NULL;
 
-	if      (device->irq_state[DIO0] == DIO_PacketSent)
-	{
+	switch (device->irq_state[DIO0]) {
+	case DIO_PacketSent:
 		device->free_in_fifo = FIFO_SIZE;
-		dev_dbg(device->dev, "DIO0 irq: Packet sent\n");
-		wake_up_interruptible(&device->fifo_wait_queue);
-	}
-	else if (device->irq_state[DIO0] == DIO_Rssi_DIO0)
-	{
-		dev_dbg(device->dev, "DIO0 irq: RSSI level over threshold\n");
-		wake_up_interruptible(&device->rx_wait_queue);
-	}
-	else if (device->irq_state[DIO0] == DIO_PayloadReady)
-	{
-		dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
+		msg = "Packet sent";
+		wq = &device->fifo_wait_queue;
+		break;
+	case DIO_Rssi_DIO0:
+		msg = "RSSI level over threshold";
+		wq = &device->rx_wait_queue;
+		break;
+	case DIO_PayloadReady:
 		device->free_in_fifo = 0;
-		wake_up_interruptible(&device->fifo_wait_queue);
+		msg = "PayloadReady";
+		wq = &device->fifo_wait_queue;
+		break;
+	}
+
+	if (msg) {
+		dev_dbg_ratelimited(device->dev, "DIO0 irq: %s\n", msg);
+		wake_up_interruptible(wq);
 	}
 
 	return IRQ_HANDLED;
@@ -158,12 +164,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 {
 	struct pi433_device *device = dev_id;
 
-	if      (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1)
-	{
+	if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) {
 		device->free_in_fifo = FIFO_SIZE;
-	}
-	else if (device->irq_state[DIO1] == DIO_FifoLevel)
-	{
+	} else if (device->irq_state[DIO1] == DIO_FifoLevel) {
 		if (device->rx_active)	device->free_in_fifo = FIFO_THRESHOLD - 1;
 		else			device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1;
 	}
@@ -220,19 +223,14 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 
 	/* lengths */
 	SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
-	if (rx_cfg->enable_length_byte == optionOn)
-	{
+	if (rx_cfg->enable_length_byte == optionOn) {
 		SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
-	}
-	else if (rx_cfg->fixed_message_length != 0)
-	{
+	} else if (rx_cfg->fixed_message_length != 0) {
 		payload_length = rx_cfg->fixed_message_length;
 		if (rx_cfg->enable_length_byte  == optionOn) payload_length++;
 		if (rx_cfg->enable_address_filtering != filteringOff) payload_length++;
 		SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length));
-	}
-	else
-	{
+	} else {
 		SET_CHECKED(rf69_set_payload_length(dev->spi, 0));
 	}
 
@@ -241,8 +239,8 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	{
 		SET_CHECKED(rf69_set_sync_values(dev->spi, rx_cfg->sync_pattern));
 	}
-	if (rx_cfg->enable_address_filtering != filteringOff)
-	{
+
+	if (rx_cfg->enable_address_filtering != filteringOff) {
 		SET_CHECKED(rf69_set_node_address     (dev->spi, rx_cfg->node_address));
 		SET_CHECKED(rf69_set_broadcast_address(dev->spi, rx_cfg->broadcast_address));
 	}
@@ -341,8 +339,8 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	/* wait for any tx to finish */
 	dev_dbg(dev->dev,"rx: going to wait for any tx to finish");
 	retval = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active);
-	if(retval) /* wait was interrupted */
-	{
+	if (retval) {
+		/* wait was interrupted */
 		dev->interrupt_rx_allowed = true;
 		wake_up_interruptible(&dev->tx_wait_queue);
 		return retval;
@@ -359,8 +357,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 		return retval;
 
 	/* now check RSSI, if low wait for getting high (RSSI interrupt) */
-	while ( !rf69_get_flag(dev->spi, rssiExceededThreshold) )
-	{
+	while (!rf69_get_flag(dev->spi, rssiExceededThreshold)) {
 		/* allow tx to interrupt us while waiting for high RSSI */
 		dev->interrupt_rx_allowed = true;
 		wake_up_interruptible(&dev->tx_wait_queue);
@@ -383,25 +380,20 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	irq_set_irq_type(dev->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
 
 	/* fixed or unlimited length? */
-	if (dev->rx_cfg.fixed_message_length != 0)
-	{
-		if (dev->rx_cfg.fixed_message_length > dev->rx_buffer_size)
-		{
+	if (dev->rx_cfg.fixed_message_length != 0) {
+		if (dev->rx_cfg.fixed_message_length > dev->rx_buffer_size) {
 			retval = -1;
 			goto abort;
 		}
 		bytes_total = dev->rx_cfg.fixed_message_length;
 		dev_dbg(dev->dev,"rx: msg len set to %d by fixed length", bytes_total);
-	}
-	else
-	{
+	} else {
 		bytes_total = dev->rx_buffer_size;
 		dev_dbg(dev->dev, "rx: msg len set to %d as requested by read", bytes_total);
 	}
 
 	/* length byte enabled? */
-	if (dev->rx_cfg.enable_length_byte == optionOn)
-	{
+	if (dev->rx_cfg.enable_length_byte == optionOn) {
 		retval = wait_event_interruptible(dev->fifo_wait_queue,
 						  dev->free_in_fifo < FIFO_SIZE);
 		if (retval) goto abort; /* wait was interrupted */
@@ -416,8 +408,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	}
 
 	/* address byte enabled? */
-	if (dev->rx_cfg.enable_address_filtering != filteringOff)
-	{
+	if (dev->rx_cfg.enable_address_filtering != filteringOff) {
 		u8 dummy;
 
 		bytes_total--;
@@ -432,10 +423,8 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	}
 
 	/* get payload */
-	while (dev->rx_position < bytes_total)
-	{
-		if ( !rf69_get_flag(dev->spi, payloadReady) )
-		{
+	while (dev->rx_position < bytes_total) {
+		if (!rf69_get_flag(dev->spi, payloadReady)) {
 			retval = wait_event_interruptible(dev->fifo_wait_queue,
 							  dev->free_in_fifo < FIFO_SIZE);
 			if (retval) goto abort; /* wait was interrupted */
@@ -489,8 +478,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	int    position, repetitions;
 	int    retval;
 
-	while (1)
-	{
+	while (1) {
 		/* wait for fifo to be populated or for request to terminate*/
 		dev_dbg(device->dev, "thread: going to wait for new messages");
 		wait_event_interruptible(device->tx_wait_queue,
@@ -565,8 +553,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 		disable_irq(device->irq_num[DIO0]);
 		device->tx_active = true;
 
-		if (device->rx_active && rx_interrupted == false)
-		{
+		if (device->rx_active && rx_interrupted == false) {
 			/* rx is currently waiting for a telegram;
 			 * we need to set the radio module to standby
 			 */
@@ -607,19 +594,17 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 		device->free_in_fifo = FIFO_SIZE;
 		position = 0;
 		repetitions = tx_cfg.repetitions;
-		while( (repetitions > 0) && (size > position) )
-		{
-			if ( (size - position) > device->free_in_fifo)
-			{	/* msg to big for fifo - take a part */
+		while ((repetitions > 0) && (size > position)) {
+			if ((size - position) > device->free_in_fifo) {
+				/* msg to big for fifo - take a part */
 				int temp = device->free_in_fifo;
 				device->free_in_fifo = 0;
 				rf69_write_fifo(spi,
 				                &buffer[position],
 				                temp);
 				position +=temp;
-			}
-			else
-			{	/* msg fits into fifo - take all */
+			} else {
+				/* msg fits into fifo - take all */
 				device->free_in_fifo -= size;
 				repetitions--;
 				rf69_write_fifo(spi,
@@ -648,8 +633,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 		/* everything sent? */
 		if (kfifo_is_empty(&device->tx_fifo)) {
 abort:
-			if (rx_interrupted)
-			{
+			if (rx_interrupted) {
 				rx_interrupted = false;
 				pi433_start_rx(device);
 			}
@@ -678,13 +662,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 
 	/* just one read request at a time */
 	mutex_lock(&device->rx_lock);
-	if (device->rx_active)
-	{
+	if (device->rx_active) {
 		mutex_unlock(&device->rx_lock);
 		return -EAGAIN;
-	}
-	else
-	{
+	} else {
 		device->rx_active = true;
 		mutex_unlock(&device->rx_lock);
 	}
@@ -909,8 +890,7 @@ static int setup_GPIOs(struct pi433_device *device)
 		DIO1_irq_handler
 	};
 
-	for (i=0; i<NUM_DIO; i++)
-	{
+	for (i = 0; i < NUM_DIO; i++) {
 		/* "construct" name and get the gpio descriptor */
 		snprintf(name, sizeof(name), "DIO%d", i);
 		device->gpiod[i] = gpiod_get(&device->spi->dev, name, 0 /*GPIOD_IN*/);
@@ -923,12 +903,10 @@ static int setup_GPIOs(struct pi433_device *device)
 		if (device->gpiod[i] == ERR_PTR(-EBUSY))
 			dev_dbg(&device->spi->dev, "%s is busy.", name);
 
-		if ( IS_ERR(device->gpiod[i]) )
-		{
+		if (IS_ERR(device->gpiod[i])) {
 			retval = PTR_ERR(device->gpiod[i]);
 			/* release already allocated gpios */
-			for (i--; i>=0; i--)
-			{
+			for (i--; i >= 0; i--) {
 				free_irq(device->irq_num[i], device);
 				gpiod_put(device->gpiod[i]);
 			}
@@ -967,8 +945,7 @@ static void free_GPIOs(struct pi433_device *device)
 {
 	int i;
 
-	for (i=0; i<NUM_DIO; i++)
-	{
+	for (i = 0; i < NUM_DIO; i++) {
 		/* check if gpiod is valid */
 		if ( IS_ERR(device->gpiod[i]) )
 			continue;
@@ -1032,13 +1009,10 @@ static int pi433_probe(struct spi_device *spi)
 	/* spi->max_speed_hz = 10000000;  1MHz already set by device tree overlay */
 
 	retval = spi_setup(spi);
-	if (retval)
-	{
+	if (retval) {
 		dev_dbg(&spi->dev, "configuration of SPI interface failed!\n");
 		return retval;
-	}
-	else
-	{
+	} else {
 		dev_dbg(&spi->dev,
 			"spi interface setup: mode 0x%2x, %d bits per word, %dhz max speed",
 			spi->mode, spi->bits_per_word, spi->max_speed_hz);
@@ -1124,8 +1098,7 @@ static int pi433_probe(struct spi_device *spi)
 		pr_err("pi433: device register failed\n");
 		retval = PTR_ERR(device->dev);
 		goto device_create_failed;
-	}
-	else {
+	} else {
 		dev_dbg(device->dev,
 			"created device for major %d, minor %d\n",
 			MAJOR(pi433_dev),
-- 
1.9.1

Powered by blists - more mailing lists