Message-ID: <68452dcc-027a-4170-b9f5-2b064972b618@baylibre.com>
Date: Sat, 22 Nov 2025 10:52:00 -0600
From: David Lechner <dlechner@...libre.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, jic23@...nel.org,
 robh@...nel.org, conor+dt@...nel.org, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 2/2] iio: amplifiers: adl8113: add driver support

On 11/21/25 3:52 AM, Antoniu Miclaus wrote:
> Add support for adl8113 10MHz to 12GHz Low Noise Amplifier with
> 10MHz to 14GHz bypass switches.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> 
> Changes in v4:
> - Replace signal_path sysfs attribute with hardwaregain-only control
> - Add support for external bypass amplifiers via DT properties
> - Introduce dynamic gain configuration system with adl8113_gain_config structure
> ---
>  drivers/iio/amplifiers/Kconfig   |  12 ++
>  drivers/iio/amplifiers/Makefile  |   1 +
>  drivers/iio/amplifiers/adl8113.c | 277 +++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/iio/amplifiers/adl8113.c
> 
> diff --git a/drivers/iio/amplifiers/Kconfig b/drivers/iio/amplifiers/Kconfig
> index 55eb16b32f6c..a8a604863eed 100644
> --- a/drivers/iio/amplifiers/Kconfig
> +++ b/drivers/iio/amplifiers/Kconfig
> @@ -36,6 +36,18 @@ config ADA4250
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ada4250.
>  
> +config ADL8113
> +	tristate "Analog Devices ADL8113 Low Noise Amplifier"
> +	depends on GPIOLIB
> +	help
> +	  Say yes here to build support for Analog Devices ADL8113 Low Noise
> +	  Amplifier with integrated bypass switches. The device supports four
> +	  operation modes controlled by GPIO pins: internal amplifier,
> +	  internal bypass, and two external bypass modes.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called adl8113.
> +
>  config HMC425
>  	tristate "Analog Devices HMC425A and similar GPIO Gain Amplifiers"
>  	depends on GPIOLIB
> diff --git a/drivers/iio/amplifiers/Makefile b/drivers/iio/amplifiers/Makefile
> index 2126331129cf..0a76443be1aa 100644
> --- a/drivers/iio/amplifiers/Makefile
> +++ b/drivers/iio/amplifiers/Makefile
> @@ -6,4 +6,5 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_AD8366) += ad8366.o
>  obj-$(CONFIG_ADA4250) += ada4250.o
> +obj-$(CONFIG_ADL8113) += adl8113.o
>  obj-$(CONFIG_HMC425) += hmc425a.o
> diff --git a/drivers/iio/amplifiers/adl8113.c b/drivers/iio/amplifiers/adl8113.c
> new file mode 100644
> index 000000000000..118789aa2c19
> --- /dev/null
> +++ b/drivers/iio/amplifiers/adl8113.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ADL8113 Low Noise Amplifier with integrated bypass switches
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +enum adl8113_signal_path {
> +	ADL8113_INTERNAL_AMP,
> +	ADL8113_INTERNAL_BYPASS,
> +	ADL8113_EXTERNAL_A,
> +	ADL8113_EXTERNAL_B,
> +};
> +
> +struct adl8113_gain_config {
> +	enum adl8113_signal_path path;
> +	int gain_db;
> +	int va;
> +	int vb;
> +};
> +
> +struct adl8113_state {
> +	struct gpio_desc *gpio_va;
> +	struct gpio_desc *gpio_vb;
> +	struct adl8113_gain_config *gain_configs;
> +	int num_gain_configs;
> +	enum adl8113_signal_path current_path;
> +};
> +
> +static const char * const adl8113_supply_names[] = {
> +	"vdd1",
> +	"vdd2",
> +	"vss2"
> +};

Datasheet says recommended power on sequence is VDD_SW, VSS, VDD_PA.
Don't know why they used different names for just that one section
in the datasheet, but at least vss2 should go in the middle. Not
sure which VSS is which though.

