On Sun, 8 Oct 2023 02:48:38 +0300 Ivan Mikhaylov <fr0st61te@xxxxxxxxx> wrote: > The MAX34408/MAX34409 are two- and four-channel current monitors that are > configured and monitored with a standard I2C/SMBus serial interface. Each > unidirectional current sensor offers precision high-side operation with a > low full-scale sense voltage. The devices automatically sequence through > two or four channels and collect the current-sense samples and average them > to reduce the effect of impulse noise. The raw ADC samples are compared to > user-programmable digital thresholds to indicate overcurrent conditions. > Overcurrent conditions trigger a hardware output to provide an immediate > indication to shut down any necessary external circuitry. > > Add as ADC driver which only supports current monitoring for now. > > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf > Signed-off-by: Ivan Mikhaylov <fr0st61te@xxxxxxxxx> A few other comments inline. Jonathan > diff --git a/drivers/iio/adc/max34408.c b/drivers/iio/adc/max34408.c > new file mode 100644 > index 000000000000..85cd7b1ec186 > --- /dev/null > +++ b/drivers/iio/adc/max34408.c > @@ -0,0 +1,278 @@ > +static const struct of_device_id max34408_of_match[] = { > + { > + .compatible = "maxim,max34408", > + .data = &max34408_model_data, > + }, > + { > + .compatible = "maxim,max34409", > + .data = &max34409_model_data, > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, max34408_of_match); Having dropped the unnecessary check in probe, move this to next to the other tables. > + > +static int max34408_probe(struct i2c_client *client) > +{ > + const struct max34408_adc_model_data *model_data; > + struct device *dev = &client->dev; > + const struct of_device_id *match; > + struct max34408_data *max34408; > + struct fwnode_handle *node; > + struct iio_dev *indio_dev; > + struct regmap *regmap; > + int rc, i; > + > + match = i2c_of_match_device(max34408_of_match, client); Why check this? This prevents any other firmware binding being used for no obvious purpose. Just check... > + if (!match) > + return -EINVAL; > + model_data = i2c_get_match_data(client); .. if (!model_data) return -EINVAL; instead. > + > + regmap = devm_regmap_init_i2c(client, &max34408_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err_probe(dev, PTR_ERR(regmap), > + "regmap_init failed\n"); > + return PTR_ERR(regmap); > + } > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*max34408)); > + if (!indio_dev) > + return -ENOMEM; > + > + max34408 = iio_priv(indio_dev); > + max34408->regmap = regmap; > + max34408->dev = dev; > + mutex_init(&max34408->lock); > + > + device_for_each_child_node(dev, node) { > + fwnode_property_read_u32(node, "maxim,rsense-val-micro-ohms", > + &max34408->input_rsense[i]); > + i++; As 0-day pointed out, i isn't initialized. > + } > + > + /* disable ALERT and averaging */ > + rc = regmap_write(max34408->regmap, MAX34408_CONTROL_REG, 0x0); > + if (rc) > + return rc; > + > + indio_dev->channels = model_data->channels; > + indio_dev->num_channels = model_data->num_channels; > + indio_dev->name = model_data->model_name; > + > + indio_dev->info = &max34408_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + return devm_iio_device_register(dev, indio_dev); > +}
