[PATCH 0/4] thermal: imx: Add nvmem-cells binding on imx6sx

[PATCH 0/4] thermal: imx: Add nvmem-cells binding on imx6sx

[Leonard Crestez]

On imx6sx accessing OCOTP directly is wrong because the ocotp clock needs to be
enabled first. Fix this by adding a nvmem-cells binding and using it on imx6sx,
imx6ul and imx6ull.

The existing binding is kept around because it works fine on imx6qdl.

This was initially reported by Lothar Waßmann <LW@KARO-electronics.de> in reply
to a mail adding imx6ul/ull support:

Link: https://lkml.org/lkml/2017/6/9/578

A previous attempt just reinterpreted the fsl,tempmon-data phandle as nvmem.
Code was actually written to use nvmem-cells first but I thought that
reinterpreting existing devicetree properties would be interesting.

Link: https://lkml.org/lkml/2017/6/19/333

Leonard Crestez (4):
  thermal: imx: Add nvmem-cells alternate binding for OCOTP access
  thermal: imx: Add support for reading OCOTP through nvmem
  ARM: dts: imx6sx: Use nvmem-cells for tempmon
  ARM: dts: imx6ul: Add imx6ul-tempmon

 .../devicetree/bindings/thermal/imx-thermal.txt    |   7 ++
 arch/arm/boot/dts/imx6sx.dtsi                      |  11 +-
 arch/arm/boot/dts/imx6ul.dtsi                      |  17 +++
 drivers/thermal/imx_thermal.c                      | 131 ++++++++++++++++-----
 4 files changed, 135 insertions(+), 31 deletions(-)

-- 
2.7.4

[PATCH 1/4] thermal: imx: Add nvmem-cells alternate binding for OCOTP access

[Leonard Crestez]

On newer imx SOCs accessing OCOTP directly is wrong because the ocotp
clock needs to be enabled first. Add a binding for accessing the same
values through the imx-ocotp nvmem driver using nvmem-cells. This is
similar to other thermal drivers.

The old binding is preserved for compatibility and because it still
works fine on imx6qdl series chips.

In theory this problem could be solved by adding a reference to the
OCOTP clock instead but it is better to hide such details in a specific
nvmem driver.

Signed-off-by: Leonard Crestez <leonard.crestez-3arQi8VN3Tc@public.gmane.org>
---
 Documentation/devicetree/bindings/thermal/imx-thermal.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/imx-thermal.txt b/Documentation/devicetree/bindings/thermal/imx-thermal.txt
index 3c67bd5..2842a05 100644
--- a/Documentation/devicetree/bindings/thermal/imx-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/imx-thermal.txt
@@ -7,9 +7,16 @@ Required properties:
   is higher than panic threshold, system will auto reboot by SRC module.
 - fsl,tempmon : phandle pointer to system controller that contains TEMPMON
   control registers, e.g. ANATOP on imx6q.
+
+Properties for OCOTP access:
 - fsl,tempmon-data : phandle pointer to fuse controller that contains TEMPMON
   calibration data, e.g. OCOTP on imx6q.  The details about calibration data
   can be found in SoC Reference Manual.
+Alternatively:
+- nvmem-cells: A phandle to the calibration cells provided by ocotp.
+- nvmem-cell-names: Should be "calib", "temp_grade".
+
+Direct access to OCOTP is deprecated, please use nvmem cells instead.
 
 Optional properties:
 - clocks : thermal sensor's clock source.
-- 
2.7.4

--
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 1/4] thermal: imx: Add nvmem-cells alternate binding for OCOTP access

[Rob Herring]

