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

[Billy Tsai]

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;

Got it. I will replace it to later one.


> > +}

> > +/*
> > + * 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.

Good catch, I’ll avoid this blank lines problem.

> >  /* 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()?

Use FIELD_MODIFY() for the channel field update is more better.

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

Thanks

Billy Tsai