Re: [PATCH v2 06/16] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 31.01.2018 19:42, Quentin Schulz wrote:
Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:09AM +0100, Philipp Rossak wrote:
For adding newer sensor some basic rework of the code is necessary.

This patch reworks the driver to be able to handle more than one
thermal sensor. Newer SoC like the A80 have 4 thermal sensors.
Because of this the maximal sensor count value was set to 4.

The sensor_id value is set during sensor registration and is for each
registered sensor indiviual. This makes it able to differntiate the
sensors when the value is read from the register.

In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor
was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects
in the temp_read function automatically sensor 0. A check for the
sensor_id is here not required since the old sensors only have one
thermal sensor. In addition to that is the sun4i_gpadc_read_raw()
function only used by the "older" sensors (before A33) where the
thermal sensor was a cobination of an adc and a thermal sensor.

Signed-off-by: Philipp Rossak <embed3d@xxxxxxxxx>
---
  drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++++++++++++++++++++++-------------
  include/linux/mfd/sun4i-gpadc.h   |  3 +++
  2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 51ec0104d678..ac9ad2f8232f 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -67,12 +67,13 @@ struct gpadc_data {
  	unsigned int	tp_adc_select;
  	unsigned int	(*adc_chan_select)(unsigned int chan);
  	unsigned int	adc_chan_mask;
-	unsigned int	temp_data;
+	unsigned int	temp_data[MAX_SENSOR_COUNT];
  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
  	bool		has_bus_clk;
  	bool		has_bus_rst;
  	bool		has_mod_clk;
+	int		sensor_count;
  };

I've noticed that for H3, A83T, A64 (at least), if DATA reg of sensor 0
is e.g. 0x80, DATA reg of sensor N is at 0x80 + 0x04 * N.

Is that verified for other SoCs? Does anyone have some input on this?

We could then just use temp_data as the DATA reg "base" and increment by
0x4 depending on the sensor id instead of using a fixed-size array.


This sounds like a good idea! I will add this to the next version.

I can verify this with a table, I created during development. I will upload it during the weekend here: [1]


  static const struct gpadc_data sun4i_gpadc_data = {
@@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = {
  	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
  	.adc_chan_select = &sun4i_gpadc_chan_select,
  	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
-	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0},
  	.sample_start = sun4i_gpadc_sample_start,
  	.sample_end = sun4i_gpadc_sample_end,
+	.sensor_count = 1,

If the solution above is not desirable/possible, could we use something
like:

unsigned int sun4i_temp_data[] = {SUN4I_GPADC_TEMP_DATA,};

static const struct gpadc_data sun4i_gpadc_data = {
	.temp_data = &sun4i_temp_data,
	.sensor_count = ARRAY_SIZE(sun4i_temp_data),
};

That avoids 1) inconsistencies between the array size and the array
itself, 2) does not require to pad the array with zeroes.

[...]

@@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
  	pm_runtime_enable(&pdev->dev);
if (IS_ENABLED(CONFIG_THERMAL_OF)) {
-		info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
-							    0, info,
-							    &sun4i_ts_tz_ops);
+		for (i = 0; i < info->data->sensor_count; i++) {
+			info->sensor_id = i;
+			info->tzd = thermal_zone_of_sensor_register(
+					info->sensor_device,
+					i, info, &sun4i_ts_tz_ops);
+		}

As Maxime said, this does not work.

One way would be to have a new structure being:
struct sun4i_sensor_info {
	struct sun4i_gpadc_iio	*info;
	unsigned int		sensor_id;
};

Or since we only use the iio_dev within the sun4i_gpadc_iio in the
.get_temp function, we may replace info by struct iio_dev *indio_dev
above.

Quentin

I will have a closer look on this next week, when I start to work on the next version..

Thanks,
Philipp

[1]: http://linux-sunxi.org/Thermal_Sensor
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux