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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®