[PATCH 0/4] Add an hwmon driver for the mxs soc internal sensor

[PATCH 0/4] Add an hwmon driver for the mxs soc internal sensor

[Alexandre Belloni]

This patchset is ading a simple driver for the i.mx28 internal die temperature
sensor.

The first two patches are preparation patches an can be applied now.

The first patch is adding the #io-channel-cells to the lrdac on i.mx28 so that
it becomes an inkernel iio provider.

The second patch defines the lradc as present on the cfa-10036 and thus removes
it from i.mx28-cfa10049.dts and imx28-cfa10055.dts as they both include
imx28-cfa10036.dts

The third patch adds the driver itself.

And the final patch adds the driver bindings to the cfa-10036.

Alexandre Belloni (4):
  arm: mxs: Add #io-channel-cells property to lradc
  arm: mxs: add lradc to cfa10036
  hwmon: Add a simple driver to read the MXS SoC temperature
  arm: mxs: Add mxs internal temp sensor to cfa-10036

 .../devicetree/bindings/hwmon/mxs-cputemp.txt      |  18 +++
 Documentation/hwmon/mxs-cputemp                    |  29 +++++
 arch/arm/boot/dts/imx28-cfa10036.dts               |  10 ++
 arch/arm/boot/dts/imx28-cfa10049.dts               |   1 -
 arch/arm/boot/dts/imx28-cfa10055.dts               |   1 -
 arch/arm/boot/dts/imx28.dtsi                       |   1 +
 drivers/hwmon/Kconfig                              |  10 ++
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/mxs-cputemp.c                        | 132 +++++++++++++++++++++
 9 files changed, 201 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
 create mode 100644 Documentation/hwmon/mxs-cputemp
 create mode 100644 drivers/hwmon/mxs-cputemp.c

-- 
1.8.1.2

[PATCH 1/4] arm: mxs: Add #io-channel-cells property to lradc

[Alexandre Belloni]

Adding #io-channel-cells property to lradc allows us to use the lradc as an iio
provider.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 arch/arm/boot/dts/imx28.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 600f7cb..9a7feeb 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -829,6 +829,7 @@
 				interrupts = <10 14 15 16 17 18 19
 						20 21 22 23 24 25>;
 				status = "disabled";
+				#io-channel-cells = <1>;
 			};
 
 			spdif@80054000 {
-- 
1.8.1.2

[PATCH 2/4] arm: mxs: add lradc to cfa10036

[Alexandre Belloni]

The lradc is actually available on the Crystalfontz cfa-10036.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10036.dts | 4 ++++
 arch/arm/boot/dts/imx28-cfa10049.dts | 1 -
 arch/arm/boot/dts/imx28-cfa10055.dts | 1 -
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index 5e5213f..802e0e9 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -94,6 +94,10 @@
 			usbphy0: usbphy@8007c000 {
 				status = "okay";
 			};
+
+			lradc@80050000 {
+				status = "okay";
+			};
 		};
 	};
 
diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
index c26676b..4d3c0a6 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -215,7 +215,6 @@
 			};
 
 			lradc@80050000 {
-				status = "okay";
 				fsl,lradc-touchscreen-wires = <4>;
 			};
 		};
