2017-12-04 18:19:07

by Marcus Wolf

[permalink] [raw]
Subject: [PATCH] staging: pi433: rf69.c: Replace macros READ_REG and WRITE_REG with inline functions rf69_set_bit, rf69_reset_bit and rf69_read_modify_write

To increase the readability of the register accesses, the abstraction
of the helpers was increased from simple read and write to set bit,
reset bit and read modify write bit. In addition - according to the
proposal from Walter Harms from 20.07.2017 - instead of marcros inline
functions were used.

Signed-off-by: Marcus Wolf <[email protected]>
---
drivers/staging/pi433/rf69.c | 340 ++++++++++++++++++++++--------------------
1 file changed, 182 insertions(+), 158 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e69a215..8a31ed7 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -33,8 +33,32 @@

/*-------------------------------------------------------------------------*/

-#define READ_REG(x) rf69_read_reg (spi, x)
-#define WRITE_REG(x, y) rf69_write_reg(spi, x, y)
+static inline int rf69_set_bit(struct spi_device *spi, u8 reg, u8 mask)
+{
+ u8 tmpVal;
+
+ tmpVal = rf69_read_reg (spi, reg);
+ tmpVal = tmpVal | mask;
+ return rf69_write_reg(spi, reg, tmpVal);
+}
+
+static inline int rf69_reset_bit(struct spi_device *spi, u8 reg, u8 mask)
+{
+ u8 tmpVal;
+
+ tmpVal = rf69_read_reg (spi, reg);
+ tmpVal = tmpVal & ~mask;
+ return rf69_write_reg(spi, reg, tmpVal);
+}
+
+static inline int rf69_read_modify_write(struct spi_device *spi, u8 reg, u8 mask, u8 value)
+{
+ u8 tmpVal;
+
+ tmpVal = rf69_read_reg (spi, reg);
+ tmpVal = (tmpVal & ~mask) | value;
+ return rf69_write_reg(spi, reg, tmpVal);
+}

/*-------------------------------------------------------------------------*/

@@ -45,11 +69,11 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode)
#endif

switch (mode) {
- case transmit: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_TRANSMIT);
- case receive: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_RECEIVE);
- case synthesizer: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_SYNTHESIZER);
- case standby: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_STANDBY);
- case mode_sleep: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_SLEEP);
+ case transmit: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_TRANSMIT);
+ case receive: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_RECEIVE);
+ case synthesizer: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_SYNTHESIZER);
+ case standby: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_STANDBY);
+ case mode_sleep: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_SLEEP);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -68,9 +92,9 @@ int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode)
#endif

switch (dataMode) {
- case packet: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_PACKET);
- case continuous: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_CONTINUOUS);
- case continuousNoSync: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_CONTINUOUS_NOSYNC);
+ case packet: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_PACKET);
+ case continuous: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS);
+ case continuousNoSync: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS_NOSYNC);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -84,8 +108,8 @@ int rf69_set_modulation(struct spi_device *spi, enum modulation modulation)
#endif

switch (modulation) {
- case OOK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | DATAMODUL_MODULATION_TYPE_OOK);
- case FSK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | DATAMODUL_MODULATION_TYPE_FSK);
+ case OOK: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_TYPE, DATAMODUL_MODULATION_TYPE_OOK);
+ case FSK: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_TYPE, DATAMODUL_MODULATION_TYPE_FSK);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -100,7 +124,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi)
dev_dbg(&spi->dev, "get: mode");
#endif

- currentValue = READ_REG(REG_DATAMODUL);
+ currentValue = rf69_read_reg(spi, REG_DATAMODUL);

switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO improvement: change 3 to define
case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
@@ -117,19 +141,19 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum modShaping modShapi

if (rf69_get_modulation(spi) == FSK) {
switch (modShaping) {
- case shapingOff: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_NONE);
- case shaping1_0: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_1_0);
- case shaping0_5: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_0_3);
- case shaping0_3: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_0_5);
+ case shapingOff: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_NONE);
+ case shaping1_0: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_1_0);
+ case shaping0_5: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_0_3);
+ case shaping0_3: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_0_5);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
}
} else {
switch (modShaping) {
- case shapingOff: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_NONE);
- case shapingBR: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_BR);
- case shaping2BR: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) | DATAMODUL_MODULATION_SHAPE_2BR);
+ case shapingOff: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_NONE);
+ case shapingBR: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_BR);
+ case shaping2BR: return rf69_read_modify_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_2BR);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -137,7 +161,7 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum modShaping modShapi
}
}

