Message ID | 20240118085856.70758-3-kimseer.paller@analog.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | Changes to admfm2000 driver | expand |
On Thu, 18 Jan 2024 16:58:56 +0800 Kim Seer Paller <kimseer.paller@analog.com> wrote: > Dual microwave down converter module with input RF and LO frequency > ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to > 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier > for each down conversion path. > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> Hi. A few comments inline. Mainly about reducing some code duplication. The note about autocleanup of fwnode_handle_put is a reference to: https://lore.kernel.org/linux-iio/Za0NxrgBb0ve233b@smile.fi.intel.com/T/ Though I'm not sure that will land exactly as currently implemented, so don't base anything on it yet. > --- > V5 -> V6: Used devm_fwnode_gpiod_get_index for getting array of gpios. > V4 -> V5: Added missing return -ENODEV in setup function. Reordered variable > declarations in probe function. > V1 -> V4: No changes. > > MAINTAINERS | 1 + > drivers/iio/frequency/Kconfig | 10 + > drivers/iio/frequency/Makefile | 1 + > drivers/iio/frequency/admfm2000.c | 307 ++++++++++++++++++++++++++++++ > 4 files changed, 319 insertions(+) > create mode 100644 drivers/iio/frequency/admfm2000.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3a86f9d6cb98..49d320535105 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1266,6 +1266,7 @@ L: linux-iio@vger.kernel.org > S: Supported > W: https://ez.analog.com/linux-software-drivers > F: Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml > +F: drivers/iio/frequency/admfm2000.c > > ANALOG DEVICES INC ADMV1013 DRIVER > M: Antoniu Miclaus <antoniu.miclaus@analog.com> > diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig > index 9e85dfa58508..c455be7d4a1c 100644 > --- a/drivers/iio/frequency/Kconfig > +++ b/drivers/iio/frequency/Kconfig > @@ -60,6 +60,16 @@ config ADF4377 > To compile this driver as a module, choose M here: the > module will be called adf4377. > > +config ADMFM2000 > + tristate "Analog Devices ADMFM2000 Dual Microwave Down Converter" > + depends on GPIOLIB > + help > + Say yes here to build support for Analog Devices ADMFM2000 Dual > + Microwave Down Converter. > + > + To compile this driver as a module, choose M here: the > + module will be called admfm2000. > + > config ADMV1013 > tristate "Analog Devices ADMV1013 Microwave Upconverter" > depends on SPI && COMMON_CLK > diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile > index b616c29b4a08..70d0e0b70e80 100644 > --- a/drivers/iio/frequency/Makefile > +++ b/drivers/iio/frequency/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_AD9523) += ad9523.o > obj-$(CONFIG_ADF4350) += adf4350.o > obj-$(CONFIG_ADF4371) += adf4371.o > obj-$(CONFIG_ADF4377) += adf4377.o > +obj-$(CONFIG_ADMFM2000) += admfm2000.o > obj-$(CONFIG_ADMV1013) += admv1013.o > obj-$(CONFIG_ADMV1014) += admv1014.o > obj-$(CONFIG_ADMV4420) += admv4420.o > diff --git a/drivers/iio/frequency/admfm2000.c b/drivers/iio/frequency/admfm2000.c > new file mode 100644 > index 000000000000..a09ec38fad22 > --- /dev/null > +++ b/drivers/iio/frequency/admfm2000.c > @@ -0,0 +1,307 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ADMFM2000 Dual Microwave Down Converter > + * > + * Copyright 2023 Analog Devices Inc. As you are updating in 2024, this might want an update to include this year. > + */ > + > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/iio/iio.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_device.h> Why this include? You are using stuff from property.h not the of specific stuff. You should have mod_devicetable.h for the of_device_id definition though. > +#include <linux/platform_device.h> > + > +static int admfm2000_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct admfm2000_state *st = iio_priv(indio_dev); > + int gain; > + > + switch (mask) { > + case IIO_CHAN_INFO_HARDWAREGAIN: > + mutex_lock(&st->lock); > + gain = ~(st->gain[chan->channel]) * -1000; > + *val = gain / 1000; > + *val2 = (gain % 1000) * 1000; > + mutex_unlock(&st->lock); > + > + return IIO_VAL_INT_PLUS_MICRO_DB; Odd extra space after return. > + default: > + return -EINVAL; > + } > +} > + > +static int admfm2000_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int val, > + int val2, long mask) > +{ > + struct admfm2000_state *st = iio_priv(indio_dev); > + int gain, ret; > + > + if (val < 0) > + gain = (val * 1000) - (val2 / 1000); > + else > + gain = (val * 1000) + (val2 / 1000); > + > + if (gain > ADMFM2000_MAX_GAIN || gain < ADMFM2000_MIN_GAIN) > + return -EINVAL; > + > + switch (mask) { > + case IIO_CHAN_INFO_HARDWAREGAIN: > + mutex_lock(&st->lock); > + st->gain[chan->channel] = ~((abs(gain) / 1000) & 0x1F); > + > + ret = admfm2000_attenuation(indio_dev, chan->channel, > + st->gain[chan->channel]); > + mutex_unlock(&st->lock); > + break; return ret; > + default: > + ret = -EINVAL; return -EINVAL; > + } > + > + return ret; Not needed with direct returns above. Returning early reduces the code a reviewer needs to consider for a given flow, which is nice to do! > +} > + > +static int admfm2000_channel_config(struct admfm2000_state *st, > + struct iio_dev *indio_dev) > +{ > + struct platform_device *pdev = to_platform_device(indio_dev->dev.parent); > + struct device *dev = &pdev->dev; > + struct fwnode_handle *child; > + u32 reg, mode; > + int ret, i; > + > + device_for_each_child_node(dev, child) { > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) { > + fwnode_handle_put(child); Once a proposed auto cleanup solution for fwnode_handle_put() lands we an tidy this up a lot as then we can get rid of all these manual reference drops. > + return dev_err_probe(dev, ret, > + "Failed to get reg property\n"); > + } > + > + if (reg >= indio_dev->num_channels) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, -EINVAL, "reg bigger than: %d\n", > + indio_dev->num_channels); > + } > + > + ret = fwnode_property_read_u32(child, "adi,mode", &mode); > + if (ret) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, ret, > + "Failed to get mode property\n"); > + } > + > + if (mode >= 2) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, -EINVAL, "mode bigger than: 1\n"); > + } > + > + switch (reg) { > + case 0: Use a couple of local variables to avoid the code duplication. struct gpio_desc *sw; struct gpio_desc *dsa; switch (ret) { case 0: sw = st->sw1_ch; dsa = st->dsa1_gpios; break; case 1: sw = st->sw2_ch; dsa = st->dsa2_gpios; /* or maybe make these arrays of pointers */ break; default: fwnode_handle_put() return; } for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { sw[i] = devm_fdnode... etc > + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { > + st->sw1_ch[i] = devm_fwnode_gpiod_get_index(dev, child, > + "switch", i, > + GPIOD_OUT_LOW, > + NULL); > + if (IS_ERR(st->sw1_ch[i])) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, PTR_ERR(st->sw1_ch[i]), > + "Failed to get gpios\n"); > + } > + } > + > + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) { > + st->dsa1_gpios[i] = devm_fwnode_gpiod_get_index(dev, child, > + "attenuation", i, > + GPIOD_OUT_LOW, > + NULL); > + if (IS_ERR(st->dsa1_gpios[i])) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, PTR_ERR(st->dsa1_gpios[i]), > + "Failed to get gpios\n"); > + } > + } > + break; > + case 1: > + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { > + st->sw2_ch[i] = devm_fwnode_gpiod_get_index(dev, child, > + "switch", i, > + GPIOD_OUT_LOW, > + NULL); > + if (IS_ERR(st->sw2_ch[i])) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, PTR_ERR(st->sw2_ch[i]), > + "Failed to get gpios\n"); > + } > + } > + > + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) { > + st->dsa2_gpios[i] = devm_fwnode_gpiod_get_index(dev, child, > + "attenuation", i, > + GPIOD_OUT_LOW, > + NULL); > + if (IS_ERR(st->dsa2_gpios[i])) { > + fwnode_handle_put(child); > + return dev_err_probe(dev, PTR_ERR(st->dsa2_gpios[i]), > + "Failed to get gpios\n"); > + } > + } > + break; > + default: > + return -EINVAL; > + } > + > + ret = admfm2000_mode(indio_dev, reg, mode); > + if (ret) { > + fwnode_handle_put(child); > + return ret; > + } > + } > + > + return 0; > +}
diff --git a/MAINTAINERS b/MAINTAINERS index 3a86f9d6cb98..49d320535105 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1266,6 +1266,7 @@ L: linux-iio@vger.kernel.org S: Supported W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml +F: drivers/iio/frequency/admfm2000.c ANALOG DEVICES INC ADMV1013 DRIVER M: Antoniu Miclaus <antoniu.miclaus@analog.com> diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig index 9e85dfa58508..c455be7d4a1c 100644 --- a/drivers/iio/frequency/Kconfig +++ b/drivers/iio/frequency/Kconfig @@ -60,6 +60,16 @@ config ADF4377 To compile this driver as a module, choose M here: the module will be called adf4377. +config ADMFM2000 + tristate "Analog Devices ADMFM2000 Dual Microwave Down Converter" + depends on GPIOLIB + help + Say yes here to build support for Analog Devices ADMFM2000 Dual + Microwave Down Converter. + + To compile this driver as a module, choose M here: the + module will be called admfm2000. + config ADMV1013 tristate "Analog Devices ADMV1013 Microwave Upconverter" depends on SPI && COMMON_CLK diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile index b616c29b4a08..70d0e0b70e80 100644 --- a/drivers/iio/frequency/Makefile +++ b/drivers/iio/frequency/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_AD9523) += ad9523.o obj-$(CONFIG_ADF4350) += adf4350.o obj-$(CONFIG_ADF4371) += adf4371.o obj-$(CONFIG_ADF4377) += adf4377.o +obj-$(CONFIG_ADMFM2000) += admfm2000.o obj-$(CONFIG_ADMV1013) += admv1013.o obj-$(CONFIG_ADMV1014) += admv1014.o obj-$(CONFIG_ADMV4420) += admv4420.o diff --git a/drivers/iio/frequency/admfm2000.c b/drivers/iio/frequency/admfm2000.c new file mode 100644 index 000000000000..a09ec38fad22 --- /dev/null +++ b/drivers/iio/frequency/admfm2000.c @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ADMFM2000 Dual Microwave Down Converter + * + * Copyright 2023 Analog Devices Inc. + */ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/iio/iio.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> + +#define ADMFM2000_MIXER_MODE 0 +#define ADMFM2000_DIRECT_IF_MODE 1 +#define ADMFM2000_DSA_GPIOS 5 +#define ADMFM2000_MODE_GPIOS 2 +#define ADMFM2000_MAX_GAIN 0 +#define ADMFM2000_MIN_GAIN -31000 +#define ADMFM2000_DEFAULT_GAIN -0x20 + +struct admfm2000_state { + struct mutex lock; /* protect sensor state */ + struct gpio_desc *sw1_ch[2]; + struct gpio_desc *sw2_ch[2]; + struct gpio_desc *dsa1_gpios[5]; + struct gpio_desc *dsa2_gpios[5]; + u32 gain[2]; +}; + +static int admfm2000_mode(struct iio_dev *indio_dev, u32 chan, u32 mode) +{ + struct admfm2000_state *st = iio_priv(indio_dev); + int i; + + switch (mode) { + case ADMFM2000_MIXER_MODE: + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { + gpiod_set_value_cansleep(st->sw1_ch[i], (chan == 0) ? 1 : 0); + gpiod_set_value_cansleep(st->sw2_ch[i], (chan == 0) ? 0 : 1); + } + return 0; + case ADMFM2000_DIRECT_IF_MODE: + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { + gpiod_set_value_cansleep(st->sw1_ch[i], (chan == 0) ? 0 : 1); + gpiod_set_value_cansleep(st->sw2_ch[i], (chan == 0) ? 1 : 0); + } + return 0; + default: + return -EINVAL; + } +} + +static int admfm2000_attenuation(struct iio_dev *indio_dev, u32 chan, u32 value) +{ + struct admfm2000_state *st = iio_priv(indio_dev); + int i; + + switch (chan) { + case 0: + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) + gpiod_set_value_cansleep(st->dsa1_gpios[i], value & (1 << i)); + return 0; + case 1: + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) + gpiod_set_value_cansleep(st->dsa2_gpios[i], value & (1 << i)); + return 0; + default: + return -EINVAL; + } +} + +static int admfm2000_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct admfm2000_state *st = iio_priv(indio_dev); + int gain; + + switch (mask) { + case IIO_CHAN_INFO_HARDWAREGAIN: + mutex_lock(&st->lock); + gain = ~(st->gain[chan->channel]) * -1000; + *val = gain / 1000; + *val2 = (gain % 1000) * 1000; + mutex_unlock(&st->lock); + + return IIO_VAL_INT_PLUS_MICRO_DB; + default: + return -EINVAL; + } +} + +static int admfm2000_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int val, + int val2, long mask) +{ + struct admfm2000_state *st = iio_priv(indio_dev); + int gain, ret; + + if (val < 0) + gain = (val * 1000) - (val2 / 1000); + else + gain = (val * 1000) + (val2 / 1000); + + if (gain > ADMFM2000_MAX_GAIN || gain < ADMFM2000_MIN_GAIN) + return -EINVAL; + + switch (mask) { + case IIO_CHAN_INFO_HARDWAREGAIN: + mutex_lock(&st->lock); + st->gain[chan->channel] = ~((abs(gain) / 1000) & 0x1F); + + ret = admfm2000_attenuation(indio_dev, chan->channel, + st->gain[chan->channel]); + mutex_unlock(&st->lock); + break; + default: + ret = -EINVAL; + } + + return ret; +} + +static int admfm2000_write_raw_get_fmt(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_HARDWAREGAIN: + return IIO_VAL_INT_PLUS_MICRO_DB; + default: + return -EINVAL; + } +} + +static const struct iio_info admfm2000_info = { + .read_raw = &admfm2000_read_raw, + .write_raw = &admfm2000_write_raw, + .write_raw_get_fmt = &admfm2000_write_raw_get_fmt, +}; + +#define ADMFM2000_CHAN(_channel) { \ + .type = IIO_VOLTAGE, \ + .output = 1, \ + .indexed = 1, \ + .channel = _channel, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_HARDWAREGAIN), \ +} + +static const struct iio_chan_spec admfm2000_channels[] = { + ADMFM2000_CHAN(0), + ADMFM2000_CHAN(1), +}; + +static int admfm2000_channel_config(struct admfm2000_state *st, + struct iio_dev *indio_dev) +{ + struct platform_device *pdev = to_platform_device(indio_dev->dev.parent); + struct device *dev = &pdev->dev; + struct fwnode_handle *child; + u32 reg, mode; + int ret, i; + + device_for_each_child_node(dev, child) { + ret = fwnode_property_read_u32(child, "reg", ®); + if (ret) { + fwnode_handle_put(child); + return dev_err_probe(dev, ret, + "Failed to get reg property\n"); + } + + if (reg >= indio_dev->num_channels) { + fwnode_handle_put(child); + return dev_err_probe(dev, -EINVAL, "reg bigger than: %d\n", + indio_dev->num_channels); + } + + ret = fwnode_property_read_u32(child, "adi,mode", &mode); + if (ret) { + fwnode_handle_put(child); + return dev_err_probe(dev, ret, + "Failed to get mode property\n"); + } + + if (mode >= 2) { + fwnode_handle_put(child); + return dev_err_probe(dev, -EINVAL, "mode bigger than: 1\n"); + } + + switch (reg) { + case 0: + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { + st->sw1_ch[i] = devm_fwnode_gpiod_get_index(dev, child, + "switch", i, + GPIOD_OUT_LOW, + NULL); + if (IS_ERR(st->sw1_ch[i])) { + fwnode_handle_put(child); + return dev_err_probe(dev, PTR_ERR(st->sw1_ch[i]), + "Failed to get gpios\n"); + } + } + + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) { + st->dsa1_gpios[i] = devm_fwnode_gpiod_get_index(dev, child, + "attenuation", i, + GPIOD_OUT_LOW, + NULL); + if (IS_ERR(st->dsa1_gpios[i])) { + fwnode_handle_put(child); + return dev_err_probe(dev, PTR_ERR(st->dsa1_gpios[i]), + "Failed to get gpios\n"); + } + } + break; + case 1: + for (i = 0; i < ADMFM2000_MODE_GPIOS; i++) { + st->sw2_ch[i] = devm_fwnode_gpiod_get_index(dev, child, + "switch", i, + GPIOD_OUT_LOW, + NULL); + if (IS_ERR(st->sw2_ch[i])) { + fwnode_handle_put(child); + return dev_err_probe(dev, PTR_ERR(st->sw2_ch[i]), + "Failed to get gpios\n"); + } + } + + for (i = 0; i < ADMFM2000_DSA_GPIOS; i++) { + st->dsa2_gpios[i] = devm_fwnode_gpiod_get_index(dev, child, + "attenuation", i, + GPIOD_OUT_LOW, + NULL); + if (IS_ERR(st->dsa2_gpios[i])) { + fwnode_handle_put(child); + return dev_err_probe(dev, PTR_ERR(st->dsa2_gpios[i]), + "Failed to get gpios\n"); + } + } + break; + default: + return -EINVAL; + } + + ret = admfm2000_mode(indio_dev, reg, mode); + if (ret) { + fwnode_handle_put(child); + return ret; + } + } + + return 0; +} + +static int admfm2000_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct admfm2000_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); + + indio_dev->name = "admfm2000"; + indio_dev->num_channels = ARRAY_SIZE(admfm2000_channels); + indio_dev->channels = admfm2000_channels; + indio_dev->info = &admfm2000_info; + indio_dev->modes = INDIO_DIRECT_MODE; + + st->gain[0] = ADMFM2000_DEFAULT_GAIN; + st->gain[1] = ADMFM2000_DEFAULT_GAIN; + + mutex_init(&st->lock); + + ret = admfm2000_channel_config(st, indio_dev); + if (ret) + return ret; + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct of_device_id admfm2000_of_match[] = { + { .compatible = "adi,admfm2000" }, + { } +}; +MODULE_DEVICE_TABLE(of, admfm2000_of_match); + +static struct platform_driver admfm2000_driver = { + .driver = { + .name = "admfm2000", + .of_match_table = admfm2000_of_match, + }, + .probe = admfm2000_probe, +}; +module_platform_driver(admfm2000_driver); + +MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>"); +MODULE_DESCRIPTION("ADMFM2000 Dual Microwave Down Converter"); +MODULE_LICENSE("GPL");
Dual microwave down converter module with input RF and LO frequency ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down conversion path. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> --- V5 -> V6: Used devm_fwnode_gpiod_get_index for getting array of gpios. V4 -> V5: Added missing return -ENODEV in setup function. Reordered variable declarations in probe function. V1 -> V4: No changes. MAINTAINERS | 1 + drivers/iio/frequency/Kconfig | 10 + drivers/iio/frequency/Makefile | 1 + drivers/iio/frequency/admfm2000.c | 307 ++++++++++++++++++++++++++++++ 4 files changed, 319 insertions(+) create mode 100644 drivers/iio/frequency/admfm2000.c