| Message ID | 20240514-ad4111-v2-5-29be6a55efb5@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for AD411x | expand |
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote: > > From: Dumitru Ceclan <dumitru.ceclan@analog.com> > > Add support for selecting REF+ and REF- inputs on all models. > Add support for selecting ((AVDD1 − AVSS)/5) inputs > on supported models. > > Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com> > --- > drivers/iio/adc/ad7173.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c > index fb33534d63a9..1e9ba3070770 100644 > --- a/drivers/iio/adc/ad7173.c > +++ b/drivers/iio/adc/ad7173.c > @@ -65,6 +65,10 @@ > FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg)) > #define AD7173_AIN_TEMP_POS 17 > #define AD7173_AIN_TEMP_NEG 18 > +#define AD7173_AIN_COM_IN_POS 19 > +#define AD7173_AIN_COM_IN_NEG 20 > +#define AD7173_AIN_REF_POS 21 > +#define AD7173_AIN_REF_NEG 22 > > #define AD7172_2_ID 0x00d0 > #define AD7175_ID 0x0cd0 > @@ -145,6 +149,8 @@ struct ad7173_device_info { > unsigned int id; > char *name; > bool has_temp; > + /* ((AVDD1 − AVSS)/5) */ > + bool has_common_input; > bool has_input_buf; > bool has_int_ref; > bool has_ref2; > @@ -215,6 +221,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_temp = true, > .has_input_buf = true, > .has_int_ref = true, > + .has_common_input = true, > .clock = 2 * HZ_PER_MHZ, > .sinc5_data_rates = ad7173_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), > @@ -228,6 +235,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_temp = false, > .has_input_buf = true, > .has_ref2 = true, > + .has_common_input = true, > .clock = 2 * HZ_PER_MHZ, > .sinc5_data_rates = ad7173_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), > @@ -243,6 +251,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_input_buf = true, > .has_int_ref = true, > .has_ref2 = true, > + .has_common_input = false, > .clock = 2 * HZ_PER_MHZ, > .sinc5_data_rates = ad7173_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), > @@ -257,6 +266,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_temp = true, > .has_input_buf = true, > .has_int_ref = true, > + .has_common_input = true, > .clock = 16 * HZ_PER_MHZ, > .sinc5_data_rates = ad7175_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), > @@ -271,6 +281,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_input_buf = true, > .has_int_ref = true, > .has_ref2 = true, > + .has_common_input = true, > .clock = 16 * HZ_PER_MHZ, > .sinc5_data_rates = ad7175_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), > @@ -285,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_temp = false, > .has_input_buf = false, > .has_int_ref = true, > + .has_common_input = false, > .clock = 16 * HZ_PER_MHZ, > .sinc5_data_rates = ad7175_sinc5_data_rates, > .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), > @@ -298,6 +310,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { > .has_temp = true, > .has_input_buf = true, > .has_int_ref = true, > + .has_common_input = true, > .clock = 16 * HZ_PER_MHZ, > .odr_start_value = AD7177_ODR_START_VALUE, > .sinc5_data_rates = ad7175_sinc5_data_rates, > @@ -920,6 +933,14 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st, > if (ain[i] < st->info->num_inputs) > continue; > > + if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG) > + continue; > + > + if ((ain[i] == AD7173_AIN_COM_IN_POS || > + ain[i] == AD7173_AIN_COM_IN_NEG) && > + st->info->has_common_input) > + continue; > + If there is only one valid combination, it seems like these should be fixed channels like the temperature input rather than something coming from the device tree. It looks like on AD411x, it is the case that there is only one valid option for the reference input in the channel configuration. But in the case of AD717x since both REF+ and REF- are listed as possible inputs for both AINPOS0 and AINNEG0, it seems like they could be mixed and matched with other channels. The datasheet doesn't seem very clear on this though. If it is valid to combine, say AIN0 with REF+ though, then the validation would need to be relaxed. But I'm guessing that is not actually the case? > return dev_err_probe(dev, -EINVAL, > "Input pin number out of range for pair (%d %d).\n", > ain[0], ain[1]); > > -- > 2.43.0 > >
On 16/05/2024 02:27, David Lechner wrote: > On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote: >> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com> >> >> Add support for selecting REF+ and REF- inputs on all models. >> Add support for selecting ((AVDD1 − AVSS)/5) inputs >> on supported models. >> >> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com> >> --- >> drivers/iio/adc/ad7173.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c >> index fb33534d63a9..1e9ba3070770 100644 >> --- a/drivers/iio/adc/ad7173.c >> +++ b/drivers/iio/adc/ad7173.c >> @@ -65,6 +65,10 @@ >> FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg)) >> #define AD7173_AIN_TEMP_POS 17 >> #define AD7173_AIN_TEMP_NEG 18 >> +#define AD7173_AIN_COM_IN_POS 19 >> +#define AD7173_AIN_COM_IN_NEG 20 >> +#define AD7173_AIN_REF_POS 21 >> +#define AD7173_AIN_REF_NEG 22 >> >> #define AD7172_2_ID 0x00d0 >> #define AD7175_ID 0x0cd0 >> @@ -145,6 +149,8 @@ struct ad7173_device_info { >> unsigned int id; >> char *name; >> bool has_temp; >> + /* ((AVDD1 − AVSS)/5) */ >> + bool has_common_input; >> bool has_input_buf; >> bool has_int_ref; >> bool has_ref2; >> @@ -215,6 +221,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_temp = true, >> .has_input_buf = true, >> .has_int_ref = true, >> + .has_common_input = true, >> .clock = 2 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7173_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), >> @@ -228,6 +235,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_temp = false, >> .has_input_buf = true, >> .has_ref2 = true, >> + .has_common_input = true, >> .clock = 2 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7173_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), >> @@ -243,6 +251,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_input_buf = true, >> .has_int_ref = true, >> .has_ref2 = true, >> + .has_common_input = false, >> .clock = 2 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7173_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), >> @@ -257,6 +266,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_temp = true, >> .has_input_buf = true, >> .has_int_ref = true, >> + .has_common_input = true, >> .clock = 16 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7175_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), >> @@ -271,6 +281,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_input_buf = true, >> .has_int_ref = true, >> .has_ref2 = true, >> + .has_common_input = true, >> .clock = 16 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7175_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), >> @@ -285,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_temp = false, >> .has_input_buf = false, >> .has_int_ref = true, >> + .has_common_input = false, >> .clock = 16 * HZ_PER_MHZ, >> .sinc5_data_rates = ad7175_sinc5_data_rates, >> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), >> @@ -298,6 +310,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { >> .has_temp = true, >> .has_input_buf = true, >> .has_int_ref = true, >> + .has_common_input = true, >> .clock = 16 * HZ_PER_MHZ, >> .odr_start_value = AD7177_ODR_START_VALUE, >> .sinc5_data_rates = ad7175_sinc5_data_rates, >> @@ -920,6 +933,14 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st, >> if (ain[i] < st->info->num_inputs) >> continue; >> >> + if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG) >> + continue; >> + >> + if ((ain[i] == AD7173_AIN_COM_IN_POS || >> + ain[i] == AD7173_AIN_COM_IN_NEG) && >> + st->info->has_common_input) >> + continue; >> + > > If there is only one valid combination, it seems like these should be > fixed channels like the temperature input rather than something coming > from the device tree. > As I've said, I do not agree with forcing one channel slot to be used. I could add a property that spawns this channel. Although as I see under, I think I'll permit these inputs to be mixed and matched. > It looks like on AD411x, it is the case that there is only one valid > option for the reference input in the channel configuration. But in > the case of AD717x since both REF+ and REF- are listed as possible > inputs for both AINPOS0 and AINNEG0, it seems like they could be mixed > and matched with other channels. The datasheet doesn't seem very clear > on this though. > This is imposed artificially, AD411x has the same cross-point mux that can mix and match all the inputs. > If it is valid to combine, say AIN0 with REF+ though, then the > validation would need to be relaxed. But I'm guessing that is not > actually the case? > I think it is the case. >> return dev_err_probe(dev, -EINVAL, >> "Input pin number out of range for pair (%d %d).\n", >> ain[0], ain[1]); >> >> -- >> 2.43.0 >> >>
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c index fb33534d63a9..1e9ba3070770 100644 --- a/drivers/iio/adc/ad7173.c +++ b/drivers/iio/adc/ad7173.c @@ -65,6 +65,10 @@ FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg)) #define AD7173_AIN_TEMP_POS 17 #define AD7173_AIN_TEMP_NEG 18 +#define AD7173_AIN_COM_IN_POS 19 +#define AD7173_AIN_COM_IN_NEG 20 +#define AD7173_AIN_REF_POS 21 +#define AD7173_AIN_REF_NEG 22 #define AD7172_2_ID 0x00d0 #define AD7175_ID 0x0cd0 @@ -145,6 +149,8 @@ struct ad7173_device_info { unsigned int id; char *name; bool has_temp; + /* ((AVDD1 − AVSS)/5) */ + bool has_common_input; bool has_input_buf; bool has_int_ref; bool has_ref2; @@ -215,6 +221,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_temp = true, .has_input_buf = true, .has_int_ref = true, + .has_common_input = true, .clock = 2 * HZ_PER_MHZ, .sinc5_data_rates = ad7173_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), @@ -228,6 +235,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_temp = false, .has_input_buf = true, .has_ref2 = true, + .has_common_input = true, .clock = 2 * HZ_PER_MHZ, .sinc5_data_rates = ad7173_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), @@ -243,6 +251,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_input_buf = true, .has_int_ref = true, .has_ref2 = true, + .has_common_input = false, .clock = 2 * HZ_PER_MHZ, .sinc5_data_rates = ad7173_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates), @@ -257,6 +266,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_temp = true, .has_input_buf = true, .has_int_ref = true, + .has_common_input = true, .clock = 16 * HZ_PER_MHZ, .sinc5_data_rates = ad7175_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), @@ -271,6 +281,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_input_buf = true, .has_int_ref = true, .has_ref2 = true, + .has_common_input = true, .clock = 16 * HZ_PER_MHZ, .sinc5_data_rates = ad7175_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), @@ -285,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_temp = false, .has_input_buf = false, .has_int_ref = true, + .has_common_input = false, .clock = 16 * HZ_PER_MHZ, .sinc5_data_rates = ad7175_sinc5_data_rates, .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates), @@ -298,6 +310,7 @@ static const struct ad7173_device_info ad7173_device_info[] = { .has_temp = true, .has_input_buf = true, .has_int_ref = true, + .has_common_input = true, .clock = 16 * HZ_PER_MHZ, .odr_start_value = AD7177_ODR_START_VALUE, .sinc5_data_rates = ad7175_sinc5_data_rates, @@ -920,6 +933,14 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st, if (ain[i] < st->info->num_inputs) continue; + if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG) + continue; + + if ((ain[i] == AD7173_AIN_COM_IN_POS || + ain[i] == AD7173_AIN_COM_IN_NEG) && + st->info->has_common_input) + continue; + return dev_err_probe(dev, -EINVAL, "Input pin number out of range for pair (%d %d).\n", ain[0], ain[1]);