diff --git a/arch/arm/boot/dts/imx28-cfa10055.dts b/arch/arm/boot/dts/imx28-cfa10055.dts
index 1581112..e0de1aa 100644
--- a/arch/arm/boot/dts/imx28-cfa10055.dts
+++ b/arch/arm/boot/dts/imx28-cfa10055.dts
@@ -136,7 +136,6 @@
 		apbx@80040000 {
 			lradc@80050000 {
 				fsl,lradc-touchscreen-wires = <4>;
-				status = "okay";
 			};
 
 			pwm: pwm@80064000 {
-- 
1.8.1.2

[PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Alexandre Belloni]

The low resolution ADC of the mxs is able to read an internal temperature
sensor, expose that using hwmon.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 .../devicetree/bindings/hwmon/mxs-cputemp.txt      |  18 +++
 Documentation/hwmon/mxs-cputemp                    |  29 +++++
 drivers/hwmon/Kconfig                              |  10 ++
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/mxs-cputemp.c                        | 132 +++++++++++++++++++++
 5 files changed, 190 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
 create mode 100644 Documentation/hwmon/mxs-cputemp
 create mode 100644 drivers/hwmon/mxs-cputemp.c

diff --git a/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
new file mode 100644
index 0000000..7d3ae47
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
@@ -0,0 +1,18 @@
+mxs cputemp hwmon sensors
+-------------------------
+
+See: Documentation/hwmon/mxs-cputemp
+
+Required properties:
+- compatible: should be "fsl,mxs-internal-temp"
+- io-channels: should list the two adc channels needed to calculate the
+	       temperature
+- io-channel-names: should map the previously listed adc channels to the "min"
+		    and "max" value
+
+Example:
+	temp {
+		compatible = "fsl,mxs-internal-temp";
+		io-channels = <&lradc 8>, <&lradc 9>;
+		io-channel-names = "min", "max";
+	};
diff --git a/Documentation/hwmon/mxs-cputemp b/Documentation/hwmon/mxs-cputemp
new file mode 100644
index 0000000..6c6201f
--- /dev/null
+++ b/Documentation/hwmon/mxs-cputemp
@@ -0,0 +1,29 @@
+Kernel driver mxs-cputemp
+=========================
+
+Supported chips:
+  * Freescale i.mx28
+    Datasheet: i.MX28 Applications Processor Reference Manual, Rev. 1, 2010
+    	       http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
+
+Author: Alexandre Belloni
+
+Description
+-----------
+This driver permits reading the internal die temperature sensor embedded inside
+Freescale i.mx28 SoCs. This sensor is read through two channels of the on chip
+Low-Resolution ADC. After calculation, the three-sigma error of the temperature
+sensor should be within ± 1.5% in degrees Kelvin. Additionally, the temperature
+sampling has a three-sigma sample-to-sample variation of 2 degrees Kelvin. If
+desired, this error can be removed by oversampling and averaging the temperature
+result.
+
+The formula is:
+	(Channel9 – Channel8) * Gain_correction/4
+
+As recommended by the datasheet, Gain_correction is equal to 1.012.
+
+sysfs entries
+-------------
+temp1_input	Measured and corrected temperature in millidegrees Celsius
+
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0428e8a..2daf794 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -929,6 +929,16 @@ config SENSORS_MCP3021
 	  This driver can also be built as a module.  If so, the module
 	  will be called mcp3021.
 
+config SENSORS_MXS_CPU
+	tristate "MXS internal CPU temperature sensor"
+	depends on MXS_LRADC
+	help
+	  If you say yes here you get support for the i.mx28 internal
+	  temperature sensor.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called mxs-cputemp
+
 config SENSORS_NCT6775
 	tristate "Nuvoton NCT6775F and compatibles"
 	depends on !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d17d3e6..366c92d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
 obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
+obj-$(CONFIG_SENSORS_MXS_CPU)	+= mxs-cputemp.o
 obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
 obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
diff --git a/drivers/hwmon/mxs-cputemp.c b/drivers/hwmon/mxs-cputemp.c
new file mode 100644
index 0000000..a312fb5
--- /dev/null
+++ b/drivers/hwmon/mxs-cputemp.c
@@ -0,0 +1,132 @@
+/*
+ * Driver for the mxs internal temperature sensor
+ *
+ * Copyright 2013 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#define DRVNAME "mxs-hwmon"
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/iio/consumer.h>
+
+#define GAIN_CORRECTION 1012
+
+/* The value we calculate from the ADCs is in Kelvins, don't forget to convert
+ * it to Celsius */
+#define VALUES_TO_MILLIC(min, max) ((max - min) * GAIN_CORRECTION / 4 - 272150)
+
+struct mxs_hwmon_data {
+	struct device *hwmon_dev;
+	struct iio_channel *chan_min;
+	struct iio_channel *chan_max;
+};
+
+static int mxs_hwmon_show_temp(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	int err, val_min, val_max;
+
+	struct mxs_hwmon_data *data = dev_get_drvdata(dev);
+
+	err = iio_read_channel_raw(data->chan_min, &val_min);
+	if (err < 0)
+		return err;
+
+	err = iio_read_channel_raw(data->chan_max, &val_max);
+	if (err < 0)
+		return err;
+
+	return sprintf(buf, "%u\n", VALUES_TO_MILLIC(val_min, val_max));
+}
+
+static DEVICE_ATTR(temp1_input, S_IRUGO, mxs_hwmon_show_temp, NULL);
+
+static int mxs_hwmon_probe(struct platform_device *pdev)
+{
+	int err;
+	struct mxs_hwmon_data *data;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, data);
+
+	err = device_create_file(&pdev->dev, &dev_attr_temp1_input);
+	if (err)
+		return err;
+
+	data->chan_min = iio_channel_get(&pdev->dev, "min");
+	if (IS_ERR(data->chan_min)) {
+		err = PTR_ERR(data->chan_min);
+		goto error_chan_min;
+	}
+
+	data->chan_max = iio_channel_get(&pdev->dev, "max");
+	if (IS_ERR(data->chan_max)) {
+		err = PTR_ERR(data->chan_max);
+		goto error_chan_max;
+	}
+
+	data->hwmon_dev = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto error_hwmon;
+	}
+
+	return 0;
+
+error_hwmon:
+	iio_channel_release(data->chan_max);
+error_chan_max:
+	iio_channel_release(data->chan_min);
+error_chan_min:
+	device_remove_file(&pdev->dev, &dev_attr_temp1_input);
+
+	return err;
+}
+
+static int mxs_hwmon_remove(struct platform_device *pdev)
+{
+	struct mxs_hwmon_data *data = platform_get_drvdata(pdev);
+
+	iio_channel_release(data->chan_min);
+	iio_channel_release(data->chan_max);
+	hwmon_device_unregister(data->hwmon_dev);
+
+	device_remove_file(&pdev->dev, &dev_attr_temp1_input);
+
+	return 0;
+}
+
+static struct of_device_id mxs_hwmon_of_match[] = {
+	{ .compatible = "fsl,mxs-internal-temp", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mxs_hwmon_of_match);
+
+static struct platform_driver mxs_hwmon_driver = {
+	.probe = mxs_hwmon_probe,
+	.remove = mxs_hwmon_remove,
+	.driver	= {
+		.name = DRVNAME,
+		.owner = THIS_MODULE,
+		.of_match_table = mxs_hwmon_of_match,
+	},
+};
+
+module_platform_driver(mxs_hwmon_driver);
+
+MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
+MODULE_DESCRIPTION("Freescale i.MX28 hwmon sensor driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mxs-hwmon");
-- 
1.8.1.2

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Guenter Roeck]