-int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate)
+int deviceName_rate(struct spi_device *spi, u16 bitRate)
{
int retval;
u32 bitRate_min;
@@ -152,7 +176,7 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate)
// check input value
bitRate_min = F_OSC / 8388608; // 8388608 = 2^23;
if (bitRate < bitRate_min) {
- dev_dbg(&spi->dev, "setBitRate: illegal input param");
+ dev_dbg(&spi->dev, "rf69_set_bitRate: illegal input param");
return -EINVAL;
}

@@ -163,11 +187,10 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate)
lsb = (bitRate_reg&0xff);

// transmit to RF 69
- retval = WRITE_REG(REG_BITRATE_MSB, msb);
+ retval = rf69_write_reg(spi, REG_BITRATE_MSB, msb);
if (retval)
return retval;
-
- retval = WRITE_REG(REG_BITRATE_LSB, lsb);
+ retval = rf69_write_reg(spi, REG_BITRATE_LSB, lsb);
if (retval)
return retval;

@@ -211,11 +234,10 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
}

// write to chip
- retval = WRITE_REG(REG_FDEV_MSB, msb);
+ retval = rf69_write_reg(spi, REG_FDEV_MSB, msb);
if (retval)
return retval;
-
- retval = WRITE_REG(REG_FDEV_LSB, lsb);
+ retval = rf69_write_reg(spi, REG_FDEV_LSB, lsb);
if (retval)
return retval;

@@ -257,15 +279,13 @@ int rf69_set_frequency(struct spi_device *spi, u32 frequency)
lsb = (f_reg&0xff);

// write to chip
- retval = WRITE_REG(REG_FRF_MSB, msb);
+ retval = rf69_write_reg(spi, REG_FRF_MSB, msb);
if (retval)
return retval;
-
- retval = WRITE_REG(REG_FRF_MID, mid);
+ retval = rf69_write_reg(spi, REG_FRF_MID, mid);
if (retval)
return retval;
-
- retval = WRITE_REG(REG_FRF_LSB, lsb);
+ retval = rf69_write_reg(spi, REG_FRF_LSB, lsb);
if (retval)
return retval;

@@ -279,8 +299,8 @@ int rf69_set_amplifier_0(struct spi_device *spi, enum optionOnOff optionOnOff)
#endif

switch (optionOnOff) {
- case optionOn: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | MASK_PALEVEL_PA0));
- case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & ~MASK_PALEVEL_PA0));
+ case optionOn: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA0);
+ case optionOff: return rf69_reset_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA0);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -294,8 +314,8 @@ int rf69_set_amplifier_1(struct spi_device *spi, enum optionOnOff optionOnOff)
#endif

switch (optionOnOff) {
- case optionOn: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | MASK_PALEVEL_PA1));
- case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & ~MASK_PALEVEL_PA1));
+ case optionOn: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA1);
+ case optionOff: return rf69_reset_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA1);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -309,8 +329,8 @@ int rf69_set_amplifier_2(struct spi_device *spi, enum optionOnOff optionOnOff)
#endif

switch (optionOnOff) {
- case optionOn: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | MASK_PALEVEL_PA2));
- case optionOff: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & ~MASK_PALEVEL_PA2));
+ case optionOn: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA2);
+ case optionOff: return rf69_reset_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA2);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -332,7 +352,7 @@ int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)
}

// write value
- return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & ~MASK_PALEVEL_OUTPUT_POWER) | powerLevel);
+ return rf69_read_modify_write(spi, REG_PALEVEL, MASK_PALEVEL_OUTPUT_POWER, powerLevel);
}

int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp)
@@ -342,22 +362,22 @@ int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp)
#endif

