linux-kernel - Re: [PATCH 1/5] iio: adc: ad4080: prepare driver for multi-part support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMknhBG5nPMnSUnPA_4OyX+c=WiCJqTdm-QLwQGo4fdkfR8w9w@mail.gmail.com>
Date: Mon, 29 Sep 2025 17:46:59 +0200
From: David Lechner <dlechner@...libre.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>
Cc: jic23@...nel.org, robh@...nel.org, conor+dt@...nel.org, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH 1/5] iio: adc: ad4080: prepare driver for multi-part support

On Mon, Sep 29, 2025 at 4:59 PM Antoniu Miclaus
<antoniu.miclaus@...log.com> wrote:
>
> Refactor the ad4080 driver to support multiple ADC variants with
> different resolution bits and LVDS CNV clock count maximums.
>
> Changes:
> - Update AD4080_CHIP_ID to correct value 0x50
> - Add lvds_cnv_clk_cnt_max field to chip_info structure
> - Create AD4080_CHANNEL_DEFINE macro for variable resolution/storage bits
> - Use AD4080_REG_PRODUCT_ID_L register for chip identification
> - Make LVDS CNV clock count configurable per chip variant
>
> This prepares the infrastructure for adding support for additional
> ADC parts with different specifications while maintaining backward
> compatibility with existing AD4080 functionality.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
>  drivers/iio/adc/ad4080.c | 46 ++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> index 6e61787ed321..9f670c290a55 100644
> --- a/drivers/iio/adc/ad4080.c
> +++ b/drivers/iio/adc/ad4080.c
> @@ -125,7 +125,7 @@
>
>  /* Miscellaneous Definitions */
>  #define AD4080_SPI_READ                                                BIT(7)
> -#define AD4080_CHIP_ID                                         GENMASK(2, 0)
> +#define AD4080_CHIP_ID                                         0x50

This sounds like a bug fix, so should be in a separate patch with a Fixes: tag

>
>  #define AD4080_LVDS_CNV_CLK_CNT_MAX                            7
>
> @@ -167,6 +167,7 @@ struct ad4080_chip_info {
>         const unsigned int (*scale_table)[2];
>         const struct iio_chan_spec *channels;
>         unsigned int num_channels;
> +       unsigned int lvds_cnv_clk_cnt_max;
>  };
>
>  struct ad4080_state {
> @@ -414,23 +415,25 @@ static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
>         { }
>  };
>
> -static const struct iio_chan_spec ad4080_channel = {
> -       .type = IIO_VOLTAGE,
> -       .indexed = 1,
> -       .channel = 0,
> -       .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),
> -       .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> -                       BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> -       .info_mask_shared_by_all_available =
> -                       BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> -       .ext_info = ad4080_ext_info,
> -       .scan_index = 0,
> -       .scan_type = {
> -               .sign = 's',
> -               .realbits = 20,
> -               .storagebits = 32,
> -       },
> -};
> +#define AD4080_CHANNEL_DEFINE(bits, storage) {\

At least add one space before \. Although I think aligning all \ on
the right is preferred for readability.

> +       .type = IIO_VOLTAGE,\
> +       .indexed = 1,\
> +       .channel = 0,\
> +       .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE),\
> +       .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
> +                       BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\
> +       .info_mask_shared_by_all_available =\
> +                       BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\
> +       .ext_info = ad4080_ext_info,\
> +       .scan_index = 0,\
> +       .scan_type = {\
> +               .sign = 's',\
> +               .realbits = (bits),\
> +               .storagebits = (storage),\
> +       },\
> +}
> +
> +static const struct iio_chan_spec ad4080_channel = AD4080_CHANNEL_DEFINE(20, 32);
>
>  static const struct ad4080_chip_info ad4080_chip_info = {
>         .name = "ad4080",
> @@ -439,6 +442,7 @@ static const struct ad4080_chip_info ad4080_chip_info = {
>         .num_scales = ARRAY_SIZE(ad4080_scale_table),
>         .num_channels = 1,
>         .channels = &ad4080_channel,
> +       .lvds_cnv_clk_cnt_max = AD4080_LVDS_CNV_CLK_CNT_MAX,
>  };
>
>  static int ad4080_setup(struct iio_dev *indio_dev)
> @@ -458,11 +462,11 @@ static int ad4080_setup(struct iio_dev *indio_dev)
>         if (ret)
>                 return ret;
>
> -       ret = regmap_read(st->regmap, AD4080_REG_CHIP_TYPE, &id);
> +       ret = regmap_read(st->regmap, AD4080_REG_PRODUCT_ID_L, &id);

If there is also a high word, then this should probably be
regmap_bulk_read() to read the entire product ID.

>         if (ret)
>                 return ret;
>
> -       if (id != AD4080_CHIP_ID)
> +       if (id != st->info->product_id)
>                 dev_info(dev, "Unrecognized CHIP_ID 0x%X\n", id);
>
>         ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A,
> @@ -488,7 +492,7 @@ static int ad4080_setup(struct iio_dev *indio_dev)
>                                  AD4080_REG_ADC_DATA_INTF_CONFIG_B,
>                                  AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
>                                  FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK,
> -                                           AD4080_LVDS_CNV_CLK_CNT_MAX));
> +                                           st->info->lvds_cnv_clk_cnt_max));
>         if (ret)
>                 return ret;
>
> --
> 2.43.0
>
>