On Thu, Jul 06, 2017 at 04:20:41PM +0300, Leonard Crestez wrote:
> On newer imx SOCs accessing OCOTP directly is wrong because the ocotp
> clock needs to be enabled first. Add a binding for accessing the same
> values through the imx-ocotp nvmem driver using nvmem-cells. This is
> similar to other thermal drivers.
> 
> The old binding is preserved for compatibility and because it still
> works fine on imx6qdl series chips.
> 
> In theory this problem could be solved by adding a reference to the
> OCOTP clock instead but it is better to hide such details in a specific
> nvmem driver.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  Documentation/devicetree/bindings/thermal/imx-thermal.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/imx-thermal.txt b/Documentation/devicetree/bindings/thermal/imx-thermal.txt
> index 3c67bd5..2842a05 100644
> --- a/Documentation/devicetree/bindings/thermal/imx-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/imx-thermal.txt
> @@ -7,9 +7,16 @@ Required properties:
>    is higher than panic threshold, system will auto reboot by SRC module.
>  - fsl,tempmon : phandle pointer to system controller that contains TEMPMON
>    control registers, e.g. ANATOP on imx6q.
> +
> +Properties for OCOTP access:
>  - fsl,tempmon-data : phandle pointer to fuse controller that contains TEMPMON
>    calibration data, e.g. OCOTP on imx6q.  The details about calibration data
>    can be found in SoC Reference Manual.
> +Alternatively:

Just put these under required props and put the deprecated one under a 
deprecated section. IOW, make it easy to remove the deprecated part.

> +- nvmem-cells: A phandle to the calibration cells provided by ocotp.
> +- nvmem-cell-names: Should be "calib", "temp_grade".
> +
> +Direct access to OCOTP is deprecated, please use nvmem cells instead.
>  
>  Optional properties:
>  - clocks : thermal sensor's clock source.
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] thermal: imx: Add support for reading OCOTP through nvmem

[Leonard Crestez]

On newer imx SOCs accessing OCOTP directly is wrong because the ocotp clock
needs to be enabled first. Add support for reading those same values through
the nvmem API instead.

The older path is preserved for compatibility with older dts and because it
works correctly on imx6qdl chips.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/thermal/imx_thermal.c | 131 ++++++++++++++++++++++++++++++++----------
 1 file changed, 101 insertions(+), 30 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index fb648a4..ffbd579 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/thermal.h>
 #include <linux/types.h>
+#include <linux/nvmem-consumer.h>
 
 #define REG_SET		0x4
 #define REG_CLR		0x8