switch (paRamp) {
- case ramp3400: return WRITE_REG(REG_PARAMP, PARAMP_3400);
- case ramp2000: return WRITE_REG(REG_PARAMP, PARAMP_2000);
- case ramp1000: return WRITE_REG(REG_PARAMP, PARAMP_1000);
- case ramp500: return WRITE_REG(REG_PARAMP, PARAMP_500);
- case ramp250: return WRITE_REG(REG_PARAMP, PARAMP_250);
- case ramp125: return WRITE_REG(REG_PARAMP, PARAMP_125);
- case ramp100: return WRITE_REG(REG_PARAMP, PARAMP_100);
- case ramp62: return WRITE_REG(REG_PARAMP, PARAMP_62);
- case ramp50: return WRITE_REG(REG_PARAMP, PARAMP_50);
- case ramp40: return WRITE_REG(REG_PARAMP, PARAMP_40);
- case ramp31: return WRITE_REG(REG_PARAMP, PARAMP_31);
- case ramp25: return WRITE_REG(REG_PARAMP, PARAMP_25);
- case ramp20: return WRITE_REG(REG_PARAMP, PARAMP_20);
- case ramp15: return WRITE_REG(REG_PARAMP, PARAMP_15);
- case ramp12: return WRITE_REG(REG_PARAMP, PARAMP_12);
- case ramp10: return WRITE_REG(REG_PARAMP, PARAMP_10);
+ case ramp3400: return rf69_write_reg(spi, REG_PARAMP, PARAMP_3400);
+ case ramp2000: return rf69_write_reg(spi, REG_PARAMP, PARAMP_2000);
+ case ramp1000: return rf69_write_reg(spi, REG_PARAMP, PARAMP_1000);
+ case ramp500: return rf69_write_reg(spi, REG_PARAMP, PARAMP_500);
+ case ramp250: return rf69_write_reg(spi, REG_PARAMP, PARAMP_250);
+ case ramp125: return rf69_write_reg(spi, REG_PARAMP, PARAMP_125);
+ case ramp100: return rf69_write_reg(spi, REG_PARAMP, PARAMP_100);
+ case ramp62: return rf69_write_reg(spi, REG_PARAMP, PARAMP_62);
+ case ramp50: return rf69_write_reg(spi, REG_PARAMP, PARAMP_50);
+ case ramp40: return rf69_write_reg(spi, REG_PARAMP, PARAMP_40);
+ case ramp31: return rf69_write_reg(spi, REG_PARAMP, PARAMP_31);
+ case ramp25: return rf69_write_reg(spi, REG_PARAMP, PARAMP_25);
+ case ramp20: return rf69_write_reg(spi, REG_PARAMP, PARAMP_20);
+ case ramp15: return rf69_write_reg(spi, REG_PARAMP, PARAMP_15);
+ case ramp12: return rf69_write_reg(spi, REG_PARAMP, PARAMP_12);
+ case ramp10: return rf69_write_reg(spi, REG_PARAMP, PARAMP_10);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -371,8 +391,8 @@ int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance ant
#endif

switch (antennaImpedance) {
- case fiftyOhm: return WRITE_REG(REG_LNA, (READ_REG(REG_LNA) & ~MASK_LNA_ZIN));
- case twohundretOhm: return WRITE_REG(REG_LNA, (READ_REG(REG_LNA) | MASK_LNA_ZIN));
+ case fiftyOhm: return rf69_reset_bit(spi, REG_LNA, MASK_LNA_ZIN);
+ case twohundretOhm: return rf69_set_bit(spi, REG_LNA, MASK_LNA_ZIN);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -386,13 +406,13 @@ int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain)
#endif

switch (lnaGain) {
- case automatic: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_AUTO));
- case max: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX));
- case maxMinus6: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_6));
- case maxMinus12: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_12));
- case maxMinus24: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_24));
- case maxMinus36: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_36));
- case maxMinus48: return WRITE_REG(REG_LNA, ((READ_REG(REG_LNA) & ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_48));
+ case automatic: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_AUTO);
+ case max: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX);
+ case maxMinus6: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX_MINUS_6);
+ case maxMinus12: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX_MINUS_12);
+ case maxMinus24: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX_MINUS_24);
+ case maxMinus36: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX_MINUS_36);
+ case maxMinus48: return rf69_read_modify_write(spi, REG_LNA, MASK_LNA_GAIN, LNA_GAIN_MAX_MINUS_48);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -407,7 +427,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
dev_dbg(&spi->dev, "get: lna gain");
#endif

- currentValue = READ_REG(REG_LNA);
+ currentValue = rf69_read_reg(spi, REG_LNA);

switch (currentValue & MASK_LNA_CURRENT_GAIN >> 3) { // improvement: change 3 to define
case LNA_GAIN_AUTO: return automatic;
@@ -424,14 +444,14 @@ 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)
{
switch (dccPercent) {
- case dcc16Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_16_PERCENT));
- case dcc8Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_8_PERCENT));
- case dcc4Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_4_PERCENT));
- case dcc2Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_2_PERCENT));
- case dcc1Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_1_PERCENT));
- case dcc0_5Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_0_5_PERCENT));
- case dcc0_25Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_0_25_PERCENT));
- case dcc0_125Percent: return WRITE_REG(reg, ((READ_REG(reg) & ~MASK_BW_DCC_FREQ) | BW_DCC_0_125_PERCENT));
+ case dcc16Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_16_PERCENT);
+ case dcc8Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_8_PERCENT);
+ case dcc4Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_4_PERCENT);
+ case dcc2Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_2_PERCENT);
+ case dcc1Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_1_PERCENT);
+ case dcc0_5Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_0_5_PERCENT);
+ case dcc0_25Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_0_25_PERCENT);
+ case dcc0_125Percent: return rf69_read_modify_write(spi, reg, MASK_BW_DCC_FREQ, BW_DCC_0_125_PERCENT);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -475,7 +495,7 @@ static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg,
}

