2017-08-03 11:53:14

by Rishabh Hardas

[permalink] [raw]
Subject: [PATCH v2 0/4] 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


2017-08-03 11:53:18

by Rishabh Hardas

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index e6ed3cd..91e4a01 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -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 1
struct 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

2017-08-03 11:53:24

by Rishabh Hardas

[permalink] [raw]
Subject: [PATCH v2 3/4] staging: pi433: 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(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 84032f3..2929de0 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -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,\
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index f83523e..b7b8c7c 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -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");
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index b81e076..b548374 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -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);

diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
index fbfb59b..7121200 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -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 modulation
FSK
};

-enum modShaping
+enum mod_shaping
{
shapingOff,
shaping1_0,
@@ -56,7 +56,7 @@ enum modShaping
shaping2BR
};

-enum paRamp
+enum pa_ramp
{
ramp3400,
ramp2000,
@@ -76,13 +76,13 @@ enum paRamp
ramp10
};

-enum antennaImpedance
+enum antenna_impedance
{
fiftyOhm,
twohundretOhm
};

-enum lnaGain
+enum lna_gain
{
automatic,
max,
@@ -132,7 +132,7 @@ enum thresholdStep
step_6_0db
};

-enum thresholdDecrement
+enum threshold_decrement
{
dec_every8th,
dec_every4th,
@@ -177,13 +177,13 @@ enum packetFormat
packetLengthVar
};

-enum txStartCondition
+enum tx_start_condition
{
fifoLevel,
fifoNotEmpty
};

-enum addressFiltering
+enum address_filtering
{
filteringOff,
nodeAddress,
--
1.9.1

2017-08-03 11:53:36

by Rishabh Hardas

[permalink] [raw]
Subject: [PATCH v2 4/4] staging: pi433: Remove camel case variable names

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

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index ed737f4..11c042b 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -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 */
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 2929de0..7f57e7d 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -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

2017-08-03 11:54:00

by Rishabh Hardas

[permalink] [raw]
Subject: [PATCH v2 2/4] staging: pi433: Change Comments

Signed-off-by: Rishabh Hardas <[email protected]>
---
drivers/staging/pi433/pi433_if.h | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 91e4a01..84032f3 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -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

2017-08-15 00:42:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

On Thu, Aug 03, 2017 at 05:20:43PM +0530, Rishabh Hardas wrote:
> 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

2017-08-16 07:31:22

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> @@ -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)])


These don't help readability. The original was better.

regards,
dan carpenter

2017-08-16 07:34:26

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] staging: pi433: Renaming Enums

On Wed, Aug 16, 2017 at 10:53:20AM +0530, Rishabh Hardas wrote:
> 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(-)
>
> diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> index 84032f3..2929de0 100644
> --- a/drivers/staging/pi433/pi433_if.h
> +++ b/drivers/staging/pi433/pi433_if.h
> @@ -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

2017-08-16 07:35:33

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] staging: pi433: Change Comments

On Wed, Aug 16, 2017 at 10:53:19AM +0530, Rishabh Hardas wrote:
> + 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

2017-08-20 04:47:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

On Wed, 2017-08-16 at 10:31 +0300, Dan Carpenter wrote:
> On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> > @@ -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)])
>
>
> 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.


2017-08-22 09:28:09

by Rishabh Hardas

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

On Sat, Aug 19, 2017 at 09:47:28PM -0700, Joe Perches wrote:
> On Wed, 2017-08-16 at 10:31 +0300, Dan Carpenter wrote:
> > On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> > > @@ -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)])
> >
> >
> > 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.
>
>
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

2017-08-22 10:03:56

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] staging: pi433: Style fix - Correct long lines

On Tue, Aug 22, 2017 at 02:57:49PM +0530, Rishabh Hardas wrote:
> On Sat, Aug 19, 2017 at 09:47:28PM -0700, Joe Perches wrote:
> > On Wed, 2017-08-16 at 10:31 +0300, Dan Carpenter wrote:
> > > On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> > > > @@ -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)])
> > >
> > >
> > > 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.
> >
> >
> 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

2017-08-22 10:30:05

by Marcus Wolf

[permalink] [raw]
Subject: 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

> Dan Carpenter <[email protected]> hat am 22. August 2017 um 12:03
> geschrieben:
>
>
> On Tue, Aug 22, 2017 at 02:57:49PM +0530, Rishabh Hardas wrote:
> > On Sat, Aug 19, 2017 at 09:47:28PM -0700, Joe Perches wrote:
> > > On Wed, 2017-08-16 at 10:31 +0300, Dan Carpenter wrote:
> > > > On Wed, Aug 16, 2017 at 10:53:18AM +0530, Rishabh Hardas wrote:
> > > > > @@ -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)])
> > > >
> > > >
> > > > 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.
> > >
> > >
> > 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
>

2017-08-22 10:44:06

by Dan Carpenter

[permalink] [raw]
Subject: 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