Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Sun, 03 Dec 2017 08:50:36 -0800 


Am 03.12.2017 um 17:17 schrieb Simon Sandström:

Renames the enum optionOnOff and its values optionOn, optionOff to enum
option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings:
"Avoid CamelCase: <optionOnOff>, <optionOn>, <optionOff>".

Signed-off-by: Simon Sandström <si...@nikanor.nu>
---
  drivers/staging/pi433/pi433_if.c  | 34 ++++++++++++++---------------
  drivers/staging/pi433/pi433_if.h  | 16 +++++++-------
  drivers/staging/pi433/rf69.c      | 45 ++++++++++++++++++++++-----------------
  drivers/staging/pi433/rf69.h      | 15 ++++++++-----
  drivers/staging/pi433/rf69_enum.h |  6 +++---
  5 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b8efe6a74a2a..4f6830f369e9 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -198,7 +198,7 @@ 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 == OPTION_ON)
        {
                SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
        }
@@ -206,7 +206,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
        {
                SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
        }
-       if (rx_cfg->enable_length_byte == optionOn) {
+       if (rx_cfg->enable_length_byte == OPTION_ON) {
                ret = rf69_set_packet_format(dev->spi, packetLengthVar);
                if (ret < 0)
                        return ret;
@@ -220,14 +220,14 @@ 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));
-       if (rx_cfg->enable_length_byte == optionOn)
+       if (rx_cfg->enable_length_byte == OPTION_ON)
        {
                SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
        }
        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_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));
        }
@@ -237,7 +237,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
        }

  
  	/* values */

-       if (rx_cfg->enable_sync == optionOn)
+       if (rx_cfg->enable_sync == OPTION_ON)
        {
                SET_CHECKED(rf69_set_sync_values(dev->spi, 
rx_cfg->sync_pattern));
        }
@@ -264,7 +264,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
        SET_CHECKED(rf69_set_tx_start_condition(dev->spi, 
tx_cfg->tx_start_condition));

  
  	/* packet format enable */

-       if (tx_cfg->enable_preamble == optionOn)
+       if (tx_cfg->enable_preamble == OPTION_ON)
        {
                SET_CHECKED(rf69_set_preamble_length(dev->spi, 
tx_cfg->preamble_length));
        }
@@ -273,7 +273,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
                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) {
+       if (tx_cfg->enable_length_byte == OPTION_ON) {
                ret = rf69_set_packet_format(dev->spi, packetLengthVar);
                if (ret < 0)
                        return ret;
@@ -285,7 +285,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
        SET_CHECKED(rf69_set_crc_enable   (dev->spi, tx_cfg->enable_crc));

  
  	/* configure sync, if enabled */

-       if (tx_cfg->enable_sync == optionOn) {
+       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));
        }
@@ -400,7 +400,7 @@ pi433_receive(void *data)
        }

  
  	/* length byte enabled? */