@@ -92,7 +93,7 @@ struct imx_thermal_data {
 	struct thermal_cooling_device *cdev;
 	enum thermal_device_mode mode;
 	struct regmap *tempmon;
-	u32 c1, c2; /* See formula in imx_get_sensor_data() */
+	u32 c1, c2; /* See formula in imx_init_calib() */
 	int temp_passive;
 	int temp_critical;
 	int temp_max;
@@ -175,7 +176,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	n_meas = (val & TEMPSENSE0_TEMP_CNT_MASK) >> TEMPSENSE0_TEMP_CNT_SHIFT;
 
-	/* See imx_get_sensor_data() for formula derivation */
+	/* See imx_init_calib() for formula derivation */
 	*temp = data->c2 - n_meas * data->c1;
 
 	/* Update alarm value to next higher trip point for TEMPMON_IMX6Q */
@@ -344,29 +345,12 @@ static struct thermal_zone_device_ops imx_tz_ops = {
 	.set_trip_temp = imx_set_trip_temp,
 };
 
-static int imx_get_sensor_data(struct platform_device *pdev)
+static int imx_init_calib(struct platform_device *pdev, u32 val)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
-	struct regmap *map;
 	int t1, n1;
-	int ret;
-	u32 val;
 	u64 temp64;
 
-	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
-					      "fsl,tempmon-data");
-	if (IS_ERR(map)) {
-		ret = PTR_ERR(map);
-		dev_err(&pdev->dev, "failed to get sensor regmap: %d\n", ret);
-		return ret;
-	}
-
-	ret = regmap_read(map, OCOTP_ANA1, &val);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
-		return ret;
-	}
-
 	if (val == 0 || val == ~0) {
 		dev_err(&pdev->dev, "invalid sensor calibration data\n");
 		return -EINVAL;
@@ -403,12 +387,12 @@ static int imx_get_sensor_data(struct platform_device *pdev)
 	data->c1 = temp64;
 	data->c2 = n1 * data->c1 + 1000 * t1;
 
-	/* use OTP for thermal grade */
-	ret = regmap_read(map, OCOTP_MEM0, &val);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to read temp grade: %d\n", ret);
-		return ret;
-	}
+	return 0;
+}
+
+static void imx_init_temp_grade(struct platform_device *pdev, u32 val)
+{
+	struct imx_thermal_data *data = platform_get_drvdata(pdev);
 
 	/* The maximum die temp is specified by the Temperature Grade */
 	switch ((val >> 6) & 0x3) {
@@ -436,10 +420,87 @@ static int imx_get_sensor_data(struct platform_device *pdev)
 	 */
 	data->temp_critical = data->temp_max - (1000 * 5);
 	data->temp_passive = data->temp_max - (1000 * 10);
+}
+
+static int imx_init_from_tempmon_data(struct platform_device *pdev)
+{
+	struct regmap *map;
+	int ret;
+	u32 val;
+
+	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+					      "fsl,tempmon-data");
+	if (IS_ERR(map)) {
+		ret = PTR_ERR(map);
+		dev_err(&pdev->dev, "failed to get sensor regmap: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_read(map, OCOTP_ANA1, &val);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
+		return ret;
+	}
+	ret = imx_init_calib(pdev, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(map, OCOTP_MEM0, &val);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
+		return ret;
+	}
+	imx_init_temp_grade(pdev, val);
 
 	return 0;
 }
 
+static int nvmem_cell_read_u32(struct device* dev, const char *cell_id, u32 *val)
+{
+	struct nvmem_cell *cell;
+	void *buf;
+	size_t len;
+
+	cell = nvmem_cell_get(dev, cell_id);
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &len);
+	if (IS_ERR(buf)) {
+		nvmem_cell_put(cell);
+		return PTR_ERR(buf);
+	}
+	if (len != sizeof(*val)) {
+		kfree(buf);
+		nvmem_cell_put(cell);
+		return -EINVAL;
+	}
+	memcpy(val, buf, sizeof(*val));
+
+	kfree(buf);
+	nvmem_cell_put(cell);
+	return 0;
+}
+
+static int imx_init_from_nvmem_cells(struct platform_device *pdev)
+{
+	int ret;
+	u32 val;
+
+	ret = nvmem_cell_read_u32(&pdev->dev, "calib", &val);
+	if (ret)
+		return ret;
+	imx_init_calib(pdev, val);
+
+	ret = nvmem_cell_read_u32(&pdev->dev, "temp_grade", &val);
+	if (ret)
+		return ret;
+	imx_init_temp_grade(pdev, val);
+
+	return 0;
+}
+
+
 static irqreturn_t imx_thermal_alarm_irq(int irq, void *dev)
 {
 	struct imx_thermal_data *data = dev;
@@ -512,10 +573,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
-	ret = imx_get_sensor_data(pdev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to get sensor data\n");
-		return ret;
+	if (of_find_property(pdev->dev.of_node, "nvmem-cells", NULL)) {
+		ret = imx_init_from_nvmem_cells(pdev);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		if (ret) {
+			dev_err(&pdev->dev, "failed to init from nvmem: %d\n", ret);
+			return ret;
+		}
+	} else {
+		ret = imx_init_from_tempmon_data(pdev);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to init from from fsl,tempmon-data\n");
+			return ret;
+		}
 	}
 
 	/* Make sure sensor is in known good state for measurements */
-- 
2.7.4

Re: [PATCH 2/4] thermal: imx: Add support for reading OCOTP through nvmem

[Shawn Guo]

On Thu, Jul 06, 2017 at 04:20:42PM +0300, Leonard Crestez wrote:
> On newer imx SOCs accessing OCOTP directly is wrong because the ocotp clock
> needs to be enabled first. Add support for reading those same values through
> the nvmem API instead.
> 
> The older path is preserved for compatibility with older dts and because it
> works correctly on imx6qdl chips.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez-3arQi8VN3Tc@public.gmane.org>
> ---
>  drivers/thermal/imx_thermal.c | 131 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 101 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index fb648a4..ffbd579 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/thermal.h>
>  #include <linux/types.h>
> +#include <linux/nvmem-consumer.h>
>  
>  #define REG_SET		0x4
>  #define REG_CLR		0x8
> @@ -92,7 +93,7 @@ struct imx_thermal_data {
>  	struct thermal_cooling_device *cdev;
>  	enum thermal_device_mode mode;
>  	struct regmap *tempmon;
> -	u32 c1, c2; /* See formula in imx_get_sensor_data() */
> +	u32 c1, c2; /* See formula in imx_init_calib() */
>  	int temp_passive;
>  	int temp_critical;
>  	int temp_max;
> @@ -175,7 +176,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>  
>  	n_meas = (val & TEMPSENSE0_TEMP_CNT_MASK) >> TEMPSENSE0_TEMP_CNT_SHIFT;
>  
> -	/* See imx_get_sensor_data() for formula derivation */
> +	/* See imx_init_calib() for formula derivation */
>  	*temp = data->c2 - n_meas * data->c1;
>  
>  	/* Update alarm value to next higher trip point for TEMPMON_IMX6Q */
> @@ -344,29 +345,12 @@ static struct thermal_zone_device_ops imx_tz_ops = {
>  	.set_trip_temp = imx_set_trip_temp,
>  };
>  
> -static int imx_get_sensor_data(struct platform_device *pdev)
> +static int imx_init_calib(struct platform_device *pdev, u32 val)
>  {
>  	struct imx_thermal_data *data = platform_get_drvdata(pdev);
> -	struct regmap *map;
>  	int t1, n1;
> -	int ret;
> -	u32 val;
>  	u64 temp64;
>  
> -	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> -					      "fsl,tempmon-data");
> -	if (IS_ERR(map)) {
> -		ret = PTR_ERR(map);
> -		dev_err(&pdev->dev, "failed to get sensor regmap: %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = regmap_read(map, OCOTP_ANA1, &val);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
> -		return ret;
> -	}
> -
>  	if (val == 0 || val == ~0) {
>  		dev_err(&pdev->dev, "invalid sensor calibration data\n");
>  		return -EINVAL;
> @@ -403,12 +387,12 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>  	data->c1 = temp64;
>  	data->c2 = n1 * data->c1 + 1000 * t1;
>  
> -	/* use OTP for thermal grade */
> -	ret = regmap_read(map, OCOTP_MEM0, &val);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to read temp grade: %d\n", ret);
> -		return ret;
> -	}
> +	return 0;
> +}
> +
> +static void imx_init_temp_grade(struct platform_device *pdev, u32 val)
> +{
> +	struct imx_thermal_data *data = platform_get_drvdata(pdev);
>  
>  	/* The maximum die temp is specified by the Temperature Grade */
>  	switch ((val >> 6) & 0x3) {
> @@ -436,10 +420,87 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>  	 */
>  	data->temp_critical = data->temp_max - (1000 * 5);
>  	data->temp_passive = data->temp_max - (1000 * 10);
> +}
> +
> +static int imx_init_from_tempmon_data(struct platform_device *pdev)
> +{
> +	struct regmap *map;
> +	int ret;
> +	u32 val;
> +
> +	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +					      "fsl,tempmon-data");
> +	if (IS_ERR(map)) {
> +		ret = PTR_ERR(map);
> +		dev_err(&pdev->dev, "failed to get sensor regmap: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_read(map, OCOTP_ANA1, &val);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
> +		return ret;
> +	}
> +	ret = imx_init_calib(pdev, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(map, OCOTP_MEM0, &val);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
> +		return ret;
> +	}
> +	imx_init_temp_grade(pdev, val);
>  
>  	return 0;
>  }
>  
> +static int nvmem_cell_read_u32(struct device* dev, const char *cell_id, u32 *val)
> +{
> +	struct nvmem_cell *cell;
> +	void *buf;
> +	size_t len;
> +
> +	cell = nvmem_cell_get(dev, cell_id);
> +	if (IS_ERR(cell))
> +		return PTR_ERR(cell);
> +
> +	buf = nvmem_cell_read(cell, &len);
> +	if (IS_ERR(buf)) {
> +		nvmem_cell_put(cell);
> +		return PTR_ERR(buf);
> +	}
> +	if (len != sizeof(*val)) {
> +		kfree(buf);
> +		nvmem_cell_put(cell);
> +		return -EINVAL;
> +	}
> +	memcpy(val, buf, sizeof(*val));
> +
> +	kfree(buf);
> +	nvmem_cell_put(cell);
> +	return 0;
> +}

The function looks nothing IMX specific, and could be a nvmem core
function?

@Srinivas, thoughts?

Shawn

> +
> +static int imx_init_from_nvmem_cells(struct platform_device *pdev)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = nvmem_cell_read_u32(&pdev->dev, "calib", &val);
> +	if (ret)
> +		return ret;
> +	imx_init_calib(pdev, val);
> +
> +	ret = nvmem_cell_read_u32(&pdev->dev, "temp_grade", &val);
> +	if (ret)
> +		return ret;
> +	imx_init_temp_grade(pdev, val);
> +
> +	return 0;
> +}
> +
> +
>  static irqreturn_t imx_thermal_alarm_irq(int irq, void *dev)
>  {
>  	struct imx_thermal_data *data = dev;
> @@ -512,10 +573,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, data);
>  
> -	ret = imx_get_sensor_data(pdev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to get sensor data\n");
> -		return ret;
> +	if (of_find_property(pdev->dev.of_node, "nvmem-cells", NULL)) {
> +		ret = imx_init_from_nvmem_cells(pdev);
> +		if (ret == -EPROBE_DEFER)
> +			return ret;
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to init from nvmem: %d\n", ret);
> +			return ret;
> +		}
> +	} else {
> +		ret = imx_init_from_tempmon_data(pdev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to init from from fsl,tempmon-data\n");
> +			return ret;
> +		}
>  	}
>  
>  	/* Make sure sensor is in known good state for measurements */
> -- 
> 2.7.4
> 
--
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 2/4] thermal: imx: Add support for reading OCOTP through nvmem

[Srinivas Kandagatla]

On 12/07/17 07:36, Shawn Guo wrote:
>>   
>> +static int nvmem_cell_read_u32(struct device* dev, const char *cell_id, u32 *val)
>> +{
>> +	struct nvmem_cell *cell;
>> +	void *buf;
>> +	size_t len;
>> +
>> +	cell = nvmem_cell_get(dev, cell_id);
>> +	if (IS_ERR(cell))
>> +		return PTR_ERR(cell);
>> +
>> +	buf = nvmem_cell_read(cell, &len);
>> +	if (IS_ERR(buf)) {
>> +		nvmem_cell_put(cell);
>> +		return PTR_ERR(buf);
>> +	}
>> +	if (len != sizeof(*val)) {
>> +		kfree(buf);
>> +		nvmem_cell_put(cell);
>> +		return -EINVAL;
>> +	}
>> +	memcpy(val, buf, sizeof(*val));

This can overflow the memory allocated to val, we should be careful here 
not to do so.
limit this to sizeof(u32) should be good. Also add some sanity checks to 
make sure that len is atleast 4 bytes.


>> +
>> +	kfree(buf);
>> +	nvmem_cell_put(cell);
>> +	return 0;
>> +}
> The function looks nothing IMX specific, and could be a nvmem core
> function?
> 
> @Srinivas, thoughts?
Yep, this function looks generic, can be moved to nvmem layer.

Thanks,
srini

Re: [PATCH 2/4] thermal: imx: Add support for reading OCOTP through nvmem

[Leonard Crestez]

On Fri, 2017-07-14 at 09:48 +0100, Srinivas Kandagatla wrote:
> On 12/07/17 07:36, Shawn Guo wrote:

> > > +static int nvmem_cell_read_u32(struct device* dev, const char *cell_id, u32 *val)
> > > +{
> > > +	struct nvmem_cell *cell;
> > > +	void *buf;
> > > +	size_t len;
> > > +
> > > +	cell = nvmem_cell_get(dev, cell_id);
> > > +	if (IS_ERR(cell))
> > > +		return PTR_ERR(cell);
> > > +
> > > +	buf = nvmem_cell_read(cell, &len);
> > > +	if (IS_ERR(buf)) {
> > > +		nvmem_cell_put(cell);
> > > +		return PTR_ERR(buf);
> > > +	}
> > > +	if (len != sizeof(*val)) {
> > > +		kfree(buf);
> > > +		nvmem_cell_put(cell);
> > > +		return -EINVAL;
> > > +	}
> > > +	memcpy(val, buf, sizeof(*val));

> This can overflow the memory allocated to val, we should be careful here 
> not to do so.
> limit this to sizeof(u32) should be good. Also add some sanity checks to 
> make sure that len is atleast 4 bytes.

I'm not sure what you mean, isn't this already done? There is an
explicit check above that the read len is exactly as expected. It's
just that the limit is written as sizeof(*val) rather than sizeof(u32).

> > > +
> > > +	kfree(buf);
> > > +	nvmem_cell_put(cell);
> > > +	return 0;
> > > +}
> > The function looks nothing IMX specific, and could be a nvmem core
> > function?
> > 
> > @Srinivas, thoughts?
> Yep, this function looks generic, can be moved to nvmem layer.

Ok, next version of the series will have this function in nvmem core as
a separate commit.
--
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 2/4] thermal: imx: Add support for reading OCOTP through nvmem

