On Wed, Feb 01, 2023 at 01:35:16PM +0300, Okan Sahin wrote:
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature.
...
> +struct max77541_adc_iio {
> + struct regmap *regmap;
> +};
Is it really needed? See below.
...
> +enum max77541_adc_channel {
> + MAX77541_ADC_VSYS_V = 0,
0 is the default in C enums, why do you need this explicitly? If these are
HW related, you need to assign them all to be crystal clear.
> + MAX77541_ADC_VOUT1_V,
> + MAX77541_ADC_VOUT2_V,
> + MAX77541_ADC_TEMP,
> +};
...
> + reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
> +
Redundant blank line.
> + switch (reg_val) {
> + case LOW_RANGE:
> + *val = 6;
> + *val2 = 250000;
> + break;
> + case MID_RANGE:
> + *val = 12;
> + *val2 = 500000;
> + break;
> + case HIGH_RANGE:
> + *val = 25;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
...
struct regmap **regmap;
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
sizeof(*regmap)
> + if (!indio_dev)
> + return -ENOMEM;
> + info = iio_priv(indio_dev);
regmap = iio_priv(...);
> + info->regmap = dev_get_regmap(dev->parent, NULL);
*regmap = ...;
It won't probably gain anything in the code generation or memory footprint,
so up to you. But in general depending on API it might be a benefit.
...
> +static const struct platform_device_id max77541_adc_platform_id[] = {
> + { "max77541-adc", },
Inner comma is not needed.
> + { /* sentinel */ }
> +};
--
With Best Regards,
Andy Shevchenko
