Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.
From: Wolf Entwicklungen
Date: Wed Aug 02 2017 - 04:08:13 EST
Reviewed-by: Marcus Wolf <linux@xxxxxxxxxxxxxxxxxxxxx>
Just reviewed, not tested.
As far as I can see, there is no technical issue with this patch.
I prefer the names of the enumerations in camel case, because then they are a bit shorter.
If camel case is unwanted, for sure we need that change.
Please mind the allignment. For enhanced readability of structs, I always try to start the type
in the same column, the variable name in the same column and - if nneded - the comments in the
same column - so you see all members of the struct optically in a kind of table.
Thanks,
Marcus
Am Di, 1.08.2017, 21:31 schrieb Rishabh Hardas:
> This is a 5 patch series which solves coding style issues
> as marked by checkpatch.pl in the file pi433_if.h and
> contains changes that have to be made in other files as a
> consequence of the changes made in pi433_if.h
>
> Signed-off-by: Rishabh Hardas <rishabhhardas@xxxxxxxxx>
> ---
> drivers/staging/pi433/pi433_if.h | 81 +++++++++++++++++++++-------------------
> 1 file changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> index e6ed3cd..d87434c 100644
> --- a/drivers/staging/pi433/pi433_if.h
> +++ b/drivers/staging/pi433/pi433_if.h
> @@ -32,15 +32,11 @@
> #include <linux/types.h>
> #include "rf69_enum.h"
>
> -/*---------------------------------------------------------------------------*/
> -
> -
> -/*---------------------------------------------------------------------------*/
> -
> /* IOCTL structs and commands */
>
> /**
> - * struct pi433_tx_config - describes the configuration of the radio module for sending
> + * struct pi433_tx_config - describes the configuration of
> + * the radio module for sending
> * @frequency:
> * @bit_rate:
> * @modulation:
> @@ -57,28 +53,26 @@
> *
> * NOTE: struct layout is the same in 64bit and 32bit userspace.
> */
> -#define PI433_TX_CFG_IOCTL_NR 0
> -struct pi433_tx_cfg
> -{
> +#define PI433_TX_CFG_IOCTL_NR 0
> +struct pi433_tx_cfg {
> __u32 frequency;
> __u16 bit_rate;
> __u32 dev_frequency;
> enum modulation modulation;
> - enum modShaping modShaping;
> + enum mod_shaping mod_shaping;
>
> - enum paRamp pa_ramp;
> + enum pa_ramp pa_ramp;
>
> - enum txStartCondition tx_start_condition;
> + enum tx_start_condition tx_start_condition;
>
> __u16 repetitions;
>
> -
> /* 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;
> @@ -88,9 +82,9 @@ struct pi433_tx_cfg
> __u8 address_byte;
> };
>
> -
> /**
> - * struct pi433_rx_config - describes the configuration of the radio module for sending
> + * struct pi433_rx_config - describes the configuration of
> + * the radio module for sending
> * @frequency:
> * @bit_rate:
> * @modulation:
> @@ -107,29 +101,37 @@ struct pi433_tx_cfg
> *
> * NOTE: struct layout is the same in 64bit and 32bit userspace.
> */
> -#define PI433_RX_CFG_IOCTL_NR 1
> +#define PI433_RX_CFG_IOCTL_NR 1
> struct pi433_rx_cfg {
> __u32 frequency;
> __u16 bit_rate;
> __u32 dev_frequency;
>
> - enum modulation modulation;
> + enum modulation modulation;
>
> - __u8 rssi_threshold;
> - enum thresholdDecrement thresholdDecrement;
> - enum antennaImpedance antenna_impedance;
> - enum lnaGain lna_gain;
> - enum mantisse bw_mantisse; /* normal: 0x50 */
> - __u8 bw_exponent; /* during AFC: 0x8b */
> - enum dagc dagc;
> + __u8 rssi_threshold;
>
> + enum threshold_decrement threshold_decrement;
> + enum antenna_impedance antenna_impedance;
>
> + enum lnagain lna_gain;
> + enum mantisse bw_mantisse; /* normal: 0x50 */
> + __u8 bw_exponent; /* during AFC: 0x8b */
> + enum dagc dagc;
>
> /* packet format */
> - enum optionOnOff enable_sync;
> - enum optionOnOff 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_sync;
> + enum option_on_off enable_length_byte;/* should be used
> + * in combination
> + * with sync,only
> + */
> + enum address_filtering enable_address_filtering;/* operational with
> + * sync, only
> + */
> + 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;
> @@ -140,13 +142,16 @@ struct pi433_rx_cfg {
> __u8 broadcast_address;
> };
>
> -
> #define PI433_IOC_MAGIC 'r'
>
> -#define PI433_IOC_RD_TX_CFG _IOR(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
> -#define PI433_IOC_WR_TX_CFG _IOW(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR, char[sizeof(struct pi433_tx_cfg)])
> +#define PI433_IOC_RD_TX_CFG _IOR(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR,\
> + char[sizeof(struct pi433_tx_cfg)])
> +#define PI433_IOC_WR_TX_CFG _IOW(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR,\
> + char[sizeof(struct pi433_tx_cfg)])
>
> -#define PI433_IOC_RD_RX_CFG _IOR(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR, char[sizeof(struct pi433_rx_cfg)])
> -#define PI433_IOC_WR_RX_CFG _IOW(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR, char[sizeof(struct pi433_rx_cfg)])
> +#define PI433_IOC_RD_RX_CFG _IOR(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR,\
> + char[sizeof(struct pi433_rx_cfg)])
> +#define PI433_IOC_WR_RX_CFG _IOW(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR,\
> + char[sizeof(struct pi433_rx_cfg)])
>
> #endif /* PI433_H */
> --
> 2.7.4
>
>
>