| Message ID | 20230627224017.1724097-6-gnstark@sberdevices.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | meson saradc: add iio channels to read channel 7 mux inputs | expand |
On Wed, Jun 28, 2023 at 01:37:18AM +0300, George Stark wrote: > Add iio channel for every channel 7 mux input. > Meson saradc channel 7 is connected to a mux that can switch channel > input to well-known sources like Vdd, GND and several Vdd dividers. ... > +static enum meson_sar_adc_chan7_mux_sel chan7_mux_values[] = { > + CHAN7_MUX_VSS, > + CHAN7_MUX_VDD_DIV4, > + CHAN7_MUX_VDD_DIV2, > + CHAN7_MUX_VDD_MUL3_DIV4, > + CHAN7_MUX_VDD, > +}; > + > +static const char * const chan7_mux_names[] = { > + "gnd", > + "0.25vdd", > + "0.5vdd", > + "0.75vdd", > + "vdd", For the sake of robustness you can assign like this [CHAN7_MUX_VDD_DIV2] = "0.5vdd", > }; ... > - if (chan->type == IIO_VOLTAGE) > - return sprintf(label, "channel-%d\n", chan->channel); > + if (chan->type == IIO_VOLTAGE) { > + if (chan->channel <= NUM_CHAN_7) I believe you can get rid of this conditional and make diff less ping-pong-ish, see below. > + return sprintf(label, "channel-%d\n", chan->channel); > + if (chan->channel >= NUM_MUX_0_VSS) > + return sprintf(label, "%s\n", > + chan7_mux_names[chan->channel - NUM_MUX_0_VSS]); > + } if (chan->type == IIO_VOLTAGE && chan->channel >= NUM_MUX_0_VSS) return sprintf(label, "%s\n", chan7_mux_names[chan->channel - NUM_MUX_0_VSS]); if (chan->type == IIO_VOLTAGE) return sprintf(label, "channel-%d\n", chan->channel);
On Wed, 28 Jun 2023 01:37:18 +0300 George Stark <gnstark@sberdevices.ru> wrote: > Add iio channel for every channel 7 mux input. > Meson saradc channel 7 is connected to a mux that can switch channel > input to well-known sources like Vdd, GND and several Vdd dividers. > > Signed-off-by: George Stark <GNStark@sberdevices.ru> > --- > drivers/iio/adc/meson_saradc.c | 83 ++++++++++++++++++++++++++++++++-- > 1 file changed, 79 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c > index 4a9dacedb6c3..c99a55944ece 100644 > --- a/drivers/iio/adc/meson_saradc.c > +++ b/drivers/iio/adc/meson_saradc.c > @@ -163,6 +163,7 @@ > #define MESON_SAR_ADC_MAX_FIFO_SIZE 32 > #define MESON_SAR_ADC_TIMEOUT 100 /* ms */ > #define MESON_SAR_ADC_VOLTAGE_AND_TEMP_CHANNEL 6 > +#define MESON_SAR_ADC_VOLTAGE_AND_MUX_CHANNEL 7 > #define MESON_SAR_ADC_TEMP_OFFSET 27 > > /* temperature sensor calibration information in eFuse */ > @@ -202,6 +203,19 @@ > .datasheet_name = "TEMP_SENSOR", \ > } > > +#define MESON_SAR_ADC_MUX(_chan, _sel) { \ > + .type = IIO_VOLTAGE, \ > + .channel = _chan, \ > + .indexed = 1, \ > + .address = MESON_SAR_ADC_VOLTAGE_AND_MUX_CHANNEL, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_AVERAGE_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_CALIBBIAS) | \ > + BIT(IIO_CHAN_INFO_CALIBSCALE), \ > + .datasheet_name = "SAR_ADC_MUX_"#_sel, \ > +} > + > enum meson_sar_adc_avg_mode { > NO_AVERAGING = 0x0, > MEAN_AVERAGING = 0x1, > @@ -235,6 +249,27 @@ enum meson_sar_adc_channel_index { > NUM_CHAN_7, > NUM_CHAN_TEMP, > NUM_CHAN_SOFT_TIMESTAMP, Silly question... Why does this device have timestamp channels? It has no buffer support so they don't 'do anything'. If it had then putting other channels after that might have broken things if not done very carefully (hence I went looking) > + NUM_MUX_0_VSS, > + NUM_MUX_1_VDD_DIV4, > + NUM_MUX_2_VDD_DIV2, > + NUM_MUX_3_VDD_MUL3_DIV4, > + NUM_MUX_4_VDD, > +}; ... > static const struct iio_chan_spec meson_sar_adc_iio_channels[] = { > @@ -247,6 +282,11 @@ static const struct iio_chan_spec meson_sar_adc_iio_channels[] = { > MESON_SAR_ADC_CHAN(NUM_CHAN_6), > MESON_SAR_ADC_CHAN(NUM_CHAN_7), > IIO_CHAN_SOFT_TIMESTAMP(NUM_CHAN_SOFT_TIMESTAMP), > + MESON_SAR_ADC_MUX(NUM_MUX_0_VSS, 0), > + MESON_SAR_ADC_MUX(NUM_MUX_1_VDD_DIV4, 1), > + MESON_SAR_ADC_MUX(NUM_MUX_2_VDD_DIV2, 2), > + MESON_SAR_ADC_MUX(NUM_MUX_3_VDD_MUL3_DIV4, 3), > + MESON_SAR_ADC_MUX(NUM_MUX_4_VDD, 4), > }; Jonathan
Hi Jonathan, On Sun, Jul 2, 2023 at 11:21 AM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: [...] > > @@ -235,6 +249,27 @@ enum meson_sar_adc_channel_index { > > NUM_CHAN_7, > > NUM_CHAN_TEMP, > > NUM_CHAN_SOFT_TIMESTAMP, > > Silly question... Why does this device have timestamp channels? > It has no buffer support so they don't 'do anything'. > If it had then putting other channels after that might have broken > things if not done very carefully (hence I went looking) This question is probably for me (not George). The answer is simple: when I wrote the Meson SAR ADC driver I looked at various other drivers (but can't recall which ones exactly). One of them probably used a soft timestamp channel so I also added that to meson_saradc. Since "it didn't break anything" I thought it would be fine. Newer SAR ADC IP blocks have buffer support, but that's not implemented in the driver (yet). So if I understand you correctly we can drop the soft timestamp channel (with a dedicated patch in this series)? Best regards, Martin
Hello Martin, Jonathan On 7/3/23 22:39, Martin Blumenstingl wrote: > Hi Jonathan, > > On Sun, Jul 2, 2023 at 11:21 AM Jonathan Cameron > <Jonathan.Cameron@huawei.com> wrote: > [...] >>> @@ -235,6 +249,27 @@ enum meson_sar_adc_channel_index { >>> NUM_CHAN_7, >>> NUM_CHAN_TEMP, >>> NUM_CHAN_SOFT_TIMESTAMP, >> >> Silly question... Why does this device have timestamp channels? >> It has no buffer support so they don't 'do anything'. >> If it had then putting other channels after that might have broken >> things if not done very carefully (hence I went looking) > This question is probably for me (not George). > The answer is simple: when I wrote the Meson SAR ADC driver I looked > at various other drivers (but can't recall which ones exactly). One of > them probably used a soft timestamp channel so I also added that to > meson_saradc. Since "it didn't break anything" I thought it would be > fine. > > Newer SAR ADC IP blocks have buffer support, but that's not > implemented in the driver (yet). > So if I understand you correctly we can drop the soft timestamp > channel (with a dedicated patch in this series)? One short comment: newly-added channels probably won't support buffering because physically they all are read thru channel7. We'll be able to add buffering only to base old channels and they are still defined before CHAN_SOFT_TIMESTAMP in channel array (if this is you're wary about). > > > Best regards, > Martin
On Tue, 4 Jul 2023 04:59:51 +0300 George Stark <gnstark@sberdevices.ru> wrote: > Hello Martin, Jonathan > > On 7/3/23 22:39, Martin Blumenstingl wrote: > > Hi Jonathan, > > > > On Sun, Jul 2, 2023 at 11:21 AM Jonathan Cameron > > <Jonathan.Cameron@huawei.com> wrote: > > [...] > >>> @@ -235,6 +249,27 @@ enum meson_sar_adc_channel_index { > >>> NUM_CHAN_7, > >>> NUM_CHAN_TEMP, > >>> NUM_CHAN_SOFT_TIMESTAMP, > >> Silly question... Why does this device have timestamp channels? > >> It has no buffer support so they don't 'do anything'. > >> If it had then putting other channels after that might have broken > >> things if not done very carefully (hence I went looking) > > This question is probably for me (not George). > > The answer is simple: when I wrote the Meson SAR ADC driver I looked > > at various other drivers (but can't recall which ones exactly). One of > > them probably used a soft timestamp channel so I also added that to > > meson_saradc. Since "it didn't break anything" I thought it would be > > fine. > > > > Newer SAR ADC IP blocks have buffer support, but that's not > > implemented in the driver (yet). > > So if I understand you correctly we can drop the soft timestamp > > channel (with a dedicated patch in this series)? yes. I think dropping it would be sensible. > > One short comment: newly-added channels probably won't support buffering > because physically they all are read thru channel7. We'll be able to add > buffering only to base old channels and they are still defined before > CHAN_SOFT_TIMESTAMP (if this is you're wary about). > That is a common situation. If adding buffering support any channels that are not suitable for buffered access are given scan_index = -1 at which point they are sysfs / polled in kernel access only. Jonathan > > Best regards, > > Martin >
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c index 4a9dacedb6c3..c99a55944ece 100644 --- a/drivers/iio/adc/meson_saradc.c +++ b/drivers/iio/adc/meson_saradc.c @@ -163,6 +163,7 @@ #define MESON_SAR_ADC_MAX_FIFO_SIZE 32 #define MESON_SAR_ADC_TIMEOUT 100 /* ms */ #define MESON_SAR_ADC_VOLTAGE_AND_TEMP_CHANNEL 6 +#define MESON_SAR_ADC_VOLTAGE_AND_MUX_CHANNEL 7 #define MESON_SAR_ADC_TEMP_OFFSET 27 /* temperature sensor calibration information in eFuse */ @@ -202,6 +203,19 @@ .datasheet_name = "TEMP_SENSOR", \ } +#define MESON_SAR_ADC_MUX(_chan, _sel) { \ + .type = IIO_VOLTAGE, \ + .channel = _chan, \ + .indexed = 1, \ + .address = MESON_SAR_ADC_VOLTAGE_AND_MUX_CHANNEL, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_AVERAGE_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_CALIBBIAS) | \ + BIT(IIO_CHAN_INFO_CALIBSCALE), \ + .datasheet_name = "SAR_ADC_MUX_"#_sel, \ +} + enum meson_sar_adc_avg_mode { NO_AVERAGING = 0x0, MEAN_AVERAGING = 0x1, @@ -235,6 +249,27 @@ enum meson_sar_adc_channel_index { NUM_CHAN_7, NUM_CHAN_TEMP, NUM_CHAN_SOFT_TIMESTAMP, + NUM_MUX_0_VSS, + NUM_MUX_1_VDD_DIV4, + NUM_MUX_2_VDD_DIV2, + NUM_MUX_3_VDD_MUL3_DIV4, + NUM_MUX_4_VDD, +}; + +static enum meson_sar_adc_chan7_mux_sel chan7_mux_values[] = { + CHAN7_MUX_VSS, + CHAN7_MUX_VDD_DIV4, + CHAN7_MUX_VDD_DIV2, + CHAN7_MUX_VDD_MUL3_DIV4, + CHAN7_MUX_VDD, +}; + +static const char * const chan7_mux_names[] = { + "gnd", + "0.25vdd", + "0.5vdd", + "0.75vdd", + "vdd", }; static const struct iio_chan_spec meson_sar_adc_iio_channels[] = { @@ -247,6 +282,11 @@ static const struct iio_chan_spec meson_sar_adc_iio_channels[] = { MESON_SAR_ADC_CHAN(NUM_CHAN_6), MESON_SAR_ADC_CHAN(NUM_CHAN_7), IIO_CHAN_SOFT_TIMESTAMP(NUM_CHAN_SOFT_TIMESTAMP), + MESON_SAR_ADC_MUX(NUM_MUX_0_VSS, 0), + MESON_SAR_ADC_MUX(NUM_MUX_1_VDD_DIV4, 1), + MESON_SAR_ADC_MUX(NUM_MUX_2_VDD_DIV2, 2), + MESON_SAR_ADC_MUX(NUM_MUX_3_VDD_MUL3_DIV4, 3), + MESON_SAR_ADC_MUX(NUM_MUX_4_VDD, 4), }; static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = { @@ -260,6 +300,11 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = { MESON_SAR_ADC_CHAN(NUM_CHAN_7), MESON_SAR_ADC_TEMP_CHAN(NUM_CHAN_TEMP), IIO_CHAN_SOFT_TIMESTAMP(NUM_CHAN_SOFT_TIMESTAMP), + MESON_SAR_ADC_MUX(NUM_MUX_0_VSS, 0), + MESON_SAR_ADC_MUX(NUM_MUX_1_VDD_DIV4, 1), + MESON_SAR_ADC_MUX(NUM_MUX_2_VDD_DIV2, 2), + MESON_SAR_ADC_MUX(NUM_MUX_3_VDD_MUL3_DIV4, 3), + MESON_SAR_ADC_MUX(NUM_MUX_4_VDD, 4), }; struct meson_sar_adc_param { @@ -298,6 +343,7 @@ struct meson_sar_adc_priv { bool temperature_sensor_calibrated; u8 temperature_sensor_coefficient; u16 temperature_sensor_adc_val; + enum meson_sar_adc_chan7_mux_sel chan7_mux_sel; }; static const struct regmap_config meson_sar_adc_regmap_config_gxbb = { @@ -314,6 +360,17 @@ static const struct regmap_config meson_sar_adc_regmap_config_meson8 = { .max_register = MESON_SAR_ADC_DELTA_10, }; +static const struct iio_chan_spec * +find_channel_by_num(struct iio_dev *indio_dev, int num) +{ + int i; + + for (i = 0; i < indio_dev->num_channels; i++) + if (indio_dev->channels[i].channel == num) + return &indio_dev->channels[i]; + return NULL; +} + static unsigned int meson_sar_adc_get_fifo_count(struct iio_dev *indio_dev) { struct meson_sar_adc_priv *priv = iio_priv(indio_dev); @@ -362,6 +419,8 @@ static void meson_sar_adc_set_chan7_mux(struct iio_dev *indio_dev, MESON_SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval); usleep_range(10, 20); + + priv->chan7_mux_sel = sel; } static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev, @@ -457,6 +516,15 @@ static void meson_sar_adc_enable_channel(struct iio_dev *indio_dev, regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELTA_10, MESON_SAR_ADC_DELTA_10_TEMP_SEL, regval); + } else if (chan->address == MESON_SAR_ADC_VOLTAGE_AND_MUX_CHANNEL) { + enum meson_sar_adc_chan7_mux_sel sel; + + if (chan->channel == NUM_CHAN_7) + sel = CHAN7_MUX_CH7_INPUT; + else + sel = chan7_mux_values[chan->channel - NUM_MUX_0_VSS]; + if (sel != priv->chan7_mux_sel) + meson_sar_adc_set_chan7_mux(indio_dev, sel); } } @@ -1029,7 +1097,8 @@ static int meson_sar_adc_calib(struct iio_dev *indio_dev) meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_VDD_DIV4); usleep_range(10, 20); ret = meson_sar_adc_get_sample(indio_dev, - &indio_dev->channels[7], + find_channel_by_num(indio_dev, + NUM_MUX_1_VDD_DIV4), MEAN_AVERAGING, EIGHT_SAMPLES, &value0); if (ret < 0) goto out; @@ -1037,7 +1106,8 @@ static int meson_sar_adc_calib(struct iio_dev *indio_dev) meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_VDD_MUL3_DIV4); usleep_range(10, 20); ret = meson_sar_adc_get_sample(indio_dev, - &indio_dev->channels[7], + find_channel_by_num(indio_dev, + NUM_MUX_3_VDD_MUL3_DIV4), MEAN_AVERAGING, EIGHT_SAMPLES, &value1); if (ret < 0) goto out; @@ -1064,8 +1134,13 @@ static int read_label(struct iio_dev *indio_dev, { if (chan->type == IIO_TEMP) return sprintf(label, "%s\n", "temp-sensor"); - if (chan->type == IIO_VOLTAGE) - return sprintf(label, "channel-%d\n", chan->channel); + if (chan->type == IIO_VOLTAGE) { + if (chan->channel <= NUM_CHAN_7) + return sprintf(label, "channel-%d\n", chan->channel); + if (chan->channel >= NUM_MUX_0_VSS) + return sprintf(label, "%s\n", + chan7_mux_names[chan->channel - NUM_MUX_0_VSS]); + } return 0; }
Add iio channel for every channel 7 mux input. Meson saradc channel 7 is connected to a mux that can switch channel input to well-known sources like Vdd, GND and several Vdd dividers. Signed-off-by: George Stark <GNStark@sberdevices.ru> --- drivers/iio/adc/meson_saradc.c | 83 ++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-)