The code is based on Amlogic source code. No public datasheet for this.
Currently the G12A SoCs are supported.
Supported features:
- possibility to set an automatic reboot temperature
Signed-off-by: Guillaume La Roque <[email protected]>
---
drivers/iio/temperature/Kconfig | 11 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/meson_tsensor.c | 416 ++++++++++++++++++++++++
3 files changed, 428 insertions(+)
create mode 100644 drivers/iio/temperature/meson_tsensor.c
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 737faa0901fe..712a0062790d 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -34,6 +34,17 @@ config HID_SENSOR_TEMP
To compile this driver as a module, choose M here: the module
will be called hid-sensor-temperature.
+config MESON_TSENSOR
+ tristate "Amlogic Meson temperature sensor Support"
+ default ARCH_MESON
+ depends on OF && ARCH_MESON
+ help
+ If you say yess here you get support for Meson Temperature sensor
+ for G12 SoC Family.
+
+ This driver can also be built as a module. If so, the module will
+ be called meson_tsensor.
+
config MLX90614
tristate "MLX90614 contact-less infrared sensor"
depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index baca4776ca0d..466d8c1c91d6 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
obj-$(CONFIG_MAX31856) += max31856.o
+obj-$(CONFIG_MESON_TSENSOR) += meson_tsensor.o
obj-$(CONFIG_MLX90614) += mlx90614.o
obj-$(CONFIG_MLX90632) += mlx90632.o
obj-$(CONFIG_TMP006) += tmp006.o
diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c
new file mode 100644
index 000000000000..be0a8d073ba3
--- /dev/null
+++ b/drivers/iio/temperature/meson_tsensor.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson Temperature Sensor
+ *
+ * Copyright (C) 2017 Huan Biao <[email protected]>
+ * Copyright (C) 2019 Guillaume La Roque <[email protected]>
+ *
+ * Register value to celsius temperature formulas:
+ * Read_Val m * U
+ * U = ---------, Uptat = ---------
+ * 2^16 1 + n * U
+ *
+ * Temperature = A * ( Uptat + u_efuse / 2^16 )- B
+ *
+ * A B m n : calibration parameters
+ * u_efuse : fused calibration value, it's a signed 16 bits value
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/iio/iio.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define TSENSOR_CFG_REG1 0x4
+ #define TSENSOR_CFG_REG1_RSET_VBG BIT(12)
+ #define TSENSOR_CFG_REG1_RSET_ADC BIT(11)
+ #define TSENSOR_CFG_REG1_VCM_EN BIT(10)
+ #define TSENSOR_CFG_REG1_VBG_EN BIT(9)
+ #define TSENSOR_CFG_REG1_OUT_CTL BIT(6)
+ #define TSENSOR_CFG_REG1_FILTER_EN BIT(5)
+ #define TSENSOR_CFG_REG1_DEM_EN BIT(3)
+ #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0)
+ #define TSENSOR_CFG_REG1_ENABLE \
+ (TSENSOR_CFG_REG1_FILTER_EN | \
+ TSENSOR_CFG_REG1_VCM_EN | \
+ TSENSOR_CFG_REG1_VBG_EN | \
+ TSENSOR_CFG_REG1_DEM_EN | \
+ TSENSOR_CFG_REG1_CH_SEL)
+
+#define TSENSOR_CFG_REG2 0x8
+ #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31)
+ #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30)
+ #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16)
+ #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \
+ (TSENSOR_CFG_REG2_HITEMP_EN | \
+ TSENSOR_CFG_REG2_REBOOT_ALL_EN | \
+ TSENSOR_CFG_REG2_REBOOT_TIME)
+ #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \
+ (GENMASK(31, 30) | GENMASK(25, 4))
+ #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \
+ GENMASK(15, 4)
+ #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \
+ (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \
+ _reg_val) | \
+ TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE)
+
+#define TSENSOR_CFG_REG3 0xC
+#define TSENSOR_CFG_REG4 0x10
+#define TSENSOR_CFG_REG5 0x14
+#define TSENSOR_CFG_REG6 0x18
+#define TSENSOR_CFG_REG7 0x1C
+#define TSENSOR_CFG_REG8 0x20
+
+#define TSENSOR_STAT0 0x40
+
+#define TSENSOR_STAT9 0x64
+
+#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0)
+#define TSENSOR_TEMP_MASK GENMASK(11, 0)
+
+#define TSENSOR_TRIM_SIGN_MASK BIT(15)
+#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0)
+#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24)
+
+#define TSENSOR_TRIM_VERSION(_version) \
+ FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version)
+
+#define TSENSOR_TRIM_CALIB_VALID_MASK (GENMASK(3, 2) | BIT(7))
+
+#define TSENSOR_CALIB_OFFSET 1
+#define TSENSOR_CALIB_SHIFT 4
+
+static const struct iio_chan_spec temperature_channel[] = {
+ {
+ .type = IIO_TEMP,
+ .channel = 0,
+ .address = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ },
+};
+
+/**
+ * struct meson_tsensor_soc_data
+ * @A, B, m, n: calibration parameters
+ * This structure is required for configuration of meson tsensor driver.
+ */
+struct meson_tsensor_soc_data {
+ int A;
+ int B;
+ int m;
+ int n;
+};
+
+/**
+ * struct meson_tsensor_data
+ * @u_efuse_off: register offset to read fused calibration value
+ * @soc: calibration parameters structure pointer
+ * @regmap_config: regmap config for the device
+ * This structure is required for configuration of meson tsensor driver.
+ */
+struct meson_tsensor_data {
+ int u_efuse_off;
+ const struct meson_tsensor_soc_data *soc;
+ const struct regmap_config *regmap_config;
+};
+
+struct meson_tsensor {
+ int id;
+ const struct meson_tsensor_data *data;
+ struct regmap *regmap;
+ struct regmap *sec_ao_map;
+ struct clk *clk;
+ u32 trim_info;
+ void __iomem *base;
+ int reboot_temp;
+};
+
+/*
+ * tsensor treats temperature as a mapped temperature code.
+ * The temperature is converted differently depending on the calibration type.
+ */
+static u32 temp_to_code(struct meson_tsensor *priv, int temp)
+{
+ const struct meson_tsensor_soc_data *param = priv->data->soc;
+ s64 divisor, factor, uefuse;
+ u32 reg_code;
+
+ uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+ ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+ (priv->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+ factor = BIT(16) * (temp * 10 + param->B);
+ factor = div_s64(factor, param->A);
+ factor = factor + uefuse;
+
+ factor = factor * 100;
+
+ divisor = param->n * factor;
+ divisor = div_s64(divisor, BIT(16));
+ divisor = param->m - divisor;
+
+ reg_code = div_s64(factor, divisor);
+ reg_code = ((reg_code >> TSENSOR_CALIB_SHIFT) & TSENSOR_TEMP_MASK) +
+ TSENSOR_CALIB_OFFSET;
+
+ return reg_code;
+}
+
+/*
+ * Calculate a temperature value from a temperature code.
+ * The unit of the temperature is degree Celsius.
+ */
+static int code_to_temp(struct meson_tsensor *priv, int temp_code)
+{
+ const struct meson_tsensor_soc_data *param = priv->data->soc;
+ int temp;
+ s64 factor, Uptat, uefuse;
+
+ uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+ ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+ (priv->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+ factor = param->n * temp_code;
+ factor = div_s64(factor, 100);
+
+ Uptat = temp_code * param->m;
+ Uptat = div_s64(Uptat, 100);
+ Uptat = Uptat * BIT(16);
+ Uptat = div_s64(Uptat, BIT(16) + factor);
+
+ temp = (Uptat + uefuse) * param->A;
+ temp = div_s64(temp, BIT(16));
+ temp = (temp - param->B) * 100;
+
+ return temp;
+}
+
+static int meson_tsensor_initialize(struct iio_dev *indio_dev)
+{
+ struct meson_tsensor *priv = iio_priv(indio_dev);
+ u32 reg_val;
+ int ret = 0;
+ int ver;
+
+ regmap_read(priv->sec_ao_map, priv->data->u_efuse_off,
+ &priv->trim_info);
+
+ ver = TSENSOR_TRIM_VERSION(priv->trim_info);
+
+ if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) {
+ ret = -EINVAL;
+ dev_err(&indio_dev->dev,
+ "tsensor thermal calibration not supported: 0x%x!\n",
+ ver);
+ goto out;
+ }
+
+ /* init the ts reboot soc function */
+ if (priv->reboot_temp) {
+ /* register need value in celsius */
+ reg_val = temp_to_code(priv, priv->reboot_temp / 1000);
+ regmap_update_bits(priv->regmap, TSENSOR_CFG_REG2,
+ TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK,
+ TSENSOR_CFG_REG2_HITEMP_REG_VAL(reg_val));
+ }
+
+out:
+ return ret;
+}
+
+static int meson_tsensor_enable(struct iio_dev *indio_dev)
+{
+ struct meson_tsensor *priv = iio_priv(indio_dev);
+
+ clk_prepare_enable(priv->clk);
+ regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
+ TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
+
+ return 0;
+}
+
+static int meson_tsensor_disable(struct iio_dev *indio_dev)
+{
+ struct meson_tsensor *priv = iio_priv(indio_dev);
+
+ regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
+ TSENSOR_CFG_REG1_ENABLE, 0);
+ clk_disable(priv->clk);
+
+ return 0;
+}
+
+static int meson_tsensor_read(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ unsigned int tvalue;
+ struct meson_tsensor *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_PROCESSED:
+ regmap_read(priv->regmap, TSENSOR_STAT0, &tvalue);
+ *val = code_to_temp(priv,
+ tvalue & TSENSOR_READ_TEMP_MASK);
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info meson_tsensor_iio_info = {
+ .read_raw = &meson_tsensor_read,
+};
+
+static const struct regmap_config meson_tsensor_regmap_config_g12a = {
+ .reg_bits = 8,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = TSENSOR_STAT9,
+};
+
+static const struct meson_tsensor_soc_data meson_tsensor_g12a = {
+ .A = 9411,
+ .B = 3159,
+ .m = 424,
+ .n = 324,
+};
+
+static const struct meson_tsensor_data meson_tsensor_g12a_cpu_param = {
+ .u_efuse_off = 0x128,
+ .soc = &meson_tsensor_g12a,
+ .regmap_config = &meson_tsensor_regmap_config_g12a,
+};
+
+static const struct meson_tsensor_data meson_tsensor_g12a_ddr_param = {
+ .u_efuse_off = 0xF0,
+ .soc = &meson_tsensor_g12a,
+ .regmap_config = &meson_tsensor_regmap_config_g12a,
+};
+
+static const struct of_device_id meson_tsensor_of_match[] = {
+ {
+ .compatible = "amlogic,meson-g12a-ddr-tsensor",
+ .data = &meson_tsensor_g12a_ddr_param,
+ },
+ {
+ .compatible = "amlogic,meson-g12a-cpu-tsensor",
+ .data = &meson_tsensor_g12a_cpu_param,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, meson_tsensor_of_match);
+
+static int meson_tsensor_probe(struct platform_device *pdev)
+{
+ struct meson_tsensor *priv;
+ struct iio_dev *indio_dev;
+ struct resource *res;
+
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+ if (!indio_dev) {
+ dev_err(&pdev->dev, "failed allocating iio device\n");
+ return -ENOMEM;
+ }
+
+ priv = iio_priv(indio_dev);
+ priv->data = of_device_get_match_data(&pdev->dev);
+ if (!priv->data) {
+ dev_err(&pdev->dev, "failed to get match data\n");
+ return -ENODEV;
+ }
+
+ indio_dev->channels = temperature_channel;
+ indio_dev->num_channels = ARRAY_SIZE(temperature_channel);
+ indio_dev->name = dev_name(&pdev->dev);
+ indio_dev->dev.parent = &pdev->dev;
+ indio_dev->dev.of_node = pdev->dev.of_node;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &meson_tsensor_iio_info;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ priv->regmap = devm_regmap_init_mmio(&pdev->dev, priv->base,
+ priv->data->regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ priv->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->clk)) {
+ if (PTR_ERR(priv->clk) != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get clock\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ if (of_property_read_u32(pdev->dev.of_node,
+ "amlogic,critical-temperature",
+ &priv->reboot_temp)) {
+ priv->reboot_temp = 0;
+ }
+
+ priv->sec_ao_map = syscon_regmap_lookup_by_phandle
+ (pdev->dev.of_node, "amlogic,ao-secure");
+ if (IS_ERR(priv->sec_ao_map)) {
+ dev_err(&pdev->dev, "syscon regmap lookup failed.\n");
+ return PTR_ERR(priv->sec_ao_map);
+ }
+
+ ret = meson_tsensor_initialize(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = meson_tsensor_enable(indio_dev);
+ if (ret)
+ goto err;
+
+ platform_set_drvdata(pdev, indio_dev);
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto err_hw;
+
+ return 0;
+
+err_hw:
+ meson_tsensor_disable(indio_dev);
+err:
+ clk_unprepare(priv->clk);
+
+ return ret;
+}
+
+static int meson_tsensor_remove(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+ iio_device_unregister(indio_dev);
+
+ return meson_tsensor_disable(indio_dev);
+}
+
+static struct platform_driver meson_tsensor_driver = {
+ .probe = meson_tsensor_probe,
+ .remove = meson_tsensor_remove,
+ .driver = {
+ .name = "meson-tsensor",
+ .of_match_table = meson_tsensor_of_match,
+ },
+};
+
+module_platform_driver(meson_tsensor_driver);
+
+MODULE_AUTHOR("Guillaume La Roque <[email protected]>");
+MODULE_DESCRIPTION("Amlogic Meson Temperature Sensor Driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
Hi Guillaume,
below are my initial impressions. I will have a closer look once
there's a decision whether this belongs to the IIO or thermal
framework.
On Tue, Jun 4, 2019 at 4:48 PM Guillaume La Roque <[email protected]> wrote:
>
> The code is based on Amlogic source code. No public datasheet for this.
the public S922X datasheet from Hardkernel [0] has some documentation
(starting at page 1106).
[...]
> +config MESON_TSENSOR
> + tristate "Amlogic Meson temperature sensor Support"
> + default ARCH_MESON
> + depends on OF && ARCH_MESON
depends on OF && (ARCH_MESON || COMPILE_TEST)
so all the nice auto-builders / testing tools will speak up if someone
tries to break your driver
> + help
> + If you say yess here you get support for Meson Temperature sensor
s/yess/yes/
> + for G12 SoC Family.
G12 (which I assume includes G12A and G12B) or G12A?
[...]
> diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c
> new file mode 100644
> index 000000000000..be0a8d073ba3
> --- /dev/null
> +++ b/drivers/iio/temperature/meson_tsensor.c
> @@ -0,0 +1,416 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson Temperature Sensor
> + *
> + * Copyright (C) 2017 Huan Biao <[email protected]>
> + * Copyright (C) 2019 Guillaume La Roque <[email protected]>
> + *
> + * Register value to celsius temperature formulas:
> + * Read_Val m * U
> + * U = ---------, Uptat = ---------
> + * 2^16 1 + n * U
> + *
> + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B
> + *
> + * A B m n : calibration parameters
> + * u_efuse : fused calibration value, it's a signed 16 bits value
it's great to have this explained in the docs (instead of having to
look it up in some out of tree driver, as it's not part of the
datasheet) - thank you for this!
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/iio/iio.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define TSENSOR_CFG_REG1 0x4
> + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12)
> + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11)
> + #define TSENSOR_CFG_REG1_VCM_EN BIT(10)
> + #define TSENSOR_CFG_REG1_VBG_EN BIT(9)
> + #define TSENSOR_CFG_REG1_OUT_CTL BIT(6)
> + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5)
> + #define TSENSOR_CFG_REG1_DEM_EN BIT(3)
> + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0)
> + #define TSENSOR_CFG_REG1_ENABLE \
> + (TSENSOR_CFG_REG1_FILTER_EN | \
> + TSENSOR_CFG_REG1_VCM_EN | \
> + TSENSOR_CFG_REG1_VBG_EN | \
> + TSENSOR_CFG_REG1_DEM_EN | \
> + TSENSOR_CFG_REG1_CH_SEL)
are all of these needed to enabled *and* disable the temperature
sensor? TSENSOR_CFG_REG1_CH_SEL doesn't seem related when disabling
the sensor reading (but I don't know since there's no documentation)
> +#define TSENSOR_CFG_REG2 0x8
> + #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31)
> + #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30)
> + #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16)
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \
> + (TSENSOR_CFG_REG2_HITEMP_EN | \
> + TSENSOR_CFG_REG2_REBOOT_ALL_EN | \
> + TSENSOR_CFG_REG2_REBOOT_TIME)
the name mix between TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE and
TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK confused me.
personally I would drop the macros which just bit-wise or multiple
other macros together
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \
> + (GENMASK(31, 30) | GENMASK(25, 4))
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \
> + GENMASK(15, 4)
> + #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \
> + (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \
> + _reg_val) | \
> + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE)
> +
> +#define TSENSOR_CFG_REG3 0xC
I like to use lower-case hex letters
and I also pad them -> 0x0c in this case because we have for example 0x10 below)
> +#define TSENSOR_CFG_REG4 0x10
> +#define TSENSOR_CFG_REG5 0x14
> +#define TSENSOR_CFG_REG6 0x18
> +#define TSENSOR_CFG_REG7 0x1C
> +#define TSENSOR_CFG_REG8 0x20
> +
> +#define TSENSOR_STAT0 0x40
> +
> +#define TSENSOR_STAT9 0x64
> +
> +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0)
TSENSOR_STAT0_FILTER_OUT would match the naming from the datasheet
> +#define TSENSOR_TEMP_MASK GENMASK(11, 0)
>
> +#define TSENSOR_TRIM_SIGN_MASK BIT(15)
> +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0)
> +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24)
> +
> +#define TSENSOR_TRIM_VERSION(_version) \
> + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version)
I would drop this and use the FIELD_GET directly where needed (it's
only one occurrence anyways)
[...]
> +static int meson_tsensor_enable(struct iio_dev *indio_dev)
> +{
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> +
> + clk_prepare_enable(priv->clk);
may return an error which you're discarding here
> + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
> + TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
> +
> + return 0;
> +}
> +
> +static int meson_tsensor_disable(struct iio_dev *indio_dev)
> +{
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> +
> + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
> + TSENSOR_CFG_REG1_ENABLE, 0);
> + clk_disable(priv->clk);
clk_disable_unprepare as you're calling clk_prepare_enable above?
> +
> + return 0;
make it a void function instead?
> +static const struct regmap_config meson_tsensor_regmap_config_g12a = {
> + .reg_bits = 8,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = TSENSOR_STAT9,
.fast_io = true
if you ever need to ACK interrupts from the IRQ handler
(IIRC fast_io will use a spinlock instead of mutex)
[...]
> +static const struct of_device_id meson_tsensor_of_match[] = {
> + {
> + .compatible = "amlogic,meson-g12a-ddr-tsensor",
> + .data = &meson_tsensor_g12a_ddr_param,
> + },
> + {
> + .compatible = "amlogic,meson-g12a-cpu-tsensor",
> + .data = &meson_tsensor_g12a_cpu_param,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, meson_tsensor_of_match);
I would move the of_device_id table above the platform_driver definition below
of_device_get_match_data doesn't need the of_device_id as parameter
(compared to it's predecessor)
> +static int meson_tsensor_probe(struct platform_device *pdev)
> +{
> + struct meson_tsensor *priv;
> + struct iio_dev *indio_dev;
> + struct resource *res;
> +
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + priv = iio_priv(indio_dev);
> + priv->data = of_device_get_match_data(&pdev->dev);
> + if (!priv->data) {
> + dev_err(&pdev->dev, "failed to get match data\n");
> + return -ENODEV;
> + }
> +
> + indio_dev->channels = temperature_channel;
> + indio_dev->num_channels = ARRAY_SIZE(temperature_channel);
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &meson_tsensor_iio_info;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(&pdev->dev, res);
you're only using priv->base in this function. consider dropping it
from struct meson_tsensor
[0] https://dn.odroid.com/S922X/ODROID-N2/Datasheet/S922X_Public_Datasheet_V0.2.pdf
On Tue, 4 Jun 2019 16:47:14 +0200
Guillaume La Roque <[email protected]> wrote:
> The code is based on Amlogic source code. No public datasheet for this.
> Currently the G12A SoCs are supported.
>
> Supported features:
> - possibility to set an automatic reboot temperature
>
> Signed-off-by: Guillaume La Roque <[email protected]>
A few comments inline.
However, I'm still of the view this is probably better as a thermal
driver than an IIO one.
Jonathan
> ---
> drivers/iio/temperature/Kconfig | 11 +
> drivers/iio/temperature/Makefile | 1 +
> drivers/iio/temperature/meson_tsensor.c | 416 ++++++++++++++++++++++++
> 3 files changed, 428 insertions(+)
> create mode 100644 drivers/iio/temperature/meson_tsensor.c
>
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 737faa0901fe..712a0062790d 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -34,6 +34,17 @@ config HID_SENSOR_TEMP
> To compile this driver as a module, choose M here: the module
> will be called hid-sensor-temperature.
>
> +config MESON_TSENSOR
> + tristate "Amlogic Meson temperature sensor Support"
> + default ARCH_MESON
> + depends on OF && ARCH_MESON
> + help
> + If you say yess here you get support for Meson Temperature sensor
> + for G12 SoC Family.
> +
> + This driver can also be built as a module. If so, the module will
> + be called meson_tsensor.
> +
> config MLX90614
> tristate "MLX90614 contact-less infrared sensor"
> depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index baca4776ca0d..466d8c1c91d6 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
> obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
> obj-$(CONFIG_MAX31856) += max31856.o
> +obj-$(CONFIG_MESON_TSENSOR) += meson_tsensor.o
> obj-$(CONFIG_MLX90614) += mlx90614.o
> obj-$(CONFIG_MLX90632) += mlx90632.o
> obj-$(CONFIG_TMP006) += tmp006.o
> diff --git a/drivers/iio/temperature/meson_tsensor.c b/drivers/iio/temperature/meson_tsensor.c
> new file mode 100644
> index 000000000000..be0a8d073ba3
> --- /dev/null
> +++ b/drivers/iio/temperature/meson_tsensor.c
> @@ -0,0 +1,416 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson Temperature Sensor
> + *
> + * Copyright (C) 2017 Huan Biao <[email protected]>
> + * Copyright (C) 2019 Guillaume La Roque <[email protected]>
> + *
> + * Register value to celsius temperature formulas:
> + * Read_Val m * U
> + * U = ---------, Uptat = ---------
> + * 2^16 1 + n * U
> + *
> + * Temperature = A * ( Uptat + u_efuse / 2^16 )- B
> + *
> + * A B m n : calibration parameters
> + * u_efuse : fused calibration value, it's a signed 16 bits value
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/iio/iio.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define TSENSOR_CFG_REG1 0x4
> + #define TSENSOR_CFG_REG1_RSET_VBG BIT(12)
> + #define TSENSOR_CFG_REG1_RSET_ADC BIT(11)
> + #define TSENSOR_CFG_REG1_VCM_EN BIT(10)
> + #define TSENSOR_CFG_REG1_VBG_EN BIT(9)
> + #define TSENSOR_CFG_REG1_OUT_CTL BIT(6)
> + #define TSENSOR_CFG_REG1_FILTER_EN BIT(5)
> + #define TSENSOR_CFG_REG1_DEM_EN BIT(3)
> + #define TSENSOR_CFG_REG1_CH_SEL GENMASK(1, 0)
> + #define TSENSOR_CFG_REG1_ENABLE \
> + (TSENSOR_CFG_REG1_FILTER_EN | \
> + TSENSOR_CFG_REG1_VCM_EN | \
> + TSENSOR_CFG_REG1_VBG_EN | \
> + TSENSOR_CFG_REG1_DEM_EN | \
> + TSENSOR_CFG_REG1_CH_SEL)
> +
> +#define TSENSOR_CFG_REG2 0x8
> + #define TSENSOR_CFG_REG2_HITEMP_EN BIT(31)
> + #define TSENSOR_CFG_REG2_REBOOT_ALL_EN BIT(30)
> + #define TSENSOR_CFG_REG2_REBOOT_TIME GENMASK(25, 16)
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE \
> + (TSENSOR_CFG_REG2_HITEMP_EN | \
> + TSENSOR_CFG_REG2_REBOOT_ALL_EN | \
> + TSENSOR_CFG_REG2_REBOOT_TIME)
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK \
> + (GENMASK(31, 30) | GENMASK(25, 4))
> + #define TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK \
> + GENMASK(15, 4)
> + #define TSENSOR_CFG_REG2_HITEMP_REG_VAL(_reg_val) \
> + (FIELD_PREP(TSENSOR_CFG_REG2_HITEMP_REBOOT_REG_MASK, \
> + _reg_val) | \
> + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE)
> +
> +#define TSENSOR_CFG_REG3 0xC
> +#define TSENSOR_CFG_REG4 0x10
> +#define TSENSOR_CFG_REG5 0x14
> +#define TSENSOR_CFG_REG6 0x18
> +#define TSENSOR_CFG_REG7 0x1C
> +#define TSENSOR_CFG_REG8 0x20
> +
> +#define TSENSOR_STAT0 0x40
> +
> +#define TSENSOR_STAT9 0x64
> +
> +#define TSENSOR_READ_TEMP_MASK GENMASK(15, 0)
> +#define TSENSOR_TEMP_MASK GENMASK(11, 0)
> +
> +#define TSENSOR_TRIM_SIGN_MASK BIT(15)
> +#define TSENSOR_TRIM_TEMP_MASK GENMASK(14, 0)
> +#define TSENSOR_TRIM_VERSION_MASK GENMASK(31, 24)
> +
> +#define TSENSOR_TRIM_VERSION(_version) \
> + FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version)
I'd just use the FIELD_GET directly inline. What it is doing
is kind of obvious from the parameter.
> +
> +#define TSENSOR_TRIM_CALIB_VALID_MASK (GENMASK(3, 2) | BIT(7))
> +
> +#define TSENSOR_CALIB_OFFSET 1
> +#define TSENSOR_CALIB_SHIFT 4
> +
> +static const struct iio_chan_spec temperature_channel[] = {
> + {
> + .type = IIO_TEMP,
> + .channel = 0,
.channel doesn't do anything unless you set indexed.
> + .address = 0,
.address not used...
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + },
> +};
> +
> +/**
> + * struct meson_tsensor_soc_data
> + * @A, B, m, n: calibration parameters
> + * This structure is required for configuration of meson tsensor driver.
> + */
> +struct meson_tsensor_soc_data {
> + int A;
> + int B;
> + int m;
> + int n;
> +};
> +
> +/**
> + * struct meson_tsensor_data
> + * @u_efuse_off: register offset to read fused calibration value
> + * @soc: calibration parameters structure pointer
> + * @regmap_config: regmap config for the device
> + * This structure is required for configuration of meson tsensor driver.
> + */
> +struct meson_tsensor_data {
> + int u_efuse_off;
> + const struct meson_tsensor_soc_data *soc;
> + const struct regmap_config *regmap_config;
> +};
> +
> +struct meson_tsensor {
> + int id;
> + const struct meson_tsensor_data *data;
> + struct regmap *regmap;
> + struct regmap *sec_ao_map;
> + struct clk *clk;
> + u32 trim_info;
> + void __iomem *base;
> + int reboot_temp;
> +};
> +
> +/*
> + * tsensor treats temperature as a mapped temperature code.
> + * The temperature is converted differently depending on the calibration type.
> + */
> +static u32 temp_to_code(struct meson_tsensor *priv, int temp)
> +{
> + const struct meson_tsensor_soc_data *param = priv->data->soc;
> + s64 divisor, factor, uefuse;
> + u32 reg_code;
> +
> + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ?
> + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
> + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK);
> +
> + factor = BIT(16) * (temp * 10 + param->B);
> + factor = div_s64(factor, param->A);
> + factor = factor + uefuse;
> +
> + factor = factor * 100;
> +
> + divisor = param->n * factor;
> + divisor = div_s64(divisor, BIT(16));
> + divisor = param->m - divisor;
> +
> + reg_code = div_s64(factor, divisor);
> + reg_code = ((reg_code >> TSENSOR_CALIB_SHIFT) & TSENSOR_TEMP_MASK) +
> + TSENSOR_CALIB_OFFSET;
> +
> + return reg_code;
> +}
> +
> +/*
> + * Calculate a temperature value from a temperature code.
> + * The unit of the temperature is degree Celsius.
> + */
> +static int code_to_temp(struct meson_tsensor *priv, int temp_code)
> +{
> + const struct meson_tsensor_soc_data *param = priv->data->soc;
> + int temp;
> + s64 factor, Uptat, uefuse;
> +
> + uefuse = priv->trim_info & TSENSOR_TRIM_SIGN_MASK ?
> + ~(priv->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
> + (priv->trim_info & TSENSOR_TRIM_TEMP_MASK);
Hmm. 2's complement from unsigned + a sign bit.
uefuse = priv->trim_info & TSENSOR_TRIM_TEMP_MASK;
if (priv->trim_info & TSENSOR_TRIM_SIGN_MASK)
uefuse *= -1;
Is probably going to do the same thing whilst being more readable.
> +
> + factor = param->n * temp_code;
> + factor = div_s64(factor, 100);
> +
> + Uptat = temp_code * param->m;
Why the capital U? Not really kernel style.
> + Uptat = div_s64(Uptat, 100);
> + Uptat = Uptat * BIT(16);
> + Uptat = div_s64(Uptat, BIT(16) + factor);
> +
> + temp = (Uptat + uefuse) * param->A;
> + temp = div_s64(temp, BIT(16));
> + temp = (temp - param->B) * 100;
> +
> + return temp;
> +}
> +
> +static int meson_tsensor_initialize(struct iio_dev *indio_dev)
> +{
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> + u32 reg_val;
> + int ret = 0;
> + int ver;
> +
> + regmap_read(priv->sec_ao_map, priv->data->u_efuse_off,
> + &priv->trim_info);
> +
> + ver = TSENSOR_TRIM_VERSION(priv->trim_info);
> +
> + if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) {
> + ret = -EINVAL;
> + dev_err(&indio_dev->dev,
> + "tsensor thermal calibration not supported: 0x%x!\n",
> + ver);
> + goto out;
return -EINVAL;
> + }
> +
> + /* init the ts reboot soc function */
> + if (priv->reboot_temp) {
> + /* register need value in celsius */
> + reg_val = temp_to_code(priv, priv->reboot_temp / 1000);
> + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG2,
> + TSENSOR_CFG_REG2_HITEMP_REBOOT_ENABLE_MASK,
> + TSENSOR_CFG_REG2_HITEMP_REG_VAL(reg_val));
> + }
> +
return 0;
> +out:
> + return ret;
> +}
> +
> +static int meson_tsensor_enable(struct iio_dev *indio_dev)
> +{
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> +
> + clk_prepare_enable(priv->clk);
> + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
> + TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
> +
> + return 0;
Given neither this nor the follow up have any error paths,
stop them having a return value and you can drop some error handling..
> +}
> +
> +static int meson_tsensor_disable(struct iio_dev *indio_dev)
> +{
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> +
> + regmap_update_bits(priv->regmap, TSENSOR_CFG_REG1,
> + TSENSOR_CFG_REG1_ENABLE, 0);
> + clk_disable(priv->clk);
> +
> + return 0;
> +}
> +
> +static int meson_tsensor_read(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + unsigned int tvalue;
> + struct meson_tsensor *priv = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_PROCESSED:
> + regmap_read(priv->regmap, TSENSOR_STAT0, &tvalue);
> + *val = code_to_temp(priv,
> + tvalue & TSENSOR_READ_TEMP_MASK);
Looks to me like this will fit on one line under 80 chars.
> +
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info meson_tsensor_iio_info = {
> + .read_raw = &meson_tsensor_read,
> +};
> +
> +static const struct regmap_config meson_tsensor_regmap_config_g12a = {
> + .reg_bits = 8,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = TSENSOR_STAT9,
> +};
> +
> +static const struct meson_tsensor_soc_data meson_tsensor_g12a = {
> + .A = 9411,
> + .B = 3159,
> + .m = 424,
> + .n = 324,
> +};
> +
> +static const struct meson_tsensor_data meson_tsensor_g12a_cpu_param = {
> + .u_efuse_off = 0x128,
> + .soc = &meson_tsensor_g12a,
> + .regmap_config = &meson_tsensor_regmap_config_g12a,
> +};
> +
> +static const struct meson_tsensor_data meson_tsensor_g12a_ddr_param = {
> + .u_efuse_off = 0xF0,
> + .soc = &meson_tsensor_g12a,
> + .regmap_config = &meson_tsensor_regmap_config_g12a,
I'm going to guess there is support for other devices on it's way where
the regmap_config and soc are different?
If it is going to be a while I'd prefer you moved them into this param
structure only when you need to.
> +};
> +
> +static const struct of_device_id meson_tsensor_of_match[] = {
> + {
> + .compatible = "amlogic,meson-g12a-ddr-tsensor",
Given we don't do a match on the compatible without the type specified
g12a-tsensor, is there any reason to have that in the binding?
> + .data = &meson_tsensor_g12a_ddr_param,
> + },
> + {
> + .compatible = "amlogic,meson-g12a-cpu-tsensor",
> + .data = &meson_tsensor_g12a_cpu_param,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, meson_tsensor_of_match);
> +
> +static int meson_tsensor_probe(struct platform_device *pdev)
> +{
> + struct meson_tsensor *priv;
> + struct iio_dev *indio_dev;
> + struct resource *res;
> +
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + priv = iio_priv(indio_dev);
> + priv->data = of_device_get_match_data(&pdev->dev);
> + if (!priv->data) {
> + dev_err(&pdev->dev, "failed to get match data\n");
> + return -ENODEV;
> + }
> +
> + indio_dev->channels = temperature_channel;
> + indio_dev->num_channels = ARRAY_SIZE(temperature_channel);
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &meson_tsensor_iio_info;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + priv->regmap = devm_regmap_init_mmio(&pdev->dev, priv->base,
> + priv->data->regmap_config);
> + if (IS_ERR(priv->regmap))
> + return PTR_ERR(priv->regmap);
> +
> + priv->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->clk)) {
> + if (PTR_ERR(priv->clk) != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get clock\n");
> + return PTR_ERR(priv->clk);
> + }
> +
> + if (of_property_read_u32(pdev->dev.of_node,
> + "amlogic,critical-temperature",
> + &priv->reboot_temp)) {
> + priv->reboot_temp = 0;
> + }
> +
> + priv->sec_ao_map = syscon_regmap_lookup_by_phandle
> + (pdev->dev.of_node, "amlogic,ao-secure");
> + if (IS_ERR(priv->sec_ao_map)) {
> + dev_err(&pdev->dev, "syscon regmap lookup failed.\n");
> + return PTR_ERR(priv->sec_ao_map);
> + }
> +
> + ret = meson_tsensor_initialize(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = meson_tsensor_enable(indio_dev);
> + if (ret)
> + goto err;
> +
> + platform_set_drvdata(pdev, indio_dev);
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err_hw;
> +
> + return 0;
> +
> +err_hw:
> + meson_tsensor_disable(indio_dev);
> +err:
> + clk_unprepare(priv->clk);
> +
> + return ret;
> +}
> +
> +static int meson_tsensor_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> + iio_device_unregister(indio_dev);
> +
> + return meson_tsensor_disable(indio_dev);
Is there a missing clk_unprepare in here?
One option to think about is whether to use devm_add_action_or_reset
to move all cleanup into the device managed queue. That way you can
drop the error handling in probe and get rid of remove entirely.
> +}
> +
> +static struct platform_driver meson_tsensor_driver = {
> + .probe = meson_tsensor_probe,
> + .remove = meson_tsensor_remove,
> + .driver = {
> + .name = "meson-tsensor",
> + .of_match_table = meson_tsensor_of_match,
> + },
> +};
> +
> +module_platform_driver(meson_tsensor_driver);
> +
> +MODULE_AUTHOR("Guillaume La Roque <[email protected]>");
> +MODULE_DESCRIPTION("Amlogic Meson Temperature Sensor Driver");
> +MODULE_LICENSE("GPL v2");