From: "Nuno Sá" <nuno.sa@analog.com>
To: Janani Sunil <janani.sunil@analog.com>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>,
Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>,
linux-iio@vger.kernel.org, linux@analog.com,
linux-kernel@vger.kernel.org, jan.sun97@gmail.com,
Andy Shevchenko <andriy.shevchenko@intel.com>
Subject: Re: [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration
Date: Mon, 12 Oct 2026 15:45:47 +0100 [thread overview]
Message-ID: <aszyX_e44Dp2MSln@nsa> (raw)
In-Reply-To: <20261008-ad5529r-review-followup-v1-1-84a98e0e27eb@analog.com>
On Thu, Oct 08, 2026 at 11:19:15AM +0200, Janani Sunil wrote:
> Use fixed width register literals matching the device's register address
> width. Rely on SPI regmap's default read flag instead of specifying the
> same value explicitly, and remove parentheses around the channel
> initializer.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> The AD5529R driver is already queued in the IIO tree. This patch follows
> up on the remaining review comments from Andy Shevchenko:
>
> https://lore.kernel.org/r/arzTnOTaL97GudZM@ashevche-desk.local/
> ---
Seems like there are some other things recommended by Andy. But those
can be different patches. Hence for this one:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> drivers/iio/dac/ad5529r.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> index 8154b87ff831..04bb02af9612 100644
> --- a/drivers/iio/dac/ad5529r.c
> +++ b/drivers/iio/dac/ad5529r.c
> @@ -24,19 +24,19 @@
>
> #include <linux/iio/iio.h>
>
> -#define AD5529R_REG_INTERFACE_CONFIG_A 0x00
> +#define AD5529R_REG_INTERFACE_CONFIG_A 0x000
> #define AD5529R_INTERFACE_CONFIG_A_SW_RESET (BIT(7) | BIT(0))
> #define AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION BIT(5)
> #define AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE BIT(4)
> -#define AD5529R_REG_DEVICE_CONFIG 0x02
> -#define AD5529R_REG_CHIP_GRADE 0x06
> -#define AD5529R_REG_SCRATCH_PAD 0x0A
> -#define AD5529R_REG_SPI_REVISION 0x0B
> -#define AD5529R_REG_VENDOR_H 0x0D
> -#define AD5529R_REG_STREAM_MODE 0x0E
> -#define AD5529R_REG_INTERFACE_STATUS_A 0x11
> -#define AD5529R_REG_MULTI_DAC_CH_SEL 0x14
> -#define AD5529R_REG_OUT_RANGE_BASE 0x3C
> +#define AD5529R_REG_DEVICE_CONFIG 0x002
> +#define AD5529R_REG_CHIP_GRADE 0x006
> +#define AD5529R_REG_SCRATCH_PAD 0x00A
> +#define AD5529R_REG_SPI_REVISION 0x00B
> +#define AD5529R_REG_VENDOR_H 0x00D
> +#define AD5529R_REG_STREAM_MODE 0x00E
> +#define AD5529R_REG_INTERFACE_STATUS_A 0x011
> +#define AD5529R_REG_MULTI_DAC_CH_SEL 0x014
> +#define AD5529R_REG_OUT_RANGE_BASE 0x03C
> #define AD5529R_REG_OUT_RANGE(ch) (AD5529R_REG_OUT_RANGE_BASE + (ch) * 2)
> #define AD5529R_REG_DAC_INPUT_A_BASE 0x148
> #define AD5529R_REG_DAC_INPUT_A(ch) (AD5529R_REG_DAC_INPUT_A_BASE + (ch) * 2)
> @@ -50,9 +50,8 @@
> #define AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC 0x1A8
>
> #define AD5529R_MAX_REGISTER 0x232
> -#define AD5529R_8BIT_REG_MAX 0x13
> +#define AD5529R_8BIT_REG_MAX 0x013
> #define AD5529R_MAX_CHANNELS 16
> -#define AD5529R_SPI_READ_FLAG 0x80
> #define AD5529R_ADDR_SHIFT 12
>
> struct ad5529r_model_data {
> @@ -60,7 +59,7 @@ struct ad5529r_model_data {
> unsigned int resolution;
> };
>
> -#define AD5529R_DAC_CHANNEL(chan) ((struct iio_chan_spec) { \
> +#define AD5529R_DAC_CHANNEL(chan) (struct iio_chan_spec) { \
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> .output = 1, \
> @@ -68,7 +67,7 @@ struct ad5529r_model_data {
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> BIT(IIO_CHAN_INFO_SCALE) | \
> BIT(IIO_CHAN_INFO_OFFSET), \
> -})
> + }
>
> static const char * const ad5529r_supply_names[] = {
> "vdd-io",
> @@ -433,7 +432,6 @@ static int ad5529r_probe(struct spi_device *spi)
> .reg_bits = 16,
> .val_bits = 8,
> .max_register = AD5529R_8BIT_REG_MAX,
> - .read_flag_mask = AD5529R_SPI_READ_FLAG,
> .rd_table = &ad5529r_8bit_readable_table,
> .wr_table = &ad5529r_8bit_writeable_table,
> .reg_base = dev_addr << AD5529R_ADDR_SHIFT,
> @@ -443,7 +441,6 @@ static int ad5529r_probe(struct spi_device *spi)
> .reg_bits = 16,
> .val_bits = 16,
> .max_register = AD5529R_MAX_REGISTER,
> - .read_flag_mask = AD5529R_SPI_READ_FLAG,
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> .rd_table = &ad5529r_16bit_readable_table,
> .wr_table = &ad5529r_16bit_writeable_table,
>
> ---
> base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
> change-id: 20261006-ad5529r-review-followup-22c8547c5b65
>
> Best regards,
> --
> Janani Sunil <janani.sunil@analog.com>
>
next prev parent reply other threads:[~2026-10-12 14:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-08 9:19 [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration Janani Sunil
2026-10-12 14:45 ` Nuno Sá [this message]
2026-10-12 21:19 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aszyX_e44Dp2MSln@nsa \
--to=nuno.sa@analog.com \
--cc=Michael.Hennerich@analog.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jan.sun97@gmail.com \
--cc=janani.sunil@analog.com \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.