2017-12-06 20:42:29

by Simon Sandström

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

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


2017-12-06 20:42:37

by Simon Sandström

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

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

Signed-off-by: Simon Sandström <[email protected]>
---
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

2017-12-06 20:42:47

by Simon Sandström

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

Call rf69_set_data_mode with DATAMODUL_MODE value directly.

Signed-off-by: Simon Sandström <[email protected]>
---
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

2017-12-06 20:42:45

by Simon Sandström

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

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

2017-12-06 20:42:42

by Simon Sandström

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

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

Signed-off-by: Simon Sandström <[email protected]>
---
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

2017-12-06 20:43:27

by Simon Sandström

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

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

2017-12-06 20:43:41

by Simon Sandström

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

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

Signed-off-by: Simon Sandström <[email protected]>
---
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

2017-12-06 22:24:43

by Marcus Wolf

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



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


2017-12-07 06:14:05

by Greg Kroah-Hartman

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

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

2017-12-07 14:38:52

by Greg Kroah-Hartman

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

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

2017-12-07 14:58:06

by Simon Sandström

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

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

2017-12-07 16:52:55

by Greg Kroah-Hartman

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

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