* [PATCH v4] iio: frequency: ad9832: Use FIELD_PREP macro to set bit fields
@ 2025-03-30 13:05 Siddharth Menon
0 siblings, 0 replies; only message in thread
From: Siddharth Menon @ 2025-03-30 13:05 UTC (permalink / raw)
To: linux-iio, lars, Michael.Hennerich, jic23, gregkh
Cc: linux-kernel, linux-staging, marcelo.schmitt1, Siddharth Menon
Use bitfield and bitmask macros to clearly specify AD9832 SPI
command fields to make register write code more readable.
Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Signed-off-by: Siddharth Menon <simeddon@gmail.com>
---
v1->v2:
- remove CMD_SHIFT and ADD_SHIFT
- use GENMASK
- store regval in an array and iterate through it
v2->v3:
- add missing header
- refactor code in the previously introduced loops
v3->v4:
- update commit message with a better one
- convert AD9832_PHASE and RES_MASK to masks
- cleanup a few if else blocks
drivers/staging/iio/frequency/ad9832.c | 85 +++++++++++++-------------
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 140ee4f9c137..1017d2e305cf 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -16,6 +16,9 @@
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/sysfs.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/unaligned.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -59,17 +62,18 @@
#define AD9832_CMD_SLEEPRESCLR 0xC
#define AD9832_FREQ BIT(11)
-#define AD9832_PHASE(x) (((x) & 3) << 9)
+#define AD9832_PHASE_MASK GENMASK(10, 9)
#define AD9832_SYNC BIT(13)
#define AD9832_SELSRC BIT(12)
#define AD9832_SLEEP BIT(13)
#define AD9832_RESET BIT(12)
#define AD9832_CLR BIT(11)
-#define CMD_SHIFT 12
-#define ADD_SHIFT 8
#define AD9832_FREQ_BITS 32
#define AD9832_PHASE_BITS 12
-#define RES_MASK(bits) ((1 << (bits)) - 1)
+#define RES_MASK(bits) GENMASK((bits) - 1, 0)
+#define AD9832_CMD_MSK GENMASK(15, 12)
+#define AD9832_ADD_MSK GENMASK(11, 8)
+#define AD9832_DAT_MSK GENMASK(7, 0)
/**
* struct ad9832_state - driver instance specific data
@@ -131,6 +135,8 @@ static int ad9832_write_frequency(struct ad9832_state *st,
{
unsigned long clk_freq;
unsigned long regval;
+ u16 freq_cmd;
+ u8 regval_bytes[4];
clk_freq = clk_get_rate(st->mclk);
@@ -138,19 +144,15 @@ static int ad9832_write_frequency(struct ad9832_state *st,
return -EINVAL;
regval = ad9832_calc_freqreg(clk_freq, fout);
+ put_unaligned_be32(regval, regval_bytes);
- st->freq_data[0] = cpu_to_be16((AD9832_CMD_FRE8BITSW << CMD_SHIFT) |
- (addr << ADD_SHIFT) |
- ((regval >> 24) & 0xFF));
- st->freq_data[1] = cpu_to_be16((AD9832_CMD_FRE16BITSW << CMD_SHIFT) |
- ((addr - 1) << ADD_SHIFT) |
- ((regval >> 16) & 0xFF));
- st->freq_data[2] = cpu_to_be16((AD9832_CMD_FRE8BITSW << CMD_SHIFT) |
- ((addr - 2) << ADD_SHIFT) |
- ((regval >> 8) & 0xFF));
- st->freq_data[3] = cpu_to_be16((AD9832_CMD_FRE16BITSW << CMD_SHIFT) |
- ((addr - 3) << ADD_SHIFT) |
- ((regval >> 0) & 0xFF));
+ for (int i = 0; i < 4; i++) {
+ freq_cmd = (i % 2 == 0) ? AD9832_CMD_FRE8BITSW : AD9832_CMD_FRE16BITSW;
+
+ st->freq_data[i] = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, freq_cmd) |
+ FIELD_PREP(AD9832_ADD_MSK, addr - i) |
+ FIELD_PREP(AD9832_DAT_MSK, regval_bytes[i]));
+ }
return spi_sync(st->spi, &st->freq_msg);
}
@@ -158,15 +160,21 @@ static int ad9832_write_frequency(struct ad9832_state *st,
static int ad9832_write_phase(struct ad9832_state *st,
unsigned long addr, unsigned long phase)
{
+ u8 phase_bytes[2];
+ u16 phase_cmd;
+
if (phase >= BIT(AD9832_PHASE_BITS))
return -EINVAL;
- st->phase_data[0] = cpu_to_be16((AD9832_CMD_PHA8BITSW << CMD_SHIFT) |
- (addr << ADD_SHIFT) |
- ((phase >> 8) & 0xFF));
- st->phase_data[1] = cpu_to_be16((AD9832_CMD_PHA16BITSW << CMD_SHIFT) |
- ((addr - 1) << ADD_SHIFT) |
- (phase & 0xFF));
+ put_unaligned_be16(phase, phase_bytes);
+
+ for (int i = 0; i < 2; i++) {
+ phase_cmd = (i % 2 == 0) ? AD9832_CMD_PHA8BITSW : AD9832_CMD_PHA16BITSW;
+
+ st->phase_data[i] = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, phase_cmd) |
+ FIELD_PREP(AD9832_ADD_MSK, addr - i) |
+ FIELD_PREP(AD9832_DAT_MSK, phase_bytes[i]));
+ }
return spi_sync(st->spi, &st->phase_msg);
}
@@ -197,24 +205,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
ret = ad9832_write_phase(st, this_attr->address, val);
break;
case AD9832_PINCTRL_EN:
- if (val)
- st->ctrl_ss &= ~AD9832_SELSRC;
- else
- st->ctrl_ss |= AD9832_SELSRC;
- st->data = cpu_to_be16((AD9832_CMD_SYNCSELSRC << CMD_SHIFT) |
+ st->ctrl_ss &= ~AD9832_SELSRC;
+ st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);
+
+ st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
st->ctrl_ss);
ret = spi_sync(st->spi, &st->msg);
break;
case AD9832_FREQ_SYM:
- if (val == 1) {
- st->ctrl_fp |= AD9832_FREQ;
- } else if (val == 0) {
+ if (val == 1 || val == 0) {
st->ctrl_fp &= ~AD9832_FREQ;
+ st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 0 : 1);
} else {
ret = -EINVAL;
break;
}
- st->data = cpu_to_be16((AD9832_CMD_FPSELECT << CMD_SHIFT) |
+ st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
st->ctrl_fp);
ret = spi_sync(st->spi, &st->msg);
break;
@@ -224,21 +230,18 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
break;
}
- st->ctrl_fp &= ~AD9832_PHASE(3);
- st->ctrl_fp |= AD9832_PHASE(val);
+ st->ctrl_fp &= ~FIELD_PREP(AD9832_PHASE_MASK, 3);
+ st->ctrl_fp |= FIELD_PREP(AD9832_PHASE_MASK, val ? 0 : 1);
- st->data = cpu_to_be16((AD9832_CMD_FPSELECT << CMD_SHIFT) |
+ st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
st->ctrl_fp);
ret = spi_sync(st->spi, &st->msg);
break;
case AD9832_OUTPUT_EN:
- if (val)
- st->ctrl_src &= ~(AD9832_RESET | AD9832_SLEEP |
- AD9832_CLR);
- else
- st->ctrl_src |= AD9832_RESET;
+ st->ctrl_src &= ~(AD9832_RESET | AD9832_SLEEP | AD9832_CLR);
+ st->ctrl_src |= FIELD_PREP(AD9832_RESET, val ? 0 : 1);
- st->data = cpu_to_be16((AD9832_CMD_SLEEPRESCLR << CMD_SHIFT) |
+ st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SLEEPRESCLR) |
st->ctrl_src);
ret = spi_sync(st->spi, &st->msg);
break;
@@ -396,7 +399,7 @@ static int ad9832_probe(struct spi_device *spi)
spi_message_add_tail(&st->phase_xfer[1], &st->phase_msg);
st->ctrl_src = AD9832_SLEEP | AD9832_RESET | AD9832_CLR;
- st->data = cpu_to_be16((AD9832_CMD_SLEEPRESCLR << CMD_SHIFT) |
+ st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SLEEPRESCLR) |
st->ctrl_src);
ret = spi_sync(st->spi, &st->msg);
if (ret) {
--
2.49.0
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2025-03-30 13:15 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-30 13:05 [PATCH v4] iio: frequency: ad9832: Use FIELD_PREP macro to set bit fields Siddharth Menon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.