Hi, See comments inline On Tue, Oct 04, 2022 at 10:18:25AM +0300, Ciprian Regus wrote: > The AD5724/AD5734/AD5754 are quad, 12-/14-/16-bit, serial > input, voltage output DACs. The devices operate from single- > supply voltages from +4.5 V up to +16.5 V or dual-supply > voltages from ±4.5 V up to ±16.5 V. The input coding is > user-selectable twos complement or offset binary for a bipolar > output (depending on the state of Pin BIN/2sComp), and straight > binary for a unipolar output. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5724_5734_5754.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5722_5732_5752.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5724r_5734r_5754r.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722R_5732R_5752R.pdf > Signed-off-by: Ciprian Regus <ciprian.regus@xxxxxxxxxx> > --- [...] > +struct ad5754_span_tbl { > + int min; > + int max; > +}; > + > +const struct ad5754_span_tbl ad5754_range[] = { > + {0, 5000000}, > + {0, 10000000}, > + {0, 10800000}, > + {-5000000, 5000000}, > + {-10000000, 10000000}, > + {-10800000, 10800000}, > +}; Should these be static? > + > +enum AD5754_TYPE { > + AD5722, > + AD5732, > + AD5752, > + AD5724, > + AD5734, > + AD5754, > + AD5722R, > + AD5732R, > + AD5752R, > + AD5724R, > + AD5734R, > + AD5754R, > +}; > + > +struct ad5754_chip_info { > + const char *name; > + u32 resolution; > + bool internal_vref; > + const u32 data_mask; > + const struct iio_chan_spec *channels; > + u32 num_channels; > +}; > + > +const struct iio_chan_spec ad5754_channels[][AD5754_MAX_CHANNELS] = { static? > + [AD5754_2_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + }, > + [AD5754_4_CHANNELS] = { > + AD5754_CHANNEL(0), > + AD5754_CHANNEL(1), > + AD5754_CHANNEL(2), > + AD5754_CHANNEL(3), > + }, > +}; > + > +const struct ad5754_chip_info ad5754_chip_info_data[] = { static? [...] > + > + > +static int ad5754_probe(struct spi_device *spi) > +{ > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad5754_state *st; > + struct device *dev; > + int ret; > + > + dev = &spi->dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + spi_set_drvdata(spi, indio_dev); > + > + st->spi = spi; > + st->dev = dev; > + st->chip_info = device_get_match_data(dev); > + if (!st->chip_info) > + st->chip_info = > + (const struct ad5754_chip_info *)spi_get_device_id(spi)->driver_data; > + > + st->regmap = devm_regmap_init(st->dev, NULL, st, &ad5754_regmap_config); > + if (IS_ERR(st->regmap)) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Regmap init error\n"); Are you sure you want to pass vref_reg here? :-) st->regmap > + > + st->dac_max_code = BIT(st->chip_info->resolution) - 1; > + st->sub_lsb = AD5754_MAX_RESOLUTION - st->chip_info->resolution; > + > + vref_reg = devm_regulator_get_optional(st->dev, "vref"); > + if (IS_ERR(vref_reg)) { > + if (!st->chip_info->internal_vref) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Failed to get the vref regulator\n"); > + > + st->vref = AD5754_INT_VREF; > + ret = ad5754_int_vref_enable(st); > + if (ret) > + return ret; > + } else { > + ret = regulator_enable(vref_reg); > + if (ret) > + return dev_err_probe(st->dev, PTR_ERR(vref_reg), > + "Failed to enable the vref regulator\n"); ret contains the error code here, not vref_reg. > + > + ret = devm_add_action_or_reset(dev, ad5754_disable_regulator, vref_reg); > + if (ret) > + return ret; > + > + ret = regulator_get_voltage(vref_reg); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to get vref\n"); > + > + st->vref = ret / 1000; > + } > + > + indio_dev->name = st->chip_info->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &ad5754_info; > + indio_dev->channels = st->chip_info->channels; > + indio_dev->num_channels = st->chip_info->num_channels; > + > + ret = ad5754_enable_channels(st); > + if (ret) > + return ret; > + > + return devm_iio_device_register(st->dev, indio_dev); > +} > + [...] > +module_driver(ad5754_driver, > + ad5754_register_driver, > + ad5754_unregister_driver); Use module_spi_driver() instead > + > +MODULE_AUTHOR("Ciprian Regus <ciprian.regus@xxxxxxxxxx>"); > +MODULE_DESCRIPTION("Analog Devices AD5754 DAC"); > +MODULE_LICENSE("GPL"); > -- > 2.30.2 > Best regards Marcus Folkesson
Attachment:
signature.asc
Description: PGP signature