[Srinivas Kandagatla]

On 14/07/17 11:49, Leonard Crestez wrote:
>>>> +	}
>>>> +	memcpy(val, buf, sizeof(*val));
>> This can overflow the memory allocated to val, we should be careful here
>> not to do so.
>> limit this to sizeof(u32) should be good. Also add some sanity checks to
>> make sure that len is atleast 4 bytes.
> I'm not sure what you mean, isn't this already done? There is an
> explicit check above that the read len is exactly as expected. It's
> just that the limit is written as sizeof(*val) rather than sizeof(u32).

Opps, I overlooked the type.. it looks okay.

thanks,
srini
> 

[PATCH 3/4] ARM: dts: imx6sx: Use nvmem-cells for tempmon

[Leonard Crestez]

On imx6sx accessing OCOTP directly is wrong because the ocotp clock
needs to be enabled first. Use the nvmem-cells binding instead.

This requirement does not apply to older imx6qdl chips because there the
ocotp access clock (clk_ipg_s) is always enabled.

This is visible by comparing the "System Clocks, Gating, and Override"
tables (OCOTP rows) in the 6DQ and 6SX manuals:
http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6SXRM.pdf
http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf

This happens to work right now because the ocotp clock might be enabled
for some other reason. In particular the it might be enabled from the
bootloader and it only gets disabled late during boot in
clk_disable_unused, after imx-thermal has completed probing.

