[PATCH 0/6] staging: pi433: Minor cleanup and style fixes

[PATCH 0/6] staging: pi433: Minor cleanup and style fixes

[Simon Sandström]

These are the six remaining patches from "[PATCH v2 00/11] Fix indentation and
CamelCase issues in staging/pi433" that couldn't be applied, rebased on top of
staging-next.

- Simon

---

Simon Sandström (6):
  staging: pi433: Split rf69_set_crc_enabled into two functions
  staging: pi433: Split rf69_set_sync_enabled into two functions
  staging: pi433: Remove enum data_mode
  staging: pi433: Combine all rf69_set_amplifier_x()
  staging: pi433: Move enum option_on_off to pi433_if.h
  staging: pi433: Remove SET_CHECKED usage from pi433_probe

 drivers/staging/pi433/pi433_if.c  | 71 +++++++++++++++++++++++-----
 drivers/staging/pi433/pi433_if.h  |  5 ++
 drivers/staging/pi433/rf69.c      | 97 ++++++++-------------------------------
 drivers/staging/pi433/rf69.h      | 18 +++-----
 drivers/staging/pi433/rf69_enum.h | 11 -----
 5 files changed, 90 insertions(+), 112 deletions(-)

-- 
2.11.0

[PATCH 1/6] staging: pi433: Split rf69_set_crc_enabled into two functions

[Simon Sandström]

Splits rf69_set_crc_enabled(dev, enabled) into
rf69_enable_crc(dev) and rf69_disable_crc(dev).

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.c | 22 ++++++++++++++++++++--
 drivers/staging/pi433/rf69.c     | 18 ++++++------------
 drivers/staging/pi433/rf69.h     |  4 ++--
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2ae19ac565d1..614eec7dd904 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
 			return ret;
 	}
 	SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
-	SET_CHECKED(rf69_set_crc_enable	    (dev->spi, rx_cfg->enable_crc));
+
+	if (rx_cfg->enable_crc == OPTION_ON) {
+		ret = rf69_enable_crc(dev->spi);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = rf69_disable_crc(dev->spi);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* lengths */
 	SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
@@ -282,7 +291,16 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 		if (ret < 0)
 			return ret;
 	}
-	SET_CHECKED(rf69_set_crc_enable	  (dev->spi, tx_cfg->enable_crc));
+
+	if (tx_cfg->enable_crc == OPTION_ON) {
+		ret = rf69_enable_crc(dev->spi);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = rf69_disable_crc(dev->spi);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* configure sync, if enabled */
 	if (tx_cfg->enable_sync == OPTION_ON) {
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index c97fd8031ecb..8c9c9bb91c53 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -844,20 +844,14 @@ int rf69_set_packet_format(struct spi_device *spi, enum packetFormat packetForma
 	}
 }
 
-int rf69_set_crc_enable(struct spi_device *spi,
-			enum option_on_off option_on_off)
+int rf69_enable_crc(struct spi_device *spi)
 {
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: crc enable");
-	#endif
+	return rf69_set_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
+}
 
-	switch (option_on_off) {
-	case OPTION_ON: return rf69_set_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
-	case OPTION_OFF: return rf69_clear_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
+int rf69_disable_crc(struct spi_device *spi)
+{
+	return rf69_clear_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
 }
 
 int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering addressFiltering)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 1cb6db33d6fe..9428dee97de7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -66,8 +66,8 @@ 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 option_on_off option_on_off);
+int rf69_enable_crc(struct spi_device *spi);
+int rf69_disable_crc(struct spi_device *spi);
 int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering addressFiltering);
 int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
 u8  rf69_get_payload_length(struct spi_device *spi);
-- 
2.11.0

Re: [PATCH 1/6] staging: pi433: Split rf69_set_crc_enabled into two functions

[Greg KH]

On Wed, Dec 06, 2017 at 09:42:19PM +0100, Simon Sandström wrote:
> Splits rf69_set_crc_enabled(dev, enabled) into
> rf69_enable_crc(dev) and rf69_disable_crc(dev).
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>  drivers/staging/pi433/pi433_if.c | 22 ++++++++++++++++++++--
>  drivers/staging/pi433/rf69.c     | 18 ++++++------------
>  drivers/staging/pi433/rf69.h     |  4 ++--
>  3 files changed, 28 insertions(+), 16 deletions(-)

