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
>> ---
>> 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;
>> elsedevice->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 !=
