[PATCH v2 00/16] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

[PATCH v2 00/16] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC

[Philipp Rossak]

Allwiner H3 and A83T SoCs have a thermal sensor, which is a large refactored
version of the old Allwinner "GPADC" (although it have already only
thermal part left in A33).

This patch tried to add support for the sensor in H3 and A83T based on
the A33 thermal sensor driver by Quentin Schulz, which is already merged.

This Patchseries is based on Icenowy Zhengs v4 patchseries [1]. 
The first 5 patches are reworked patches from the v4 patchseries. 
The rest of the patches add step by step a feature to support multible
sensors, nvmem calibration and interupts. This patchseries should make it
easy also to add other sunxi SoCs, like the H5, A64 and A80.

Patches that adds support for H5, A64 and A80 SoCs are allready prepared,
and will be upstreamed if this patchseries is applied and the testing is done.

I tried to pick up all the feedback from [1]. I hope I didn't miss anything.

Regards,
Philipp 

@Jonathan
	Could you please check Patch 8 again. I moved some code from 
	Patch 8 to 9. Please chek it again, if your still ok with it.
	But I think it sould be ok.

changes since v1:
	* collecting all acks 
	* rewording commits/fix typos
	* move code in place where it is used
	* fix naming conventions of defines
	* clarify commits
	* update documentation to cover the new nvmem calibraion
	* change nvmem calibration

Icenowy Zheng (1):
  iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain
    A33

Philipp Rossak (15):
  dt-bindings: update the Allwinner GPADC device tree binding for H3 &
    A83T
  arm: config: sunxi_defconfig: enable SUN4I_GPADC
  iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg
  iio: adc: sun4i-gpadc-iio: rework: support clocks and reset
  iio: adc: sun4i-gpadc-iio: rework: support multiple sensors
  iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data
  iio: adc: sun4i-gpadc-iio: rework: add interrupt support
  iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
  iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor
  arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5
  arm: dts: sun8i: h3: add support for the thermal sensor in H3
  arm: dts: sun8i: h3: add thermal zone to H3
  arm: dts: sun8i: h3: enable H3 sid controller
  arm: dts: sun8i: a83t: add support for the thermal sensor in A83T
  arm: dts: sun8i: a83t: add thermal zone to A83T

 .../devicetree/bindings/mfd/sun4i-gpadc.txt        |  50 ++-
 arch/arm/boot/dts/sun8i-a83t.dtsi                  |  28 ++
 arch/arm/boot/dts/sun8i-h3.dtsi                    |  21 ++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi                 |   9 +
 arch/arm/configs/sunxi_defconfig                   |   1 +
 drivers/iio/adc/sun4i-gpadc-iio.c                  | 367 ++++++++++++++++++++-
 include/linux/mfd/sun4i-gpadc.h                    |  51 ++-
 7 files changed, 503 insertions(+), 24 deletions(-)

-- 
2.11.0

[PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[Philipp Rossak]

This patch rewors the driver to support interrupts for the thermal part
of the sensor.

This is only available for the newer sensor (currently H3 and A83T).
The interrupt will be trigerd on data available and triggers the update
for the thermal sensors. All newer sensors have different amount of
sensors and different interrupts for each device the reset of the
interrupts need to be done different

For the newer sensors is the autosuspend disabled.

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
Acked-by: Jonathan  Cameron <jonathan.cameron@huawei.com>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
 include/linux/mfd/sun4i-gpadc.h   |  2 ++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 74eeb5cd5218..b7b5451226b0 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -71,11 +71,14 @@ struct gpadc_data {
 	unsigned int	temp_data[MAX_SENSOR_COUNT];
 	int		(*sample_start)(struct sun4i_gpadc_iio *info);
 	int		(*sample_end)(struct sun4i_gpadc_iio *info);
+	u32		irq_clear_map;
+	u32		irq_control_map;
 	bool		has_bus_clk;
 	bool		has_bus_rst;
 	bool		has_mod_clk;
 	int		sensor_count;
 	bool		supports_nvmem;
+	bool		support_irq;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {
@@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
 	.supports_nvmem = false,
+	.support_irq = false,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {
@@ -104,6 +108,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
 	.supports_nvmem = false,
+	.support_irq = false,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {
@@ -118,6 +123,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
 	.supports_nvmem = false,
+	.support_irq = false,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -129,6 +135,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
 	.supports_nvmem = false,
+	.support_irq = false,
 };
 
 struct sun4i_gpadc_iio {
@@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
 		return 0;
 	}
 
+	if (info->data->support_irq) {
+		regmap_read(info->regmap, info->data->temp_data[sensor], val);
+		return 0;
+	}
+
 	return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
 }
 
@@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t sunxi_irq_thread(int irq, void *data)
+{
+	struct sun4i_gpadc_iio *info = data;
+
+	regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
+
+	thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
+
+	return IRQ_HANDLED;
+}
+
 static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
 {
 	/* Disable the ADC on IP */
@@ -572,12 +595,29 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	struct nvmem_cell *cell;
 	ssize_t cell_size;
 	u64 *cell_data;
+	int irq;
 
 	info->data = of_device_get_match_data(&pdev->dev);
 	if (!info->data)
 		return -ENODEV;
 
-	info->no_irq = true;
+	if (info->data->support_irq) {
+		/* only the new versions of ths support right now irqs */
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+			return irq;
+		}
+
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+				sunxi_irq_thread, IRQF_ONESHOT,
+				dev_name(&pdev->dev), info);
+		if (ret)
+			return ret;
+
+	} else
+		info->no_irq = true;
+
 	indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
 	indio_dev->channels = sun8i_a33_gpadc_channels;
 
@@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev,
-					 SUN4I_GPADC_AUTOSUSPEND_DELAY);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_suspended(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
+	if (!info->data->support_irq) {
+		pm_runtime_set_autosuspend_delay(&pdev->dev,
+						 SUN4I_GPADC_AUTOSUSPEND_DELAY);
+		pm_runtime_use_autosuspend(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
 
 	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
 		for (i = 0; i < info->data->sensor_count; i++) {
@@ -814,6 +856,9 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (info->data->support_irq)
+		info->data->sample_start(info);
+
 	ret = devm_iio_device_register(&pdev->dev, indio_dev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "could not register the device\n");
@@ -843,6 +888,9 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
 	if (!IS_ENABLED(CONFIG_THERMAL_OF))
 		return 0;
 
+	if (info->data->support_irq)
+		info->data->sample_end(info);
+
 	thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
 
 	if (!info->no_irq)
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 20fa02040007..458f2159a95a 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -91,6 +91,8 @@
 #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
 
 /* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_INTC				0x44
+
 #define MAX_SENSOR_COUNT				4
 
 struct sun4i_gpadc_dev {
-- 
2.11.0

Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[Quentin Schulz]

[-- Attachment #1: Type: text/plain, Size: 4360 bytes --]

Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:11AM +0100, Philipp Rossak wrote:
> This patch rewors the driver to support interrupts for the thermal part
> of the sensor.
> 
> This is only available for the newer sensor (currently H3 and A83T).
> The interrupt will be trigerd on data available and triggers the update
> for the thermal sensors. All newer sensors have different amount of
> sensors and different interrupts for each device the reset of the
> interrupts need to be done different
> 
> For the newer sensors is the autosuspend disabled.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Jonathan  Cameron <jonathan.cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
>  include/linux/mfd/sun4i-gpadc.h   |  2 ++
>  2 files changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 74eeb5cd5218..b7b5451226b0 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -71,11 +71,14 @@ struct gpadc_data {
>  	unsigned int	temp_data[MAX_SENSOR_COUNT];
>  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
>  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
> +	u32		irq_clear_map;
> +	u32		irq_control_map;

I would say to use a regmap_irq_chip for controlling IRQs.

>  	bool		has_bus_clk;
>  	bool		has_bus_rst;
>  	bool		has_mod_clk;
>  	int		sensor_count;
>  	bool		supports_nvmem;
> +	bool		support_irq;
>  };
>  
>  static const struct gpadc_data sun4i_gpadc_data = {
> @@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>  	.sample_end = sun4i_gpadc_sample_end,
>  	.sensor_count = 1,
>  	.supports_nvmem = false,
> +	.support_irq = false,

False is the default, no need to set support_irq.

[...]

>  struct sun4i_gpadc_iio {
> @@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
>  		return 0;
>  	}
>  
> +	if (info->data->support_irq) {
> +		regmap_read(info->regmap, info->data->temp_data[sensor], val);
> +		return 0;
> +	}
> +

Maybe you could define a new thermal_zone_of_device_ops for these new
thermal sensors? That way, you don't even need the boolean support_irq.

>  	return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
>  }
>  
> @@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t sunxi_irq_thread(int irq, void *data)

I think we're trying to avoid sunxi mentions but rather using the name
of the first IP (in term of product release, not support) using this
function.

> +{
> +	struct sun4i_gpadc_iio *info = data;
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
> +

Will be handled by regmap_irq_chip.
[...]
> -	info->no_irq = true;
> +	if (info->data->support_irq) {
> +		/* only the new versions of ths support right now irqs */
> +		irq = platform_get_irq(pdev, 0);
> +		if (irq < 0) {
> +			dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
> +			return irq;
> +		}
> +
> +		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> +				sunxi_irq_thread, IRQF_ONESHOT,
> +				dev_name(&pdev->dev), info);
> +		if (ret)
> +			return ret;
> +
> +	} else
> +		info->no_irq = true;
> +

That's a bit funny to have two booleans named no_irq and support_irq :)

>  	indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
>  	indio_dev->channels = sun8i_a33_gpadc_channels;
>  
> @@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	pm_runtime_set_autosuspend_delay(&pdev->dev,
> -					 SUN4I_GPADC_AUTOSUSPEND_DELAY);
> -	pm_runtime_use_autosuspend(&pdev->dev);
> -	pm_runtime_set_suspended(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> +	if (!info->data->support_irq) {
> +		pm_runtime_set_autosuspend_delay(&pdev->dev,
> +						 SUN4I_GPADC_AUTOSUSPEND_DELAY);
> +		pm_runtime_use_autosuspend(&pdev->dev);
> +		pm_runtime_set_suspended(&pdev->dev);
> +		pm_runtime_enable(&pdev->dev);
> +	}

Why would you not want your IP to do runtime PM?

Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[Philipp Rossak]

On 31.01.2018 20:07, Quentin Schulz wrote:
> Hi Philipp,
> 
> On Mon, Jan 29, 2018 at 12:29:11AM +0100, Philipp Rossak wrote:
>> This patch rewors the driver to support interrupts for the thermal part
>> of the sensor.
>>
>> This is only available for the newer sensor (currently H3 and A83T).
>> The interrupt will be trigerd on data available and triggers the update
>> for the thermal sensors. All newer sensors have different amount of
>> sensors and different interrupts for each device the reset of the
>> interrupts need to be done different
>>
>> For the newer sensors is the autosuspend disabled.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Acked-by: Jonathan  Cameron <jonathan.cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   drivers/iio/adc/sun4i-gpadc-iio.c | 60 +++++++++++++++++++++++++++++++++++----
>>   include/linux/mfd/sun4i-gpadc.h   |  2 ++
>>   2 files changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 74eeb5cd5218..b7b5451226b0 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -71,11 +71,14 @@ struct gpadc_data {
>>   	unsigned int	temp_data[MAX_SENSOR_COUNT];
>>   	int		(*sample_start)(struct sun4i_gpadc_iio *info);
>>   	int		(*sample_end)(struct sun4i_gpadc_iio *info);
>> +	u32		irq_clear_map;
>> +	u32		irq_control_map;
> 
> I would say to use a regmap_irq_chip for controlling IRQs.
> 
Sounds good for me! I will rework that in the next version.
>>   	bool		has_bus_clk;
>>   	bool		has_bus_rst;
>>   	bool		has_mod_clk;
>>   	int		sensor_count;
>>   	bool		supports_nvmem;
>> +	bool		support_irq;
>>   };
>>   
>>   static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>>   	.sample_end = sun4i_gpadc_sample_end,
>>   	.sensor_count = 1,
>>   	.supports_nvmem = false,
>> +	.support_irq = false,
> 
> False is the default, no need to set support_irq.
> 
> [...]
> 
>>   struct sun4i_gpadc_iio {
>> @@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
>>   		return 0;
>>   	}
>>   
>> +	if (info->data->support_irq) {
>> +		regmap_read(info->regmap, info->data->temp_data[sensor], val);
>> +		return 0;
>> +	}
>> +
> 
> Maybe you could define a new thermal_zone_of_device_ops for these new
> thermal sensors? That way, you don't even need the boolean support_irq.
> 
Sounds good for me! I will rework that in the next version.

>>   	return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
>>   }
>>   
>> @@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> +static irqreturn_t sunxi_irq_thread(int irq, void *data)
> 
> I think we're trying to avoid sunxi mentions but rather using the name
> of the first IP (in term of product release, not support) using this
> function.
> 
>> +{
>> +	struct sun4i_gpadc_iio *info = data;
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
>> +
> 
> Will be handled by regmap_irq_chip.
> [...]
>> -	info->no_irq = true;
>> +	if (info->data->support_irq) {
>> +		/* only the new versions of ths support right now irqs */
>> +		irq = platform_get_irq(pdev, 0);
>> +		if (irq < 0) {
>> +			dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
>> +			return irq;
>> +		}
>> +
>> +		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
>> +				sunxi_irq_thread, IRQF_ONESHOT,
>> +				dev_name(&pdev->dev), info);
>> +		if (ret)
>> +			return ret;
>> +
>> +	} else
>> +		info->no_irq = true;
>> +
> 
> That's a bit funny to have two booleans named no_irq and support_irq :)
> 
I know this looks very funny. I thought this would be better to keep, to 
_not_ break anything. Since I will rework the whole driver and integrate 
the mfd part I hope I can remove both.

>>   	indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
>>   	indio_dev->channels = sun8i_a33_gpadc_channels;
>>   
>> @@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> -	pm_runtime_set_autosuspend_delay(&pdev->dev,
>> -					 SUN4I_GPADC_AUTOSUSPEND_DELAY);
>> -	pm_runtime_use_autosuspend(&pdev->dev);
>> -	pm_runtime_set_suspended(&pdev->dev);
>> -	pm_runtime_enable(&pdev->dev);
>> +	if (!info->data->support_irq) {
>> +		pm_runtime_set_autosuspend_delay(&pdev->dev,
>> +						 SUN4I_GPADC_AUTOSUSPEND_DELAY);
>> +		pm_runtime_use_autosuspend(&pdev->dev);
>> +		pm_runtime_set_suspended(&pdev->dev);
>> +		pm_runtime_enable(&pdev->dev);
>> +	}
> 
> Why would you not want your IP to do runtime PM?

I will enable this again, in the next version! I had some issues with 
this, thus I disabled this, but I know now what I did wrong!

Thanks,
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

Hi Philipp,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

Note: the linux-review/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 HEAD e571c3ced84a9d7f150cb2d239919d31d25114e2 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/iio/adc/sun4i-gpadc-iio.c: In function 'sunxi_irq_thread':
>> drivers/iio/adc/sun4i-gpadc-iio.c:448:29: error: 'SUN8I_H3_THS_STAT' undeclared (first use in this function); did you mean 'SUN8I_H3_THS_INTC'?
     regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
                                ^~~~~~~~~~~~~~~~~
                                SUN8I_H3_THS_INTC
   drivers/iio/adc/sun4i-gpadc-iio.c:448:29: note: each undeclared identifier is reported only once for each function it appears in

vim +448 drivers/iio/adc/sun4i-gpadc-iio.c

   443	
   444	static irqreturn_t sunxi_irq_thread(int irq, void *data)
   445	{
   446		struct sun4i_gpadc_iio *info = data;
   447	
 > 448		regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map);
   449	
   450		thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE);
   451	
   452		return IRQ_HANDLED;
   453	}
   454	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52622 bytes --]

[PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

[Philipp Rossak]

Allwinner H3 features a thermal sensor like the one in A33, but has its
register re-arranged, the clock divider moved to CCU (originally the
clock divider is in ADC) and added a pair of bus clock and reset.

Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
the bus clock and the reset was removed from the CCU. The THS in A83T
has a clock that is directly connected and runs with 24 MHz.

Update the binding document to cover H3 and A83T.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index 86dd8191b04c..22df0c5c23d4 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
 and sometimes as a touchscreen controller.
 
 Required properties:
-  - compatible: "allwinner,sun8i-a33-ths",
+  - compatible: must contain one of the following compatibles:
+		- "allwinner,sun8i-a33-ths"
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
   - reg: mmio address range of the chip,
-  - #thermal-sensor-cells: shall be 0,
+  - #thermal-sensor-cells: shall be 0 or 1,
   - #io-channel-cells: shall be 0,
 
-Example:
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+		- "allwinner,sun8i-a83t-ths"
+  - interrupts: the sampling interrupt of the ADC,
+
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - clocks: the bus clock and the input clock of the ADC,
+  - clock-names: should be "bus" and "mod",
+  - resets: the bus reset of the ADC,
+
+Optional properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+		If unspecified default values shall be used. The size should
+		be 0x2 * sensorcount.
+  - nvmem-cell-names: Should be "calibration".
+
+Details see: bindings/nvmem/nvmem.txt
+
+Example for A33:
 	ths: ths@1c25000 {
 		compatible = "allwinner,sun8i-a33-ths";
 		reg = <0x01c25000 0x100>;
@@ -17,6 +40,27 @@ Example:
 		#io-channel-cells = <0>;
 	};
 
+Example for H3:
+	ths: thermal-sensor@1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};
+
+Example for A83T:
+	ths: thermal-sensor@1f04000 {
+		compatible = "allwinner,sun8i-a83t-ths";
+		reg = <0x01f04000 0x100>;
+		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <1>;
+		#io-channel-cells = <0>;
+	};
+
 sun4i, sun5i and sun6i SoCs are also supported via the older binding:
 
 sun4i resistive touchscreen controller
-- 
2.11.0

Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4085 bytes --]

Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:04AM +0100, Philipp Rossak wrote:
> Allwinner H3 features a thermal sensor like the one in A33, but has its
> register re-arranged, the clock divider moved to CCU (originally the
> clock divider is in ADC) and added a pair of bus clock and reset.
> 
> Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
> the bus clock and the reset was removed from the CCU. The THS in A83T
> has a clock that is directly connected and runs with 24 MHz.
> 
> Update the binding document to cover H3 and A83T.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks a lot for tackling this.

> ---
>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index 86dd8191b04c..22df0c5c23d4 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>  and sometimes as a touchscreen controller.
>  
>  Required properties:
> -  - compatible: "allwinner,sun8i-a33-ths",
> +  - compatible: must contain one of the following compatibles:
> +		- "allwinner,sun8i-a33-ths"
> +		- "allwinner,sun8i-h3-ths"
> +		- "allwinner,sun8i-a83t-ths"
>    - reg: mmio address range of the chip,
> -  - #thermal-sensor-cells: shall be 0,
> +  - #thermal-sensor-cells: shall be 0 or 1,
>    - #io-channel-cells: shall be 0,
>  
> -Example:
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +		- "allwinner,sun8i-a83t-ths"
> +  - interrupts: the sampling interrupt of the ADC,
> +
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - clocks: the bus clock and the input clock of the ADC,
> +  - clock-names: should be "bus" and "mod",
> +  - resets: the bus reset of the ADC,
> +
> +Optional properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> +		If unspecified default values shall be used. The size should
> +		be 0x2 * sensorcount.
> +  - nvmem-cell-names: Should be "calibration".
> +
> +Details see: bindings/nvmem/nvmem.txt
> +
> +Example for A33:
>  	ths: ths@1c25000 {
>  		compatible = "allwinner,sun8i-a33-ths";
>  		reg = <0x01c25000 0x100>;
> @@ -17,6 +40,27 @@ Example:
>  		#io-channel-cells = <0>;
>  	};
>  
> +Example for H3:
> +	ths: thermal-sensor@1c25000 {
> +		compatible = "allwinner,sun8i-h3-ths";
> +		reg = <0x01c25000 0x400>;
> +		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
> +		clock-names = "bus", "mod";
> +		resets = <&ccu RST_BUS_THS>;
> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +		#thermal-sensor-cells = <0>;
> +		#io-channel-cells = <0>;
> +	};
> +
> +Example for A83T:
> +	ths: thermal-sensor@1f04000 {
> +		compatible = "allwinner,sun8i-a83t-ths";
> +		reg = <0x01f04000 0x100>;
> +		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +		#thermal-sensor-cells = <1>;
> +		#io-channel-cells = <0>;
> +	};
> +

I'm wondering if this is actually needed. We've used this convoluted
constructs to be compatible with the old driver, but I'm not sure this
is actually worth it now, and this is causing several issues, among
which:
  - We need to have a bunch of quirks to handle all the DT cases.
  - We need to have an MFD, which isn't really optimal

So I'd rather introduce a new compatible for the old SoCs, keep the
old driver around, and simplify a lot that driver code that will ease
further developments. And we can also get rid of the MFD in the
process. I discussed it with Quentin, and he was ok with it, what do
you think?

(and that would involve creating a new file for the bindings you
introduce here).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

[Philipp Rossak]

>> +Example for A33:
>>   	ths: ths@1c25000 {
>>   		compatible = "allwinner,sun8i-a33-ths";
>>   		reg = <0x01c25000 0x100>;
>> @@ -17,6 +40,27 @@ Example:
>>   		#io-channel-cells = <0>;
>>   	};
>>   
>> +Example for H3:
>> +	ths: thermal-sensor@1c25000 {
>> +		compatible = "allwinner,sun8i-h3-ths";
>> +		reg = <0x01c25000 0x400>;
>> +		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
>> +		clock-names = "bus", "mod";
>> +		resets = <&ccu RST_BUS_THS>;
>> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
>> +		#thermal-sensor-cells = <0>;
>> +		#io-channel-cells = <0>;
>> +	};
>> +
>> +Example for A83T:
>> +	ths: thermal-sensor@1f04000 {
>> +		compatible = "allwinner,sun8i-a83t-ths";
>> +		reg = <0x01f04000 0x100>;
>> +		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
>> +		#thermal-sensor-cells = <1>;
>> +		#io-channel-cells = <0>;
>> +	};
>> +
> 
> I'm wondering if this is actually needed. We've used this convoluted
> constructs to be compatible with the old driver, but I'm not sure this
> is actually worth it now, and this is causing several issues, among
> which:
>    - We need to have a bunch of quirks to handle all the DT cases.
>    - We need to have an MFD, which isn't really optimal
> 
> So I'd rather introduce a new compatible for the old SoCs, keep the
> old driver around, and simplify a lot that driver code that will ease
> further developments. And we can also get rid of the MFD in the
> process. I discussed it with Quentin, and he was ok with it, what do
> you think?
> 
> (and that would involve creating a new file for the bindings you
> introduce here).
> 
> Maxime
> 

I think this is a good idea, and the desired way to rework the driver.

To sum up what we talked on IRC:

This will end up in removing the MFD driver and moving the interrupt 
handling into the iio driver. At the end this will also simplify the IRQ 
part.

Philipp

Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

[Quentin Schulz]

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:04AM +0100, Philipp Rossak wrote:
> Allwinner H3 features a thermal sensor like the one in A33, but has its
> register re-arranged, the clock divider moved to CCU (originally the
> clock divider is in ADC) and added a pair of bus clock and reset.
> 
> Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
> the bus clock and the reset was removed from the CCU. The THS in A83T
> has a clock that is directly connected and runs with 24 MHz.
> 
> Update the binding document to cover H3 and A83T.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index 86dd8191b04c..22df0c5c23d4 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>  and sometimes as a touchscreen controller.
>  
>  Required properties:
> -  - compatible: "allwinner,sun8i-a33-ths",
> +  - compatible: must contain one of the following compatibles:
> +		- "allwinner,sun8i-a33-ths"
> +		- "allwinner,sun8i-h3-ths"
> +		- "allwinner,sun8i-a83t-ths"
>    - reg: mmio address range of the chip,
> -  - #thermal-sensor-cells: shall be 0,
> +  - #thermal-sensor-cells: shall be 0 or 1,

Well, thermal-sensor-cells is either 0 or 1 :)

Better to point to the documentation describing this
thermal-sensor-cells IMHO.

>    - #io-channel-cells: shall be 0,
>  
> -Example:
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +		- "allwinner,sun8i-a83t-ths"
> +  - interrupts: the sampling interrupt of the ADC,
> +
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - clocks: the bus clock and the input clock of the ADC,
> +  - clock-names: should be "bus" and "mod",
> +  - resets: the bus reset of the ADC,
> +
> +Optional properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> +		If unspecified default values shall be used. The size should
> +		be 0x2 * sensorcount.

"twice the number of sensors" ?

> +  - nvmem-cell-names: Should be "calibration".
> +
> +Details see: bindings/nvmem/nvmem.txt
> +
> +Example for A33:
>  	ths: ths@1c25000 {
>  		compatible = "allwinner,sun8i-a33-ths";
>  		reg = <0x01c25000 0x100>;
> @@ -17,6 +40,27 @@ Example:
>  		#io-channel-cells = <0>;
>  	};
>  
> +Example for H3:
> +	ths: thermal-sensor@1c25000 {
> +		compatible = "allwinner,sun8i-h3-ths";
> +		reg = <0x01c25000 0x400>;
> +		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
> +		clock-names = "bus", "mod";
> +		resets = <&ccu RST_BUS_THS>;
> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +		#thermal-sensor-cells = <0>;
> +		#io-channel-cells = <0>;
> +	};
> +
> +Example for A83T:
> +	ths: thermal-sensor@1f04000 {
> +		compatible = "allwinner,sun8i-a83t-ths";
> +		reg = <0x01f04000 0x100>;
> +		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +		#thermal-sensor-cells = <1>;
> +		#io-channel-cells = <0>;
> +	};
> +

Aside from Maxime's comment on how we would like to refactor GPADC/THS,
I'm not sure we really want an example for each an every thermal sensor
supported.

Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T

[Philipp Rossak]

On 31.01.2018 18:40, Quentin Schulz wrote:
> Hi Philipp,
> 
> On Mon, Jan 29, 2018 at 12:29:04AM +0100, Philipp Rossak wrote:
>> Allwinner H3 features a thermal sensor like the one in A33, but has its
>> register re-arranged, the clock divider moved to CCU (originally the
>> clock divider is in ADC) and added a pair of bus clock and reset.
>>
>> Allwinner A83T features a thermal sensor similar to the H3, the ths clock,
>> the bus clock and the reset was removed from the CCU. The THS in A83T
>> has a clock that is directly connected and runs with 24 MHz.
>>
>> Update the binding document to cover H3 and A83T.
>>
>> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
>> ---
>>   .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 50 ++++++++++++++++++++--
>>   1 file changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> index 86dd8191b04c..22df0c5c23d4 100644
>> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>>   and sometimes as a touchscreen controller.
>>   
>>   Required properties:
>> -  - compatible: "allwinner,sun8i-a33-ths",
>> +  - compatible: must contain one of the following compatibles:
>> +		- "allwinner,sun8i-a33-ths"
>> +		- "allwinner,sun8i-h3-ths"
>> +		- "allwinner,sun8i-a83t-ths"
>>     - reg: mmio address range of the chip,
>> -  - #thermal-sensor-cells: shall be 0,
>> +  - #thermal-sensor-cells: shall be 0 or 1,
> 
> Well, thermal-sensor-cells is either 0 or 1 :)
> 
> Better to point to the documentation describing this
> thermal-sensor-cells IMHO.
> 

I agree, I will change this in the next version.

>>     - #io-channel-cells: shall be 0,
>>   
>> -Example:
>> +Required properties for the following compatibles:
>> +		- "allwinner,sun8i-h3-ths"
>> +		- "allwinner,sun8i-a83t-ths"
>> +  - interrupts: the sampling interrupt of the ADC,
>> +
>> +Required properties for the following compatibles:
>> +		- "allwinner,sun8i-h3-ths"
>> +  - clocks: the bus clock and the input clock of the ADC,
>> +  - clock-names: should be "bus" and "mod",
>> +  - resets: the bus reset of the ADC,
>> +
>> +Optional properties for the following compatibles:
>> +		- "allwinner,sun8i-h3-ths"
>> +  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
>> +		If unspecified default values shall be used. The size should
>> +		be 0x2 * sensorcount.
> 
> "twice the number of sensors" ?
> 

As already mentioned in an other answers, this here is not correct.
I got somehow a wrong information or mixed something up. For H5, H3, 
A83T and A64 the thermal sensor calibration data is always 64 bit wide 
and placed on the eFuse address  0x34 [1].

>> +  - nvmem-cell-names: Should be "calibration".
>> +
>> +Details see: bindings/nvmem/nvmem.txt
>> +
>> +Example for A33:
>>   	ths: ths@1c25000 {
>>   		compatible = "allwinner,sun8i-a33-ths";
>>   		reg = <0x01c25000 0x100>;
>> @@ -17,6 +40,27 @@ Example:
>>   		#io-channel-cells = <0>;
>>   	};
>>   
>> +Example for H3:
>> +	ths: thermal-sensor@1c25000 {
>> +		compatible = "allwinner,sun8i-h3-ths";
>> +		reg = <0x01c25000 0x400>;
>> +		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
>> +		clock-names = "bus", "mod";
>> +		resets = <&ccu RST_BUS_THS>;
>> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
>> +		#thermal-sensor-cells = <0>;
>> +		#io-channel-cells = <0>;
>> +	};
>> +
>> +Example for A83T:
>> +	ths: thermal-sensor@1f04000 {
>> +		compatible = "allwinner,sun8i-a83t-ths";
>> +		reg = <0x01f04000 0x100>;
>> +		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
>> +		#thermal-sensor-cells = <1>;
>> +		#io-channel-cells = <0>;
>> +	};
>> +
> 
> Aside from Maxime's comment on how we would like to refactor GPADC/THS,
> I'm not sure we really want an example for each an every thermal sensor
> supported.
> 
> Quentin
> 
I agree we don't want to have an example for every sensor, but I think 
at least those two are interesting, since one has 3 sensors and no 
clocks, and one has 1 sensor and clocks.

Thanks,
Philipp

[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

[PATCH v2 02/16] arm: config: sunxi_defconfig: enable SUN4I_GPADC

[Philipp Rossak]

This commit enables the SUN4I_GPADC config option and
sets the value to yes. This is needed to enable the ths sensors.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 5caaf971fb50..52c3990b9d91 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
 CONFIG_EXTCON=y
 CONFIG_IIO=y
 CONFIG_AXP20X_ADC=y
+CONFIG_SUN4I_GPADC=y
 CONFIG_PWM=y
 CONFIG_PWM_SUN4I=y
 CONFIG_PHY_SUN4I_USB=y
-- 
2.11.0

Re: [PATCH v2 02/16] arm: config: sunxi_defconfig: enable SUN4I_GPADC

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

Hi,

On Mon, Jan 29, 2018 at 12:29:05AM +0100, Philipp Rossak wrote:
> This commit enables the SUN4I_GPADC config option and
> sets the value to yes. This is needed to enable the ths sensors.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/configs/sunxi_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
> index 5caaf971fb50..52c3990b9d91 100644
> --- a/arch/arm/configs/sunxi_defconfig
> +++ b/arch/arm/configs/sunxi_defconfig
> @@ -131,6 +131,7 @@ CONFIG_DMA_SUN6I=y
>  CONFIG_EXTCON=y
>  CONFIG_IIO=y
>  CONFIG_AXP20X_ADC=y
> +CONFIG_SUN4I_GPADC=y

Unfortunately, we can't do that since TOUCHSCREEN_SUN4I is already
enabled and we have a depends on !TOUCHSCREEN_SUN4I.

One more reason to stop worrying about replacing that driver DT
binding.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 03/16] iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain A33