On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
> The low resolution ADC of the mxs is able to read an internal temperature
> sensor, expose that using hwmon.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---

Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?

Guenter

>  .../devicetree/bindings/hwmon/mxs-cputemp.txt      |  18 +++
>  Documentation/hwmon/mxs-cputemp                    |  29 +++++
>  drivers/hwmon/Kconfig                              |  10 ++
>  drivers/hwmon/Makefile                             |   1 +
>  drivers/hwmon/mxs-cputemp.c                        | 132 +++++++++++++++++++++
>  5 files changed, 190 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
>  create mode 100644 Documentation/hwmon/mxs-cputemp
>  create mode 100644 drivers/hwmon/mxs-cputemp.c
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
> new file mode 100644
> index 0000000..7d3ae47
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
> @@ -0,0 +1,18 @@
> +mxs cputemp hwmon sensors
> +-------------------------
> +
> +See: Documentation/hwmon/mxs-cputemp
> +
> +Required properties:
> +- compatible: should be "fsl,mxs-internal-temp"
> +- io-channels: should list the two adc channels needed to calculate the
> +	       temperature
> +- io-channel-names: should map the previously listed adc channels to the "min"
> +		    and "max" value
> +
> +Example:
> +	temp {
> +		compatible = "fsl,mxs-internal-temp";
> +		io-channels = <&lradc 8>, <&lradc 9>;
> +		io-channel-names = "min", "max";
> +	};
> diff --git a/Documentation/hwmon/mxs-cputemp b/Documentation/hwmon/mxs-cputemp
> new file mode 100644
> index 0000000..6c6201f
> --- /dev/null
> +++ b/Documentation/hwmon/mxs-cputemp
> @@ -0,0 +1,29 @@
> +Kernel driver mxs-cputemp
> +=========================
> +
> +Supported chips:
> +  * Freescale i.mx28
> +    Datasheet: i.MX28 Applications Processor Reference Manual, Rev. 1, 2010
> +    	       http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
> +
> +Author: Alexandre Belloni
> +
> +Description
> +-----------
> +This driver permits reading the internal die temperature sensor embedded inside
> +Freescale i.mx28 SoCs. This sensor is read through two channels of the on chip
> +Low-Resolution ADC. After calculation, the three-sigma error of the temperature
> +sensor should be within ± 1.5% in degrees Kelvin. Additionally, the temperature
> +sampling has a three-sigma sample-to-sample variation of 2 degrees Kelvin. If
> +desired, this error can be removed by oversampling and averaging the temperature
> +result.
> +
> +The formula is:
> +	(Channel9 – Channel8) * Gain_correction/4
> +
> +As recommended by the datasheet, Gain_correction is equal to 1.012.
> +
> +sysfs entries
> +-------------
> +temp1_input	Measured and corrected temperature in millidegrees Celsius
> +
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 0428e8a..2daf794 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -929,6 +929,16 @@ config SENSORS_MCP3021
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called mcp3021.
>  
> +config SENSORS_MXS_CPU
> +	tristate "MXS internal CPU temperature sensor"
> +	depends on MXS_LRADC
> +	help
> +	  If you say yes here you get support for the i.mx28 internal
> +	  temperature sensor.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called mxs-cputemp
> +
>  config SENSORS_NCT6775
>  	tristate "Nuvoton NCT6775F and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index d17d3e6..366c92d 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -108,6 +108,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
>  obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
>  obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
>  obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
> +obj-$(CONFIG_SENSORS_MXS_CPU)	+= mxs-cputemp.o
>  obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
>  obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
> diff --git a/drivers/hwmon/mxs-cputemp.c b/drivers/hwmon/mxs-cputemp.c
> new file mode 100644
> index 0000000..a312fb5
> --- /dev/null
> +++ b/drivers/hwmon/mxs-cputemp.c
> @@ -0,0 +1,132 @@
> +/*
> + * Driver for the mxs internal temperature sensor
> + *
> + * Copyright 2013 Free Electrons
> + *
> + * Licensed under the GPLv2 or later.
> + */
> +
> +#define DRVNAME "mxs-hwmon"
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/iio/consumer.h>
> +
> +#define GAIN_CORRECTION 1012
> +
> +/* The value we calculate from the ADCs is in Kelvins, don't forget to convert
> + * it to Celsius */
> +#define VALUES_TO_MILLIC(min, max) ((max - min) * GAIN_CORRECTION / 4 - 272150)
> +
> +struct mxs_hwmon_data {
> +	struct device *hwmon_dev;
> +	struct iio_channel *chan_min;
> +	struct iio_channel *chan_max;
> +};
> +
> +static int mxs_hwmon_show_temp(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	int err, val_min, val_max;
> +
> +	struct mxs_hwmon_data *data = dev_get_drvdata(dev);
> +
> +	err = iio_read_channel_raw(data->chan_min, &val_min);
> +	if (err < 0)
> +		return err;
> +
> +	err = iio_read_channel_raw(data->chan_max, &val_max);
> +	if (err < 0)
> +		return err;
> +
> +	return sprintf(buf, "%u\n", VALUES_TO_MILLIC(val_min, val_max));
> +}
> +
> +static DEVICE_ATTR(temp1_input, S_IRUGO, mxs_hwmon_show_temp, NULL);
> +
> +static int mxs_hwmon_probe(struct platform_device *pdev)
> +{
> +	int err;
> +	struct mxs_hwmon_data *data;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	err = device_create_file(&pdev->dev, &dev_attr_temp1_input);
> +	if (err)
> +		return err;
> +
> +	data->chan_min = iio_channel_get(&pdev->dev, "min");
> +	if (IS_ERR(data->chan_min)) {
> +		err = PTR_ERR(data->chan_min);
> +		goto error_chan_min;
> +	}
> +
> +	data->chan_max = iio_channel_get(&pdev->dev, "max");
> +	if (IS_ERR(data->chan_max)) {
> +		err = PTR_ERR(data->chan_max);
> +		goto error_chan_max;
> +	}
> +
> +	data->hwmon_dev = hwmon_device_register(&pdev->dev);
> +	if (IS_ERR(data->hwmon_dev)) {
> +		err = PTR_ERR(data->hwmon_dev);
> +		goto error_hwmon;
> +	}
> +
> +	return 0;
> +
> +error_hwmon:
> +	iio_channel_release(data->chan_max);
> +error_chan_max:
> +	iio_channel_release(data->chan_min);
> +error_chan_min:
> +	device_remove_file(&pdev->dev, &dev_attr_temp1_input);
> +
> +	return err;
> +}
> +
> +static int mxs_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct mxs_hwmon_data *data = platform_get_drvdata(pdev);
> +
> +	iio_channel_release(data->chan_min);
> +	iio_channel_release(data->chan_max);
> +	hwmon_device_unregister(data->hwmon_dev);
> +
> +	device_remove_file(&pdev->dev, &dev_attr_temp1_input);
> +
> +	return 0;
> +}
> +
> +static struct of_device_id mxs_hwmon_of_match[] = {
> +	{ .compatible = "fsl,mxs-internal-temp", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mxs_hwmon_of_match);
> +
> +static struct platform_driver mxs_hwmon_driver = {
> +	.probe = mxs_hwmon_probe,
> +	.remove = mxs_hwmon_remove,
> +	.driver	= {
> +		.name = DRVNAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = mxs_hwmon_of_match,
> +	},
> +};
> +
> +module_platform_driver(mxs_hwmon_driver);
> +
> +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
> +MODULE_DESCRIPTION("Freescale i.MX28 hwmon sensor driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:mxs-hwmon");
> -- 
> 1.8.1.2
> 
> 

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Maxime Ripard]

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

