[PATCH v2 2/3] iio: adc: Enable multiple consecutive channels based on model data

Billy Tsai posted 3 patches 3 weeks, 1 day ago
There is a newer version of this series
[PATCH v2 2/3] iio: adc: Enable multiple consecutive channels based on model data
Posted by Billy Tsai 3 weeks, 1 day ago
Add helpers to generate channel masks and enable multiple ADC channels
according to the device model's channel count.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index af9a95d31d81..81a2dd752541 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -72,6 +72,29 @@
 #define ASPEED_ADC_BAT_SENSING_ENABLE		BIT(13)
 #define ASPEED_ADC_CTRL_CHANNEL			GENMASK(31, 16)
 #define ASPEED_ADC_CTRL_CHANNEL_ENABLE(ch)	FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, BIT(ch))
+
+/*
+ * Enable multiple consecutive channels starting from channel 0.
+ * This creates a bitmask for channels 0 to (num_channels - 1).
+ * For example: num_channels=3 creates mask 0x0007 (channels 0,1,2)
+ */
+static inline u32 aspeed_adc_channels_mask(unsigned int num_channels)
+{
+	if (num_channels == 0)
+		return 0;
+	if (num_channels >= 16)
+		return GENMASK(15, 0);
+	return GENMASK(num_channels - 1, 0);
+}
+
+/*
+ * Helper function to enable multiple channels in the control register
+ */
+static inline u32 aspeed_adc_enable_channels(unsigned int num_channels)
+{
+	return FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, aspeed_adc_channels_mask(num_channels));
+}
+
 /* Battery sensing is typically on the last channel */
 #define ASPEED_ADC_BATTERY_CHANNEL		7
 
@@ -123,6 +146,11 @@ struct aspeed_adc_data {
 	struct adc_gain		battery_mode_gain;
 };
 
+static inline unsigned int aspeed_adc_get_active_channels(const struct aspeed_adc_data *data)
+{
+	return data->model_data->num_channels;
+}
+
 #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
@@ -610,9 +638,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 
 	aspeed_adc_compensation(indio_dev);
 	/* Start all channels in normal mode. */
-	adc_engine_control_reg_val =
-		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
-	adc_engine_control_reg_val |= ASPEED_ADC_CTRL_CHANNEL;
+	adc_engine_control_reg_val = readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+	adc_engine_control_reg_val |=
+		aspeed_adc_enable_channels(aspeed_adc_get_active_channels(data));
+
 	writel(adc_engine_control_reg_val,
 	       data->base + ASPEED_REG_ENGINE_CONTROL);
 

-- 
2.34.1
Re: [PATCH v2 2/3] iio: adc: Enable multiple consecutive channels based on model data
Posted by Andy Shevchenko 3 weeks ago
On Mon, Mar 16, 2026 at 11:00:47AM +0800, Billy Tsai wrote:
> Add helpers to generate channel masks and enable multiple ADC channels
> according to the device model's channel count.

...

> +

(1)

> +/*
> + * Enable multiple consecutive channels starting from channel 0.
> + * This creates a bitmask for channels 0 to (num_channels - 1).
> + * For example: num_channels=3 creates mask 0x0007 (channels 0,1,2)
> + */
> +static inline u32 aspeed_adc_channels_mask(unsigned int num_channels)
> +{
> +	if (num_channels == 0)
> +		return 0;

> +	if (num_channels >= 16)
> +		return GENMASK(15, 0);
> +	return GENMASK(num_channels - 1, 0);

This entire function can be folded into

	return BIT(min(num_channels, 16U)) - 1;

Or

	if (num_channels > 16)
		return GENMASK(15, 0);

	return BIT(num_channels) - 1;

> +}

> +/*
> + * Helper function to enable multiple channels in the control register
> + */
> +static inline u32 aspeed_adc_enable_channels(unsigned int num_channels)
> +{
> +	return FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, aspeed_adc_channels_mask(num_channels));
> +}

> +

One of these (see 1 above) new blank lines should be rather added in the previous patch.

>  /* Battery sensing is typically on the last channel */
>  #define ASPEED_ADC_BATTERY_CHANNEL		7

...

>  	/* Start all channels in normal mode. */
> -	adc_engine_control_reg_val =
> -		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
> -	adc_engine_control_reg_val |= ASPEED_ADC_CTRL_CHANNEL;
> +	adc_engine_control_reg_val = readl(data->base + ASPEED_REG_ENGINE_CONTROL);

> +	adc_engine_control_reg_val |=
> +		aspeed_adc_enable_channels(aspeed_adc_get_active_channels(data));

Why not FIELD_MODIFY()?

>  	writel(adc_engine_control_reg_val,
>  	       data->base + ASPEED_REG_ENGINE_CONTROL);

-- 
With Best Regards,
Andy Shevchenko