[PATCH] thermal: imx: interpret fsl,tempmon-data through nvmem
Eduardo Valentin
edubezval at gmail.com
Tue Jul 4 11:54:03 PDT 2017
On Mon, Jun 19, 2017 at 04:40:43PM +0300, Leonard Crestez wrote:
> On imx6sx accessing the ocotp memory area directly is wrong because the
> ocotp clock needs to be enabled first. Fix this by reinterpreting the
> fsl,tempmon-data phandle as a reference to a nvmem_device and doing all
> the read through that.
>
> This clock 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 without this patch because the ocotp
> clock might be enabled for some other reason. In particular 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.
>
> This makes IMX_THERMAL depend on NVMEM_IMX_OCOTP but that driver seems
> be already available for all chips that contain tempmon so it's
> acceptable.
>
> Reported-by: Lothar Waßmann <LW at KARO-electronics.de>
> Signed-off-by: Leonard Crestez <leonard.crestez at nxp.com>
>
> ---
>
> This was reported as a comment to a patch adding tempmon support for
> imx6ul (which is very similar to imx6sx). Since it already affects a
> supported chip this patch is sent as a separate bugfix.
>
> Link: https://lkml.org/lkml/2017/6/9/578
>
> There are various other ways to fix this problem. The main advantage of
> this solution is that it does not add a new binding but rather preserves
> compatibility with old DTBs. It also aligns with the idea that
> devicetree describes hardware relationships rather than a specific linux
> implementation.
>
> An alternative would have been to add a nvmem-cells binding to imx-thermal and
> use that if available instead of fsl,tempmon-data. It might not be good to
> sidestep the official nvmem bindings, the devicetree people were added so that
> they have an opportunity to object.
>
> In theory the "thermal grade" is a two-bit quantity and might be a
> candidate for using a cell with a "bits" binding. However this causes
> the nvmem core to issue reads of length and alignment less than 4 to the
> imx-ocotp driver so additional fixes might be required.
>
> drivers/nvmem/core.c | 15 ++++++++++++
> drivers/thermal/Kconfig | 2 +-
> drivers/thermal/imx_thermal.c | 53 ++++++++++++++++++++++++++----------------
> include/linux/nvmem-consumer.h | 6 +++++
> 4 files changed, 55 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 8c830a8..66502ca 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -630,6 +630,21 @@ struct nvmem_device *of_nvmem_device_get(struct device_node *np, const char *id)
> return __nvmem_device_get(nvmem_np, NULL, NULL);
> }
> EXPORT_SYMBOL_GPL(of_nvmem_device_get);
> +
> +/**
> + * of_nvmem_device_phandle_get() - Get nvmem device from a given phandle
> + *
> + * @nvmem_np: Device tree node for the nvmem device
> + *
> + * Return: ERR_PTR() on error or a valid pointer to a struct nvmem_device
> + * on success.
> + */
> +struct nvmem_device *of_nvmem_device_phandle_get(struct device_node *nvmem_np)
> +{
> +
> + return __nvmem_device_get(nvmem_np, NULL, NULL);
> +}
> +EXPORT_SYMBOL_GPL(of_nvmem_device_phandle_get);
> #endif
>
> /**
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index b5b5fac..a427936 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -206,7 +206,7 @@ config HISI_THERMAL
> config IMX_THERMAL
> tristate "Temperature sensor driver for Freescale i.MX SoCs"
> depends on (ARCH_MXC && CPU_THERMAL) || COMPILE_TEST
> - depends on MFD_SYSCON
> + depends on NVMEM_IMX_OCOTP
> depends on OF
> help
> Support for Temperature Monitor (TEMPMON) found on Freescale i.MX SoCs.
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index fb648a4..1cf35bd 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
> @@ -55,8 +56,8 @@
> #define TEMPSENSE2_PANIC_VALUE_SHIFT 16
> #define TEMPSENSE2_PANIC_VALUE_MASK 0xfff0000
>
> -#define OCOTP_MEM0 0x0480
> -#define OCOTP_ANA1 0x04e0
> +#define OCOTP_MEM0_OFFSET 32
> +#define OCOTP_ANA1_OFFSET 56
>
> /* The driver supports 1 passive trip point and 1 critical trip point */
> enum imx_thermal_trip {
> @@ -347,29 +348,39 @@ static struct thermal_zone_device_ops imx_tz_ops = {
> static int imx_get_sensor_data(struct platform_device *pdev)
> {
> struct imx_thermal_data *data = platform_get_drvdata(pdev);
> - struct regmap *map;
> + struct device_node *ocotp_np;
> + struct nvmem_device *ocotp;
> 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);
> + ocotp_np = of_parse_phandle(pdev->dev.of_node, "fsl,tempmon-data", 0);
> + if (IS_ERR(ocotp_np)) {
> + ret = PTR_ERR(ocotp_np);
> + dev_err(&pdev->dev, "failed to parse fsl,tempmon-data phandle: %d\n", ret);
> + return ret;
> + }
> + ocotp = of_nvmem_device_phandle_get(ocotp_np);
> + of_node_put(ocotp_np);
> + if (IS_ERR(ocotp)) {
> + ret = PTR_ERR(ocotp);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get fsl,tempmon-data nvmem device: %d\n", ret);
> return ret;
> }
>
> - ret = regmap_read(map, OCOTP_ANA1, &val);
> - if (ret) {
> + ret = nvmem_device_read(ocotp, OCOTP_ANA1_OFFSET, sizeof(val), &val);
> + if (ret != sizeof(val)) {
> dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
> - return ret;
> + ret = -EIO;
> + goto out;
> }
>
> if (val == 0 || val == ~0) {
> dev_err(&pdev->dev, "invalid sensor calibration data\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
>
> /*
> @@ -404,10 +415,11 @@ static int imx_get_sensor_data(struct platform_device *pdev)
> 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;
I see a few other occurences of regmap_read() in this driver, for
example, inside imx_get_temp(). Do they also get affect by the reported
bug? Should they be replaced with nvmem_device_read() too?
> + ret = nvmem_device_read(ocotp, OCOTP_MEM0_OFFSET, sizeof(val), &val);
> + if (ret != sizeof(val)) {
> + dev_err(&pdev->dev, "failed to read sensor data: %d\n", ret);
> + ret = -EIO;
> + goto out;
> }
>
> /* The maximum die temp is specified by the Temperature Grade */
> @@ -437,7 +449,10 @@ 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);
>
> - return 0;
> + ret = 0;
> +out:
> + nvmem_device_put(ocotp);
> + return ret;
> }
>
> static irqreturn_t imx_thermal_alarm_irq(int irq, void *dev)
> @@ -513,10 +528,8 @@ 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");
> + if (ret)
> return ret;
> - }
>
> /* Make sure sensor is in known good state for measurements */
> regmap_write(map, TEMPSENSE0 + REG_CLR, TEMPSENSE0_POWER_DOWN);
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index c2256d7..3606167 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -140,6 +140,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> const char *name);
> struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> const char *name);
> +struct nvmem_device *of_nvmem_device_phandle_get(struct device_node *nvmem_np);
> #else
> static inline struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> const char *name)
> @@ -152,6 +153,11 @@ static inline struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> {
> return ERR_PTR(-ENOSYS);
> }
> +
> +static inline struct nvmem_device *of_nvmem_device_phandle_get(struct device_node *nvmem_np)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> #endif /* CONFIG_NVMEM && CONFIG_OF */
>
> #endif /* ifndef _LINUX_NVMEM_CONSUMER_H */
> --
> 2.7.4
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170704/07e7daea/attachment-0001.sig>
More information about the linux-arm-kernel
mailing list