Re: [PATCH v5 05/16] iio: adc: xilinx-xadc: Use bitfield helpers and bit macros

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 10 Nov 2026 10:31:50 +0530
Dileep Kumar Nagavarapu <dileepkumar.nagavarapu@amd.com> wrote:

> Replace open-coded bit manipulation with FIELD_GET(),
> FIELD_PREP(), BIT(), and GENMASK() helpers.
> 
> Using the kernel bitfield helpers improves readability and
> makes bit operations less error-prone.
> 
> No functional changes intended.
> 
> Co-developed-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
> Signed-off-by: Dileep Kumar Nagavarapu <dileepkumar.nagavarapu@amd.com>
Sashiko seems to have found an issue. Looks plausible to me.
https://sashiko.dev/#/patchset/20261110050201.1850399-1-dileepkumar.nagavarapu%40amd.com

> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 50 ++++++++++++++++++------------
>  drivers/iio/adc/xilinx-xadc.h      | 29 +++++++++--------
>  2 files changed, 46 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index 66c6f3319ca0..eb63675464bb 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -11,6 +11,8 @@
>   *  - AXI XADC interface: Xilinx PG019
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
>  #include <linux/clk.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> @@ -47,28 +49,28 @@ static const unsigned int XADC_ZYNQ_UNMASK_TIMEOUT = 500;
>  #define XADC_ZYNQ_REG_CTL		0x18
>  
>  #define XADC_ZYNQ_CFG_ENABLE		BIT(31)
> -#define XADC_ZYNQ_CFG_CFIFOTH_MASK	(0xf << 20)
> +#define XADC_ZYNQ_CFG_CFIFOTH_MASK	GENMASK(23, 20)
>  #define XADC_ZYNQ_CFG_CFIFOTH_OFFSET	20
> -#define XADC_ZYNQ_CFG_DFIFOTH_MASK	(0xf << 16)
> +#define XADC_ZYNQ_CFG_DFIFOTH_MASK	GENMASK(19, 16)
>  #define XADC_ZYNQ_CFG_DFIFOTH_OFFSET	16

As part of this patch I'd expect the changes to FIELD_PREP()
and FIELD_GET() to remove the need for these offsets.

This one is no longer used for example. I'd expect almost all
if not all the _OFFSET defines to be dropped.
Many of them weren't used in the first place. I didn't track
down if any are still used after this patch.

>  #define XADC_ZYNQ_CFG_WEDGE		BIT(13)
>  #define XADC_ZYNQ_CFG_REDGE		BIT(12)
> -#define XADC_ZYNQ_CFG_TCKRATE_MASK	(0x3 << 8)
> -#define XADC_ZYNQ_CFG_TCKRATE_DIV2	(0x0 << 8)

>  
> @@ -822,7 +832,7 @@ static int xadc_preenable(struct iio_dev *indio_dev)
>  	int ret;
>  
>  	ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_SEQ_MASK,
> -				  XADC_CONF1_SEQ_DEFAULT);
> +				  FIELD_PREP(XADC_CONF1_SEQ_MASK, XADC_CONF1_SEQ_DEFAULT));
>  	if (ret)
>  		goto err;
>  

Sashiko calls out there is another write of CONF1 later in the function that still
seems to be using the prior approach rather than FIELD_PREP().

> diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
> index fb911ed87600..f5bf2d08eedc 100644
> --- a/drivers/iio/adc/xilinx-xadc.h
> +++ b/drivers/iio/adc/xilinx-xadc.h
> @@ -9,6 +9,7 @@
>  
> -#define XADC_CONF2_DIV_MASK	0xff00
> +#define XADC_CONF2_DIV_MASK	GENMASK(15, 8)
>  #define XADC_CONF2_DIV_OFFSET	8

Similar to above, I'd expect us to not need the _OFFSET defines
after moving to FIELD_PREP/GET()