diff mbox series

[1/3] iio: adc: sun4i-gpadc-iio: no temp sensor on R40

Message ID 20211119191456.510435-2-boger@wirenboard.com (mailing list archive)
State New, archived
Headers show
Series Enable RTP (GPADC) controller on Allwinner R40 | expand

Commit Message

Evgeny Boger Nov. 19, 2021, 7:14 p.m. UTC 
R40 SoC has touchscreen controller also serving as general-purpose ADC.
The hardware is very similar to A31, except that R40 has separate
calibrated thermal sensor IP (handled by sun8i_thermal).

Despite the temperature sensor in the RTP is never mentioned in
the R40 family user manuals, it appears to be working. However,
it's not very useful as it lacks calibration data and there is another
fully functioning temperature sensor anyway.

This patch disables the temperature sensor in RTP/GPADC IP on R40.

The reason for disabling the temperature sensor is that the IP
needs to be switched back and forth between RTP and GPADC modes for
temperature measurements. Not only this introduces delays, but it also
disturbs external circuitry by injecting current into ADC inputs.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 32 +++++++++++++++++++++++--------
 drivers/mfd/sun4i-gpadc.c         | 17 ++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

Comments

Maxime Ripard Nov. 22, 2021, 9:11 a.m. UTC | #1 
On Fri, Nov 19, 2021 at 10:14:54PM +0300, Evgeny Boger wrote:
> R40 SoC has touchscreen controller also serving as general-purpose ADC.
> The hardware is very similar to A31, except that R40 has separate
> calibrated thermal sensor IP (handled by sun8i_thermal).
> 
> Despite the temperature sensor in the RTP is never mentioned in
> the R40 family user manuals, it appears to be working. However,
> it's not very useful as it lacks calibration data and there is another
> fully functioning temperature sensor anyway.
> 
> This patch disables the temperature sensor in RTP/GPADC IP on R40.
> 
> The reason for disabling the temperature sensor is that the IP
> needs to be switched back and forth between RTP and GPADC modes for
> temperature measurements. Not only this introduces delays, but it also
> disturbs external circuitry by injecting current into ADC inputs.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime
Jonathan Cameron Nov. 22, 2021, 12:41 p.m. UTC | #2 
On Mon, 22 Nov 2021 10:11:32 +0100
Maxime Ripard <maxime@cerno.tech> wrote:

> On Fri, Nov 19, 2021 at 10:14:54PM +0300, Evgeny Boger wrote:
> > R40 SoC has touchscreen controller also serving as general-purpose ADC.
> > The hardware is very similar to A31, except that R40 has separate
> > calibrated thermal sensor IP (handled by sun8i_thermal).
> > 
> > Despite the temperature sensor in the RTP is never mentioned in
> > the R40 family user manuals, it appears to be working. However,
> > it's not very useful as it lacks calibration data and there is another
> > fully functioning temperature sensor anyway.
> > 
> > This patch disables the temperature sensor in RTP/GPADC IP on R40.
> > 
> > The reason for disabling the temperature sensor is that the IP
> > needs to be switched back and forth between RTP and GPADC modes for
> > temperature measurements. Not only this introduces delays, but it also
> > disturbs external circuitry by injecting current into ADC inputs.
> > 
> > Signed-off-by: Evgeny Boger <boger@wirenboard.com>  
> 
> Acked-by: Maxime Ripard <maxime@cerno.tech>

+Cc linux-iio@vger.kernel.org

I'm fine with this and assuming Lee is happy to pick it up + send
me an immutable branch in case anything crosses with it this cycle..

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks,

Jonathan

> 
> Maxime
>
Evgeny Boger Jan. 11, 2022, 1:13 p.m. UTC | #3 
22.11.2021 15:41, Jonathan Cameron пишет:
> On Mon, 22 Nov 2021 10:11:32 +0100
> Maxime Ripard <maxime@cerno.tech> wrote:
>
>> On Fri, Nov 19, 2021 at 10:14:54PM +0300, Evgeny Boger wrote:
>>> R40 SoC has touchscreen controller also serving as general-purpose ADC.
>>> The hardware is very similar to A31, except that R40 has separate
>>> calibrated thermal sensor IP (handled by sun8i_thermal).
>>>
>>> Despite the temperature sensor in the RTP is never mentioned in
>>> the R40 family user manuals, it appears to be working. However,
>>> it's not very useful as it lacks calibration data and there is another
>>> fully functioning temperature sensor anyway.
>>>
>>> This patch disables the temperature sensor in RTP/GPADC IP on R40.
>>>
>>> The reason for disabling the temperature sensor is that the IP
>>> needs to be switched back and forth between RTP and GPADC modes for
>>> temperature measurements. Not only this introduces delays, but it also
>>> disturbs external circuitry by injecting current into ADC inputs.
>>>
>>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>> Acked-by: Maxime Ripard <maxime@cerno.tech>
> +Cc linux-iio@vger.kernel.org
>
> I'm fine with this and assuming Lee is happy to pick it up + send
> me an immutable branch in case anything crosses with it this cycle..
>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Hi Jonathan!