On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote:
> On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
> > The low resolution ADC of the mxs is able to read an internal temperature
> > sensor, expose that using hwmon.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> 
> Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?

Actually, I wonder if we should not just put the hwmon driver
capabilities directly into the mxs-lradc driver, just like it's already
been done in this driver for the touchscreen support.

The probing of this hwmon driver doesn't really belong to the DT, it's
not really realistic to probe it from the machine definition, and it
really is the IP that is wired that way.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Guenter Roeck]

On Thu, Jun 27, 2013 at 11:17:32AM +0200, Maxime Ripard wrote:
> On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote:
> > On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
> > > The low resolution ADC of the mxs is able to read an internal temperature
> > > sensor, expose that using hwmon.
> > > 
> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > ---
> > 
> > Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?
> 
> Actually, I wonder if we should not just put the hwmon driver
> capabilities directly into the mxs-lradc driver, just like it's already
> been done in this driver for the touchscreen support.
> 
> The probing of this hwmon driver doesn't really belong to the DT, it's
> not really realistic to probe it from the machine definition, and it
> really is the IP that is wired that way.
> 
Merging iio-hwmon functionality into an adc driver seems just as bad
(or even worse) as copying it into a new driver.

If the lradc driver knows that the ADC channels are temperature sensors, it
should register them with the iio subsystem as IIO_TEMP type. Then you should
be able to use iio_hwmon as is.

