AD5686 driver fixes

drivers/iio/dac/Kconfig

@@ -224,6 +224,8 @@ config LTC2688
 
 config AD5686
 	tristate
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 
 config AD5686_SPI
 	tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"

drivers/iio/dac/ad5686-spi.c

@@ -1,3 +1,3 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * AD5672R, AD5674R, AD5676, AD5676R, AD5679R,
@@ -95,11 +95,6 @@
 			    ad5686_spi_write, ad5686_spi_read, spi->irq);
 }
 
-static void ad5686_spi_remove(struct spi_device *spi)
-{
-	ad5686_remove(&spi->dev);
-}
-
 static const struct spi_device_id ad5686_spi_id[] = {
 	{"ad5310r", ID_AD5310R},
 	{"ad5672r", ID_AD5672R},
@@ -126,7 +121,6 @@
 		.name = "ad5686",
 	},
 	.probe = ad5686_spi_probe,
-	.remove = ad5686_spi_remove,
 	.id_table = ad5686_spi_id,
 };
 

drivers/iio/dac/ad5686.c

@@ -1,3 +1,3 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * AD5686R, AD5685R, AD5684R Digital to analog converters  driver
@@ -197,8 +197,7 @@
 static int ad5686_trig_set_state(struct iio_trigger *trig,
 				 bool state)
 {
-	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
-	struct ad5686_state *st = iio_priv(indio_dev);
+	struct ad5686_state *st = iio_trigger_get_drvdata(trig);
 	struct pwm_state pwm_st;
 
 	if (!st->pwm)
@@ -215,7 +214,7 @@
 {
 	struct ad5686_state *st = iio_priv(indio_dev);
 
-	if (st->trig != trig)
+	if (st->trig && st->trig != trig)
 		return -EINVAL;
 
 	return 0;
@@ -512,22 +511,18 @@
 	const struct iio_chan_spec *chan;
 	struct iio_buffer *buffer = indio_dev->buffer;
 	struct ad5686_state *st = iio_priv(indio_dev);
-	u8 sample[2];
-	unsigned int i;
-	u16 val;
-	int ret;
+	u16 val[AD5686_MAX_CHANNELS];
+	int ret, ch, i = 0;
 
-	ret = iio_pop_from_buffer(buffer, sample);
+	ret = iio_pop_from_buffer(buffer, val);
 	if (ret < 0)
 		goto out;
 
 	mutex_lock(&st->lock);
-	iio_for_each_active_channel(indio_dev, i) {
-		val = (sample[1] << 8) + sample[0];
-
-		chan = &indio_dev->channels[i];
+	iio_for_each_active_channel(indio_dev, ch) {
+		chan = &indio_dev->channels[ch];
 		ret = st->write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
-				chan->address, val << chan->scan_type.shift);
+				chan->address, val[i++] << chan->scan_type.shift);
 	}
 	mutex_unlock(&st->lock);
 
@@ -557,54 +552,42 @@
 	struct iio_dev *indio_dev;
 	struct pwm_state state;
 	unsigned int val, ref_bit_msk;
+	bool has_external_vref;
 	u8 cmd;
-	int ret, i, voltage_uv = 0;
+	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return  -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	dev_set_drvdata(dev, indio_dev);
 
 	st->dev = dev;
 	st->write = write;
 	st->read = read;
+	st->chip_info = &ad5686_chip_info_tbl[chip_type];
 
-	mutex_init(&st->lock);
-
-	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", name,
-					  iio_device_id(indio_dev));
-	if (st->trig == NULL)
-		ret = -ENOMEM;
-
-	st->trig->ops = &ad5686_trigger_ops;
-	st->trig->dev.parent = dev;
-	iio_trigger_set_drvdata(st->trig, indio_dev);
-
-	ret = devm_iio_trigger_register(dev, st->trig);
+	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;
 
-	/* select default trigger */
-	indio_dev->trig = iio_trigger_get(st->trig);
+	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
+	if (ret < 0 && ret != -ENODEV)
+		return ret;
 
-	st->reg = devm_regulator_get_optional(dev, "vcc");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
+	has_external_vref = ret != -ENODEV;
+	st->vref_mv = has_external_vref ? ret / 1000 : st->chip_info->int_vref_mv;
 
-		ret = regulator_get_voltage(st->reg);
-		if (ret < 0)
-			goto error_disable_reg;
-
-		voltage_uv = ret;
+	/* PWM configuration */
+	st->pwm = devm_pwm_get(dev, NULL);
+	if (IS_ERR(st->pwm)) {
+		ret = PTR_ERR(st->pwm);
+		if (ret != -ENOENT && ret != -ENODEV)
+			return dev_err_probe(dev, ret, "failed to get pwm\n");
+		st->pwm = NULL;
 	}
 
-	/* PWM configuration */
-	st->pwm = devm_pwm_get(dev, "pwm-trigger");
-	if (!IS_ERR(st->pwm)) {
+	if (st->pwm) {
 		/* Set a default pwm frequency of 1kHz and 50% duty cycle */
 		pwm_init_state(st->pwm, &state);
 		state.enabled = false;
@@ -613,12 +596,26 @@
 		ret = pwm_apply_might_sleep(st->pwm, &state);
 		if (ret < 0)
 			return ret;
-	} else {
-		st->pwm = NULL;
 	}
 
 	/* Configure IRQ */
 	if (irq) {
+		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", name,
+						  iio_device_id(indio_dev));
+		if (!st->trig)
+			return -ENOMEM;
+
+		st->trig->ops = &ad5686_trigger_ops;
+		st->trig->dev.parent = dev;
+		iio_trigger_set_drvdata(st->trig, st);
+
+		ret = devm_iio_trigger_register(dev, st->trig);
+		if (ret)
+			return ret;
+
+		/* select default trigger */
+		indio_dev->trig = iio_trigger_get(st->trig);
+
 		ret = devm_request_threaded_irq(dev, irq, NULL, ad5686_irq_handler,
 						IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 						"ad5686 irq", indio_dev);
@@ -628,13 +625,6 @@
 		st->irq = irq;
 	}
 
