]> asedeno.scripts.mit.edu Git - linux.git/commitdiff
staging: pi433: pi433_if.c remove SET_CHECKED macro
authorNguyen Phan Quang Minh <minhnpq16@gmail.com>
Fri, 8 Dec 2017 10:43:19 +0000 (11:43 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 8 Dec 2017 15:31:35 +0000 (16:31 +0100)
The macro calls its argument -a function- twice, makes the calling
function return prematurely -skipping resource cleanup code- and hurts
understandability.

Signed-off-by: Nguyen Phan Quang Minh <minhnpq16@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/staging/pi433/pi433_if.c

index 55ed45f4599836acb8e2ba4141d4c8417cf9c888..32db3b320cbdc52cb2d48c2a35263c03abae590a 100644 (file)
@@ -119,12 +119,6 @@ struct pi433_instance {
        struct pi433_tx_cfg     tx_cfg;
 };
 
-/*-------------------------------------------------------------------------*/
-
-/* macro for checked access of registers of radio module */
-#define SET_CHECKED(retval) \
-       if (retval < 0) \
-               return retval;
 
 /*-------------------------------------------------------------------------*/
 
@@ -183,15 +177,33 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
        int payload_length;
 
        /* receiver config */
-       SET_CHECKED(rf69_set_frequency  (dev->spi, rx_cfg->frequency));
-       SET_CHECKED(rf69_set_bit_rate   (dev->spi, rx_cfg->bit_rate));
-       SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
-       SET_CHECKED(rf69_set_antenna_impedance   (dev->spi, rx_cfg->antenna_impedance));
-       SET_CHECKED(rf69_set_rssi_threshold      (dev->spi, rx_cfg->rssi_threshold));
-       SET_CHECKED(rf69_set_ook_threshold_dec   (dev->spi, rx_cfg->threshold_decrement));
-       SET_CHECKED(rf69_set_bandwidth           (dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-       SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-       SET_CHECKED(rf69_set_dagc                (dev->spi, rx_cfg->dagc));
+       ret = rf69_set_frequency(dev->spi, rx_cfg->frequency);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_modulation(dev->spi, rx_cfg->modulation);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_antenna_impedance(dev->spi, rx_cfg->antenna_impedance);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_ook_threshold_dec(dev->spi, rx_cfg->threshold_decrement);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_dagc(dev->spi, rx_cfg->dagc);
+       if (ret < 0)
+               return ret;
 
        dev->rx_bytes_to_drop = rx_cfg->bytes_to_drop;
 
@@ -203,7 +215,9 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
                if (ret < 0)
                        return ret;
 
-               SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
+               ret = rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt);
+               if (ret < 0)
+                       return ret;
        }
        else
        {
@@ -211,7 +225,9 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
                if (ret < 0)
                        return ret;
 
-               SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
+               ret = rf69_set_fifo_fill_condition(dev->spi, always);
+               if (ret < 0)
+                       return ret;
        }
        if (rx_cfg->enable_length_byte == OPTION_ON) {
                ret = rf69_set_packet_format(dev->spi, packetLengthVar);
@@ -222,7 +238,9 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
                if (ret < 0)
                        return ret;
        }
-       SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
+       ret = rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering);
+       if (ret < 0)
+               return ret;
 
        if (rx_cfg->enable_crc == OPTION_ON) {
                ret = rf69_enable_crc(dev->spi);
@@ -235,32 +253,46 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
        }
 
        /* lengths */
-       SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
+       ret = rf69_set_sync_size(dev->spi, rx_cfg->sync_length);
+       if (ret < 0)
+               return ret;
        if (rx_cfg->enable_length_byte == OPTION_ON)
        {
-               SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
+               ret = rf69_set_payload_length(dev->spi, 0xff);
+               if (ret < 0)
+                       return ret;
        }
        else if (rx_cfg->fixed_message_length != 0)
        {
                payload_length = rx_cfg->fixed_message_length;
                if (rx_cfg->enable_length_byte  == OPTION_ON) payload_length++;
                if (rx_cfg->enable_address_filtering != filteringOff) payload_length++;
-               SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length));
+               ret = rf69_set_payload_length(dev->spi, payload_length);
+               if (ret < 0)
+                       return ret;
        }
        else
        {
-               SET_CHECKED(rf69_set_payload_length(dev->spi, 0));
+               ret = rf69_set_payload_length(dev->spi, 0);
+               if (ret < 0)
+                       return ret;
        }
 
        /* values */
        if (rx_cfg->enable_sync == OPTION_ON)
        {
-               SET_CHECKED(rf69_set_sync_values(dev->spi, rx_cfg->sync_pattern));
+               ret = rf69_set_sync_values(dev->spi, rx_cfg->sync_pattern);
+               if (ret < 0)
+                       return ret;
        }
        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));