This series did not apply at all.

What tree and branch did you make it against?

Please redo it against my staging.git tree, the staging-next branch
should be fine, or the staging-testing will work as well.

thanks,

greg k-h

Re: [PATCH 1/6] staging: pi433: Split rf69_set_crc_enabled into two functions

[Simon Sandström]

On Thu, Dec 07, 2017 at 03:38:57PM +0100, Greg KH wrote:
> On Wed, Dec 06, 2017 at 09:42:19PM +0100, Simon Sandström wrote:
> > Splits rf69_set_crc_enabled(dev, enabled) into
> > rf69_enable_crc(dev) and rf69_disable_crc(dev).
> > 
> > Signed-off-by: Simon Sandström <simon@nikanor.nu>
> > ---
> >  drivers/staging/pi433/pi433_if.c | 22 ++++++++++++++++++++--
> >  drivers/staging/pi433/rf69.c     | 18 ++++++------------
> >  drivers/staging/pi433/rf69.h     |  4 ++--
> >  3 files changed, 28 insertions(+), 16 deletions(-)
> 
> This series did not apply at all.
> 
> What tree and branch did you make it against?
> 
> Please redo it against my staging.git tree, the staging-next branch
> should be fine, or the staging-testing will work as well.
> 
> thanks,
> 
> greg k-h

Hmm. Haven't you already applied these?

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=39252a4bcf63d9dbc168b9ef56eb4ca42e045b9d

>patch "staging: pi433: Split rf69_set_crc_enabled into two functions" added to staging-testing
>
>This is a note to let you know that I've just added the patch titled
>
>    staging: pi433: Split rf69_set_crc_enabled into two functions
>
> ...


Regards,
Simon

Re: [PATCH 1/6] staging: pi433: Split rf69_set_crc_enabled into two functions

[Greg KH]

On Thu, Dec 07, 2017 at 03:58:01PM +0100, Simon Sandström wrote:
> On Thu, Dec 07, 2017 at 03:38:57PM +0100, Greg KH wrote:
> > On Wed, Dec 06, 2017 at 09:42:19PM +0100, Simon Sandström wrote:
> > > Splits rf69_set_crc_enabled(dev, enabled) into
> > > rf69_enable_crc(dev) and rf69_disable_crc(dev).
> > > 
> > > Signed-off-by: Simon Sandström <simon@nikanor.nu>
> > > ---
> > >  drivers/staging/pi433/pi433_if.c | 22 ++++++++++++++++++++--
> > >  drivers/staging/pi433/rf69.c     | 18 ++++++------------
> > >  drivers/staging/pi433/rf69.h     |  4 ++--
> > >  3 files changed, 28 insertions(+), 16 deletions(-)
> > 
> > This series did not apply at all.
> > 
> > What tree and branch did you make it against?
> > 
> > Please redo it against my staging.git tree, the staging-next branch
> > should be fine, or the staging-testing will work as well.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hmm. Haven't you already applied these?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=39252a4bcf63d9dbc168b9ef56eb4ca42e045b9d
> 
> >patch "staging: pi433: Split rf69_set_crc_enabled into two functions" added to staging-testing
> >
> >This is a note to let you know that I've just added the patch titled
> >
> >    staging: pi433: Split rf69_set_crc_enabled into two functions
> >
> > ...

Odd, I guess so, why did they end up in my mbox again?  strange...

greg k-h

[PATCH 2/6] staging: pi433: Split rf69_set_sync_enabled into two functions

[Simon Sandström]

Splits rf69_set_sync_enabled(dev, enabled) into
rf69_enable_sync(dev) and rf69_disable_sync(dev).

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.c | 21 +++++++++++++++++++--
 drivers/staging/pi433/rf69.c     | 18 ++++++------------
 drivers/staging/pi433/rf69.h     |  4 ++--
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 614eec7dd904..fb500d062df8 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -197,13 +197,20 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
 
 	/* packet config */
 	/* enable */