> +
> +static int adl8113_set_path(struct adl8113_state *st,
> +			    enum adl8113_signal_path path)
> +{
> +	int va, vb;
> +
> +	/* Determine GPIO values based on signal path */
> +	switch (path) {
> +	case ADL8113_INTERNAL_AMP:
> +		va = 0; vb = 0; /* Internal amplifier */
> +		break;
> +	case ADL8113_INTERNAL_BYPASS:
> +		va = 1; vb = 1; /* Internal bypass */
> +		break;
> +	case ADL8113_EXTERNAL_A:
> +		va = 0; vb = 1; /* External bypass A */
> +		break;
> +	case ADL8113_EXTERNAL_B:
> +		va = 1; vb = 0; /* External bypass B */
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

This could be simplified by using a gpiod array.

> +
> +	gpiod_set_value(st->gpio_va, va);
> +	gpiod_set_value(st->gpio_vb, vb);

This doesn't need to be atomic, so we can use the _cansleep() versions of
the gpio functions.

> +	st->current_path = path;
> +	return 0;
> +}
> +
> +static int adl8113_find_gain_config(struct adl8113_state *st, int gain_db)
> +{
> +	int i;
> +
> +	for (i = 0; i < st->num_gain_configs; i++) {
> +		if (st->gain_configs[i].gain_db == gain_db)
> +			return i;
> +	}
> +	return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec adl8113_channels[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> +		.indexed = 1,
> +		.channel = 0,

.indexed and .channel have no effect for shared_by_type so can be
omitted.

> +	},
> +};
> +
> +static int adl8113_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct adl8113_state *st = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		/* Find current gain configuration */
> +		for (i = 0; i < st->num_gain_configs; i++) {
> +			if (st->gain_configs[i].path == st->current_path) {
> +				*val = st->gain_configs[i].gain_db;
> +				*val2 = 0;
> +				return IIO_VAL_INT_PLUS_MICRO_DB;
> +			}
> +		}
> +		return -EINVAL;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int adl8113_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct adl8113_state *st = iio_priv(indio_dev);
> +	int config_idx;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		if (val2 != 0)
> +			return -EINVAL;
> +
> +		config_idx = adl8113_find_gain_config(st, val);
> +		if (config_idx < 0)
> +			return -EINVAL;

			return config_idx; 

would be more typical.

> +
> +		return adl8113_set_path(st, st->gain_configs[config_idx].path);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info adl8113_info = {
> +	.read_raw = adl8113_read_raw,
> +	.write_raw = adl8113_write_raw,
> +};
> +
> +static int adl8113_init_gain_configs(struct device *dev, struct adl8113_state *st)
> +{
> +	int external_a_gain, external_b_gain;
> +	int num_configs = 2; /* Always have internal amp and internal bypass */
> +	int i = 0;
> +
> +	/* Check for optional external bypass amplifier gains */
> +	if (!device_property_read_u32(dev, "adi,external-bypass-a-gain-db",
> +				      &external_a_gain))

Looks like we could just use device_property_present() here.

> +		num_configs++;
> +
> +	if (!device_property_read_u32(dev, "adi,external-bypass-b-gain-db",
> +				      &external_b_gain))
> +		num_configs++;
> +
> +	st->gain_configs = devm_kcalloc(dev, num_configs,
> +					sizeof(*st->gain_configs), GFP_KERNEL);
> +	if (!st->gain_configs)
> +		return -ENOMEM;
> +
> +	/* Always include internal amplifier (14dB) */
> +	st->gain_configs[i].path = ADL8113_INTERNAL_AMP;
> +	st->gain_configs[i].gain_db = 14;
> +	i++;
> +
> +	/* Always include internal bypass (0dB) */
> +	st->gain_configs[i].path = ADL8113_INTERNAL_BYPASS;
> +	st->gain_configs[i].gain_db = 0;

The datasheet says there is actually a loss here. In one place it
says 2.2dB and in another 1.8db.

So it would make sense for this to be -2.

> +	i++;
> +
> +	/* Add external bypass A if configured */
> +	if (!device_property_read_u32(dev, "adi,external-bypass-a-gain-db",
> +				      &external_a_gain)) {
> +		st->gain_configs[i].path = ADL8113_EXTERNAL_A;
> +		st->gain_configs[i].gain_db = external_a_gain;
> +		i++;
> +	}
> +
> +	/* Add external bypass B if configured */
> +	if (!device_property_read_u32(dev, "adi,external-bypass-b-gain-db",
> +				      &external_b_gain)) {
> +		st->gain_configs[i].path = ADL8113_EXTERNAL_B;
> +		st->gain_configs[i].gain_db = external_b_gain;
> +		i++;
> +	}
> +
> +	st->num_gain_configs = num_configs;

The datasheet mentioned switching to a bypass with nothing connected for
testing, so it could make sense that if there is a free config, add one
more with gain of INT_MIN (for near-infinite attenuation) to allow that
if needed.

> +
> +	/* Check for duplicate gain values */
> +	for (i = 0; i < num_configs - 1; i++) {
> +		for (int j = i + 1; j < num_configs; j++) {

This type of inline variable declaration is usually avoided in the kernel.

> +			if (st->gain_configs[i].gain_db == st->gain_configs[j].gain_db) {
> +				dev_err(dev, "Duplicate gain values not allowed: %d dB\n",
> +					st->gain_configs[i].gain_db);
> +				return -EINVAL;

Should be OK to use dev_err_probe() here.

> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int adl8113_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct adl8113_state *st;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +
> +	st->gpio_va = devm_gpiod_get(dev, "va", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_va))
> +		return dev_err_probe(dev, PTR_ERR(st->gpio_va),
> +				     "failed to get VA GPIO\n");
> +
> +	st->gpio_vb = devm_gpiod_get(dev, "vb", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->gpio_vb))
> +		return dev_err_probe(dev, PTR_ERR(st->gpio_vb),
> +				     "failed to get VB GPIO\n");
> +
> +	ret = devm_regulator_bulk_get_enable(dev,
> +					     ARRAY_SIZE(adl8113_supply_names),
> +					     adl8113_supply_names);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get and enable supplies\n");
> +
> +	/* Initialize gain configurations from devicetree */
> +	ret = adl8113_init_gain_configs(dev, st);
> +	if (ret)
> +		return ret;
> +
> +	/* Initialize to internal amplifier path (14dB) */
> +	ret = adl8113_set_path(st, ADL8113_INTERNAL_AMP);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->info = &adl8113_info;
> +	indio_dev->name = "adl8113";
> +	indio_dev->channels = adl8113_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(adl8113_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;

This doesn't do single conversions, so no INDIO_DIRECT_MODE (or any
other mode).

> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct of_device_id adl8113_of_match[] = {
> +	{ .compatible = "adi,adl8113" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, adl8113_of_match);
> +
> +static struct platform_driver adl8113_driver = {
> +	.driver = {
> +		.name = "adl8113",
> +		.of_match_table = adl8113_of_match,
> +	},
> +	.probe = adl8113_probe,
> +};
> +
> +module_platform_driver(adl8113_driver);
> +
> +MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@...log.com>");
> +MODULE_DESCRIPTION("Analog Devices ADL8113 Low Noise Amplifier");
> +MODULE_LICENSE("GPL");

Powered by blists - more mailing lists