LinuxLists.cc - [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

[permalink] [raw]

Subject: [PATCH 2/5] staging/pi433/pi433_if.c:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Signed-off-by: Rishabh Hardas <[email protected]>
---
drivers/staging/pi433/pi433_if.c | 4 ++--
1 file changed, 2 insertions(+), 2 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 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
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,5 +254,5 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
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 */
--
2.7.4

2017-08-01 19:33:21

[permalink] [raw]

Subject: [PATCH 3/5] staging/pi433/rf69.h:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

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

diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index b81e076..e98e24e 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -29,18 +29,18 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode);
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 lnagain lnaGain);
+enum lnagain 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(struct spi_device *spi, enum mantisse mantisse, u8 expone
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,18 +56,18 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8 threshold);
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);

--
2.7.4

2017-08-01 19:33:27

[permalink] [raw]

Subject: [PATCH 4/5] staging/pi433/rf69.c:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Signed-off-by: Rishabh Hardas <[email protected]>
---
drivers/staging/pi433/rf69.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index f83523e..7fa63e1 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -109,2 +109,2 @@ 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,2 +264,2 @@ 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,2 +277,2 @@ 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,2 +290,2 @@ 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,2 +319,2 @@ 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,2 +346,2 @@ 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,2 +359,2 @@ 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 lnagain lnaGain)
{
#ifdef DEBUG
dev_dbg(&spi->dev, "set: lna gain");
@@ -377,2 +377,2 @@ int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain)
}
}

-enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
+enum lnagain rf69_get_lna_gain(struct spi_device *spi)
{
u8 currentValue;

@@ -516,2 +516,2 @@ 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,2 +666,2 @@ 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,2 +753,2 @@ 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,2 +766,2 @@ 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,2 +816,2 @@ 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");
--
2.7.4

2017-08-01 19:33:32

[permalink] [raw]

Subject: [PATCH 5/5] staging/pi433/rf69_enum.h:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

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

diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
index fbfb59b..f51eec3 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,2 +18,2 @@
#ifndef RF69_ENUM_H
#define RF69_ENUM_H

-enum optionOnOff
+enum option_on_off
{
optionOff,
optionOn
@@ -46,2 +46,2 @@ enum modulation
FSK
};

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

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

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

-enum lnaGain
+enum lnagain
{
automatic,
max,
@@ -132,2 +132,2 @@ enum thresholdStep
step_6_0db
};

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

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

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

2017-08-02 08:08:07

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Reviewed-by: Marcus Wolf <[email protected]>

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 <[email protected]>
> ---
> 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
>
>
>

2017-08-02 08:09:30

[permalink] [raw]

Subject: Re: [PATCH 2/5] staging/pi433/pi433_if.c:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

2017-08-02 08:10:41

[permalink] [raw]

Subject: Re: [PATCH 3/5] staging/pi433/rf69.h:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Reviewed-by: Marcus Wolf <[email protected]>

Am Di, 1.08.2017, 21:31 schrieb Rishabh Hardas:
> Signed-off-by: Rishabh Hardas <[email protected]>
> ---
> drivers/staging/pi433/rf69.h | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> index b81e076..e98e24e 100644
> --- a/drivers/staging/pi433/rf69.h
> +++ b/drivers/staging/pi433/rf69.h
> @@ -29,18 +29,18 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode);
> 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 lnagain lnaGain);
> +enum lnagain 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(struct spi_device *spi, enum mantisse mantisse, u8 expone
> 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,18 +56,18 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8 threshold);
> 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);
>
> --
> 2.7.4
>
>
>

2017-08-02 08:13:08

[permalink] [raw]

Subject: Re: [PATCH 4/5] staging/pi433/rf69.c:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Reviewed-by: Marcus Wolf <[email protected]>

Maybe lnaGain should move to lna_gain instead of lnagain (also applies to the headers...)

Marcus

Am Di, 1.08.2017, 21:31 schrieb Rishabh Hardas:
> Signed-off-by: Rishabh Hardas <[email protected]>
> ---
> drivers/staging/pi433/rf69.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index f83523e..7fa63e1 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -109,2 +109,2 @@ 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,2 +264,2 @@ 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,2 +277,2 @@ 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,2 +290,2 @@ 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,2 +319,2 @@ 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,2 +346,2 @@ 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,2 +359,2 @@ 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 lnagain lnaGain)
> {
> #ifdef DEBUG
> dev_dbg(&spi->dev, "set: lna gain");
> @@ -377,2 +377,2 @@ int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain)
> }
> }
>
> -enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
> +enum lnagain rf69_get_lna_gain(struct spi_device *spi)
> {
> u8 currentValue;
>
> @@ -516,2 +516,2 @@ 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,2 +666,2 @@ 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,2 +753,2 @@ 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,2 +766,2 @@ 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,2 +816,2 @@ 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");
> --
> 2.7.4
>
>
>

2017-08-02 08:14:07

[permalink] [raw]

Subject: Re: [PATCH 5/5] staging/pi433/rf69_enum.h:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

2017-08-02 08:19:39

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

The subject is too long.

On Wed, Aug 02, 2017 at 01:01:55AM +0530, Rishabh Hardas wrote:
> 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

This looks like the description of 5 patches, but that should be in the
[PATCH 0/5] summary. What we want here is the description of this
patch.

This patch does too many things, deleting stuff, breaking up long lines,
changing comments and renaming enums. It should be 4 patches:
[patch 1] delete stuff
[patch 2] break long lines
...
etc.

regards,
dan carpenter

2017-08-02 08:23:24

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

This patch is going to break the build. Every patch has to be able to
be compiled so we can use git bisect. You'll need to redo the whole
series.

regards,
dan carpenter

2017-08-02 08:26:38

[permalink] [raw]

Subject: Re: [PATCH 2/5] staging/pi433/pi433_if.c:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

The subject is too vague. The patch prefix isn't right. Do
`git log --oneline drivers/staging/pi433/` to see what other people are
doing:

99859541a92d staging: pi433: use div_u64 for 64-bit division
056eeda2f9e6 staging: pi433: Style fix - align block comments
775f6ab013d3 staging: pi433: Make functions rf69_set_bandwidth_intern static
c7d42f37087d Staging: pi433: check error after kthread_run()
7de77a3917e3 Staging: pi433: declare functions static
0119a48b69fe staging: pi433: depends on SPI

So the subject should be something like:

[PATCH] staging: pi433: rename camel case variables

Also there was no patch description and that's required.

regards,
dan carpenter

2017-08-02 08:34:22

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

On Wed, Aug 02, 2017 at 10:08:04AM +0200, Wolf Entwicklungen wrote:
> Reviewed-by: Marcus Wolf <[email protected]>
>
> Just reviewed, not tested.
> As far as I can see, there is no technical issue with this patch.

You need to be a bit more strict in your reviews... There were a few
obvious problems in this patchset. These are show stoppers:
1) Breaks git bisect
2) Doing multiple things in the same patch
3) No changelog

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

Camel case are unwanted.

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

Rishabh is going to have to redo the patchset anyway so don't feel bad
about asking for changes. Put these review comments next to the change
you are complaining about.

No top posting.

regards,
dan carpenter

2017-08-02 08:52:42

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Hi!

Ok. Didn't know that I have to check for such stuff.

So far i was just checking the code changes, not the style of the patch itself.

Will try to be more strict...

Is it mandatory, that I compile the code, or is it ok, if I do the review "just"
by reading?
Reason for the question: If reading is ok, too, I can do a review of a simple
change,
even when I am abroad at a customer (my customers do not deal with linux, just
comercial stuff). With compiling, I can only do them at home...

Cheers,

Marcus

> Dan Carpenter <[email protected]> hat am 2. August 2017 um 10:34
> geschrieben:
>
>
> On Wed, Aug 02, 2017 at 10:08:04AM +0200, Wolf Entwicklungen wrote:
> > Reviewed-by: Marcus Wolf <[email protected]>
> >
> > Just reviewed, not tested.
> > As far as I can see, there is no technical issue with this patch.
>
> You need to be a bit more strict in your reviews... There were a few
> obvious problems in this patchset. These are show stoppers:
> 1) Breaks git bisect
> 2) Doing multiple things in the same patch
> 3) No changelog
>
> >
> > 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.
>
> Camel case are unwanted.
>
> >
> > 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.
>
> Rishabh is going to have to redo the patchset anyway so don't feel bad
> about asking for changes. Put these review comments next to the change
> you are complaining about.
>
> No top posting.
>
> regards,
> dan carpenter
>
>
>

2017-08-02 09:00:03

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

I seldom Ack patches or add Reviewed-by tags. Greg is going to compile
test them all so that buys you a little slack but if you were committing
patches yourself and you broke the build everyone would get annoyed with
you. Just put a comment if you can't build test things.

regards,
dan carpenter

2017-08-02 09:15:19

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

Hi Dan,

I offered Greg to have an eye on the patches, that were submitted. He asked
me to reply to those patches, I had a look for, with the Reviewed-by tag.

Now I am a bit unsure, what I should do.

Since I am interested in, I for sure will read every patch. It's just a little
thing, to reply with reviewed-tag - if you / Greg like(s). I can do so on every
reviewed patch, only on patches without need/wish of improvement or never.

Compiling every patch is a bit inconvenient.

Please let me know, what you and Greg would like.

cheers,

Marcus

Am Mi, 2.08.2017, 10:59 schrieb Dan Carpenter:
> I seldom Ack patches or add Reviewed-by tags. Greg is going to compile
> test them all so that buys you a little slack but if you were committing
> patches yourself and you broke the build everyone would get annoyed with
> you. Just put a comment if you can't build test things.
>
> regards,
> dan carpenter
>
>
>

2017-08-02 09:33:22

[permalink] [raw]

Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

On Wed, Aug 02, 2017 at 11:15:16AM +0200, Wolf Entwicklungen wrote:
> Hi Dan,
>
> I offered Greg to have an eye on the patches, that were submitted. He asked
> me to reply to those patches, I had a look for, with the Reviewed-by tag.
>
> Now I am a bit unsure, what I should do.
>
> Since I am interested in, I for sure will read every patch. It's just a little
> thing, to reply with reviewed-tag - if you / Greg like(s). I can do so on every
> reviewed patch, only on patches without need/wish of improvement or never.
>
> Compiling every patch is a bit inconvenient.
>
> Please let me know, what you and Greg would like.

That's fine, you don't need to build test them if you don't want to.
Greg is going to do it.

But in this case, it was pretty obvious that it was going to break git
bisect right? That's not allowed. You're not expected to know all the
rules when you start so that's fine. I'm just trying to give you some
ideas what to look for in the future.

regards,
dan carpenter

2017-08-02 10:51:40