Is there anything else I should do to make this patch series accepted?


> Thanks,
>
> Jonathan
>
>> Maxime
>>
Jonathan Cameron Jan. 15, 2022, 3:41 p.m. UTC | #4 
On Tue, 11 Jan 2022 16:13:02 +0300
Evgeny Boger <boger@wirenboard.com> wrote:

> 22.11.2021 15:41, Jonathan Cameron пишет:
> > On Mon, 22 Nov 2021 10:11:32 +0100
> > Maxime Ripard <maxime@cerno.tech> wrote:
> >  
> >> On Fri, Nov 19, 2021 at 10:14:54PM +0300, Evgeny Boger wrote:  
> >>> R40 SoC has touchscreen controller also serving as general-purpose ADC.
> >>> The hardware is very similar to A31, except that R40 has separate
> >>> calibrated thermal sensor IP (handled by sun8i_thermal).
> >>>
> >>> Despite the temperature sensor in the RTP is never mentioned in
> >>> the R40 family user manuals, it appears to be working. However,
> >>> it's not very useful as it lacks calibration data and there is another
> >>> fully functioning temperature sensor anyway.
> >>>
> >>> This patch disables the temperature sensor in RTP/GPADC IP on R40.
> >>>
> >>> The reason for disabling the temperature sensor is that the IP
> >>> needs to be switched back and forth between RTP and GPADC modes for
> >>> temperature measurements. Not only this introduces delays, but it also
> >>> disturbs external circuitry by injecting current into ADC inputs.
> >>>
> >>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>  
> >> Acked-by: Maxime Ripard <maxime@cerno.tech>  
> > +Cc linux-iio@vger.kernel.org
> >
> > I'm fine with this and assuming Lee is happy to pick it up + send
> > me an immutable branch in case anything crosses with it this cycle..
> >
> > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Hi Jonathan!
> 
> Is there anything else I should do to make this patch series accepted?

Perhaps try a resend to Lee Jones, making sure to cc linux-iio
Chances are it just got missed in the deluge!

Jonathan


> 
> 
> > Thanks,
> >
> > Jonathan
> >  
> >> Maxime
> >>  
>
Lee Jones Jan. 17, 2022, 9 a.m. UTC | #5 
On Fri, 19 Nov 2021, Evgeny Boger wrote:

> R40 SoC has touchscreen controller also serving as general-purpose ADC.
> The hardware is very similar to A31, except that R40 has separate
> calibrated thermal sensor IP (handled by sun8i_thermal).
> 
> Despite the temperature sensor in the RTP is never mentioned in
> the R40 family user manuals, it appears to be working. However,
> it's not very useful as it lacks calibration data and there is another
> fully functioning temperature sensor anyway.
> 
> This patch disables the temperature sensor in RTP/GPADC IP on R40.
> 
> The reason for disabling the temperature sensor is that the IP
> needs to be switched back and forth between RTP and GPADC modes for
> temperature measurements. Not only this introduces delays, but it also
> disturbs external circuitry by injecting current into ADC inputs.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 32 +++++++++++++++++++++++--------
>  drivers/mfd/sun4i-gpadc.c         | 17 ++++++++++++++++

Please split these out.

They don't *need* to go in via a single tree.

>  2 files changed, 41 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 2d393a4dfff6..e63a45c092ab 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -53,6 +53,7 @@  struct gpadc_data {
 	unsigned int	tp_adc_select;
 	unsigned int	(*adc_chan_select)(unsigned int chan);
 	unsigned int	adc_chan_mask;
+	bool            has_temp_sensor;
 };
 
 static const struct gpadc_data sun4i_gpadc_data = {
@@ -62,6 +63,7 @@  static const struct gpadc_data sun4i_gpadc_data = {
 	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun4i_gpadc_chan_select,
 	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
+	.has_temp_sensor = true,
 };
 
 static const struct gpadc_data sun5i_gpadc_data = {
@@ -71,6 +73,7 @@  static const struct gpadc_data sun5i_gpadc_data = {
 	.tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun4i_gpadc_chan_select,
 	.adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK,
+	.has_temp_sensor = true,
 };
 
 static const struct gpadc_data sun6i_gpadc_data = {
@@ -80,12 +83,24 @@  static const struct gpadc_data sun6i_gpadc_data = {
 	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
 	.adc_chan_select = &sun6i_gpadc_chan_select,
 	.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
+	.has_temp_sensor = true,
+};
+
+static const struct gpadc_data sun8i_r40_gpadc_data = {
+	.temp_offset = -1623,
+	.temp_scale = 167,
+	.tp_mode_en = SUN6I_GPADC_CTRL1_TP_MODE_EN,
+	.tp_adc_select = SUN6I_GPADC_CTRL1_TP_ADC_SELECT,
+	.adc_chan_select = &sun6i_gpadc_chan_select,
+	.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
+	.has_temp_sensor = false,
 };
 
 static const struct gpadc_data sun8i_a33_gpadc_data = {
 	.temp_offset = -1662,
 	.temp_scale = 162,
 	.tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+	.has_temp_sensor = true,
 };
 
 struct sun4i_gpadc_iio {
@@ -499,7 +514,7 @@  static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 	void __iomem *base;
 	int ret;
 
-	info->data = of_device_get_match_data(&pdev->dev);
+	info->data = (struct gpadc_data *)of_device_get_match_data(&pdev->dev);
 	if (!info->data)
 		return -ENODEV;
 
@@ -519,7 +534,7 @@  static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
 		return ret;
 	}
 
-	if (IS_ENABLED(CONFIG_THERMAL_OF))
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor)
 		info->sensor_device = &pdev->dev;
 
 	return 0;
@@ -552,7 +567,7 @@  static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
 	 * that choice to the user.
 	 */
 
-	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor) {
 		/*
 		 * This driver is a child of an MFD which has a node in the DT
 		 * but not its children, because of DT backward compatibility
@@ -575,7 +590,7 @@  static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
 		indio_dev->channels = sun4i_gpadc_channels_no_temp;
 	}
 
-	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor) {
 		ret = sun4i_irq_init(pdev, "TEMP_DATA_PENDING",
 				     sun4i_gpadc_temp_data_irq_handler,
 				     "temp_data", &info->temp_data_irq,
@@ -590,7 +605,7 @@  static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
 	if (ret < 0)
 		return ret;
 
-	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor) {
 		ret = iio_map_array_register(indio_dev, sun4i_gpadc_hwmon_maps);
 		if (ret < 0) {
 			dev_err(&pdev->dev,
@@ -636,7 +651,7 @@  static int sun4i_gpadc_probe(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
-	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor) {
 		info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
 							    0, info,
 							    &sun4i_ts_tz_ops);
@@ -661,7 +676,7 @@  static int sun4i_gpadc_probe(struct platform_device *pdev)
 	return 0;
 
 err_map:
-	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
+	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF) && info->data->has_temp_sensor)
 		iio_map_array_unregister(indio_dev);
 
 	pm_runtime_put(&pdev->dev);
@@ -678,7 +693,7 @@  static int sun4i_gpadc_remove(struct platform_device *pdev)
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	if (!IS_ENABLED(CONFIG_THERMAL_OF))
+	if (!IS_ENABLED(CONFIG_THERMAL_OF) || !info->data->has_temp_sensor)
 		return 0;
 
 	thermal_zone_of_sensor_unregister(info->sensor_device, info->tzd);
@@ -693,6 +708,7 @@  static const struct platform_device_id sun4i_gpadc_id[] = {
 	{ "sun4i-a10-gpadc-iio", (kernel_ulong_t)&sun4i_gpadc_data },
 	{ "sun5i-a13-gpadc-iio", (kernel_ulong_t)&sun5i_gpadc_data },
 	{ "sun6i-a31-gpadc-iio", (kernel_ulong_t)&sun6i_gpadc_data },
+	{ "sun8i-r40-gpadc-iio", (kernel_ulong_t)&sun8i_r40_gpadc_data },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(platform, sun4i_gpadc_id);
diff --git a/drivers/mfd/sun4i-gpadc.c b/drivers/mfd/sun4i-gpadc.c
index cfe14d9bf6dc..293acad52e5a 100644
--- a/drivers/mfd/sun4i-gpadc.c
+++ b/drivers/mfd/sun4i-gpadc.c
@@ -17,6 +17,7 @@ 
 #define ARCH_SUN4I_A10 0
 #define ARCH_SUN5I_A13 1
 #define ARCH_SUN6I_A31 2
+#define ARCH_SUN8I_R40 3
 
 static const struct resource adc_resources[] = {
 	DEFINE_RES_IRQ_NAMED(SUN4I_GPADC_IRQ_FIFO_DATA, "FIFO_DATA_PENDING"),
@@ -69,6 +70,15 @@  static struct mfd_cell sun6i_gpadc_cells[] = {
 	{ .name = "iio_hwmon" },
 };
 
+static struct mfd_cell sun8i_r40_gpadc_cells[] = {
+	{
+		.name	= "sun8i-r40-gpadc-iio",
+		.resources = adc_resources,
+		.num_resources = ARRAY_SIZE(adc_resources),
+	},
+	{ .name = "iio_hwmon" },
+};
+
 static const struct regmap_config sun4i_gpadc_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,
@@ -86,6 +96,9 @@  static const struct of_device_id sun4i_gpadc_of_match[] = {
 	}, {
 		.compatible = "allwinner,sun6i-a31-ts",
 		.data = (void *)ARCH_SUN6I_A31,
+	}, {
+		.compatible = "allwinner,sun8i-r40-ts",
+		.data = (void *)ARCH_SUN8I_R40,
 	}, { /* sentinel */ }
 };
 
@@ -117,6 +130,10 @@  static int sun4i_gpadc_probe(struct platform_device *pdev)
 		cells = sun6i_gpadc_cells;
 		size = ARRAY_SIZE(sun6i_gpadc_cells);
 		break;
+	case ARCH_SUN8I_R40:
+		cells = sun8i_r40_gpadc_cells;
+		size = ARRAY_SIZE(sun8i_r40_gpadc_cells);
+		break;
 	default:
 		return -EINVAL;
 	}