+               ret = rf69_set_node_address(dev->spi, rx_cfg->node_address);
+               if (ret < 0)
+                       return ret;
+               ret = rf69_set_broadcast_address(dev->spi, rx_cfg->broadcast_address);
+               if (ret < 0)
+                       return ret;
        }
 
        return 0;
@@ -271,22 +303,40 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 {
        int ret;
 
-       SET_CHECKED(rf69_set_frequency  (dev->spi, tx_cfg->frequency));
-       SET_CHECKED(rf69_set_bit_rate   (dev->spi, tx_cfg->bit_rate));
-       SET_CHECKED(rf69_set_modulation (dev->spi, tx_cfg->modulation));
-       SET_CHECKED(rf69_set_deviation  (dev->spi, tx_cfg->dev_frequency));
-       SET_CHECKED(rf69_set_pa_ramp    (dev->spi, tx_cfg->pa_ramp));
-       SET_CHECKED(rf69_set_modulation_shaping(dev->spi, tx_cfg->mod_shaping));
-       SET_CHECKED(rf69_set_tx_start_condition(dev->spi, tx_cfg->tx_start_condition));
+       ret = rf69_set_frequency(dev->spi, tx_cfg->frequency);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_bit_rate(dev->spi, tx_cfg->bit_rate);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_modulation(dev->spi, tx_cfg->modulation);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_deviation(dev->spi, tx_cfg->dev_frequency);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_pa_ramp(dev->spi, tx_cfg->pa_ramp);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_modulation_shaping(dev->spi, tx_cfg->mod_shaping);
+       if (ret < 0)
+               return ret;
+       ret = rf69_set_tx_start_condition(dev->spi, tx_cfg->tx_start_condition);
+       if (ret < 0)
+               return ret;
 
        /* packet format enable */
        if (tx_cfg->enable_preamble == OPTION_ON)
        {
-               SET_CHECKED(rf69_set_preamble_length(dev->spi, tx_cfg->preamble_length));
+               ret = rf69_set_preamble_length(dev->spi, tx_cfg->preamble_length);
+               if (ret < 0)
+                       return ret;
        }
        else
        {
-               SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
+               ret = rf69_set_preamble_length(dev->spi, 0);
+               if (ret < 0)
+                       return ret;
        }
 
        if (tx_cfg->enable_sync == OPTION_ON) {
@@ -321,8 +371,12 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 
        /* configure sync, if enabled */
        if (tx_cfg->enable_sync == OPTION_ON) {
-               SET_CHECKED(rf69_set_sync_size(dev->spi, tx_cfg->sync_length));
-               SET_CHECKED(rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern));
+               ret = rf69_set_sync_size(dev->spi, tx_cfg->sync_length);
+               if (ret < 0)
+                       return ret;
+               ret = rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern);
+               if (ret < 0)
+                       return ret;
        }
 
        return 0;
@@ -344,18 +398,26 @@ pi433_start_rx(struct pi433_device *dev)
        if (retval) return retval;
 
        /* setup rssi irq */
-       SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO0, DIO_RSSI_DIO0));
+       retval = rf69_set_dio_mapping(dev->spi, DIO0, DIO_RSSI_DIO0);
+       if (retval < 0)
+               return retval;
        dev->irq_state[DIO0] = DIO_RSSI_DIO0;
        irq_set_irq_type(dev->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
 
        /* setup fifo level interrupt */
-       SET_CHECKED(rf69_set_fifo_threshold(dev->spi, FIFO_SIZE - FIFO_THRESHOLD));
-       SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO1, DIO_FIFO_LEVEL));
+       retval = rf69_set_fifo_threshold(dev->spi, FIFO_SIZE - FIFO_THRESHOLD);
+       if (retval < 0)
+               return retval;
+       retval = rf69_set_dio_mapping(dev->spi, DIO1, DIO_FIFO_LEVEL);
+       if (retval < 0)
+               return retval;
        dev->irq_state[DIO1] = DIO_FIFO_LEVEL;
        irq_set_irq_type(dev->irq_num[DIO1], IRQ_TYPE_EDGE_RISING);
 
        /* set module to receiving mode */
-       SET_CHECKED(rf69_set_mode(dev->spi, receive));
+       retval = rf69_set_mode(dev->spi, receive);
+       if (retval < 0)
+               return retval;
 
        return 0;
 }
@@ -367,7 +429,7 @@ static int
 pi433_receive(void *data)
 {
        struct pi433_device *dev = data;
-       struct spi_device *spi = dev->spi; /* needed for SET_CHECKED */
+       struct spi_device *spi = dev->spi;
        int bytes_to_read, bytes_total;
        int retval;
 
@@ -413,7 +475,9 @@ pi433_receive(void *data)
        }
 
        /* configure payload ready irq */
-       SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PAYLOAD_READY));
+       retval = rf69_set_dio_mapping(spi, DIO0, DIO_PAYLOAD_READY);
+       if (retval < 0)
+               goto abort;
        dev->irq_state[DIO0] = DIO_PAYLOAD_READY;
        irq_set_irq_type(dev->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
 
@@ -503,7 +567,8 @@ pi433_receive(void *data)
        /* rx done, wait was interrupted or error occurred */
 abort:
        dev->interrupt_rx_allowed = true;
-       SET_CHECKED(rf69_set_mode(dev->spi, standby));
+       if (rf69_set_mode(dev->spi, standby))
+               pr_err("rf69_set_mode(): radio module failed to go standby\n");
        wake_up_interruptible(&dev->tx_wait_queue);
 
        if (retval)
@@ -516,7 +581,7 @@ static int
 pi433_tx_thread(void *data)
 {
        struct pi433_device *device = data;
-       struct spi_device *spi = device->spi; /* needed for SET_CHECKED */
+       struct spi_device *spi = device->spi;
        struct pi433_tx_cfg tx_cfg;
        u8     *buffer = device->buffer;
        size_t size;
@@ -605,38 +670,56 @@ pi433_tx_thread(void *data)
                        /* rx is currently waiting for a telegram;
                         * we need to set the radio module to standby
                         */
-                       SET_CHECKED(rf69_set_mode(device->spi, standby));
+                       retval = rf69_set_mode(device->spi, standby);
+                       if (retval < 0)
+                               return retval;
                        rx_interrupted = true;
                }
 
                /* clear fifo, set fifo threshold, set payload length */
-               SET_CHECKED(rf69_set_mode(spi, standby)); /* this clears the fifo */
-               SET_CHECKED(rf69_set_fifo_threshold(spi, FIFO_THRESHOLD));
+               retval = rf69_set_mode(spi, standby); /* this clears the fifo */
+               if (retval < 0)
+                       return retval;
+               retval = rf69_set_fifo_threshold(spi, FIFO_THRESHOLD);
+               if (retval < 0)
+                       return retval;
                if (tx_cfg.enable_length_byte == OPTION_ON)
                {
-                       SET_CHECKED(rf69_set_payload_length(spi, size * tx_cfg.repetitions));
+                       retval = rf69_set_payload_length(spi, size * tx_cfg.repetitions);
+                       if (retval < 0)
+                               return retval;
                }
                else
                {
-                       SET_CHECKED(rf69_set_payload_length(spi, 0));
+                       retval = rf69_set_payload_length(spi, 0);
+                       if (retval < 0)
+                               return retval;
                }
 
                /* configure the rf chip */
-               rf69_set_tx_cfg(device, &tx_cfg);
+               retval = rf69_set_tx_cfg(device, &tx_cfg);
+               if (retval < 0)
+                       return retval;
 
                /* enable fifo level interrupt */
-               SET_CHECKED(rf69_set_dio_mapping(spi, DIO1, DIO_FIFO_LEVEL));
+               retval = rf69_set_dio_mapping(spi, DIO1, DIO_FIFO_LEVEL);
+               if (retval < 0)
+                       return retval;
                device->irq_state[DIO1] = DIO_FIFO_LEVEL;
                irq_set_irq_type(device->irq_num[DIO1], IRQ_TYPE_EDGE_FALLING);
 
                /* enable packet sent interrupt */
-               SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PACKET_SENT));
+               retval = rf69_set_dio_mapping(spi, DIO0, DIO_PACKET_SENT);
+               if (retval < 0)
+                       return retval;
                device->irq_state[DIO0] = DIO_PACKET_SENT;
                irq_set_irq_type(device->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
                enable_irq(device->irq_num[DIO0]); /* was disabled by rx active check */
 
                /* enable transmission */
-               SET_CHECKED(rf69_set_mode(spi, transmit));
+               retval = rf69_set_mode(spi, transmit);
+               if (retval < 0)
+                       return retval;
 
                /* transfer this msg (and repetitions) to chip fifo */
                device->free_in_fifo = FIFO_SIZE;
@@ -678,7 +761,9 @@ pi433_tx_thread(void *data)
 
                /* STOP_TRANSMISSION */
                dev_dbg(device->dev, "thread: Packet sent. Set mode to stby.");
-               SET_CHECKED(rf69_set_mode(spi, standby));
+               retval = rf69_set_mode(spi, standby);
+               if (retval < 0)
+                       return retval;
 
                /* everything sent? */
                if (kfifo_is_empty(&device->tx_fifo)) {