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

在 2019-05-06一的 14:28 +0200,Maxime Ripard写道:
> 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).

I think a thermal driver is better.

Other SoCs' thermal sensor drivers are all thermal drivers.

> 
> 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?

I think the developer abandoned to upstream it because of the previous
problem ;-)

Maybe it can be taken and add A33&H6 support.

> 
> Sorry for wasting everybody's time on this.
> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


  reply	other threads:[~2019-05-06 16:52 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
2019-05-06 16:52       ` Icenowy Zheng [this message]
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=282ccf0979e6c58effd0e177917bdf824c32f64e.camel@aosc.io \
    --to=icenowy@aosc.io \
    --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=maxime.ripard@bootlin.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).