// read old value
- newValue = READ_REG(reg);
+ newValue = rf69_read_reg(spi, reg);

// "delete" mantisse and exponent = just keep the DCC setting
newValue = newValue & MASK_BW_DCC_FREQ;
@@ -497,7 +517,7 @@ static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg,
newValue = newValue | exponent;

// write back
- return WRITE_REG(reg, newValue);
+ return rf69_write_reg(spi, reg, newValue);
}

int rf69_set_bandwidth(struct spi_device *spi, enum mantisse mantisse, u8 exponent)
@@ -525,9 +545,9 @@ int rf69_set_ook_threshold_type(struct spi_device *spi, enum thresholdType thres
#endif

switch (thresholdType) {
- case fixed: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESTYPE) | OOKPEAK_THRESHTYPE_FIXED));
- case peak: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESTYPE) | OOKPEAK_THRESHTYPE_PEAK));
- case average: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESTYPE) | OOKPEAK_THRESHTYPE_AVERAGE));
+ case fixed: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESTYPE, OOKPEAK_THRESHTYPE_FIXED);
+ case peak: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESTYPE, OOKPEAK_THRESHTYPE_PEAK);
+ case average: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESTYPE, OOKPEAK_THRESHTYPE_AVERAGE);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -541,14 +561,14 @@ int rf69_set_ook_threshold_step(struct spi_device *spi, enum thresholdStep thres
#endif

switch (thresholdStep) {
- case step_0_5db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_0_5_DB));
- case step_1_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_1_0_DB));
- case step_1_5db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_1_5_DB));
- case step_2_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_2_0_DB));
- case step_3_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_3_0_DB));
- case step_4_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_4_0_DB));
- case step_5_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_5_0_DB));
- case step_6_0db: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESSTEP) | OOKPEAK_THRESHSTEP_6_0_DB));
+ case step_0_5db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_0_5_DB);
+ case step_1_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_1_0_DB);
+ case step_1_5db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_1_5_DB);
+ case step_2_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_2_0_DB);
+ case step_3_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_3_0_DB);
+ case step_4_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_4_0_DB);
+ case step_5_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_5_0_DB);
+ case step_6_0db: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESSTEP, OOKPEAK_THRESHSTEP_6_0_DB);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -562,14 +582,14 @@ int rf69_set_ook_threshold_dec(struct spi_device *spi, enum thresholdDecrement t
#endif

switch (thresholdDecrement) {
- case dec_every8th: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_EVERY_8TH));
- case dec_every4th: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_EVERY_4TH));
- case dec_every2nd: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_EVERY_2ND));
- case dec_once: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_ONCE));
- case dec_twice: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_TWICE));
- case dec_4times: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_4_TIMES));
- case dec_8times: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_8_TIMES));
- case dec_16times: return WRITE_REG(REG_OOKPEAK, ((READ_REG(REG_OOKPEAK) & ~MASK_OOKPEAK_THRESDEC) | OOKPEAK_THRESHDEC_16_TIMES));
+ case dec_every8th: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_EVERY_8TH);
+ case dec_every4th: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_EVERY_4TH);
+ case dec_every2nd: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_EVERY_2ND);
+ case dec_once: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_ONCE);
+ case dec_twice: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_TWICE);
+ case dec_4times: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_4_TIMES);
+ case dec_8times: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_8_TIMES);
+ case dec_16times: return rf69_read_modify_write(spi, REG_OOKPEAK, MASK_OOKPEAK_THRESDEC, OOKPEAK_THRESHDEC_16_TIMES);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -607,18 +627,18 @@ int rf69_set_dio_mapping(struct spi_device *spi, u8 DIONumber, u8 value)
mask = MASK_DIO5; shift = SHIFT_DIO5; regaddr = REG_DIOMAPPING2;
break;
default:
- dev_dbg(&spi->dev, "set: illegal input param");
+ dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
}

