lkml.org 
[lkml]   [2017]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h
On Sun, Dec 03, 2017 at 04:17:24PM +0100, Simon Sandström wrote:
> 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@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)

I haven't told anyone yet, but I wish someone would just get rid of
optionOn entirely. This should just be:

if (rx_cfg->enable_sync) {


> 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;

These should be bool.

> -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)


This should be two functions:

int rf69_enable_amplifier_0(struct spi_device *spi)
{
return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | MASK_PALEVEL_PA0));
}

int rf69_disable_amplifier_0(struct spi_device *spi)
{
return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & ~MASK_PALEVEL_PA0));
}

Perhaps choose different function names if you want? You could do it
as several patches:

patch 1: change types to bool
patch 2: sed -e '/ == optionOn//'
patch 3: split the functions into two functions
patch 4: delete optionOnOff enum

patches 1 and 2 could be merged together (your choice).

regards,
dan carpenter
\
 
 \ /
  Last update: 2017-12-04 11:18    [W:0.073 / U:5.680 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site