Messages in this thread | | | Date | Fri, 24 Mar 2023 15:40:50 +0530 | Subject | Re: [PATCH v2 1/2] iio: max597x: Add support for max597x | From | Naresh Solanki <> |
| |
Hi,
On 24-03-2023 01:36 am, Christophe JAILLET wrote: > Le 23/03/2023 à 20:45, Naresh Solanki a écrit : >> From: Patrick Rudolph >> <patrick.rudolph-cWEv32IpryCakBO8gow8eQ@public.gmane.org> >> >> max597x has 10bit ADC for voltage & current monitoring. >> Use iio framework to expose the same in sysfs. >> >> Signed-off-by: Patrick Rudolph >> <patrick.rudolph-cWEv32IpryCakBO8gow8eQ@public.gmane.org> >> Signed-off-by: Naresh Solanki >> <Naresh.Solanki-cWEv32IpryCakBO8gow8eQ@public.gmane.org> > > Hi, a few nits below, should there be a v3. > > CJ > >> ... >> Changes in V2: >> - Remove fallthrough >> - Use pdev->dev instead of i2c->dev >> - Init indio_dev->name based on device type. >> --- >> drivers/iio/adc/Kconfig | 15 ++++ >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/max597x-iio.c | 152 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 168 insertions(+) >> create mode 100644 drivers/iio/adc/max597x-iio.c >> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index 45af2302be53..0d1a3dea0b7d 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -735,6 +735,21 @@ config MAX1363 >> To compile this driver as a module, choose M here: the module >> will be >> called max1363. >> +config MAX597X_IIO >> + tristate "Maxim 597x power switch and monitor" >> + depends on I2C && OF >> + select MFD_MAX597X >> + help >> + This driver enables support for the Maxim 597x smart switch and >> + voltage/current monitoring interface using the Industrial I/O >> (IIO) >> + framework. The Maxim 597x is a power switch and monitor that can >> + provide voltage and current measurements via the I2C bus. Enabling >> + this driver will allow user space applications to read the voltage >> + and current measurements using IIO interfaces. >> + >> + To compile this driver as a module, choose M here: the module >> will be >> + called max597x-iio. >> + >> config MAX9611 >> tristate "Maxim max9611/max9612 ADC driver" >> depends on I2C >> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile >> index 36c18177322a..7ec0c2cf7bbb 100644 >> --- a/drivers/iio/adc/Makefile >> +++ b/drivers/iio/adc/Makefile >> @@ -67,6 +67,7 @@ obj-$(CONFIG_MAX11205) += max11205.o >> obj-$(CONFIG_MAX11410) += max11410.o >> obj-$(CONFIG_MAX1241) += max1241.o >> obj-$(CONFIG_MAX1363) += max1363.o >> +obj-$(CONFIG_MAX597X_IIO) += max597x-iio.o >> obj-$(CONFIG_MAX9611) += max9611.o >> obj-$(CONFIG_MCP320X) += mcp320x.o >> obj-$(CONFIG_MCP3422) += mcp3422.o >> diff --git a/drivers/iio/adc/max597x-iio.c >> b/drivers/iio/adc/max597x-iio.c >> new file mode 100644 >> index 000000000000..8a9fc27ff71e >> --- /dev/null >> +++ b/drivers/iio/adc/max597x-iio.c >> @@ -0,0 +1,152 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Device driver for IIO in MAX5970 and MAX5978 IC >> + * >> + * Copyright (c) 2022 9elements GmbH >> + * >> + * Author: Patrick Rudolph >> <patrick.rudolph-cWEv32IpryCakBO8gow8eQ@public.gmane.org> >> + */ >> + >> +#include <linux/iio/iio.h> >> +#include <linux/mfd/max597x.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +struct max597x_iio { >> + struct regmap *regmap; >> + int shunt_micro_ohms[MAX5970_NUM_SWITCHES]; >> + unsigned int irng[MAX5970_NUM_SWITCHES]; >> + unsigned int mon_rng[MAX5970_NUM_SWITCHES]; >> +}; >> + >> +#define MAX597X_ADC_CHANNEL(_idx, _type) { \ >> + .type = IIO_ ## _type, \ >> + .indexed = 1, \ >> + .channel = (_idx), \ >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ >> + BIT(IIO_CHAN_INFO_SCALE), \ >> + .address = MAX5970_REG_ ## _type ## _L(_idx), \ >> +} >> + >> +static const struct iio_chan_spec max5978_adc_iio_channels[] = { >> + MAX597X_ADC_CHANNEL(0, VOLTAGE), >> + MAX597X_ADC_CHANNEL(0, CURRENT), >> +}; >> + >> +static const struct iio_chan_spec max5970_adc_iio_channels[] = { >> + MAX597X_ADC_CHANNEL(0, VOLTAGE), >> + MAX597X_ADC_CHANNEL(0, CURRENT), >> + MAX597X_ADC_CHANNEL(1, VOLTAGE), >> + MAX597X_ADC_CHANNEL(1, CURRENT), >> +}; >> + >> +static int max597x_iio_read_raw(struct iio_dev *iio_dev, >> + struct iio_chan_spec const *chan, >> + int *val, int *val2, long info) >> +{ >> + int ret; >> + struct max597x_iio *data = iio_priv(iio_dev); >> + unsigned int reg_l, reg_h; >> + >> + switch (info) { >> + case IIO_CHAN_INFO_RAW: >> + ret = regmap_read(data->regmap, chan->address, ®_l); >> + if (ret < 0) >> + return ret; >> + ret = regmap_read(data->regmap, chan->address - 1, ®_h); >> + if (ret < 0) >> + return ret; >> + *val = (reg_h << 2) | (reg_l & 3); >> + >> + return IIO_VAL_INT; >> + case IIO_CHAN_INFO_SCALE: >> + > > Nit: This blank line would look nicer if above the case: Oh yes. Will do that in V3 > >> + switch (chan->address) { >> + case MAX5970_REG_CURRENT_L(0): >> + case MAX5970_REG_CURRENT_L(1): >> + /* in A, convert to mA */ >> + *val = data->irng[chan->channel] * 1000; >> + *val2 = >> + data->shunt_micro_ohms[chan->channel] * ADC_MASK; >> + return IIO_VAL_FRACTIONAL; >> + >> + case MAX5970_REG_VOLTAGE_L(0): >> + case MAX5970_REG_VOLTAGE_L(1): >> + /* in uV, convert to mV */ >> + *val = data->mon_rng[chan->channel]; >> + *val2 = ADC_MASK * 1000; >> + return IIO_VAL_FRACTIONAL; >> + } >> + >> + break; >> + } >> + return -EINVAL; >> +} >> + >> +static const struct iio_info max597x_adc_iio_info = { >> + .read_raw = &max597x_iio_read_raw, >> +}; >> + >> +static int max597x_iio_probe(struct platform_device *pdev) >> +{ >> + struct max597x_data *max597x = dev_get_drvdata(pdev->dev.parent); >> + struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL); >> + struct iio_dev *indio_dev; >> + struct max597x_iio *priv; >> + int ret, i; >> + >> + if (!regmap) >> + return -EPROBE_DEFER; >> + >> + if (!max597x || !max597x->num_switches) >> + return -EPROBE_DEFER; >> + >> + /* registering iio */ >> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv)); >> + if (!indio_dev) >> + return dev_err_probe(&pdev->dev, -ENOMEM, >> + "failed to allocate iio device\n"); >> + >> + indio_dev->info = &max597x_adc_iio_info; >> + indio_dev->modes = INDIO_DIRECT_MODE; >> + >> + switch (max597x->num_switches) { >> + case MAX597x_TYPE_MAX5970: >> + indio_dev->channels = max5970_adc_iio_channels; >> + indio_dev->num_channels = ARRAY_SIZE(max5970_adc_iio_channels); >> + indio_dev->name = "max5970"; >> + break; >> + case MAX597x_TYPE_MAX5978: >> + indio_dev->channels = max5978_adc_iio_channels; >> + indio_dev->num_channels = ARRAY_SIZE(max5978_adc_iio_channels); >> + indio_dev->name = "max5978"; >> + break; >> + } >> + >> + priv = iio_priv(indio_dev); >> + priv->regmap = regmap; >> + for (i = 0; i < indio_dev->num_channels; i++) { >> + priv->irng[i] = max597x->irng[i]; >> + priv->mon_rng[i] = max597x->mon_rng[i]; >> + priv->shunt_micro_ohms[i] = max597x->shunt_micro_ohms[i]; >> + } >> + >> + ret = devm_iio_device_register(&pdev->dev, indio_dev); >> + if (ret) >> + dev_err_probe(&pdev->dev, ret, "could not register iio device"); > > Nit: \n missing > Sure will make it like this: if (ret) return dev_err_probe(&pdev->dev, ret, "could not register iio device\n"); >> + >> + return ret; > > Nit: return 0; Sure > >> +} >> + >> +static struct platform_driver max597x_iio_driver = { >> + .driver = { >> + .name = "max597x-iio", >> + }, >> + .probe = max597x_iio_probe, >> +}; >> + >> +module_platform_driver(max597x_iio_driver); >> + >> +MODULE_AUTHOR("Patrick Rudolph >> <patrick.rudolph-cWEv32IpryCakBO8gow8eQ@public.gmane.org>"); >> +MODULE_DESCRIPTION("MAX5970_hot-swap controller driver"); >> +MODULE_LICENSE("GPL"); >> >> base-commit: 368eb79f738a21e16c2bdbcac2444dfa96b01aaa >
| |