[Philipp Rossak]

From: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>

As the H3 SoC, which is also in sun8i line, has totally different
register map for the thermal sensor (a cut down version of GPADC), we
should rename A23/A33-specified registers to contain A33, in order to
prevent obfuscation with H3 registers. Currently these registers are
only prefixed "SUN8I", not "SUN8I_A33".

Add "_A33" after "SUN8I" on the register names.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
Reviewed-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Jonathan Cameron <jonathan.cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
 include/linux/mfd/sun4i-gpadc.h   | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 04d7147e0110..03804ff9c006 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -88,7 +88,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
 static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.temp_offset = -1662,
 	.temp_scale = 162,
-	.tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+	.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
 };
 
 struct sun4i_gpadc_iio {
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 139872c2e0fe..78d31984a222 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -38,9 +38,9 @@
 #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x)		(GENMASK(3, 0) & BIT(x))
 #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK			GENMASK(3, 0)
 
-/* TP_CTRL1 bits for sun8i SoCs */
-#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN			BIT(8)
-#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN			BIT(7)
+/* TP_CTRL1 bits for A33 */
+#define SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN		BIT(8)
+#define SUN8I_A33_GPADC_CTRL1_GPADC_CALI_EN		BIT(7)
 
 #define SUN4I_GPADC_CTRL2				0x08
 
-- 
2.11.0

[PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg

[Philipp Rossak]

For adding newer sensor some basic rework of the code is necessary.

This commit reworks the code and allows the sampling start/end code and
the position of value readout register to be altered. Later the start/end
functions will be used to configure the ths and start/stop the
sampling.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 03804ff9c006..db57d9fffe48 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
 	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
 }
 
+struct sun4i_gpadc_iio;
+
+/*
+ * Prototypes for these functions, which enable these functions to be
+ * referenced in gpadc_data structures.
+ */
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
+
 struct gpadc_data {
 	int		temp_offset;
 	int		temp_scale;
@@ -56,6 +65,9 @@ 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;
+	int		(*sample_start)(struct sun4i_gpadc_iio *info);
+	int		(*sample_end)(struct sun4i_gpadc_iio *info);
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {
@@ -65,6 +77,9 @@ 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,
+	.sample_start = sun4i_gpadc_sample_start,
+	.sample_end = sun4i_gpadc_sample_end,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {
@@ -74,6 +89,9 @@ static const struct gpadc_data sun5i_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,
+	.sample_start = sun4i_gpadc_sample_start,
+	.sample_end = sun4i_gpadc_sample_end,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {
@@ -83,12 +101,18 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun6i_gpadc_chan_select,
 	.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
+	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.sample_start = sun4i_gpadc_sample_start,
+	.sample_end = sun4i_gpadc_sample_end,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.temp_offset = -1662,
 	.temp_scale = 162,
 	.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
+	.temp_data = SUN4I_GPADC_TEMP_DATA,
+	.sample_start = sun4i_gpadc_sample_start,
+	.sample_end = sun4i_gpadc_sample_end,
 };
 
 struct sun4i_gpadc_iio {
@@ -277,7 +301,7 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
 	if (info->no_irq) {
 		pm_runtime_get_sync(indio_dev->dev.parent);
 
-		regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+		regmap_read(info->regmap, info->data->temp_data, val);
 
 		pm_runtime_mark_last_busy(indio_dev->dev.parent);
 		pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -382,10 +406,8 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int sun4i_gpadc_runtime_suspend(struct device *dev)
+static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
 {
-	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
-
 	/* Disable the ADC on IP */
 	regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
 	/* Disable temperature sensor on IP */
@@ -394,10 +416,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int sun4i_gpadc_runtime_resume(struct device *dev)
+static int sun4i_gpadc_runtime_suspend(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
 
+	return info->data->sample_end(info);
+}
+
+static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
+{
 	/* clkin = 6MHz */
 	regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
 		     SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
@@ -415,6 +442,13 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int sun4i_gpadc_runtime_resume(struct device *dev)
+{
+	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
+
+	return info->data->sample_start(info);
+}
+
 static int sun4i_gpadc_get_temp(void *data, int *temp)
 {
 	struct sun4i_gpadc_iio *info = data;
-- 
2.11.0

Re: [PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

Hi,

On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
> 
> This commit reworks the code and allows the sampling start/end code and
> the position of value readout register to be altered. Later the start/end
> functions will be used to configure the ths and start/stop the
> sampling.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

That signed-off-by chain doesn't really make much sense. If Icenowy is
the author, she should be reported as such in the commit, and if
you're the author, you shouldn't have her Signed-off-by.

> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 03804ff9c006..db57d9fffe48 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
>  	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
>  }
>  
> +struct sun4i_gpadc_iio;
> +
> +/*
> + * Prototypes for these functions, which enable these functions to be
> + * referenced in gpadc_data structures.
> + */
> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
> +
>  struct gpadc_data {
>  	int		temp_offset;
>  	int		temp_scale;
> @@ -56,6 +65,9 @@ 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;
> +	int		(*sample_start)(struct sun4i_gpadc_iio *info);
> +	int		(*sample_end)(struct sun4i_gpadc_iio *info);
>  };
>  
>  static const struct gpadc_data sun4i_gpadc_data = {
> @@ -65,6 +77,9 @@ 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,

You can use a regmap_field there.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg

[Quentin Schulz]

[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]

Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
> 
> This commit reworks the code and allows the sampling start/end code and
> the position of value readout register to be altered. Later the start/end
> functions will be used to configure the ths and start/stop the
> sampling.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 03804ff9c006..db57d9fffe48 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
>  	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
>  }
>  
> +struct sun4i_gpadc_iio;
> +
> +/*
> + * Prototypes for these functions, which enable these functions to be
> + * referenced in gpadc_data structures.
> + */

Comment not needed.

> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
> +
>  struct gpadc_data {
>  	int		temp_offset;
>  	int		temp_scale;
> @@ -56,6 +65,9 @@ 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;

Does not really have anything to do with sample_start/end. I would have
made a different commit for it.

Otherwise,
Reviewed-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg

[Philipp Rossak]

On 31.01.2018 18:51, Quentin Schulz wrote:
> Hi Philipp,
> 
> On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote:
>> For adding newer sensor some basic rework of the code is necessary.
>>
>> This commit reworks the code and allows the sampling start/end code and
>> the position of value readout register to be altered. Later the start/end
>> functions will be used to configure the ths and start/stop the
>> sampling.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 03804ff9c006..db57d9fffe48 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan)
>>   	return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan);
>>   }
>>   
>> +struct sun4i_gpadc_iio;
>> +
>> +/*
>> + * Prototypes for these functions, which enable these functions to be
>> + * referenced in gpadc_data structures.
>> + */
> 
> Comment not needed.
> 
>> +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>> +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>> +
>>   struct gpadc_data {
>>   	int		temp_offset;
>>   	int		temp_scale;
>> @@ -56,6 +65,9 @@ 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;
> 
> Does not really have anything to do with sample_start/end. I would have
> made a different commit for it.
> 
> Otherwise,
> Reviewed-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Quentin
> 

Ok I will split this.

Thanks,
Philipp

[PATCH v2 05/16] iio: adc: sun4i-gpadc-iio: rework: support clocks and reset

[Philipp Rossak]

For adding newer sensor some basic rework of the code is necessary.

The SoCs after H3 has newer thermal sensor ADCs, which have two clock
inputs (bus clock and sampling clock) and a reset. The registers are
also re-arranged.

This commit reworks the code, adds the process of the clocks and
resets.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index db57d9fffe48..51ec0104d678 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -22,6 +22,7 @@
  * shutdown for not being used.
  */
 
+#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -31,6 +32,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/thermal.h>
 #include <linux/delay.h>
 
@@ -68,6 +70,9 @@ struct gpadc_data {
 	unsigned int	temp_data;
 	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;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {
@@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
 	atomic_t			ignore_temp_data_irq;
 	const struct gpadc_data		*data;
 	bool				no_irq;
+	struct clk			*bus_clk;
+	struct clk			*mod_clk;
+	struct reset_control		*reset;
 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;
@@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
 
+	clk_disable(info->mod_clk);
+
+	clk_disable(info->bus_clk);
+
 	return info->data->sample_end(info);
 }
 
@@ -446,6 +458,10 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
 
+	clk_enable(info->mod_clk);
+
+	clk_enable(info->bus_clk);
+
 	return info->data->sample_start(info);
 }
 
@@ -560,10 +576,59 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 		return ret;
 	}
 
+	if (info->data->has_bus_rst) {
+		info->reset = devm_reset_control_get(&pdev->dev, NULL);
+		if (IS_ERR(info->reset)) {
+			ret = PTR_ERR(info->reset);
+			return ret;
+		}
+
+		ret = reset_control_deassert(info->reset);
+		if (ret)
+			return ret;
+	}
+
+	if (info->data->has_bus_clk) {
+		info->bus_clk = devm_clk_get(&pdev->dev, "bus");
+		if (IS_ERR(info->bus_clk)) {
+			ret = PTR_ERR(info->bus_clk);
+			goto assert_reset;
+		}
+
+		ret = clk_prepare_enable(info->bus_clk);
+		if (ret)
+			goto assert_reset;
+	}
+
+	if (info->data->has_mod_clk) {
+		info->mod_clk = devm_clk_get(&pdev->dev, "mod");
+		if (IS_ERR(info->mod_clk)) {
+			ret = PTR_ERR(info->mod_clk);
+			goto disable_bus_clk;
+		}
+
+		/* Running at 6MHz */
+		ret = clk_set_rate(info->mod_clk, 4000000);
+		if (ret)
+			goto disable_bus_clk;
+
+		ret = clk_prepare_enable(info->mod_clk);
+		if (ret)
+			goto disable_bus_clk;
+	}
+
 	if (IS_ENABLED(CONFIG_THERMAL_OF))
 		info->sensor_device = &pdev->dev;
 
 	return 0;
+
+disable_bus_clk:
+	clk_disable_unprepare(info->bus_clk);
+
+assert_reset:
+	reset_control_assert(info->reset);
+
+	return ret;
 }
 
 static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
@@ -729,6 +794,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
 	if (!info->no_irq)
 		iio_map_array_unregister(indio_dev);
 
+	clk_disable_unprepare(info->mod_clk);
+
+	clk_disable_unprepare(info->bus_clk);
+
+	reset_control_assert(info->reset);
+
 	return 0;
 }
 
-- 
2.11.0

Re: [PATCH v2 05/16] iio: adc: sun4i-gpadc-iio: rework: support clocks and reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3985 bytes --]

On Mon, Jan 29, 2018 at 12:29:08AM +0100, Philipp Rossak wrote:
> For adding newer sensor some basic rework of the code is necessary.
> 
> The SoCs after H3 has newer thermal sensor ADCs, which have two clock
> inputs (bus clock and sampling clock) and a reset. The registers are
> also re-arranged.
> 
> This commit reworks the code, adds the process of the clocks and
> resets.
> 
> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

Same remark for the SoB

> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 71 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index db57d9fffe48..51ec0104d678 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -22,6 +22,7 @@
>   * shutdown for not being used.
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -31,6 +32,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/thermal.h>
>  #include <linux/delay.h>
>  
> @@ -68,6 +70,9 @@ struct gpadc_data {
>  	unsigned int	temp_data;
>  	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;

Is there SoCs where this insn't all true, or all false?

>  };
>  
>  static const struct gpadc_data sun4i_gpadc_data = {
> @@ -127,6 +132,9 @@ struct sun4i_gpadc_iio {
>  	atomic_t			ignore_temp_data_irq;
>  	const struct gpadc_data		*data;
>  	bool				no_irq;
> +	struct clk			*bus_clk;
> +	struct clk			*mod_clk;
> +	struct reset_control		*reset;
>  	/* prevents concurrent reads of temperature and ADC */
>  	struct mutex			mutex;
>  	struct thermal_zone_device	*tzd;
> @@ -420,6 +428,10 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>  
> +	clk_disable(info->mod_clk);
> +

Why clk_disable?

> +	clk_disable(info->bus_clk);
> +

You can tie the bus_clk to the regmap directly, instead of having to
maintain it yourself here.

And you can probably put the device in reset and out of reset here as
well.

>  	return info->data->sample_end(info);
>  }
>  
> @@ -446,6 +458,10 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>  
> +	clk_enable(info->mod_clk);
> +
> +	clk_enable(info->bus_clk);
> +
>  	return info->data->sample_start(info);
>  }
>  
> @@ -560,10 +576,59 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>  		return ret;
>  	}
>  
> +	if (info->data->has_bus_rst) {
> +		info->reset = devm_reset_control_get(&pdev->dev, NULL);
> +		if (IS_ERR(info->reset)) {
> +			ret = PTR_ERR(info->reset);
> +			return ret;
> +		}
> +
> +		ret = reset_control_deassert(info->reset);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (info->data->has_bus_clk) {
> +		info->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +		if (IS_ERR(info->bus_clk)) {
> +			ret = PTR_ERR(info->bus_clk);
> +			goto assert_reset;
> +		}
> +
> +		ret = clk_prepare_enable(info->bus_clk);
> +		if (ret)
> +			goto assert_reset;
> +	}
> +
> +	if (info->data->has_mod_clk) {
> +		info->mod_clk = devm_clk_get(&pdev->dev, "mod");
> +		if (IS_ERR(info->mod_clk)) {
> +			ret = PTR_ERR(info->mod_clk);
> +			goto disable_bus_clk;
> +		}
> +
> +		/* Running at 6MHz */
> +		ret = clk_set_rate(info->mod_clk, 4000000);

Your comment and the line below doesn't really make much sense :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Philipp Rossak]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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;
 };
 
 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,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {
@@ -94,9 +96,10 @@ static const struct gpadc_data sun5i_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,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {
@@ -106,18 +109,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun6i_gpadc_chan_select,
 	.adc_chan_mask = SUN6I_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,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.temp_offset = -1662,
 	.temp_scale = 162,
 	.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
-	.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,
 };
 
 struct sun4i_gpadc_iio {
@@ -135,6 +140,7 @@ struct sun4i_gpadc_iio {
 	struct clk			*bus_clk;
 	struct clk			*mod_clk;
 	struct reset_control		*reset;
+	int				sensor_id;
 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;
@@ -302,14 +308,15 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
 	return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
 }
 
-static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
+static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
+				int sensor)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
 
 	if (info->no_irq) {
 		pm_runtime_get_sync(indio_dev->dev.parent);
 
-		regmap_read(info->regmap, info->data->temp_data, val);
+		regmap_read(info->regmap, info->data->temp_data[sensor], val);
 
 		pm_runtime_mark_last_busy(indio_dev->dev.parent);
 		pm_runtime_put_autosuspend(indio_dev->dev.parent);
@@ -356,7 +363,7 @@ static int sun4i_gpadc_read_raw(struct iio_dev *indio_dev,
 			ret = sun4i_gpadc_adc_read(indio_dev, chan->channel,
 						   val);
 		else
-			ret = sun4i_gpadc_temp_read(indio_dev, val);
+			ret = sun4i_gpadc_temp_read(indio_dev, val, 0);
 
 		if (ret)
 			return ret;
@@ -470,7 +477,7 @@ static int sun4i_gpadc_get_temp(void *data, int *temp)
 	struct sun4i_gpadc_iio *info = data;
 	int val, scale, offset;
 
-	if (sun4i_gpadc_temp_read(info->indio_dev, &val))
+	if (sun4i_gpadc_temp_read(info->indio_dev, &val, info->sensor_id))
 		return -ETIMEDOUT;
 
 	sun4i_gpadc_temp_scale(info->indio_dev, &scale);
@@ -712,7 +719,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 {
 	struct sun4i_gpadc_iio *info;
 	struct iio_dev *indio_dev;
-	int ret;
+	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
 	if (!indio_dev)
@@ -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);
+		}
 		/*
 		 * Do not fail driver probing when failing to register in
 		 * thermal because no thermal DT node is found.
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 78d31984a222..20fa02040007 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -90,6 +90,9 @@
 /* 10s delay before suspending the IP */
 #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
 
+/* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define MAX_SENSOR_COUNT				4
+
 struct sun4i_gpadc_dev {
 	struct device			*dev;
 	struct regmap			*regmap;
-- 
2.11.0

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6720 bytes --]

Hi,

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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;
>  };
>  
>  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,
>  };
>  
>  static const struct gpadc_data sun5i_gpadc_data = {
> @@ -94,9 +96,10 @@ static const struct gpadc_data sun5i_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,
>  };
>  
>  static const struct gpadc_data sun6i_gpadc_data = {
> @@ -106,18 +109,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
>  	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
>  	.adc_chan_select = &sun6i_gpadc_chan_select,
>  	.adc_chan_mask = SUN6I_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,
>  };
>  
>  static const struct gpadc_data sun8i_a33_gpadc_data = {
>  	.temp_offset = -1662,
>  	.temp_scale = 162,
>  	.tp_mode_en = SUN8I_A33_GPADC_CTRL1_CHOP_TEMP_EN,
> -	.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,
>  };
>  
>  struct sun4i_gpadc_iio {
> @@ -135,6 +140,7 @@ struct sun4i_gpadc_iio {
>  	struct clk			*bus_clk;
>  	struct clk			*mod_clk;
>  	struct reset_control		*reset;
> +	int				sensor_id;
>  	/* prevents concurrent reads of temperature and ADC */
>  	struct mutex			mutex;
>  	struct thermal_zone_device	*tzd;
> @@ -302,14 +308,15 @@ static int sun4i_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
>  	return sun4i_gpadc_read(indio_dev, channel, val, info->fifo_data_irq);
>  }
>  
> -static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> +static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val,
> +				int sensor)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>  
>  	if (info->no_irq) {
>  		pm_runtime_get_sync(indio_dev->dev.parent);
>  
> -		regmap_read(info->regmap, info->data->temp_data, val);
> +		regmap_read(info->regmap, info->data->temp_data[sensor], val);
>  
>  		pm_runtime_mark_last_busy(indio_dev->dev.parent);
>  		pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -356,7 +363,7 @@ static int sun4i_gpadc_read_raw(struct iio_dev *indio_dev,
>  			ret = sun4i_gpadc_adc_read(indio_dev, chan->channel,
>  						   val);
>  		else
> -			ret = sun4i_gpadc_temp_read(indio_dev, val);
> +			ret = sun4i_gpadc_temp_read(indio_dev, val, 0);
>  
>  		if (ret)
>  			return ret;
> @@ -470,7 +477,7 @@ static int sun4i_gpadc_get_temp(void *data, int *temp)
>  	struct sun4i_gpadc_iio *info = data;
>  	int val, scale, offset;
>  
> -	if (sun4i_gpadc_temp_read(info->indio_dev, &val))
> +	if (sun4i_gpadc_temp_read(info->indio_dev, &val, info->sensor_id))
>  		return -ETIMEDOUT;
>  
>  	sun4i_gpadc_temp_scale(info->indio_dev, &scale);
> @@ -712,7 +719,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>  {
>  	struct sun4i_gpadc_iio *info;
>  	struct iio_dev *indio_dev;
> -	int ret;
> +	int ret, i;
>  
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
>  	if (!indio_dev)
> @@ -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);
> +		}

I'm not sure how that works. Isn't the info structure shared between
all the sensors? The sensor_id value would be always set to the last
sensor then.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Quentin Schulz]

[-- Attachment #1: Type: text/plain, Size: 4093 bytes --]

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@gmail.com>
> ---
>  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.

>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Philipp Rossak]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[Philipp Rossak]

This patch reworks the driver to support nvmem calibration cells.
The driver checks if the nvmem calibration is supported and reads out
the nvmem.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index ac9ad2f8232f..74eeb5cd5218 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -27,6 +27,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -74,6 +75,7 @@ struct gpadc_data {
 	bool		has_bus_rst;
 	bool		has_mod_clk;
 	int		sensor_count;
+	bool		supports_nvmem;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {
@@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {
@@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {
@@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
@@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.sample_start = sun4i_gpadc_sample_start,
 	.sample_end = sun4i_gpadc_sample_end,
 	.sensor_count = 1,
+	.supports_nvmem = false,
 };
 
 struct sun4i_gpadc_iio {
@@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
 	struct clk			*mod_clk;
 	struct reset_control		*reset;
 	int				sensor_id;
+	u32				calibration_data[2];
+	bool				has_calibration_data[2];
 	/* prevents concurrent reads of temperature and ADC */
 	struct mutex			mutex;
 	struct thermal_zone_device	*tzd;
@@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	struct resource *mem;
 	void __iomem *base;
 	int ret;
+	struct nvmem_cell *cell;
+	ssize_t cell_size;
+	u64 *cell_data;
 
 	info->data = of_device_get_match_data(&pdev->dev);
 	if (!info->data)
@@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	info->has_calibration_data[0] = false;
+	info->has_calibration_data[1] = false;
+
+	if (!info->data->supports_nvmem)
+		goto no_nvmem;
+
+	cell = nvmem_cell_get(&pdev->dev, "calibration");
+	if (IS_ERR(cell)) {
+		if (PTR_ERR(cell) == -EPROBE_DEFER)
+			return PTR_ERR(cell);
+		goto no_nvmem;
+	}
+
+	cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
+	nvmem_cell_put(cell);
+	switch (cell_size) {
+		case 8:
+		case 6:
+			info->has_calibration_data[1] = true;
+			info->calibration_data[1] = be32_to_cpu(
+					upper_32_bits(cell_data[0]));
+		case 4:
+		case 2:
+			info->has_calibration_data[0] = true;
+			info->calibration_data[0] = be32_to_cpu(
+					lower_32_bits(cell_data[0]));
+			break;
+		default:
+			break;
+	}
+
+no_nvmem:
+
 	info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
 					     &sun4i_gpadc_regmap_config);
 	if (IS_ERR(info->regmap)) {
-- 
2.11.0

Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4163 bytes --]

On Mon, Jan 29, 2018 at 12:29:10AM +0100, Philipp Rossak wrote:
> This patch reworks the driver to support nvmem calibration cells.
> The driver checks if the nvmem calibration is supported and reads out
> the nvmem.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index ac9ad2f8232f..74eeb5cd5218 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -27,6 +27,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> @@ -74,6 +75,7 @@ struct gpadc_data {
>  	bool		has_bus_rst;
>  	bool		has_mod_clk;
>  	int		sensor_count;
> +	bool		supports_nvmem;

I think you should add some documentation along with all the fields
you're adding.

>  };
>  
>  static const struct gpadc_data sun4i_gpadc_data = {
> @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>  	.sample_start = sun4i_gpadc_sample_start,
>  	.sample_end = sun4i_gpadc_sample_end,
>  	.sensor_count = 1,
> +	.supports_nvmem = false,

That's already its value if you leave it out.

>  };
>  
>  static const struct gpadc_data sun5i_gpadc_data = {
> @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
>  	.sample_start = sun4i_gpadc_sample_start,
>  	.sample_end = sun4i_gpadc_sample_end,
>  	.sensor_count = 1,
> +	.supports_nvmem = false,
>  };
>  
>  static const struct gpadc_data sun6i_gpadc_data = {
> @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
>  	.sample_start = sun4i_gpadc_sample_start,
>  	.sample_end = sun4i_gpadc_sample_end,
>  	.sensor_count = 1,
> +	.supports_nvmem = false,
>  };
>  
>  static const struct gpadc_data sun8i_a33_gpadc_data = {
> @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>  	.sample_start = sun4i_gpadc_sample_start,
>  	.sample_end = sun4i_gpadc_sample_end,
>  	.sensor_count = 1,
> +	.supports_nvmem = false,
>  };
>  
>  struct sun4i_gpadc_iio {
> @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
>  	struct clk			*mod_clk;
>  	struct reset_control		*reset;
>  	int				sensor_id;
> +	u32				calibration_data[2];
> +	bool				has_calibration_data[2];

Why do you have two different values here?

>  	/* prevents concurrent reads of temperature and ADC */
>  	struct mutex			mutex;
>  	struct thermal_zone_device	*tzd;
> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>  	struct resource *mem;
>  	void __iomem *base;
>  	int ret;
> +	struct nvmem_cell *cell;
> +	ssize_t cell_size;
> +	u64 *cell_data;
>  
>  	info->data = of_device_get_match_data(&pdev->dev);
>  	if (!info->data)
> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	info->has_calibration_data[0] = false;
> +	info->has_calibration_data[1] = false;
> +
> +	if (!info->data->supports_nvmem)
> +		goto no_nvmem;
> +
> +	cell = nvmem_cell_get(&pdev->dev, "calibration");
> +	if (IS_ERR(cell)) {
> +		if (PTR_ERR(cell) == -EPROBE_DEFER)
> +			return PTR_ERR(cell);
> +		goto no_nvmem;

goto considered evil ? :)

> +	}
> +
> +	cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
> +	nvmem_cell_put(cell);
> +	switch (cell_size) {
> +		case 8:
> +		case 6:
> +			info->has_calibration_data[1] = true;
> +			info->calibration_data[1] = be32_to_cpu(
> +					upper_32_bits(cell_data[0]));
> +		case 4:
> +		case 2:
> +			info->has_calibration_data[0] = true;
> +			info->calibration_data[0] = be32_to_cpu(
> +					lower_32_bits(cell_data[0]));

Why do you need that switch?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[Philipp Rossak]

On 29.01.2018 10:40, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:10AM +0100, Philipp Rossak wrote:
>> This patch reworks the driver to support nvmem calibration cells.
>> The driver checks if the nvmem calibration is supported and reads out
>> the nvmem.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/iio/adc/sun4i-gpadc-iio.c | 44 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index ac9ad2f8232f..74eeb5cd5218 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -27,6 +27,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>>   #include <linux/module.h>
>> +#include <linux/nvmem-consumer.h>
>>   #include <linux/of.h>
>>   #include <linux/of_device.h>
>>   #include <linux/platform_device.h>
>> @@ -74,6 +75,7 @@ struct gpadc_data {
>>   	bool		has_bus_rst;
>>   	bool		has_mod_clk;
>>   	int		sensor_count;
>> +	bool		supports_nvmem;
> 
> I think you should add some documentation along with all the fields
> you're adding.

ok I will add more informations in the next version into the commit message.

> 
>>   };
>>   
>>   static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
>>   	.sample_start = sun4i_gpadc_sample_start,
>>   	.sample_end = sun4i_gpadc_sample_end,
>>   	.sensor_count = 1,
>> +	.supports_nvmem = false,
> 
> That's already its value if you leave it out.
> 
>>   };
>>   
>>   static const struct gpadc_data sun5i_gpadc_data = {
>> @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
>>   	.sample_start = sun4i_gpadc_sample_start,
>>   	.sample_end = sun4i_gpadc_sample_end,
>>   	.sensor_count = 1,
>> +	.supports_nvmem = false,
>>   };
>>   
>>   static const struct gpadc_data sun6i_gpadc_data = {
>> @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
>>   	.sample_start = sun4i_gpadc_sample_start,
>>   	.sample_end = sun4i_gpadc_sample_end,
>>   	.sensor_count = 1,
>> +	.supports_nvmem = false,
>>   };
>>   
>>   static const struct gpadc_data sun8i_a33_gpadc_data = {
>> @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>>   	.sample_start = sun4i_gpadc_sample_start,
>>   	.sample_end = sun4i_gpadc_sample_end,
>>   	.sensor_count = 1,
>> +	.supports_nvmem = false,
>>   };
>>   
>>   struct sun4i_gpadc_iio {
>> @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
>>   	struct clk			*mod_clk;
>>   	struct reset_control		*reset;
>>   	int				sensor_id;
>> +	u32				calibration_data[2];
>> +	bool				has_calibration_data[2];
> 
> Why do you have two different values here?
> 

I think my idea was too complex! I thought it would be better to check 
if calibration data was read, and is able to be written to hardware. 
those information were split per register.

I think a u64 should be fine for calibration_data. When I write the 
calibration data I can check on the sensor count and write only the 
lower 32 bits if there are less than 3 sensors.

Is this ok for you?


>>   	/* prevents concurrent reads of temperature and ADC */
>>   	struct mutex			mutex;
>>   	struct thermal_zone_device	*tzd;
>> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>   	struct resource *mem;
>>   	void __iomem *base;
>>   	int ret;
>> +	struct nvmem_cell *cell;
>> +	ssize_t cell_size;
>> +	u64 *cell_data;
>>   
>>   	info->data = of_device_get_match_data(&pdev->dev);
>>   	if (!info->data)
>> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>   	if (IS_ERR(base))
>>   		return PTR_ERR(base);
>>   
>> +	info->has_calibration_data[0] = false;
>> +	info->has_calibration_data[1] = false;
>> +
>> +	if (!info->data->supports_nvmem)
>> +		goto no_nvmem;
>> +
>> +	cell = nvmem_cell_get(&pdev->dev, "calibration");
>> +	if (IS_ERR(cell)) {
>> +		if (PTR_ERR(cell) == -EPROBE_DEFER)
>> +			return PTR_ERR(cell);
>> +		goto no_nvmem;
> 
> goto considered evil ? :)
> 

this was a suggestion from Jonatan in version one, to make the code 
better readable.
.
>> +	}
>> +
>> +	cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
>> +	nvmem_cell_put(cell);
>> +	switch (cell_size) {
>> +		case 8:
>> +		case 6:
>> +			info->has_calibration_data[1] = true;
>> +			info->calibration_data[1] = be32_to_cpu(
>> +					upper_32_bits(cell_data[0]));
>> +		case 4:
>> +		case 2:
>> +			info->has_calibration_data[0] = true;
>> +			info->calibration_data[0] = be32_to_cpu(
>> +					lower_32_bits(cell_data[0]));
> 
> Why do you need that switch?

You are right! The calibration reg seems to be always 64 bit wide. [1]
So I will just check for the length of 8.


> 
> Thanks!
> Maxime
> 

[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

Thanks,
Philipp

Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]

On Mon, Jan 29, 2018 at 01:33:12PM +0100, Philipp Rossak wrote:
> > >   static const struct gpadc_data sun4i_gpadc_data = {
> > > @@ -87,6 +89,7 @@ static const struct gpadc_data sun4i_gpadc_data = {
> > >   	.sample_start = sun4i_gpadc_sample_start,
> > >   	.sample_end = sun4i_gpadc_sample_end,
> > >   	.sensor_count = 1,
> > > +	.supports_nvmem = false,
> > 
> > That's already its value if you leave it out.
> > 
> > >   };
> > >   static const struct gpadc_data sun5i_gpadc_data = {
> > > @@ -100,6 +103,7 @@ static const struct gpadc_data sun5i_gpadc_data = {
> > >   	.sample_start = sun4i_gpadc_sample_start,
> > >   	.sample_end = sun4i_gpadc_sample_end,
> > >   	.sensor_count = 1,
> > > +	.supports_nvmem = false,
> > >   };
> > >   static const struct gpadc_data sun6i_gpadc_data = {
> > > @@ -113,6 +117,7 @@ static const struct gpadc_data sun6i_gpadc_data = {
> > >   	.sample_start = sun4i_gpadc_sample_start,
> > >   	.sample_end = sun4i_gpadc_sample_end,
> > >   	.sensor_count = 1,
> > > +	.supports_nvmem = false,
> > >   };
> > >   static const struct gpadc_data sun8i_a33_gpadc_data = {
> > > @@ -123,6 +128,7 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
> > >   	.sample_start = sun4i_gpadc_sample_start,
> > >   	.sample_end = sun4i_gpadc_sample_end,
> > >   	.sensor_count = 1,
> > > +	.supports_nvmem = false,
> > >   };
> > >   struct sun4i_gpadc_iio {
> > > @@ -141,6 +147,8 @@ struct sun4i_gpadc_iio {
> > >   	struct clk			*mod_clk;
> > >   	struct reset_control		*reset;
> > >   	int				sensor_id;
> > > +	u32				calibration_data[2];
> > > +	bool				has_calibration_data[2];
> > 
> > Why do you have two different values here?
> 
> I think my idea was too complex! I thought it would be better to check if
> calibration data was read, and is able to be written to hardware. those
> information were split per register.
> 
> I think a u64 should be fine for calibration_data. When I write the
> calibration data I can check on the sensor count and write only the lower 32
> bits if there are less than 3 sensors.
> 
> Is this ok for you?

I'd need to see the implementation, but make sure that this is
documented in your driver :)

> 
> > >   	/* prevents concurrent reads of temperature and ADC */
> > >   	struct mutex			mutex;
> > >   	struct thermal_zone_device	*tzd;
> > > @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> > >   	struct resource *mem;
> > >   	void __iomem *base;
> > >   	int ret;
> > > +	struct nvmem_cell *cell;
> > > +	ssize_t cell_size;
> > > +	u64 *cell_data;
> > >   	info->data = of_device_get_match_data(&pdev->dev);
> > >   	if (!info->data)
> > > @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> > >   	if (IS_ERR(base))
> > >   		return PTR_ERR(base);
> > > +	info->has_calibration_data[0] = false;
> > > +	info->has_calibration_data[1] = false;
> > > +
> > > +	if (!info->data->supports_nvmem)
> > > +		goto no_nvmem;
> > > +
> > > +	cell = nvmem_cell_get(&pdev->dev, "calibration");
> > > +	if (IS_ERR(cell)) {
> > > +		if (PTR_ERR(cell) == -EPROBE_DEFER)
> > > +			return PTR_ERR(cell);
> > > +		goto no_nvmem;
> > 
> > goto considered evil ? :)
> 
> this was a suggestion from Jonatan in version one, to make the code better
> readable.

Isn't

if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))

pretty much the same thing?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[Philipp Rossak]

>>
>>>>    	/* prevents concurrent reads of temperature and ADC */
>>>>    	struct mutex			mutex;
>>>>    	struct thermal_zone_device	*tzd;
>>>> @@ -561,6 +569,9 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>>>    	struct resource *mem;
>>>>    	void __iomem *base;
>>>>    	int ret;
>>>> +	struct nvmem_cell *cell;
>>>> +	ssize_t cell_size;
>>>> +	u64 *cell_data;
>>>>    	info->data = of_device_get_match_data(&pdev->dev);
>>>>    	if (!info->data)
>>>> @@ -575,6 +586,39 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>>>>    	if (IS_ERR(base))
>>>>    		return PTR_ERR(base);
>>>> +	info->has_calibration_data[0] = false;
>>>> +	info->has_calibration_data[1] = false;
>>>> +
>>>> +	if (!info->data->supports_nvmem)
>>>> +		goto no_nvmem;
>>>> +
>>>> +	cell = nvmem_cell_get(&pdev->dev, "calibration");
>>>> +	if (IS_ERR(cell)) {
>>>> +		if (PTR_ERR(cell) == -EPROBE_DEFER)
>>>> +			return PTR_ERR(cell);
>>>> +		goto no_nvmem;
>>>
>>> goto considered evil ? :)
>>
>> this was a suggestion from Jonatan in version one, to make the code better
>> readable.
> 
> Isn't
> 
> if (info->data->supports_nvmem && IS_ERR(cell = nvmem_cell_get()))
> 
> pretty much the same thing?
> 
> Maxime
> 
I would say :

if (info->data->supports_nvmem && !IS_ERR(cell = nvmem_cell_get())) is

the same.
This would require an else if statement like this:

else if (info->data->supports_nvmem && PTR_ERR(cell) == -EPROBE_DEFER)
		return PTR_ERR(cell);

to avoid errors if the thermal sensor is probed before the sid driver.

Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data

[kbuild test robot]

Hi Philipp,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
>> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32
   drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32

vim +608 drivers/iio/adc/sun4i-gpadc-iio.c

   564	
   565	static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
   566					struct iio_dev *indio_dev)
   567	{
   568		struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
   569		struct resource *mem;
   570		void __iomem *base;
   571		int ret;
   572		struct nvmem_cell *cell;
   573		ssize_t cell_size;
   574		u64 *cell_data;
   575	
   576		info->data = of_device_get_match_data(&pdev->dev);
   577		if (!info->data)
   578			return -ENODEV;
   579	
   580		info->no_irq = true;
   581		indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
   582		indio_dev->channels = sun8i_a33_gpadc_channels;
   583	
   584		mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   585		base = devm_ioremap_resource(&pdev->dev, mem);
   586		if (IS_ERR(base))
   587			return PTR_ERR(base);
   588	
   589		info->has_calibration_data[0] = false;
   590		info->has_calibration_data[1] = false;
   591	
   592		if (!info->data->supports_nvmem)
   593			goto no_nvmem;
   594	
   595		cell = nvmem_cell_get(&pdev->dev, "calibration");
   596		if (IS_ERR(cell)) {
   597			if (PTR_ERR(cell) == -EPROBE_DEFER)
   598				return PTR_ERR(cell);
   599			goto no_nvmem;
   600		}
   601	
   602		cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
   603		nvmem_cell_put(cell);
   604		switch (cell_size) {
   605			case 8:
   606			case 6:
   607				info->has_calibration_data[1] = true;
 > 608				info->calibration_data[1] = be32_to_cpu(
   609						upper_32_bits(cell_data[0]));
   610			case 4:
   611			case 2:
   612				info->has_calibration_data[0] = true;
   613				info->calibration_data[0] = be32_to_cpu(
   614						lower_32_bits(cell_data[0]));
   615				break;
   616			default:
   617				break;
   618		}
   619	
   620	no_nvmem:
   621	
   622		info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
   623						     &sun4i_gpadc_regmap_config);
   624		if (IS_ERR(info->regmap)) {
   625			ret = PTR_ERR(info->regmap);
   626			dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
   627			return ret;
   628		}
   629	
   630		if (info->data->has_bus_rst) {
   631			info->reset = devm_reset_control_get(&pdev->dev, NULL);
   632			if (IS_ERR(info->reset)) {
   633				ret = PTR_ERR(info->reset);
   634				return ret;
   635			}
   636	
   637			ret = reset_control_deassert(info->reset);
   638			if (ret)
   639				return ret;
   640		}
   641	
   642		if (info->data->has_bus_clk) {
   643			info->bus_clk = devm_clk_get(&pdev->dev, "bus");
   644			if (IS_ERR(info->bus_clk)) {
   645				ret = PTR_ERR(info->bus_clk);
   646				goto assert_reset;
   647			}
   648	
   649			ret = clk_prepare_enable(info->bus_clk);
   650			if (ret)
   651				goto assert_reset;
   652		}
   653	
   654		if (info->data->has_mod_clk) {
   655			info->mod_clk = devm_clk_get(&pdev->dev, "mod");
   656			if (IS_ERR(info->mod_clk)) {
   657				ret = PTR_ERR(info->mod_clk);
   658				goto disable_bus_clk;
   659			}
   660	
   661			/* Running at 6MHz */
   662			ret = clk_set_rate(info->mod_clk, 4000000);
   663			if (ret)
   664				goto disable_bus_clk;
   665	
   666			ret = clk_prepare_enable(info->mod_clk);
   667			if (ret)
   668				goto disable_bus_clk;
   669		}
   670	
   671		if (IS_ENABLED(CONFIG_THERMAL_OF))
   672			info->sensor_device = &pdev->dev;
   673	
   674		return 0;
   675	
   676	disable_bus_clk:
   677		clk_disable_unprepare(info->bus_clk);
   678	
   679	assert_reset:
   680		reset_control_assert(info->reset);
   681	
   682		return ret;
   683	}
   684	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH v2 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

[Philipp Rossak]

This patch adds support for the H3 ths sensor.

The H3 supports interrupts. The interrupt is configured to update the
the sensor values every second. The calibration data is writen at the
begin of the init process.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
 2 files changed, 108 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index b7b5451226b0..8196203d65fe 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
 static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
 static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
+
 struct gpadc_data {
 	int		temp_offset;
 	int		temp_scale;
@@ -71,6 +74,10 @@ struct gpadc_data {
 	unsigned int	temp_data[MAX_SENSOR_COUNT];
 	int		(*sample_start)(struct sun4i_gpadc_iio *info);
 	int		(*sample_end)(struct sun4i_gpadc_iio *info);
+	u32		ctrl0_map;
+	u32		ctrl2_map;
+	u32		sensor_en_map;
+	u32		filter_map;
 	u32		irq_clear_map;
 	u32		irq_control_map;
 	bool		has_bus_clk;
@@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.support_irq = false,
 };
 
+static const struct gpadc_data sun8i_h3_ths_data = {
+	.temp_offset = -1791,
+	.temp_scale = -121,
+	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
+	.sample_start = sunxi_ths_sample_start,
+	.sample_end = sunxi_ths_sample_end,
+	.has_bus_clk = true,
+	.has_bus_rst = true,
+	.has_mod_clk = true,
+	.sensor_count = 1,
+	.supports_nvmem = true,
+	.support_irq = true,
+	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
+	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
+	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
+	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+			SUN8I_H3_THS_INTS_SHUT_INT_0   |
+			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
+			SUN8I_H3_THS_INTS_ALARM_OFF_0,
+	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+		SUN8I_H3_THS_TEMP_PERIOD(0x7),
+};
+
 struct sun4i_gpadc_iio {
 	struct iio_dev			*indio_dev;
 	struct completion		completion;
@@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
 	return 0;
 }
 
+static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
+{
+	/* Disable ths interrupt */
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
+	/* Disable temperature sensor */
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
+
+	return 0;
+}
+
 static int sun4i_gpadc_runtime_suspend(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
 	return info->data->sample_end(info);
 }
 
+static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
+{
+	if (info->has_calibration_data[0])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
+			info->calibration_data[0]);
+
+	if (info->has_calibration_data[1])
+		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
+			info->calibration_data[1]);
+}
+
 static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
 {
 	/* clkin = 6MHz */
@@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
 	return 0;
 }
 
+static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
+{
+	u32 value;
+	sunxi_calibrate(info);
+
+	if (info->data->ctrl0_map)
+		regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
+			info->data->ctrl0_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+		info->data->ctrl2_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_STAT,
+			info->data->irq_clear_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
+		info->data->filter_map);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_INTC,
+		info->data->irq_control_map);
+
+	regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
+
+	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
+		info->data->sensor_en_map | value);
+
+	return 0;
+}
+
 static int sun4i_gpadc_runtime_resume(struct device *dev)
 {
 	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
@@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
 		.compatible = "allwinner,sun8i-a33-ths",
 		.data = &sun8i_a33_gpadc_data,
 	},
+	{
+		.compatible = "allwinner,sun8i-h3-ths",
+		.data = &sun8i_h3_ths_data,
+	},
 	{ /* sentinel */ }
 };
 
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 458f2159a95a..80b79c31cea3 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -91,7 +91,29 @@
 #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
 
 /* SUNXI_THS COMMON REGISTERS + DEFINES */
+#define SUN8I_H3_THS_CTRL0				0x00
+#define SUN8I_H3_THS_CTRL2				0x40
 #define SUN8I_H3_THS_INTC				0x44
+#define SUN8I_H3_THS_STAT				0x48
+#define SUN8I_H3_THS_FILTER				0x70
+#define SUNXI_THS_CDATA_0_1				0x74
+#define SUNXI_THS_CDATA_2_3				0x78
+#define SUN8I_H3_THS_TDATA0				0x80
+
+#define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
+
+#define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
+
+#define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))
+
+#define SUN8I_H3_THS_INTS_ALARM_INT_0			BIT(0)
+#define SUN8I_H3_THS_INTS_SHUT_INT_0			BIT(4)
+#define SUN8I_H3_THS_INTS_TDATA_IRQ_0			BIT(8)
+#define SUN8I_H3_THS_INTS_ALARM_OFF_0			BIT(12)
+
+#define SUN8I_H3_THS_INTC_ALARM_INT_EN0			BIT(0)
+#define SUN8I_H3_THS_INTC_SHUT_INT_EN0			BIT(4)
+#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0			BIT(8)
 
 #define MAX_SENSOR_COUNT				4
 
-- 
2.11.0

Re: [PATCH v2 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4206 bytes --]

On Mon, Jan 29, 2018 at 12:29:12AM +0100, Philipp Rossak wrote:
> This patch adds support for the H3 ths sensor.
> 
> The H3 supports interrupts. The interrupt is configured to update the
> the sensor values every second. The calibration data is writen at the
> begin of the init process.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
>  2 files changed, 108 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index b7b5451226b0..8196203d65fe 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
>  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>  static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>  
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
> +
>  struct gpadc_data {
>  	int		temp_offset;
>  	int		temp_scale;
> @@ -71,6 +74,10 @@ struct gpadc_data {
>  	unsigned int	temp_data[MAX_SENSOR_COUNT];
>  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
>  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
> +	u32		ctrl0_map;
> +	u32		ctrl2_map;
> +	u32		sensor_en_map;
> +	u32		filter_map;

You can use a regmap_field for that.

>  	u32		irq_clear_map;
>  	u32		irq_control_map;
>  	bool		has_bus_clk;
> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>  	.support_irq = false,
>  };
>  
> +static const struct gpadc_data sun8i_h3_ths_data = {
> +	.temp_offset = -1791,
> +	.temp_scale = -121,
> +	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
> +	.sample_start = sunxi_ths_sample_start,
> +	.sample_end = sunxi_ths_sample_end,
> +	.has_bus_clk = true,
> +	.has_bus_rst = true,
> +	.has_mod_clk = true,
> +	.sensor_count = 1,
> +	.supports_nvmem = true,
> +	.support_irq = true,
> +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
> +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
> +	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
> +	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> +		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> +	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> +			SUN8I_H3_THS_INTS_SHUT_INT_0   |
> +			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
> +			SUN8I_H3_THS_INTS_ALARM_OFF_0,
> +	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> +		SUN8I_H3_THS_TEMP_PERIOD(0x7),
> +};
> +
>  struct sun4i_gpadc_iio {
>  	struct iio_dev			*indio_dev;
>  	struct completion		completion;
> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
>  	return 0;
>  }
>  
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
> +{
> +	/* Disable ths interrupt */
> +	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
> +	/* Disable temperature sensor */
> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
> +
> +	return 0;
> +}
> +
>  static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  	return info->data->sample_end(info);
>  }
>  
> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
> +{
> +	if (info->has_calibration_data[0])
> +		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> +			info->calibration_data[0]);
> +
> +	if (info->has_calibration_data[1])
> +		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> +			info->calibration_data[1]);
> +}
> +
>  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>  {
>  	/* clkin = 6MHz */
> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>  	return 0;
>  }
>  
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)

