Re: [PATCH 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

Andy Shevchenko
2025-12-29T09:40:28Z

On Tue, Dec 23, 2025 at 5:55 PM Maxime Chevallier
<maxime.chevallier@bootlin.com> wrote:
>
> This adds a new driver for the TI TLA2528 ADC chip. It ha 8 12-bit
> channels, that can also be configured as 16-bit averaging channels.
>
> Add a very simple driver for it, allowing reading raw values for each
> channel.

...

> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regulator/consumer.h>

Follow IWYU. Here are missing headers, such as bits.h, mutex.h, types.h.

...

> + case IIO_CHAN_INFO_SCALE:
> + *val = tla2528->vref_uv / 1000;

(MICRO/MILLI) ?

> + *val2 = 12;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;

> + tla2528->vref_uv = devm_regulator_get_enable_read_voltage(&client->dev,
> + "vref");

With

struct device *dev = &client->dev;

at the top of the function this will be one line and others also can
be shortened.

> + if (tla2528->vref_uv < 0)
> + return tla2528->vref_uv;

...

> + /* Set all inputs as analog */
> + ret = tla2528_write_reg(tla2528->client, TLA2528_PIN_CFG_ADR, 0x00);

Why not simply use the "client"?

> + if (ret < 0)
> + return ret;
> +
> + ret = tla2528_write_reg(tla2528->client, TLA2528_DATA_CFG_ADR,
> + TLA2528_DATA_CFG_APPEND_STATUS);
> + if (ret < 0)
> + return ret;
> +
> + /* Set manual mode */
> + ret = tla2528_write_reg(tla2528->client, TLA2528_SEQUENCE_CFG_ADR, 0x00);
> + if (ret < 0)
> + return ret;

Ditto.

...

> +static const struct i2c_device_id tla2528_id[] = {
> + { "tla2528", 0 },

No ', 0' part.

> + { }
> +};

...

> +static const struct of_device_id tla2528_of_match[] = {
> + { .compatible = "ti,tla2528", },

No inner comma.

> + { },

No trailing comma.

> +};

--
With Best Regards,
Andy Shevchenko