Re: [PATCH v7 07/17] iio: adc: ad7768: Add power mode helper
From: Jonathan Cameron <jic23@kernel.org>
Date: 2026-09-13 20:52:11
Also in:
linux-devicetree, linux-doc, linux-iio, lkml
Describe each power mode and its internal master-clock divisor in a table. Replace the open-coded fast-mode setup with a helper that records the active mode, preparing for dynamic mode selection. Signed-off-by: Janani Sunil <janani.sunil@analog.com>
This is just highlighting some feedback from sashiko. I have very little idea what it is talking about! If it is garbage, just reply to say so. Thanks, Jonathan
quoted hunk ↗ jump to hunk
diff --git a/drivers/iio/adc/ad7768.c b/drivers/iio/adc/ad7768.c index 44ba19fb2096..be815b8c6766 100644 --- a/drivers/iio/adc/ad7768.c +++ b/drivers/iio/adc/ad7768.c@@ -41,6 +41,8 @@ #define AD7768_REG_POWER_MODE 0x04 #define AD7768_SLEEP_MODE_MSK BIT(7) #define AD7768_POWER_MODE_POWER_MODE_MSK GENMASK(5, 4) +#define AD7768_POWER_MODE_POWER_MODE_LOW 0x0 +#define AD7768_POWER_MODE_POWER_MODE_MEDIAN 0x2 #define AD7768_POWER_MODE_POWER_MODE_FAST 0x3 #define AD7768_POWER_MODE_LVDS_ENABLE BIT(3) #define AD7768_POWER_MODE_MCLK_DIV_MSK GENMASK(1, 0)@@ -102,6 +104,17 @@ enum ad7768_clock_source { AD7768_CLOCK_SOURCE_LVDS, }; +struct ad7768_power_mode_info { + unsigned int mode; + unsigned int mclk_div; +}; + +static const struct ad7768_power_mode_info ad7768_power_modes[] = { + { .mode = AD7768_POWER_MODE_POWER_MODE_LOW, .mclk_div = 32 }, + { .mode = AD7768_POWER_MODE_POWER_MODE_MEDIAN, .mclk_div = 8 }, + { .mode = AD7768_POWER_MODE_POWER_MODE_FAST, .mclk_div = 4 }, +}; + struct ad7768_precharge_config { bool prebufp_en; bool prebufn_en;@@ -127,6 +140,7 @@ struct ad7768_state { const struct ad7768_chip_info *chip_info; struct iio_backend *back; unsigned int vref_uV[2]; + unsigned int power_mode_idx; __be16 d16 __aligned(IIO_DMA_MINALIGN); };@@ -330,6 +344,32 @@ static int ad7768_sync(struct ad7768_state *st) AD7768_DATA_CONTROL_SPI_SYNC); } +static int ad7768_set_power_mode(struct ad7768_state *st, + unsigned int mode_idx) +{ + const struct ad7768_power_mode_info *mode_info; + int ret; + + mode_info = &ad7768_power_modes[mode_idx]; + ret = regmap_update_bits(st->regmap, AD7768_REG_POWER_MODE, + AD7768_POWER_MODE_POWER_MODE_MSK | + AD7768_POWER_MODE_MCLK_DIV_MSK, + FIELD_PREP(AD7768_POWER_MODE_POWER_MODE_MSK, + mode_info->mode) | + FIELD_PREP(AD7768_POWER_MODE_MCLK_DIV_MSK, + mode_info->mode));
Sashiko: [Severity: High] When configuring MEDIAN mode via ad7768_set_power_mode(), does this code inadvertently configure the MCLK_DIV bitfield using the power mode register value instead of the correct clock divider register code? For MEDIAN power mode, mode_info->mode is 0x2. Writing 0x2 to the MCLK_DIV bitfield sets the hardware divider to 4 (MCLK/4), instead of the expected divider of 8 (MCLK/8, which requires register value 0x1). Since ad7768_power_modes correctly defines mclk_div = 8 for MEDIAN mode, the software will calculate output rates and delays based on a divider of 8, while the hardware runs twice as fast. - I haven't looked into this one so please check it out. -- Jonathan Cameron [off-list ref]