Please remain consistent with the prefixes used in the driver

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

[Quentin Schulz]

[-- Attachment #1: Type: text/plain, Size: 7277 bytes --]

Hi Philipp,

On Mon, Jan 29, 2018 at 12:29:12AM +0100, Philipp Rossak wrote:
> This patch adds support for the H3 ths sensor.
> 
> The H3 supports interrupts. The interrupt is configured to update the
> the sensor values every second. The calibration data is writen at the
> begin of the init process.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
>  2 files changed, 108 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index b7b5451226b0..8196203d65fe 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
>  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>  static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>  
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
> +

We try to avoid using the generic sunxi prefix.

>  struct gpadc_data {
>  	int		temp_offset;
>  	int		temp_scale;
> @@ -71,6 +74,10 @@ struct gpadc_data {
>  	unsigned int	temp_data[MAX_SENSOR_COUNT];
>  	int		(*sample_start)(struct sun4i_gpadc_iio *info);
>  	int		(*sample_end)(struct sun4i_gpadc_iio *info);
> +	u32		ctrl0_map;
> +	u32		ctrl2_map;
> +	u32		sensor_en_map;
> +	u32		filter_map;
>  	u32		irq_clear_map;
>  	u32		irq_control_map;
>  	bool		has_bus_clk;
> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>  	.support_irq = false,
>  };
>  
> +static const struct gpadc_data sun8i_h3_ths_data = {
> +	.temp_offset = -1791,
> +	.temp_scale = -121,
> +	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
> +	.sample_start = sunxi_ths_sample_start,
> +	.sample_end = sunxi_ths_sample_end,
> +	.has_bus_clk = true,
> +	.has_bus_rst = true,
> +	.has_mod_clk = true,
> +	.sensor_count = 1,
> +	.supports_nvmem = true,
> +	.support_irq = true,
> +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
> +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
> +	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
> +	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> +		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> +	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> +			SUN8I_H3_THS_INTS_SHUT_INT_0   |
> +			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
> +			SUN8I_H3_THS_INTS_ALARM_OFF_0,
> +	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
> +		SUN8I_H3_THS_TEMP_PERIOD(0x7),

>From what I've understood, ACQ regs are basically clock dividers. We
should make a better job at explaining it :)

> +};
> +
>  struct sun4i_gpadc_iio {
>  	struct iio_dev			*indio_dev;
>  	struct completion		completion;
> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
>  	return 0;
>  }
>  
> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
> +{
> +	/* Disable ths interrupt */
> +	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
> +	/* Disable temperature sensor */
> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
> +
> +	return 0;
> +}
> +
>  static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  	return info->data->sample_end(info);
>  }
>  
> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
> +{
> +	if (info->has_calibration_data[0])
> +		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
> +			info->calibration_data[0]);
> +
> +	if (info->has_calibration_data[1])
> +		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
> +			info->calibration_data[1]);
> +}
> +
>  static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>  {
>  	/* clkin = 6MHz */
> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>  	return 0;
>  }
>  
> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
> +{
> +	u32 value;
> +	sunxi_calibrate(info);
> +
> +	if (info->data->ctrl0_map)
> +		regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
> +			info->data->ctrl0_map);
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> +		info->data->ctrl2_map);
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_STAT,
> +			info->data->irq_clear_map);
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
> +		info->data->filter_map);
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_INTC,
> +		info->data->irq_control_map);
> +
> +	regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
> +
> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
> +		info->data->sensor_en_map | value);
> +
> +	return 0;
> +}
> +