Guenter

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Alexandre Belloni]

Hi,

On 27/06/2013 16:27, Guenter Roeck wrote:
> On Thu, Jun 27, 2013 at 11:17:32AM +0200, Maxime Ripard wrote:
>> On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote:
>>> On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
>>>> The low resolution ADC of the mxs is able to read an internal temperature
>>>> sensor, expose that using hwmon.
>>>>
>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> ---
>>> Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?
>> Actually, I wonder if we should not just put the hwmon driver
>> capabilities directly into the mxs-lradc driver, just like it's already
>> been done in this driver for the touchscreen support.
>>
>> The probing of this hwmon driver doesn't really belong to the DT, it's
>> not really realistic to probe it from the machine definition, and it
>> really is the IP that is wired that way.
>>
> Merging iio-hwmon functionality into an adc driver seems just as bad
> (or even worse) as copying it into a new driver.
>
> If the lradc driver knows that the ADC channels are temperature sensors, it
> should register them with the iio subsystem as IIO_TEMP type. Then you should
> be able to use iio_hwmon as is.

They are already registered as IIO_TEMP but only implement read_raw. Also,

iio_hwmon_read_val() is using iio_read_channel_processed() and that will
basically only read one of the 2 channels. As I documented, you actually
need to read both channel 8 and channel 9 and then compute the value in
Kelvins. I'm not sure how you want me to do that in the current framework.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Lars-Peter Clausen]

