Re: [PATCH v7] iio: frequency: ad9832: Use FIELD_PREP macro to set bit fields

From: Marcelo Schmitt
Date: Tue Apr 15 2025 - 12:08:50 EST


On 04/15, Siddharth Menon wrote:
> Use bitfield and bitmask macros to clearly specify AD9832 SPI
> command fields to make register write code more readable.
>
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>
> Suggested-by: Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>
> Signed-off-by: Siddharth Menon <simeddon@xxxxxxxxx>
> ---
...
> v6->v7
> - fix st->ctrl_x alignment
> drivers/staging/iio/frequency/ad9832.c | 92 ++++++++++++++------------
> 1 file changed, 48 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
...
> - st->data = cpu_to_be16((AD9832_CMD_SYNCSELSRC << CMD_SHIFT) |
> - st->ctrl_ss);
> + 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);
Oof getting Linux code style compliant tabs and spaces correctly can be tricky.
In sum, tabs should be size 8 and be actual tabs (not 8 blank characters).
Also, when we need to align code at some column that doesn't match 8 sized tabs
exactly, we start with tabs and go until the last tab that precedes the column
we want to align to, then we use spaces to go from that last tab until the
column we want to align to.

For this particular example, the opening parenthesis of FIELD_PREP is at column
50, so to align st->ctrl_ss with the arguments of FIELD_PREP on the preceding
line, we use 6 tabs and 2 spaces.

st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
st->ctrl_ss);

Setting tabs the right size depends on the code editor. For vim, the following
settings should ensure the correct tab size.
set tabstop=8
set shiftwidth=8
set softtabstop=0
set noexpandtab

See Documentation/process/coding-style.rst for complete code style guidance.

Apply the same spacing to the other cases.

Regards,
Marcelo