[PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[Tomas Marek]

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.

Signed-off-by: Tomas Marek <marek_tomas@centrum.cz>
---
 drivers/staging/pi433/pi433_if.c | 130 ++++++++++++---------------------------
 1 file changed, 40 insertions(+), 90 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2a205c6..795ae36 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -133,19 +133,14 @@ static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
 {
 	struct pi433_device *device = dev_id;
 
-	if      (device->irq_state[DIO0] == DIO_PacketSent)
-	{
+	if (device->irq_state[DIO0] == 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)
-	{
+	} 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)
-	{
+	} else if (device->irq_state[DIO0] == DIO_PayloadReady) {
 		dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
 		device->free_in_fifo = 0;
 		wake_up_interruptible(&device->fifo_wait_queue);
@@ -158,12 +153,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;
 	}
@@ -198,12 +190,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 	/* packet config */
 	/* enable */
 	SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
-	if (rx_cfg->enable_sync == optionOn)
-	{
+	if (rx_cfg->enable_sync == optionOn) {
 		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
-	}
-	else
-	{
+	} else {
 		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
 	}
 	if (rx_cfg->enable_length_byte == optionOn) {
@@ -220,29 +209,22 @@ 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));
 	}
 
 	/* values */
 	if (rx_cfg->enable_sync == optionOn)
-	{
 		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));
 	}
@@ -265,13 +247,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 
 	/* packet format enable */
 	if (tx_cfg->enable_preamble == optionOn)
-	{
 		SET_CHECKED(rf69_set_preamble_length(dev->spi, tx_cfg->preamble_length));
-	}
 	else
