From: quentin.schulz@free-electrons.com (Quentin Schulz)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
Date: Wed, 29 Mar 2017 08:54:05 +0200 [thread overview]
Message-ID: <813dc1d8-89b4-9c0e-c1cc-e76efde2be1f@free-electrons.com> (raw)
In-Reply-To: <20170328173013.16539-3-icenowy@aosc.io>
Hi,
On 28/03/2017 19:30, Icenowy Zheng wrote:
> This adds support for the Allwinner H3 thermal sensor.
>
> Allwinner H3 has a thermal sensor like the one in A33, but have its
> registers nearly all re-arranged, sample clock moved to CCU and a pair
> of bus clock and reset added. It's also the base of newer SoCs' thermal
> sensors.
>
> An option is added to gpadc_data struct, to indicate whether this device
> is a new-generation Allwinner thermal sensor.
>
> The thermal sensors on A64 and H5 is like the one on H3, but with of
> course different formula factors.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> drivers/iio/adc/sun4i-gpadc-iio.c | 130 ++++++++++++++++++++++++++++++++------
> include/linux/mfd/sun4i-gpadc.h | 33 +++++++++-
> 2 files changed, 141 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 74705aa37982..7512b1cae877 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -22,6 +22,7 @@
> * shutdown for not being used.
> */
>
> +#include <linux/clk.h>
> #include <linux/completion.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> @@ -31,6 +32,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> +#include <linux/reset.h>
> #include <linux/thermal.h>
> #include <linux/delay.h>
>
> @@ -56,6 +58,7 @@ struct gpadc_data {
> unsigned int tp_adc_select;
> unsigned int (*adc_chan_select)(unsigned int chan);
> unsigned int adc_chan_mask;
> + bool gen2_ths;
> };
>
Instead of a boolean, give the TEMP_DATA register address.
> static const struct gpadc_data sun4i_gpadc_data = {
> @@ -88,7 +91,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
> static const struct gpadc_data sun8i_a33_gpadc_data = {
> .temp_offset = -1662,
> .temp_scale = 162,
> - .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> + .tp_mode_en = SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN,
> +};
Separate patch for this?
> +
> +static const struct gpadc_data sun8i_h3_gpadc_data = {
> + /*
> + * The original formula on the datasheet seems to be wrong.
> + * These factors are calculated based on the formula in the BSP
> + * kernel, which is originally Tem = 217 - (T / 8.253), in which Tem
> + * is the temperature in Celsius degree and T is the raw value
> + * from the sensor.
> + */
> + .temp_offset = -1791,
> + .temp_scale = -121,
> + .gen2_ths = true,
> };
>
> struct sun4i_gpadc_iio {
> @@ -103,6 +119,9 @@ struct sun4i_gpadc_iio {
> atomic_t ignore_temp_data_irq;
> const struct gpadc_data *data;
> bool no_irq;
> + struct clk *ths_bus_clk;
> + struct clk *ths_clk;
> + struct reset_control *reset;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> };
> @@ -274,7 +293,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> if (info->no_irq) {
> pm_runtime_get_sync(indio_dev->dev.parent);
>
> - regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> + if (info->data->gen2_ths)
> + regmap_read(info->regmap, SUN8I_H3_GPADC_TEMP_DATA,
> + val);
> + else
> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
>
Instead of gen2_ths, use the TEMP_DATA register address.
> pm_runtime_mark_last_busy(indio_dev->dev.parent);
> pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -386,10 +409,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>
> - /* Disable the ADC on IP */
> - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
> - /* Disable temperature sensor on IP */
> - regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
> + if (info->data->gen2_ths) {
> + /* Disable temperature sensor */
> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2, 0);
> + } else {
> + /* Disable the ADC on IP */
> + regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
> + /* Disable temperature sensor on IP */
> + regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
> + }
>
Either use another register address or add a suspend function to struct
gpadc_data which will be different for each version of the IP.
> return 0;
> }
> @@ -398,19 +426,36 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
> {
> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>
> - /* clkin = 6MHz */
> - regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> - SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
> - SUN4I_GPADC_CTRL0_FS_DIV(7) |
> - SUN4I_GPADC_CTRL0_T_ACQ(63));
> - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, info->data->tp_mode_en);
> - regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
> - SUN4I_GPADC_CTRL3_FILTER_EN |
> - SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> - /* period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin; ~0.6s */
> - regmap_write(info->regmap, SUN4I_GPADC_TPR,
> - SUN4I_GPADC_TPR_TEMP_ENABLE |
> - SUN4I_GPADC_TPR_TEMP_PERIOD(800));
> + if (info->data->gen2_ths) {
> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2,
> + SUN8I_H3_GPADC_CTRL2_TEMP_SENSE_EN |
> + SUN8I_H3_GPADC_CTRL2_T_ACQ1(31));
> + regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> + SUN4I_GPADC_CTRL0_T_ACQ(31));
> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL3,
> + SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> + regmap_write(info->regmap, SUN8I_H3_GPADC_INTC,
> + SUN8I_H3_GPADC_INTC_TEMP_PERIOD(800));
> + } else {
> + /* clkin = 6MHz */
> + regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> + SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
> + SUN4I_GPADC_CTRL0_FS_DIV(7) |
> + SUN4I_GPADC_CTRL0_T_ACQ(63));
> + regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
> + info->data->tp_mode_en);
> + regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
> + SUN4I_GPADC_CTRL3_FILTER_EN |
> + SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> + /*
> + * period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin;
> + * ~0.6s
> + */
> + regmap_write(info->regmap, SUN4I_GPADC_TPR,
> + SUN4I_GPADC_TPR_TEMP_ENABLE |
> + SUN4I_GPADC_TPR_TEMP_PERIOD(800));
> + }
>
Same here as suspend function?
> return 0;
> }
> @@ -494,6 +539,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
> .compatible = "allwinner,sun8i-a33-ths",
> .data = &sun8i_a33_gpadc_data,
> },
> + {
> + .compatible = "allwinner,sun8i-h3-ths",
> + .data = &sun8i_h3_gpadc_data,
> + },
> { /* sentinel */ }
> };
>
> @@ -529,6 +578,43 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> return ret;
> }
>
> + if (info->data->gen2_ths) {
> + info->reset = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(info->reset)) {
> + ret = PTR_ERR(info->reset);
> + return ret;
> + }
> +
> + ret = reset_control_deassert(info->reset);
> + if (ret)
> + return ret;
> +
> + info->ths_bus_clk = devm_clk_get(&pdev->dev, "bus");
> + if (IS_ERR(info->ths_bus_clk)) {
> + ret = PTR_ERR(info->ths_bus_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(info->ths_bus_clk);
> + if (ret)
> + return ret;
> +
> + info->ths_clk = devm_clk_get(&pdev->dev, "ths");
> + if (IS_ERR(info->ths_clk)) {
> + ret = PTR_ERR(info->ths_clk);
> + return ret;
> + }
> +
> + /* Running at 6MHz */
> + ret = clk_set_rate(info->ths_clk, 6000000);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(info->ths_clk);
> + if (ret)
> + return ret;
> + }
> +
> if (!IS_ENABLED(CONFIG_THERMAL_OF))
> return 0;
>
> @@ -691,6 +777,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
> if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> + if (info->data->gen2_ths) {
> + clk_disable_unprepare(info->ths_clk);
> + clk_disable_unprepare(info->ths_bus_clk);
> + reset_control_deassert(info->reset);
> + }
> +
I'm not really fond of using this boolean as I don't see it being
possibly reused for any other SoCs that has a GPADC or THS.
Here, you could make use of a list/array of clk which then can be reused
for other SoCs just by changing the list. Add a default rate to the
gpadc_data structure and you're go to go.
> return 0;
> }
>
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 139872c2e0fe..f794a2988a93 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -38,9 +38,12 @@
> #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x))
> #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0)
>
> -/* TP_CTRL1 bits for sun8i SoCs */
> -#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
> -#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
> +/* TP_CTRL1 bits for sun8i A23/A33 SoCs */
> +#define SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
> +#define SUN8I_A23_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
> +
Different patch for these?
Thanks,
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
next prev parent reply other threads:[~2017-03-29 6:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-28 17:30 [RFC PATCH 0/3] IIO-based thermal sensor driver for Allwinner H3 SoC Icenowy Zheng
2017-03-28 17:30 ` [RFC PATCH 1/3] dt-bindings: update the Allwinner GPADC device tree binding for H3 Icenowy Zheng
2017-03-28 17:30 ` [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor Icenowy Zheng
2017-03-29 6:54 ` Quentin Schulz [this message]
2017-04-02 10:51 ` Jonathan Cameron
2017-03-28 17:30 ` [RFC PATCH 3/3] ARM: sun8i: h3: add support for the thermal sensor in H3 Icenowy Zheng
[not found] <20170329065717.D0AF37C1EFF@relay.mailchannels.net>
2017-03-29 7:54 ` [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor Lee Jones
2017-03-29 12:28 ` Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=813dc1d8-89b4-9c0e-c1cc-e76efde2be1f@free-electrons.com \
--to=quentin.schulz@free-electrons.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).