| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4419d69e-dc2d-6944-752f-d925c8223b96@smarthome-wolf.de>
Date: Sun, 3 Dec 2017 18:49:40 +0200
From: Marcus Wolf <marcus.wolf@...rthome-wolf.de>
To: Simon Sandström <simon@...anor.nu>,
gregkh@...uxfoundation.org
Cc: linux@...f-Entwicklungen.de, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in
rf69_enum.h
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 <simon@...anor.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
Powered by blists - more mailing lists