* [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration
@ 2026-10-08 9:19 Janani Sunil
2026-10-12 14:45 ` Nuno Sá
2026-10-12 21:19 ` Andy Shevchenko
0 siblings, 2 replies; 3+ messages in thread
From: Janani Sunil @ 2026-10-08 9:19 UTC (permalink / raw)
To: Nuno Sá, Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko
Cc: linux-iio, linux, linux-kernel, jan.sun97, Andy Shevchenko,
Janani Sunil
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/
---
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>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration
2026-10-08 9:19 [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration Janani Sunil
@ 2026-10-12 14:45 ` Nuno Sá
2026-10-12 21:19 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Nuno Sá @ 2026-10-12 14:45 UTC (permalink / raw)
To: Janani Sunil
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, linux-iio, linux, linux-kernel, jan.sun97,
Andy Shevchenko
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>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration
2026-10-08 9:19 [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration Janani Sunil
2026-10-12 14:45 ` Nuno Sá
@ 2026-10-12 21:19 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-10-12 21:19 UTC (permalink / raw)
To: Janani Sunil
Cc: Nuno Sá, Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, linux-iio, linux, linux-kernel, jan.sun97
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.
It sounds like a three patches, but I don't care much (up to Jonathan)
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-12 21:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 9:19 [PATCH] iio: dac: ad5529r: Clean up definitions and regmap configuration Janani Sunil
2026-10-12 14:45 ` Nuno Sá
2026-10-12 21:19 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox