From: Nguyen Phan Quang Minh Date: Fri, 8 Dec 2017 10:43:19 +0000 (+0100) Subject: staging: pi433: pi433_if.c remove SET_CHECKED macro X-Git-Tag: v4.16-rc1~112^2~396 X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=commitdiff_plain;h=038a5b4eb3f36a10170e15b34e583ad7c362df26;p=linux.git staging: pi433: pi433_if.c remove SET_CHECKED macro 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 Reviewed-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 55ed45f45998..32db3b320cbd 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -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)) {