-	SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
 	if (rx_cfg->enable_sync == OPTION_ON)
 	{
+		ret = rf69_enable_sync(dev->spi);
+		if (ret < 0)
+			return ret;
+
 		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
 	}
 	else
 	{
+		ret = rf69_disable_sync(dev->spi);
+		if (ret < 0)
+			return ret;
+
 		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
 	}
 	if (rx_cfg->enable_length_byte == OPTION_ON) {
@@ -281,7 +288,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 	{
 		SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
 	}
-	SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
+
+	if (tx_cfg->enable_sync == OPTION_ON) {
+		ret = rf69_enable_sync(dev->spi);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = rf69_disable_sync(dev->spi);
+		if (ret < 0)
+			return ret;
+	}
+
 	if (tx_cfg->enable_length_byte == OPTION_ON) {
 		ret = rf69_set_packet_format(dev->spi, packetLengthVar);
 		if (ret < 0)
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 8c9c9bb91c53..12a1091d9936 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -746,20 +746,14 @@ int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength)
 	return retval;
 }
 
-int rf69_set_sync_enable(struct spi_device *spi,
-			 enum option_on_off option_on_off)
+int rf69_enable_sync(struct spi_device *spi)
 {
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: sync enable");
-	#endif
+	return rf69_set_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
+}
 
-	switch (option_on_off) {
-	case OPTION_ON: return rf69_set_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
-	case OPTION_OFF: return rf69_clear_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
+int rf69_disable_sync(struct spi_device *spi)
+{
+	return rf69_clear_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
 }
 
 int rf69_set_fifo_fill_condition(struct spi_device *spi, enum fifoFillCondition fifoFillCondition)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 9428dee97de7..177223451c87 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -59,8 +59,8 @@ 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 option_on_off option_on_off);
+int rf69_enable_sync(struct spi_device *spi);
+int rf69_disable_sync(struct spi_device *spi);
 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);
-- 
2.11.0

[PATCH 3/6] staging: pi433: Remove enum data_mode

[Simon Sandström]

Call rf69_set_data_mode with DATAMODUL_MODE value directly.

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.c  |  2 +-
 drivers/staging/pi433/rf69.c      | 15 ++-------------
 drivers/staging/pi433/rf69.h      |  2 +-
 drivers/staging/pi433/rf69_enum.h |  6 ------
 4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index fb500d062df8..3b4170b9ba94 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1125,7 +1125,7 @@ static int pi433_probe(struct spi_device *spi)
 
 	/* setup the radio module */
 	SET_CHECKED(rf69_set_mode		(spi, standby));
-	SET_CHECKED(rf69_set_data_mode		(spi, packet));
+	SET_CHECKED(rf69_set_data_mode		(spi, DATAMODUL_MODE_PACKET));
 	SET_CHECKED(rf69_set_amplifier_0	(spi, OPTION_ON));
 	SET_CHECKED(rf69_set_amplifier_1	(spi, OPTION_OFF));
 	SET_CHECKED(rf69_set_amplifier_2	(spi, OPTION_OFF));
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 12a1091d9936..9f2ffb89033e 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -85,20 +85,9 @@ 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_data_mode(struct spi_device *spi, u8 data_mode)
 {
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: data mode");
-	#endif
-
-	switch (dataMode) {
-	case packet:		return rf69_read_mod_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_PACKET);
-	case continuous:	return rf69_read_mod_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS);
-	case continuousNoSync:	return rf69_read_mod_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS_NOSYNC);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
+	return rf69_read_mod_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, data_mode);
 }
 
 int rf69_set_modulation(struct spi_device *spi, enum modulation modulation)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 177223451c87..18296b4502f3 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -26,7 +26,7 @@
 #define FIFO_THRESHOLD	15		/* in byte */
 
 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_data_mode(struct spi_device *spi, u8 data_mode);
 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 mod_shaping mod_shaping);
diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
index 97f615effca3..b0715b4eb6ac 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -31,12 +31,6 @@ enum mode {
 	receive
 };
 