-	st->chip_info = &ad5686_chip_info_tbl[chip_type];
-
-	if (voltage_uv)
-		st->vref_mv = voltage_uv / 1000;
-	else
-		st->vref_mv = st->chip_info->int_vref_mv;
-
 	/* Set all the power down mode for all channels to 1K pulldown */
 	for (i = 0; i < st->chip_info->num_channels; i++)
 		st->pwr_down_mode |= (0x01 << (i * 2));
@@ -645,18 +635,16 @@
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	mutex_init(&st->lock);
-
 	switch (st->chip_info->regmap_type) {
 	case AD5310_REGMAP:
 		cmd = AD5686_CMD_CONTROL_REG;
 		ref_bit_msk = AD5310_REF_BIT_MSK;
-		st->use_internal_vref = !voltage_uv;
+		st->use_internal_vref = !has_external_vref;
 		break;
 	case AD5683_REGMAP:
 		cmd = AD5686_CMD_CONTROL_REG;
 		ref_bit_msk = AD5683_REF_BIT_MSK;
-		st->use_internal_vref = !voltage_uv;
+		st->use_internal_vref = !has_external_vref;
 		break;
 	case AD5686_REGMAP:
 		cmd = AD5686_CMD_INTERNAL_REFER_SETUP;
@@ -665,50 +653,29 @@
 	case AD5693_REGMAP:
 		cmd = AD5686_CMD_CONTROL_REG;
 		ref_bit_msk = AD5693_REF_BIT_MSK;
-		st->use_internal_vref = !voltage_uv;
+		st->use_internal_vref = !has_external_vref;
 		break;
 	default:
-		ret = -EINVAL;
-		goto error_disable_reg;
+		return -EINVAL;
 	}
 
-	val = (voltage_uv | ref_bit_msk);
+	val = (has_external_vref | ref_bit_msk);
 
 	ret = st->write(st, cmd, 0, !!val);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
 	ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
 						  &ad5686_trigger_handler,
 						  IIO_BUFFER_DIRECTION_OUT,
 						  NULL, NULL);
 	if (ret)
-		goto error_disable_reg;
-
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_disable_reg;
-
-	return 0;
+		return ret;
 
-error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
-	return ret;
+	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(ad5686_probe, IIO_AD5686);
 
-void ad5686_remove(struct device *dev)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct ad5686_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
-}
-EXPORT_SYMBOL_NS_GPL(ad5686_remove, IIO_AD5686);
-
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_DESCRIPTION("Analog Devices AD5686/85/84 DAC");
 MODULE_LICENSE("GPL v2");

drivers/iio/dac/ad5686.h

@@ -25,6 +25,7 @@
 
 #define AD5686_ADDR_DAC(chan)			(0x1 << (chan))
 #define AD5686_ADDR_ALL_DAC			0xF
+#define AD5686_MAX_CHANNELS			16
 
 #define AD5686_CMD_NOOP				0x0
 #define AD5686_CMD_WRITE_INPUT_N		0x1
@@ -116,11 +117,10 @@ struct ad5686_chip_info {
 };
 
 /**
- * struct ad5446_state - driver instance specific data
+ * struct ad5686_state - driver instance specific data
  * @spi:		spi_device
  * @pwm:		pwm used for buffer trigger
  * @chip_info:		chip model specific constants, available modes etc
- * @reg:		supply regulator
  * @vref_mv:		actual reference voltage used
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
@@ -134,7 +134,6 @@ struct ad5686_state {
 	struct pwm_device		*pwm;
 	struct iio_trigger		*trig;
 	const struct ad5686_chip_info	*chip_info;
-	struct regulator		*reg;
 	unsigned short			vref_mv;
 	unsigned int			pwr_down_mask;
 	unsigned int			pwr_down_mode;
@@ -162,7 +161,5 @@ int ad5686_probe(struct device *dev,
 		 const char *name, ad5686_write_func write,
 		 ad5686_read_func read, int irq);
 
-void ad5686_remove(struct device *dev);
-
 
 #endif /* __DRIVERS_IIO_DAC_AD5686_H__ */

drivers/iio/dac/ad5696-i2c.c

@@ -1,3 +1,3 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * AD5338R, AD5671R, AD5673R, AD5675R, AD5677R, AD5691R, AD5692R, AD5693,
@@ -65,11 +65,6 @@
 			    ad5686_i2c_write, ad5686_i2c_read, i2c->irq);
 }
 
-static void ad5686_i2c_remove(struct i2c_client *i2c)
-{
-	ad5686_remove(&i2c->dev);
-}
-
 static const struct i2c_device_id ad5686_i2c_id[] = {
 	{"ad5311r", ID_AD5311R},
 	{"ad5337r", ID_AD5337R},
@@ -116,7 +111,6 @@
 		.of_match_table = ad5686_of_match,
 	},
 	.probe = ad5686_i2c_probe,
-	.remove = ad5686_i2c_remove,
 	.id_table = ad5686_i2c_id,
 };