LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Yangtao Li <tiny.windzz@gmail.com>,
	lee.jones@linaro.org, robh+dt@kernel.org, mark.rutland@arm.com,
	wens@csie.org, knaack.h@gmx.de, lars@metafoo.de,
	pmeerw@pmeerw.net, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
Subject: Re: [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple thermal sensor
Date: Mon, 6 May 2019 14:28:07 +0200	[thread overview]
Message-ID: <20190506122807.4u323iys74jddcet@flea> (raw)
In-Reply-To: <20190505162215.3594f77d@archlinux>

[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]

Hi,

On Sun, May 05, 2019 at 04:22:15PM +0100, Jonathan Cameron wrote:
> On Fri,  3 May 2019 03:28:07 -0400
> Yangtao Li <tiny.windzz@gmail.com> wrote:
>
> > For some SOCs, there are more than one thermal sensor, and there are
> > currently four sensors on the A80. So we need to do some work in order
> > to support multiple thermal sensors:
> >
> >   1) add sensor_count in gpadc_data.
> >   2) introduce sun4i_sensor_tzd in sun4i_gpadc_iio, to support multiple
> >      thermal_zone_device and distinguish between different sensors.
> >   3) modify read temperature and initialization function.
>
> This comment doesn't mention the devm change. If it had it would have
> raised immediate alarm bells.
>
> I'm also not keen on the web of pointers that this driver is steadily
> evolving.  I can't immediately see how to reduce that complexity however.

So I might be responsible for that, and looking back, this has been a
mistake.

This driver was initally put together to support a controller found in
older (A10 up to A31) Allwinner SoCs. This controller had an ADC
driver that could be operated as a touchscreen controller, and was
providing a CPU temperature sensor and a general purpose ADC.

However, we already had a driver for that controller in drivers/input
to report the CPU temperature, and the one in IIO was introduced to
support the general purpose ADC (and the CPU temperature). The long
term goal was to add the touchscreen feature as well eventually so
that we could remove the one in drivers/input. That didn't happen.

At the same time, the Allwinner hardware slowly evolved to remove the
touchscreen and ADC features, and only keep the CPU temperature
readout. It then evolved further on to support multiple temperatures
(for different clusters, the GPU, and so on).

So, today, we're in a situation where I was pushing everything into
that IIO drivers since there was similiraties between all the
generations, but the fact that we have to support so many odd cases
(DT bindings compatibility, controllers with and without ADC, etc)
that it becomes a real mess.

And that mess isn't really used by anybody, since we want to have the
touchscreen.

There's only one SoC that is supported only by that driver, which is
the A33 that only had a CPU temperature readout, and is still pretty
similar to the latest SoC from Allwinner (that is supported by this
series).

I guess, for everyone's sanity and in order to not stall this further,
it would just be better to create an hwmon driver for the A33 (and
onwards, including the H6) for the SoC that just have the temperature
readout feature. And for the older SoC, we just keep the older driver
under input/. Once the A33 is supported, we'll remove the driver in
IIO (and the related bits in drivers/mfd).

Armbian already has a driver for that they never upstreamed iirc, so
it might be a good starting point, and we would add the support for
the H6. How does that sound?

Sorry for wasting everybody's time on this.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2019-05-06 12:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-03  7:28 [PATCH 0/7] Add support for H6 " Yangtao Li
2019-05-03  7:28 ` [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple " Yangtao Li
2019-05-05 15:22   ` Jonathan Cameron
2019-05-06 12:28     ` Maxime Ripard [this message]
2019-05-06 16:52       ` Icenowy Zheng
2019-05-06 17:08         ` Frank Lee
2019-05-06 17:55           ` Ondřej Jirman
2019-05-07 13:59             ` Maxime Ripard
2019-05-03  7:28 ` [PATCH 2/7] iio: adc: sun4i-gpadc: introduce temp_data in gpadc_data Yangtao Li
2019-05-03  7:28 ` [PATCH 3/7] iio: adc: sun4i-gpadc: introduce gpadc_enable and gpadc_disable " Yangtao Li
2019-05-03  7:28 ` [PATCH 4/7] iio: adc: sun4i-gpadc-iio: support clocks and reset Yangtao Li
2019-05-03  7:28 ` [PATCH 5/7] dt-bindings: mfd: Add H6 GPADC binding Yangtao Li
2019-05-08 11:44   ` Lee Jones
2019-05-03  7:28 ` [PATCH 6/7] iio: adc: sun4i-gpadc-iio: add support for H6 thermal sensor Yangtao Li
2019-05-05 15:25   ` Jonathan Cameron
2019-05-08 11:45   ` Lee Jones
2019-05-03  7:28 ` [PATCH 7/7] iio: adc: sun4i-gpadc-iio convert to SPDX license tags Yangtao Li
2019-05-03  9:18   ` 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=20190506122807.4u323iys74jddcet@flea \
    --to=maxime.ripard@bootlin.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pmeerw@pmeerw.net \
    --cc=robh+dt@kernel.org \
    --cc=tiny.windzz@gmail.com \
    --cc=wens@csie.org \
    --subject='Re: [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple thermal sensor' \
    /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

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).