| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5e861c22-84e0-6f97-e25b-f754257e2f74@9elements.com>
Date: Fri, 24 Mar 2023 15:40:50 +0530
From: Naresh Solanki <naresh.solanki@...ements.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: jic23@...nel.org, lars@...afoo.de, lee@...nel.org,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
patrick.rudolph@...ements.com
Subject: Re: [PATCH v2 1/2] iio: max597x: Add support for max597x
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@...lic.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@...lic.gmane.org>
>> Signed-off-by: Naresh Solanki
>> <Naresh.Solanki-cWEv32IpryCakBO8gow8eQ@...lic.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@...lic.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@...lic.gmane.org>");
>> +MODULE_DESCRIPTION("MAX5970_hot-swap controller driver");
>> +MODULE_LICENSE("GPL");
>>
>> base-commit: 368eb79f738a21e16c2bdbcac2444dfa96b01aaa
>
Powered by blists - more mailing lists