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

[Joshua Crofts <crofts@kernel.org>]

On Wed,  4 Nov 2026 13:57:25 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

...

> diff --git a/drivers/iio/adc/ti-tla2528.c b/drivers/iio/adc/ti-tla2528.c
> new file mode 100644
> index 000000000000..ec754ec43d7c
> --- /dev/null
> +++ b/drivers/iio/adc/ti-tla2528.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Texas Instruments TLA2528 ADC
> + *
> + * Copyright (C) 2020-2021 Rodolfo Giometti <giometti@enneenne.com>
> + * Copyright (C) 2025 Maxime Chevallier <maxime.chevallier@bootlin.com>
> + * Datasheet: https://www.ti.com/lit/gpn/tla2528
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>

Remove mod_devicetable.h, now i2c.h supplies this. See Uwe Kleine-Konig's
work on this.

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/adc-helpers.h>
> +#include <linux/iio/iio.h>
> +

+ #include <asm/byteorder.h> for be16_to_cpu()

> +#define TLA2528_OP_WRITE_REG		0x08
> +#define TLA2528_OP_READ_REG		0x10
> +
> +#define TLA2528_GENERAL_CFG_ADR		0x01
> +#define TLA2528_GENERAL_CFG_RST		BIT(0)
> +#define TLA2528_CHANNEL_SEL_ADR		0x11
> +#define TLA2528_CHANNEL_SEL_MANUAL_CHID	GENMASK(3, 0)
> +#define TLA2528_AUTO_SEQ_CH_SEL_ADR	0x12
> +
> +#define TLA2528_NUM_CHANNELS		8
> +
> +struct tla2528 {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	int vref_mv;

Slight nit, but _mV so we emphasize this is a SI unit.

> +
> +	/* Protects manual channel selection */
> +	struct mutex lock;
> +};
> +

...

> +static int tla2528_read(struct tla2528 *tla2528, u8 channel, int *val)
> +{
> +	__be16 data;
> +	int ret;
> +
> +	guard(mutex)(&tla2528->lock);
> +
> +	ret = regmap_update_bits(tla2528->regmap, TLA2528_CHANNEL_SEL_ADR,
> +				 TLA2528_CHANNEL_SEL_MANUAL_CHID, channel);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_master_recv(tla2528->client, (char *)&data, sizeof(data));
> +	if (ret < 0)
> +		return ret;
>

As rare as short reads may be, perhaps add a check whether the expected amount
of bits have been returned from the device?

-- 
Kind regards,
Joshua Crofts