-enum dataMode {
-	packet,
-	continuous,
-	continuousNoSync
-};
-
 enum modulation {
 	OOK,
 	FSK
-- 
2.11.0

[PATCH 4/6] staging: pi433: Combine all rf69_set_amplifier_x()

[Simon Sandström]

Replaces the functions rf69_set_amplifier_1, _2, _3 with two
functions: rf69_enable_amplifier(dev, amp_mask) and
rf69_disable_amplifier(dev, amp_mask).

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.c |  6 +++---
 drivers/staging/pi433/rf69.c     | 46 ++++------------------------------------
 drivers/staging/pi433/rf69.h     |  8 ++-----
 3 files changed, 9 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3b4170b9ba94..688d0cf00782 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1126,9 +1126,9 @@ static int pi433_probe(struct spi_device *spi)
 	/* setup the radio module */
 	SET_CHECKED(rf69_set_mode		(spi, standby));
 	SET_CHECKED(rf69_set_data_mode		(spi, DATAMODUL_MODE_PACKET));
-	SET_CHECKED(rf69_set_amplifier_0	(spi, OPTION_ON));
-	SET_CHECKED(rf69_set_amplifier_1	(spi, OPTION_OFF));
-	SET_CHECKED(rf69_set_amplifier_2	(spi, OPTION_OFF));
+	SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
+	SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
+	SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
 	SET_CHECKED(rf69_set_output_power_level	(spi, 13));
 	SET_CHECKED(rf69_set_antenna_impedance	(spi, fiftyOhm));
 
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 9f2ffb89033e..7140fa2ea592 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -282,52 +282,14 @@ int rf69_set_frequency(struct spi_device *spi, u32 frequency)
 	return 0;
 }
 
-int rf69_set_amplifier_0(struct spi_device *spi,
-			 enum option_on_off option_on_off)
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask)
 {
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: amp #0");
-	#endif
-
-	switch (option_on_off) {
-	case OPTION_ON: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA0);
-	case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA0);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
-}
-
-int rf69_set_amplifier_1(struct spi_device *spi,
-			 enum option_on_off option_on_off)
-{
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: amp #1");
-	#endif
-
-	switch (option_on_off) {
-	case OPTION_ON:	return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA1);
-	case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA1);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
+	return rf69_set_bit(spi, REG_PALEVEL, amplifier_mask);
 }
 
-int rf69_set_amplifier_2(struct spi_device *spi,
-			 enum option_on_off option_on_off)
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask)
 {
-	#ifdef DEBUG
-		dev_dbg(&spi->dev, "set: amp #2");
-	#endif
-
-	switch (option_on_off) {
-	case OPTION_ON: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA2);
-	case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA2);
-	default:
-		dev_dbg(&spi->dev, "set: illegal input param");
-		return -EINVAL;
-	}
+	return rf69_clear_bit(spi, REG_PALEVEL, amplifier_mask);
 }
 
 int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 18296b4502f3..ba25ab6aeae7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -33,12 +33,8 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum mod_shaping mod_sha
 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 option_on_off option_on_off);
-int rf69_set_amplifier_1(struct spi_device *spi,
-			 enum option_on_off option_on_off);
-int rf69_set_amplifier_2(struct spi_device *spi,
-			 enum option_on_off option_on_off);
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask);
 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);
-- 
2.11.0

[PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Simon Sandström]

The enum is now only used for ioctl, so move it pi433_if.h.

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.h  | 5 +++++
 drivers/staging/pi433/rf69_enum.h | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index bcfe29840889..c8697d144e2b 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -37,6 +37,11 @@
 
 /*---------------------------------------------------------------------------*/
 
+enum option_on_off {
+	OPTION_OFF,
+	OPTION_ON
+};
+
 /* IOCTL structs and commands */
 
 /**
diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
index b0715b4eb6ac..4e64e38ae4ff 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,11 +18,6 @@
 #ifndef RF69_ENUM_H
 #define RF69_ENUM_H
 
-enum option_on_off {
-	OPTION_OFF,
-	OPTION_ON
-};
-
 enum mode {
 	mode_sleep,
 	standby,
-- 
2.11.0

Re: [PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Marcus Wolf]

Am 06.12.2017 um 22:42 schrieb Simon Sandström:
> The enum is now only used for ioctl, so move it pi433_if.h.
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>   drivers/staging/pi433/pi433_if.h  | 5 +++++
>   drivers/staging/pi433/rf69_enum.h | 5 -----
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> index bcfe29840889..c8697d144e2b 100644
> --- a/drivers/staging/pi433/pi433_if.h
> +++ b/drivers/staging/pi433/pi433_if.h
> @@ -37,6 +37,11 @@
>   
>   /*---------------------------------------------------------------------------*/
>   
> +enum option_on_off {
> +	OPTION_OFF,
> +	OPTION_ON
> +};
> +
>   /* IOCTL structs and commands */
>   
>   /**
> diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
> index b0715b4eb6ac..4e64e38ae4ff 100644
> --- a/drivers/staging/pi433/rf69_enum.h
> +++ b/drivers/staging/pi433/rf69_enum.h
> @@ -18,11 +18,6 @@
>   #ifndef RF69_ENUM_H
>   #define RF69_ENUM_H
>   
> -enum option_on_off {
> -	OPTION_OFF,
> -	OPTION_ON
> -};
> -
>   enum mode {
>   	mode_sleep,
>   	standby,
> 

Hi Simon,

sorry - I have one more question.

Why do you want to get rid of the enum option_on_off in rf69_enum.h. I 
generated that file just to store the enums.

Now we have one enum at an other place...

Regards,
Marcus

Re: [PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Greg KH]

On Thu, Dec 07, 2017 at 12:24:34AM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 22:42 schrieb Simon Sandström:
> > The enum is now only used for ioctl, so move it pi433_if.h.
> > 
> > Signed-off-by: Simon Sandström <simon@nikanor.nu>
> > ---
> >   drivers/staging/pi433/pi433_if.h  | 5 +++++
> >   drivers/staging/pi433/rf69_enum.h | 5 -----
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> > index bcfe29840889..c8697d144e2b 100644
> > --- a/drivers/staging/pi433/pi433_if.h
> > +++ b/drivers/staging/pi433/pi433_if.h
> > @@ -37,6 +37,11 @@
> >   /*---------------------------------------------------------------------------*/
> > +enum option_on_off {
> > +	OPTION_OFF,
> > +	OPTION_ON
> > +};
> > +
> >   /* IOCTL structs and commands */
> >   /**
> > diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h
> > index b0715b4eb6ac..4e64e38ae4ff 100644
> > --- a/drivers/staging/pi433/rf69_enum.h
> > +++ b/drivers/staging/pi433/rf69_enum.h
> > @@ -18,11 +18,6 @@
> >   #ifndef RF69_ENUM_H
> >   #define RF69_ENUM_H
> > -enum option_on_off {
> > -	OPTION_OFF,
> > -	OPTION_ON
> > -};
> > -
> >   enum mode {
> >   	mode_sleep,
> >   	standby,
> > 
> 
> Hi Simon,
> 
> sorry - I have one more question.
> 
> Why do you want to get rid of the enum option_on_off in rf69_enum.h. I
> generated that file just to store the enums.
> 
> Now we have one enum at an other place...

No need to have lots of .h files.   This is just a simple driver, it
should only have 1 .h file at most (for anything needed by userspace).

So moving these all into one file is good.

thanks,

greg k-h

[PATCH 6/6] staging: pi433: Remove SET_CHECKED usage from pi433_probe

[Simon Sandström]

SET_CHECKED returns from the function on failure and in pi433_probe it is
necessary to free the GPIOs and the device on failure.

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/pi433/pi433_if.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 688d0cf00782..55ed45f45998 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1124,13 +1124,27 @@ static int pi433_probe(struct spi_device *spi)
 	}
 
 	/* setup the radio module */
-	SET_CHECKED(rf69_set_mode		(spi, standby));
-	SET_CHECKED(rf69_set_data_mode		(spi, DATAMODUL_MODE_PACKET));
-	SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
-	SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
-	SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
-	SET_CHECKED(rf69_set_output_power_level	(spi, 13));
-	SET_CHECKED(rf69_set_antenna_impedance	(spi, fiftyOhm));
+	retval = rf69_set_mode(spi, standby);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_set_data_mode(spi, DATAMODUL_MODE_PACKET);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_enable_amplifier(spi, MASK_PALEVEL_PA0);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA1);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA2);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_set_output_power_level(spi, 13);
+	if (retval < 0)
+		goto minor_failed;
+	retval = rf69_set_antenna_impedance(spi, fiftyOhm);
+	if (retval < 0)
+		goto minor_failed;
 
 	/* determ minor number */
 	retval = pi433_get_minor(device);
-- 
2.11.0