-	{
 		SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
-	}
+
 	SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
 	if (tx_cfg->enable_length_byte == optionOn) {
 		ret = rf69_set_packet_format(dev->spi, packetLengthVar);
@@ -341,8 +320,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 */
-	{
+	/* wait was interrupted */
+	if (retval) {
 		dev->interrupt_rx_allowed = true;
 		wake_up_interruptible(&dev->tx_wait_queue);
 		return retval;
@@ -359,8 +338,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 +361,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 +389,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 +404,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 +459,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 +534,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
 			 */
@@ -578,13 +546,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
 		SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
 		SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
 		if (tx_cfg.enable_length_byte == optionOn)
-		{
 			SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
-		}
 		else
-		{
 			SET_CHECKED(rf69_set_payload_length(spi, 0));
-		}
 
 		/* configure the rf chip */
 		rf69_set_tx_cfg(device, &tx_cfg);
@@ -607,19 +571,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 +610,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 +639,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 +867,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 +880,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 +922,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 +986,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 +1075,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

Re: [PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[Joe Perches]

On Sat, 2017-12-02 at 22:05 +0100, Tomas Marek wrote:
> 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.
> 
> Signed-off-by: Tomas Marek <marek_tomas@centrum.cz>
> ---
>  drivers/staging/pi433/pi433_if.c | 130 ++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index 2a205c6..795ae36 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -133,19 +133,14 @@ static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
>  {
>  	struct pi433_device *device = dev_id;
>  
> -	if      (device->irq_state[DIO0] == DIO_PacketSent)
> -	{
> +	if (device->irq_state[DIO0] == 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)
> -	{
> +	} 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)
> -	{
> +	} else if (device->irq_state[DIO0] == DIO_PayloadReady) {
>  		dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
>  		device->free_in_fifo = 0;
>  		wake_up_interruptible(&device->fifo_wait_queue);

I suggest using a switch/case and presumably the
dev_dbg should be dev_dbg_ratelimited.

Perhaps:

{
	const char *msg = NULL;
	struct wait_queue_head *wqh;

	switch (device->irq_state[DIO0]) {
	case DIO_PacketSent:
		device_free_in_fifo = FIFO_SIZE;
		wqh =  &device->fifo_wait_queue;
		msg = "Packet sent";
		break;
	case DIO_PayloadReady:
		device_free_in_fifo = 0;
		wqh =  &device->fifo_wait_queue;
		msg = "PayloadReady";
		break;
	case DIO_Rssi_DIO0:
		wqh =  &device->rx_wait_queue;
		msg = "RSSI level over
threshold";
		break;
	}

	if (msg) {
		dev_dbg_ratelimited(device->dev, "DIO0 irq: %s\n", msg);
		wake_up_interruptible(whq);
	}

	return IRQ_HANDLED;
}


> @@ -158,12 +153,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;
>  	}
> @@ -198,12 +190,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>  	/* packet config */
>  	/* enable */
>  	SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
> -	if (rx_cfg->enable_sync == optionOn)
> -	{
> +	if (rx_cfg->enable_sync == optionOn) {
>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
> -	}
> -	else
> -	{
> +	} else {
>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
>  	}
>  	if (rx_cfg->enable_length_byte == optionOn) {
> @@ -220,29 +209,22 @@ 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));
>  	}
>  
>  	/* values */
>  	if (rx_cfg->enable_sync == optionOn)
> -	{
>  		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));
>  	}
> @@ -265,13 +247,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>  
>  	/* packet format enable */
>  	if (tx_cfg->enable_preamble == optionOn)
> -	{
>  		SET_CHECKED(rf69_set_preamble_length(dev->spi, tx_cfg->preamble_length));
> -	}
>  	else
> -	{
>  		SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
> -	}
> +
>  	SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
>  	if (tx_cfg->enable_length_byte == optionOn) {
>  		ret = rf69_set_packet_format(dev->spi, packetLengthVar);
> @@ -341,8 +320,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 */
> -	{
> +	/* wait was interrupted */
> +	if (retval) {
>  		dev->interrupt_rx_allowed = true;
>  		wake_up_interruptible(&dev->tx_wait_queue);
>  		return retval;
> @@ -359,8 +338,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 +361,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 +389,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 +404,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 +459,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 +534,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
>  			 */
> @@ -578,13 +546,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>  		SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
>  		SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
>  		if (tx_cfg.enable_length_byte == optionOn)
> -		{
>  			SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
> -		}
>  		else
> -		{
>  			SET_CHECKED(rf69_set_payload_length(spi, 0));
> -		}
>  
>  		/* configure the rf chip */
>  		rf69_set_tx_cfg(device, &tx_cfg);
> @@ -607,19 +571,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 +610,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 +639,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 +867,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 +880,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 +922,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 +986,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 +1075,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),

Re: [PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[Marek Tomas]

On 12/02/2017 10:50 PM, Joe Perches wrote:
> On Sat, 2017-12-02 at 22:05 +0100, Tomas Marek wrote:
>> 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.
>>
>> Signed-off-by: Tomas Marek <marek_tomas@centrum.cz>
>> ---
>>  drivers/staging/pi433/pi433_if.c | 130 ++++++++++++---------------------------
>>  1 file changed, 40 insertions(+), 90 deletions(-)
>>
>> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
>> index 2a205c6..795ae36 100644
>> --- a/drivers/staging/pi433/pi433_if.c
>> +++ b/drivers/staging/pi433/pi433_if.c
>> @@ -133,19 +133,14 @@ static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
>>  {
>>  	struct pi433_device *device = dev_id;
>>  
>> -	if      (device->irq_state[DIO0] == DIO_PacketSent)
>> -	{
>> +	if (device->irq_state[DIO0] == 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)
>> -	{
>> +	} 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)
>> -	{
>> +	} else if (device->irq_state[DIO0] == DIO_PayloadReady) {
>>  		dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
>>  		device->free_in_fifo = 0;
>>  		wake_up_interruptible(&device->fifo_wait_queue);
> I suggest using a switch/case and presumably the
> dev_dbg should be dev_dbg_ratelimited.
>
> Perhaps:
>
> {
> 	const char *msg = NULL;
> 	struct wait_queue_head *wqh;
>
> 	switch (device->irq_state[DIO0]) {
> 	case DIO_PacketSent:
> 		device_free_in_fifo = FIFO_SIZE;
> 		wqh =  &device->fifo_wait_queue;
> 		msg = "Packet sent";
> 		break;
> 	case DIO_PayloadReady:
> 		device_free_in_fifo = 0;
> 		wqh =  &device->fifo_wait_queue;
> 		msg = "PayloadReady";
> 		break;
> 	case DIO_Rssi_DIO0:
> 		wqh =  &device->rx_wait_queue;
> 		msg = "RSSI level over
> threshold";
> 		break;
> 	}
>
> 	if (msg) {
> 		dev_dbg_ratelimited(device->dev, "DIO0 irq: %s\n", msg);
> 		wake_up_interruptible(whq);
> 	}
>
> 	return IRQ_HANDLED;
> }
Thanks for the input. Initially, I plan just a simple fix of errors reported by checkpatch to get familiar with patch submit procedure. Anyway, proposed change seems to be reasonable and I will try to implement it (and take advantage to send also v2 patch).

Would you like to be noted in v2 as suggested-by?

v2 coming up soon

Thank you again

Tomas
>
>> @@ -158,12 +153,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;
>>  	}
>> @@ -198,12 +190,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>>  	/* packet config */
>>  	/* enable */
>>  	SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
>> -	if (rx_cfg->enable_sync == optionOn)
>> -	{
>> +	if (rx_cfg->enable_sync == optionOn) {
>>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
>> -	}
>> -	else
>> -	{
>> +	} else {
>>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
>>  	}
>>  	if (rx_cfg->enable_length_byte == optionOn) {
>> @@ -220,29 +209,22 @@ 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));
>>  	}
>>  
>>  	/* values */
>>  	if (rx_cfg->enable_sync == optionOn)
>> -	{
>>  		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));
>>  	}
>> @@ -265,13 +247,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>>  
>>  	/* packet format enable */
>>  	if (tx_cfg->enable_preamble == optionOn)
>> -	{
>>  		SET_CHECKED(rf69_set_preamble_length(dev->spi, tx_cfg->preamble_length));
>> -	}
>>  	else
>> -	{
>>  		SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
>> -	}
>> +
>>  	SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
>>  	if (tx_cfg->enable_length_byte == optionOn) {
>>  		ret = rf69_set_packet_format(dev->spi, packetLengthVar);
>> @@ -341,8 +320,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 */
>> -	{
>> +	/* wait was interrupted */
>> +	if (retval) {
>>  		dev->interrupt_rx_allowed = true;
>>  		wake_up_interruptible(&dev->tx_wait_queue);
>>  		return retval;
>> @@ -359,8 +338,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 +361,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 +389,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 +404,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 +459,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 +534,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
>>  			 */
>> @@ -578,13 +546,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>>  		SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
>>  		SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
>>  		if (tx_cfg.enable_length_byte == optionOn)
>> -		{
>>  			SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
>> -		}
>>  		else
>> -		{
>>  			SET_CHECKED(rf69_set_payload_length(spi, 0));
>> -		}
>>  
>>  		/* configure the rf chip */
>>  		rf69_set_tx_cfg(device, &tx_cfg);
>> @@ -607,19 +571,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 +610,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 +639,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 +867,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 +880,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 +922,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 +986,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 +1075,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),

Re: [PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[Dan Carpenter]

On Sat, Dec 02, 2017 at 10:05:20PM +0100, Tomas Marek wrote:
> @@ -578,13 +546,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>  		SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
>  		SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
>  		if (tx_cfg.enable_length_byte == optionOn)
> -		{
>  			SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
> -		}
>  		else
> -		{
>  			SET_CHECKED(rf69_set_payload_length(spi, 0));
> -		}
>  

You can't delete the curly braces here because SET_CHECKED() is utter
garbage.  SET_CHECKED() needs to be wrapped in a do-while(0) loop.  But
someone sent a patch to delete it entirely which is even better.

regards,
dan carpenter

Re: [PATCH] Staging: pi433: fix brace coding style issues in pi433_if.c

[Marek Tomas]

On 12/04/2017 10:32 AM, Dan Carpenter wrote:

> On Sat, Dec 02, 2017 at 10:05:20PM +0100, Tomas Marek wrote:
>> @@ -578,13 +546,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
>>  		SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
>>  		SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
>>  		if (tx_cfg.enable_length_byte == optionOn)
>> -		{
>>  			SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
>> -		}
>>  		else
>> -		{
>>  			SET_CHECKED(rf69_set_payload_length(spi, 0));
>> -		}
>>  
> You can't delete the curly braces here because SET_CHECKED() is utter
> garbage.  SET_CHECKED() needs to be wrapped in a do-while(0) loop.  But
> someone sent a patch to delete it entirely which is even better.
>
> regards,
> dan carpenter
>
Thanks for pointing that out. I already noticed that during implementation
of another suggestion too. I will revert this change in v2 of the patch. Anyway,
as you wrote there is the patch "[PATCHv2] staging: pi433: pi433_if.c remove
SET_CHECKED macro" which removes this macro completely and solve the
styling issue.

Best regards,
Tomas Marek