On Sat, 4 Mar 2023 17:15:42 +0000 Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Wed, 1 Mar 2023 12:55:11 +0100 > Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > > > Currently, the driver handles CH_FUNC_CURRENT_INPUT_LOOP_POWER and > > CH_FUNC_CURRENT_INPUT_EXT_POWER completely identically. But that's not > > correct. In order for CH_FUNC_CURRENT_INPUT_LOOP_POWER to work, two > > changes must be made: > > > > (1) expose access to the DAC_CODE_x register so that the intended > > output current can be set, i.e. expose the channel as both current > > output and current input, and > > > > (2) per the data sheet > > > > When selecting the current input loop powered function, tie the > > VIOUTN_x pin to ground via the on-chip 200 kΩ resistor by enabling > > the CH_200K_TO_GND bit in the ADC_CONFIGx registers. > > > > Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> > > --- > > There's of course also CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART which is > > likely to require a similar fix, but as I don't have a ad74413r I > > can't test that. > > Looks correct to me based on the "current input, loop powered" section > of the datasheet, but want to leave a little more time for someone > familiar with the part to comment. > > Given me a poke if I seem to have lost this in 2 weeks. (That happens > less now I track with patchwork, but you never know ;) > > Thanks, > > Jonathan Applied, Thanks Jonathan > > > > > drivers/iio/addac/ad74413r.c | 21 +++++++++++++++++++-- > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c > > index f32c8c2fb26d..f5d072092709 100644 > > --- a/drivers/iio/addac/ad74413r.c > > +++ b/drivers/iio/addac/ad74413r.c > > @@ -99,6 +99,7 @@ struct ad74413r_state { > > #define AD74413R_REG_ADC_CONFIG_X(x) (0x05 + (x)) > > #define AD74413R_ADC_CONFIG_RANGE_MASK GENMASK(7, 5) > > #define AD74413R_ADC_CONFIG_REJECTION_MASK GENMASK(4, 3) > > +#define AD74413R_ADC_CONFIG_CH_200K_TO_GND BIT(2) > > #define AD74413R_ADC_RANGE_10V 0b000 > > #define AD74413R_ADC_RANGE_2P5V_EXT_POW 0b001 > > #define AD74413R_ADC_RANGE_2P5V_INT_POW 0b010 > > @@ -424,9 +425,20 @@ static int ad74413r_set_channel_dac_code(struct ad74413r_state *st, > > static int ad74413r_set_channel_function(struct ad74413r_state *st, > > unsigned int channel, u8 func) > > { > > - return regmap_update_bits(st->regmap, > > + int ret; > > + > > + ret = regmap_update_bits(st->regmap, > > AD74413R_REG_CH_FUNC_SETUP_X(channel), > > AD74413R_CH_FUNC_SETUP_MASK, func); > > + if (ret) > > + return ret; > > + > > + if (func == CH_FUNC_CURRENT_INPUT_LOOP_POWER) > > + ret = regmap_set_bits(st->regmap, > > + AD74413R_REG_ADC_CONFIG_X(channel), > > + AD74413R_ADC_CONFIG_CH_200K_TO_GND); > > + > > + return ret; > > } > > > > static int ad74413r_set_adc_conv_seq(struct ad74413r_state *st, > > @@ -1112,6 +1124,11 @@ static struct iio_chan_spec ad74413r_current_input_channels[] = { > > AD74413R_ADC_CURRENT_CHANNEL, > > }; > > > > +static struct iio_chan_spec ad74413r_current_input_loop_channels[] = { > > + AD74413R_DAC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE)), > > + AD74413R_ADC_CURRENT_CHANNEL, > > +}; > > + > > static struct iio_chan_spec ad74413r_resistance_input_channels[] = { > > AD74413R_ADC_CHANNEL(IIO_RESISTANCE, BIT(IIO_CHAN_INFO_PROCESSED)), > > }; > > @@ -1135,7 +1152,7 @@ static const struct ad74413r_channels ad74413r_channels_map[] = { > > [CH_FUNC_CURRENT_OUTPUT] = AD74413R_CHANNELS(current_output), > > [CH_FUNC_VOLTAGE_INPUT] = AD74413R_CHANNELS(voltage_input), > > [CH_FUNC_CURRENT_INPUT_EXT_POWER] = AD74413R_CHANNELS(current_input), > > - [CH_FUNC_CURRENT_INPUT_LOOP_POWER] = AD74413R_CHANNELS(current_input), > > + [CH_FUNC_CURRENT_INPUT_LOOP_POWER] = AD74413R_CHANNELS(current_input_loop), > > [CH_FUNC_RESISTANCE_INPUT] = AD74413R_CHANNELS(resistance_input), > > [CH_FUNC_DIGITAL_INPUT_LOGIC] = AD74413R_CHANNELS(digital_input), > > [CH_FUNC_DIGITAL_INPUT_LOOP_POWER] = AD74413R_CHANNELS(digital_input), >