// read reg
- regValue = READ_REG(regaddr);
+ regValue = rf69_read_reg(spi, regaddr);
// delete old value
regValue = regValue & ~mask;
// add new value
regValue = regValue | value << shift;
// write back
- return WRITE_REG(regaddr, regValue);
+ return rf69_write_reg(spi, regaddr, regValue);
}

bool rf69_get_flag(struct spi_device *spi, enum flag flag)
@@ -628,23 +648,23 @@ bool rf69_get_flag(struct spi_device *spi, enum flag flag)
#endif

switch (flag) {
- case modeSwitchCompleted: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_MODE_READY);
- case readyToReceive: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_RX_READY);
- case readyToSend: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_TX_READY);
- case pllLocked: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_PLL_LOCK);
- case rssiExceededThreshold: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_RSSI);
- case timeout: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_TIMEOUT);
- case automode: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_AUTOMODE);
- case syncAddressMatch: return (READ_REG(REG_IRQFLAGS1) & MASK_IRQFLAGS1_SYNC_ADDRESS_MATCH);
- case fifoFull: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_FULL);
-/* case fifoNotEmpty: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY); */
- case fifoEmpty: return !(READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY);
- case fifoLevelBelowThreshold: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_LEVEL);
- case fifoOverrun: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_OVERRUN);
- case packetSent: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_PACKET_SENT);
- case payloadReady: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_PAYLOAD_READY);
- case crcOk: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_CRC_OK);
- case batteryLow: return (READ_REG(REG_IRQFLAGS2) & MASK_IRQFLAGS2_LOW_BAT);
+ case modeSwitchCompleted: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_MODE_READY);
+ case readyToReceive: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_RX_READY);
+ case readyToSend: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_TX_READY);
+ case pllLocked: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_PLL_LOCK);
+ case rssiExceededThreshold: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_RSSI);
+ case timeout: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_TIMEOUT);
+ case automode: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_AUTOMODE);
+ case syncAddressMatch: return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_SYNC_ADDRESS_MATCH);
+ case fifoFull: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_FULL);
+/* case fifoNotEmpty: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY); */
+ case fifoEmpty: return !(rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY);
+ case fifoLevelBelowThreshold: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_LEVEL);
+ case fifoOverrun: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_OVERRUN);
+ case packetSent: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_PACKET_SENT);
+ case payloadReady: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_PAYLOAD_READY);
+ case crcOk: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_CRC_OK);
+ case batteryLow: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_LOW_BAT);
default: return false;
}
}
@@ -656,9 +676,9 @@ int rf69_reset_flag(struct spi_device *spi, enum flag flag)
#endif

switch (flag) {
- case rssiExceededThreshold: return WRITE_REG(REG_IRQFLAGS1, MASK_IRQFLAGS1_RSSI);
- case syncAddressMatch: return WRITE_REG(REG_IRQFLAGS1, MASK_IRQFLAGS1_SYNC_ADDRESS_MATCH);
- case fifoOverrun: return WRITE_REG(REG_IRQFLAGS2, MASK_IRQFLAGS2_FIFO_OVERRUN);
+ case rssiExceededThreshold: return rf69_write_reg(spi, REG_IRQFLAGS1, MASK_IRQFLAGS1_RSSI);
+ case syncAddressMatch: return rf69_write_reg(spi, REG_IRQFLAGS1, MASK_IRQFLAGS1_SYNC_ADDRESS_MATCH);
+ case fifoOverrun: return rf69_write_reg(spi, REG_IRQFLAGS2, MASK_IRQFLAGS2_FIFO_OVERRUN);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -673,7 +693,7 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8 threshold)

/* no value check needed - u8 exactly matches register size */

- return WRITE_REG(REG_RSSITHRESH, threshold);
+ return rf69_write_reg(spi, REG_RSSITHRESH, threshold);
}

int rf69_set_rx_start_timeout(struct spi_device *spi, u8 timeout)
@@ -684,7 +704,7 @@ int rf69_set_rx_start_timeout(struct spi_device *spi, u8 timeout)

/* no value check needed - u8 exactly matches register size */

- return WRITE_REG(REG_RXTIMEOUT1, timeout);
+ return rf69_write_reg(spi, REG_RXTIMEOUT1, timeout);
}

int rf69_set_rssi_timeout(struct spi_device *spi, u8 timeout)
@@ -695,7 +715,7 @@ int rf69_set_rssi_timeout(struct spi_device *spi, u8 timeout)

/* no value check needed - u8 exactly matches register size */

- return WRITE_REG(REG_RXTIMEOUT2, timeout);
+ return rf69_write_reg(spi, REG_RXTIMEOUT2, timeout);
}