On 06/27/2013 09:26 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 27/06/2013 16:27, Guenter Roeck wrote:
>> On Thu, Jun 27, 2013 at 11:17:32AM +0200, Maxime Ripard wrote:
>>> On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote:
>>>> On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
>>>>> The low resolution ADC of the mxs is able to read an internal temperature
>>>>> sensor, expose that using hwmon.
>>>>>
>>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>>>> ---
>>>> Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?
>>> Actually, I wonder if we should not just put the hwmon driver
>>> capabilities directly into the mxs-lradc driver, just like it's already
>>> been done in this driver for the touchscreen support.
>>>
>>> The probing of this hwmon driver doesn't really belong to the DT, it's
>>> not really realistic to probe it from the machine definition, and it
>>> really is the IP that is wired that way.
>>>
>> Merging iio-hwmon functionality into an adc driver seems just as bad
>> (or even worse) as copying it into a new driver.
>>
>> If the lradc driver knows that the ADC channels are temperature sensors, it
>> should register them with the iio subsystem as IIO_TEMP type. Then you should
>> be able to use iio_hwmon as is.
> 
> They are already registered as IIO_TEMP but only implement read_raw. Also,
> 
> iio_hwmon_read_val() is using iio_read_channel_processed() and that will
> basically only read one of the 2 channels. As I documented, you actually
> need to read both channel 8 and channel 9 and then compute the value in
> Kelvins. I'm not sure how you want me to do that in the current framework.

What are these two channels actually measuring? Is the value of a single
channel meaningful on it's own? If not it might make sense to update the IIO
driver to just have one temperature channel.

- Lars

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Alexandre Belloni]