I'm a bit worried. Will all the sensors have the same sample start? Or
will we need at some point also entries in info->data for register
addresses, if they have ctrl2 or filter, etc...

Maybe we could define a sample_start for the h3 only and reuse-it if
possible and if not declare another sample start? All depends if the
sample start is most likely to change in the near future or not. If it
is, then we should avoid adding a lot of variables in info->data and
just go for a single sample_start per SoC.

>  static int sun4i_gpadc_runtime_resume(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
> @@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
>  		.compatible = "allwinner,sun8i-a33-ths",
>  		.data = &sun8i_a33_gpadc_data,
>  	},
> +	{
> +		.compatible = "allwinner,sun8i-h3-ths",
> +		.data = &sun8i_h3_ths_data,
> +	},
>  	{ /* sentinel */ }
>  };
>  
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 458f2159a95a..80b79c31cea3 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -91,7 +91,29 @@
>  #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
>  
>  /* SUNXI_THS COMMON REGISTERS + DEFINES */
> +#define SUN8I_H3_THS_CTRL0				0x00
> +#define SUN8I_H3_THS_CTRL2				0x40
>  #define SUN8I_H3_THS_INTC				0x44
> +#define SUN8I_H3_THS_STAT				0x48
> +#define SUN8I_H3_THS_FILTER				0x70
> +#define SUNXI_THS_CDATA_0_1				0x74
> +#define SUNXI_THS_CDATA_2_3				0x78
> +#define SUN8I_H3_THS_TDATA0				0x80
> +
> +#define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
> +
> +#define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
> +
> +#define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))
> +
> +#define SUN8I_H3_THS_INTS_ALARM_INT_0			BIT(0)
> +#define SUN8I_H3_THS_INTS_SHUT_INT_0			BIT(4)
> +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0			BIT(8)
> +#define SUN8I_H3_THS_INTS_ALARM_OFF_0			BIT(12)
> +
> +#define SUN8I_H3_THS_INTC_ALARM_INT_EN0			BIT(0)
> +#define SUN8I_H3_THS_INTC_SHUT_INT_EN0			BIT(4)
> +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0			BIT(8)
>  