int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength)
@@ -714,10 +734,12 @@ int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength)
lsb = (preambleLength&0xff);

/* transmit to chip */
- retval = WRITE_REG(REG_PREAMBLE_MSB, msb);
+ retval = rf69_write_reg(spi, REG_PREAMBLE_MSB, msb);
if (retval)
return retval;
- return WRITE_REG(REG_PREAMBLE_LSB, lsb);
+ retval = rf69_write_reg(spi, REG_PREAMBLE_LSB, lsb);
+
+ return retval;
}

int rf69_set_sync_enable(struct spi_device *spi, enum optionOnOff optionOnOff)
@@ -727,8 +749,8 @@ int rf69_set_sync_enable(struct spi_device *spi, enum optionOnOff optionOnOff)
#endif

switch (optionOnOff) {
- case optionOn: return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) | MASK_SYNC_CONFIG_SYNC_ON));
- case optionOff: return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_ON));
+ case optionOn: return rf69_set_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
+ case optionOff: return rf69_reset_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -742,8 +764,8 @@ int rf69_set_fifo_fill_condition(struct spi_device *spi, enum fifoFillCondition
#endif

switch (fifoFillCondition) {
- case always: return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) | MASK_SYNC_CONFIG_FIFO_FILL_CONDITION));
- case afterSyncInterrupt: return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_FIFO_FILL_CONDITION));
+ case always: return rf69_set_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_FIFO_FILL_CONDITION);
+ case afterSyncInterrupt: return rf69_reset_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_FIFO_FILL_CONDITION);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -763,7 +785,7 @@ int rf69_set_sync_size(struct spi_device *spi, u8 syncSize)
}

// write value
- return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_SIZE) | (syncSize << 3));
+ return rf69_read_modify_write(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_SIZE, (syncSize << 3));
}

int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance)
@@ -779,7 +801,7 @@ int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance)
}

// write value
- return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_SIZE) | syncTolerance);
+ return rf69_read_modify_write(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_SIZE, syncTolerance);
}

int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8])
@@ -790,14 +812,14 @@ int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8])
dev_dbg(&spi->dev, "set: sync values");
#endif

- retval += WRITE_REG(REG_SYNCVALUE1, syncValues[0]);
- retval += WRITE_REG(REG_SYNCVALUE2, syncValues[1]);
- retval += WRITE_REG(REG_SYNCVALUE3, syncValues[2]);
- retval += WRITE_REG(REG_SYNCVALUE4, syncValues[3]);
- retval += WRITE_REG(REG_SYNCVALUE5, syncValues[4]);
- retval += WRITE_REG(REG_SYNCVALUE6, syncValues[5]);
- retval += WRITE_REG(REG_SYNCVALUE7, syncValues[6]);
- retval += WRITE_REG(REG_SYNCVALUE8, syncValues[7]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE1, syncValues[0]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE2, syncValues[1]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE3, syncValues[2]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE4, syncValues[3]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE5, syncValues[4]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE6, syncValues[5]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE7, syncValues[6]);
+ retval += rf69_write_reg(spi, REG_SYNCVALUE8, syncValues[7]);

return retval;
}
@@ -809,8 +831,8 @@ int rf69_set_packet_format(struct spi_device *spi, enum packetFormat packetForma
#endif

switch (packetFormat) {
- case packetLengthVar: return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) | MASK_PACKETCONFIG1_PAKET_FORMAT_VARIABLE));
- case packetLengthFix: return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_PAKET_FORMAT_VARIABLE));
+ case packetLengthVar: return rf69_set_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_PAKET_FORMAT_VARIABLE);
+ case packetLengthFix: return rf69_reset_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_PAKET_FORMAT_VARIABLE);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -824,8 +846,8 @@ int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff)
#endif

switch (optionOnOff) {
- case optionOn: return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) | MASK_PACKETCONFIG1_CRC_ON));
- case optionOff: return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_CRC_ON));
+ case optionOn: return rf69_set_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
+ case optionOff: return rf69_reset_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -839,9 +861,9 @@ int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering addre
#endif

