Re: [PATCH v5 09/10] iio: adc: ti-ads1015: inline ads1015_set_conv_mode()
From: Andy Shevchenko
Date: Thu Oct 01 2026 - 13:54:52 EST
On Wed, Sep 30, 2026 at 10:42:03PM +0530, Archit Anant wrote:
> The ads1015_set_conv_mode() helper function is a single-line wrapper
> around regmap_update_bits().
>
> Since the function is simple and only called during initialization and
> power management state transitions, the wrapper is unnecessary. Remove
> it and inline the regmap_update_bits() logic directly at the call sites
> to improve code readability and traceabilty.
>
> Where applicable, introduce a local 'map' pointer to keep the
> inlined function calls concise.
...
> - ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ret = regmap_update_bits(map, ADS1015_CFG_REG,
> ADS1015_CFG_COMP_QUE_MASK,
> ADS1015_CFG_COMP_DISABLE <<
> ADS1015_CFG_COMP_QUE_SHIFT);
The indentation is broken even in the original code.
> static void ads1015_power_down(void *p)
> {
> struct ads1015_data *data = p;
> + struct regmap *map = data->regmap;
> int ret;
>
> - ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
> + ret = regmap_update_bits(map, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
> if (ret)
> dev_warn(regmap_get_device(data->regmap),
At some point (may be even in this patch) it makes sense to switch to use 'map'
in all applicable cases.
> "Failed to power down (%pe)\n", ERR_PTR(ret));
...
> static int ads1015_probe(struct i2c_client *client)
>
> - ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
Can you also use 'map' here?
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> if (ret)
> return ret;
...
> + ret = regmap_update_bits(map, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
This is now perfectly a single line, same for the cases above.
> + ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> if (!ret)
> data->conv_invalid = true;
--
With Best Regards,
Andy Shevchenko