Personal taste here but I prefer the register bits to be defined just
below the register address define.

i.e.:

#define SUN8I_H3_THS_CTRL2                           0x40
#define SUN8I_H3_THS_ACQ1(x)                         (GENMASK(31, 16) & ((x) << 16))
#define SUN8I_H3_THS_TEMP_SENSE_EN0                  BIT(0)

that way we know for which register we should use which constants.

Quentin

Re: [PATCH v2 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

[Philipp Rossak]

On 31.01.2018 20:23, Quentin Schulz wrote:
> Hi Philipp,
> 
> On Mon, Jan 29, 2018 at 12:29:12AM +0100, Philipp Rossak wrote:
>> This patch adds support for the H3 ths sensor.
>>
>> The H3 supports interrupts. The interrupt is configured to update the
>> the sensor values every second. The calibration data is writen at the
>> begin of the init process.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/iio/adc/sun4i-gpadc-iio.c | 86 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/sun4i-gpadc.h   | 22 ++++++++++
>>   2 files changed, 108 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index b7b5451226b0..8196203d65fe 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio;
>>   static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info);
>>   static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info);
>>   
>> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info);
>> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info);
>> +
> 
> We try to avoid using the generic sunxi prefix.
> 
>>   struct gpadc_data {
>>   	int		temp_offset;
>>   	int		temp_scale;
>> @@ -71,6 +74,10 @@ struct gpadc_data {
>>   	unsigned int	temp_data[MAX_SENSOR_COUNT];
>>   	int		(*sample_start)(struct sun4i_gpadc_iio *info);
>>   	int		(*sample_end)(struct sun4i_gpadc_iio *info);
>> +	u32		ctrl0_map;
>> +	u32		ctrl2_map;
>> +	u32		sensor_en_map;
>> +	u32		filter_map;
>>   	u32		irq_clear_map;
>>   	u32		irq_control_map;
>>   	bool		has_bus_clk;
>> @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = {
>>   	.support_irq = false,
>>   };
>>   
>> +static const struct gpadc_data sun8i_h3_ths_data = {
>> +	.temp_offset = -1791,
>> +	.temp_scale = -121,
>> +	.temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0},
>> +	.sample_start = sunxi_ths_sample_start,
>> +	.sample_end = sunxi_ths_sample_end,
>> +	.has_bus_clk = true,
>> +	.has_bus_rst = true,
>> +	.has_mod_clk = true,
>> +	.sensor_count = 1,
>> +	.supports_nvmem = true,
>> +	.support_irq = true,
>> +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff),
>> +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f),
>> +	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0,
>> +	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
>> +		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
>> +	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
>> +			SUN8I_H3_THS_INTS_SHUT_INT_0   |
>> +			SUN8I_H3_THS_INTS_TDATA_IRQ_0  |
>> +			SUN8I_H3_THS_INTS_ALARM_OFF_0,
>> +	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
>> +		SUN8I_H3_THS_TEMP_PERIOD(0x7),
> 
>  From what I've understood, ACQ regs are basically clock dividers. We
> should make a better job at explaining it :)
> 

I agree, I will add this in the next version in the commit message.

>> +};
>> +
>>   struct sun4i_gpadc_iio {
>>   	struct iio_dev			*indio_dev;
>>   	struct completion		completion;
>> @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info)
>>   	return 0;
>>   }
>>   
>> +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info)
>> +{
>> +	/* Disable ths interrupt */
>> +	regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0);
>> +	/* Disable temperature sensor */
>> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0);
>> +
>> +	return 0;
>> +}
>> +
>>   static int sun4i_gpadc_runtime_suspend(struct device *dev)
>>   {
>>   	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>> @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>>   	return info->data->sample_end(info);
>>   }
>>   
>> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
>> +{
>> +	if (info->has_calibration_data[0])
>> +		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
>> +			info->calibration_data[0]);
>> +
>> +	if (info->has_calibration_data[1])
>> +		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
>> +			info->calibration_data[1]);
>> +}
>> +
>>   static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>>   {
>>   	/* clkin = 6MHz */
>> @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>>   	return 0;
>>   }
>>   
>> +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
>> +{
>> +	u32 value;
>> +	sunxi_calibrate(info);
>> +
>> +	if (info->data->ctrl0_map)
>> +		regmap_write(info->regmap, SUN8I_H3_THS_CTRL0,
>> +			info->data->ctrl0_map);
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> +		info->data->ctrl2_map);
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_STAT,
>> +			info->data->irq_clear_map);
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_FILTER,
>> +		info->data->filter_map);
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_INTC,
>> +		info->data->irq_control_map);
>> +
>> +	regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value);
>> +
>> +	regmap_write(info->regmap, SUN8I_H3_THS_CTRL2,
>> +		info->data->sensor_en_map | value);
>> +
>> +	return 0;
>> +}
>> +
> 
> I'm a bit worried. Will all the sensors have the same sample start? Or
> will we need at some point also entries in info->data for register
> addresses, if they have ctrl2 or filter, etc...
>  > Maybe we could define a sample_start for the h3 only and reuse-it if
> possible and if not declare another sample start? All depends if the
> sample start is most likely to change in the near future or not. If it
> is, then we should avoid adding a lot of variables in info->data and
> just go for a single sample_start per SoC.
> 

for H3, A83T, H5, A64 we can use the same sample start. So I would use 
here a H3 sample start function and reuse it.

A80 is special since it has no crtl0 register, thus it would get also 
its own function.

H6 will need also an own sample start function (looking in the near future).

>>   static int sun4i_gpadc_runtime_resume(struct device *dev)
>>   {
>>   	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>> @@ -582,6 +664,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
>>   		.compatible = "allwinner,sun8i-a33-ths",
>>   		.data = &sun8i_a33_gpadc_data,
>>   	},
>> +	{
>> +		.compatible = "allwinner,sun8i-h3-ths",
>> +		.data = &sun8i_h3_ths_data,
>> +	},
>>   	{ /* sentinel */ }
>>   };
>>   
>> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
>> index 458f2159a95a..80b79c31cea3 100644
>> --- a/include/linux/mfd/sun4i-gpadc.h
>> +++ b/include/linux/mfd/sun4i-gpadc.h
>> @@ -91,7 +91,29 @@
>>   #define SUN4I_GPADC_AUTOSUSPEND_DELAY			10000
>>   
>>   /* SUNXI_THS COMMON REGISTERS + DEFINES */
>> +#define SUN8I_H3_THS_CTRL0				0x00
>> +#define SUN8I_H3_THS_CTRL2				0x40
>>   #define SUN8I_H3_THS_INTC				0x44
>> +#define SUN8I_H3_THS_STAT				0x48
>> +#define SUN8I_H3_THS_FILTER				0x70
>> +#define SUNXI_THS_CDATA_0_1				0x74
>> +#define SUNXI_THS_CDATA_2_3				0x78
>> +#define SUN8I_H3_THS_TDATA0				0x80
>> +
>> +#define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
>> +
>> +#define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
>> +
>> +#define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))
>> +
>> +#define SUN8I_H3_THS_INTS_ALARM_INT_0			BIT(0)
>> +#define SUN8I_H3_THS_INTS_SHUT_INT_0			BIT(4)
>> +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0			BIT(8)
>> +#define SUN8I_H3_THS_INTS_ALARM_OFF_0			BIT(12)
>> +
>> +#define SUN8I_H3_THS_INTC_ALARM_INT_EN0			BIT(0)
>> +#define SUN8I_H3_THS_INTC_SHUT_INT_EN0			BIT(4)
>> +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0			BIT(8)
>>   
> 
> Personal taste here but I prefer the register bits to be defined just
> below the register address define.
> 
> i.e.:
> 
> #define SUN8I_H3_THS_CTRL2                           0x40
> #define SUN8I_H3_THS_ACQ1(x)                         (GENMASK(31, 16) & ((x) << 16))
> #define SUN8I_H3_THS_TEMP_SENSE_EN0                  BIT(0)
> 
> that way we know for which register we should use which constants.
> 
> Quentin
> 

I agree, this the better way to do it.


Thanks,
Philipp

[PATCH v2 10/16] iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor

[Philipp Rossak]

This patch adds support for the A83T ths sensor.

The A83T supports interrupts. The interrupt is configured to update the
the sensor values every second.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sun4i-gpadc.h   | 18 ++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 8196203d65fe..9f7895ba1966 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
 		SUN8I_H3_THS_TEMP_PERIOD(0x7),
 };
 
+static const struct gpadc_data sun8i_a83t_ths_data = {
+	.temp_offset = -2724,
+	.temp_scale = -70,
+	.temp_data = {SUN8I_H3_THS_TDATA0,
+		SUN8I_A83T_THS_TDATA1,
+		SUN8I_A83T_THS_TDATA2,
+		0},
+	.sample_start = sunxi_ths_sample_start,
+	.sample_end = sunxi_ths_sample_end,
+	.sensor_count = 3,
+	.supports_nvmem = false,
+	.support_irq = true,
+	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
+	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
+	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
+		SUN8I_A83T_THS_TEMP_SENSE_EN1 |
+		SUN8I_A83T_THS_TEMP_SENSE_EN2,
+	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
+		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
+	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
+		SUN8I_A83T_THS_INTS_ALARM_INT_1 |
+		SUN8I_A83T_THS_INTS_ALARM_INT_2 |
+		SUN8I_H3_THS_INTS_SHUT_INT_0  |
+		SUN8I_A83T_THS_INTS_SHUT_INT_1  |
+		SUN8I_A83T_THS_INTS_SHUT_INT_2  |
+		SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
+		SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
+		SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
+	.irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 |
+		SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1 |
+		SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2 |
+		SUN8I_H3_THS_TEMP_PERIOD(0x257),
+};
+
 struct sun4i_gpadc_iio {
 	struct iio_dev			*indio_dev;
 	struct completion		completion;
@@ -668,6 +702,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
 		.compatible = "allwinner,sun8i-h3-ths",
 		.data = &sun8i_h3_ths_data,
 	},
+	{
+		.compatible = "allwinner,sun8i-a83t-ths",
+		.data = &sun8i_a83t_ths_data,
+	},
 	{ /* sentinel */ }
 };
 
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 80b79c31cea3..32f15cc03363 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -99,21 +99,39 @@
 #define SUNXI_THS_CDATA_0_1				0x74
 #define SUNXI_THS_CDATA_2_3				0x78
 #define SUN8I_H3_THS_TDATA0				0x80
+#define SUN8I_A83T_THS_TDATA1				0x84
+#define SUN8I_A83T_THS_TDATA2				0x88
 
 #define SUN8I_H3_THS_ACQ1(x)				(GENMASK(31, 16) & ((x) << 16))
 
 #define SUN8I_H3_THS_TEMP_SENSE_EN0			BIT(0)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN1			BIT(1)
+#define SUN8I_A83T_THS_TEMP_SENSE_EN2			BIT(2)
 
 #define SUN8I_H3_THS_TEMP_PERIOD(x)			(GENMASK(31, 12) & ((x) << 12))
 
 #define SUN8I_H3_THS_INTS_ALARM_INT_0			BIT(0)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_1			BIT(1)
+#define SUN8I_A83T_THS_INTS_ALARM_INT_2			BIT(2)
 #define SUN8I_H3_THS_INTS_SHUT_INT_0			BIT(4)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_1			BIT(5)
+#define SUN8I_A83T_THS_INTS_SHUT_INT_2			BIT(6)
 #define SUN8I_H3_THS_INTS_TDATA_IRQ_0			BIT(8)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_1			BIT(9)
+#define SUN8I_A83T_THS_INTS_TDATA_IRQ_2			BIT(10)
 #define SUN8I_H3_THS_INTS_ALARM_OFF_0			BIT(12)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_1			BIT(13)
+#define SUN8I_A83T_THS_INTS_ALARM_OFF_2			BIT(14)
 
 #define SUN8I_H3_THS_INTC_ALARM_INT_EN0			BIT(0)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN1		BIT(1)
+#define SUN8I_A83T_THS_INTC_ALARM_INT_EN2		BIT(2)
 #define SUN8I_H3_THS_INTC_SHUT_INT_EN0			BIT(4)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN1		BIT(5)
+#define SUN8I_A83T_THS_INTC_SHUT_INT_EN2		BIT(6)
 #define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0			BIT(8)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN1		BIT(9)
+#define SUN8I_A83T_THS_INTC_TDATA_IRQ_EN2		BIT(10)
 
 #define MAX_SENSOR_COUNT				4
 
-- 
2.11.0

Re: [PATCH v2 10/16] iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
> This patch adds support for the A83T ths sensor.
> 
> The A83T supports interrupts. The interrupt is configured to update the
> the sensor values every second.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/sun4i-gpadc.h   | 18 ++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 8196203d65fe..9f7895ba1966 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
>  		SUN8I_H3_THS_TEMP_PERIOD(0x7),
>  };
>  
> +static const struct gpadc_data sun8i_a83t_ths_data = {
> +	.temp_offset = -2724,
> +	.temp_scale = -70,
> +	.temp_data = {SUN8I_H3_THS_TDATA0,
> +		SUN8I_A83T_THS_TDATA1,
> +		SUN8I_A83T_THS_TDATA2,
> +		0},
> +	.sample_start = sunxi_ths_sample_start,
> +	.sample_end = sunxi_ths_sample_end,
> +	.sensor_count = 3,
> +	.supports_nvmem = false,
> +	.support_irq = true,
> +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
> +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),

Where are these values coming from?

> +	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
> +		SUN8I_A83T_THS_TEMP_SENSE_EN1 |
> +		SUN8I_A83T_THS_TEMP_SENSE_EN2,
> +	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
> +		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
> +	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
> +		SUN8I_A83T_THS_INTS_ALARM_INT_1 |
> +		SUN8I_A83T_THS_INTS_ALARM_INT_2 |
> +		SUN8I_H3_THS_INTS_SHUT_INT_0  |
> +		SUN8I_A83T_THS_INTS_SHUT_INT_1  |
> +		SUN8I_A83T_THS_INTS_SHUT_INT_2  |
> +		SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
> +		SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
> +		SUN8I_A83T_THS_INTS_TDATA_IRQ_2,

Do you reall need to clear all these interrupts if you're using only
one?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 10/16] iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor

[Philipp Rossak]

On 29.01.2018 10:48, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
>> This patch adds support for the A83T ths sensor.
>>
>> The A83T supports interrupts. The interrupt is configured to update the
>> the sensor values every second.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/sun4i-gpadc.h   | 18 ++++++++++++++++++
>>   2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 8196203d65fe..9f7895ba1966 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
>>   		SUN8I_H3_THS_TEMP_PERIOD(0x7),
>>   };
>>   
>> +static const struct gpadc_data sun8i_a83t_ths_data = {
>> +	.temp_offset = -2724,
>> +	.temp_scale = -70,
>> +	.temp_data = {SUN8I_H3_THS_TDATA0,
>> +		SUN8I_A83T_THS_TDATA1,
>> +		SUN8I_A83T_THS_TDATA2,
>> +		0},
>> +	.sample_start = sunxi_ths_sample_start,
>> +	.sample_end = sunxi_ths_sample_end,
>> +	.sensor_count = 3,
>> +	.supports_nvmem = false,
>> +	.support_irq = true,
>> +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
>> +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
> 
> Where are these values coming from?
> 

These values are calculated with the formulas from the datasheet and 
also tested on hardware. These settings seem ok.

>> +	.sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0 |
>> +		SUN8I_A83T_THS_TEMP_SENSE_EN1 |
>> +		SUN8I_A83T_THS_TEMP_SENSE_EN2,
>> +	.filter_map = SUN4I_GPADC_CTRL3_FILTER_EN |
>> +		SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2),
>> +	.irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 |
>> +		SUN8I_A83T_THS_INTS_ALARM_INT_1 |
>> +		SUN8I_A83T_THS_INTS_ALARM_INT_2 |
>> +		SUN8I_H3_THS_INTS_SHUT_INT_0  |
>> +		SUN8I_A83T_THS_INTS_SHUT_INT_1  |
>> +		SUN8I_A83T_THS_INTS_SHUT_INT_2  |
>> +		SUN8I_H3_THS_INTS_TDATA_IRQ_0 |
>> +		SUN8I_A83T_THS_INTS_TDATA_IRQ_1 |
>> +		SUN8I_A83T_THS_INTS_TDATA_IRQ_2,
> 
> Do you reall need to clear all these interrupts if you're using only
> one?
> 

No, I don't think so, I will remove them in the next version.

> Maxime
> 

Philipp

Re: [PATCH v2 10/16] iio: adc: sun4i-gpadc-iio: add support for A83T thermal sensor

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1933 bytes --]

1;5002;0c
On Mon, Jan 29, 2018 at 12:53:48PM +0100, Philipp Rossak wrote:
> 
> 
> On 29.01.2018 10:48, Maxime Ripard wrote:
> > On Mon, Jan 29, 2018 at 12:29:13AM +0100, Philipp Rossak wrote:
> > > This patch adds support for the A83T ths sensor.
> > > 
> > > The A83T supports interrupts. The interrupt is configured to update the
> > > the sensor values every second.
> > > 
> > > Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> > > ---
> > >   drivers/iio/adc/sun4i-gpadc-iio.c | 38 ++++++++++++++++++++++++++++++++++++++
> > >   include/linux/mfd/sun4i-gpadc.h   | 18 ++++++++++++++++++
> > >   2 files changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> > > index 8196203d65fe..9f7895ba1966 100644
> > > --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> > > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> > > @@ -170,6 +170,40 @@ static const struct gpadc_data sun8i_h3_ths_data = {
> > >   		SUN8I_H3_THS_TEMP_PERIOD(0x7),
> > >   };
> > > +static const struct gpadc_data sun8i_a83t_ths_data = {
> > > +	.temp_offset = -2724,
> > > +	.temp_scale = -70,
> > > +	.temp_data = {SUN8I_H3_THS_TDATA0,
> > > +		SUN8I_A83T_THS_TDATA1,
> > > +		SUN8I_A83T_THS_TDATA2,
> > > +		0},
> > > +	.sample_start = sunxi_ths_sample_start,
> > > +	.sample_end = sunxi_ths_sample_end,
> > > +	.sensor_count = 3,
> > > +	.supports_nvmem = false,
> > > +	.support_irq = true,
> > > +	.ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0x1f3),
> > > +	.ctrl2_map = SUN8I_H3_THS_ACQ1(0x1f3),
> > 
> > Where are these values coming from?
> > 
> 
> These values are calculated with the formulas from the datasheet and also
> tested on hardware. These settings seem ok.

You should at least put a comment explaining how you got to these values.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 11/16] arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5

[Philipp Rossak]

As we have gained the support for the thermal sensor in H3 and H5,
we can now add its device nodes to the device tree. The H3 and H5 share
most of its compatible. The compatible and the thermal sensor cells
will be added in an additional patch per device.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 7a83b15225c7..413c789b588d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -426,6 +426,15 @@
 			};
 		};
 
+		ths: thermal-sensor@1c25000 {
+			reg = <0x01c25000 0x100>;
+			interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_THS>;
+			#io-channel-cells = <0>;
+		};
+
 		timer@1c20c00 {
 			compatible = "allwinner,sun4i-a10-timer";
 			reg = <0x01c20c00 0xa0>;
-- 
2.11.0

Re: [PATCH v2 11/16] arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Hi,

On Mon, Jan 29, 2018 at 12:29:14AM +0100, Philipp Rossak wrote:
> As we have gained the support for the thermal sensor in H3 and H5,
> we can now add its device nodes to the device tree. The H3 and H5 share
> most of its compatible. The compatible and the thermal sensor cells
> will be added in an additional patch per device.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 7a83b15225c7..413c789b588d 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -426,6 +426,15 @@
>  			};
>  		};
>  
> +		ths: thermal-sensor@1c25000 {
> +			reg = <0x01c25000 0x100>;

The size is 0x400

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 11/16] arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5

[Philipp Rossak]

On 29.01.2018 10:49, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 29, 2018 at 12:29:14AM +0100, Philipp Rossak wrote:
>> As we have gained the support for the thermal sensor in H3 and H5,
>> we can now add its device nodes to the device tree. The H3 and H5 share
>> most of its compatible. The compatible and the thermal sensor cells
>> will be added in an additional patch per device.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/sunxi-h3-h5.dtsi | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> index 7a83b15225c7..413c789b588d 100644
>> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> @@ -426,6 +426,15 @@
>>   			};
>>   		};
>>   
>> +		ths: thermal-sensor@1c25000 {
>> +			reg = <0x01c25000 0x100>;
> 
> The size is 0x400
> 
> Maxime
> 

Ok, I will fix this.

Philipp

[PATCH v2 12/16] arm: dts: sun8i: h3: add support for the thermal sensor in H3

[Philipp Rossak]

This patch adds the missing compatible and the thermal sensor cells.
The H3 has one sensor.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 8495deecedad..fbb007e5798e 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -128,3 +128,8 @@
 &pio {
 	compatible = "allwinner,sun8i-h3-pinctrl";
 };
+
+&ths {
+	compatible = "allwinner,sun8i-h3-ths";
+	#thermal-sensor-cells = <0>;
+};
-- 
2.11.0

[PATCH v2 13/16] arm: dts: sun8i: h3: add thermal zone to H3

[Philipp Rossak]

This patch adds the thermal zones to the H3. We have only one sensor and
that is placed in the cpu.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fbb007e5798e..3f83f6a27c74 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,15 @@
 		};
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			/* milliseconds */
+			polling-delay-passive = <250>;
+			polling-delay = <1000>;
+			thermal-sensors = <&ths 0>;
+		};
+	};
+
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
-- 
2.11.0

Re: [PATCH v2 13/16] arm: dts: sun8i: h3: add thermal zone to H3

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Mon, Jan 29, 2018 at 12:29:16AM +0100, Philipp Rossak wrote:
> This patch adds the thermal zones to the H3. We have only one sensor and
> that is placed in the cpu.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index fbb007e5798e..3f83f6a27c74 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -72,6 +72,15 @@
>  		};
>  	};
>  
> +	thermal-zones {
> +		cpu-thermal {
> +			/* milliseconds */
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +			thermal-sensors = <&ths 0>;

if the thermal-sensor-cells value is indeed 0, the phandle parsing
will be broken here.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 13/16] arm: dts: sun8i: h3: add thermal zone to H3

[Philipp Rossak]

On 29.01.2018 10:50, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:16AM +0100, Philipp Rossak wrote:
>> This patch adds the thermal zones to the H3. We have only one sensor and
>> that is placed in the cpu.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/sun8i-h3.dtsi | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index fbb007e5798e..3f83f6a27c74 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -72,6 +72,15 @@
>>   		};
>>   	};
>>   
>> +	thermal-zones {
>> +		cpu-thermal {
>> +			/* milliseconds */
>> +			polling-delay-passive = <250>;
>> +			polling-delay = <1000>;
>> +			thermal-sensors = <&ths 0>;
> 
> if the thermal-sensor-cells value is indeed 0, the phandle parsing
> will be broken here.
> 
> Maxime
> 

Ok, then I will remove the 0.

Thanks,

Philipp

[PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Philipp Rossak]

This patch enables the the sid controller in the H3. It can be used
for thermal calibration data.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 3f83f6a27c74..9bb5cc29fec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -72,6 +72,13 @@
 		};
 	};
 
+	soc {
+		sid: eeprom@1c14000 {
+			compatible = "allwinner,sun8i-h3-sid";
+			reg = <0x01c14000 0x400>;
+		};
+	};
+
 	thermal-zones {
 		cpu-thermal {
 			/* milliseconds */
-- 
2.11.0

Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> This patch enables the the sid controller in the H3. It can be used
> for thermal calibration data.
> 
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 3f83f6a27c74..9bb5cc29fec5 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -72,6 +72,13 @@
>  		};
>  	};
>  
> +	soc {
> +		sid: eeprom@1c14000 {
> +			compatible = "allwinner,sun8i-h3-sid";
> +			reg = <0x01c14000 0x400>;
> +		};
> +	};
> +

Shouldn't you also use a nvmem-cells property to the THS node?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Philipp Rossak]

On 29.01.2018 10:52, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>> This patch enables the the sid controller in the H3. It can be used
>> for thermal calibration data.
>>
>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index 3f83f6a27c74..9bb5cc29fec5 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -72,6 +72,13 @@
>>   		};
>>   	};
>>   
>> +	soc {
>> +		sid: eeprom@1c14000 {
>> +			compatible = "allwinner,sun8i-h3-sid";
>> +			reg = <0x01c14000 0x400>;
>> +		};
>> +	};
>> +
> 
> Shouldn't you also use a nvmem-cells property to the THS node?
> 
> Maxime
> 

Oh seems like I forgot that.
As related to the wiki [1] this should be 64 bit wide at the address 
0x34. I will add that in the next version.


[1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE

Thanks,
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Kyle Evans]

On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d@gmail.com> wrote:
>
>
> On 29.01.2018 10:52, Maxime Ripard wrote:
>>
>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>
>>> This patch enables the the sid controller in the H3. It can be used
>>> for thermal calibration data.
>>>
>>> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
>>> ---
>>>   arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>> b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> index 3f83f6a27c74..9bb5cc29fec5 100644
>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> @@ -72,6 +72,13 @@
>>>                 };
>>>         };
>>>   +     soc {
>>> +               sid: eeprom@1c14000 {
>>> +                       compatible = "allwinner,sun8i-h3-sid";
>>> +                       reg = <0x01c14000 0x400>;
>>> +               };
>>> +       };
>>> +
>>
>>
>> Shouldn't you also use a nvmem-cells property to the THS node?
>>
>> Maxime
>>
>
> Oh seems like I forgot that.
> As related to the wiki [1] this should be 64 bit wide at the address 0x34. I
> will add that in the next version.
>
>
> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>
> Thanks,
> Philipp
>

Hi,

Any chance this will see a v3 soon? I'm kind of interested in sid node
for h3. =)

Thanks,

Kyle Evans

Re: [linux-sunxi] Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Icenowy Zheng]

于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <kevans@freebsd.org> 写到:
>On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d@gmail.com>
>wrote:
>>
>>
>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>
>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>
>>>> This patch enables the the sid controller in the H3. It can be used
>>>> for thermal calibration data.
>>>>
>>>> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
>>>> ---
>>>>   arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> index 3f83f6a27c74..9bb5cc29fec5 100644
>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> @@ -72,6 +72,13 @@
>>>>                 };
>>>>         };
>>>>   +     soc {
>>>> +               sid: eeprom@1c14000 {
>>>> +                       compatible = "allwinner,sun8i-h3-sid";
>>>> +                       reg = <0x01c14000 0x400>;
>>>> +               };
>>>> +       };
>>>> +
>>>
>>>
>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>
>>> Maxime
>>>
>>
>> Oh seems like I forgot that.
>> As related to the wiki [1] this should be 64 bit wide at the address
>0x34. I
>> will add that in the next version.
>>
>>
>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>
>> Thanks,
>> Philipp
>>
>
>Hi,
>
>Any chance this will see a v3 soon? I'm kind of interested in sid node
>for h3. =)

This patch is independent and can be easily sent out
by its own.

>
>Thanks,
>
>Kyle Evans

Re: [linux-sunxi] Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Kyle Evans]

On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <icenowy@aosc.io> wrote:
>
>
> 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <kevans@freebsd.org> 写到:
>>On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d@gmail.com>
>>wrote:
>>>
>>>
>>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>>
>>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>>
>>>>> This patch enables the the sid controller in the H3. It can be used
>>>>> for thermal calibration data.
>>>>>
>>>>> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
>>>>> ---
>>>>>   arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>>>>>   1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> index 3f83f6a27c74..9bb5cc29fec5 100644
>>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>> @@ -72,6 +72,13 @@
>>>>>                 };
>>>>>         };
>>>>>   +     soc {
>>>>> +               sid: eeprom@1c14000 {
>>>>> +                       compatible = "allwinner,sun8i-h3-sid";
>>>>> +                       reg = <0x01c14000 0x400>;
>>>>> +               };
>>>>> +       };
>>>>> +
>>>>
>>>>
>>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>>
>>>> Maxime
>>>>
>>>
>>> Oh seems like I forgot that.
>>> As related to the wiki [1] this should be 64 bit wide at the address
>>0x34. I
>>> will add that in the next version.
>>>
>>>
>>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>>
>>> Thanks,
>>> Philipp
>>>
>>
>>Hi,
>>
>>Any chance this will see a v3 soon? I'm kind of interested in sid node
>>for h3. =)
>
> This patch is independent and can be easily sent out
> by its own.
>

Right- I had considered doing so, but wanted to make sure I wasn't
going to collide with this series if a v3 is imminent.

Re: Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Philipp Rossak]

Hi Kyle,

I'm already working on a Version 3 of this patch series. Right now this 
slowed down since I'm very busy and the ToDo-List is still very long.

My plan is to send out a version during this release cycle.

If you need it right now feel free to submit patches!

Philipp

On 19.04.2018 17:19, Kyle Evans wrote:
> On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>>
>>
>> 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <kevans-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> 写到:
>>> On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> wrote:
>>>>
>>>>
>>>> On 29.01.2018 10:52, Maxime Ripard wrote:
>>>>>
>>>>> On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
>>>>>>
>>>>>> This patch enables the the sid controller in the H3. It can be used
>>>>>> for thermal calibration data.
>>>>>>
>>>>>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>>    arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
>>>>>>    1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> index 3f83f6a27c74..9bb5cc29fec5 100644
>>>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>>>> @@ -72,6 +72,13 @@
>>>>>>                  };
>>>>>>          };
>>>>>>    +     soc {
>>>>>> +               sid: eeprom@1c14000 {
>>>>>> +                       compatible = "allwinner,sun8i-h3-sid";
>>>>>> +                       reg = <0x01c14000 0x400>;
>>>>>> +               };
>>>>>> +       };
>>>>>> +
>>>>>
>>>>>
>>>>> Shouldn't you also use a nvmem-cells property to the THS node?
>>>>>
>>>>> Maxime
>>>>>
>>>>
>>>> Oh seems like I forgot that.
>>>> As related to the wiki [1] this should be 64 bit wide at the address
>>> 0x34. I
>>>> will add that in the next version.
>>>>
>>>>
>>>> [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
>>>>
>>>> Thanks,
>>>> Philipp
>>>>
>>>
>>> Hi,
>>>
>>> Any chance this will see a v3 soon? I'm kind of interested in sid node
>>> for h3. =)
>>
>> This patch is independent and can be easily sent out
>> by its own.
>>
> 
> Right- I had considered doing so, but wanted to make sure I wasn't
> going to collide with this series if a v3 is imminent.
> 

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 3843 bytes --]

Hi,

Le vendredi 20 avril 2018 à 11:35 +0200, Philipp Rossak a écrit :
> Hi Kyle,
> 
> I'm already working on a Version 3 of this patch series. Right now this 
> slowed down since I'm very busy and the ToDo-List is still very long.
> 
> My plan is to send out a version during this release cycle.
> 
> If you need it right now feel free to submit patches!

I just came around this patch while testing SID support for H3, that is
not enabled in the dts at this point despite driver support.

Maxime, would it make sense to merge this patch as-is, without support
for the thermal calibration section, since the two aspects seem rather
independent (and some other SoCs also have a SID node without linkage
for thermal support anyway)?

Cheers,

Paul

> Philipp
> 
> On 19.04.2018 17:19, Kyle Evans wrote:
> > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> > > 
> > > 
> > > 于 2018年4月19日 GMT+08:00 下午11:11:22, Kyle Evans <kevans-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> 写到:
> > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > wrote:
> > > > > 
> > > > > 
> > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > > 
> > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > > 
> > > > > > > This patch enables the the sid controller in the H3. It can be used
> > > > > > > for thermal calibration data.
> > > > > > > 
> > > > > > > Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > ---
> > > > > > >    arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
> > > > > > >    1 file changed, 7 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > index 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > @@ -72,6 +72,13 @@
> > > > > > >                  };
> > > > > > >          };
> > > > > > >    +     soc {
> > > > > > > +               sid: eeprom@1c14000 {
> > > > > > > +                       compatible = "allwinner,sun8i-h3-sid";
> > > > > > > +                       reg = <0x01c14000 0x400>;
> > > > > > > +               };
> > > > > > > +       };
> > > > > > > +
> > > > > > 
> > > > > > 
> > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > > 
> > > > > > Maxime
> > > > > > 
> > > > > 
> > > > > Oh seems like I forgot that.
> > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > > 
> > > > 0x34. I
> > > > > will add that in the next version.
> > > > > 
> > > > > 
> > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > > 
> > > > > Thanks,
> > > > > Philipp
> > > > > 
> > > > 
> > > > Hi,
> > > > 
> > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > for h3. =)
> > > 
> > > This patch is independent and can be easily sent out
> > > by its own.
> > > 
> > 
> > Right- I had considered doing so, but wanted to make sure I wasn't
> > going to collide with this series if a v3 is imminent.
> > 
> 
> 
-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [linux-sunxi] Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Emmanuel Vadot]

On Tue, 24 Jul 2018 19:19:54 +0200
Paul Kocialkowski <contact@paulk.fr> wrote:

> Hi,
> 
> Le vendredi 20 avril 2018 à 11:35 +0200, Philipp Rossak a écrit :
> > Hi Kyle,
> > 
> > I'm already working on a Version 3 of this patch series. Right now this 
> > slowed down since I'm very busy and the ToDo-List is still very long.
> > 
> > My plan is to send out a version during this release cycle.
> > 
> > If you need it right now feel free to submit patches!
> 
> I just came around this patch while testing SID support for H3, that is
> not enabled in the dts at this point despite driver support.
> 
> Maxime, would it make sense to merge this patch as-is, without support
> for the thermal calibration section, since the two aspects seem rather
> independent (and some other SoCs also have a SID node without linkage
> for thermal support anyway)?
> 
> Cheers,
> 
> Paul

 Hello Paul,

 I've sent a serie yesterday for SID on A64/H3/H5.
 https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=761

 Cheers,

> > Philipp
> > 
> > On 19.04.2018 17:19, Kyle Evans wrote:
> > > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <icenowy@aosc.io> wrote:
> > > > 
> > > > 
> > > > ? 2018?4?19? GMT+08:00 ??11:11:22, Kyle Evans <kevans@freebsd.org> ??:
> > > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d@gmail.com>
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > > > 
> > > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > > > 
> > > > > > > > This patch enables the the sid controller in the H3. It can be used
> > > > > > > > for thermal calibration data.
> > > > > > > > 
> > > > > > > > Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> > > > > > > > ---
> > > > > > > >    arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
> > > > > > > >    1 file changed, 7 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > index 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > @@ -72,6 +72,13 @@
> > > > > > > >                  };
> > > > > > > >          };
> > > > > > > >    +     soc {
> > > > > > > > +               sid: eeprom@1c14000 {
> > > > > > > > +                       compatible = "allwinner,sun8i-h3-sid";
> > > > > > > > +                       reg = <0x01c14000 0x400>;
> > > > > > > > +               };
> > > > > > > > +       };
> > > > > > > > +
> > > > > > > 
> > > > > > > 
> > > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > > > 
> > > > > > > Maxime
> > > > > > > 
> > > > > > 
> > > > > > Oh seems like I forgot that.
> > > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > > > 
> > > > > 0x34. I
> > > > > > will add that in the next version.
> > > > > > 
> > > > > > 
> > > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > > > 
> > > > > > Thanks,
> > > > > > Philipp
> > > > > > 
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > > for h3. =)
> > > > 
> > > > This patch is independent and can be easily sent out
> > > > by its own.
> > > > 
> > > 
> > > Right- I had considered doing so, but wanted to make sure I wasn't
> > > going to collide with this series if a v3 is imminent.
> > > 
> > 
> > 
> -- 
> Developer of free digital technology and hardware support.
> 
> Website: https://www.paulk.fr/
> Coding blog: https://code.paulk.fr/
> Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

Re: Re: [PATCH v2 14/16] arm: dts: sun8i: h3: enable H3 sid controller

[Paul Kocialkowski]

Hi,

On Wed, 2018-07-25 at 11:05 +0200, Emmanuel Vadot wrote:

[...]

>  Hello Paul,
> 
>  I've sent a serie yesterday for SID on A64/H3/H5.
>  https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=761

Awesome, thanks for taking care of that :)

Cheers,

Paul

> > > On 19.04.2018 17:19, Kyle Evans wrote:
> > > > On Thu, Apr 19, 2018 at 10:13 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> > > > > 
> > > > > 
> > > > > ? 2018?4?19? GMT+08:00 ??11:11:22, Kyle Evans <kevans-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> ??:
> > > > > > On Mon, Jan 29, 2018 at 6:03 AM, Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 29.01.2018 10:52, Maxime Ripard wrote:
> > > > > > > > 
> > > > > > > > On Mon, Jan 29, 2018 at 12:29:17AM +0100, Philipp Rossak wrote:
> > > > > > > > > 
> > > > > > > > > This patch enables the the sid controller in the H3. It can be used
> > > > > > > > > for thermal calibration data.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > > > ---
> > > > > > > > >    arch/arm/boot/dts/sun8i-h3.dtsi | 7 +++++++
> > > > > > > > >    1 file changed, 7 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > index 3f83f6a27c74..9bb5cc29fec5 100644
> > > > > > > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > > > > > > > @@ -72,6 +72,13 @@
> > > > > > > > >                  };
> > > > > > > > >          };
> > > > > > > > >    +     soc {
> > > > > > > > > +               sid: eeprom@1c14000 {
> > > > > > > > > +                       compatible = "allwinner,sun8i-h3-sid";
> > > > > > > > > +                       reg = <0x01c14000 0x400>;
> > > > > > > > > +               };
> > > > > > > > > +       };
> > > > > > > > > +
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Shouldn't you also use a nvmem-cells property to the THS node?
> > > > > > > > 
> > > > > > > > Maxime
> > > > > > > > 
> > > > > > > 
> > > > > > > Oh seems like I forgot that.
> > > > > > > As related to the wiki [1] this should be 64 bit wide at the address
> > > > > > 
> > > > > > 0x34. I
> > > > > > > will add that in the next version.
> > > > > > > 
> > > > > > > 
> > > > > > > [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > Philipp
> > > > > > > 
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > Any chance this will see a v3 soon? I'm kind of interested in sid node
> > > > > > for h3. =)
> > > > > 
> > > > > This patch is independent and can be easily sent out
> > > > > by its own.
> > > > > 
> > > > 
> > > > Right- I had considered doing so, but wanted to make sure I wasn't
> > > > going to collide with this series if a v3 is imminent.
> > > > 
> > > 
> > > 
> > 
> > -- 
> > Developer of free digital technology and hardware support.
> > 
> > Website: https://www.paulk.fr/
> > Coding blog: https://code.paulk.fr/
> > Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
> 
> 
-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[PATCH v2 15/16] arm: dts: sun8i: a83t: add support for the thermal sensor in A83T

[Philipp Rossak]

As we have gained the support for the thermal sensor in A83T,
we can now add its device nodes to the device tree.

The A83T seems to have a broken IRQ 31, thus we use here IRQ 41.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 7f4955a5fab7..9e53ff5ac4ed 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -518,6 +518,14 @@
 			clocks = <&osc24M>;
 		};
 
+		ths: thermal-sensor@1f04000 {
+			compatible = "allwinner,sun8i-a83t-ths";
+			reg = <0x01f04000 0x100>;
+			interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+			#thermal-sensor-cells = <1>;
+			#io-channel-cells = <0>;
+		};
+
 		watchdog@1c20ca0 {
 			compatible = "allwinner,sun6i-a31-wdt";
 			reg = <0x01c20ca0 0x20>;
-- 
2.11.0

[PATCH v2 16/16] arm: dts: sun8i: a83t: add thermal zone to A83T

[Philipp Rossak]

This patch adds the thermal zones to the A83T. Sensor 0 is located besides the
cpu cluster 0. Sensor 1 is located besides cluster 1 and sensor 2 is located
besides in the gpu.

Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 9e53ff5ac4ed..4259a8726031 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -747,4 +747,24 @@
 			#size-cells = <0>;
 		};
 	};
+
+	thermal-zones {
+		cpu0_thermal: cpu0-thermal {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+			thermal-sensors = <&ths 0>;
+		};
+
+		cpu1_thermal: cpu1-thermal {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+			thermal-sensors = <&ths 1>;
+		};
+
+		gpu_thermal: gpu-thermal {
+			polling-delay-passive = <1000>;
+			polling-delay = <5000>;
+			thermal-sensors = <&ths 2>;
+		};
+	};
 };
-- 
2.11.0