switch (addressFiltering) {
- case filteringOff: return WRITE_REG(REG_PACKETCONFIG1, ((READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_ADDRESSFILTERING) | PACKETCONFIG1_ADDRESSFILTERING_OFF));
- case nodeAddress: return WRITE_REG(REG_PACKETCONFIG1, ((READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_ADDRESSFILTERING) | PACKETCONFIG1_ADDRESSFILTERING_NODE));
- case nodeOrBroadcastAddress: return WRITE_REG(REG_PACKETCONFIG1, ((READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_ADDRESSFILTERING) | PACKETCONFIG1_ADDRESSFILTERING_NODEBROADCAST));
+ case filteringOff: return rf69_read_modify_write(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_ADDRESSFILTERING, PACKETCONFIG1_ADDRESSFILTERING_OFF);
+ case nodeAddress: return rf69_read_modify_write(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_ADDRESSFILTERING, PACKETCONFIG1_ADDRESSFILTERING_NODE);
+ case nodeOrBroadcastAddress: return rf69_read_modify_write(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_ADDRESSFILTERING, PACKETCONFIG1_ADDRESSFILTERING_NODEBROADCAST);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -854,7 +876,7 @@ int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength)
dev_dbg(&spi->dev, "set: payload length");
#endif

- return WRITE_REG(REG_PAYLOAD_LENGTH, payloadLength);
+ return rf69_write_reg(spi, REG_PAYLOAD_LENGTH, payloadLength);
}

u8 rf69_get_payload_length(struct spi_device *spi)
@@ -863,7 +885,7 @@ u8 rf69_get_payload_length(struct spi_device *spi)
dev_dbg(&spi->dev, "get: payload length");
#endif

- return (u8) READ_REG(REG_PAYLOAD_LENGTH);
+ return (u8)rf69_read_reg(spi, REG_PAYLOAD_LENGTH);
}

int rf69_set_node_address(struct spi_device *spi, u8 nodeAddress)
@@ -872,7 +894,7 @@ int rf69_set_node_address(struct spi_device *spi, u8 nodeAddress)
dev_dbg(&spi->dev, "set: node address");
#endif

- return WRITE_REG(REG_NODEADRS, nodeAddress);
+ return rf69_write_reg(spi, REG_NODEADRS, nodeAddress);
}

int rf69_set_broadcast_address(struct spi_device *spi, u8 broadcastAddress)
@@ -881,7 +903,7 @@ int rf69_set_broadcast_address(struct spi_device *spi, u8 broadcastAddress)
dev_dbg(&spi->dev, "set: broadcast address");
#endif

- return WRITE_REG(REG_BROADCASTADRS, broadcastAddress);
+ return rf69_write_reg(spi, REG_BROADCASTADRS, broadcastAddress);
}

