staging: pi433: Rename camel case and other style issues
v2 cover staging: pi433: Rename camel case and other style issues
Hi,
This series pf patches solves some of the coding style issues. I have corrected long lines, changed comment style, renamed enums and variables that were in camel case.
Tried to get zero erros and warnings on the pi433_if.h file.
Regards,
Rishabh Hardas
Rishabh Hardas (4):
staging: pi433: Style fix - Correct long lines
staging: pi433: Change Comments
staging: pi433: Renaming Enums
staging: pi433: Remove camel case variable names
drivers/staging/pi433/pi433_if.c | 4 +-- drivers/staging/pi433/pi433_if.h | 73 +++++++++++++++++++++------------------ drivers/staging/pi433/rf69.c | 26 +++++++------- drivers/staging/pi433/rf69.h | 26 +++++++------- drivers/staging/pi433/rf69_enum.h | 16 ++++----- 5 files changed, 76 insertions(+), 69 deletions(-)
--
1.9.1
PATCH v2 1/4 staging: pi433: Style fix - Correct long lines
Signed-off-by Rishabh Hardas <[email protected]>
--- drivers/staging/pi433/pi433_if.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
drivers/staging/pi433/pi433_if.h +15 -13
@@ -33,14 +33,13 @@#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,9 +56,8 @@* * 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;
@@ -90,7 +88,8 @@ struct pi433_tx_cfg/** -* 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,7 +106,7 @@ 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 1struct pi433_rx_cfg {__u32 frequency;__u16 bit_rate;
@@ -143,10 +142,13 @@ struct pi433_rx_cfg {#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_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_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)])#endif /* PI433_H */
--
1.9.1
PATCH v2 3/4 staging: pi433: Renaming Enums
In reply to Rishabh HardasSigned-off-by Rishabh Hardas <[email protected]>
--- drivers/staging/pi433/pi433_if.h | 36 ++++++++++++++++-------------------- drivers/staging/pi433/rf69.c | 26 +++++++++++++------------- drivers/staging/pi433/rf69.h | 26 +++++++++++++------------- drivers/staging/pi433/rf69_enum.h | 16 ++++++++-------- 4 files changed, 50 insertions(+), 54 deletions(-)
drivers/staging/pi433/pi433_if.h +16 -20
@@ -62,21 +62,20 @@ struct pi433_tx_cfg {__u16 bit_rate;__u32 dev_frequency;enum modulation modulation; -enum modShaping modShaping; +enum mod_shaping modShaping; -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;
@@ -86,7 +85,6 @@ struct pi433_tx_cfg {__u8 address_byte; }; -/** * struct pi433_rx_config - describes the configuration of the radio module for * sending
@@ -115,25 +113,24 @@ struct pi433_rx_cfg {enum modulation modulation;__u8 rssi_threshold; -enum thresholdDecrement thresholdDecrement; -enum antennaImpedance antenna_impedance; -enum lnaGain lna_gain; + +enum threshold_decrement thresholdDecrement; +enum antenna_impedance antenna_impedance; +enum lna_gain 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 +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 +enum address_filtering enable_address_filtering;/* operational * with sync, only */ -enum optionOnOff enable_crc;/* only operational, +enum option_on_off enable_crc;/* only operational, *if sync on and fixed * length or length * byte is used
@@ -148,7 +145,6 @@ 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,\
drivers/staging/pi433/rf69.c +13 -13
@@ -109,7 +109,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi) } } -int rf69_set_modulation_shaping (struct spi_device *spi,enum modShaping modShaping) +int rf69_set_modulation_shaping (struct spi_device *spi,enum mod_shaping modShaping) {#ifdef DEBUG dev_dbg (&spi->dev,"set: mod shaping" );
@@ -264,7 +264,7 @@ 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 optionOnOff) {#ifdef DEBUG dev_dbg (&spi->dev,"set: amp #0" );
@@ -277,7 +277,7 @@ 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_1 (struct spi_device *spi,enum option_on_off optionOnOff) {#ifdef DEBUG dev_dbg (&spi->dev,"set: amp #1" );
@@ -290,7 +290,7 @@ 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_2 (struct spi_device *spi,enum option_on_off optionOnOff) {#ifdef DEBUG dev_dbg (&spi->dev,"set: amp #2" );
@@ -319,7 +319,7 @@ int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)return WRITE_REG (REG_PALEVEL , (READ_REG (REG_PALEVEL ) & ~MASK_PALEVEL_OUTPUT_POWER ) | powerLevel); } -int rf69_set_pa_ramp (struct spi_device *spi,enum paRamp paRamp) +int rf69_set_pa_ramp (struct spi_device *spi,enum pa_ramp paRamp) {#ifdef DEBUG dev_dbg (&spi->dev,"set: pa ramp" );
@@ -346,7 +346,7 @@ int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp) } } -int rf69_set_antenna_impedance (struct spi_device *spi,enum antennaImpedance antennaImpedance) +int rf69_set_antenna_impedance (struct spi_device *spi,enum antenna_impedance antennaImpedance) {#ifdef DEBUG dev_dbg (&spi->dev,"set: antenna impedance" );
@@ -359,7 +359,7 @@ int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance ant } } -int rf69_set_lna_gain (struct spi_device *spi,enum lnaGain lnaGain) +int rf69_set_lna_gain (struct spi_device *spi,enum lna_gain lnaGain) {#ifdef DEBUG dev_dbg (&spi->dev,"set: lna gain" );
@@ -377,7 +377,7 @@ int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain) } } -enum lnaGain rf69_get_lna_gain (struct spi_device *spi) +enum lna_gain rf69_get_lna_gain (struct spi_device *spi) {u8 currentValue;
@@ -516,7 +516,7 @@ int rf69_set_ook_threshold_step(struct spi_device *spi, enum thresholdStep thres } } -int rf69_set_ook_threshold_dec (struct spi_device *spi,enum thresholdDecrement thresholdDecrement) +int rf69_set_ook_threshold_dec (struct spi_device *spi,enum threshold_decrement thresholdDecrement) {#ifdef DEBUG dev_dbg (&spi->dev,"set: threshold decrement" );
@@ -666,7 +666,7 @@ int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength)return retval; } -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 optionOnOff) {#ifdef DEBUG dev_dbg (&spi->dev,"set: sync enable" );
@@ -753,7 +753,7 @@ int rf69_set_packet_format(struct spi_device * spi, enum packetFormat packetForm } } -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 optionOnOff) {#ifdef DEBUG dev_dbg (&spi->dev,"set: crc enable" );
@@ -766,7 +766,7 @@ int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff) } } -int rf69_set_adressFiltering (struct spi_device *spi,enum addressFiltering addressFiltering) +int rf69_set_adressFiltering (struct spi_device *spi,enum address_filtering addressFiltering) {#ifdef DEBUG dev_dbg (&spi->dev,"set: address filtering" );
@@ -816,7 +816,7 @@ int rf69_set_broadcast_address(struct spi_device *spi, u8 broadcastAddress)return WRITE_REG (REG_BROADCASTADRS , broadcastAddress); } -int rf69_set_tx_start_condition (struct spi_device *spi,enum txStartCondition txStartCondition) +int rf69_set_tx_start_condition (struct spi_device *spi,enum tx_start_condition txStartCondition) {#ifdef DEBUG dev_dbg (&spi->dev,"set: start condition" );
drivers/staging/pi433/rf69.h +13 -13
@@ -29,18 +29,18 @@int rf69_set_data_mode (struct spi_device *spi,enum dataMode dataMode);int rf69_set_modulation (struct spi_device *spi,enum modulation modulation);enum modulation rf69_get_modulation (struct spi_device *spi); -int rf69_set_modulation_shaping (struct spi_device *spi,enum modShaping modShaping); +int rf69_set_modulation_shaping (struct spi_device *spi,enum mod_shaping modShaping);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 optionOnOff); +int rf69_set_amplifier_1 (struct spi_device *spi,enum option_on_off optionOnOff); +int rf69_set_amplifier_2 (struct spi_device *spi,enum option_on_off optionOnOff);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); -int rf69_set_lna_gain (struct spi_device *spi,enum lnaGain lnaGain); -enum lnaGain rf69_get_lna_gain (struct spi_device *spi); +int rf69_set_pa_ramp (struct spi_device *spi,enum pa_ramp paRamp); +int rf69_set_antenna_impedance (struct spi_device *spi,enum antenna_impedance antennaImpedance); +int rf69_set_lna_gain (struct spi_device *spi,enum lna_gain lnaGain); +enum lna_gain rf69_get_lna_gain (struct spi_device *spi);int rf69_set_dc_cut_off_frequency_intern (struct spi_device *spi,u8 reg,enum dccPercent dccPercent);int rf69_set_dc_cut_off_frequency (struct spi_device *spi,enum dccPercent dccPercent);int rf69_set_dc_cut_off_frequency_during_afc (struct spi_device *spi,enum dccPercent dccPercent);
@@ -48,7 +48,7 @@int rf69_set_bandwidth_during_afc (struct spi_device *spi,enum mantisse mantisse,u8 exponent);int rf69_set_ook_threshold_type (struct spi_device *spi,enum thresholdType thresholdType);int rf69_set_ook_threshold_step (struct spi_device *spi,enum thresholdStep thresholdStep); -int rf69_set_ook_threshold_dec (struct spi_device *spi,enum thresholdDecrement thresholdDecrement); +int rf69_set_ook_threshold_dec (struct spi_device *spi,enum threshold_decrement thresholdDecrement);int rf69_set_dio_mapping (struct spi_device *spi,u8 DIONumber,u8 value);bool rf69_get_flag (struct spi_device *spi,enum flag flag);int rf69_reset_flag (struct spi_device *spi,enum flag flag);
@@ -56,19 +56,19 @@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 optionOnOff);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_adressFiltering (struct spi_device *spi,enum addressFiltering addressFiltering); +int rf69_set_crc_enable (struct spi_device *spi,enum option_on_off optionOnOff); +int rf69_set_adressFiltering (struct spi_device *spi,enum address_filtering addressFiltering);int rf69_set_payload_length (struct spi_device *spi,u8 payloadLength);u8 rf69_get_payload_length (struct spi_device *spi);int rf69_set_node_address (struct spi_device *spi,u8 nodeAddress);int rf69_set_broadcast_address (struct spi_device *spi,u8 broadcastAddress); -int rf69_set_tx_start_condition (struct spi_device *spi,enum txStartCondition txStartCondition); +int rf69_set_tx_start_condition (struct spi_device *spi,enum tx_start_condition txStartCondition);int rf69_set_fifo_threshold (struct spi_device *spi,u8 threshold);int rf69_set_dagc (struct spi_device *spi,enum dagc dagc);
drivers/staging/pi433/rf69_enum.h +8 -8
@@ -18,7 +18,7 @@#ifndef RF69_ENUM_H #define RF69_ENUM_H -enum optionOnOff +enum option_on_off { optionOff, optionOn
@@ -46,7 +46,7 @@ enum modulationFSK }; -enum modShaping +enum mod_shaping { shapingOff, shaping1_0,
@@ -56,7 +56,7 @@ enum modShapingshaping2BR }; -enum paRamp +enum pa_ramp { ramp3400, ramp2000,
@@ -76,13 +76,13 @@ enum paRampramp10 }; -enum antennaImpedance +enum antenna_impedance { fiftyOhm, twohundretOhm }; -enum lnaGain +enum lna_gain { automatic, max,
@@ -132,7 +132,7 @@ enum thresholdStepstep_6_0db }; -enum thresholdDecrement +enum threshold_decrement { dec_every8th, dec_every4th,
@@ -177,13 +177,13 @@ enum packetFormatpacketLengthVar }; -enum txStartCondition +enum tx_start_condition { fifoLevel, fifoNotEmpty }; -enum addressFiltering +enum address_filtering { filteringOff, nodeAddress,
--
1.9.1
PATCH v2 4/4 staging: pi433: Remove camel case variable names
In reply to Rishabh HardasSigned-off-by Rishabh Hardas <[email protected]>
--- drivers/staging/pi433/pi433_if.c | 4 ++-- drivers/staging/pi433/pi433_if.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)
drivers/staging/pi433/pi433_if.c +2 -2
@@ -192,7 +192,7 @@ struct pi433_instance {SET_CHECKED (rf69_set_modulation (dev->spi, rx_cfg->modulation));SET_CHECKED (rf69_set_antenna_impedance (dev->spi, rx_cfg->antenna_impedance));SET_CHECKED (rf69_set_rssi_threshold (dev->spi, rx_cfg->rssi_threshold)); -SET_CHECKED (rf69_set_ook_threshold_dec (dev->spi, rx_cfg->thresholdDecrement)); +SET_CHECKED (rf69_set_ook_threshold_dec (dev->spi, rx_cfg->threshold_decrement));SET_CHECKED (rf69_set_bandwidth (dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));SET_CHECKED (rf69_set_bandwidth_during_afc (dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));SET_CHECKED (rf69_set_dagc (dev->spi, rx_cfg->dagc));
@@ -254,7 +254,7 @@ struct pi433_instance {SET_CHECKED (rf69_set_modulation (dev->spi, tx_cfg->modulation));SET_CHECKED (rf69_set_deviation (dev->spi, tx_cfg->dev_frequency));SET_CHECKED (rf69_set_pa_ramp (dev->spi, tx_cfg->pa_ramp)); -SET_CHECKED (rf69_set_modulation_shaping (dev->spi, tx_cfg->modShaping)); +SET_CHECKED (rf69_set_modulation_shaping (dev->spi, tx_cfg->mod_shaping));SET_CHECKED (rf69_set_tx_start_condition (dev->spi, tx_cfg->tx_start_condition));/* packet format enable */
drivers/staging/pi433/pi433_if.h +3 -3
@@ -62,7 +62,7 @@ struct pi433_tx_cfg {__u16 bit_rate;__u32 dev_frequency;enum modulation modulation; -enum mod_shaping modShaping; +enum mod_shaping mod_shaping;enum pa_ramp pa_ramp;
@@ -114,8 +114,8 @@ struct pi433_rx_cfg {__u8 rssi_threshold; -enum threshold_decrement thresholdDecrement; -enum antenna_impedance antenna_impedance; +enum threshold_decrement threshold_decrement; +enum antenna_impedance antenna_impedance;enum lna_gain lna_gain;enum mantisse bw_mantisse;/* normal: 0x50 */ __u8 bw_exponent;/* during AFC: 0x8b */
--
1.9.1
PATCH v2 2/4 staging: pi433: Change Comments
In reply to Rishabh HardasSigned-off-by Rishabh Hardas <[email protected]>
--- drivers/staging/pi433/pi433_if.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/staging/pi433/pi433_if.h +12 -3
@@ -126,9 +126,18 @@ struct pi433_rx_cfg {/* 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 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 +*/ __u8 sync_length;__u8 fixed_message_length;
--
1.9.1
Greg KH
maintainerRe: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
In reply to Rishabh HardasQuoted 2 lines · Rishabh Hardas
Signed-off-by Rishabh Hardas <[email protected]>---
I can not take patches without any changelog text at all, sorry.
Please fix up and resend this series.
thanks,
greg k-h
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
In reply to Rishabh HardasQuoted 7 lines · Rishabh Hardas
drivers/staging/pi433/pi433_if.h +0
-3@@ -143,10 +142,13 @@ struct pi433_rx_cfg {#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)]) -
drivers/staging/pi433/pi433_if.h struct pi433_rx_cfg { · line 152
Quoted 10 lines
drivers/staging/pi433/pi433_if.h +8
-2-#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_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)])
These don't help readability. The original was better.
regards,
dan carpenter
Re: [PATCH v2 3/4] staging: pi433: Renaming Enums
In reply to Rishabh HardasQuoted 15 lines · Rishabh Hardas
Remove camel casing by renaming enums.
Signed-off-by Rishabh Hardas <[email protected]>--- drivers/staging/pi433/pi433_if.h | 36 ++++++++++++++++-------------------- drivers/staging/pi433/rf69.c | 26 +++++++++++++------------- drivers/staging/pi433/rf69.h | 26 +++++++++++++------------- drivers/staging/pi433/rf69_enum.h | 16 ++++++++-------- 4 files changed, 50 insertions(+), 54 deletions(-)drivers/staging/pi433/pi433_if.h +0
-0
drivers/staging/pi433/pi433_if.h struct pi433_tx_cfg { · line 65
Quoted 5 lines
drivers/staging/pi433/pi433_if.h +1
-1@@ -62,21 +62,20 @@ struct pi433_tx_cfg {__u16 bit_rate;__u32 dev_frequency;enum modulation modulation; -enum modShaping modShaping; +enum mod_shaping modShaping;
Now the names don't line up.
regards,
dan carpenter
Re: [PATCH v2 2/4] staging: pi433: Change Comments
In reply to Rishabh Hardasdrivers/staging/pi433/pi433_if.h struct pi433_rx_cfg { · line 140
Quoted 5 lines · Rishabh Hardas
drivers/staging/pi433/pi433_if.h +5
-0+enum optionOnOff enable_crc;/* only operational, +*if sync on and fixed +* length or length +* byte is used +*/
This could be done better.
regards,
dan carpenter
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
In reply to Dan CarpenterQuoted 5 lines · Dan Carpenter
Quoted 4 lines · Rishabh Hardas
drivers/staging/pi433/pi433_if.h +0
-0@@ -143,10 +142,13 @@ struct pi433_rx_cfg {#define PI433_IOC_MAGIC 'r'
Quoted 16 lines
drivers/staging/pi433/pi433_if.h struct pi433_rx_cfg { · line 152
Quoted 13 lines
drivers/staging/pi433/pi433_if.h +8
-5-#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_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)])These don't help readability. The original was better.
The original wasn't any good either.
It'd be better to avoid the macros altogether
as almost all are use-once.
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
Quoted 23 lines · Joe Perches
Quoted 5 lines · Dan Carpenter
Quoted 4 lines · Rishabh Hardas
drivers/staging/pi433/pi433_if.h +0
-0@@ -143,10 +142,13 @@ struct pi433_rx_cfg {#define PI433_IOC_MAGIC 'r'Quoted 16 lines
drivers/staging/pi433/pi433_if.h struct pi433_rx_cfg { · line 152
Quoted 13 lines
drivers/staging/pi433/pi433_if.h +8
-5-#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_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)])These don't help readability. The original was better.
Quoted 6 lines
The original wasn't any good either.
It'd be better to avoid the macros altogether
as almost all are use-once.
So should I keep this as it is or remove the macros ? Because as Dan said the corrections that I made aren't goo either.
Regards
Rishabh Hardas
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
Quoted 24 lines · Rishabh Hardas
Quoted 23 lines · Joe Perches
Quoted 21 lines · Dan Carpenter
Quoted 7 lines · Rishabh Hardas
drivers/staging/pi433/pi433_if.h +0
-3@@ -143,10 +142,13 @@ struct pi433_rx_cfg {#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)]) -drivers/staging/pi433/pi433_if.h struct pi433_rx_cfg { · line 152
Quoted 10 lines
drivers/staging/pi433/pi433_if.h +8
-2-#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_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)])These don't help readability. The original was better.
Quoted 9 lines
Quoted 6 lines
The original wasn't any good either.
It'd be better to avoid the macros altogether
as almost all are use-once.So should I keep this as it is or remove the macros ? Because as Dan said the corrections that I made aren't goo either.
Find a way to correct it which makes the code more readable than it was before.
regards,
dan carpenter
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
Hi everybody,
from my point of view, we should stay with the old implementation.
Ok - line is too long according to style guide. But these long lines are IMHO easy to read:
All four are pretty similar. By having all Tokens in exact the same length and having one below other, you can easily detect the differences between the lines and that's important. As soon as you start to wrap them
- regardless how - you won't be able to detect the differences that easy any more - and from my Point of view that's a disadvantage.
Cheers,
Marcus
Quoted 53 lines
Dan Carpenter <[email protected]> hat am 22. August 2017 um 12:03 geschrieben:
Quoted 32 lines · Rishabh Hardas
Quoted 31 lines · Joe Perches
Quoted 29 lines · Dan Carpenter
Quoted 17 lines · Rishabh Hardas
(quoted diff) +0
-5@@ -143,10 +142,13 @@ struct pi433_rx_cfg { #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_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(structpi433_rx_cfg)]) +#definePI433_IOC_RD_TX_CFG_IOR(PI433_IOC_MAGIC, PI433_TX_CFG_IOCTL_NR,\
+ char[sizeof(structpi433_tx_cfg)])
+#definePI433_IOC_WR_TX_CFG_IOW(PI433_IOC_MAGIC,Quoted 8 lines
PI433_TX_CFG_IOCTL_NR,\
+ char[sizeof(structpi433_tx_cfg)]) +#definePI433_IOC_RD_RX_CFG_IOR(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR,\
+ char[sizeof(structpi433_rx_cfg)]) +#definePI433_IOC_WR_RX_CFG_IOW(PI433_IOC_MAGIC, PI433_RX_CFG_IOCTL_NR,\
+ char[sizeof(structpi433_rx_cfg)])These don't help readability. The original was better.
Quoted 9 lines
Quoted 6 lines
The original wasn't any good either.
It'd be better to avoid the macros altogether
as almost all are use-once.So should I keep this as it is or remove the macros ? Because as Dan said the corrections that I made aren't goo either.
Find a way to correct it which makes the code more readable than it was before.
regards,
dan carpenter
Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines
I'm skeptical that these are the absolute platonic ideal of what the code should look like... You may be right that it's mathematically impossible to make these lines more readable, but we can't really debate alternate versions of this until someone sends us a patch.
regards,
dan carpenter