Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752638AbdLCQto (ORCPT ); Sun, 3 Dec 2017 11:49:44 -0500 Received: from dd39320.kasserver.com ([85.13.155.146]:45724 "EHLO dd39320.kasserver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752377AbdLCQtn (ORCPT ); Sun, 3 Dec 2017 11:49:43 -0500 Subject: Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h To: =?UTF-8?Q?Simon_Sandstr=c3=b6m?= , gregkh@linuxfoundation.org Cc: linux@Wolf-Entwicklungen.de, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org References: <20171203151726.16639-1-simon@nikanor.nu> <20171203151726.16639-5-simon@nikanor.nu> From: Marcus Wolf Message-ID: <4419d69e-dc2d-6944-752f-d925c8223b96@smarthome-wolf.de> Date: Sun, 3 Dec 2017 18:49:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171203151726.16639-5-simon@nikanor.nu> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 16188 Lines: 367 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: , , ". > > Signed-off-by: Simon Sandström > --- > 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