int rf69_set_tx_start_condition(struct spi_device *spi, enum txStartCondition txStartCondition)
@@ -891,8 +913,8 @@ int rf69_set_tx_start_condition(struct spi_device *spi, enum txStartCondition tx
#endif

switch (txStartCondition) {
- case fifoLevel: return WRITE_REG(REG_FIFO_THRESH, (READ_REG(REG_FIFO_THRESH) & ~MASK_FIFO_THRESH_TXSTART));
- case fifoNotEmpty: return WRITE_REG(REG_FIFO_THRESH, (READ_REG(REG_FIFO_THRESH) | MASK_FIFO_THRESH_TXSTART));
+ case fifoLevel: return rf69_reset_bit(spi, REG_FIFO_THRESH, MASK_FIFO_THRESH_TXSTART);
+ case fifoNotEmpty: return rf69_set_bit(spi, REG_FIFO_THRESH, MASK_FIFO_THRESH_TXSTART);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
@@ -907,19 +929,21 @@ int rf69_set_fifo_threshold(struct spi_device *spi, u8 threshold)
dev_dbg(&spi->dev, "set: fifo threshold");
#endif

- // check input value
+ /* check input value */
if (threshold & 0x80) {
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
}

- // write value
- retval = WRITE_REG(REG_FIFO_THRESH, (READ_REG(REG_FIFO_THRESH) & ~MASK_FIFO_THRESH_VALUE) | threshold);
+ /* write value */
+ retval = rf69_read_modify_write(spi, REG_FIFO_THRESH, MASK_FIFO_THRESH_VALUE, threshold);
if (retval)
return retval;

- // access the fifo to activate new threshold
- return rf69_read_fifo(spi, (u8 *)&retval, 1); // retval used as buffer
+ /* access the fifo to activate new threshold
+ * retval (mis-) used as buffer here
+ */
+ return rf69_read_fifo(spi, (u8 *)&retval, 1);
}

int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
@@ -929,9 +953,9 @@ int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
#endif

switch (dagc) {
- case normalMode: return WRITE_REG(REG_TESTDAGC, DAGC_NORMAL);
- case improve: return WRITE_REG(REG_TESTDAGC, DAGC_IMPROVED_LOWBETA0);
- case improve4LowModulationIndex: return WRITE_REG(REG_TESTDAGC, DAGC_IMPROVED_LOWBETA1);
+ case normalMode: return rf69_write_reg(spi, REG_TESTDAGC, DAGC_NORMAL);
+ case improve: return rf69_write_reg(spi, REG_TESTDAGC, DAGC_IMPROVED_LOWBETA0);
+ case improve4LowModulationIndex: return rf69_write_reg(spi, REG_TESTDAGC, DAGC_IMPROVED_LOWBETA1);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
--
1.7.10.4


2017-12-04 19:29:59

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: pi433: rf69.c: Replace macros READ_REG and WRITE_REG with inline functions rf69_set_bit, rf69_reset_bit and rf69_read_modify_write

The subject is way too long.

On Mon, Dec 04, 2017 at 08:18:51PM +0200, Marcus Wolf wrote:
> To increase the readability of the register accesses, the abstraction
> of the helpers was increased from simple read and write to set bit,
> reset bit and read modify write bit. In addition - according to the
> proposal from Walter Harms from 20.07.2017 - instead of marcros inline
> functions were used.
>
> Signed-off-by: Marcus Wolf <[email protected]>
> ---
> drivers/staging/pi433/rf69.c | 340 ++++++++++++++++++++++--------------------
> 1 file changed, 182 insertions(+), 158 deletions(-)
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e69a215..8a31ed7 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -33,8 +33,32 @@
>
> /*-------------------------------------------------------------------------*/
>
> -#define READ_REG(x) rf69_read_reg (spi, x)
> -#define WRITE_REG(x, y) rf69_write_reg(spi, x, y)
> +static inline int rf69_set_bit(struct spi_device *spi, u8 reg, u8 mask)
^^^^^^
Remove the inline. Leave that for the compiler to decide.


> +{
> + u8 tmpVal;

Use checkpatch.pl. No camelCase.

> +
> + tmpVal = rf69_read_reg (spi, reg);
^
Remove this space.

> + tmpVal = tmpVal | mask;
> + return rf69_write_reg(spi, reg, tmpVal);
> +}
> +
> +static inline int rf69_reset_bit(struct spi_device *spi, u8 reg, u8 mask)
^^^^^
Change the name to rf69_clear_bit(). That matches the rest of kernel
naming. "reset" is ambigous to me.

> +{
> + u8 tmpVal;
> +
> + tmpVal = rf69_read_reg (spi, reg);
> + tmpVal = tmpVal & ~mask;
> + return rf69_write_reg(spi, reg, tmpVal);
> +}
> +
> +static inline int rf69_read_modify_write(struct spi_device *spi, u8 reg, u8 mask, u8 value)
> +{
> + u8 tmpVal;
> +
> + tmpVal = rf69_read_reg (spi, reg);
> + tmpVal = (tmpVal & ~mask) | value;
> + return rf69_write_reg(spi, reg, tmpVal);
> +}
>
> /*-------------------------------------------------------------------------*/
>
> @@ -45,11 +69,11 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode)
> #endif
>
> switch (mode) {
> - case transmit: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_TRANSMIT);
> - case receive: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_RECEIVE);
> - case synthesizer: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_SYNTHESIZER);
> - case standby: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_STANDBY);
> - case mode_sleep: return WRITE_REG(REG_OPMODE, (READ_REG(REG_OPMODE) & ~MASK_OPMODE_MODE) | OPMODE_MODE_SLEEP);
> + case transmit: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_TRANSMIT);
> + case receive: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_RECEIVE);
> + case synthesizer: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_SYNTHESIZER);
> + case standby: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_STANDBY);
> + case mode_sleep: return rf69_read_modify_write(spi, REG_OPMODE, MASK_OPMODE_MODE, OPMODE_MODE_SLEEP);

All these lines are over 80 characters long. The new
rf69_read_modify_write() function name is too many characters. We could
probably change the names to rf69_read(), rf69_write() and
rf69_read_mod_write().

> @@ -137,7 +161,7 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum modShaping modShapi
> }
> }
>
> -int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate)
> +int deviceName_rate(struct spi_device *spi, u16 bitRate)

CamelCase

> {
> int retval;
> u32 bitRate_min;
> @@ -152,7 +176,7 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate)
> // check input value
> bitRate_min = F_OSC / 8388608; // 8388608 = 2^23;
> if (bitRate < bitRate_min) {
> - dev_dbg(&spi->dev, "setBitRate: illegal input param");
> + dev_dbg(&spi->dev, "rf69_set_bitRate: illegal input param");

Just use __func__