Thread (33 messages) flat view 33 messages, 6 authors, 4d ago

Re: [PATCH v7 07/17] iio: adc: ad7768: Add power mode helper

From: sashiko-bot@kernel.org
Date: 2026-09-10 17:48:04

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect register value is written to the MCLK_DIV bitfield for MEDIAN power mode, leading to hardware and software clock divergence.
--

commit 52c81df3b6fbeb2c9857e18cf49af9fcfe8ebeae
Author: Janani Sunil [off-list ref]

iio: adc: ad7768: Add power mode helper

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.
quoted hunk ↗ jump to hunk
diff --git a/drivers/iio/adc/ad7768.c b/drivers/iio/adc/ad7768.c
index 44ba19fb20962..be815b8c6766e 100644
--- a/drivers/iio/adc/ad7768.c
+++ b/drivers/iio/adc/ad7768.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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 },
+};
+
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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));
[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.
+	if (ret)
+		return ret;
+
+	ret = ad7768_sync(st);
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-ad7768-driver-v7-0-7f70b07627e1@analog.com?part=7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help