Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752019AbdHBIIH (ORCPT ); Wed, 2 Aug 2017 04:08:07 -0400 Received: from mailout02.webmailer.hosteurope.de ([80.237.138.58]:52732 "EHLO mailout02.webmailer.hosteurope.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751145AbdHBIIG (ORCPT ); Wed, 2 Aug 2017 04:08:06 -0400 X-Squirrel-UserHash: EhVcX1pHQwdXWkQFBhENSgEKLlwACw== X-Squirrel-FromHash: UwFaC1lCF1E= Message-ID: <88d4ff61d322e563ffd52f7375227f8d-EhVcX1pHQwdXWkQFBhENSgEKLlwACzJXX19HAVhEWENbS1kLMF52CEtUX1pBSEwcXlJRL1lQWAlZWXcDXVE=-webmailer1@server03.webmailer.webmailer.hosteurope.de> In-Reply-To: <1501615919-31875-1-git-send-email-rishabhhardas@gmail.com> References: <1501615919-31875-1-git-send-email-rishabhhardas@gmail.com> Date: Wed, 2 Aug 2017 10:08:04 +0200 Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it. From: "Wolf Entwicklungen" To: "Rishabh Hardas" Cc: gregkh@linuxfoundation.org, linux@Wolf-Entwicklungen.de, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, "Rishabh Hardas" Reply-To: Marcus.Wolf@Wolf-Entwicklungen.de User-Agent: Host Europe Webmailer/1.0 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT X-Priority: 3 (Normal) Importance: Normal X-KLMS-Rule-ID: 1 X-KLMS-Message-Action: clean X-KLMS-AntiSpam-Status: not scanned, license restriction X-KLMS-AntiPhishing: not scanned, license restriction X-KLMS-AntiVirus: Kaspersky Security 8.0 for Linux Mail Server, version 8.0.1.721, bases: 2017/08/02 01:57:00 #10257287; khse: 2015-01-01 01:01:01 X-KLMS-AntiVirus-Status: Clean, skipped X-HE-Access: Yes X-bounce-key: webpack.hosteurope.de;marcus.wolf@wolf-entwicklungen.de;1501661284;eb4114dd; Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6032 Lines: 178 Reviewed-by: Marcus Wolf 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 > --- > 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 > #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 > > >