Re: [PATCH] Staging: pi433: Coding style fixes

From: Greg KH
Date: Tue Jul 18 2017 - 09:34:57 EST


On Tue, Jul 18, 2017 at 02:41:33PM +0200, Simo Koskinen wrote:
> Fixes if-statement related warning and errors reported
> by checkpatch.pl
>
> Signed-off-by: Simo Koskinen <koskisoft@xxxxxxxxx>
> ---
> drivers/staging/pi433/pi433_if.c | 253 +++++++++++++++++----------------------
> drivers/staging/pi433/rf69.c | 69 ++++++-----
> 2 files changed, 148 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index 1bc478a..3e4be2e 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -131,19 +131,14 @@ DIO0_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs)
> {
> 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;
> printk("DIO0 irq: Packet sent\n"); // TODO: printk() should include KERN_ facility level
> 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) {
> printk("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) {
> printk("DIO0 irq: PayloadReady\n");
> device->free_in_fifo = 0;
> wake_up_interruptible(&device->fifo_wait_queue);
> @@ -157,15 +152,15 @@ DIO1_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs)
> {
> 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) {
> + if (device->rx_active)
> + device->free_in_fifo = FIFO_THRESHOLD - 1;
> + else
> + device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1;
> }
> - 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;
> - }
> +
> printk("DIO1 irq: %d bytes free in fifo\n", device->free_in_fifo); // TODO: printk() should include KERN_ facility level
> wake_up_interruptible(&device->fifo_wait_queue);
>
> @@ -200,43 +195,38 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
> /* 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));
> }
> +
> SET_CHECKED(rf69_set_packet_format (dev->spi, rx_cfg->enable_length_byte));
> SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
> SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
>
> /* 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++;
> + if (rx_cfg->enable_length_byte == optionOn)
> + payload_length++;
> +

Don't add new coding style issues :(

Always be careful, and ideally, you would show the before and after
results are the same for such a large patch...

thanks,

greg k-h