On 28/06/2013 16:18, Lars-Peter Clausen wrote:
> On 06/27/2013 09:26 PM, Alexandre Belloni wrote:
>>
>> They are already registered as IIO_TEMP but only implement read_raw. Also,
>>
>> iio_hwmon_read_val() is using iio_read_channel_processed() and that will
>> basically only read one of the 2 channels. As I documented, you actually
>> need to read both channel 8 and channel 9 and then compute the value in
>> Kelvins. I'm not sure how you want me to do that in the current framework.
> What are these two channels actually measuring? Is the value of a single
> channel meaningful on it's own? If not it might make sense to update the IIO
> driver to just have one temperature channel.

It's not actually meaningful on its own. So, what you would do is expose
one iio channel for two ADC channels and do the computation in read_raw
? or read_processed ? Then using iio-hwon to export it. ?

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Lars-Peter Clausen]

On 06/28/2013 04:50 PM, Alexandre Belloni wrote:
> On 28/06/2013 16:18, Lars-Peter Clausen wrote:
>> On 06/27/2013 09:26 PM, Alexandre Belloni wrote:
>>>
>>> They are already registered as IIO_TEMP but only implement read_raw. Also,
>>>
>>> iio_hwmon_read_val() is using iio_read_channel_processed() and that will
>>> basically only read one of the 2 channels. As I documented, you actually
>>> need to read both channel 8 and channel 9 and then compute the value in
>>> Kelvins. I'm not sure how you want me to do that in the current framework.
>> What are these two channels actually measuring? Is the value of a single
>> channel meaningful on it's own? If not it might make sense to update the IIO
>> driver to just have one temperature channel.
> 
> It's not actually meaningful on its own. So, what you would do is expose
> one iio channel for two ADC channels and do the computation in read_raw
> ? or read_processed ? Then using iio-hwon to export it. ?
> 
> Regards,
> 

Yes, return channel9 - channel8 as the raw value for the temperature channel
and provide proper scale and offset values, so that
iio_read_channel_processed() will return the correct value.

- Lars

Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature

[Guenter Roeck]

On Fri, Jun 28, 2013 at 05:24:43PM +0200, Lars-Peter Clausen wrote:
> On 06/28/2013 04:50 PM, Alexandre Belloni wrote:
> > On 28/06/2013 16:18, Lars-Peter Clausen wrote:
> >> On 06/27/2013 09:26 PM, Alexandre Belloni wrote:
> >>>
> >>> They are already registered as IIO_TEMP but only implement read_raw. Also,
> >>>
> >>> iio_hwmon_read_val() is using iio_read_channel_processed() and that will
> >>> basically only read one of the 2 channels. As I documented, you actually
> >>> need to read both channel 8 and channel 9 and then compute the value in
> >>> Kelvins. I'm not sure how you want me to do that in the current framework.
> >> What are these two channels actually measuring? Is the value of a single
> >> channel meaningful on it's own? If not it might make sense to update the IIO
> >> driver to just have one temperature channel.
> > 
> > It's not actually meaningful on its own. So, what you would do is expose
> > one iio channel for two ADC channels and do the computation in read_raw
> > ? or read_processed ? Then using iio-hwon to export it. ?
> > 
> > Regards,
> > 
> 
> Yes, return channel9 - channel8 as the raw value for the temperature channel
> and provide proper scale and offset values, so that
> iio_read_channel_processed() will return the correct value.
> 
Agreed.

Guenter

[PATCH 4/4] arm: mxs: Add mxs internal temp sensor to cfa-10036

[Alexandre Belloni]

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10036.dts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index 802e0e9..5afb945 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -95,7 +95,7 @@
 				status = "okay";
 			};
 
-			lradc@80050000 {
+			lradc: lradc@80050000 {
 				status = "okay";
 			};
 		};
@@ -121,4 +121,10 @@
 			default-state = "on";
 		};
 	};
+
+	temp {
+		compatible = "fsl,mxs-internal-temp";
+		io-channels = <&lradc 8>, <&lradc 9>;
+		io-channel-names = "min", "max";
+	};
 };
-- 
1.8.1.2