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 <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)
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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
