From: Rodrigo Gobbi rodrigo.gobbi.7@gmail.com
[ Upstream commit d75c7021c08e8ae3f311ef2464dca0eaf75fab9f ]
avg sample info is a bit field coded inside the following bits: 5,6,7 and 8 of a device status register.
Channel num info the same, but over bits: 1, 2 and 3.
Mask both values in order to avoid touching other register bits, since the first info (avg sample), came from DT.
Signed-off-by: Rodrigo Gobbi rodrigo.gobbi.7@gmail.com Reviewed-by: David Lechner dlechner@baylibre.com Link: https://patch.msgid.link/20250717221559.158872-1-rodrigo.gobbi.7@gmail.com Signed-off-by: Jonathan Cameron Jonathan.Cameron@huawei.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation
- Bug fixed: The original code shifts unmasked values into the status register, so out-of-range inputs can clobber unrelated control bits. Specifically, `avg_samples` is read from firmware/DT without runtime validation and then used as `SPEAR_ADC_STATUS_AVG_SAMPLE(st->avg_samples)` (a plain left shift) when building the status word in `spear_adc_read_raw()` at drivers/iio/adc/spear_adc.c:160 and drivers/iio/adc/spear_adc.c:161. Since the average field resides in bits 5–8, any high bit of `avg_samples` bleeds into higher status bits. Critically, if `avg_samples` has bit 4 set (value >= 16), then `(x << 5)` sets bit 9, which is `SPEAR_ADC_STATUS_VREF_INTERNAL` (drivers/iio/adc/spear_adc.c:35). That can force internal Vref selection even when an external Vref is configured, causing wrong measurements and unpredictable behavior. - Source of the risk: `avg_samples` comes from DT via `device_property_read_u32(dev, "average-samples", &st->avg_samples);` with no runtime bounds checking (drivers/iio/adc/spear_adc.c:319). While the binding restricts it to 0..15 (Documentation/devicetree/bindings/iio/adc/st,spear600-adc.yaml:43), the driver cannot rely on DT schema validation being present or enforced at runtime. - The fix: The patch adds `#include <linux/bitfield.h>` and replaces the shift macros with masks using `GENMASK` and `FIELD_PREP`, ensuring values are masked to their field width before being merged: - Replaces `#define SPEAR_ADC_STATUS_CHANNEL_NUM(x) ((x) << 1)` (drivers/iio/adc/spear_adc.c:32) with `#define SPEAR_ADC_STATUS_CHANNEL_NUM_MASK GENMASK(3, 1)` and uses `FIELD_PREP` when composing the register. - Replaces `#define SPEAR_ADC_STATUS_AVG_SAMPLE(x) ((x) << 5)` (drivers/iio/adc/spear_adc.c:34) with `#define SPEAR_ADC_STATUS_AVG_SAMPLE_MASK GENMASK(8, 5)` and uses `FIELD_PREP`. - In `spear_adc_read_raw()`, it now uses `FIELD_PREP(SPEAR_ADC_STATUS_CHANNEL_NUM_MASK, chan->channel)` and `FIELD_PREP(SPEAR_ADC_STATUS_AVG_SAMPLE_MASK, st->avg_samples)` instead of raw shifts when building `status`. - Scope and risk: The change is small and localized to a single driver and code path used for starting a conversion. It introduces no new features or architectural changes. The new include `<linux/bitfield.h>` is standard in supported stable kernels and `FIELD_PREP/GENMASK` are widely used in-tree. Masking the channel is also a safe improvement (even though `chan->channel` is in-range), keeping register writes robust. - User impact: Prevents accidental toggling of unrelated control bits (notably Vref selection) and writing ones to reserved/unknown bits if DT passes an out-of-range `average-samples`. This is a clear functional bug that can affect users with misconfigured or legacy DTs. - Stable criteria: - Fixes a real bug (register bit clobbering; can produce incorrect ADC behavior). - Minimal and contained change. - No functional side effects beyond enforcing correct bitfields. - Touches a single IIO ADC driver, not core subsystems. - Commit message explains rationale; even without an explicit “Cc: stable” tag, it meets stable backport rules.
Conclusion: This is a straightforward, low-risk bug fix that prevents corruption of control bits when programming the ADC status register. It should be backported to stable.
drivers/iio/adc/spear_adc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c index e3a865c79686e..df100dce77da4 100644 --- a/drivers/iio/adc/spear_adc.c +++ b/drivers/iio/adc/spear_adc.c @@ -14,6 +14,7 @@ #include <linux/kernel.h> #include <linux/slab.h> #include <linux/io.h> +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/err.h> #include <linux/completion.h> @@ -29,9 +30,9 @@
/* Bit definitions for SPEAR_ADC_STATUS */ #define SPEAR_ADC_STATUS_START_CONVERSION BIT(0) -#define SPEAR_ADC_STATUS_CHANNEL_NUM(x) ((x) << 1) +#define SPEAR_ADC_STATUS_CHANNEL_NUM_MASK GENMASK(3, 1) #define SPEAR_ADC_STATUS_ADC_ENABLE BIT(4) -#define SPEAR_ADC_STATUS_AVG_SAMPLE(x) ((x) << 5) +#define SPEAR_ADC_STATUS_AVG_SAMPLE_MASK GENMASK(8, 5) #define SPEAR_ADC_STATUS_VREF_INTERNAL BIT(9)
#define SPEAR_ADC_DATA_MASK 0x03ff @@ -157,8 +158,8 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_RAW: mutex_lock(&st->lock);
- status = SPEAR_ADC_STATUS_CHANNEL_NUM(chan->channel) | - SPEAR_ADC_STATUS_AVG_SAMPLE(st->avg_samples) | + status = FIELD_PREP(SPEAR_ADC_STATUS_CHANNEL_NUM_MASK, chan->channel) | + FIELD_PREP(SPEAR_ADC_STATUS_AVG_SAMPLE_MASK, st->avg_samples) | SPEAR_ADC_STATUS_START_CONVERSION | SPEAR_ADC_STATUS_ADC_ENABLE; if (st->vref_external == 0)