-       if (dev->rx_cfg.enable_length_byte == optionOn)
+       if (dev->rx_cfg.enable_length_byte == OPTION_ON)
        {
                retval = wait_event_interruptible(dev->fifo_wait_queue,
                                                  dev->free_in_fifo < 
FIFO_SIZE);
@@ -525,11 +525,11 @@ pi433_tx_thread(void *data)
                        size = tx_cfg.fixed_message_length;

  
  		/* increase size, if len byte is requested */

-               if (tx_cfg.enable_length_byte == optionOn)
+               if (tx_cfg.enable_length_byte == OPTION_ON)
                        size++;

  
  		/* increase size, if adr byte is requested */

-               if (tx_cfg.enable_address_byte == optionOn)
+               if (tx_cfg.enable_address_byte == OPTION_ON)
                        size++;

  
  		/* prime buffer */

@@ -537,11 +537,11 @@ pi433_tx_thread(void *data)
                position = 0;

  
  		/* add length byte, if requested */

-               if (tx_cfg.enable_length_byte  == optionOn)
+               if (tx_cfg.enable_length_byte  == OPTION_ON)
                        buffer[position++] = size-1; /* according to spec 
length byte itself must be excluded from the length calculation */

  
  		/* add adr byte, if requested */

-               if (tx_cfg.enable_address_byte == optionOn)
+               if (tx_cfg.enable_address_byte == OPTION_ON)
                        buffer[position++] = tx_cfg.address_byte;

  
  		/* finally get message data from fifo */

@@ -577,7 +577,7 @@ pi433_tx_thread(void *data)
                /* 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));
-               if (tx_cfg.enable_length_byte == optionOn)
+               if (tx_cfg.enable_length_byte == OPTION_ON)
                {
                        SET_CHECKED(rf69_set_payload_length(spi, size * 
tx_cfg.repetitions));
                }
@@ -1091,9 +1091,9 @@ static int pi433_probe(struct spi_device *spi)
        /* setup the radio module */
        SET_CHECKED(rf69_set_mode               (spi, standby));
        SET_CHECKED(rf69_set_data_mode          (spi, packet));
-       SET_CHECKED(rf69_set_amplifier_0        (spi, optionOn));
-       SET_CHECKED(rf69_set_amplifier_1        (spi, optionOff));
-       SET_CHECKED(rf69_set_amplifier_2        (spi, optionOff));
+       SET_CHECKED(rf69_set_amplifier_0        (spi, OPTION_ON));
+       SET_CHECKED(rf69_set_amplifier_1        (spi, OPTION_OFF));
+       SET_CHECKED(rf69_set_amplifier_2        (spi, OPTION_OFF));
        SET_CHECKED(rf69_set_output_power_level (spi, 13));
        SET_CHECKED(rf69_set_antenna_impedance  (spi, fiftyOhm));

  
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h

index 6b31c1584b3a..34ff0d4807bd 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -73,11 +73,11 @@ struct pi433_tx_cfg {

  
  
  	/* packet format */

-       enum optionOnOff        enable_preamble;
-       enum optionOnOff        enable_sync;
-       enum optionOnOff        enable_length_byte;
-       enum optionOnOff        enable_address_byte;
-       enum optionOnOff        enable_crc;
+       enum option_on_off      enable_preamble;
+       enum option_on_off      enable_sync;
+       enum option_on_off      enable_length_byte;
+       enum option_on_off      enable_address_byte;
+       enum option_on_off      enable_crc;

  
  	__u16			preamble_length;

        __u8                    sync_length;
@@ -125,10 +125,10 @@ struct pi433_rx_cfg {

  
  
  	/* packet format */

-       enum optionOnOff        enable_sync;
-       enum optionOnOff        enable_length_byte;       /* should be used in 
combination with sync, only */
+       enum option_on_off      enable_sync;
+       enum option_on_off      enable_length_byte;       /* should be used in 
combination with sync, only */
        enum addressFiltering   enable_address_filtering; /* operational with 
sync, only */
-       enum optionOnOff        enable_crc;               /* only operational, 
if sync on and fixed length or length byte is used */
+       enum option_on_off      enable_crc;               /* only operational, 
if sync on and fixed length or length byte is used */

  
  	__u8			sync_length;

        __u8                    fixed_message_length;
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e69a2153c999..ebb3ddd1a957 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -272,45 +272,48 @@ int rf69_set_frequency(struct spi_device *spi, u32 
frequency)
        return 0;
  }

  
-int rf69_set_amplifier_0(struct spi_device *spi, enum optionOnOff optionOnOff)

+int rf69_set_amplifier_0(struct spi_device *spi,
+                        enum option_on_off option_on_off)
  {
        #ifdef DEBUG
                dev_dbg(&spi->dev, "set: amp #0");
        #endif

  
-	switch (optionOnOff) {

-       case optionOn:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |  
MASK_PALEVEL_PA0));
-       case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA0));
+       switch (option_on_off) {
+       case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA0));
+       case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA0));
        default:
                dev_dbg(&spi->dev, "set: illegal input param");
                return -EINVAL;
        }
  }

  
-int rf69_set_amplifier_1(struct spi_device *spi, enum optionOnOff optionOnOff)

+int rf69_set_amplifier_1(struct spi_device *spi,
+                        enum option_on_off option_on_off)
  {
        #ifdef DEBUG
                dev_dbg(&spi->dev, "set: amp #1");
        #endif

  
-	switch (optionOnOff) {

-       case optionOn:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |  
MASK_PALEVEL_PA1));
-       case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA1));
+       switch (option_on_off) {
+       case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA1));
+       case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA1));
        default:
                dev_dbg(&spi->dev, "set: illegal input param");
                return -EINVAL;
        }
  }

  
-int rf69_set_amplifier_2(struct spi_device *spi, enum optionOnOff optionOnOff)

+int rf69_set_amplifier_2(struct spi_device *spi,
+                        enum option_on_off option_on_off)
  {
        #ifdef DEBUG
                dev_dbg(&spi->dev, "set: amp #2");
        #endif

  
-	switch (optionOnOff) {

-       case optionOn:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |  
MASK_PALEVEL_PA2));
-       case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA2));
+       switch (option_on_off) {
+       case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA2));
+       case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA2));
        default:
                dev_dbg(&spi->dev, "set: illegal input param");
                return -EINVAL;
@@ -720,15 +723,16 @@ int rf69_set_preamble_length(struct spi_device *spi, u16 
preambleLength)
        return WRITE_REG(REG_PREAMBLE_LSB, lsb);
  }

  
-int rf69_set_sync_enable(struct spi_device *spi, enum optionOnOff optionOnOff)

+int rf69_set_sync_enable(struct spi_device *spi,
+                        enum option_on_off option_on_off)
  {
        #ifdef DEBUG
                dev_dbg(&spi->dev, "set: sync enable");
        #endif

  
-	switch (optionOnOff) {

-       case optionOn:  return WRITE_REG(REG_SYNC_CONFIG, 
(READ_REG(REG_SYNC_CONFIG) |  MASK_SYNC_CONFIG_SYNC_ON));
-       case optionOff: return WRITE_REG(REG_SYNC_CONFIG, 
(READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_ON));
+       switch (option_on_off) {
+       case OPTION_ON:  return WRITE_REG(REG_SYNC_CONFIG, 
(READ_REG(REG_SYNC_CONFIG) |  MASK_SYNC_CONFIG_SYNC_ON));
+       case OPTION_OFF: return WRITE_REG(REG_SYNC_CONFIG, 
(READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_ON));
        default:
                dev_dbg(&spi->dev, "set: illegal input param");
                return -EINVAL;
@@ -817,15 +821,16 @@ int rf69_set_packet_format(struct spi_device *spi, enum 
packetFormat packetForma
        }
  }

  
-int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff)

+int rf69_set_crc_enable(struct spi_device *spi,
+                       enum option_on_off option_on_off)
  {
        #ifdef DEBUG
                dev_dbg(&spi->dev, "set: crc enable");
        #endif

  
-	switch (optionOnOff) {

-       case optionOn:  return WRITE_REG(REG_PACKETCONFIG1, 
(READ_REG(REG_PACKETCONFIG1) |  MASK_PACKETCONFIG1_CRC_ON));
-       case optionOff: return WRITE_REG(REG_PACKETCONFIG1, 
(READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_CRC_ON));
+       switch (option_on_off) {
+       case OPTION_ON:  return WRITE_REG(REG_PACKETCONFIG1, 
(READ_REG(REG_PACKETCONFIG1) |  MASK_PACKETCONFIG1_CRC_ON));
+       case OPTION_OFF: return WRITE_REG(REG_PACKETCONFIG1, 
(READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_CRC_ON));
        default:
                dev_dbg(&spi->dev, "set: illegal input param");
                return -EINVAL;
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 5c0c95628f2f..12c2e106785e 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -33,9 +33,12 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum 
modShaping modShapi
  int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate);
  int rf69_set_deviation(struct spi_device *spi, u32 deviation);
  int rf69_set_frequency(struct spi_device *spi, u32 frequency);
-int rf69_set_amplifier_0(struct spi_device *spi, enum optionOnOff optionOnOff);
-int rf69_set_amplifier_1(struct spi_device *spi, enum optionOnOff optionOnOff);
-int rf69_set_amplifier_2(struct spi_device *spi, enum optionOnOff optionOnOff);
+int rf69_set_amplifier_0(struct spi_device *spi,
+                        enum option_on_off option_on_off);
+int rf69_set_amplifier_1(struct spi_device *spi,
+                        enum option_on_off option_on_off);
+int rf69_set_amplifier_2(struct spi_device *spi,
+                        enum option_on_off option_on_off);
  int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel);
  int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp);
  int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance 
antennaImpedance);
@@ -56,13 +59,15 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8 
threshold);
  int rf69_set_rx_start_timeout(struct spi_device *spi, u8 timeout);
  int rf69_set_rssi_timeout(struct spi_device *spi, u8 timeout);
  int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength);
-int rf69_set_sync_enable(struct spi_device *spi, enum optionOnOff optionOnOff);
+int rf69_set_sync_enable(struct spi_device *spi,
+                        enum option_on_off option_on_off);
  int rf69_set_fifo_fill_condition(struct spi_device *spi, enum 
fifoFillCondition fifoFillCondition);
  int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
  int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
  int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8]);
  int rf69_set_packet_format(struct spi_device *spi, enum packetFormat 
packetFormat);
-int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff);
+int rf69_set_crc_enable(struct spi_device *spi,
+                       enum option_on_off option_on_off);
  int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering 
addressFiltering);
  int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
  u8  rf69_get_payload_length(struct spi_device *spi);
diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index babe597e2ec6..5247e9269de9 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,9 +18,9 @@
  #ifndef RF69_ENUM_H
  #define RF69_ENUM_H

  
-enum optionOnOff {

-       optionOff,
-       optionOn
+enum option_on_off {
+       OPTION_OFF,
+       OPTION_ON
  };

  
  enum mode {




Hi Simon,

thanks for your effort.

I have two questions:

* According to my practical experiance, enums were always written in 
lower case. Does kernel style guide ask for upper case for enums?
For me upper case indicates, that this value will be processed by 
preprocessor, not by compiler. So I used it for define constants and 
macros so far...
* The enums are the interface to user space. Isn't it necessary to 
increse the interface version number on every change of the enums?
Or can we stay with old version number, since order of the enums is 
untouched?


Thanks,

Marcus























					Reply via email to