From: George Stark <[email protected]>
Changelog:
v1->v2:
split refactoring patch [1] into 4 smaller patches, fix comment style
[1] https://lore.kernel.org/lkml/[email protected]/
v2->v3:
remove patch 'meson saradc: unite iio channel array definitions' [1] after discussion
patch 'meson saradc: add enum for iio channel array indexes'
- change enum items prefix from INDEX_ to NUM_ since name 'channel index' is
more relevant to channel array index in iio world and with 2 tables our array index is
not always equal to channel number
- resolve conflicts after deleting [1]
- update commit message, previous patch [2]
- return channel number for temp channel. It wasn't used and isn't used currently
but may need later
patch meson saradc: support reading from channel 7 mux inputs
- resolve conflicts after deleting [1]
- update commit message, previous patch [3]
- add routine find_channel_by_num to get channel by channel number
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/
George Stark (5):
meson saradc: move enums declaration before variables declaration
meson saradc: move meson_sar_adc_set_chan7_mux routine upper
meson saradc: add enum for iio channel numbers
meson saradc: add channel labels
meson saradc: support reading from channel 7 mux inputs
drivers/iio/adc/meson_saradc.c | 165 +++++++++++++++++++++++++--------
1 file changed, 126 insertions(+), 39 deletions(-)
--
2.38.4
Add attribute 'label' to all iio channels.
Signed-off-by: George Stark <[email protected]>
---
drivers/iio/adc/meson_saradc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index b87f05dfb322..85970fe852af 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -1058,8 +1058,20 @@ static int meson_sar_adc_calib(struct iio_dev *indio_dev)
return ret;
}
+static int read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ char *label)
+{
+ 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);
+ return 0;
+}
+
static const struct iio_info meson_sar_adc_iio_info = {
.read_raw = meson_sar_adc_iio_info_read_raw,
+ .read_label = read_label,
};
static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
--
2.38.4
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 <[email protected]>
---
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,
+ 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;
}
--
2.38.4
Move meson_sar_adc_set_chan7_mux routine upper.
Signed-off-by: George Stark <[email protected]>
---
drivers/iio/adc/meson_saradc.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index af38d95bd504..6e69b40b3309 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -338,6 +338,19 @@ static int meson_sar_adc_wait_busy_clear(struct iio_dev *indio_dev)
1, 10000);
}
+static void meson_sar_adc_set_chan7_mux(struct iio_dev *indio_dev,
+ enum meson_sar_adc_chan7_mux_sel sel)
+{
+ struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+ u32 regval;
+
+ regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
+ regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
+ MESON_SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
+
+ usleep_range(10, 20);
+}
+
static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
int *val)
@@ -434,19 +447,6 @@ static void meson_sar_adc_enable_channel(struct iio_dev *indio_dev,
}
}
-static void meson_sar_adc_set_chan7_mux(struct iio_dev *indio_dev,
- enum meson_sar_adc_chan7_mux_sel sel)
-{
- struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
- u32 regval;
-
- regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
- regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
- MESON_SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
-
- usleep_range(10, 20);
-}
-
static void meson_sar_adc_start_sample_engine(struct iio_dev *indio_dev)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
--
2.38.4
From: George Stark <[email protected]>
Add enum for iio channel numbers.
Signed-off-by: George Stark <[email protected]>
---
drivers/iio/adc/meson_saradc.c | 51 +++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 6e69b40b3309..803d7ca2e0fa 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -224,29 +224,42 @@ enum meson_sar_adc_chan7_mux_sel {
CHAN7_MUX_CH7_INPUT = 0x7,
};
+enum meson_sar_adc_channel_index {
+ NUM_CHAN_0,
+ NUM_CHAN_1,
+ NUM_CHAN_2,
+ NUM_CHAN_3,
+ NUM_CHAN_4,
+ NUM_CHAN_5,
+ NUM_CHAN_6,
+ NUM_CHAN_7,
+ NUM_CHAN_TEMP,
+ NUM_CHAN_SOFT_TIMESTAMP,
+};
+
static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
- MESON_SAR_ADC_CHAN(0),
- MESON_SAR_ADC_CHAN(1),
- MESON_SAR_ADC_CHAN(2),
- MESON_SAR_ADC_CHAN(3),
- MESON_SAR_ADC_CHAN(4),
- MESON_SAR_ADC_CHAN(5),
- MESON_SAR_ADC_CHAN(6),
- MESON_SAR_ADC_CHAN(7),
- IIO_CHAN_SOFT_TIMESTAMP(8),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_0),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_1),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_2),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_3),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_4),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_5),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_6),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_7),
+ IIO_CHAN_SOFT_TIMESTAMP(NUM_CHAN_SOFT_TIMESTAMP),
};
static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
- MESON_SAR_ADC_CHAN(0),
- MESON_SAR_ADC_CHAN(1),
- MESON_SAR_ADC_CHAN(2),
- MESON_SAR_ADC_CHAN(3),
- MESON_SAR_ADC_CHAN(4),
- MESON_SAR_ADC_CHAN(5),
- MESON_SAR_ADC_CHAN(6),
- MESON_SAR_ADC_CHAN(7),
- MESON_SAR_ADC_TEMP_CHAN(8),
- IIO_CHAN_SOFT_TIMESTAMP(9),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_0),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_1),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_2),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_3),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_4),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_5),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_6),
+ MESON_SAR_ADC_CHAN(NUM_CHAN_7),
+ MESON_SAR_ADC_TEMP_CHAN(NUM_CHAN_TEMP),
+ IIO_CHAN_SOFT_TIMESTAMP(NUM_CHAN_SOFT_TIMESTAMP),
};
struct meson_sar_adc_param {
--
2.38.4
Move enums declaration before variables declaration.
Signed-off-by: George Stark <[email protected]>
---
drivers/iio/adc/meson_saradc.c | 44 +++++++++++++++++-----------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 18937a262af6..af38d95bd504 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -202,6 +202,28 @@
.datasheet_name = "TEMP_SENSOR", \
}
+enum meson_sar_adc_avg_mode {
+ NO_AVERAGING = 0x0,
+ MEAN_AVERAGING = 0x1,
+ MEDIAN_AVERAGING = 0x2,
+};
+
+enum meson_sar_adc_num_samples {
+ ONE_SAMPLE = 0x0,
+ TWO_SAMPLES = 0x1,
+ FOUR_SAMPLES = 0x2,
+ EIGHT_SAMPLES = 0x3,
+};
+
+enum meson_sar_adc_chan7_mux_sel {
+ CHAN7_MUX_VSS = 0x0,
+ CHAN7_MUX_VDD_DIV4 = 0x1,
+ CHAN7_MUX_VDD_DIV2 = 0x2,
+ CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
+ CHAN7_MUX_VDD = 0x4,
+ CHAN7_MUX_CH7_INPUT = 0x7,
+};
+
static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
MESON_SAR_ADC_CHAN(0),
MESON_SAR_ADC_CHAN(1),
@@ -227,28 +249,6 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(9),
};
-enum meson_sar_adc_avg_mode {
- NO_AVERAGING = 0x0,
- MEAN_AVERAGING = 0x1,
- MEDIAN_AVERAGING = 0x2,
-};
-
-enum meson_sar_adc_num_samples {
- ONE_SAMPLE = 0x0,
- TWO_SAMPLES = 0x1,
- FOUR_SAMPLES = 0x2,
- EIGHT_SAMPLES = 0x3,
-};
-
-enum meson_sar_adc_chan7_mux_sel {
- CHAN7_MUX_VSS = 0x0,
- CHAN7_MUX_VDD_DIV4 = 0x1,
- CHAN7_MUX_VDD_DIV2 = 0x2,
- CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
- CHAN7_MUX_VDD = 0x4,
- CHAN7_MUX_CH7_INPUT = 0x7,
-};
-
struct meson_sar_adc_param {
bool has_bl30_integration;
unsigned long clock_rate;
--
2.38.4
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);
--
With Best Regards,
Andy Shevchenko
On Wed, 28 Jun 2023 01:37:13 +0300
George Stark <[email protected]> wrote:
> From: George Stark <[email protected]>
>
> Changelog:
>
> v1->v2:
> split refactoring patch [1] into 4 smaller patches, fix comment style
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> v2->v3:
> remove patch 'meson saradc: unite iio channel array definitions' [1] after discussion
>
> patch 'meson saradc: add enum for iio channel array indexes'
> - change enum items prefix from INDEX_ to NUM_ since name 'channel index' is
> more relevant to channel array index in iio world and with 2 tables our array index is
> not always equal to channel number
> - resolve conflicts after deleting [1]
> - update commit message, previous patch [2]
> - return channel number for temp channel. It wasn't used and isn't used currently
> but may need later
>
> patch meson saradc: support reading from channel 7 mux inputs
> - resolve conflicts after deleting [1]
> - update commit message, previous patch [3]
> - add routine find_channel_by_num to get channel by channel number
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/
> [3] https://lore.kernel.org/lkml/[email protected]/
>
> George Stark (5):
> meson saradc: move enums declaration before variables declaration
> meson saradc: move meson_sar_adc_set_chan7_mux routine upper
> meson saradc: add enum for iio channel numbers
> meson saradc: add channel labels
> meson saradc: support reading from channel 7 mux inputs
iio: adc: meson: ....
to make it clear these are changing the IIO driver.
Jonathan
`
>
> drivers/iio/adc/meson_saradc.c | 165 +++++++++++++++++++++++++--------
> 1 file changed, 126 insertions(+), 39 deletions(-)
>
On Wed, 28 Jun 2023 01:37:17 +0300
George Stark <[email protected]> wrote:
> Add attribute 'label' to all iio channels.
Why? Reasoning is more useful here than a simple statement of 'what'.
>
> Signed-off-by: George Stark <[email protected]>
> ---
> drivers/iio/adc/meson_saradc.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index b87f05dfb322..85970fe852af 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -1058,8 +1058,20 @@ static int meson_sar_adc_calib(struct iio_dev *indio_dev)
> return ret;
> }
>
> +static int read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + char *label)
> +{
> + 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);
> + return 0;
> +}
> +
> static const struct iio_info meson_sar_adc_iio_info = {
> .read_raw = meson_sar_adc_iio_info_read_raw,
> + .read_label = read_label,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
On Wed, 28 Jun 2023 01:37:14 +0300
George Stark <[email protected]> wrote:
> Move enums declaration before variables declaration.
>
This is fairly harmless, but would be nice to say 'why'.
Is this just for consistency with the rest of the driver or will
it be required after changes later in the patch set?
> Signed-off-by: George Stark <[email protected]>
> ---
> drivers/iio/adc/meson_saradc.c | 44 +++++++++++++++++-----------------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 18937a262af6..af38d95bd504 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -202,6 +202,28 @@
> .datasheet_name = "TEMP_SENSOR", \
> }
>
> +enum meson_sar_adc_avg_mode {
> + NO_AVERAGING = 0x0,
> + MEAN_AVERAGING = 0x1,
> + MEDIAN_AVERAGING = 0x2,
> +};
> +
> +enum meson_sar_adc_num_samples {
> + ONE_SAMPLE = 0x0,
> + TWO_SAMPLES = 0x1,
> + FOUR_SAMPLES = 0x2,
> + EIGHT_SAMPLES = 0x3,
> +};
> +
> +enum meson_sar_adc_chan7_mux_sel {
> + CHAN7_MUX_VSS = 0x0,
> + CHAN7_MUX_VDD_DIV4 = 0x1,
> + CHAN7_MUX_VDD_DIV2 = 0x2,
> + CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
> + CHAN7_MUX_VDD = 0x4,
> + CHAN7_MUX_CH7_INPUT = 0x7,
> +};
> +
> static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
> MESON_SAR_ADC_CHAN(0),
> MESON_SAR_ADC_CHAN(1),
> @@ -227,28 +249,6 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(9),
> };
>
> -enum meson_sar_adc_avg_mode {
> - NO_AVERAGING = 0x0,
> - MEAN_AVERAGING = 0x1,
> - MEDIAN_AVERAGING = 0x2,
> -};
> -
> -enum meson_sar_adc_num_samples {
> - ONE_SAMPLE = 0x0,
> - TWO_SAMPLES = 0x1,
> - FOUR_SAMPLES = 0x2,
> - EIGHT_SAMPLES = 0x3,
> -};
> -
> -enum meson_sar_adc_chan7_mux_sel {
> - CHAN7_MUX_VSS = 0x0,
> - CHAN7_MUX_VDD_DIV4 = 0x1,
> - CHAN7_MUX_VDD_DIV2 = 0x2,
> - CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
> - CHAN7_MUX_VDD = 0x4,
> - CHAN7_MUX_CH7_INPUT = 0x7,
> -};
> -
> struct meson_sar_adc_param {
> bool has_bl30_integration;
> unsigned long clock_rate;
On Wed, 28 Jun 2023 01:37:18 +0300
George Stark <[email protected]> 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 <[email protected]>
> ---
> 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
<[email protected]> 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 Jonathan
On 7/2/23 12:14, Jonathan Cameron wrote:
> On Wed, 28 Jun 2023 01:37:14 +0300
> George Stark <[email protected]> wrote:
>
>> Move enums declaration before variables declaration.
>>
> This is fairly harmless, but would be nice to say 'why'.
> Is this just for consistency with the rest of the driver or will
> it be required after changes later in the patch set?
Both refactoring patches 1/5 and 2/5 are required for the latter changes
in this patch-set.
Ack for extending the commit message.
>
>> Signed-off-by: George Stark <[email protected]>
>> ---
>> drivers/iio/adc/meson_saradc.c | 44 +++++++++++++++++-----------------
>> 1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>> index 18937a262af6..af38d95bd504 100644
>> --- a/drivers/iio/adc/meson_saradc.c
>> +++ b/drivers/iio/adc/meson_saradc.c
>> @@ -202,6 +202,28 @@
>> .datasheet_name = "TEMP_SENSOR", \
>> }
>>
>> +enum meson_sar_adc_avg_mode {
>> + NO_AVERAGING = 0x0,
>> + MEAN_AVERAGING = 0x1,
>> + MEDIAN_AVERAGING = 0x2,
>> +};
>> +
>> +enum meson_sar_adc_num_samples {
>> + ONE_SAMPLE = 0x0,
>> + TWO_SAMPLES = 0x1,
>> + FOUR_SAMPLES = 0x2,
>> + EIGHT_SAMPLES = 0x3,
>> +};
>> +
>> +enum meson_sar_adc_chan7_mux_sel {
>> + CHAN7_MUX_VSS = 0x0,
>> + CHAN7_MUX_VDD_DIV4 = 0x1,
>> + CHAN7_MUX_VDD_DIV2 = 0x2,
>> + CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
>> + CHAN7_MUX_VDD = 0x4,
>> + CHAN7_MUX_CH7_INPUT = 0x7,
>> +};
>> +
>> static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
>> MESON_SAR_ADC_CHAN(0),
>> MESON_SAR_ADC_CHAN(1),
>> @@ -227,28 +249,6 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
>> IIO_CHAN_SOFT_TIMESTAMP(9),
>> };
>>
>> -enum meson_sar_adc_avg_mode {
>> - NO_AVERAGING = 0x0,
>> - MEAN_AVERAGING = 0x1,
>> - MEDIAN_AVERAGING = 0x2,
>> -};
>> -
>> -enum meson_sar_adc_num_samples {
>> - ONE_SAMPLE = 0x0,
>> - TWO_SAMPLES = 0x1,
>> - FOUR_SAMPLES = 0x2,
>> - EIGHT_SAMPLES = 0x3,
>> -};
>> -
>> -enum meson_sar_adc_chan7_mux_sel {
>> - CHAN7_MUX_VSS = 0x0,
>> - CHAN7_MUX_VDD_DIV4 = 0x1,
>> - CHAN7_MUX_VDD_DIV2 = 0x2,
>> - CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
>> - CHAN7_MUX_VDD = 0x4,
>> - CHAN7_MUX_CH7_INPUT = 0x7,
>> -};
>> -
>> struct meson_sar_adc_param {
>> bool has_bl30_integration;
>> unsigned long clock_rate;
>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
--
Best regards
George
On 7/2/23 12:16, Jonathan Cameron wrote:
> On Wed, 28 Jun 2023 01:37:17 +0300
> George Stark <[email protected]> wrote:
>
>> Add attribute 'label' to all iio channels.
> Why? Reasoning is more useful here than a simple statement of 'what'.
Adding labels make sense only for newly-added channels,
base channels' default node names are close enough to datasheet names.
Ack for extending the commit message.
>> Signed-off-by: George Stark <[email protected]>
>> ---
>> drivers/iio/adc/meson_saradc.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>> index b87f05dfb322..85970fe852af 100644
>> --- a/drivers/iio/adc/meson_saradc.c
>> +++ b/drivers/iio/adc/meson_saradc.c
>> @@ -1058,8 +1058,20 @@ static int meson_sar_adc_calib(struct iio_dev *indio_dev)
>> return ret;
>> }
>>
>> +static int read_label(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + char *label)
>> +{
>> + 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);
>> + return 0;
>> +}
>> +
>> static const struct iio_info meson_sar_adc_iio_info = {
>> .read_raw = meson_sar_adc_iio_info_read_raw,
>> + .read_label = read_label,
>> };
>>
>> static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
--
Best regards
George
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
> <[email protected]> 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
--
Best regards
George
On Tue, 4 Jul 2023 04:59:51 +0300
George Stark <[email protected]> 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
> > <[email protected]> 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
>