If imx-thermal is compiled as a module then the system can hang on
probe.

Reported-by: Lothar Waßmann <LW@KARO-electronics.de>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm/boot/dts/imx6sx.dtsi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index f16b9df..5cfee85 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -675,7 +675,8 @@
 				compatible = "fsl,imx6sx-tempmon", "fsl,imx6q-tempmon";
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
 				fsl,tempmon = <&anatop>;
-				fsl,tempmon-data = <&ocotp>;
+				nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
+				nvmem-cell-names = "calib", "temp_grade";
 				clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
 			};
 
@@ -993,9 +994,17 @@
 			};
 
 			ocotp: ocotp@021bc000 {
+				#address-cells = <1>;
+				#size-cells = <1>;
 				compatible = "fsl,imx6sx-ocotp", "syscon";
 				reg = <0x021bc000 0x4000>;
 				clocks = <&clks IMX6SX_CLK_OCOTP>;
+				tempmon_calib: calib {
+					reg = <56 4>;
+				};
+				tempmon_temp_grade: temp_grade {
+					reg = <32 4>;
+				};
 			};
 
 			sai1: sai@021d4000 {
-- 
2.7.4

Re: [PATCH 3/4] ARM: dts: imx6sx: Use nvmem-cells for tempmon

[Shawn Guo]

On Thu, Jul 06, 2017 at 04:20:43PM +0300, Leonard Crestez wrote:
> On imx6sx accessing OCOTP directly is wrong because the ocotp clock
> needs to be enabled first. Use the nvmem-cells binding instead.
> 
> This requirement does not apply to older imx6qdl chips because there the
> ocotp access clock (clk_ipg_s) is always enabled.
> 
> This is visible by comparing the "System Clocks, Gating, and Override"
> tables (OCOTP rows) in the 6DQ and 6SX manuals:
> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6SXRM.pdf
> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf
> 
> This happens to work right now because the ocotp clock might be enabled
> for some other reason. In particular the it might be enabled from the
> bootloader and it only gets disabled late during boot in
> clk_disable_unused, after imx-thermal has completed probing.
> 
> If imx-thermal is compiled as a module then the system can hang on
> probe.
> 
> Reported-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> Signed-off-by: Leonard Crestez <leonard.crestez-3arQi8VN3Tc@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6sx.dtsi | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
> index f16b9df..5cfee85 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -675,7 +675,8 @@
>  				compatible = "fsl,imx6sx-tempmon", "fsl,imx6q-tempmon";
>  				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
>  				fsl,tempmon = <&anatop>;
> -				fsl,tempmon-data = <&ocotp>;
> +				nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> +				nvmem-cell-names = "calib", "temp_grade";
>  				clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
>  			};
>  
> @@ -993,9 +994,17 @@
>  			};
>  
>  			ocotp: ocotp@021bc000 {
> +				#address-cells = <1>;
> +				#size-cells = <1>;
>  				compatible = "fsl,imx6sx-ocotp", "syscon";
>  				reg = <0x021bc000 0x4000>;
>  				clocks = <&clks IMX6SX_CLK_OCOTP>;

Please have a newline between child node and property list ...

> +				tempmon_calib: calib {
> +					reg = <56 4>;
> +				};

... and between child nodes as well.

> +				tempmon_temp_grade: temp_grade {

We prefer to use hyphen rather than underscore in node name.  And node
name should have a corresponding unit-address, if the node has a 'reg'
property.

Shawn

> +					reg = <32 4>;
> +				};
>  			};
>  
>  			sai1: sai@021d4000 {
> -- 
> 2.7.4
> 
--
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 4/4] ARM: dts: imx6ul: Add imx6ul-tempmon

[Leonard Crestez]

This works identically to imx6sx-tempmon on both imx6ul and imx6ull.
It just needs to be defined in dts.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm/boot/dts/imx6ul.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 6da2b77..60084eb 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -598,6 +598,15 @@
 				fsl,anatop = <&anatop>;
 			};
 
+			tempmon: tempmon {
+				compatible = "fsl,imx6ul-tempmon", "fsl,imx6sx-tempmon";
+				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				fsl,tempmon = <&anatop>;
+				nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
+				nvmem-cell-names = "calib", "temp_grade";
+				clocks = <&clks IMX6UL_CLK_PLL3_USB_OTG>;
+			};
+
 			snvs: snvs@020cc000 {
 				compatible = "fsl,sec-v4.0-mon", "syscon", "simple-mfd";
 				reg = <0x020cc000 0x4000>;
@@ -860,9 +869,17 @@
 			};
 
 			ocotp: ocotp-ctrl@021bc000 {
+				#address-cells = <1>;
+				#size-cells = <1>;
 				compatible = "fsl,imx6ul-ocotp", "syscon";
 				reg = <0x021bc000 0x4000>;
 				clocks = <&clks IMX6UL_CLK_OCOTP>;
+				tempmon_calib: calib {
+					reg = <56 4>;
+				};
+				tempmon_temp_grade: temp_grade {
+					reg = <32 4>;
+				};
 			};
 
 			lcdif: lcdif@021c8000 {
-- 
2.7.4